diff --git a/.changelog/16957.txt b/.changelog/16957.txt new file mode 100644 index 000000000..83b7dc245 --- /dev/null +++ b/.changelog/16957.txt @@ -0,0 +1,5 @@ +```release-note:breaking-change +peering: Removed deprecated backward-compatibility behavior. +Upstream overrides in service-defaults will now only apply to peer upstreams when the `peer` field is provided. +Visit the 1.16.x [upgrade instructions](https://developer.hashicorp.com/consul/docs/upgrading/upgrade-specific) for more information. +``` diff --git a/agent/configentry/merge_service_config.go b/agent/configentry/merge_service_config.go index ace4f4617..ee6b7cf24 100644 --- a/agent/configentry/merge_service_config.go +++ b/agent/configentry/merge_service_config.go @@ -157,44 +157,20 @@ func MergeServiceConfig(defaults *structs.ServiceConfigResponse, service *struct // remoteUpstreams contains synthetic Upstreams generated from central config (service-defaults.UpstreamConfigs). remoteUpstreams := make(map[structs.PeeredServiceName]structs.Upstream) - if len(defaults.UpstreamIDConfigs) > 0 { - // Handle legacy upstreams. This should be removed in Consul 1.16. - for _, us := range defaults.UpstreamIDConfigs { - parsed, err := structs.ParseUpstreamConfigNoDefaults(us.Config) - if err != nil { - return nil, fmt.Errorf("failed to parse upstream config map for %s: %v", us.Upstream.String(), err) - } - psn := structs.PeeredServiceName{ - Peer: "", - ServiceName: structs.NewServiceName(us.Upstream.ID, &us.Upstream.EnterpriseMeta), - } - - remoteUpstreams[psn] = structs.Upstream{ - DestinationNamespace: us.Upstream.NamespaceOrDefault(), - DestinationPartition: us.Upstream.PartitionOrDefault(), - DestinationName: us.Upstream.ID, - DestinationPeer: "", - Config: us.Config, - MeshGateway: parsed.MeshGateway, - CentrallyConfigured: true, - } + for _, us := range defaults.UpstreamConfigs { + parsed, err := structs.ParseUpstreamConfigNoDefaults(us.Config) + if err != nil { + return nil, fmt.Errorf("failed to parse upstream config map for %s: %v", us.Upstream.String(), err) } - } else { - for _, us := range defaults.UpstreamConfigs { - parsed, err := structs.ParseUpstreamConfigNoDefaults(us.Config) - if err != nil { - return nil, fmt.Errorf("failed to parse upstream config map for %s: %v", us.Upstream.String(), err) - } - remoteUpstreams[us.Upstream] = structs.Upstream{ - DestinationNamespace: us.Upstream.ServiceName.NamespaceOrDefault(), - DestinationPartition: us.Upstream.ServiceName.PartitionOrDefault(), - DestinationName: us.Upstream.ServiceName.Name, - DestinationPeer: us.Upstream.Peer, - Config: us.Config, - MeshGateway: parsed.MeshGateway, - CentrallyConfigured: true, - } + remoteUpstreams[us.Upstream] = structs.Upstream{ + DestinationNamespace: us.Upstream.ServiceName.NamespaceOrDefault(), + DestinationPartition: us.Upstream.ServiceName.PartitionOrDefault(), + DestinationName: us.Upstream.ServiceName.Name, + DestinationPeer: us.Upstream.Peer, + Config: us.Config, + MeshGateway: parsed.MeshGateway, + CentrallyConfigured: true, } } diff --git a/agent/configentry/resolve.go b/agent/configentry/resolve.go index da877279e..c7c03d40d 100644 --- a/agent/configentry/resolve.go +++ b/agent/configentry/resolve.go @@ -133,7 +133,7 @@ func ComputeResolvedServiceConfig( seenUpstreams := map[structs.PeeredServiceName]struct{}{} var ( - noUpstreamArgs = len(args.UpstreamServiceNames) == 0 && len(args.UpstreamIDs) == 0 + noUpstreamArgs = len(args.UpstreamServiceNames) == 0 // Check the args and the resolved value. If it was exclusively set via a config entry, then args.Mode // will never be transparent because the service config request does not use the resolved value. @@ -153,15 +153,6 @@ func ComputeResolvedServiceConfig( seenUpstreams[psn] = struct{}{} } } - // For 1.14, service-defaults overrides would apply to peer upstreams incorrectly - // because the config merging logic was oblivious to the concept of a peer. - // We replicate this behavior on legacy calls for backwards-compatibility. - for _, sid := range args.UpstreamIDs { - psn := structs.PeeredServiceName{ - ServiceName: structs.NewServiceName(sid.ID, &sid.EnterpriseMeta), - } - seenUpstreams[psn] = struct{}{} - } // Then store upstreams inferred from service-defaults and mapify the overrides. var ( @@ -206,22 +197,6 @@ func ComputeResolvedServiceConfig( resolvedConfigs[wildcard] = wildcardUpstreamDefaults } - // For Consul 1.14.x, service-defaults would apply to either local or peer services as long - // as the `name` matched. We introduce `legacyUpstreams` as a compatibility mode for: - // 1. old agents, that are using the deprecated UpstreamIDs api - // 2. Migrations to 1.15 that do not specify the "peer" field. The behavior should remain the same - // until the config entries are updates. - // - // This should be remove in Consul 1.16 - var hasPeerUpstream bool - for _, override := range upstreamOverrides { - if override.Peer != "" { - hasPeerUpstream = true - break - } - } - legacyUpstreams := len(args.UpstreamIDs) > 0 || !hasPeerUpstream - for upstream := range seenUpstreams { resolvedCfg := make(map[string]interface{}) @@ -273,16 +248,6 @@ func ComputeResolvedServiceConfig( } // Merge in Overrides for the upstream (step 5). - // In the legacy case, overrides only match on name. We remove the peer and try to match against - // our map of overrides. We still want to check the full PSN in the map in case there is a specific - // override that applies to peers. - if legacyUpstreams { - peerlessUpstream := upstream - peerlessUpstream.Peer = "" - if upstreamOverrides[peerlessUpstream] != nil { - upstreamOverrides[peerlessUpstream].MergeInto(resolvedCfg) - } - } if upstreamOverrides[upstream] != nil { upstreamOverrides[upstream].MergeInto(resolvedCfg) } @@ -297,21 +262,11 @@ func ComputeResolvedServiceConfig( return &thisReply, nil } - if len(args.UpstreamIDs) > 0 { - // DEPRECATED: Remove these legacy upstreams in Consul v1.16 - thisReply.UpstreamIDConfigs = make(structs.OpaqueUpstreamConfigsDeprecated, 0, len(resolvedConfigs)) + thisReply.UpstreamConfigs = make(structs.OpaqueUpstreamConfigs, 0, len(resolvedConfigs)) - for us, conf := range resolvedConfigs { - thisReply.UpstreamIDConfigs = append(thisReply.UpstreamIDConfigs, - structs.OpaqueUpstreamConfigDeprecated{Upstream: us.ServiceName.ToServiceID(), Config: conf}) - } - } else { - thisReply.UpstreamConfigs = make(structs.OpaqueUpstreamConfigs, 0, len(resolvedConfigs)) - - for us, conf := range resolvedConfigs { - thisReply.UpstreamConfigs = append(thisReply.UpstreamConfigs, - structs.OpaqueUpstreamConfig{Upstream: us, Config: conf}) - } + for us, conf := range resolvedConfigs { + thisReply.UpstreamConfigs = append(thisReply.UpstreamConfigs, + structs.OpaqueUpstreamConfig{Upstream: us, Config: conf}) } return &thisReply, nil diff --git a/agent/consul/config_endpoint_test.go b/agent/consul/config_endpoint_test.go index 85170378a..4772e10de 100644 --- a/agent/consul/config_endpoint_test.go +++ b/agent/consul/config_endpoint_test.go @@ -1344,66 +1344,6 @@ func TestConfigEntry_ResolveServiceConfig_Upstreams(t *testing.T) { }, }, }, - { - name: "upstream config entries from UpstreamIDs and service-defaults", - entries: []structs.ConfigEntry{ - &structs.ProxyConfigEntry{ - Kind: structs.ProxyDefaults, - Name: structs.ProxyConfigGlobal, - Config: map[string]interface{}{ - "protocol": "grpc", - }, - }, - &structs.ServiceConfigEntry{ - Kind: structs.ServiceDefaults, - Name: "api", - UpstreamConfig: &structs.UpstreamConfiguration{ - Overrides: []*structs.UpstreamConfig{ - { - Name: "mysql", - Peer: "peer1", // This should be ignored for legacy UpstreamIDs mode - Protocol: "http", - }, - }, - }, - }, - }, - request: structs.ServiceConfigRequest{ - Name: "api", - Datacenter: "dc1", - UpstreamIDs: []structs.ServiceID{ - structs.NewServiceID("cache", nil), - }, - }, - expect: structs.ServiceConfigResponse{ - ProxyConfig: map[string]interface{}{ - "protocol": "grpc", - }, - UpstreamIDConfigs: structs.OpaqueUpstreamConfigsDeprecated{ - { - Upstream: structs.NewServiceID( - structs.WildcardSpecifier, - acl.DefaultEnterpriseMeta().WithWildcardNamespace(), - ), - Config: map[string]interface{}{ - "protocol": "grpc", - }, - }, - { - Upstream: structs.NewServiceID("cache", nil), - Config: map[string]interface{}{ - "protocol": "grpc", - }, - }, - { - Upstream: structs.NewServiceID("mysql", nil), - Config: map[string]interface{}{ - "protocol": "http", - }, - }, - }, - }, - }, { name: "upstream config entries from UpstreamServiceNames and service-defaults", entries: []structs.ConfigEntry{ @@ -1764,9 +1704,6 @@ func TestConfigEntry_ResolveServiceConfig_Upstreams(t *testing.T) { sort.SliceStable(out.UpstreamConfigs, func(i, j int) bool { return out.UpstreamConfigs[i].Upstream.String() < out.UpstreamConfigs[j].Upstream.String() }) - sort.SliceStable(out.UpstreamIDConfigs, func(i, j int) bool { - return out.UpstreamIDConfigs[i].Upstream.String() < out.UpstreamIDConfigs[j].Upstream.String() - }) require.Equal(t, tc.expect, out) }) @@ -2037,9 +1974,9 @@ func TestConfigEntry_ResolveServiceConfig_Upstreams_Blocking(t *testing.T) { &structs.ServiceConfigRequest{ Name: "foo", Datacenter: "dc1", - UpstreamIDs: []structs.ServiceID{ - structs.NewServiceID("bar", nil), - structs.NewServiceID("other", nil), + UpstreamServiceNames: []structs.PeeredServiceName{ + {ServiceName: structs.NewServiceName("bar", nil)}, + {ServiceName: structs.NewServiceName("other", nil)}, }, QueryOptions: structs.QueryOptions{ MinQueryIndex: index, @@ -2073,9 +2010,9 @@ func TestConfigEntry_ResolveServiceConfig_Upstreams_Blocking(t *testing.T) { &structs.ServiceConfigRequest{ Name: "foo", Datacenter: "dc1", - UpstreamIDs: []structs.ServiceID{ - structs.NewServiceID("bar", nil), - structs.NewServiceID("other", nil), + UpstreamServiceNames: []structs.PeeredServiceName{ + {ServiceName: structs.NewServiceName("bar", nil)}, + {ServiceName: structs.NewServiceName("other", nil)}, }, }, &out, @@ -2116,9 +2053,9 @@ func TestConfigEntry_ResolveServiceConfig_Upstreams_Blocking(t *testing.T) { &structs.ServiceConfigRequest{ Name: "foo", Datacenter: "dc1", - UpstreamIDs: []structs.ServiceID{ - structs.NewServiceID("bar", nil), - structs.NewServiceID("other", nil), + UpstreamServiceNames: []structs.PeeredServiceName{ + {ServiceName: structs.NewServiceName("bar", nil)}, + {ServiceName: structs.NewServiceName("other", nil)}, }, QueryOptions: structs.QueryOptions{ MinQueryIndex: index, @@ -2355,8 +2292,8 @@ func TestConfigEntry_ResolveServiceConfig_BlockOnNoChange(t *testing.T) { func(minQueryIndex uint64) (*structs.QueryMeta, <-chan error) { args := structs.ServiceConfigRequest{ Name: "foo", - UpstreamIDs: []structs.ServiceID{ - structs.NewServiceID("bar", nil), + UpstreamServiceNames: []structs.PeeredServiceName{ + {ServiceName: structs.NewServiceName("bar", nil)}, }, } args.QueryOptions.MinQueryIndex = minQueryIndex diff --git a/agent/proxycfg-glue/resolved_service_config.go b/agent/proxycfg-glue/resolved_service_config.go index 4484e2a27..7289db090 100644 --- a/agent/proxycfg-glue/resolved_service_config.go +++ b/agent/proxycfg-glue/resolved_service_config.go @@ -5,7 +5,6 @@ package proxycfgglue import ( "context" - "errors" "github.com/hashicorp/go-memdb" @@ -40,10 +39,6 @@ func (s *serverResolvedServiceConfig) Notify(ctx context.Context, req *structs.S return s.remoteSource.Notify(ctx, req, correlationID, ch) } - if len(req.UpstreamIDs) != 0 { - return errors.New("ServerResolvedServiceConfig does not support the legacy UpstreamIDs parameter") - } - return watch.ServerLocalNotify(ctx, correlationID, s.deps.GetStore, func(ws memdb.WatchSet, store Store) (uint64, *structs.ServiceConfigResponse, error) { authz, err := s.deps.ACLResolver.ResolveTokenAndDefaultMeta(req.Token, &req.EnterpriseMeta, nil) diff --git a/agent/service_manager_test.go b/agent/service_manager_test.go index 1a4c7f6de..724022d42 100644 --- a/agent/service_manager_test.go +++ b/agent/service_manager_test.go @@ -829,9 +829,6 @@ func fixPersistedServiceConfigForTest(content []byte) ([]byte, error) { return nil, err } // Sort the output, since it's randomized and causes flaky tests otherwise. - sort.Slice(parsed.Defaults.UpstreamIDConfigs, func(i, j int) bool { - return parsed.Defaults.UpstreamIDConfigs[i].Upstream.ID < parsed.Defaults.UpstreamIDConfigs[j].Upstream.ID - }) sort.Slice(parsed.Defaults.UpstreamConfigs, func(i, j int) bool { return parsed.Defaults.UpstreamConfigs[i].Upstream.String() < parsed.Defaults.UpstreamConfigs[j].Upstream.String() }) diff --git a/agent/structs/config_entry.go b/agent/structs/config_entry.go index 81efa5286..d8b3e39e9 100644 --- a/agent/structs/config_entry.go +++ b/agent/structs/config_entry.go @@ -775,11 +775,6 @@ type ServiceConfigRequest struct { // UpstreamServiceNames is a list of upstream service names to use for resolving the service config. UpstreamServiceNames []PeeredServiceName - // DEPRECATED - // UpstreamIDs is a list of upstream service names to use for resolving the service config. - // This field does not take the peer name into consideration, so it is deprecated. - UpstreamIDs []ServiceID - acl.EnterpriseMeta `hcl:",squash" mapstructure:",squash"` QueryOptions } @@ -792,12 +787,6 @@ func (s *ServiceConfigRequest) RequestDatacenter() string { // This is often used for fetching service-defaults config entries. func (s *ServiceConfigRequest) GetLocalUpstreamIDs() []ServiceID { var upstreams []ServiceID - if len(s.UpstreamIDs) > 0 { - // The legacy mode is mutually exclusive with the new mode, so we don't need - // to join the two lists together. - upstreams = append(upstreams, s.UpstreamIDs...) - return upstreams - } for i := range s.UpstreamServiceNames { u := &s.UpstreamServiceNames[i] if u.Peer == "" { @@ -826,7 +815,6 @@ func (r *ServiceConfigRequest) CacheInfo() cache.RequestInfo { Name string EnterpriseMeta acl.EnterpriseMeta UpstreamServiceNames []PeeredServiceName `hash:"set"` - UpstreamIDs []ServiceID `hash:"set"` MeshGatewayConfig MeshGatewayConfig ProxyMode ProxyMode Filter string @@ -834,7 +822,6 @@ func (r *ServiceConfigRequest) CacheInfo() cache.RequestInfo { Name: r.Name, EnterpriseMeta: r.EnterpriseMeta, UpstreamServiceNames: r.UpstreamServiceNames, - UpstreamIDs: r.UpstreamIDs, ProxyMode: r.Mode, MeshGatewayConfig: r.MeshGateway, Filter: r.QueryOptions.Filter, @@ -1153,12 +1140,6 @@ func (ul UpstreamLimits) Validate() error { return nil } -type OpaqueUpstreamConfigDeprecated struct { - Upstream ServiceID - Config map[string]interface{} -} -type OpaqueUpstreamConfigsDeprecated []OpaqueUpstreamConfigDeprecated - type OpaqueUpstreamConfig struct { Upstream PeeredServiceName Config map[string]interface{} @@ -1166,19 +1147,16 @@ type OpaqueUpstreamConfig struct { type OpaqueUpstreamConfigs []OpaqueUpstreamConfig type ServiceConfigResponse struct { - ProxyConfig map[string]interface{} - // DEPRECATED: UpstreamIDConfigs field exists only for backwards-compatibility - // during upgrades and should be removed in Consul 1.16. - UpstreamIDConfigs OpaqueUpstreamConfigsDeprecated - UpstreamConfigs OpaqueUpstreamConfigs - MeshGateway MeshGatewayConfig `json:",omitempty"` - Expose ExposeConfig `json:",omitempty"` - TransparentProxy TransparentProxyConfig `json:",omitempty"` - Mode ProxyMode `json:",omitempty"` - Destination DestinationConfig `json:",omitempty"` - AccessLogs AccessLogsConfig `json:",omitempty"` - Meta map[string]string `json:",omitempty"` - EnvoyExtensions []EnvoyExtension `json:",omitempty"` + ProxyConfig map[string]interface{} + UpstreamConfigs OpaqueUpstreamConfigs + MeshGateway MeshGatewayConfig `json:",omitempty"` + Expose ExposeConfig `json:",omitempty"` + TransparentProxy TransparentProxyConfig `json:",omitempty"` + Mode ProxyMode `json:",omitempty"` + Destination DestinationConfig `json:",omitempty"` + AccessLogs AccessLogsConfig `json:",omitempty"` + Meta map[string]string `json:",omitempty"` + EnvoyExtensions []EnvoyExtension `json:",omitempty"` QueryMeta } @@ -1222,13 +1200,6 @@ func (r *ServiceConfigResponse) UnmarshalBinary(data []byte) error { return err } - for k := range r.UpstreamIDConfigs { - r.UpstreamIDConfigs[k].Config, err = lib.MapWalk(r.UpstreamIDConfigs[k].Config) - if err != nil { - return err - } - } - for k := range r.UpstreamConfigs { r.UpstreamConfigs[k].Config, err = lib.MapWalk(r.UpstreamConfigs[k].Config) if err != nil { diff --git a/agent/structs/config_entry_test.go b/agent/structs/config_entry_test.go index de9e5ddc9..a77d36b55 100644 --- a/agent/structs/config_entry_test.go +++ b/agent/structs/config_entry_test.go @@ -2146,13 +2146,6 @@ func TestDecodeConfigEntry(t *testing.T) { func TestServiceConfigRequest(t *testing.T) { - makeLegacyUpstreamIDs := func(services ...string) []ServiceID { - u := make([]ServiceID, 0, len(services)) - for _, s := range services { - u = append(u, NewServiceID(s, acl.DefaultEnterpriseMeta())) - } - return u - } tests := []struct { name string req ServiceConfigRequest @@ -2182,40 +2175,18 @@ func TestServiceConfigRequest(t *testing.T) { }, wantSame: false, }, - { - name: "legacy upstreams should be different", - req: ServiceConfigRequest{ - Name: "web", - UpstreamIDs: makeLegacyUpstreamIDs("foo"), - }, - mutate: func(req *ServiceConfigRequest) { - req.UpstreamIDs = makeLegacyUpstreamIDs("foo", "bar") - }, - wantSame: false, - }, - { - name: "legacy upstreams should not depend on order", - req: ServiceConfigRequest{ - Name: "web", - UpstreamIDs: makeLegacyUpstreamIDs("bar", "foo"), - }, - mutate: func(req *ServiceConfigRequest) { - req.UpstreamIDs = makeLegacyUpstreamIDs("foo", "bar") - }, - wantSame: true, - }, { name: "upstreams should be different", req: ServiceConfigRequest{ Name: "web", - UpstreamIDs: []ServiceID{ - NewServiceID("foo", nil), + UpstreamServiceNames: []PeeredServiceName{ + {ServiceName: NewServiceName("foo", nil)}, }, }, mutate: func(req *ServiceConfigRequest) { - req.UpstreamIDs = []ServiceID{ - NewServiceID("foo", nil), - NewServiceID("bar", nil), + req.UpstreamServiceNames = []PeeredServiceName{ + {ServiceName: NewServiceName("foo", nil)}, + {ServiceName: NewServiceName("bar", nil)}, } }, wantSame: false, @@ -2224,15 +2195,15 @@ func TestServiceConfigRequest(t *testing.T) { name: "upstreams should not depend on order", req: ServiceConfigRequest{ Name: "web", - UpstreamIDs: []ServiceID{ - NewServiceID("bar", nil), - NewServiceID("foo", nil), + UpstreamServiceNames: []PeeredServiceName{ + {ServiceName: NewServiceName("foo", nil)}, + {ServiceName: NewServiceName("bar", nil)}, }, }, mutate: func(req *ServiceConfigRequest) { - req.UpstreamIDs = []ServiceID{ - NewServiceID("foo", nil), - NewServiceID("bar", nil), + req.UpstreamServiceNames = []PeeredServiceName{ + {ServiceName: NewServiceName("foo", nil)}, + {ServiceName: NewServiceName("bar", nil)}, } }, wantSame: true, diff --git a/agent/structs/structs.deepcopy.go b/agent/structs/structs.deepcopy.go index 95e50b453..988349fdd 100644 --- a/agent/structs/structs.deepcopy.go +++ b/agent/structs/structs.deepcopy.go @@ -783,18 +783,6 @@ func (o *ServiceConfigResponse) DeepCopy() *ServiceConfigResponse { cp.ProxyConfig[k2] = v2 } } - if o.UpstreamIDConfigs != nil { - cp.UpstreamIDConfigs = make([]OpaqueUpstreamConfigDeprecated, len(o.UpstreamIDConfigs)) - copy(cp.UpstreamIDConfigs, o.UpstreamIDConfigs) - for i2 := range o.UpstreamIDConfigs { - if o.UpstreamIDConfigs[i2].Config != nil { - cp.UpstreamIDConfigs[i2].Config = make(map[string]interface{}, len(o.UpstreamIDConfigs[i2].Config)) - for k4, v4 := range o.UpstreamIDConfigs[i2].Config { - cp.UpstreamIDConfigs[i2].Config[k4] = v4 - } - } - } - } if o.UpstreamConfigs != nil { cp.UpstreamConfigs = make([]OpaqueUpstreamConfig, len(o.UpstreamConfigs)) copy(cp.UpstreamConfigs, o.UpstreamConfigs) diff --git a/website/content/docs/upgrading/upgrade-specific.mdx b/website/content/docs/upgrading/upgrade-specific.mdx index 1c9a73849..8a73a721b 100644 --- a/website/content/docs/upgrading/upgrade-specific.mdx +++ b/website/content/docs/upgrading/upgrade-specific.mdx @@ -14,6 +14,16 @@ provided for their upgrades as a result of new features or changed behavior. This page is used to document those details separately from the standard upgrade flow. +## Consul 1.16.x + +#### Remove deprecated service-defaults peer upstream override behavior + +In Consul 1.16.x, `service-defaults` upstream [`overrides`](/consul/docs/connect/config-entries/service-defaults#overrides) +will not apply to peer upstreams, unless the [`peer`](/consul/docs/connect/config-entries/service-defaults#peer) field is explicitly provided. + +Consul 1.15.x introduced a backward-compatibility behavior to allow for a smoother transition during the upgrade. This behavior will be removed in Consul 1.16.x. +Visit the [upgrade instructions for 1.15.x](#service-defaults-overrides-for-upstream-peered-services) for more info. + ## Consul 1.15.x #### Service mesh known issue