diff --git a/.changelog/13012.txt b/.changelog/13012.txt new file mode 100644 index 000000000..7fc66b49d --- /dev/null +++ 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. +``` \ No newline at end of file diff --git a/agent/agent.go b/agent/agent.go index 7b81821ef..b1e5fd4aa 100644 --- 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}) diff --git a/agent/cache-types/config_entry.go b/agent/cache-types/config_entry.go index a50c5c382..d48572d7f 100644 --- 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. diff --git a/agent/cache-types/config_entry_test.go b/agent/cache-types/config_entry_test.go index 169022965..19522b796 100644 --- 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( diff --git a/agent/proxycfg/mesh_gateway.go b/agent/proxycfg/mesh_gateway.go index 3bcce18e7..5e5fa131a 100644 --- 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, diff --git a/agent/proxycfg/state_test.go b/agent/proxycfg/state_test.go index fcff78d92..29a176e0c 100644 --- 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]) }, }, { diff --git a/agent/proxycfg/terminating_gateway.go b/agent/proxycfg/terminating_gateway.go index 73b968272..592a454ca 100644 --- 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 diff --git a/agent/proxycfg/testing_terminating_gateway.go b/agent/proxycfg/testing_terminating_gateway.go index 38c0245a2..5907b0199 100644 --- 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": {}, }, }, },