Manual backport of #17894. (#18008)

.changelog/17894.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+connect: Fix incorrect protocol config merging for transparent proxy implicit upstreams.
+```

agent/configentry/resolve.go

@@ -33,6 +33,7 @@ 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
@@ -59,9 +60,30 @@ func ComputeResolvedServiceConfig(
 		if !proxyConf.MeshGateway.IsZero() {
 			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(
@@ -227,6 +249,10 @@ 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)
+		if proxyConfGlobalProtocol != "" {
+			resolvedCfg["protocol"] = proxyConfGlobalProtocol
+		}
+
 		if err := mergo.MergeWithOverwrite(&resolvedCfg, wildcardUpstreamDefaults); err != nil {
 			return nil, fmt.Errorf("failed to merge wildcard defaults into upstream: %v", err)
 		}

agent/consul/config_endpoint_test.go

@@ -1316,16 +1316,6 @@ func TestConfigEntry_ResolveServiceConfig_Upstreams(t *testing.T) {
 					"protocol": "grpc",
 				},
 				UpstreamConfigs: structs.OpaqueUpstreamConfigs{
-					{
-						Upstream: structs.PeeredServiceName{
-							ServiceName: structs.NewServiceName(
-								structs.WildcardSpecifier,
-								acl.DefaultEnterpriseMeta().WithWildcardNamespace()),
-						},
-						Config: map[string]interface{}{
-							"protocol": "grpc",
-						},
-					},
 					{
 						Upstream: cache,
 						Config: map[string]interface{}{
@@ -1377,15 +1367,6 @@ func TestConfigEntry_ResolveServiceConfig_Upstreams(t *testing.T) {
 					"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{}{
@@ -1442,12 +1423,6 @@ func TestConfigEntry_ResolveServiceConfig_Upstreams(t *testing.T) {
 					"protocol": "grpc",
 				},
 				UpstreamConfigs: structs.OpaqueUpstreamConfigs{
-					{
-						Upstream: wildcard,
-						Config: map[string]interface{}{
-							"protocol": "grpc",
-						},
-					},
 					{
 						Upstream: cache,
 						Config: map[string]interface{}{
@@ -2202,17 +2177,6 @@ func TestConfigEntry_ResolveServiceConfig_UpstreamProxyDefaultsProtocol(t *testi
 	require.NoError(t, msgpackrpc.CallWithCodec(codec, "ConfigEntry.ResolveServiceConfig", &args, &out))
 
 	expected := structs.OpaqueUpstreamConfigs{
-		{
-			Upstream: structs.PeeredServiceName{
-				ServiceName: structs.NewServiceName(
-					structs.WildcardSpecifier,
-					acl.DefaultEnterpriseMeta().WithWildcardNamespace(),
-				),
-			},
-			Config: map[string]interface{}{
-				"protocol": "http",
-			},
-		},
 		{
 			Upstream: id("bar"),
 			Config: map[string]interface{}{
@@ -2281,16 +2245,6 @@ func TestConfigEntry_ResolveServiceConfig_ProxyDefaultsProtocol_UsedForAllUpstre
 			"protocol": "http",
 		},
 		UpstreamConfigs: structs.OpaqueUpstreamConfigs{
-			{
-				Upstream: structs.PeeredServiceName{
-					ServiceName: structs.NewServiceName(
-						structs.WildcardSpecifier,
-						acl.DefaultEnterpriseMeta().WithWildcardNamespace()),
-				},
-				Config: map[string]interface{}{
-					"protocol": "http",
-				},
-			},
 			{
 				Upstream: psn,
 				Config: map[string]interface{}{