fix 'consul connect envoy' to try to use previously-configured grpc port (#6245)

fix 'consul connect envoy' to try to use previously-configured grpc port on running agent before defaulting to 8502

Fixes #5011

command/connect/envoy/envoy.go

@@ -115,11 +115,6 @@ func (c *cmd) Run(args []string) int {
 	if c.grpcAddr == "" {
 		c.grpcAddr = os.Getenv(api.GRPCAddrEnvName)
 	}
-	if c.grpcAddr == "" {
-		// This is the dev mode default and recommended production setting if
-		// enabled.
-		c.grpcAddr = "localhost:8502"
-	}
 	if c.http.Token() == "" && c.http.TokenFile() == "" {
 		// Extra check needed since CONSUL_HTTP_TOKEN has not been consulted yet but
 		// calling SetToken with empty will force that to override the
@@ -151,6 +146,21 @@ func (c *cmd) Run(args []string) int {
 		return 1
 	}
 
+	// See if we need to lookup grpcAddr
+	if c.grpcAddr == "" {
+		port, err := c.lookupGRPCPort()
+		if err != nil {
+			c.UI.Error(fmt.Sprintf("Error connecting to Consul agent: %s", err))
+		}
+		if port <= 0 {
+			// This is the dev mode default and recommended production setting if
+			// enabled.
+			port = 8502
+			c.UI.Info(fmt.Sprintf("Defaulting to grpc port = %d", port))
+		}
+		c.grpcAddr = fmt.Sprintf("localhost:%v", port)
+	}
+
 	// Generate config
 	bootstrapJson, err := c.generateConfig()
 	if err != nil {
@@ -321,6 +331,27 @@ func (c *cmd) lookupProxyIDForSidecar() (string, error) {
 	return proxyCmd.LookupProxyIDForSidecar(c.client, c.sidecarFor)
 }
 
+func (c *cmd) lookupGRPCPort() (int, error) {
+	self, err := c.client.Agent().Self()
+	if err != nil {
+		return 0, err
+	}
+	cfg, ok := self["DebugConfig"]
+	if !ok {
+		return 0, fmt.Errorf("unexpected agent response: no debug config")
+	}
+	port, ok := cfg["GRPCPort"]
+	if !ok {
+		return 0, fmt.Errorf("agent does not have grpc port enabled")
+	}
+	portN, ok := port.(float64)
+	if !ok {
+		return 0, fmt.Errorf("invalid grpc port in agent response")
+	}
+
+	return int(portN), nil
+}
+
 func (c *cmd) Synopsis() string {
 	return synopsis
 }

command/connect/envoy/envoy_test.go

@@ -11,6 +11,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/hashicorp/consul/agent"
 	"github.com/hashicorp/consul/agent/xds"
 	"github.com/hashicorp/consul/api"
 	"github.com/hashicorp/consul/sdk/testutil"
@@ -66,6 +67,7 @@ func TestGenerateConfig(t *testing.T) {
 		Env         []string
 		Files       map[string]string
 		ProxyConfig map[string]interface{}
+		GRPCPort    int // only used for testing custom-configured grpc port
 		WantArgs    BootstrapTplArgs
 		WantErr     string
 	}{
@@ -206,6 +208,24 @@ func TestGenerateConfig(t *testing.T) {
 				LocalAgentClusterName: xds.LocalAgentClusterName,
 			},
 		},
+		{
+			Name:     "grpc-addr-config",
+			Flags:    []string{"-proxy-id", "test-proxy"},
+			GRPCPort: 9999,
+			WantArgs: BootstrapTplArgs{
+				ProxyCluster: "test-proxy",
+				ProxyID:      "test-proxy",
+				// Should resolve IP, note this might not resolve the same way
+				// everywhere which might make this test brittle but not sure what else
+				// to do.
+				AgentAddress:          "127.0.0.1",
+				AgentPort:             "9999",
+				AdminAccessLogPath:    "/dev/null",
+				AdminBindAddress:      "127.0.0.1",
+				AdminBindPort:         "19000",
+				LocalAgentClusterName: xds.LocalAgentClusterName,
+			},
+		},
 		{
 			Name:  "access-log-path",
 			Flags: []string{"-proxy-id", "test-proxy", "-admin-access-log-path", "/some/path/access.log"},
@@ -437,7 +457,7 @@ func TestGenerateConfig(t *testing.T) {
 
 			// Run a mock agent API that just always returns the proxy config in the
 			// test.
-			srv := httptest.NewServer(testMockAgentProxyConfig(tc.ProxyConfig))
+			srv := httptest.NewServer(testMockAgent(tc.ProxyConfig, tc.GRPCPort))
 			defer srv.Close()
 
 			// Set the agent HTTP address in ENV to be our mock
@@ -485,6 +505,25 @@ func TestGenerateConfig(t *testing.T) {
 	}
 }
 
+// testMockAgent combines testMockAgentProxyConfig and testMockAgentSelf,
+// routing /agent/service/... requests to testMockAgentProxyConfig and
+// routing /agent/self requests to testMockAgentSelf.
+func testMockAgent(agentCfg map[string]interface{}, grpcPort int) http.HandlerFunc {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if strings.Contains(r.URL.Path, "/agent/service") {
+			testMockAgentProxyConfig(agentCfg)(w, r)
+			return
+		}
+
+		if strings.Contains(r.URL.Path, "/agent/self") {
+			testMockAgentSelf(grpcPort)(w, r)
+			return
+		}
+
+		http.NotFound(w, r)
+	})
+}
+
 func testMockAgentProxyConfig(cfg map[string]interface{}) http.HandlerFunc {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		// Parse the proxy-id from the end of the URL (blindly assuming it's correct
@@ -512,3 +551,23 @@ func testMockAgentProxyConfig(cfg map[string]interface{}) http.HandlerFunc {
 		w.Write(cfgJSON)
 	})
 }
+
+// testMockAgentSelf returns an empty /v1/agent/self response except GRPC
+// port is filled in to match the given wantGRPCPort argument.
+func testMockAgentSelf(wantGRPCPort int) http.HandlerFunc {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		resp := agent.Self{
+			DebugConfig: map[string]interface{}{
+				"GRPCPort": wantGRPCPort,
+			},
+		}
+
+		selfJSON, err := json.Marshal(resp)
+		if err != nil {
+			w.WriteHeader(500)
+			w.Write([]byte(err.Error()))
+			return
+		}
+		w.Write(selfJSON)
+	})
+}

