From b14696e32ad54cca96784c9010910c1606272b56 Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Thu, 21 May 2020 07:08:12 -0700 Subject: [PATCH] Standardize support for Tagged and BindAddresses in Ingress Gateways (#7924) * Standardize support for Tagged and BindAddresses in Ingress Gateways This updates the TaggedAddresses and BindAddresses behavior for Ingress to match Mesh/Terminating gateways. The `consul connect envoy` command now also allows passing an address without a port for tagged/bind addresses. * Update command/connect/envoy/envoy.go Co-authored-by: Freddy * PR comments * Check to see if address is an actual IP address * Update agent/xds/listeners.go Co-authored-by: Freddy * fix whitespace Co-authored-by: Chris Piraino Co-authored-by: Freddy --- agent/proxycfg/state.go | 8 +- agent/proxycfg/state_test.go | 1 - agent/xds/listeners.go | 193 +++++++++--------- agent/xds/listeners_test.go | 28 +++ ...ream-typed-ignored-with-disco-chain.golden | 116 +++++++++++ .../ingress-gateway-bind-addrs.golden | 76 +++++++ .../testdata/listeners/ingress-gateway.golden | 4 +- .../ingress-with-chain-and-overrides.golden | 4 +- .../ingress-with-chain-external-sni.golden | 4 +- ...hain-failover-through-local-gateway.golden | 4 +- ...ain-failover-through-remote-gateway.golden | 4 +- .../ingress-with-tls-listener.golden | 4 +- command/connect/envoy/envoy.go | 3 +- command/connect/envoy/flags.go | 11 +- command/connect/envoy/flags_test.go | 14 +- 15 files changed, 351 insertions(+), 123 deletions(-) create mode 100644 agent/xds/testdata/listeners/custom-upstream-typed-ignored-with-disco-chain.golden create mode 100644 agent/xds/testdata/listeners/ingress-gateway-bind-addrs.golden diff --git a/agent/proxycfg/state.go b/agent/proxycfg/state.go index af6de2e312..740024e5d3 100644 --- a/agent/proxycfg/state.go +++ b/agent/proxycfg/state.go @@ -1350,7 +1350,7 @@ func (s *state) handleUpdateIngressGateway(u cache.UpdateEvent, snap *ConfigSnap watchedSvcs := make(map[string]struct{}) upstreamsMap := make(map[IngressListenerKey]structs.Upstreams) for _, service := range services.Services { - u := makeUpstream(service, s.address) + u := makeUpstream(service) err := s.watchIngressDiscoveryChain(snap, u) if err != nil { @@ -1386,7 +1386,7 @@ func (s *state) handleUpdateIngressGateway(u cache.UpdateEvent, snap *ConfigSnap return nil } -func makeUpstream(g *structs.GatewayService, bindAddr string) structs.Upstream { +func makeUpstream(g *structs.GatewayService) structs.Upstream { upstream := structs.Upstream{ DestinationName: g.Service.ID, DestinationNamespace: g.Service.NamespaceOrDefault(), @@ -1398,10 +1398,6 @@ func makeUpstream(g *structs.GatewayService, bindAddr string) structs.Upstream { "protocol": g.Protocol, }, } - upstream.LocalBindAddress = bindAddr - if bindAddr == "" { - upstream.LocalBindAddress = "0.0.0.0" - } return upstream } diff --git a/agent/proxycfg/state_test.go b/agent/proxycfg/state_test.go index 1431d9c541..1d741b6e5d 100644 --- a/agent/proxycfg/state_test.go +++ b/agent/proxycfg/state_test.go @@ -782,7 +782,6 @@ func TestState_WatchesAndUpdates(t *testing.T) { { DestinationNamespace: "default", DestinationName: "api", - LocalBindAddress: "10.0.1.1", LocalBindPort: 9999, Config: map[string]interface{}{ "protocol": "http", diff --git a/agent/xds/listeners.go b/agent/xds/listeners.go index fc237a4620..3eb150c8df 100644 --- a/agent/xds/listeners.go +++ b/agent/xds/listeners.go @@ -11,6 +11,7 @@ import ( "strings" "github.com/hashicorp/consul/logging" + "github.com/hashicorp/go-hclog" envoy "github.com/envoyproxy/go-control-plane/envoy/api/v2" envoyauth "github.com/envoyproxy/go-control-plane/envoy/api/v2/auth" @@ -45,7 +46,7 @@ func (s *Server) listenersFromSnapshot(cfgSnap *proxycfg.ConfigSnapshot, token s case structs.ServiceKindMeshGateway: return s.listenersFromSnapshotGateway(cfgSnap, token) case structs.ServiceKindIngressGateway: - return s.listenersFromSnapshotIngressGateway(cfgSnap) + return s.listenersFromSnapshotGateway(cfgSnap, token) default: return nil, fmt.Errorf("Invalid service kind: %v", cfgSnap.Kind) } @@ -71,11 +72,13 @@ func (s *Server) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg.ConfigSnaps } var upstreamListener proto.Message - if chain == nil || chain.IsDefault() { - upstreamListener, err = s.makeUpstreamListenerIgnoreDiscoveryChain(&u, chain, cfgSnap, nil) - } else { - upstreamListener, err = s.makeUpstreamListenerForDiscoveryChain(&u, chain, cfgSnap, nil) - } + upstreamListener, err = s.makeUpstreamListenerForDiscoveryChain( + &u, + u.LocalBindAddress, + chain, + cfgSnap, + nil, + ) if err != nil { return nil, err } @@ -254,6 +257,12 @@ func (s *Server) listenersFromSnapshotGateway(cfgSnap *proxycfg.ConfigSnapshot, if err != nil { return nil, err } + case structs.ServiceKindIngressGateway: + listeners, err := s.makeIngressGatewayListeners(a.Address, cfgSnap) + if err != nil { + return nil, err + } + resources = append(resources, listeners...) case structs.ServiceKindMeshGateway: l, err = s.makeMeshGatewayListener(a.name, a.Address, a.Port, cfgSnap) if err != nil { @@ -267,10 +276,9 @@ func (s *Server) listenersFromSnapshotGateway(cfgSnap *proxycfg.ConfigSnapshot, return resources, err } -// TODO(ingress): Support configured bind addresses from similar to mesh gateways -// See: https://www.consul.io/docs/connect/proxies/envoy.html#mesh-gateway-options -func (s *Server) listenersFromSnapshotIngressGateway(cfgSnap *proxycfg.ConfigSnapshot) ([]proto.Message, error) { +func (s *Server) makeIngressGatewayListeners(address string, cfgSnap *proxycfg.ConfigSnapshot) ([]proto.Message, error) { var resources []proto.Message + for listenerKey, upstreams := range cfgSnap.IngressGateway.Upstreams { var tlsContext *envoyauth.DownstreamTlsContext if cfgSnap.IngressGateway.TLSEnabled { @@ -290,24 +298,20 @@ func (s *Server) listenersFromSnapshotIngressGateway(cfgSnap *proxycfg.ConfigSna chain := cfgSnap.IngressGateway.DiscoveryChain[id] var upstreamListener proto.Message - var err error - if chain == nil || chain.IsDefault() { - upstreamListener, err = s.makeUpstreamListenerIgnoreDiscoveryChain(&u, chain, cfgSnap, tlsContext) - } else { - upstreamListener, err = s.makeUpstreamListenerForDiscoveryChain(&u, chain, cfgSnap, tlsContext) - } + upstreamListener, err := s.makeUpstreamListenerForDiscoveryChain( + &u, + address, + chain, + cfgSnap, + tlsContext, + ) if err != nil { return nil, err } resources = append(resources, upstreamListener) } else { // If multiple upstreams share this port, make a special listener for the protocol. - addr := cfgSnap.Address - if addr == "" { - addr = "0.0.0.0" - } - - listener := makeListener(listenerKey.Protocol, addr, listenerKey.Port) + listener := makeListener(listenerKey.Protocol, address, listenerKey.Port) filter, err := makeListenerFilter( true, listenerKey.Protocol, listenerKey.RouteName(), "", "ingress_upstream_", "", false) if err != nil { @@ -541,56 +545,6 @@ func (s *Server) makeExposedCheckListener(cfgSnap *proxycfg.ConfigSnapshot, clus return l, err } -// makeUpstreamListenerIgnoreDiscoveryChain counterintuitively takes an (optional) chain -func (s *Server) makeUpstreamListenerIgnoreDiscoveryChain( - u *structs.Upstream, - chain *structs.CompiledDiscoveryChain, - cfgSnap *proxycfg.ConfigSnapshot, - tlsContext *envoyauth.DownstreamTlsContext, -) (proto.Message, error) { - cfg, err := ParseUpstreamConfig(u.Config) - if err != nil { - // Don't hard fail on a config typo, just warn. The parse func returns - // default config if there is an error so it's safe to continue. - s.Logger.Warn("failed to parse", "upstream", u.Identifier(), "error", err) - } - if cfg.ListenerJSON != "" { - return makeListenerFromUserConfig(cfg.ListenerJSON) - } - - addr := u.LocalBindAddress - if addr == "" { - addr = "127.0.0.1" - } - - upstreamID := u.Identifier() - - dc := u.Datacenter - if dc == "" { - dc = cfgSnap.Datacenter - } - sni := connect.UpstreamSNI(u, "", dc, cfgSnap.Roots.TrustDomain) - - clusterName := CustomizeClusterName(sni, chain) - - l := makeListener(upstreamID, addr, u.LocalBindPort) - filter, err := makeListenerFilter( - false, cfg.Protocol, upstreamID, clusterName, "upstream_", "", false) - if err != nil { - return nil, err - } - - l.FilterChains = []envoylistener.FilterChain{ - { - Filters: []envoylistener.Filter{ - filter, - }, - TlsContext: tlsContext, - }, - } - return l, nil -} - func (s *Server) makeTerminatingGatewayListener(name, addr string, port int, cfgSnap *proxycfg.ConfigSnapshot, token string) (*envoy.Listener, error) { l := makeListener(name, addr, port) @@ -790,56 +744,50 @@ func (s *Server) makeMeshGatewayListener(name, addr string, port int, cfgSnap *p func (s *Server) makeUpstreamListenerForDiscoveryChain( u *structs.Upstream, + address string, chain *structs.CompiledDiscoveryChain, cfgSnap *proxycfg.ConfigSnapshot, tlsContext *envoyauth.DownstreamTlsContext, ) (proto.Message, error) { - cfg, err := ParseUpstreamConfigNoDefaults(u.Config) - if err != nil { - // Don't hard fail on a config typo, just warn. The parse func returns - // default config if there is an error so it's safe to continue. - s.Logger.Warn("failed to parse", "upstream", u.Identifier(), "error", err) + if address == "" { + address = "127.0.0.1" } - if cfg.ListenerJSON != "" { - s.Logger.Warn("ignoring escape hatch setting because already configured for", - "discovery chain", chain.ServiceName, "upstream", u.Identifier(), "config", "envoy_listener_json") - } - - addr := u.LocalBindAddress - if addr == "" { - addr = "127.0.0.1" - } - upstreamID := u.Identifier() + l := makeListener(upstreamID, address, u.LocalBindPort) - l := makeListener(upstreamID, addr, u.LocalBindPort) - - proto := cfg.Protocol - if proto == "" { - proto = chain.Protocol - } - - if proto == "" { - proto = "tcp" + cfg := getAndModifyUpstreamConfigForListener(s.Logger, u, chain) + if cfg.ListenerJSON != "" { + return makeListenerFromUserConfig(cfg.ListenerJSON) } useRDS := true clusterName := "" - if proto == "tcp" { + if chain == nil || chain.IsDefault() { + dc := u.Datacenter + if dc == "" { + dc = cfgSnap.Datacenter + } + sni := connect.UpstreamSNI(u, "", dc, cfgSnap.Roots.TrustDomain) + + useRDS = false + clusterName = CustomizeClusterName(sni, chain) + + } else if cfg.Protocol == "tcp" { startNode := chain.Nodes[chain.StartNode] if startNode == nil { panic("missing first node in compiled discovery chain for: " + chain.ServiceName) } else if startNode.Type != structs.DiscoveryGraphNodeTypeResolver { - panic(fmt.Sprintf("unexpected first node in discovery chain using protocol=%q: %s", proto, startNode.Type)) + panic(fmt.Sprintf("unexpected first node in discovery chain using protocol=%q: %s", cfg.Protocol, startNode.Type)) } targetID := startNode.Resolver.Target target := chain.Targets[targetID] - clusterName = CustomizeClusterName(target.Name, chain) + useRDS = false + clusterName = CustomizeClusterName(target.Name, chain) } filter, err := makeListenerFilter( - useRDS, proto, upstreamID, clusterName, "upstream_", "", false) + useRDS, cfg.Protocol, upstreamID, clusterName, "upstream_", "", false) if err != nil { return nil, err } @@ -855,6 +803,53 @@ func (s *Server) makeUpstreamListenerForDiscoveryChain( return l, nil } +func getAndModifyUpstreamConfigForListener(logger hclog.Logger, u *structs.Upstream, chain *structs.CompiledDiscoveryChain) UpstreamConfig { + var ( + cfg UpstreamConfig + err error + ) + + if chain == nil || chain.IsDefault() { + cfg, err = ParseUpstreamConfig(u.Config) + if err != nil { + // Don't hard fail on a config typo, just warn. The parse func returns + // default config if there is an error so it's safe to continue. + logger.Warn("failed to parse", "upstream", u.Identifier(), "error", err) + } + } else { + // Use NoDefaults here so that we can set the protocol to the chain + // protocol if necessary + cfg, err = ParseUpstreamConfigNoDefaults(u.Config) + if err != nil { + // Don't hard fail on a config typo, just warn. The parse func returns + // default config if there is an error so it's safe to continue. + logger.Warn("failed to parse", "upstream", u.Identifier(), "error", err) + } + + if cfg.ListenerJSON != "" { + logger.Warn("ignoring escape hatch setting because already configured for", + "discovery chain", chain.ServiceName, "upstream", u.Identifier(), "config", "envoy_listener_json") + + // Remove from config struct so we don't use it later on + cfg.ListenerJSON = "" + } + + proto := cfg.Protocol + if proto == "" { + proto = chain.Protocol + } + + if proto == "" { + proto = "tcp" + } + + // set back on the config so that we can use it from return value + cfg.Protocol = proto + } + + return cfg +} + func makeListenerFilter( useRDS bool, protocol, filterName, cluster, statPrefix, routePath string, ingress bool) (envoylistener.Filter, error) { diff --git a/agent/xds/listeners_test.go b/agent/xds/listeners_test.go index 64526aa8d9..9b262c4bea 100644 --- a/agent/xds/listeners_test.go +++ b/agent/xds/listeners_test.go @@ -128,6 +128,17 @@ func TestListenersFromSnapshot(t *testing.T) { }) }, }, + { + name: "custom-upstream-typed-ignored-with-disco-chain", + create: proxycfg.TestConfigSnapshotDiscoveryChainWithFailover, + setup: func(snap *proxycfg.ConfigSnapshot) { + snap.Proxy.Upstreams[0].Config["envoy_listener_json"] = + customListenerJSON(t, customListenerJSONOptions{ + Name: "custom-upstream", + IncludeType: true, + }) + }, + }, { name: "splitter-with-resolver-redirect", create: proxycfg.TestConfigSnapshotDiscoveryChain_SplitterWithResolverRedirectMultiDC, @@ -268,6 +279,23 @@ func TestListenersFromSnapshot(t *testing.T) { create: proxycfg.TestConfigSnapshotIngressGateway, setup: nil, }, + { + name: "ingress-gateway-bind-addrs", + create: proxycfg.TestConfigSnapshotIngressGateway, + setup: func(snap *proxycfg.ConfigSnapshot) { + snap.TaggedAddresses = map[string]structs.ServiceAddress{ + "lan": structs.ServiceAddress{Address: "10.0.0.1"}, + "wan": structs.ServiceAddress{Address: "172.16.0.1"}, + } + snap.Proxy.Config = map[string]interface{}{ + "envoy_gateway_no_default_bind": true, + "envoy_gateway_bind_tagged_addresses": true, + "envoy_gateway_bind_addresses": map[string]structs.ServiceAddress{ + "foo": structs.ServiceAddress{Address: "8.8.8.8"}, + }, + } + }, + }, { name: "ingress-gateway-no-services", create: proxycfg.TestConfigSnapshotIngressGatewayNoServices, diff --git a/agent/xds/testdata/listeners/custom-upstream-typed-ignored-with-disco-chain.golden b/agent/xds/testdata/listeners/custom-upstream-typed-ignored-with-disco-chain.golden new file mode 100644 index 0000000000..8908c8c0ef --- /dev/null +++ b/agent/xds/testdata/listeners/custom-upstream-typed-ignored-with-disco-chain.golden @@ -0,0 +1,116 @@ +{ + "versionInfo": "00000001", + "resources": [ + { + "@type": "type.googleapis.com/envoy.api.v2.Listener", + "name": "db:127.0.0.1:9191", + "address": { + "socketAddress": { + "address": "127.0.0.1", + "portValue": 9191 + } + }, + "filterChains": [ + { + "filters": [ + { + "name": "envoy.tcp_proxy", + "config": { + "cluster": "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", + "stat_prefix": "upstream_db_tcp" + } + } + ] + } + ] + }, + { + "@type": "type.googleapis.com/envoy.api.v2.Listener", + "name": "prepared_query:geo-cache:127.10.10.10:8181", + "address": { + "socketAddress": { + "address": "127.10.10.10", + "portValue": 8181 + } + }, + "filterChains": [ + { + "filters": [ + { + "name": "envoy.tcp_proxy", + "config": { + "cluster": "geo-cache.default.dc1.query.11111111-2222-3333-4444-555555555555.consul", + "stat_prefix": "upstream_prepared_query_geo-cache_tcp" + } + } + ] + } + ] + }, + { + "@type": "type.googleapis.com/envoy.api.v2.Listener", + "name": "public_listener:0.0.0.0:9999", + "address": { + "socketAddress": { + "address": "0.0.0.0", + "portValue": 9999 + } + }, + "filterChains": [ + { + "tlsContext": { + "commonTlsContext": { + "tlsParams": { + + }, + "tlsCertificates": [ + { + "certificateChain": { + "inlineString": "-----BEGIN CERTIFICATE-----\nMIICjDCCAjKgAwIBAgIIC5llxGV1gB8wCgYIKoZIzj0EAwIwFDESMBAGA1UEAxMJ\nVGVzdCBDQSAyMB4XDTE5MDMyMjEzNTgyNloXDTI5MDMyMjEzNTgyNlowDjEMMAoG\nA1UEAxMDd2ViMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEADPv1RHVNRfa2VKR\nAB16b6rZnEt7tuhaxCFpQXPj7M2omb0B9Favq5E0ivpNtv1QnFhxtPd7d5k4e+T7\nSkW1TaOCAXIwggFuMA4GA1UdDwEB/wQEAwIDuDAdBgNVHSUEFjAUBggrBgEFBQcD\nAgYIKwYBBQUHAwEwDAYDVR0TAQH/BAIwADBoBgNVHQ4EYQRfN2Q6MDc6ODc6M2E6\nNDA6MTk6NDc6YzM6NWE6YzA6YmE6NjI6ZGY6YWY6NGI6ZDQ6MDU6MjU6NzY6M2Q6\nNWE6OGQ6MTY6OGQ6Njc6NWU6MmU6YTA6MzQ6N2Q6ZGM6ZmYwagYDVR0jBGMwYYBf\nZDE6MTE6MTE6YWM6MmE6YmE6OTc6YjI6M2Y6YWM6N2I6YmQ6ZGE6YmU6YjE6OGE6\nZmM6OWE6YmE6YjU6YmM6ODM6ZTc6NWU6NDE6NmY6ZjI6NzM6OTU6NTg6MGM6ZGIw\nWQYDVR0RBFIwUIZOc3BpZmZlOi8vMTExMTExMTEtMjIyMi0zMzMzLTQ0NDQtNTU1\nNTU1NTU1NTU1LmNvbnN1bC9ucy9kZWZhdWx0L2RjL2RjMS9zdmMvd2ViMAoGCCqG\nSM49BAMCA0gAMEUCIGC3TTvvjj76KMrguVyFf4tjOqaSCRie3nmHMRNNRav7AiEA\npY0heYeK9A6iOLrzqxSerkXXQyj5e9bE4VgUnxgPU6g=\n-----END CERTIFICATE-----\n" + }, + "privateKey": { + "inlineString": "-----BEGIN EC PRIVATE KEY-----\nMHcCAQEEIMoTkpRggp3fqZzFKh82yS4LjtJI+XY+qX/7DefHFrtdoAoGCCqGSM49\nAwEHoUQDQgAEADPv1RHVNRfa2VKRAB16b6rZnEt7tuhaxCFpQXPj7M2omb0B9Fav\nq5E0ivpNtv1QnFhxtPd7d5k4e+T7SkW1TQ==\n-----END EC PRIVATE KEY-----\n" + } + } + ], + "validationContext": { + "trustedCa": { + "inlineString": "-----BEGIN CERTIFICATE-----\nMIICXDCCAgKgAwIBAgIICpZq70Z9LyUwCgYIKoZIzj0EAwIwFDESMBAGA1UEAxMJ\nVGVzdCBDQSAyMB4XDTE5MDMyMjEzNTgyNloXDTI5MDMyMjEzNTgyNlowFDESMBAG\nA1UEAxMJVGVzdCBDQSAyMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEIhywH1gx\nAsMwuF3ukAI5YL2jFxH6Usnma1HFSfVyxbXX1/uoZEYrj8yCAtdU2yoHETyd+Zx2\nThhRLP79pYegCaOCATwwggE4MA4GA1UdDwEB/wQEAwIBhjAPBgNVHRMBAf8EBTAD\nAQH/MGgGA1UdDgRhBF9kMToxMToxMTphYzoyYTpiYTo5NzpiMjozZjphYzo3Yjpi\nZDpkYTpiZTpiMTo4YTpmYzo5YTpiYTpiNTpiYzo4MzplNzo1ZTo0MTo2ZjpmMjo3\nMzo5NTo1ODowYzpkYjBqBgNVHSMEYzBhgF9kMToxMToxMTphYzoyYTpiYTo5Nzpi\nMjozZjphYzo3YjpiZDpkYTpiZTpiMTo4YTpmYzo5YTpiYTpiNTpiYzo4MzplNzo1\nZTo0MTo2ZjpmMjo3Mzo5NTo1ODowYzpkYjA/BgNVHREEODA2hjRzcGlmZmU6Ly8x\nMTExMTExMS0yMjIyLTMzMzMtNDQ0NC01NTU1NTU1NTU1NTUuY29uc3VsMAoGCCqG\nSM49BAMCA0gAMEUCICOY0i246rQHJt8o8Oya0D5PLL1FnmsQmQqIGCi31RwnAiEA\noR5f6Ku+cig2Il8T8LJujOp2/2A72QcHZA57B13y+8o=\n-----END CERTIFICATE-----\n" + } + } + }, + "requireClientCertificate": true + }, + "filters": [ + { + "name": "envoy.ext_authz", + "config": { + "grpc_service": { + "envoy_grpc": { + "cluster_name": "local_agent" + }, + "initial_metadata": [ + { + "key": "x-consul-token", + "value": "my-token" + } + ] + }, + "stat_prefix": "connect_authz" + } + }, + { + "name": "envoy.tcp_proxy", + "config": { + "cluster": "local_app", + "stat_prefix": "public_listener_tcp" + } + } + ] + } + ] + } + ], + "typeUrl": "type.googleapis.com/envoy.api.v2.Listener", + "nonce": "00000001" +} \ No newline at end of file diff --git a/agent/xds/testdata/listeners/ingress-gateway-bind-addrs.golden b/agent/xds/testdata/listeners/ingress-gateway-bind-addrs.golden new file mode 100644 index 0000000000..edf9167e36 --- /dev/null +++ b/agent/xds/testdata/listeners/ingress-gateway-bind-addrs.golden @@ -0,0 +1,76 @@ +{ + "versionInfo": "00000001", + "resources": [ + { + "@type": "type.googleapis.com/envoy.api.v2.Listener", + "name": "db:10.0.0.1:9191", + "address": { + "socketAddress": { + "address": "10.0.0.1", + "portValue": 9191 + } + }, + "filterChains": [ + { + "filters": [ + { + "name": "envoy.tcp_proxy", + "config": { + "cluster": "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", + "stat_prefix": "upstream_db_tcp" + } + } + ] + } + ] + }, + { + "@type": "type.googleapis.com/envoy.api.v2.Listener", + "name": "db:172.16.0.1:9191", + "address": { + "socketAddress": { + "address": "172.16.0.1", + "portValue": 9191 + } + }, + "filterChains": [ + { + "filters": [ + { + "name": "envoy.tcp_proxy", + "config": { + "cluster": "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", + "stat_prefix": "upstream_db_tcp" + } + } + ] + } + ] + }, + { + "@type": "type.googleapis.com/envoy.api.v2.Listener", + "name": "db:8.8.8.8:9191", + "address": { + "socketAddress": { + "address": "8.8.8.8", + "portValue": 9191 + } + }, + "filterChains": [ + { + "filters": [ + { + "name": "envoy.tcp_proxy", + "config": { + "cluster": "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", + "stat_prefix": "upstream_db_tcp" + } + } + ] + } + ] + } + ], + "typeUrl": "type.googleapis.com/envoy.api.v2.Listener", + "nonce": "00000001" +} \ No newline at end of file diff --git a/agent/xds/testdata/listeners/ingress-gateway.golden b/agent/xds/testdata/listeners/ingress-gateway.golden index c97c71bbf2..c91167dfad 100644 --- a/agent/xds/testdata/listeners/ingress-gateway.golden +++ b/agent/xds/testdata/listeners/ingress-gateway.golden @@ -3,10 +3,10 @@ "resources": [ { "@type": "type.googleapis.com/envoy.api.v2.Listener", - "name": "db:2.3.4.5:9191", + "name": "db:1.2.3.4:9191", "address": { "socketAddress": { - "address": "2.3.4.5", + "address": "1.2.3.4", "portValue": 9191 } }, diff --git a/agent/xds/testdata/listeners/ingress-with-chain-and-overrides.golden b/agent/xds/testdata/listeners/ingress-with-chain-and-overrides.golden index cfa7780de7..a3965f01ae 100644 --- a/agent/xds/testdata/listeners/ingress-with-chain-and-overrides.golden +++ b/agent/xds/testdata/listeners/ingress-with-chain-and-overrides.golden @@ -3,10 +3,10 @@ "resources": [ { "@type": "type.googleapis.com/envoy.api.v2.Listener", - "name": "db:2.3.4.5:9191", + "name": "db:1.2.3.4:9191", "address": { "socketAddress": { - "address": "2.3.4.5", + "address": "1.2.3.4", "portValue": 9191 } }, diff --git a/agent/xds/testdata/listeners/ingress-with-chain-external-sni.golden b/agent/xds/testdata/listeners/ingress-with-chain-external-sni.golden index c97c71bbf2..c91167dfad 100644 --- a/agent/xds/testdata/listeners/ingress-with-chain-external-sni.golden +++ b/agent/xds/testdata/listeners/ingress-with-chain-external-sni.golden @@ -3,10 +3,10 @@ "resources": [ { "@type": "type.googleapis.com/envoy.api.v2.Listener", - "name": "db:2.3.4.5:9191", + "name": "db:1.2.3.4:9191", "address": { "socketAddress": { - "address": "2.3.4.5", + "address": "1.2.3.4", "portValue": 9191 } }, diff --git a/agent/xds/testdata/listeners/ingress-with-tcp-chain-failover-through-local-gateway.golden b/agent/xds/testdata/listeners/ingress-with-tcp-chain-failover-through-local-gateway.golden index c97c71bbf2..c91167dfad 100644 --- a/agent/xds/testdata/listeners/ingress-with-tcp-chain-failover-through-local-gateway.golden +++ b/agent/xds/testdata/listeners/ingress-with-tcp-chain-failover-through-local-gateway.golden @@ -3,10 +3,10 @@ "resources": [ { "@type": "type.googleapis.com/envoy.api.v2.Listener", - "name": "db:2.3.4.5:9191", + "name": "db:1.2.3.4:9191", "address": { "socketAddress": { - "address": "2.3.4.5", + "address": "1.2.3.4", "portValue": 9191 } }, diff --git a/agent/xds/testdata/listeners/ingress-with-tcp-chain-failover-through-remote-gateway.golden b/agent/xds/testdata/listeners/ingress-with-tcp-chain-failover-through-remote-gateway.golden index c97c71bbf2..c91167dfad 100644 --- a/agent/xds/testdata/listeners/ingress-with-tcp-chain-failover-through-remote-gateway.golden +++ b/agent/xds/testdata/listeners/ingress-with-tcp-chain-failover-through-remote-gateway.golden @@ -3,10 +3,10 @@ "resources": [ { "@type": "type.googleapis.com/envoy.api.v2.Listener", - "name": "db:2.3.4.5:9191", + "name": "db:1.2.3.4:9191", "address": { "socketAddress": { - "address": "2.3.4.5", + "address": "1.2.3.4", "portValue": 9191 } }, diff --git a/agent/xds/testdata/listeners/ingress-with-tls-listener.golden b/agent/xds/testdata/listeners/ingress-with-tls-listener.golden index c616ffbaa6..fcddd66348 100644 --- a/agent/xds/testdata/listeners/ingress-with-tls-listener.golden +++ b/agent/xds/testdata/listeners/ingress-with-tls-listener.golden @@ -3,10 +3,10 @@ "resources": [ { "@type": "type.googleapis.com/envoy.api.v2.Listener", - "name": "db:2.3.4.5:9191", + "name": "db:1.2.3.4:9191", "address": { "socketAddress": { - "address": "2.3.4.5", + "address": "1.2.3.4", "portValue": 9191 } }, diff --git a/command/connect/envoy/envoy.go b/command/connect/envoy/envoy.go index e6b1b08c2a..2f0ad30b48 100644 --- a/command/connect/envoy/envoy.go +++ b/command/connect/envoy/envoy.go @@ -136,7 +136,8 @@ func (c *cmd) init() { "LAN address to advertise in the gateway service registration") c.flags.Var(&c.wanAddress, "wan-address", - "WAN address to advertise in the gateway service registration") + "WAN address to advertise in the gateway service registration. For ingress gateways, "+ + "only an IP address (without a port) is required.") c.flags.Var(&c.bindAddresses, "bind-address", "Bind "+ "address to use instead of the default binding rules given as `=:` "+ diff --git a/command/connect/envoy/flags.go b/command/connect/envoy/flags.go index 4cf8c37514..7dd714506f 100644 --- a/command/connect/envoy/flags.go +++ b/command/connect/envoy/flags.go @@ -47,8 +47,17 @@ func parseAddress(raw string) (api.ServiceAddress, error) { } addr, portStr, err := net.SplitHostPort(x) + // Error message from Go's net/ipsock.go if err != nil { - return result, fmt.Errorf("Error parsing address %q: %v", x, err) + if !strings.Contains(err.Error(), "missing port in address") { + return result, fmt.Errorf("Error parsing address %q: %v", x, err) + } + + // Use the whole input as the address if there wasn't a port. + if ip := net.ParseIP(x); ip == nil { + return result, fmt.Errorf("Error parsing address %q: not an IP address", x) + } + addr = x } port := defaultGatewayPort diff --git a/command/connect/envoy/flags_test.go b/command/connect/envoy/flags_test.go index 84244a148e..4d6cba1245 100644 --- a/command/connect/envoy/flags_test.go +++ b/command/connect/envoy/flags_test.go @@ -67,9 +67,17 @@ func TestServiceAddressValue_Set(t *testing.T) { expectedValue: api.ServiceAddress{Address: "8.8.8.8", Port: 1234}, }, { - name: "invalid address", - input: "not-an-address", - expectedErr: "missing port in address", + name: "address with no port", + input: "8.8.8.8", + expectedValue: api.ServiceAddress{ + Address: "8.8.8.8", + Port: defaultGatewayPort, + }, + }, + { + name: "invalid addres", + input: "not-an-ip-address", + expectedErr: "not an IP address", }, { name: "invalid port",