Disable peering queries by default in proxycfg (#17731)

This PR disables peering functionality in proxycfg by default, unless `peering.enabled = true` is explicitly set in the agent config. This change was necessary to prevent expensive polling watch queries (that were introduced for peer upstreams) from impacting users that do not wish to leverage peering features. While this is technically a breaking change, the 1.13 implementation of peering is considered to be experimental and not production ready. After this PR is merged, any users that wish to use peering in conjunction with mesh will need to enable peering on each agent and server.
hashicorp · Jun 14, 2023 · ffdfe04 · ffdfe04
1 parent 80aea95
commit ffdfe04
Show file tree

Hide file tree

Showing 18 changed files with 131 additions and 102 deletions.
diff --git a/.changelog/17731.txt b/.changelog/17731.txt
@@ -0,0 +1,7 @@
+```release-note:breaking-change
+connect: Disable peering by default in connect proxies for Consul 1.13. This change was made to prevent inefficient polling
+queries from having a negative impact on server performance. Peering in Consul 1.13 is an experimental feature and is not
+recommended for use in production environments. If you still wish to use the experimental peering feature, ensure
+[`peering.enabled = true`](https://developer.hashicorp.com/consul/docs/v1.13.x/agent/config/config-files#peering_enabled)
+is set on all clients and servers.
+```
diff --git a/agent/agent.go b/agent/agent.go
@@ -646,6 +646,7 @@ func (a *Agent) Start(ctx context.Context) error {
 		},
 		TLSConfigurator:       a.tlsConfigurator,
 		IntentionDefaultAllow: intentionDefaultAllow,
+		PeeringEnabled:        a.config.PeeringEnabled,
 	})
 	if err != nil {
 		return err

diff --git a/agent/proxycfg/connect_proxy.go b/agent/proxycfg/connect_proxy.go
@@ -46,16 +46,18 @@ func (s *handlerConnectProxy) initialize(ctx context.Context) (ConfigSnapshot, e
 		return snap, err
 	}
 
-	err = s.dataSources.TrustBundleList.Notify(ctx, &cachetype.TrustBundleListRequest{
-		Request: &pbpeering.TrustBundleListByServiceRequest{
-			ServiceName: s.proxyCfg.DestinationServiceName,
-			Namespace:   s.proxyID.NamespaceOrDefault(),
-			Partition:   s.proxyID.PartitionOrDefault(),
-		},
-		QueryOptions: structs.QueryOptions{Token: s.token},
-	}, peeringTrustBundlesWatchID, s.ch)
-	if err != nil {
-		return snap, err
+	if s.peeringEnabled {
+		err = s.dataSources.TrustBundleList.Notify(ctx, &cachetype.TrustBundleListRequest{
+			Request: &pbpeering.TrustBundleListByServiceRequest{
+				ServiceName: s.proxyCfg.DestinationServiceName,
+				Namespace:   s.proxyID.NamespaceOrDefault(),
+				Partition:   s.proxyID.PartitionOrDefault(),
+			},
+			QueryOptions: structs.QueryOptions{Token: s.token},
+		}, peeringTrustBundlesWatchID, s.ch)
+		if err != nil {
+			return snap, err
+		}
 	}
 
 	// Watch the leaf cert
@@ -112,13 +114,15 @@ func (s *handlerConnectProxy) initialize(ctx context.Context) (ConfigSnapshot, e
 		if err != nil {
 			return snap, err
 		}
-		err = s.dataSources.PeeredUpstreams.Notify(ctx, &structs.PartitionSpecificRequest{
-			QueryOptions:   structs.QueryOptions{Token: s.token},
-			Datacenter:     s.source.Datacenter,
-			EnterpriseMeta: s.proxyID.EnterpriseMeta,
-		}, peeredUpstreamsID, s.ch)
-		if err != nil {
-			return snap, err
+		if s.peeringEnabled {
+			err = s.dataSources.PeeredUpstreams.Notify(ctx, &structs.PartitionSpecificRequest{
+				QueryOptions:   structs.QueryOptions{Token: s.token},
+				Datacenter:     s.source.Datacenter,
+				EnterpriseMeta: s.proxyID.EnterpriseMeta,
+			}, peeredUpstreamsID, s.ch)
+			if err != nil {
+				return snap, err
+			}
 		}
 		// We also infer upstreams from destinations (egress points)
 		err = s.dataSources.IntentionUpstreamsDestination.Notify(ctx, &structs.ServiceSpecificRequest{
@@ -194,7 +198,7 @@ func (s *handlerConnectProxy) initialize(ctx context.Context) (ConfigSnapshot, e
 			fallthrough
 
 		case "":
-			if u.DestinationPeer != "" {
+			if u.DestinationPeer != "" && s.peeringEnabled {
 				// NOTE: An upstream that points to a peer by definition will
 				// only ever watch a single catalog query, so a map key of just
 				// "UID" is sufficient to cover the peer data watches here.
@@ -285,7 +289,7 @@ func (s *handlerConnectProxy) handleUpdate(ctx context.Context, u UpdateEvent, s
 			snap.ConnectProxy.UpstreamPeerTrustBundles.Set(peer, resp.Bundle)
 		}
 
-	case u.CorrelationID == peeringTrustBundlesWatchID:
+	case u.CorrelationID == peeringTrustBundlesWatchID && s.peeringEnabled:
 		resp, ok := u.Result.(*pbpeering.TrustBundleListByServiceResponse)
 		if !ok {
 			return fmt.Errorf("invalid type for response: %T", u.Result)
@@ -303,7 +307,7 @@ func (s *handlerConnectProxy) handleUpdate(ctx context.Context, u UpdateEvent, s
 		snap.ConnectProxy.Intentions = resp
 		snap.ConnectProxy.IntentionsSet = true
 
-	case u.CorrelationID == peeredUpstreamsID:
+	case u.CorrelationID == peeredUpstreamsID && s.peeringEnabled:
 		resp, ok := u.Result.(*structs.IndexedPeeredServiceList)
 		if !ok {
 			return fmt.Errorf("invalid type for response %T", u.Result)

diff --git a/agent/proxycfg/manager.go b/agent/proxycfg/manager.go
@@ -75,6 +75,8 @@ type ManagerConfig struct {
 	// information to proxies that need to make intention decisions on their
 	// own.
 	IntentionDefaultAllow bool
+
+	PeeringEnabled bool
 }
 
 // NewManager constructs a Manager.
@@ -137,6 +139,7 @@ func (m *Manager) Register(id ProxyID, ns *structs.NodeService, source ProxySour
 		source:                m.Source,
 		dnsConfig:             m.DNSConfig,
 		intentionDefaultAllow: m.IntentionDefaultAllow,
+		peeringEnabled:        m.PeeringEnabled,
 	}
 	if m.TLSConfigurator != nil {
 		stateConfig.serverSNIFn = m.TLSConfigurator.ServerSNI

diff --git a/agent/proxycfg/mesh_gateway.go b/agent/proxycfg/mesh_gateway.go
@@ -32,17 +32,24 @@ func (s *handlerMeshGateway) initialize(ctx context.Context) (ConfigSnapshot, er
 	}
 
 	// Watch for all peer trust bundles we may need.
-	err = s.dataSources.TrustBundleList.Notify(ctx, &cachetype.TrustBundleListRequest{
-		Request: &pbpeering.TrustBundleListByServiceRequest{
-			Kind:        string(structs.ServiceKindMeshGateway),
-			ServiceName: s.service,
-			Namespace:   s.proxyID.NamespaceOrDefault(),
-			Partition:   s.proxyID.PartitionOrDefault(),
-		},
-		QueryOptions: structs.QueryOptions{Token: s.token},
-	}, peeringTrustBundlesWatchID, s.ch)
-	if err != nil {
-		return snap, err
+	if s.peeringEnabled {
+		err = s.dataSources.TrustBundleList.Notify(ctx, &cachetype.TrustBundleListRequest{
+			Request: &pbpeering.TrustBundleListByServiceRequest{
+				Kind:        string(structs.ServiceKindMeshGateway),
+				ServiceName: s.service,
+				Namespace:   s.proxyID.NamespaceOrDefault(),
+				Partition:   s.proxyID.PartitionOrDefault(),
+			},
+			QueryOptions: structs.QueryOptions{Token: s.token},
+		}, peeringTrustBundlesWatchID, s.ch)
+		if err != nil {
+			return snap, err
+		}
+	} else {
+		// Initialize these fields even when peering is not enabled, so that the mesh gateway
+		// returns the proper value in calls to `Valid()`
+		snap.MeshGateway.PeeringTrustBundles = make([]*pbpeering.PeeringTrustBundle, 0)
+		snap.MeshGateway.PeeringTrustBundlesSet = true
 	}
 
 	wildcardEntMeta := s.proxyID.WithWildcardNamespace()
@@ -448,14 +455,16 @@ func (s *handlerMeshGateway) handleUpdate(ctx context.Context, u UpdateEvent, sn
 		snap.MeshGateway.Leaf = leaf
 
 	case peeringTrustBundlesWatchID:
-		resp, ok := u.Result.(*pbpeering.TrustBundleListByServiceResponse)
-		if !ok {
-			return fmt.Errorf("invalid type for response: %T", u.Result)
-		}
-		if len(resp.Bundles) > 0 {
-			snap.MeshGateway.PeeringTrustBundles = resp.Bundles
+		if s.peeringEnabled {
+			resp, ok := u.Result.(*pbpeering.TrustBundleListByServiceResponse)
+			if !ok {
+				return fmt.Errorf("invalid type for response: %T", u.Result)
+			}
+			if len(resp.Bundles) > 0 {
+				snap.MeshGateway.PeeringTrustBundles = resp.Bundles
+			}
+			snap.MeshGateway.PeeringTrustBundlesSet = true
 		}
-		snap.MeshGateway.PeeringTrustBundlesSet = true
 
 	case meshConfigEntryID:
 		resp, ok := u.Result.(*structs.ConfigEntryResponse)

diff --git a/agent/proxycfg/state.go b/agent/proxycfg/state.go
@@ -54,6 +54,7 @@ type stateConfig struct {
 	dnsConfig             DNSConfig
 	serverSNIFn           ServerSNIFunc
 	intentionDefaultAllow bool
+	peeringEnabled        bool
 }
 
 // state holds all the state needed to maintain the config for a registered

diff --git a/agent/proxycfg/state_test.go b/agent/proxycfg/state_test.go
@@ -441,9 +441,10 @@ func TestState_WatchesAndUpdates(t *testing.T) {
 	type testCase struct {
 		// the state to operate on. the logger, source, cache,
 		// ctx and cancel fields will be filled in by the test
-		ns       structs.NodeService
-		sourceDC string
-		stages   []verificationStage
+		ns             structs.NodeService
+		sourceDC       string
+		stages         []verificationStage
+		peeringEnabled bool
 	}
 
 	newConnectProxyCase := func(meshGatewayProxyConfigValue structs.MeshGatewayMode) testCase {
@@ -747,7 +748,6 @@ func TestState_WatchesAndUpdates(t *testing.T) {
 						rootsWatchID:               genVerifyDCSpecificWatch("dc1"),
 						exportedServiceListWatchID: genVerifyDCSpecificWatch("dc1"),
 						meshConfigEntryID:          genVerifyMeshConfigWatch("dc1"),
-						peeringTrustBundlesWatchID: genVerifyTrustBundleListWatchForMeshGateway(""),
 					},
 					verifySnapshot: func(t testing.TB, snap *ConfigSnapshot) {
 						require.False(t, snap.Valid(), "gateway without root is not valid")
@@ -827,7 +827,6 @@ func TestState_WatchesAndUpdates(t *testing.T) {
 						rootsWatchID:               genVerifyDCSpecificWatch("dc1"),
 						exportedServiceListWatchID: genVerifyDCSpecificWatch("dc1"),
 						meshConfigEntryID:          genVerifyMeshConfigWatch("dc1"),
-						peeringTrustBundlesWatchID: genVerifyTrustBundleListWatchForMeshGateway(""),
 					},
 					events: []UpdateEvent{
 						rootWatchEvent(),
@@ -2715,6 +2714,7 @@ func TestState_WatchesAndUpdates(t *testing.T) {
 			},
 		},
 		"transparent-proxy-initial-with-peers": {
+			peeringEnabled: true,
 			ns: structs.NodeService{
 				Kind:    structs.ServiceKindConnectProxy,
 				ID:      "api-proxy",
@@ -2964,6 +2964,7 @@ func TestState_WatchesAndUpdates(t *testing.T) {
 		"connect-proxy":                    newConnectProxyCase(structs.MeshGatewayModeDefault),
 		"connect-proxy-mesh-gateway-local": newConnectProxyCase(structs.MeshGatewayModeLocal),
 		"connect-proxy-with-peers": {
+			peeringEnabled: true,
 			ns: structs.NodeService{
 				Kind:    structs.ServiceKindConnectProxy,
 				ID:      "web-sidecar-proxy",
@@ -3141,6 +3142,7 @@ func TestState_WatchesAndUpdates(t *testing.T) {
 					Domain:    "consul.",
 					AltDomain: "alt.consul.",
 				},
+				peeringEnabled: tc.peeringEnabled,
 			}
 			wr := recordWatches(&sc)
 

diff --git a/agent/proxycfg/testing.go b/agent/proxycfg/testing.go
@@ -719,6 +719,7 @@ func testConfigSnapshotFixture(
 	nsFn func(ns *structs.NodeService),
 	serverSNIFn ServerSNIFunc,
 	updates []UpdateEvent,
+	peeringEnabled bool,
 ) *ConfigSnapshot {
 	const token = ""
 
@@ -761,6 +762,7 @@ func testConfigSnapshotFixture(
 		},
 		serverSNIFn:           serverSNIFn,
 		intentionDefaultAllow: false, // TODO: make configurable
+		peeringEnabled:        peeringEnabled,
 	}
 	testConfigSnapshotFixtureEnterprise(&config)
 	s, err := newServiceInstanceFromNodeService(ProxyID{ServiceID: ns.CompoundServiceID()}, ns, token)

diff --git a/agent/proxycfg/testing_connect_proxy.go b/agent/proxycfg/testing_connect_proxy.go
@@ -13,7 +13,7 @@ import (
 )
 
 // TestConfigSnapshot returns a fully populated snapshot
-func TestConfigSnapshot(t testing.T, nsFn func(ns *structs.NodeService), extraUpdates []UpdateEvent) *ConfigSnapshot {
+func TestConfigSnapshot(t testing.T, nsFn func(ns *structs.NodeService), extraUpdates []UpdateEvent, peeringEnabled bool) *ConfigSnapshot {
 	roots, leaf := TestCerts(t)
 
 	// no entries implies we'll get a default chain
@@ -84,7 +84,7 @@ func TestConfigSnapshot(t testing.T, nsFn func(ns *structs.NodeService), extraUp
 		},
 		Meta:            nil,
 		TaggedAddresses: nil,
-	}, nsFn, nil, testSpliceEvents(baseEvents, extraUpdates))
+	}, nsFn, nil, testSpliceEvents(baseEvents, extraUpdates), peeringEnabled)
 }
 
 // TestConfigSnapshotDiscoveryChain returns a fully populated snapshot using a discovery chain
@@ -155,7 +155,7 @@ func TestConfigSnapshotDiscoveryChain(
 		},
 		Meta:            nil,
 		TaggedAddresses: nil,
-	}, nsFn, nil, testSpliceEvents(baseEvents, extraUpdates))
+	}, nsFn, nil, testSpliceEvents(baseEvents, extraUpdates), false)
 }
 
 func TestConfigSnapshotExposeConfig(t testing.T, nsFn func(ns *structs.NodeService)) *ConfigSnapshot {
@@ -210,7 +210,7 @@ func TestConfigSnapshotExposeConfig(t testing.T, nsFn func(ns *structs.NodeServi
 		},
 		Meta:            nil,
 		TaggedAddresses: nil,
-	}, nsFn, nil, baseEvents)
+	}, nsFn, nil, baseEvents, false)
 }
 
 func TestConfigSnapshotExposeChecks(t testing.T) *ConfigSnapshot {
@@ -237,7 +237,7 @@ func TestConfigSnapshotExposeChecks(t testing.T) *ConfigSnapshot {
 				}},
 			},
 		},
-	)
+		false)
 }
 
 func TestConfigSnapshotGRPCExposeHTTP1(t testing.T) *ConfigSnapshot {
@@ -286,5 +286,5 @@ func TestConfigSnapshotGRPCExposeHTTP1(t testing.T) *ConfigSnapshot {
 			CorrelationID: svcChecksWatchIDPrefix + structs.ServiceIDString("grpc", nil),
 			Result:        []structs.CheckType{},
 		},
-	})
+	}, false)
 }
diff --git a/agent/proxycfg/testing_ingress_gateway.go b/agent/proxycfg/testing_ingress_gateway.go
@@ -99,7 +99,7 @@ func TestConfigSnapshotIngressGateway(
 		Address:         "1.2.3.4",
 		Meta:            nil,
 		TaggedAddresses: nil,
-	}, nsFn, nil, testSpliceEvents(baseEvents, extraUpdates))
+	}, nsFn, nil, testSpliceEvents(baseEvents, extraUpdates), true)
 }
 
 func TestConfigSnapshotIngressGatewaySDS_GatewayLevel_MixedTLS(t testing.T) *ConfigSnapshot {

diff --git a/agent/proxycfg/testing_mesh_gateway.go b/agent/proxycfg/testing_mesh_gateway.go
@@ -461,7 +461,7 @@ func TestConfigSnapshotMeshGateway(t testing.T, variant string, nsFn func(ns *st
 				Port:    443,
 			},
 		},
-	}, nsFn, nil, testSpliceEvents(baseEvents, extraUpdates))
+	}, nsFn, nil, testSpliceEvents(baseEvents, extraUpdates), false)
 }
 
 func TestConfigSnapshotPeeredMeshGateway(t testing.T, variant string, nsFn func(ns *structs.NodeService), extraUpdates []UpdateEvent) *ConfigSnapshot {
@@ -737,5 +737,5 @@ func TestConfigSnapshotPeeredMeshGateway(t testing.T, variant string, nsFn func(
 				Port:    443,
 			},
 		},
-	}, nsFn, nil, testSpliceEvents(baseEvents, extraUpdates))
+	}, nsFn, nil, testSpliceEvents(baseEvents, extraUpdates), true)
 }
