From dbb948a6c3cf54bb6f05f292db9dea193140dbd2 Mon Sep 17 00:00:00 2001 From: Derek Menteer <105233703+hashi-derek@users.noreply.github.com> Date: Fri, 20 Jan 2023 11:25:51 -0600 Subject: [PATCH] Fix proxy-defaults incorrectly merging config on upstreams. (#16021) --- agent/configentry/resolve.go | 29 +++++++++++----------------- agent/configentry/resolve_test.go | 11 +++++++++-- agent/consul/config_endpoint_test.go | 20 ++++++++++++++++--- agent/service_manager_test.go | 19 ------------------ 4 files changed, 37 insertions(+), 42 deletions(-) diff --git a/agent/configentry/resolve.go b/agent/configentry/resolve.go index d5e5b7f0a3224..dc20721a578bb 100644 --- a/agent/configentry/resolve.go +++ b/agent/configentry/resolve.go @@ -33,7 +33,6 @@ func ComputeResolvedServiceConfig( // blocking query, this function will be rerun and these state store lookups will both be current. // We use the default enterprise meta to look up the global proxy defaults because they are not namespaced. - var proxyConfGlobalProtocol string proxyConf := entries.GetProxyDefaults(args.PartitionOrDefault()) if proxyConf != nil { // Apply the proxy defaults to the sidecar's proxy config @@ -50,21 +49,19 @@ func ComputeResolvedServiceConfig( thisReply.EnvoyExtensions = proxyConf.EnvoyExtensions thisReply.AccessLogs = proxyConf.AccessLogs - parsed, err := structs.ParseUpstreamConfigNoDefaults(mapCopy.(map[string]interface{})) - if err != nil { - return nil, fmt.Errorf("failed to parse upstream config map for proxy-defaults: %v", err) - } - proxyConfGlobalProtocol = parsed.Protocol - + // Only MeshGateway and Protocol should affect upstreams. // MeshGateway is strange. It's marshaled into UpstreamConfigs via the arbitrary map, but it // uses concrete fields everywhere else. We always take the explicit definition here for // wildcard upstreams and discard the user setting it via arbitrary map in proxy-defaults. - if err := mergo.MergeWithOverwrite(&wildcardUpstreamDefaults, mapCopy); err != nil { - return nil, fmt.Errorf("failed to merge upstream config map for proxy-defaults: %v", err) + if mgw, ok := thisReply.ProxyConfig["mesh_gateway"]; ok { + wildcardUpstreamDefaults["mesh_gateway"] = mgw } if !proxyConf.MeshGateway.IsZero() { wildcardUpstreamDefaults["mesh_gateway"] = proxyConf.MeshGateway } + if protocol, ok := thisReply.ProxyConfig["protocol"]; ok { + wildcardUpstreamDefaults["protocol"] = protocol + } } serviceConf := entries.GetServiceDefaults( @@ -149,9 +146,7 @@ func ComputeResolvedServiceConfig( // First store all upstreams that were provided in the request for _, sid := range upstreamIDs { - if _, ok := seenUpstreams[sid]; !ok { - seenUpstreams[sid] = struct{}{} - } + seenUpstreams[sid] = struct{}{} } // Then store upstreams inferred from service-defaults and mapify the overrides. @@ -204,21 +199,19 @@ func ComputeResolvedServiceConfig( // 2. Protocol for upstream service defined in its service-defaults (how the upstream wants to be addressed) // 3. Protocol defined for the upstream in the service-defaults.(upstream_config.defaults|upstream_config.overrides) of the downstream // (how the downstream wants to address it) - protocol := proxyConfGlobalProtocol + if err := mergo.MergeWithOverwrite(&resolvedCfg, wildcardUpstreamDefaults); err != nil { + return nil, fmt.Errorf("failed to merge wildcard defaults into upstream: %v", err) + } upstreamSvcDefaults := entries.GetServiceDefaults( structs.NewServiceID(upstream.ID, &upstream.EnterpriseMeta), ) if upstreamSvcDefaults != nil { if upstreamSvcDefaults.Protocol != "" { - protocol = upstreamSvcDefaults.Protocol + resolvedCfg["protocol"] = upstreamSvcDefaults.Protocol } } - if protocol != "" { - resolvedCfg["protocol"] = protocol - } - // When dialing an upstream, the goal is to flatten the mesh gateway mode in this order // (larger number wins): // 1. Value from the proxy-defaults diff --git a/agent/configentry/resolve_test.go b/agent/configentry/resolve_test.go index cc07712b4b70a..96590cdc88bbb 100644 --- a/agent/configentry/resolve_test.go +++ b/agent/configentry/resolve_test.go @@ -141,8 +141,10 @@ func Test_ComputeResolvedServiceConfig(t *testing.T) { name: "proxy inherits kitchen sink from proxy-defaults", args: args{ scReq: &structs.ServiceConfigRequest{ - Name: "sid", + Name: "sid", + UpstreamIDs: uids, }, + upstreamIDs: uids, entries: &ResolvedServiceConfigSet{ ProxyDefaults: map[string]*structs.ProxyConfigEntry{ acl.DefaultEnterpriseMeta().PartitionOrDefault(): { @@ -195,7 +197,12 @@ func Test_ComputeResolvedServiceConfig(t *testing.T) { { Upstream: wildcard, Config: map[string]interface{}{ - "foo": "bar", + "mesh_gateway": remoteMeshGW, + }, + }, + { + Upstream: uid, + Config: map[string]interface{}{ "mesh_gateway": remoteMeshGW, }, }, diff --git a/agent/consul/config_endpoint_test.go b/agent/consul/config_endpoint_test.go index 34e9ce72b3001..63e8577359c01 100644 --- a/agent/consul/config_endpoint_test.go +++ b/agent/consul/config_endpoint_test.go @@ -1025,8 +1025,9 @@ func TestConfigEntry_ResolveServiceConfig(t *testing.T) { // Create a dummy proxy/service config in the state store to look up. state := s1.fsm.State() require.NoError(t, state.EnsureConfigEntry(1, &structs.ProxyConfigEntry{ - Kind: structs.ProxyDefaults, - Name: structs.ProxyConfigGlobal, + Kind: structs.ProxyDefaults, + Name: structs.ProxyConfigGlobal, + MeshGateway: structs.MeshGatewayConfig{Mode: structs.MeshGatewayModeLocal}, Config: map[string]interface{}{ "foo": 1, }, @@ -1056,12 +1057,25 @@ func TestConfigEntry_ResolveServiceConfig(t *testing.T) { "foo": int64(1), "protocol": "http", }, + MeshGateway: structs.MeshGatewayConfig{ + Mode: structs.MeshGatewayModeLocal, + }, UpstreamConfigs: map[string]map[string]interface{}{ "*": { - "foo": int64(1), + "mesh_gateway": map[string]interface{}{ + "Mode": "local", + }, }, "bar": { "protocol": "grpc", + "mesh_gateway": map[string]interface{}{ + "Mode": "local", + }, + }, + "baz": { + "mesh_gateway": map[string]interface{}{ + "Mode": "local", + }, }, }, Meta: map[string]string{"foo": "bar"}, diff --git a/agent/service_manager_test.go b/agent/service_manager_test.go index 6760627e0eed5..2015344afe338 100644 --- a/agent/service_manager_test.go +++ b/agent/service_manager_test.go @@ -11,7 +11,6 @@ import ( "github.com/stretchr/testify/require" - "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/sdk/testutil/retry" "github.com/hashicorp/consul/testrpc" @@ -421,12 +420,6 @@ func TestServiceManager_PersistService_API(t *testing.T) { "protocol": "http", }, UpstreamIDConfigs: structs.OpaqueUpstreamConfigs{ - { - Upstream: structs.NewServiceID(structs.WildcardSpecifier, acl.DefaultEnterpriseMeta().WithWildcardNamespace()), - Config: map[string]interface{}{ - "foo": int64(1), - }, - }, { Upstream: structs.NewServiceID("redis", nil), Config: map[string]interface{}{ @@ -475,12 +468,6 @@ func TestServiceManager_PersistService_API(t *testing.T) { "protocol": "http", }, UpstreamIDConfigs: structs.OpaqueUpstreamConfigs{ - { - Upstream: structs.NewServiceID(structs.WildcardSpecifier, acl.DefaultEnterpriseMeta().WithWildcardNamespace()), - Config: map[string]interface{}{ - "foo": int64(1), - }, - }, { Upstream: structs.NewServiceID("redis", nil), Config: map[string]interface{}{ @@ -660,12 +647,6 @@ func TestServiceManager_PersistService_ConfigFiles(t *testing.T) { "protocol": "http", }, UpstreamIDConfigs: structs.OpaqueUpstreamConfigs{ - { - Upstream: structs.NewServiceID(structs.WildcardSpecifier, acl.DefaultEnterpriseMeta().WithWildcardNamespace()), - Config: map[string]interface{}{ - "foo": int64(1), - }, - }, { Upstream: structs.NewServiceID("redis", nil), Config: map[string]interface{}{