diff --git a/agent/consul/config_endpoint.go b/agent/consul/config_endpoint.go index c0db5431eb..3d3c0ff354 100644 --- a/agent/consul/config_endpoint.go +++ b/agent/consul/config_endpoint.go @@ -444,6 +444,11 @@ func (c *ConfigEntry) ResolveServiceConfig(args *structs.ServiceConfigRequest, r func (c *ConfigEntry) preflightCheck(kind string) error { switch kind { case structs.ServiceIntentions: + // Exit early if Connect hasn't been enabled. + if !c.srv.config.ConnectEnabled { + return ErrConnectNotEnabled + } + usingConfigEntries, err := c.srv.fsm.State().AreIntentionsInConfigEntries() if err != nil { return fmt.Errorf("system metadata lookup failed: %v", err) diff --git a/agent/consul/intention_endpoint.go b/agent/consul/intention_endpoint.go index 3ff26c7222..9b1931e00f 100644 --- a/agent/consul/intention_endpoint.go +++ b/agent/consul/intention_endpoint.go @@ -59,6 +59,11 @@ func (s *Intention) legacyUpgradeCheck() error { // Apply creates or updates an intention in the data store. func (s *Intention) Apply(args *structs.IntentionRequest, reply *string) error { + // Exit early if Connect hasn't been enabled. + if !s.srv.config.ConnectEnabled { + return ErrConnectNotEnabled + } + // Ensure that all service-intentions config entry writes go to the primary // datacenter. These will then be replicated to all the other datacenters. args.Datacenter = s.srv.config.PrimaryDatacenter @@ -402,9 +407,12 @@ func (s *Intention) computeApplyChangesDelete( } // Get returns a single intention by ID. -func (s *Intention) Get( - args *structs.IntentionQueryRequest, - reply *structs.IndexedIntentions) error { +func (s *Intention) Get(args *structs.IntentionQueryRequest, reply *structs.IndexedIntentions) error { + // Exit early if Connect hasn't been enabled. + if !s.srv.config.ConnectEnabled { + return ErrConnectNotEnabled + } + // Forward if necessary if done, err := s.srv.ForwardRPC("Intention.Get", args, args, reply); done { return err @@ -477,9 +485,12 @@ func (s *Intention) Get( } // List returns all the intentions. -func (s *Intention) List( - args *structs.IntentionListRequest, - reply *structs.IndexedIntentions) error { +func (s *Intention) List(args *structs.IntentionListRequest, reply *structs.IndexedIntentions) error { + // Exit early if Connect hasn't been enabled. + if !s.srv.config.ConnectEnabled { + return ErrConnectNotEnabled + } + // Forward if necessary if done, err := s.srv.ForwardRPC("Intention.List", args, args, reply); done { return err @@ -544,9 +555,12 @@ func (s *Intention) List( } // Match returns the set of intentions that match the given source/destination. -func (s *Intention) Match( - args *structs.IntentionQueryRequest, - reply *structs.IndexedIntentionMatches) error { +func (s *Intention) Match(args *structs.IntentionQueryRequest, reply *structs.IndexedIntentionMatches) error { + // Exit early if Connect hasn't been enabled. + if !s.srv.config.ConnectEnabled { + return ErrConnectNotEnabled + } + // Forward if necessary if done, err := s.srv.ForwardRPC("Intention.Match", args, args, reply); done { return err @@ -615,9 +629,12 @@ func (s *Intention) Match( // Note: Whenever the logic for this method is changed, you should take // a look at the agent authorize endpoint (agent/agent_endpoint.go) since // the logic there is similar. -func (s *Intention) Check( - args *structs.IntentionQueryRequest, - reply *structs.IntentionQueryCheckResponse) error { +func (s *Intention) Check(args *structs.IntentionQueryRequest, reply *structs.IntentionQueryCheckResponse) error { + // Exit early if Connect hasn't been enabled. + if !s.srv.config.ConnectEnabled { + return ErrConnectNotEnabled + } + // Forward maybe if done, err := s.srv.ForwardRPC("Intention.Check", args, args, reply); done { return err diff --git a/agent/consul/leader.go b/agent/consul/leader.go index 6fba3af672..a1d90131ae 100644 --- a/agent/consul/leader.go +++ b/agent/consul/leader.go @@ -1017,18 +1017,32 @@ func (s *Server) bootstrapConfigEntries(entries []structs.ConfigEntry) error { state := s.fsm.State() - // Do a quick preflight check to see if someone is trying to upgrade from - // an older pre-1.9.0 version of consul with intentions AND are trying to - // bootstrap a service-intentions config entry at the same time. + // Do some quick preflight checks to see if someone is doing something + // that's not allowed at this time: + // + // - Trying to upgrade from an older pre-1.9.0 version of consul with + // intentions AND are trying to bootstrap a service-intentions config entry + // at the same time. + // + // - Trying to insert service-intentions config entries when connect is + // disabled. + usingConfigEntries, err := s.fsm.State().AreIntentionsInConfigEntries() if err != nil { return fmt.Errorf("Failed to determine if we are migrating intentions yet: %v", err) } - if !usingConfigEntries { + + if !usingConfigEntries || !s.config.ConnectEnabled { for _, entry := range entries { if entry.GetKind() == structs.ServiceIntentions { - return fmt.Errorf("Refusing to apply configuration entry %q / %q because intentions are still being migrated to config entries: %v", - entry.GetKind(), entry.GetName(), err) + if !s.config.ConnectEnabled { + return fmt.Errorf("Refusing to apply configuration entry %q / %q because Connect must be enabled to bootstrap intentions", + entry.GetKind(), entry.GetName()) + } + if !usingConfigEntries { + return fmt.Errorf("Refusing to apply configuration entry %q / %q because intentions are still being migrated to config entries", + entry.GetKind(), entry.GetName()) + } } } } diff --git a/agent/consul/leader_test.go b/agent/consul/leader_test.go index 0d56b8ac48..60f817a8d3 100644 --- a/agent/consul/leader_test.go +++ b/agent/consul/leader_test.go @@ -1230,63 +1230,133 @@ func TestLeader_ConfigEntryBootstrap(t *testing.T) { func TestLeader_ConfigEntryBootstrap_Fail(t *testing.T) { t.Parallel() - pr, pw := io.Pipe() - defer pw.Close() + type testcase struct { + name string + entries []structs.ConfigEntry + serverCB func(c *Config) + expectMessage string + } - ch := make(chan string, 1) - go func() { - defer pr.Close() - scan := bufio.NewScanner(pr) - for scan.Scan() { - line := scan.Text() - - if strings.Contains(line, "failed to establish leadership") { - ch <- "" - return - } - if strings.Contains(line, "successfully established leadership") { - ch <- "leadership should not have gotten here if config entries properly failed" - return - } - } - - if scan.Err() != nil { - ch <- fmt.Sprintf("ERROR: %v", scan.Err()) - } else { - ch <- "should not get here" - } - }() - - _, config := testServerConfig(t) - config.Build = "1.6.0" - config.ConfigEntryBootstrap = []structs.ConfigEntry{ - &structs.ServiceSplitterConfigEntry{ - Kind: structs.ServiceSplitter, - Name: "web", - Splits: []structs.ServiceSplit{ - {Weight: 100, Service: "web"}, + cases := []testcase{ + { + name: "service-splitter without L7 protocol", + entries: []structs.ConfigEntry{ + &structs.ServiceSplitterConfigEntry{ + Kind: structs.ServiceSplitter, + Name: "web", + Splits: []structs.ServiceSplit{ + {Weight: 100, Service: "web"}, + }, + }, }, + expectMessage: `Failed to apply configuration entry "service-splitter" / "web": discovery chain "web" uses a protocol "tcp" that does not permit advanced routing or splitting behavior"`, + }, + { + name: "service-intentions without migration", + entries: []structs.ConfigEntry{ + &structs.ServiceIntentionsConfigEntry{ + Kind: structs.ServiceIntentions, + Name: "web", + Sources: []*structs.SourceIntention{ + { + Name: "debug", + Action: structs.IntentionActionAllow, + }, + }, + }, + }, + serverCB: func(c *Config) { + c.OverrideInitialSerfTags = func(tags map[string]string) { + tags["ft_si"] = "0" + } + }, + expectMessage: `Refusing to apply configuration entry "service-intentions" / "web" because intentions are still being migrated to config entries`, + }, + { + name: "service-intentions without Connect", + entries: []structs.ConfigEntry{ + &structs.ServiceIntentionsConfigEntry{ + Kind: structs.ServiceIntentions, + Name: "web", + Sources: []*structs.SourceIntention{ + { + Name: "debug", + Action: structs.IntentionActionAllow, + }, + }, + }, + }, + serverCB: func(c *Config) { + c.ConnectEnabled = false + }, + expectMessage: `Refusing to apply configuration entry "service-intentions" / "web" because Connect must be enabled to bootstrap intentions"`, }, } - logger := hclog.NewInterceptLogger(&hclog.LoggerOptions{ - Name: config.NodeName, - Level: hclog.Debug, - Output: io.MultiWriter(pw, testutil.NewLogBuffer(t)), - }) + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + pr, pw := io.Pipe() + defer pw.Close() - deps := newDefaultDeps(t, config) - deps.Logger = logger + var ( + ch = make(chan string, 1) + applyErrorLine string + ) + go func() { + defer pr.Close() + scan := bufio.NewScanner(pr) + for scan.Scan() { + line := scan.Text() - srv, err := NewServer(config, deps) - require.NoError(t, err) - defer srv.Shutdown() + if strings.Contains(line, "failed to establish leadership") { + applyErrorLine = line + ch <- "" + return + } + if strings.Contains(line, "successfully established leadership") { + ch <- "leadership should not have gotten here if config entries properly failed" + return + } + } - select { - case result := <-ch: - require.Empty(t, result) - case <-time.After(time.Second): - t.Fatal("timeout waiting for a result from tailing logs") + if scan.Err() != nil { + ch <- fmt.Sprintf("ERROR: %v", scan.Err()) + } else { + ch <- "should not get here" + } + }() + + _, config := testServerConfig(t) + config.Build = "1.6.0" + config.ConfigEntryBootstrap = tc.entries + if tc.serverCB != nil { + tc.serverCB(config) + } + + logger := hclog.NewInterceptLogger(&hclog.LoggerOptions{ + Name: config.NodeName, + Level: hclog.Debug, + Output: io.MultiWriter(pw, testutil.NewLogBuffer(t)), + }) + + deps := newDefaultDeps(t, config) + deps.Logger = logger + + srv, err := NewServer(config, deps) + require.NoError(t, err) + defer srv.Shutdown() + + select { + case result := <-ch: + require.Empty(t, result) + if tc.expectMessage != "" { + require.Contains(t, applyErrorLine, tc.expectMessage) + } + case <-time.After(time.Second): + t.Fatal("timeout waiting for a result from tailing logs") + } + }) } }