backport of commit fe5c145cddd89da804d0de7d19bdc5d0d276df3b (#18010)
Co-authored-by: Derek Menteer <derek.menteer@hashicorp.com>
This commit is contained in:
parent
810870d1c8
commit
3dcc3cb95a
|
@ -0,0 +1,3 @@
|
||||||
|
```release-note:bug
|
||||||
|
connect: Fix incorrect protocol config merging for transparent proxy implicit upstreams.
|
||||||
|
```
|
|
@ -36,6 +36,7 @@ func ComputeResolvedServiceConfig(
|
||||||
// blocking query, this function will be rerun and these state store lookups will both be current.
|
// 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.
|
// 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())
|
proxyConf := entries.GetProxyDefaults(args.PartitionOrDefault())
|
||||||
if proxyConf != nil {
|
if proxyConf != nil {
|
||||||
// Apply the proxy defaults to the sidecar's proxy config
|
// Apply the proxy defaults to the sidecar's proxy config
|
||||||
|
@ -63,9 +64,30 @@ func ComputeResolvedServiceConfig(
|
||||||
if !proxyConf.MeshGateway.IsZero() {
|
if !proxyConf.MeshGateway.IsZero() {
|
||||||
wildcardUpstreamDefaults["mesh_gateway"] = proxyConf.MeshGateway
|
wildcardUpstreamDefaults["mesh_gateway"] = proxyConf.MeshGateway
|
||||||
}
|
}
|
||||||
if protocol, ok := thisReply.ProxyConfig["protocol"]; ok {
|
|
||||||
wildcardUpstreamDefaults["protocol"] = protocol
|
// We explicitly DO NOT merge the protocol from proxy-defaults into the wildcard upstream here.
|
||||||
|
// TProxy will try to use the data from the `wildcardUpstreamDefaults` as a source of truth, which is
|
||||||
|
// normally correct to inherit from proxy-defaults. However, it is NOT correct for protocol.
|
||||||
|
//
|
||||||
|
// This edge-case is different for `protocol` from other fields, since the protocol can be
|
||||||
|
// set on both the local `ServiceDefaults.UpstreamOverrides` and upstream `ServiceDefaults.Protocol`.
|
||||||
|
// This means that when proxy-defaults is set, it would always be treated as an explicit override,
|
||||||
|
// and take precedence over the protocol that is set on the discovery chain (which comes from the
|
||||||
|
// service's preference in its service-defaults), which is wrong.
|
||||||
|
//
|
||||||
|
// When the upstream is not explicitly defined, we should only get the protocol from one of these locations:
|
||||||
|
// 1. For tproxy non-peering services, it can be fetched via the discovery chain.
|
||||||
|
// The chain compiler merges the proxy-defaults protocol with the upstream's preferred service-defaults protocol.
|
||||||
|
// 2. For tproxy non-peering services with default upstream overrides, it will come from the wildcard upstream overrides.
|
||||||
|
// 3. For tproxy non-peering services with specific upstream overrides, it will come from the specific upstream override defined.
|
||||||
|
// 4. For tproxy peering services, they do not honor the proxy-defaults, since they reside in a different cluster.
|
||||||
|
// The data will come from a separate peerMeta field.
|
||||||
|
// In all of these cases, it is not necessary for the proxy-defaults to exist in the wildcard upstream.
|
||||||
|
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
|
||||||
}
|
}
|
||||||
|
|
||||||
serviceConf := entries.GetServiceDefaults(
|
serviceConf := entries.GetServiceDefaults(
|
||||||
|
@ -210,6 +232,10 @@ func ComputeResolvedServiceConfig(
|
||||||
// 2. Protocol for upstream service defined in its service-defaults (how the upstream wants to be addressed)
|
// 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
|
// 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)
|
// (how the downstream wants to address it)
|
||||||
|
if proxyConfGlobalProtocol != "" {
|
||||||
|
resolvedCfg["protocol"] = proxyConfGlobalProtocol
|
||||||
|
}
|
||||||
|
|
||||||
if err := mergo.MergeWithOverwrite(&resolvedCfg, wildcardUpstreamDefaults); err != nil {
|
if err := mergo.MergeWithOverwrite(&resolvedCfg, wildcardUpstreamDefaults); err != nil {
|
||||||
return nil, fmt.Errorf("failed to merge wildcard defaults into upstream: %v", err)
|
return nil, fmt.Errorf("failed to merge wildcard defaults into upstream: %v", err)
|
||||||
}
|
}
|
||||||
|
|
|
@ -1444,16 +1444,6 @@ func TestConfigEntry_ResolveServiceConfig_Upstreams(t *testing.T) {
|
||||||
"protocol": "grpc",
|
"protocol": "grpc",
|
||||||
},
|
},
|
||||||
UpstreamConfigs: structs.OpaqueUpstreamConfigs{
|
UpstreamConfigs: structs.OpaqueUpstreamConfigs{
|
||||||
{
|
|
||||||
Upstream: structs.PeeredServiceName{
|
|
||||||
ServiceName: structs.NewServiceName(
|
|
||||||
structs.WildcardSpecifier,
|
|
||||||
acl.DefaultEnterpriseMeta().WithWildcardNamespace()),
|
|
||||||
},
|
|
||||||
Config: map[string]interface{}{
|
|
||||||
"protocol": "grpc",
|
|
||||||
},
|
|
||||||
},
|
|
||||||
{
|
{
|
||||||
Upstream: cache,
|
Upstream: cache,
|
||||||
Config: map[string]interface{}{
|
Config: map[string]interface{}{
|
||||||
|
@ -1510,12 +1500,6 @@ func TestConfigEntry_ResolveServiceConfig_Upstreams(t *testing.T) {
|
||||||
"protocol": "grpc",
|
"protocol": "grpc",
|
||||||
},
|
},
|
||||||
UpstreamConfigs: structs.OpaqueUpstreamConfigs{
|
UpstreamConfigs: structs.OpaqueUpstreamConfigs{
|
||||||
{
|
|
||||||
Upstream: wildcard,
|
|
||||||
Config: map[string]interface{}{
|
|
||||||
"protocol": "grpc",
|
|
||||||
},
|
|
||||||
},
|
|
||||||
{
|
{
|
||||||
Upstream: cache,
|
Upstream: cache,
|
||||||
Config: map[string]interface{}{
|
Config: map[string]interface{}{
|
||||||
|
@ -2267,17 +2251,6 @@ func TestConfigEntry_ResolveServiceConfig_UpstreamProxyDefaultsProtocol(t *testi
|
||||||
require.NoError(t, msgpackrpc.CallWithCodec(codec, "ConfigEntry.ResolveServiceConfig", &args, &out))
|
require.NoError(t, msgpackrpc.CallWithCodec(codec, "ConfigEntry.ResolveServiceConfig", &args, &out))
|
||||||
|
|
||||||
expected := structs.OpaqueUpstreamConfigs{
|
expected := structs.OpaqueUpstreamConfigs{
|
||||||
{
|
|
||||||
Upstream: structs.PeeredServiceName{
|
|
||||||
ServiceName: structs.NewServiceName(
|
|
||||||
structs.WildcardSpecifier,
|
|
||||||
acl.DefaultEnterpriseMeta().WithWildcardNamespace(),
|
|
||||||
),
|
|
||||||
},
|
|
||||||
Config: map[string]interface{}{
|
|
||||||
"protocol": "http",
|
|
||||||
},
|
|
||||||
},
|
|
||||||
{
|
{
|
||||||
Upstream: id("bar"),
|
Upstream: id("bar"),
|
||||||
Config: map[string]interface{}{
|
Config: map[string]interface{}{
|
||||||
|
@ -2346,16 +2319,6 @@ func TestConfigEntry_ResolveServiceConfig_ProxyDefaultsProtocol_UsedForAllUpstre
|
||||||
"protocol": "http",
|
"protocol": "http",
|
||||||
},
|
},
|
||||||
UpstreamConfigs: structs.OpaqueUpstreamConfigs{
|
UpstreamConfigs: structs.OpaqueUpstreamConfigs{
|
||||||
{
|
|
||||||
Upstream: structs.PeeredServiceName{
|
|
||||||
ServiceName: structs.NewServiceName(
|
|
||||||
structs.WildcardSpecifier,
|
|
||||||
acl.DefaultEnterpriseMeta().WithWildcardNamespace()),
|
|
||||||
},
|
|
||||||
Config: map[string]interface{}{
|
|
||||||
"protocol": "http",
|
|
||||||
},
|
|
||||||
},
|
|
||||||
{
|
{
|
||||||
Upstream: psn,
|
Upstream: psn,
|
||||||
Config: map[string]interface{}{
|
Config: map[string]interface{}{
|
||||||
|
|
Loading…
Reference in New Issue