diff --git a/agent/proxycfg/testing_peering.go b/agent/proxycfg/testing_peering.go
@@ -106,7 +106,7 @@ func TestConfigSnapshotPeering(t testing.T) *ConfigSnapshot {
 				},
 			},
 		},
-	})
+	}, true)
 }
 
 func TestConfigSnapshotPeeringTProxy(t testing.T) *ConfigSnapshot {
@@ -247,5 +247,5 @@ func TestConfigSnapshotPeeringTProxy(t testing.T) *ConfigSnapshot {
 				},
 			},
 		},
-	})
+	}, true)
 }
diff --git a/agent/proxycfg/testing_terminating_gateway.go b/agent/proxycfg/testing_terminating_gateway.go
@@ -321,7 +321,7 @@ func TestConfigSnapshotTerminatingGateway(t testing.T, populateServices bool, ns
 				Port:    443,
 			},
 		},
-	}, nsFn, nil, testSpliceEvents(baseEvents, extraUpdates))
+	}, nsFn, nil, testSpliceEvents(baseEvents, extraUpdates), false)
 }
 
 func TestConfigSnapshotTerminatingGatewayDestinations(t testing.T, populateDestinations bool, extraUpdates []UpdateEvent) *ConfigSnapshot {
@@ -520,7 +520,7 @@ func TestConfigSnapshotTerminatingGatewayDestinations(t testing.T, populateDesti
 				Port:    443,
 			},
 		},
-	}, nil, nil, testSpliceEvents(baseEvents, extraUpdates))
+	}, nil, nil, testSpliceEvents(baseEvents, extraUpdates), false)
 }
 
 func TestConfigSnapshotTerminatingGatewayServiceSubsets(t testing.T) *ConfigSnapshot {