Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Append port number to ingress host domain #8190

Merged
merged 1 commit into from
Jul 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 48 additions & 13 deletions agent/xds/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package xds
import (
"errors"
"fmt"
"net"
"strings"

envoy "github.com/envoyproxy/go-control-plane/envoy/api/v2"
Expand Down Expand Up @@ -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
// "<service-name>.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
Expand All @@ -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
// "<service-name>.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)
freddygv marked this conversation as resolved.
Show resolved Hide resolved

// 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,
Expand Down
6 changes: 5 additions & 1 deletion agent/xds/routes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
13 changes: 9 additions & 4 deletions agent/xds/testdata/routes/ingress-http-multiple-services.golden
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
{
"name": "baz",
"domains": [
"baz.ingress.*"
"baz.ingress.*",
"baz.ingress.*:443"
],
"routes": [
{
Expand All @@ -24,7 +25,8 @@
{
"name": "qux",
"domains": [
"qux.ingress.*"
"qux.ingress.*",
"qux.ingress.*:443"
],
"routes": [
{
Expand All @@ -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": [
{
Expand All @@ -64,7 +68,8 @@
{
"name": "bar",
"domains": [
"bar.ingress.*"
"bar.ingress.*",
"bar.ingress.*:8080"
],
"routes": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
{
"name": "db",
"domains": [
"db.ingress.*"
"db.ingress.*",
"db.ingress.*:9191"
],
"routes": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
{
"name": "db",
"domains": [
"db.ingress.*"
"db.ingress.*",
"db.ingress.*:9191"
],
"routes": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
{
"name": "db",
"domains": [
"db.ingress.*"
"db.ingress.*",
"db.ingress.*:9191"
],
"routes": [
{
Expand Down
3 changes: 2 additions & 1 deletion agent/xds/testdata/routes/ingress-with-grpc-router.golden
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
{
"name": "db",
"domains": [
"db.ingress.*"
"db.ingress.*",
"db.ingress.*:9191"
],
"routes": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,15 @@ config_entries {
}

listeners = [
{
port = 9998
protocol = "http"
services = [
{
name = "s1"
}
]
},
{
port = 9999
protocol = "http"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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" ]
}
Expand Down
1 change: 1 addition & 0 deletions test/integration/connect/envoy/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down