From 52bc3300afc1641141bde734d01b524300f06c24 Mon Sep 17 00:00:00 2001 From: Chris Piraino Date: Thu, 25 Jun 2020 11:31:40 -0500 Subject: [PATCH] Append port number to ingress host domain A port can be sent in the Host header as defined in the HTTP RFC, so we take any hosts that we want to match traffic to and also add another host with the listener port added. Also fix an issue with envoy integration tests not running the case-ingress-gateway-tls test. --- agent/xds/routes.go | 61 +++++++++++++++---- agent/xds/routes_test.go | 6 +- .../ingress-http-multiple-services.golden | 13 ++-- ...ess-splitter-with-resolver-redirect.golden | 3 +- .../ingress-with-chain-and-router.golden | 3 +- .../ingress-with-chain-and-splitter.golden | 3 +- .../routes/ingress-with-grpc-router.golden | 3 +- .../config_entries.hcl | 9 +++ .../case-ingress-gateway-tls/verify.bats | 6 +- test/integration/connect/envoy/main_test.go | 1 + 10 files changed, 83 insertions(+), 25 deletions(-) diff --git a/agent/xds/routes.go b/agent/xds/routes.go index dc03229ea97e..bb7c3ddd9afe 100644 --- a/agent/xds/routes.go +++ b/agent/xds/routes.go @@ -3,6 +3,7 @@ package xds import ( "errors" "fmt" + "net" "strings" envoy "github.com/envoyproxy/go-control-plane/envoy/api/v2" @@ -98,19 +99,7 @@ func routesFromSnapshotIngressGateway(cfgSnap *proxycfg.ConfigSnapshot) ([]proto continue } - namespace := u.GetEnterpriseMeta().NamespaceOrDefault() - var domains []string - switch { - case len(u.IngressHosts) > 0: - // If a user has specified hosts, do not add the default - // ".ingress.*" prefixes - domains = u.IngressHosts - case namespace != structs.IntentionDefaultNamespace: - domains = []string{fmt.Sprintf("%s.ingress.%s.*", chain.ServiceName, namespace)} - default: - domains = []string{fmt.Sprintf("%s.ingress.*", chain.ServiceName)} - } - + domains := generateUpstreamIngressDomains(listenerKey, u) virtualHost, err := makeUpstreamRouteForDiscoveryChain(upstreamID, chain, domains) if err != nil { return nil, err @@ -124,6 +113,52 @@ func routesFromSnapshotIngressGateway(cfgSnap *proxycfg.ConfigSnapshot) ([]proto return result, nil } +func generateUpstreamIngressDomains(listenerKey proxycfg.IngressListenerKey, u structs.Upstream) []string { + var domains []string + domainsSet := make(map[string]bool) + + namespace := u.GetEnterpriseMeta().NamespaceOrDefault() + switch { + case len(u.IngressHosts) > 0: + // If a user has specified hosts, do not add the default + // ".ingress.*" prefixes + domains = u.IngressHosts + case namespace != structs.IntentionDefaultNamespace: + domains = []string{fmt.Sprintf("%s.ingress.%s.*", u.DestinationName, namespace)} + default: + domains = []string{fmt.Sprintf("%s.ingress.*", u.DestinationName)} + } + + for _, h := range domains { + domainsSet[h] = true + } + + // Host headers may contain port numbers in them, so we need to make sure + // we match on the host with and without the port number. Well-known + // ports like HTTP/HTTPS are stripped from Host headers, but other ports + // will be in the header. + for _, h := range domains { + _, _, err := net.SplitHostPort(h) + // Error message from Go's net/ipsock.go + // We check to see if a port is not missing, and ignore the + // error from SplitHostPort otherwise, since we have previously + // validated the Host values and should trust the user's input. + if err == nil || !strings.Contains(err.Error(), "missing port in address") { + continue + } + + domainWithPort := fmt.Sprintf("%s:%d", h, listenerKey.Port) + + // Do not add a duplicate domain if a hostname with port is already in the + // set + if !domainsSet[domainWithPort] { + domains = append(domains, domainWithPort) + } + } + + return domains +} + func makeUpstreamRouteForDiscoveryChain( routeName string, chain *structs.CompiledDiscoveryChain, diff --git a/agent/xds/routes_test.go b/agent/xds/routes_test.go index 176e4492a232..a11000ae4a86 100644 --- a/agent/xds/routes_test.go +++ b/agent/xds/routes_test.go @@ -117,7 +117,11 @@ func TestRoutesFromSnapshot(t *testing.T) { { DestinationName: "foo", LocalBindPort: 8080, - IngressHosts: []string{"test1.example.com", "test2.example.com"}, + IngressHosts: []string{ + "test1.example.com", + "test2.example.com", + "test2.example.com:8080", + }, }, { DestinationName: "bar", diff --git a/agent/xds/testdata/routes/ingress-http-multiple-services.golden b/agent/xds/testdata/routes/ingress-http-multiple-services.golden index cdb5d2daa616..aaab96ef9b75 100644 --- a/agent/xds/testdata/routes/ingress-http-multiple-services.golden +++ b/agent/xds/testdata/routes/ingress-http-multiple-services.golden @@ -8,7 +8,8 @@ { "name": "baz", "domains": [ - "baz.ingress.*" + "baz.ingress.*", + "baz.ingress.*:443" ], "routes": [ { @@ -24,7 +25,8 @@ { "name": "qux", "domains": [ - "qux.ingress.*" + "qux.ingress.*", + "qux.ingress.*:443" ], "routes": [ { @@ -48,7 +50,9 @@ "name": "foo", "domains": [ "test1.example.com", - "test2.example.com" + "test2.example.com", + "test2.example.com:8080", + "test1.example.com:8080" ], "routes": [ { @@ -64,7 +68,8 @@ { "name": "bar", "domains": [ - "bar.ingress.*" + "bar.ingress.*", + "bar.ingress.*:8080" ], "routes": [ { diff --git a/agent/xds/testdata/routes/ingress-splitter-with-resolver-redirect.golden b/agent/xds/testdata/routes/ingress-splitter-with-resolver-redirect.golden index ebcc9f0845ad..ef5665e0f2b5 100644 --- a/agent/xds/testdata/routes/ingress-splitter-with-resolver-redirect.golden +++ b/agent/xds/testdata/routes/ingress-splitter-with-resolver-redirect.golden @@ -8,7 +8,8 @@ { "name": "db", "domains": [ - "db.ingress.*" + "db.ingress.*", + "db.ingress.*:9191" ], "routes": [ { diff --git a/agent/xds/testdata/routes/ingress-with-chain-and-router.golden b/agent/xds/testdata/routes/ingress-with-chain-and-router.golden index 0f15394b64b2..1c8b1b2c3bce 100644 --- a/agent/xds/testdata/routes/ingress-with-chain-and-router.golden +++ b/agent/xds/testdata/routes/ingress-with-chain-and-router.golden @@ -8,7 +8,8 @@ { "name": "db", "domains": [ - "db.ingress.*" + "db.ingress.*", + "db.ingress.*:9191" ], "routes": [ { diff --git a/agent/xds/testdata/routes/ingress-with-chain-and-splitter.golden b/agent/xds/testdata/routes/ingress-with-chain-and-splitter.golden index b9a403fa052d..4601126e10b7 100644 --- a/agent/xds/testdata/routes/ingress-with-chain-and-splitter.golden +++ b/agent/xds/testdata/routes/ingress-with-chain-and-splitter.golden @@ -8,7 +8,8 @@ { "name": "db", "domains": [ - "db.ingress.*" + "db.ingress.*", + "db.ingress.*:9191" ], "routes": [ { diff --git a/agent/xds/testdata/routes/ingress-with-grpc-router.golden b/agent/xds/testdata/routes/ingress-with-grpc-router.golden index 789c7015badd..5e2dd93481bf 100644 --- a/agent/xds/testdata/routes/ingress-with-grpc-router.golden +++ b/agent/xds/testdata/routes/ingress-with-grpc-router.golden @@ -8,7 +8,8 @@ { "name": "db", "domains": [ - "db.ingress.*" + "db.ingress.*", + "db.ingress.*:9191" ], "routes": [ { diff --git a/test/integration/connect/envoy/case-ingress-gateway-tls/config_entries.hcl b/test/integration/connect/envoy/case-ingress-gateway-tls/config_entries.hcl index 9a248763018a..2cde25a7c582 100644 --- a/test/integration/connect/envoy/case-ingress-gateway-tls/config_entries.hcl +++ b/test/integration/connect/envoy/case-ingress-gateway-tls/config_entries.hcl @@ -18,6 +18,15 @@ config_entries { } listeners = [ + { + port = 9998 + protocol = "http" + services = [ + { + name = "s1" + } + ] + }, { port = 9999 protocol = "http" diff --git a/test/integration/connect/envoy/case-ingress-gateway-tls/verify.bats b/test/integration/connect/envoy/case-ingress-gateway-tls/verify.bats index 0c08a224d96f..6ee2288908d0 100644 --- a/test/integration/connect/envoy/case-ingress-gateway-tls/verify.bats +++ b/test/integration/connect/envoy/case-ingress-gateway-tls/verify.bats @@ -23,12 +23,12 @@ load helpers } @test "should be able to connect to s1 through the TLS-enabled ingress port" { - assert_dnssan_in_cert localhost:9999 '\*.ingress.consul' + assert_dnssan_in_cert localhost:9998 '\*.ingress.consul' # Use the --resolve argument to fake dns resolution for now so we can use the # s1.ingress.consul domain to validate the cert run retry_default curl --cacert <(get_ca_root) -s -f -d hello \ - --resolve s1.ingress.consul:9999:127.0.0.1 \ - https://s1.ingress.consul:9999 + --resolve s1.ingress.consul:9998:127.0.0.1 \ + https://s1.ingress.consul:9998 [ "$status" -eq 0 ] [ "$output" = "hello" ] } diff --git a/test/integration/connect/envoy/main_test.go b/test/integration/connect/envoy/main_test.go index 9ada95da7d7f..a4f6db32bec9 100644 --- a/test/integration/connect/envoy/main_test.go +++ b/test/integration/connect/envoy/main_test.go @@ -32,6 +32,7 @@ func TestEnvoy(t *testing.T) { "case-ingress-gateway-http", "case-ingress-gateway-multiple-services", "case-ingress-gateway-simple", + "case-ingress-gateway-tls", "case-ingress-mesh-gateways-resolver", "case-multidc-rsa-ca", "case-prometheus",