From 9e5fd7f925df1066df3d44773a63aee832ac6d24 Mon Sep 17 00:00:00 2001 From: Matt Keeler Date: Thu, 6 Feb 2020 10:52:25 -0500 Subject: [PATCH] OSS Changes for various config entry namespacing bugs (#7226) --- agent/config/runtime_test.go | 26 ++------ agent/consul/state/config_entry_test.go | 64 ++++++++++++++++---- agent/proxycfg/manager_test.go | 12 +++- agent/proxycfg/state.go | 48 ++++++++++++--- agent/proxycfg/state_test.go | 12 ++-- agent/sidecar_service_test.go | 16 +++-- agent/structs/config_entry_discoverychain.go | 41 +++++++++---- agent/structs/connect_proxy_config.go | 6 +- agent/structs/discovery_chain.go | 2 +- agent/structs/service_definition.go | 2 + 10 files changed, 159 insertions(+), 70 deletions(-) diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index f5174236cd..bb1ce55b56 100644 --- a/agent/config/runtime_test.go +++ b/agent/config/runtime_test.go @@ -2207,8 +2207,8 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { }, patch: func(rt *RuntimeConfig) { rt.Checks = []*structs.CheckDefinition{ - &structs.CheckDefinition{Name: "a", ScriptArgs: []string{"/bin/true"}, OutputMaxSize: checks.DefaultBufSize, EnterpriseMeta: *structs.DefaultEnterpriseMeta()}, - &structs.CheckDefinition{Name: "b", ScriptArgs: []string{"/bin/false"}, OutputMaxSize: checks.DefaultBufSize, EnterpriseMeta: *structs.DefaultEnterpriseMeta()}, + &structs.CheckDefinition{Name: "a", ScriptArgs: []string{"/bin/true"}, OutputMaxSize: checks.DefaultBufSize}, + &structs.CheckDefinition{Name: "b", ScriptArgs: []string{"/bin/false"}, OutputMaxSize: checks.DefaultBufSize}, } rt.DataDir = dataDir }, @@ -2226,7 +2226,7 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { }, patch: func(rt *RuntimeConfig) { rt.Checks = []*structs.CheckDefinition{ - &structs.CheckDefinition{Name: "a", GRPC: "localhost:12345/foo", GRPCUseTLS: true, OutputMaxSize: checks.DefaultBufSize, EnterpriseMeta: *structs.DefaultEnterpriseMeta()}, + &structs.CheckDefinition{Name: "a", GRPC: "localhost:12345/foo", GRPCUseTLS: true, OutputMaxSize: checks.DefaultBufSize}, } rt.DataDir = dataDir }, @@ -2244,7 +2244,7 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { }, patch: func(rt *RuntimeConfig) { rt.Checks = []*structs.CheckDefinition{ - &structs.CheckDefinition{Name: "a", AliasService: "foo", OutputMaxSize: checks.DefaultBufSize, EnterpriseMeta: *structs.DefaultEnterpriseMeta()}, + &structs.CheckDefinition{Name: "a", AliasService: "foo", OutputMaxSize: checks.DefaultBufSize}, } rt.DataDir = dataDir }, @@ -2271,7 +2271,6 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { Passing: 1, Warning: 1, }, - EnterpriseMeta: *structs.DefaultEnterpriseMeta(), }, &structs.ServiceDefinition{ Name: "b", @@ -2281,7 +2280,6 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { Passing: 13, Warning: 1, }, - EnterpriseMeta: *structs.DefaultEnterpriseMeta(), }, } rt.DataDir = dataDir @@ -2399,7 +2397,6 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { Passing: 1, Warning: 1, }, - EnterpriseMeta: *structs.DefaultEnterpriseMeta(), }, } rt.DataDir = dataDir @@ -2540,14 +2537,12 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { Passing: 1, Warning: 1, }, - EnterpriseMeta: *structs.DefaultEnterpriseMeta(), }, }, Weights: &structs.Weights{ Passing: 1, Warning: 1, }, - EnterpriseMeta: *structs.DefaultEnterpriseMeta(), }, } }, @@ -2671,14 +2666,12 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { Passing: 1, Warning: 1, }, - EnterpriseMeta: *structs.DefaultEnterpriseMeta(), }, }, Weights: &structs.Weights{ Passing: 1, Warning: 1, }, - EnterpriseMeta: *structs.DefaultEnterpriseMeta(), }, } }, @@ -5087,7 +5080,6 @@ func TestFullConfig(t *testing.T) { Timeout: 1813 * time.Second, TTL: 21743 * time.Second, DeregisterCriticalServiceAfter: 14232 * time.Second, - EnterpriseMeta: *structs.DefaultEnterpriseMeta(), }, &structs.CheckDefinition{ ID: "Cqq95BhP", @@ -5112,7 +5104,6 @@ func TestFullConfig(t *testing.T) { Timeout: 18506 * time.Second, TTL: 31006 * time.Second, DeregisterCriticalServiceAfter: 2366 * time.Second, - EnterpriseMeta: *structs.DefaultEnterpriseMeta(), }, &structs.CheckDefinition{ ID: "fZaCAXww", @@ -5137,7 +5128,6 @@ func TestFullConfig(t *testing.T) { Timeout: 5954 * time.Second, TTL: 30044 * time.Second, DeregisterCriticalServiceAfter: 13209 * time.Second, - EnterpriseMeta: *structs.DefaultEnterpriseMeta(), }, }, CheckUpdateInterval: 16507 * time.Second, @@ -5324,10 +5314,8 @@ func TestFullConfig(t *testing.T) { Passing: 1, Warning: 1, }, - EnterpriseMeta: *structs.DefaultEnterpriseMeta(), }, }, - EnterpriseMeta: *structs.DefaultEnterpriseMeta(), }, { ID: "MRHVMZuD", @@ -5387,8 +5375,7 @@ func TestFullConfig(t *testing.T) { DeregisterCriticalServiceAfter: 68482 * time.Second, }, }, - Connect: &structs.ServiceConnect{}, - EnterpriseMeta: *structs.DefaultEnterpriseMeta(), + Connect: &structs.ServiceConnect{}, }, { ID: "Kh81CPF6", @@ -5436,7 +5423,6 @@ func TestFullConfig(t *testing.T) { Passing: 1, Warning: 1, }, - EnterpriseMeta: *structs.DefaultEnterpriseMeta(), }, { ID: "kvVqbwSE", @@ -5452,7 +5438,6 @@ func TestFullConfig(t *testing.T) { Passing: 1, Warning: 1, }, - EnterpriseMeta: *structs.DefaultEnterpriseMeta(), }, { ID: "dLOXpSCI", @@ -5548,7 +5533,6 @@ func TestFullConfig(t *testing.T) { DeregisterCriticalServiceAfter: 68787 * time.Second, }, }, - EnterpriseMeta: *structs.DefaultEnterpriseMeta(), }, }, SerfAdvertiseAddrLAN: tcpAddr("17.99.29.16:8301"), diff --git a/agent/consul/state/config_entry_test.go b/agent/consul/state/config_entry_test.go index f3596283fa..f2e9e479ca 100644 --- a/agent/consul/state/config_entry_test.go +++ b/agent/consul/state/config_entry_test.go @@ -191,8 +191,7 @@ func TestStore_ConfigEntry_GraphValidation(t *testing.T) { Kind: structs.ServiceSplitter, Name: "main", Splits: []structs.ServiceSplit{ - {Weight: 90, Namespace: "v1"}, - {Weight: 10, Namespace: "v2"}, + {Weight: 100}, }, } return s.EnsureConfigEntry(0, entry, nil) @@ -213,8 +212,7 @@ func TestStore_ConfigEntry_GraphValidation(t *testing.T) { Kind: structs.ServiceSplitter, Name: "main", Splits: []structs.ServiceSplit{ - {Weight: 90, Namespace: "v1"}, - {Weight: 10, Namespace: "v2"}, + {Weight: 100}, }, } return s.EnsureConfigEntry(0, entry, nil) @@ -272,14 +270,26 @@ func TestStore_ConfigEntry_GraphValidation(t *testing.T) { "protocol": "http", }, }, + &structs.ServiceResolverConfigEntry{ + Kind: structs.ServiceResolver, + Name: "main", + Subsets: map[string]structs.ServiceResolverSubset{ + "v1": structs.ServiceResolverSubset{ + Filter: "Service.Meta.version == v1", + }, + "v2": structs.ServiceResolverSubset{ + Filter: "Service.Meta.version == v2", + }, + }, + }, }, op: func(t *testing.T, s *Store) error { entry := &structs.ServiceSplitterConfigEntry{ Kind: structs.ServiceSplitter, Name: "main", Splits: []structs.ServiceSplit{ - {Weight: 90, Namespace: "v1"}, - {Weight: 10, Namespace: "v2"}, + {Weight: 90, ServiceSubset: "v1"}, + {Weight: 10, ServiceSubset: "v2"}, }, } return s.EnsureConfigEntry(0, entry, nil) @@ -292,6 +302,15 @@ func TestStore_ConfigEntry_GraphValidation(t *testing.T) { Name: "main", Protocol: "tcp", }, + &structs.ServiceResolverConfigEntry{ + Kind: structs.ServiceResolver, + Name: "main", + Subsets: map[string]structs.ServiceResolverSubset{ + "other": structs.ServiceResolverSubset{ + Filter: "Service.Meta.version == other", + }, + }, + }, }, op: func(t *testing.T, s *Store) error { entry := &structs.ServiceRouterConfigEntry{ @@ -305,7 +324,7 @@ func TestStore_ConfigEntry_GraphValidation(t *testing.T) { }, }, Destination: &structs.ServiceRouteDestination{ - Namespace: "other", + ServiceSubset: "other", }, }, }, @@ -316,7 +335,17 @@ func TestStore_ConfigEntry_GraphValidation(t *testing.T) { expectGraphErr: true, }, "router fails without default protocol": tcase{ - entries: []structs.ConfigEntry{}, + entries: []structs.ConfigEntry{ + &structs.ServiceResolverConfigEntry{ + Kind: structs.ServiceResolver, + Name: "main", + Subsets: map[string]structs.ServiceResolverSubset{ + "other": structs.ServiceResolverSubset{ + Filter: "Service.Meta.version == other", + }, + }, + }, + }, op: func(t *testing.T, s *Store) error { entry := &structs.ServiceRouterConfigEntry{ Kind: structs.ServiceRouter, @@ -329,7 +358,7 @@ func TestStore_ConfigEntry_GraphValidation(t *testing.T) { }, }, Destination: &structs.ServiceRouteDestination{ - Namespace: "other", + ServiceSubset: "other", }, }, }, @@ -383,12 +412,24 @@ func TestStore_ConfigEntry_GraphValidation(t *testing.T) { "protocol": "http", }, }, + &structs.ServiceResolverConfigEntry{ + Kind: structs.ServiceResolver, + Name: "main", + Subsets: map[string]structs.ServiceResolverSubset{ + "v1": structs.ServiceResolverSubset{ + Filter: "Service.Meta.version == v1", + }, + "v2": structs.ServiceResolverSubset{ + Filter: "Service.Meta.version == v2", + }, + }, + }, &structs.ServiceSplitterConfigEntry{ Kind: structs.ServiceSplitter, Name: "main", Splits: []structs.ServiceSplit{ - {Weight: 90, Namespace: "v1"}, - {Weight: 10, Namespace: "v2"}, + {Weight: 90, ServiceSubset: "v1"}, + {Weight: 10, ServiceSubset: "v2"}, }, }, }, @@ -798,6 +839,7 @@ func TestStore_ConfigEntry_GraphValidation(t *testing.T) { t.Run(name, func(t *testing.T) { s := testStateStore(t) for _, entry := range tc.entries { + require.NoError(t, entry.Normalize()) require.NoError(t, s.EnsureConfigEntry(0, entry, nil)) } diff --git a/agent/proxycfg/manager_test.go b/agent/proxycfg/manager_test.go index 360c1e227d..cd4405e01c 100644 --- a/agent/proxycfg/manager_test.go +++ b/agent/proxycfg/manager_test.go @@ -18,6 +18,12 @@ import ( "github.com/hashicorp/consul/sdk/testutil" ) +func mustCopyProxyConfig(t *testing.T, ns *structs.NodeService) structs.ConnectProxyConfig { + cfg, err := copyProxyConfig(ns) + require.NoError(t, err) + return cfg +} + // assertLastReqArgs verifies that each request type had the correct source // parameters (e.g. Datacenter name) and token. func assertLastReqArgs(t *testing.T, types *TestCacheTypes, token string, source *structs.QuerySource) { @@ -137,7 +143,7 @@ func TestManager_BasicLifecycle(t *testing.T) { dbChainCacheKey := testGenCacheKey(&structs.DiscoveryChainRequest{ Name: "db", EvaluateInDatacenter: "dc1", - EvaluateInNamespace: "", + EvaluateInNamespace: "default", // This is because structs.TestUpstreams uses an opaque config // to override connect timeouts. OverrideConnectTimeout: 1 * time.Second, @@ -190,7 +196,7 @@ func TestManager_BasicLifecycle(t *testing.T) { ProxyID: webProxy.CompoundServiceID(), Address: webProxy.Address, Port: webProxy.Port, - Proxy: webProxy.Proxy, + Proxy: mustCopyProxyConfig(t, webProxy), TaggedAddresses: make(map[string]structs.ServiceAddress), Roots: roots, ConnectProxy: configSnapshotConnectProxy{ @@ -234,7 +240,7 @@ func TestManager_BasicLifecycle(t *testing.T) { ProxyID: webProxy.CompoundServiceID(), Address: webProxy.Address, Port: webProxy.Port, - Proxy: webProxy.Proxy, + Proxy: mustCopyProxyConfig(t, webProxy), TaggedAddresses: make(map[string]structs.ServiceAddress), Roots: roots, ConnectProxy: configSnapshotConnectProxy{ diff --git a/agent/proxycfg/state.go b/agent/proxycfg/state.go index d6203a3464..8c5327638b 100644 --- a/agent/proxycfg/state.go +++ b/agent/proxycfg/state.go @@ -63,6 +63,35 @@ type state struct { reqCh chan chan *ConfigSnapshot } +func copyProxyConfig(ns *structs.NodeService) (structs.ConnectProxyConfig, error) { + if ns == nil { + return structs.ConnectProxyConfig{}, nil + } + // Copy the config map + proxyCfgRaw, err := copystructure.Copy(ns.Proxy) + if err != nil { + return structs.ConnectProxyConfig{}, err + } + proxyCfg, ok := proxyCfgRaw.(structs.ConnectProxyConfig) + if !ok { + return structs.ConnectProxyConfig{}, errors.New("failed to copy proxy config") + } + + // we can safely modify these since we just copied them + for idx, _ := range proxyCfg.Upstreams { + us := &proxyCfg.Upstreams[idx] + if us.DestinationType != structs.UpstreamDestTypePreparedQuery && us.DestinationNamespace == "" { + // default the upstreams target namespace to the namespace of the proxy + // doing this here prevents needing much more complex logic a bunch of other + // places and makes tracking these upstreams simpler as we can dedup them + // with the maps tracking upstream ids being watched. + proxyCfg.Upstreams[idx].DestinationNamespace = ns.EnterpriseMeta.NamespaceOrDefault() + } + } + + return proxyCfg, nil +} + // newState populates the state struct by copying relevant fields from the // NodeService and Token. We copy so that we can use them in a separate // goroutine later without reasoning about races with the NodeService passed @@ -75,15 +104,10 @@ func newState(ns *structs.NodeService, token string) (*state, error) { return nil, errors.New("not a connect-proxy or mesh-gateway") } - // Copy the config map - proxyCfgRaw, err := copystructure.Copy(ns.Proxy) + proxyCfg, err := copyProxyConfig(ns) if err != nil { return nil, err } - proxyCfg, ok := proxyCfgRaw.(structs.ConnectProxyConfig) - if !ok { - return nil, errors.New("failed to copy proxy config") - } taggedAddresses := make(map[string]structs.ServiceAddress) for k, v := range ns.TaggedAddresses { @@ -232,8 +256,8 @@ func (s *state) initWatchesConnectProxy() error { return err } - // let namespace inference happen server side - currentNamespace := "" + // default the namespace to the namespace of this proxy service + currentNamespace := s.proxyID.NamespaceOrDefault() // Watch for updates to service endpoints for all upstreams for _, u := range s.proxyCfg.Upstreams { @@ -889,10 +913,16 @@ func (s *state) Changed(ns *structs.NodeService, token string) bool { if ns == nil { return true } + + proxyCfg, err := copyProxyConfig(ns) + if err != nil { + s.logger.Warn("Failed to parse proxy config and will treat the new service as unchanged") + } + return ns.Kind != s.kind || s.proxyID != ns.CompoundServiceID() || s.address != ns.Address || s.port != ns.Port || - !reflect.DeepEqual(s.proxyCfg, ns.Proxy) || + !reflect.DeepEqual(s.proxyCfg, proxyCfg) || s.token != token } diff --git a/agent/proxycfg/state_test.go b/agent/proxycfg/state_test.go index 1e8b1bc4bc..f80ff870bf 100644 --- a/agent/proxycfg/state_test.go +++ b/agent/proxycfg/state_test.go @@ -147,7 +147,7 @@ func (cn *testCacheNotifier) getNotifierRequest(t testing.TB, correlationId stri cn.lock.RLock() req, ok := cn.notifiers[correlationId] cn.lock.RUnlock() - require.True(t, ok) + require.True(t, ok, "Correlation ID: %s is missing", correlationId) return req } @@ -384,7 +384,7 @@ func TestState_WatchesAndUpdates(t *testing.T) { "discovery-chain:api": genVerifyDiscoveryChainWatch(&structs.DiscoveryChainRequest{ Name: "api", EvaluateInDatacenter: "dc1", - EvaluateInNamespace: "", + EvaluateInNamespace: "default", Datacenter: "dc1", OverrideMeshGateway: structs.MeshGatewayConfig{ Mode: meshGatewayProxyConfigValue, @@ -393,7 +393,7 @@ func TestState_WatchesAndUpdates(t *testing.T) { "discovery-chain:api-failover-remote?dc=dc2": genVerifyDiscoveryChainWatch(&structs.DiscoveryChainRequest{ Name: "api-failover-remote", EvaluateInDatacenter: "dc2", - EvaluateInNamespace: "", + EvaluateInNamespace: "default", Datacenter: "dc1", OverrideMeshGateway: structs.MeshGatewayConfig{ Mode: structs.MeshGatewayModeRemote, @@ -402,7 +402,7 @@ func TestState_WatchesAndUpdates(t *testing.T) { "discovery-chain:api-failover-local?dc=dc2": genVerifyDiscoveryChainWatch(&structs.DiscoveryChainRequest{ Name: "api-failover-local", EvaluateInDatacenter: "dc2", - EvaluateInNamespace: "", + EvaluateInNamespace: "default", Datacenter: "dc1", OverrideMeshGateway: structs.MeshGatewayConfig{ Mode: structs.MeshGatewayModeLocal, @@ -411,7 +411,7 @@ func TestState_WatchesAndUpdates(t *testing.T) { "discovery-chain:api-failover-direct?dc=dc2": genVerifyDiscoveryChainWatch(&structs.DiscoveryChainRequest{ Name: "api-failover-direct", EvaluateInDatacenter: "dc2", - EvaluateInNamespace: "", + EvaluateInNamespace: "default", Datacenter: "dc1", OverrideMeshGateway: structs.MeshGatewayConfig{ Mode: structs.MeshGatewayModeNone, @@ -420,7 +420,7 @@ func TestState_WatchesAndUpdates(t *testing.T) { "discovery-chain:api-dc2": genVerifyDiscoveryChainWatch(&structs.DiscoveryChainRequest{ Name: "api-dc2", EvaluateInDatacenter: "dc1", - EvaluateInNamespace: "", + EvaluateInNamespace: "default", Datacenter: "dc1", OverrideMeshGateway: structs.MeshGatewayConfig{ Mode: meshGatewayProxyConfigValue, diff --git a/agent/sidecar_service_test.go b/agent/sidecar_service_test.go index 12f4e2dc3c..d2e23a9ae1 100644 --- a/agent/sidecar_service_test.go +++ b/agent/sidecar_service_test.go @@ -46,6 +46,7 @@ func TestAgent_sidecarServiceFromNodeService(t *testing.T) { }, token: "foo", wantNS: &structs.NodeService{ + EnterpriseMeta: *structs.DefaultEnterpriseMeta(), Kind: structs.ServiceKindConnectProxy, ID: "web1-sidecar-proxy", Service: "web-sidecar-proxy", @@ -105,12 +106,13 @@ func TestAgent_sidecarServiceFromNodeService(t *testing.T) { }, token: "foo", wantNS: &structs.NodeService{ - Kind: structs.ServiceKindConnectProxy, - ID: "web1-sidecar-proxy", - Service: "motorbike1", - Port: 3333, - Tags: []string{"foo", "bar"}, - Address: "127.127.127.127", + EnterpriseMeta: *structs.DefaultEnterpriseMeta(), + Kind: structs.ServiceKindConnectProxy, + ID: "web1-sidecar-proxy", + Service: "motorbike1", + Port: 3333, + Tags: []string{"foo", "bar"}, + Address: "127.127.127.127", Meta: map[string]string{ "foo": "bar", }, @@ -182,6 +184,7 @@ func TestAgent_sidecarServiceFromNodeService(t *testing.T) { }, }, wantNS: &structs.NodeService{ + EnterpriseMeta: *structs.DefaultEnterpriseMeta(), Kind: structs.ServiceKindConnectProxy, ID: "web1-sidecar-proxy", Service: "web-sidecar-proxy", @@ -271,6 +274,7 @@ func TestAgent_sidecarServiceFromNodeService(t *testing.T) { }, token: "foo", wantNS: &structs.NodeService{ + EnterpriseMeta: *structs.DefaultEnterpriseMeta(), Kind: structs.ServiceKindConnectProxy, ID: "web1-sidecar-proxy", Service: "web-sidecar-proxy", diff --git a/agent/structs/config_entry_discoverychain.go b/agent/structs/config_entry_discoverychain.go index 1830fa375a..a300859733 100644 --- a/agent/structs/config_entry_discoverychain.go +++ b/agent/structs/config_entry_discoverychain.go @@ -204,11 +204,15 @@ func (e *ServiceRouterConfigEntry) ListRelatedServices() []ServiceID { found := make(map[ServiceID]struct{}) // We always inject a default catch-all route to the same service as the router. - found[NewServiceID(e.Name, &e.EnterpriseMeta)] = struct{}{} + svcID := NewServiceID(e.Name, &e.EnterpriseMeta) + found[svcID] = struct{}{} for _, route := range e.Routes { - if route.Destination != nil && route.Destination.Service != "" { - found[NewServiceID(route.Destination.Service, route.Destination.GetEnterpriseMeta(&e.EnterpriseMeta))] = struct{}{} + if route.Destination != nil { + destID := NewServiceID(defaultIfEmpty(route.Destination.Service, e.Name), route.Destination.GetEnterpriseMeta(&e.EnterpriseMeta)) + if destID != svcID { + found[destID] = struct{}{} + } } } @@ -527,9 +531,12 @@ func (e *ServiceSplitterConfigEntry) GetEnterpriseMeta() *EnterpriseMeta { func (e *ServiceSplitterConfigEntry) ListRelatedServices() []ServiceID { found := make(map[ServiceID]struct{}) + svcID := NewServiceID(e.Name, &e.EnterpriseMeta) for _, split := range e.Splits { - if split.Service != "" { - found[NewServiceID(split.Service, split.GetEnterpriseMeta(&e.EnterpriseMeta))] = struct{}{} + splitID := NewServiceID(defaultIfEmpty(split.Service, e.Name), split.GetEnterpriseMeta(&e.EnterpriseMeta)) + + if splitID != svcID { + found[splitID] = struct{}{} } } @@ -830,16 +837,19 @@ func (e *ServiceResolverConfigEntry) GetEnterpriseMeta() *EnterpriseMeta { func (e *ServiceResolverConfigEntry) ListRelatedServices() []ServiceID { found := make(map[ServiceID]struct{}) + svcID := NewServiceID(e.Name, &e.EnterpriseMeta) if e.Redirect != nil { - if e.Redirect.Service != "" { - found[NewServiceID(e.Redirect.Service, e.Redirect.GetEnterpriseMeta(&e.EnterpriseMeta))] = struct{}{} + redirectID := NewServiceID(defaultIfEmpty(e.Redirect.Service, e.Name), e.Redirect.GetEnterpriseMeta(&e.EnterpriseMeta)) + if redirectID != svcID { + found[redirectID] = struct{}{} } } if len(e.Failover) > 0 { for _, failover := range e.Failover { - if failover.Service != "" { - found[NewServiceID(failover.Service, failover.GetEnterpriseMeta(&e.EnterpriseMeta))] = struct{}{} + failoverID := NewServiceID(defaultIfEmpty(failover.Service, e.Name), failover.GetEnterpriseMeta(&e.EnterpriseMeta)) + if failoverID != svcID { + found[failoverID] = struct{}{} } } } @@ -947,8 +957,10 @@ func canReadDiscoveryChain(entry discoveryChainConfigEntry, authz acl.Authorizer } func canWriteDiscoveryChain(entry discoveryChainConfigEntry, rule acl.Authorizer) bool { + entryID := NewServiceID(entry.GetName(), entry.GetEnterpriseMeta()) + var authzContext acl.AuthorizerContext - entry.GetEnterpriseMeta().FillAuthzContext(&authzContext) + entryID.FillAuthzContext(&authzContext) name := entry.GetName() @@ -957,7 +969,7 @@ func canWriteDiscoveryChain(entry discoveryChainConfigEntry, rule acl.Authorizer } for _, svc := range entry.ListRelatedServices() { - if svc.ID == name { + if entryID == svc { continue } @@ -1193,3 +1205,10 @@ func validateServiceSubset(subset string) error { } return nil } + +func defaultIfEmpty(val, defaultVal string) string { + if val != "" { + return val + } + return defaultVal +} diff --git a/agent/structs/connect_proxy_config.go b/agent/structs/connect_proxy_config.go index a526617625..3ead018d1a 100644 --- a/agent/structs/connect_proxy_config.go +++ b/agent/structs/connect_proxy_config.go @@ -356,13 +356,15 @@ func (k UpstreamKey) String() string { // upstream in a canonical but human readable way. func (u *Upstream) Identifier() string { name := u.DestinationName - if u.DestinationNamespace != "" && u.DestinationNamespace != "default" { + typ := u.DestinationType + + if typ != UpstreamDestTypePreparedQuery && u.DestinationNamespace != "" && u.DestinationNamespace != "default" { name = u.DestinationNamespace + "/" + u.DestinationName } if u.Datacenter != "" { name += "?dc=" + u.Datacenter } - typ := u.DestinationType + // Service is default type so never prefix it. This is more readable and long // term it is the only type that matters so we can drop the prefix and have // nicer naming in metrics etc. diff --git a/agent/structs/discovery_chain.go b/agent/structs/discovery_chain.go index 0bddb49e1e..e5a3f72284 100644 --- a/agent/structs/discovery_chain.go +++ b/agent/structs/discovery_chain.go @@ -85,7 +85,7 @@ func (c *CompiledDiscoveryChain) IsDefault() bool { target := c.Targets[node.Resolver.Target] - return target.Service == c.ServiceName + return target.Service == c.ServiceName && target.Namespace == c.Namespace } const ( diff --git a/agent/structs/service_definition.go b/agent/structs/service_definition.go index 43f21d54e2..bc77ec5950 100644 --- a/agent/structs/service_definition.go +++ b/agent/structs/service_definition.go @@ -71,6 +71,8 @@ func (s *ServiceDefinition) NodeService() *NodeService { EnableTagOverride: s.EnableTagOverride, EnterpriseMeta: s.EnterpriseMeta, } + ns.EnterpriseMeta.Normalize() + if s.Connect != nil { ns.Connect = *s.Connect }