luxolus
/
open-consul
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

Remove deprecated service-defaults upstream behavior. (#16957) 

Prior to this change, peer services would be targeted by service-default
overrides as long as the new `peer` field was not found in the config entry.
This commit removes that deprecated backwards-compatibility behavior. Now
it is necessary to specify the `peer` field in order for upstream overrides
to apply to a peer upstream.
This commit is contained in:
Derek Menteer 2023-04-11 10:20:33 -05:00 committed by GitHub
parent 8d0d600ea3
commit 2a13c9af1f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 64 additions and 259 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
.changelog/16957.txt Normal file
View File

@ -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.
```

48
agent/configentry/merge_service_config.go
View File

@ -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,
}
}

55
agent/configentry/resolve.go
View File

@ -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

85
agent/consul/config_endpoint_test.go
View File

@ -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

5
agent/proxycfg-glue/resolved_service_config.go
View File

@ -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)

3
agent/service_manager_test.go
View File

@ -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()
})

49
agent/structs/config_entry.go
View File

@ -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 {

51
agent/structs/config_entry_test.go
View File

@ -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,

12
agent/structs/structs.deepcopy.go
View File

@ -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)

10
website/content/docs/upgrading/upgrade-specific.mdx
View File

@ -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