Fix proxy-defaults incorrectly merging config on upstreams. (#16021)

This commit is contained in:
Derek Menteer 2023-01-20 11:25:51 -06:00 committed by GitHub
parent feacfb2886
commit bb6951f99d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 37 additions and 42 deletions

View File

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

View File

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

View File

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

View File

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