server: intentions CRUD requires connect to be enabled (#9194)

Fixes #9123

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)

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

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())
+				}
 			}
 		}
 	}

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")
+			}
+		})
 	}
 }