Watch the singular service resolver instead of the list + filtering to 1 (#13012)

* Watch the singular service resolver instead of the list + filtering to 1 * Rename the ConfigEntries cache type to ConfigEntryList
2022-05-12 16:34:17 -04:00 · 2022-05-12 16:34:17 -04:00 · 42aec5caf4
parent b6594144ee
commit 42aec5caf4
8 changed files with 119 additions and 141 deletions
--- a/.changelog/13012.txt
+++ b/.changelog/13012.txt
@ -0,0 +1,3 @@
+```release-note:bug
+proxycfg: Fixed a minor bug that would cause configuring a terminating gateway to watch too many service resolvers and waste resources doing filtering.
+```
--- a/agent/agent.go
+++ b/agent/agent.go
@ -4045,7 +4045,7 @@ func (a *Agent) registerCache() {

 	a.cache.RegisterType(cachetype.GatewayServicesName, &cachetype.GatewayServices{RPC: a})

-	a.cache.RegisterType(cachetype.ConfigEntriesName, &cachetype.ConfigEntries{RPC: a})
+	a.cache.RegisterType(cachetype.ConfigEntryListName, &cachetype.ConfigEntryList{RPC: a})

 	a.cache.RegisterType(cachetype.ConfigEntryName, &cachetype.ConfigEntry{RPC: a})

--- a/agent/cache-types/config_entry.go
+++ b/agent/cache-types/config_entry.go
@ -9,17 +9,17 @@ import (

 // Recommended name for registration.
 const (
-	ConfigEntriesName = "config-entries"
-	ConfigEntryName   = "config-entry"
+	ConfigEntryListName = "config-entries"
+	ConfigEntryName     = "config-entry"
 )

-// ConfigEntries supports fetching discovering configuration entries
-type ConfigEntries struct {
+// ConfigEntryList supports fetching discovering configuration entries
+type ConfigEntryList struct {
 	RegisterOptionsBlockingRefresh
 	RPC RPC
 }

-func (c *ConfigEntries) Fetch(opts cache.FetchOptions, req cache.Request) (cache.FetchResult, error) {
+func (c *ConfigEntryList) Fetch(opts cache.FetchOptions, req cache.Request) (cache.FetchResult, error) {
 	var result cache.FetchResult

 	// The request should be a ConfigEntryQuery.
--- a/agent/cache-types/config_entry_test.go
+++ b/agent/cache-types/config_entry_test.go
@ -12,7 +12,7 @@ import (

 func TestConfigEntries(t *testing.T) {
 	rpc := TestRPC(t)
-	typ := &ConfigEntries{RPC: rpc}
+	typ := &ConfigEntryList{RPC: rpc}

 	// Expect the proper RPC call. This also sets the expected value
 	// since that is return-by-pointer in the arguments.
@ -99,7 +99,7 @@ func TestConfigEntry(t *testing.T) {

 func TestConfigEntries_badReqType(t *testing.T) {
 	rpc := TestRPC(t)
-	typ := &ConfigEntries{RPC: rpc}
+	typ := &ConfigEntryList{RPC: rpc}

 	// Fetch
 	_, err := typ.Fetch(cache.FetchOptions{}, cache.TestRequest(
--- a/agent/proxycfg/mesh_gateway.go
+++ b/agent/proxycfg/mesh_gateway.go
@ -47,7 +47,7 @@ func (s *handlerMeshGateway) initialize(ctx context.Context) (ConfigSnapshot, er
 	}

 	// Watch service-resolvers so we can setup service subset clusters
-	err = s.cache.Notify(ctx, cachetype.ConfigEntriesName, &structs.ConfigEntryQuery{
+	err = s.cache.Notify(ctx, cachetype.ConfigEntryListName, &structs.ConfigEntryQuery{
 		Datacenter:     s.source.Datacenter,
 		QueryOptions:   structs.QueryOptions{Token: s.token},
 		Kind:           structs.ServiceResolver,
--- a/agent/proxycfg/state_test.go
+++ b/agent/proxycfg/state_test.go
@ -221,7 +221,7 @@ func genVerifyLeafWatch(expectedService string, expectedDatacenter string) verif

 func genVerifyResolverWatch(expectedService, expectedDatacenter, expectedKind string) verifyWatchRequest {
 	return func(t testing.TB, cacheType string, request cache.Request) {
-		require.Equal(t, cachetype.ConfigEntriesName, cacheType)
+		require.Equal(t, cachetype.ConfigEntryName, cacheType)

 		reqReal, ok := request.(*structs.ConfigEntryQuery)
 		require.True(t, ok)
@ -718,16 +718,13 @@ func TestState_WatchesAndUpdates(t *testing.T) {
 		},
 	}

-	dbResolver := &structs.IndexedConfigEntries{
-		Kind: structs.ServiceResolver,
-		Entries: []structs.ConfigEntry{
-			&structs.ServiceResolverConfigEntry{
-				Name: "db",
-				Kind: structs.ServiceResolver,
-				Redirect: &structs.ServiceResolverRedirect{
-					Service:    "db",
-					Datacenter: "dc2",
-				},
+	dbResolver := &structs.ConfigEntryResponse{
+		Entry: &structs.ServiceResolverConfigEntry{
+			Name: "db",
+			Kind: structs.ServiceResolver,
+			Redirect: &structs.ServiceResolverRedirect{
+				Service:    "db",
+				Datacenter: "dc2",
 			},
 		},
 	}
@ -1641,7 +1638,7 @@ func TestState_WatchesAndUpdates(t *testing.T) {
 						require.True(t, snap.TerminatingGateway.ServiceResolversSet[db])

 						require.Len(t, snap.TerminatingGateway.ServiceResolvers, 1)
-						require.Equal(t, dbResolver.Entries[0], snap.TerminatingGateway.ServiceResolvers[db])
+						require.Equal(t, dbResolver.Entry, snap.TerminatingGateway.ServiceResolvers[db])
 					},
 				},
 				{
--- a/agent/proxycfg/terminating_gateway.go
+++ b/agent/proxycfg/terminating_gateway.go
@ -216,7 +216,7 @@ func (s *handlerTerminatingGateway) handleUpdate(ctx context.Context, u cache.Up
 			// These are used to create clusters and endpoints for the service subsets
 			if _, ok := snap.TerminatingGateway.WatchedResolvers[svc.Service]; !ok {
 				ctx, cancel := context.WithCancel(ctx)
-				err := s.cache.Notify(ctx, cachetype.ConfigEntriesName, &structs.ConfigEntryQuery{
+				err := s.cache.Notify(ctx, cachetype.ConfigEntryName, &structs.ConfigEntryQuery{
 					Datacenter:     s.source.Datacenter,
 					QueryOptions:   structs.QueryOptions{Token: s.token},
 					Kind:           structs.ServiceResolver,
@ -340,16 +340,15 @@ func (s *handlerTerminatingGateway) handleUpdate(ctx context.Context, u cache.Up
 		snap.TerminatingGateway.ServiceConfigs[sn] = serviceConfig

 	case strings.HasPrefix(u.CorrelationID, serviceResolverIDPrefix):
-		configEntries, ok := u.Result.(*structs.IndexedConfigEntries)
+		resp, ok := u.Result.(*structs.ConfigEntryResponse)
 		if !ok {
 			return fmt.Errorf("invalid type for response: %T", u.Result)
 		}
+
 		sn := structs.ServiceNameFromString(strings.TrimPrefix(u.CorrelationID, serviceResolverIDPrefix))
 		// There should only ever be one entry for a service resolver within a namespace
-		if len(configEntries.Entries) == 1 {
-			if resolver, ok := configEntries.Entries[0].(*structs.ServiceResolverConfigEntry); ok {
-				snap.TerminatingGateway.ServiceResolvers[sn] = resolver
-			}
+		if resolver, ok := resp.Entry.(*structs.ServiceResolverConfigEntry); ok {
+			snap.TerminatingGateway.ServiceResolvers[sn] = resolver
 		}
 		snap.TerminatingGateway.ServiceResolversSet[sn] = true

--- a/agent/proxycfg/testing_terminating_gateway.go
+++ b/agent/proxycfg/testing_terminating_gateway.go
@ -297,30 +297,34 @@ func TestConfigSnapshotTerminatingGateway(
 			// ========
 			{
 				CorrelationID: serviceResolverIDPrefix + web.String(),
-				Result: &structs.IndexedConfigEntries{
-					Kind:    structs.ServiceResolver,
-					Entries: nil,
+				Result: &structs.ConfigEntryResponse{
+					Entry: &structs.ServiceResolverConfigEntry{
+						Kind: structs.ServiceResolver,
+					},
 				},
 			},
 			{
 				CorrelationID: serviceResolverIDPrefix + api.String(),
-				Result: &structs.IndexedConfigEntries{
-					Kind:    structs.ServiceResolver,
-					Entries: nil,
+				Result: &structs.ConfigEntryResponse{
+					Entry: &structs.ServiceResolverConfigEntry{
+						Kind: structs.ServiceResolver,
+					},
 				},
 			},
 			{
 				CorrelationID: serviceResolverIDPrefix + db.String(),
-				Result: &structs.IndexedConfigEntries{
-					Kind:    structs.ServiceResolver,
-					Entries: nil,
+				Result: &structs.ConfigEntryResponse{
+					Entry: &structs.ServiceResolverConfigEntry{
+						Kind: structs.ServiceResolver,
+					},
 				},
 			},
 			{
 				CorrelationID: serviceResolverIDPrefix + cache.String(),
-				Result: &structs.IndexedConfigEntries{
-					Kind:    structs.ServiceResolver,
-					Entries: nil,
+				Result: &structs.ConfigEntryResponse{
+					Entry: &structs.ServiceResolverConfigEntry{
+						Kind: structs.ServiceResolver,
+					},
 				},
 			},
 		})
@ -355,20 +359,17 @@ func testConfigSnapshotTerminatingGatewayServiceSubsets(t testing.T, alsoAdjustC
 	events := []agentcache.UpdateEvent{
 		{
 			CorrelationID: serviceResolverIDPrefix + web.String(),
-			Result: &structs.IndexedConfigEntries{
-				Kind: structs.ServiceResolver,
-				Entries: []structs.ConfigEntry{
-					&structs.ServiceResolverConfigEntry{
-						Kind: structs.ServiceResolver,
-						Name: "web",
-						Subsets: map[string]structs.ServiceResolverSubset{
-							"v1": {
-								Filter: "Service.Meta.version == 1",
-							},
-							"v2": {
-								Filter:      "Service.Meta.version == 2",
-								OnlyPassing: true,
-							},
+			Result: &structs.ConfigEntryResponse{
+				Entry: &structs.ServiceResolverConfigEntry{
+					Kind: structs.ServiceResolver,
+					Name: "web",
+					Subsets: map[string]structs.ServiceResolverSubset{
+						"v1": {
+							Filter: "Service.Meta.version == 1",
+						},
+						"v2": {
+							Filter:      "Service.Meta.version == 2",
+							OnlyPassing: true,
 						},
 					},
 				},
@ -386,16 +387,13 @@ func testConfigSnapshotTerminatingGatewayServiceSubsets(t testing.T, alsoAdjustC
 		events = testSpliceEvents(events, []agentcache.UpdateEvent{
 			{
 				CorrelationID: serviceResolverIDPrefix + cache.String(),
-				Result: &structs.IndexedConfigEntries{
-					Kind: structs.ServiceResolver,
-					Entries: []structs.ConfigEntry{
-						&structs.ServiceResolverConfigEntry{
-							Kind: structs.ServiceResolver,
-							Name: "cache",
-							Subsets: map[string]structs.ServiceResolverSubset{
-								"prod": {
-									Filter: "Service.Meta.Env == prod",
-								},
+				Result: &structs.ConfigEntryResponse{
+					Entry: &structs.ServiceResolverConfigEntry{
+						Kind: structs.ServiceResolver,
+						Name: "cache",
+						Subsets: map[string]structs.ServiceResolverSubset{
+							"prod": {
+								Filter: "Service.Meta.Env == prod",
 							},
 						},
 					},
@ -419,21 +417,18 @@ func TestConfigSnapshotTerminatingGatewayDefaultServiceSubset(t testing.T) *Conf
 	return TestConfigSnapshotTerminatingGateway(t, true, nil, []agentcache.UpdateEvent{
 		{
 			CorrelationID: serviceResolverIDPrefix + web.String(),
-			Result: &structs.IndexedConfigEntries{
-				Kind: structs.ServiceResolver,
-				Entries: []structs.ConfigEntry{
-					&structs.ServiceResolverConfigEntry{
-						Kind:          structs.ServiceResolver,
-						Name:          "web",
-						DefaultSubset: "v2",
-						Subsets: map[string]structs.ServiceResolverSubset{
-							"v1": {
-								Filter: "Service.Meta.version == 1",
-							},
-							"v2": {
-								Filter:      "Service.Meta.version == 2",
-								OnlyPassing: true,
-							},
+			Result: &structs.ConfigEntryResponse{
+				Entry: &structs.ServiceResolverConfigEntry{
+					Kind:          structs.ServiceResolver,
+					Name:          "web",
+					DefaultSubset: "v2",
+					Subsets: map[string]structs.ServiceResolverSubset{
+						"v1": {
+							Filter: "Service.Meta.version == 1",
+						},
+						"v2": {
+							Filter:      "Service.Meta.version == 2",
+							OnlyPassing: true,
 						},
 					},
 				},
@ -512,9 +507,8 @@ func testConfigSnapshotTerminatingGatewayLBConfig(t testing.T, variant string) *
 		},
 		{
 			CorrelationID: serviceResolverIDPrefix + web.String(),
-			Result: &structs.IndexedConfigEntries{
-				Kind:    structs.ServiceResolver,
-				Entries: []structs.ConfigEntry{entry},
+			Result: &structs.ConfigEntryResponse{
+				Entry: entry,
 			},
 		},
 		{
@ -559,16 +553,13 @@ func TestConfigSnapshotTerminatingGatewayHostnameSubsets(t testing.T) *ConfigSna
 	return TestConfigSnapshotTerminatingGateway(t, true, nil, []agentcache.UpdateEvent{
 		{
 			CorrelationID: serviceResolverIDPrefix + api.String(),
-			Result: &structs.IndexedConfigEntries{
-				Kind: structs.ServiceResolver,
-				Entries: []structs.ConfigEntry{
-					&structs.ServiceResolverConfigEntry{
-						Kind: structs.ServiceResolver,
-						Name: "api",
-						Subsets: map[string]structs.ServiceResolverSubset{
-							"alt": {
-								Filter: "Service.Meta.domain == alt",
-							},
+			Result: &structs.ConfigEntryResponse{
+				Entry: &structs.ServiceResolverConfigEntry{
+					Kind: structs.ServiceResolver,
+					Name: "api",
+					Subsets: map[string]structs.ServiceResolverSubset{
+						"alt": {
+							Filter: "Service.Meta.domain == alt",
 						},
 					},
 				},
@ -576,16 +567,13 @@ func TestConfigSnapshotTerminatingGatewayHostnameSubsets(t testing.T) *ConfigSna
 		},
 		{
 			CorrelationID: serviceResolverIDPrefix + cache.String(),
-			Result: &structs.IndexedConfigEntries{
-				Kind: structs.ServiceResolver,
-				Entries: []structs.ConfigEntry{
-					&structs.ServiceResolverConfigEntry{
-						Kind: structs.ServiceResolver,
-						Name: "cache",
-						Subsets: map[string]structs.ServiceResolverSubset{
-							"prod": {
-								Filter: "Service.Meta.Env == prod",
-							},
+			Result: &structs.ConfigEntryResponse{
+				Entry: &structs.ServiceResolverConfigEntry{
+					Kind: structs.ServiceResolver,
+					Name: "cache",
+					Subsets: map[string]structs.ServiceResolverSubset{
+						"prod": {
+							Filter: "Service.Meta.Env == prod",
 						},
 					},
 				},
@ -615,21 +603,18 @@ func TestConfigSnapshotTerminatingGatewayIgnoreExtraResolvers(t testing.T) *Conf
 	return TestConfigSnapshotTerminatingGateway(t, true, nil, []agentcache.UpdateEvent{
 		{
 			CorrelationID: serviceResolverIDPrefix + web.String(),
-			Result: &structs.IndexedConfigEntries{
-				Kind: structs.ServiceResolver,
-				Entries: []structs.ConfigEntry{
-					&structs.ServiceResolverConfigEntry{
-						Kind:          structs.ServiceResolver,
-						Name:          "web",
-						DefaultSubset: "v2",
-						Subsets: map[string]structs.ServiceResolverSubset{
-							"v1": {
-								Filter: "Service.Meta.Version == 1",
-							},
-							"v2": {
-								Filter:      "Service.Meta.Version == 2",
-								OnlyPassing: true,
-							},
+			Result: &structs.ConfigEntryResponse{
+				Entry: &structs.ServiceResolverConfigEntry{
+					Kind:          structs.ServiceResolver,
+					Name:          "web",
+					DefaultSubset: "v2",
+					Subsets: map[string]structs.ServiceResolverSubset{
+						"v1": {
+							Filter: "Service.Meta.Version == 1",
+						},
+						"v2": {
+							Filter:      "Service.Meta.Version == 2",
+							OnlyPassing: true,
 						},
 					},
 				},
@ -637,21 +622,18 @@ func TestConfigSnapshotTerminatingGatewayIgnoreExtraResolvers(t testing.T) *Conf
 		},
 		{
 			CorrelationID: serviceResolverIDPrefix + notfound.String(),
-			Result: &structs.IndexedConfigEntries{
-				Kind: structs.ServiceResolver,
-				Entries: []structs.ConfigEntry{
-					&structs.ServiceResolverConfigEntry{
-						Kind:          structs.ServiceResolver,
-						Name:          "notfound",
-						DefaultSubset: "v2",
-						Subsets: map[string]structs.ServiceResolverSubset{
-							"v1": {
-								Filter: "Service.Meta.Version == 1",
-							},
-							"v2": {
-								Filter:      "Service.Meta.Version == 2",
-								OnlyPassing: true,
-							},
+			Result: &structs.ConfigEntryResponse{
+				Entry: &structs.ServiceResolverConfigEntry{
+					Kind:          structs.ServiceResolver,
+					Name:          "notfound",
+					DefaultSubset: "v2",
+					Subsets: map[string]structs.ServiceResolverSubset{
+						"v1": {
+							Filter: "Service.Meta.Version == 1",
+						},
+						"v2": {
+							Filter:      "Service.Meta.Version == 2",
+							OnlyPassing: true,
 						},
 					},
 				},
@ -689,16 +671,13 @@ func TestConfigSnapshotTerminatingGatewayWithLambdaServiceAndServiceResolvers(t
 	return TestConfigSnapshotTerminatingGatewayWithLambdaService(t,
 		agentcache.UpdateEvent{
 			CorrelationID: serviceResolverIDPrefix + web.String(),
-			Result: &structs.IndexedConfigEntries{
-				Kind: structs.ServiceResolver,
-				Entries: []structs.ConfigEntry{
-					&structs.ServiceResolverConfigEntry{
-						Kind: structs.ServiceResolver,
-						Name: web.String(),
-						Subsets: map[string]structs.ServiceResolverSubset{
-							"canary1": {},
-							"canary2": {},
-						},
+			Result: &structs.ConfigEntryResponse{
+				Entry: &structs.ServiceResolverConfigEntry{
+					Kind: structs.ServiceResolver,
+					Name: web.String(),
+					Subsets: map[string]structs.ServiceResolverSubset{
+						"canary1": {},
+						"canary2": {},
 					},
 				},
 			},