command/connect/envoy/testdata/grpc-addr-config.golden

@@ -0,0 +1,60 @@
+{
+  "admin": {
+    "access_log_path": "/dev/null",
+    "address": {
+      "socket_address": {
+        "address": "127.0.0.1",
+        "port_value": 19000
+      }
+    }
+  },
+  "node": {
+    "cluster": "test-proxy",
+    "id": "test-proxy"
+  },
+  "static_resources": {
+    "clusters": [
+      {
+        "name": "local_agent",
+        "connect_timeout": "1s",
+        "type": "STATIC",
+        "http2_protocol_options": {},
+        "hosts": [
+          {
+            "socket_address": {
+              "address": "127.0.0.1",
+              "port_value": 9999
+            }
+          }
+        ]
+      }
+    ]
+  },
+  "stats_config": {
+			"stats_tags": [
+				{
+			"tag_name": "local_cluster",
+			"fixed_value": "test-proxy"
+		}
+			],
+			"use_all_default_tags": true
+		},
+  "dynamic_resources": {
+    "lds_config": { "ads": {} },
+    "cds_config": { "ads": {} },
+    "ads_config": {
+      "api_type": "GRPC",
+      "grpc_services": {
+        "initial_metadata": [
+          {
+            "key": "x-consul-token",
+            "value": ""
+          }
+        ],
+        "envoy_grpc": {
+          "cluster_name": "local_agent"
+        }
+      }
+    }
+  }
+}