Fix incorrect protocol for transparent proxy upstreams. (#17894)

This PR fixes a bug that was introduced in:
https://github.com/hashicorp/consul/pull/16021

A user setting a protocol in proxy-defaults would cause tproxy implicit
upstreams to not honor the upstream service's protocol set in its
`ServiceDefaults.Protocol` field, and would instead always use the
proxy-defaults value.

Due to the fact that upstreams configured with "tcp" can successfully contact
upstream "http" services, this issue was not recognized until recently (a
proxy-defaults with "tcp" and a listening service with "http" would make
successful requests, but not the opposite).

As a temporary work-around, users experiencing this issue can explicitly set
the protocol on the `ServiceDefaults.UpstreamConfig.Overrides`, which should
take precedence.

The fix in this PR removes the proxy-defaults protocol from the wildcard
upstream that tproxy uses to configure implicit upstreams. When the protocol
was included, it would always overwrite the value during discovery chain
compilation, which was not correct. The discovery chain compiler also consumes
proxy defaults to determine the protocol, so simply excluding it from the
wildcard upstream config map resolves the issue.

.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

@@ -36,6 +36,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
@@ -63,9 +64,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(
@@ -210,6 +232,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

@@ -1444,16 +1444,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{}{
@@ -1510,12 +1500,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{}{
@@ -2267,17 +2251,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{}{
@@ -2346,16 +2319,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{}{