From 4ad80ccee331030c32b669fda9b14d1eacb1628f Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 9 Jul 2021 15:17:29 -0400 Subject: [PATCH] command/envoy: stop using the DebugConfig from Self endpoint The DebugConfig in the self endpoint can change at any time. It's not a stable API. With the previous change to rename GRPCPort to XDSPort this command would have broken. This commit adds the XDSPort to a stable part of the XDS api, and changes the envoy command to read this new field. It includes support for the old API as well, in case a newer CLI is used with an older API, and adds a test for both cases. --- agent/agent_endpoint.go | 10 +- command/connect/envoy/envoy.go | 25 ++- command/connect/envoy/envoy_test.go | 94 +++++---- ...den => deprecated-grpc-addr-config.golden} | 0 .../envoy/testdata/xds-addr-config.golden | 180 ++++++++++++++++++ 5 files changed, 265 insertions(+), 44 deletions(-) rename command/connect/envoy/testdata/{grpc-addr-config.golden => deprecated-grpc-addr-config.golden} (100%) create mode 100644 command/connect/envoy/testdata/xds-addr-config.golden diff --git a/agent/agent_endpoint.go b/agent/agent_endpoint.go index 8543c5d663..7b06875d7d 100644 --- a/agent/agent_endpoint.go +++ b/agent/agent_endpoint.go @@ -38,11 +38,12 @@ type Self struct { Member serf.Member Stats map[string]map[string]string Meta map[string]string - XDS *xdsSelf `json:"xDS,omitempty"` + XDS *XDSSelf `json:"xDS,omitempty"` } -type xdsSelf struct { +type XDSSelf struct { SupportedProxies map[string][]string + Port int } func (s *HTTPHandlers) AgentSelf(resp http.ResponseWriter, req *http.Request) (interface{}, error) { @@ -65,12 +66,13 @@ func (s *HTTPHandlers) AgentSelf(resp http.ResponseWriter, req *http.Request) (i } } - var xds *xdsSelf + var xds *XDSSelf if s.agent.grpcServer != nil { - xds = &xdsSelf{ + xds = &XDSSelf{ SupportedProxies: map[string][]string{ "envoy": proxysupport.EnvoyVersions, }, + Port: s.agent.config.XDSPort, } } diff --git a/command/connect/envoy/envoy.go b/command/connect/envoy/envoy.go index 1547085237..d1c23f6dde 100644 --- a/command/connect/envoy/envoy.go +++ b/command/connect/envoy/envoy.go @@ -426,7 +426,7 @@ func (c *cmd) templateArgs() (*BootstrapTplArgs, error) { return nil, err } - grpcAddr, err := c.grpcAddress(httpCfg) + xdsAddr, err := c.xdsAddress(httpCfg) if err != nil { return nil, err } @@ -471,7 +471,7 @@ func (c *cmd) templateArgs() (*BootstrapTplArgs, error) { caPEM = strings.Replace(strings.Join(pems, ""), "\n", "\\n", -1) return &BootstrapTplArgs{ - GRPC: grpcAddr, + GRPC: xdsAddr, ProxyCluster: cluster, ProxyID: c.proxyID, ProxySourceService: proxySourceService, @@ -554,13 +554,12 @@ func (c *cmd) generateConfig() ([]byte, error) { } // TODO: make method a function -func (c *cmd) grpcAddress(httpCfg *api.Config) (GRPC, error) { +func (c *cmd) xdsAddress(httpCfg *api.Config) (GRPC, error) { g := GRPC{} addr := c.grpcAddr - // See if we need to lookup grpcAddr if addr == "" { - port, err := c.lookupGRPCPort() + port, err := c.lookupXDSPort() if err != nil { c.UI.Error(fmt.Sprintf("Error connecting to Consul agent: %s", err)) } @@ -618,11 +617,25 @@ func (c *cmd) grpcAddress(httpCfg *api.Config) (GRPC, error) { return g, nil } -func (c *cmd) lookupGRPCPort() (int, error) { +func (c *cmd) lookupXDSPort() (int, error) { self, err := c.client.Agent().Self() if err != nil { return 0, err } + + type response struct { + XDS struct { + Port int + } + } + + var resp response + if err := mapstructure.Decode(self, &resp); err == nil && resp.XDS.Port != 0 { + return resp.XDS.Port, nil + } + + // Fallback to old API for the case where a new consul CLI is being used with + // an older API version. cfg, ok := self["DebugConfig"] if !ok { return 0, fmt.Errorf("unexpected agent response: no debug config") diff --git a/command/connect/envoy/envoy_test.go b/command/connect/envoy/envoy_test.go index 67698a7c65..fdf574a9da 100644 --- a/command/connect/envoy/envoy_test.go +++ b/command/connect/envoy/envoy_test.go @@ -111,7 +111,8 @@ type generateConfigTestCase struct { Files map[string]string ProxyConfig map[string]interface{} NamespacesEnabled bool - GRPCPort int // only used for testing custom-configured grpc port + XDSPort int // only used for testing custom-configured grpc port + AgentSelf110 bool // fake the agent API from versions v1.10 and earlier WantArgs BootstrapTplArgs WantErr string } @@ -356,9 +357,35 @@ func TestGenerateConfig(t *testing.T) { }, }, { - Name: "grpc-addr-config", - Flags: []string{"-proxy-id", "test-proxy"}, - GRPCPort: 9999, + Name: "xds-addr-config", + Flags: []string{"-proxy-id", "test-proxy"}, + XDSPort: 9999, + WantArgs: BootstrapTplArgs{ + EnvoyVersion: defaultEnvoyVersion, + ProxyCluster: "test-proxy", + ProxyID: "test-proxy", + // We don't know this til after the lookup so it will be empty in the + // initial args call we are testing here. + ProxySourceService: "", + // 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. + GRPC: GRPC{ + AgentAddress: "127.0.0.1", + AgentPort: "9999", + }, + AdminAccessLogPath: "/dev/null", + AdminBindAddress: "127.0.0.1", + AdminBindPort: "19000", + LocalAgentClusterName: xds.LocalAgentClusterName, + PrometheusScrapePath: "/metrics", + }, + }, + { + Name: "deprecated-grpc-addr-config", + Flags: []string{"-proxy-id", "test-proxy"}, + XDSPort: 9999, + AgentSelf110: true, WantArgs: BootstrapTplArgs{ EnvoyVersion: defaultEnvoyVersion, ProxyCluster: "test-proxy", @@ -860,7 +887,7 @@ func TestGenerateConfig(t *testing.T) { // Run a mock agent API that just always returns the proxy config in the // test. - srv := httptest.NewServer(testMockAgent(tc.ProxyConfig, tc.GRPCPort, tc.NamespacesEnabled)) + srv := httptest.NewServer(testMockAgent(tc)) defer srv.Close() client, err := api.NewClient(&api.Config{Address: srv.URL}) require.NoError(err) @@ -1005,29 +1032,23 @@ func TestEnvoy_GatewayRegistration(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, namespacesEnabled bool) http.HandlerFunc { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if strings.Contains(r.URL.Path, "/agent/services") { - testMockAgentGatewayConfig(namespacesEnabled)(w, r) - return - } - - if strings.Contains(r.URL.Path, "/agent/service") { - testMockAgentProxyConfig(agentCfg, namespacesEnabled)(w, r) - return - } - - if strings.Contains(r.URL.Path, "/agent/self") { - testMockAgentSelf(grpcPort)(w, r) - return +func testMockAgent(tc generateConfigTestCase) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + switch { + case strings.Contains(r.URL.Path, "/agent/services"): + testMockAgentGatewayConfig(tc.NamespacesEnabled)(w, r) + case strings.Contains(r.URL.Path, "/agent/service"): + testMockAgentProxyConfig(tc.ProxyConfig, tc.NamespacesEnabled)(w, r) + case strings.Contains(r.URL.Path, "/agent/self"): + testMockAgentSelf(tc.XDSPort, tc.AgentSelf110)(w, r) + default: + http.NotFound(w, r) } - - http.NotFound(w, r) - }) + } } func testMockAgentGatewayConfig(namespacesEnabled bool) http.HandlerFunc { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + return func(w http.ResponseWriter, r *http.Request) { // Parse the proxy-id from the end of the URL (blindly assuming it's correct // format) params := r.URL.Query() @@ -1061,7 +1082,7 @@ func testMockAgentGatewayConfig(namespacesEnabled bool) http.HandlerFunc { return } w.Write(cfgJSON) - }) + } } func namespaceFromQuery(r *http.Request) string { @@ -1074,7 +1095,7 @@ func namespaceFromQuery(r *http.Request) string { } func testMockAgentProxyConfig(cfg map[string]interface{}, namespacesEnabled bool) http.HandlerFunc { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + return func(w http.ResponseWriter, r *http.Request) { // Parse the proxy-id from the end of the URL (blindly assuming it's correct // format) proxyID := strings.TrimPrefix(r.URL.Path, "/v1/agent/service/") @@ -1103,7 +1124,7 @@ func testMockAgentProxyConfig(cfg map[string]interface{}, namespacesEnabled bool return } w.Write(cfgJSON) - }) + } } func TestEnvoyCommand_canBindInternal(t *testing.T) { @@ -1203,16 +1224,21 @@ func TestEnvoyCommand_canBindInternal(t *testing.T) { } // 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) { +// port is filled in to match the given wantXDSPort argument. +func testMockAgentSelf(wantXDSPort int, agentSelf110 bool) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { resp := agent.Self{ Config: map[string]interface{}{ "Datacenter": "dc1", }, - DebugConfig: map[string]interface{}{ - "GRPCPort": wantGRPCPort, - }, + } + + if agentSelf110 { + resp.DebugConfig = map[string]interface{}{ + "GRPCPort": wantXDSPort, + } + } else { + resp.XDS = &agent.XDSSelf{Port: wantXDSPort} } selfJSON, err := json.Marshal(resp) @@ -1222,5 +1248,5 @@ func testMockAgentSelf(wantGRPCPort int) http.HandlerFunc { return } w.Write(selfJSON) - }) + } } diff --git a/command/connect/envoy/testdata/grpc-addr-config.golden b/command/connect/envoy/testdata/deprecated-grpc-addr-config.golden similarity index 100% rename from command/connect/envoy/testdata/grpc-addr-config.golden rename to command/connect/envoy/testdata/deprecated-grpc-addr-config.golden diff --git a/command/connect/envoy/testdata/xds-addr-config.golden b/command/connect/envoy/testdata/xds-addr-config.golden new file mode 100644 index 0000000000..34ed73682b --- /dev/null +++ b/command/connect/envoy/testdata/xds-addr-config.golden @@ -0,0 +1,180 @@ +{ + "admin": { + "access_log_path": "/dev/null", + "address": { + "socket_address": { + "address": "127.0.0.1", + "port_value": 19000 + } + } + }, + "node": { + "cluster": "test-proxy", + "id": "test-proxy", + "metadata": { + "namespace": "default", + "envoy_version": "1.18.3" + } + }, + "static_resources": { + "clusters": [ + { + "name": "local_agent", + "ignore_health_on_host_removal": false, + "connect_timeout": "1s", + "type": "STATIC", + "http2_protocol_options": {}, + "loadAssignment": { + "clusterName": "local_agent", + "endpoints": [ + { + "lbEndpoints": [ + { + "endpoint": { + "address": { + "socket_address": { + "address": "127.0.0.1", + "port_value": 9999 + } + } + } + } + ] + } + ] + } + } + ] + }, + "stats_config": { + "stats_tags": [ + { + "regex": "^cluster\\.(?:passthrough~)?((?:([^.]+)~)?(?:[^.]+\\.)?[^.]+\\.[^.]+\\.[^.]+\\.[^.]+\\.[^.]+\\.consul\\.)", + "tag_name": "consul.destination.custom_hash" + }, + { + "regex": "^cluster\\.(?:passthrough~)?((?:[^.]+~)?(?:([^.]+)\\.)?[^.]+\\.[^.]+\\.[^.]+\\.[^.]+\\.[^.]+\\.consul\\.)", + "tag_name": "consul.destination.service_subset" + }, + { + "regex": "^cluster\\.(?:passthrough~)?((?:[^.]+~)?(?:[^.]+\\.)?([^.]+)\\.[^.]+\\.[^.]+\\.[^.]+\\.[^.]+\\.consul\\.)", + "tag_name": "consul.destination.service" + }, + { + "regex": "^cluster\\.(?:passthrough~)?((?:[^.]+~)?(?:[^.]+\\.)?[^.]+\\.([^.]+)\\.[^.]+\\.[^.]+\\.[^.]+\\.consul\\.)", + "tag_name": "consul.destination.namespace" + }, + { + "regex": "^cluster\\.(?:passthrough~)?((?:[^.]+~)?(?:[^.]+\\.)?[^.]+\\.[^.]+\\.([^.]+)\\.[^.]+\\.[^.]+\\.consul\\.)", + "tag_name": "consul.destination.datacenter" + }, + { + "regex": "^cluster\\.(?:passthrough~)?((?:[^.]+~)?(?:[^.]+\\.)?[^.]+\\.[^.]+\\.[^.]+\\.([^.]+)\\.[^.]+\\.consul\\.)", + "tag_name": "consul.destination.routing_type" + }, + { + "regex": "^cluster\\.(?:passthrough~)?((?:[^.]+~)?(?:[^.]+\\.)?[^.]+\\.[^.]+\\.[^.]+\\.[^.]+\\.([^.]+)\\.consul\\.)", + "tag_name": "consul.destination.trust_domain" + }, + { + "regex": "^cluster\\.(?:passthrough~)?(((?:[^.]+~)?(?:[^.]+\\.)?[^.]+\\.[^.]+\\.[^.]+)\\.[^.]+\\.[^.]+\\.consul\\.)", + "tag_name": "consul.destination.target" + }, + { + "regex": "^cluster\\.(?:passthrough~)?(((?:[^.]+~)?(?:[^.]+\\.)?[^.]+\\.[^.]+\\.[^.]+\\.[^.]+\\.[^.]+)\\.consul\\.)", + "tag_name": "consul.destination.full_target" + }, + { + "regex": "^(?:tcp|http)\\.upstream\\.(([^.]+)(?:\\.[^.]+)?\\.[^.]+\\.)", + "tag_name": "consul.upstream.service" + }, + { + "regex": "^(?:tcp|http)\\.upstream\\.([^.]+(?:\\.[^.]+)?\\.([^.]+)\\.)", + "tag_name": "consul.upstream.datacenter" + }, + { + "regex": "^(?:tcp|http)\\.upstream\\.([^.]+(?:\\.([^.]+))?\\.[^.]+\\.)", + "tag_name": "consul.upstream.namespace" + }, + { + "regex": "^cluster\\.((?:([^.]+)~)?(?:[^.]+\\.)?[^.]+\\.[^.]+\\.[^.]+\\.[^.]+\\.[^.]+\\.consul\\.)", + "tag_name": "consul.custom_hash" + }, + { + "regex": "^cluster\\.((?:[^.]+~)?(?:([^.]+)\\.)?[^.]+\\.[^.]+\\.[^.]+\\.[^.]+\\.[^.]+\\.consul\\.)", + "tag_name": "consul.service_subset" + }, + { + "regex": "^cluster\\.((?:[^.]+~)?(?:[^.]+\\.)?([^.]+)\\.[^.]+\\.[^.]+\\.[^.]+\\.[^.]+\\.consul\\.)", + "tag_name": "consul.service" + }, + { + "regex": "^cluster\\.((?:[^.]+~)?(?:[^.]+\\.)?[^.]+\\.([^.]+)\\.[^.]+\\.[^.]+\\.[^.]+\\.consul\\.)", + "tag_name": "consul.namespace" + }, + { + "regex": "^cluster\\.((?:[^.]+~)?(?:[^.]+\\.)?[^.]+\\.[^.]+\\.([^.]+)\\.[^.]+\\.[^.]+\\.consul\\.)", + "tag_name": "consul.datacenter" + }, + { + "regex": "^cluster\\.((?:[^.]+~)?(?:[^.]+\\.)?[^.]+\\.[^.]+\\.[^.]+\\.([^.]+)\\.[^.]+\\.consul\\.)", + "tag_name": "consul.routing_type" + }, + { + "regex": "^cluster\\.((?:[^.]+~)?(?:[^.]+\\.)?[^.]+\\.[^.]+\\.[^.]+\\.[^.]+\\.([^.]+)\\.consul\\.)", + "tag_name": "consul.trust_domain" + }, + { + "regex": "^cluster\\.(((?:[^.]+~)?(?:[^.]+\\.)?[^.]+\\.[^.]+\\.[^.]+)\\.[^.]+\\.[^.]+\\.consul\\.)", + "tag_name": "consul.target" + }, + { + "regex": "^cluster\\.(((?:[^.]+~)?(?:[^.]+\\.)?[^.]+\\.[^.]+\\.[^.]+\\.[^.]+\\.[^.]+)\\.consul\\.)", + "tag_name": "consul.full_target" + }, + { + "tag_name": "local_cluster", + "fixed_value": "test-proxy" + }, + { + "tag_name": "consul.source.service", + "fixed_value": "test-proxy" + }, + { + "tag_name": "consul.source.namespace", + "fixed_value": "default" + }, + { + "tag_name": "consul.source.datacenter", + "fixed_value": "dc1" + } + ], + "use_all_default_tags": true + }, + "dynamic_resources": { + "lds_config": { + "ads": {}, + "resource_api_version": "V3" + }, + "cds_config": { + "ads": {}, + "resource_api_version": "V3" + }, + "ads_config": { + "api_type": "DELTA_GRPC", + "transport_api_version": "V3", + "grpc_services": { + "initial_metadata": [ + { + "key": "x-consul-token", + "value": "" + } + ], + "envoy_grpc": { + "cluster_name": "local_agent" + } + } + } + } +} +