From 9ed3e645101c25783d005c8e30eae583ddca05db Mon Sep 17 00:00:00 2001
From: Sarah Adams <sadams.codes@gmail.com>
Date: Mon, 5 Aug 2019 12:52:35 -0700
Subject: [PATCH] fallback to proxy config global protocol when upstream
 services' protocol is unset (#6277)

fallback to proxy config global protocol when upstream services' protocol is unset

Fixes #5857
---
 agent/consul/config_endpoint.go      | 21 ++++++++-
 agent/consul/config_endpoint_test.go | 67 ++++++++++++++++++++++++++++
 2 files changed, 86 insertions(+), 2 deletions(-)

diff --git a/agent/consul/config_endpoint.go b/agent/consul/config_endpoint.go
index 9d1054745..ea92eded2 100644
--- a/agent/consul/config_endpoint.go
+++ b/agent/consul/config_endpoint.go
@@ -274,6 +274,12 @@ func (c *ConfigEntry) ResolveServiceConfig(args *structs.ServiceConfigRequest, r
 				reply.ProxyConfig["protocol"] = serviceConf.Protocol
 			}
 
+			// Extract the global protocol from proxyConf for upstream configs.
+			var proxyConfGlobalProtocol interface{}
+			if proxyConf != nil && proxyConf.Config != nil {
+				proxyConfGlobalProtocol = proxyConf.Config["protocol"]
+			}
+
 			// Apply the upstream protocols to the upstream configs
 			for _, upstream := range args.Upstreams {
 				_, upstreamEntry, err := state.ConfigEntry(ws, structs.ServiceDefaults, upstream)
@@ -289,8 +295,19 @@ func (c *ConfigEntry) ResolveServiceConfig(args *structs.ServiceConfigRequest, r
 					}
 				}
 
+				// No upstream found; skip.
+				if upstreamConf == nil {
+					continue
+				}
+
+				// Fallback to proxyConf global protocol.
+				protocol := proxyConfGlobalProtocol
+				if upstreamConf.Protocol != "" {
+					protocol = upstreamConf.Protocol
+				}
+
 				// Nothing to configure if a protocol hasn't been set.
-				if upstreamConf == nil || upstreamConf.Protocol == "" {
+				if protocol == nil {
 					continue
 				}
 
@@ -298,7 +315,7 @@ func (c *ConfigEntry) ResolveServiceConfig(args *structs.ServiceConfigRequest, r
 					reply.UpstreamConfigs = make(map[string]map[string]interface{})
 				}
 				reply.UpstreamConfigs[upstream] = map[string]interface{}{
-					"protocol": upstreamConf.Protocol,
+					"protocol": protocol,
 				}
 			}
 
diff --git a/agent/consul/config_endpoint_test.go b/agent/consul/config_endpoint_test.go
index 23e0d01f9..3a5ca6012 100644
--- a/agent/consul/config_endpoint_test.go
+++ b/agent/consul/config_endpoint_test.go
@@ -704,6 +704,73 @@ func TestConfigEntry_ResolveServiceConfig(t *testing.T) {
 	require.Equal(map[string]interface{}{"foo": 1}, proxyConf.Config)
 }
 
+func TestConfigEntry_ResolveServiceConfig_UpstreamProxyDefaultsProtocol(t *testing.T) {
+	t.Parallel()
+
+	require := require.New(t)
+
+	dir1, s1 := testServer(t)
+	defer os.RemoveAll(dir1)
+	defer s1.Shutdown()
+	codec := rpcClient(t, s1)
+	defer codec.Close()
+
+	// Create a dummy proxy/service config in the state store to look up.
+	state := s1.fsm.State()
+	require.NoError(state.EnsureConfigEntry(1, &structs.ProxyConfigEntry{
+		Kind: structs.ProxyDefaults,
+		Name: structs.ProxyConfigGlobal,
+		Config: map[string]interface{}{
+			"protocol": "http",
+		},
+	}))
+	require.NoError(state.EnsureConfigEntry(2, &structs.ServiceConfigEntry{
+		Kind: structs.ServiceDefaults,
+		Name: "foo",
+	}))
+	require.NoError(state.EnsureConfigEntry(2, &structs.ServiceConfigEntry{
+		Kind: structs.ServiceDefaults,
+		Name: "bar",
+	}))
+	require.NoError(state.EnsureConfigEntry(2, &structs.ServiceConfigEntry{
+		Kind: structs.ServiceDefaults,
+		Name: "other",
+	}))
+	require.NoError(state.EnsureConfigEntry(2, &structs.ServiceConfigEntry{
+		Kind:     structs.ServiceDefaults,
+		Name:     "alreadyprotocol",
+		Protocol: "grpc",
+	}))
+
+	args := structs.ServiceConfigRequest{
+		Name:       "foo",
+		Datacenter: s1.config.Datacenter,
+		Upstreams:  []string{"bar", "other", "alreadyprotocol", "dne"},
+	}
+	var out structs.ServiceConfigResponse
+	require.NoError(msgpackrpc.CallWithCodec(codec, "ConfigEntry.ResolveServiceConfig", &args, &out))
+
+	expected := structs.ServiceConfigResponse{
+		ProxyConfig: map[string]interface{}{
+			"protocol": "http",
+		},
+		UpstreamConfigs: map[string]map[string]interface{}{
+			"bar": map[string]interface{}{
+				"protocol": "http",
+			},
+			"other": map[string]interface{}{
+				"protocol": "http",
+			},
+			"alreadyprotocol": map[string]interface{}{
+				"protocol": "grpc",
+			},
+		},
+		// Don't know what this is deterministically
+		QueryMeta: out.QueryMeta,
+	}
+	require.Equal(expected, out)
+}
+
 func TestConfigEntry_ResolveServiceConfigNoConfig(t *testing.T) {
 	t.Parallel()