From 896749d5857e6d91bafcc5876d94de984e99e767 Mon Sep 17 00:00:00 2001 From: Sarah Adams Date: Thu, 1 Aug 2019 09:53:34 -0700 Subject: [PATCH] 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 | 41 +++++++++++-- command/connect/envoy/envoy_test.go | 61 ++++++++++++++++++- .../envoy/testdata/grpc-addr-config.golden | 60 ++++++++++++++++++ 3 files changed, 156 insertions(+), 6 deletions(-) create mode 100644 command/connect/envoy/testdata/grpc-addr-config.golden diff --git a/command/connect/envoy/envoy.go b/command/connect/envoy/envoy.go index 9e3e06fbcf..f2a5a83e3c 100644 --- a/command/connect/envoy/envoy.go +++ b/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 } diff --git a/command/connect/envoy/envoy_test.go b/command/connect/envoy/envoy_test.go index 2f15ac59a8..f012126db8 100644 --- a/command/connect/envoy/envoy_test.go +++ b/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) + }) +} diff --git a/command/connect/envoy/testdata/grpc-addr-config.golden b/command/connect/envoy/testdata/grpc-addr-config.golden new file mode 100644 index 0000000000..e9c823b905 --- /dev/null +++ b/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" + } + } + } + } +}