From 170192d994361ab5aa89248f670492f592da759b Mon Sep 17 00:00:00 2001 From: Saylor Berman Date: Tue, 20 Jun 2023 14:09:37 -0600 Subject: [PATCH 1/3] Add wildcard hostname support Until now, users were only able to use hostnames without wildcards, limiting their options. We now support wildcard hostnames for HTTPRoutes and Gateway listeners. --- docs/gateway-api-compatibility.md | 4 +-- examples/cafe-example/README.md | 23 ++++++++++++++++ examples/cafe-example/gateway.yaml | 1 + internal/state/dataplane/configuration.go | 26 +++++++++---------- .../state/dataplane/configuration_test.go | 18 +++++++++++-- internal/state/graph/gateway_listener_test.go | 2 +- internal/state/graph/httproute.go | 16 ++++++++++-- internal/state/graph/httproute_test.go | 25 ++++++++++++++++++ internal/state/graph/validation.go | 9 +++++-- internal/state/graph/validation_test.go | 2 +- 10 files changed, 103 insertions(+), 23 deletions(-) diff --git a/docs/gateway-api-compatibility.md b/docs/gateway-api-compatibility.md index ccd0926bb..4b9f4b441 100644 --- a/docs/gateway-api-compatibility.md +++ b/docs/gateway-api-compatibility.md @@ -57,7 +57,7 @@ Fields: * `gatewayClassName` - supported. * `listeners` * `name` - supported. - * `hostname` - partially supported. Wildcard hostnames like `*.example.com` are not yet supported. + * `hostname` - supported. * `port` - supported. * `protocol` - partially supported. Allowed values: `HTTP`, `HTTPS`. * `tls` @@ -101,7 +101,7 @@ Fields: Fields: * `spec` * `parentRefs` - partially supported. Port not supported. - * `hostnames` - partially supported. Wildcard binding is not supported: a hostname like `example.com` will not bind to a listener with the hostname `*.example.com`. However, `example.com` will bind to a listener with the empty hostname. + * `hostnames` - supported. * `rules` * `matches` * `path` - partially supported. Only `PathPrefix` and `Exact` types. diff --git a/examples/cafe-example/README.md b/examples/cafe-example/README.md index 83d089336..0e6f1210c 100644 --- a/examples/cafe-example/README.md +++ b/examples/cafe-example/README.md @@ -70,3 +70,26 @@ curl --resolve cafe.example.com:$GW_PORT:$GW_IP http://cafe.example.com:$GW_PORT Server address: 10.12.0.19:80 Server name: tea-7cd44fcb4d-xfw2x ``` + +## 5. Using different hostnames + +Traffic is allowed to `cafe.example.com` because the Gateway listener's hostname allows `*.example.com`. You can +change an HTTPRoute's hostname to something that matches this wildcard and still pass traffic. + +For example, run the following command to open your editor and change the HTTPRoute's hostname to `foo.example.com`. + +``` +kubectl -n default edit httproute tea +``` + +Once changed, update the `curl` command above for the `tea` service to use the new hostname. Traffic should still pass successfully. + +Likewise, if you change the Gateway listener's hostname to something else, you can prevent the HTTPRoute's traffic from passing successfully. + +For example, run the following to open your editor and change the Gateway listener's hostname to `bar.example.com`: + +``` +kubectl -n default edit gateway gateway +``` + +Once changed, try running the same `curl` requests as above. They should be denied with a `404 Not Found`. diff --git a/examples/cafe-example/gateway.yaml b/examples/cafe-example/gateway.yaml index 9fb0ebd1a..f9859bfa1 100644 --- a/examples/cafe-example/gateway.yaml +++ b/examples/cafe-example/gateway.yaml @@ -10,3 +10,4 @@ spec: - name: http port: 80 protocol: HTTP + hostname: "*.example.com" diff --git a/internal/state/dataplane/configuration.go b/internal/state/dataplane/configuration.go index 2f49b9c3e..664ecefec 100644 --- a/internal/state/dataplane/configuration.go +++ b/internal/state/dataplane/configuration.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "sort" + "strings" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/gateway-api/apis/v1beta1" @@ -567,20 +568,9 @@ func convertPathType(pathType v1beta1.PathMatchType) PathType { } } -// listenerHostnameMoreSpecific returns true if host1 is more specific than host2 (using length). +// listenerHostnameMoreSpecific returns true if host1 is more specific than host2. // -// Since the only caller of this function specifies listener hostnames that are both -// bound to the same route hostname, this function assumes that host1 and host2 match, either -// exactly or as a substring. -// -// For example: -// - foo.example.com and "" (host1 wins) -// Non-example: -// - foo.example.com and bar.example.com (should not be given to this function) -// -// As we add regex support, we should put in the proper -// validation and error handling for this function to ensure that the hostnames are actually matching, -// to avoid the unintended inputs above for the invalid case. +// This function assumes that host1 and host2 match, either exactly or as a substring. func listenerHostnameMoreSpecific(host1, host2 *v1beta1.Hostname) bool { var host1Str, host2Str string if host1 != nil { @@ -591,5 +581,15 @@ func listenerHostnameMoreSpecific(host1, host2 *v1beta1.Hostname) bool { host2Str = string(*host2) } + host1Segments := len(strings.Split(host1Str, ".")) + host2Segments := len(strings.Split(host2Str, ".")) + if host1Segments > host2Segments { + return true + } + + if host2Segments > host1Segments { + return false + } + return len(host1Str) >= len(host2Str) } diff --git a/internal/state/dataplane/configuration_test.go b/internal/state/dataplane/configuration_test.go index f65497fa2..63ed81c79 100644 --- a/internal/state/dataplane/configuration_test.go +++ b/internal/state/dataplane/configuration_test.go @@ -2029,15 +2029,29 @@ func TestHostnameMoreSpecific(t *testing.T) { msg: "host1 has value; host2 empty", }, { - host1: helpers.GetPointer(v1beta1.Hostname("example.com")), + host1: helpers.GetPointer(v1beta1.Hostname("foo.bar.example.com")), host2: helpers.GetPointer(v1beta1.Hostname("foo.example.com")), + host1Wins: true, + msg: "host1 has more segments than host2", + }, + { + host1: helpers.GetPointer(v1beta1.Hostname("somelongname.example.com")), + host2: helpers.GetPointer(v1beta1.Hostname("foo.bar.example.com")), + host1Wins: false, + msg: "host2 has more segments than host1", + }, + { + host1: helpers.GetPointer(v1beta1.Hostname("example.com")), + host2: helpers.GetPointer(v1beta1.Hostname("longerexample.com")), host1Wins: false, msg: "host2 longer than host1", }, } for _, tc := range tests { - g.Expect(listenerHostnameMoreSpecific(tc.host1, tc.host2)).To(Equal(tc.host1Wins), tc.msg) + t.Run(tc.msg, func(t *testing.T) { + g.Expect(listenerHostnameMoreSpecific(tc.host1, tc.host2)).To(Equal(tc.host1Wins)) + }) } } diff --git a/internal/state/graph/gateway_listener_test.go b/internal/state/graph/gateway_listener_test.go index f8a9ff3a3..6f229c34b 100644 --- a/internal/state/graph/gateway_listener_test.go +++ b/internal/state/graph/gateway_listener_test.go @@ -222,7 +222,7 @@ func TestValidateListenerHostname(t *testing.T) { }, { hostname: (*v1beta1.Hostname)(helpers.GetStringPointer("*.example.com")), - expectErr: true, + expectErr: false, name: "wildcard hostname", }, { diff --git a/internal/state/graph/httproute.go b/internal/state/graph/httproute.go index 7b9f422fc..27d7a5517 100644 --- a/internal/state/graph/httproute.go +++ b/internal/state/graph/httproute.go @@ -3,6 +3,7 @@ package graph import ( "errors" "fmt" + "strings" apiv1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/labels" @@ -405,20 +406,31 @@ func findAcceptedHostnames(listenerHostname *v1beta1.Hostname, routeHostnames [] if hostname == "" { return true } - return string(h) == hostname + + routeHost := string(h) + return routeHost == hostname || wildcardMatch(hostname, routeHost) || wildcardMatch(routeHost, hostname) } var result []string for _, h := range routeHostnames { if match(h) { - result = append(result, string(h)) + if len(hostname) > len(h) { + result = append(result, hostname) + } else { + result = append(result, string(h)) + } } } return result } +// wildcardMatch checks if host1 is a wildcard host, and if so, checks if host2 is a match for that wildcard. +func wildcardMatch(host1, host2 string) bool { + return strings.HasPrefix(host1, "*.") && strings.HasSuffix(host2, strings.TrimPrefix(host1, "*")) +} + func routeAllowedByListener( listener *Listener, routeNS, diff --git a/internal/state/graph/httproute_test.go b/internal/state/graph/httproute_test.go index 4e0f8c43b..8a46237d8 100644 --- a/internal/state/graph/httproute_test.go +++ b/internal/state/graph/httproute_test.go @@ -1217,6 +1217,7 @@ func TestBindRouteToListeners(t *testing.T) { func TestFindAcceptedHostnames(t *testing.T) { var listenerHostnameFoo v1beta1.Hostname = "foo.example.com" var listenerHostnameCafe v1beta1.Hostname = "cafe.example.com" + var listenerHostnameWildcard v1beta1.Hostname = "*.example.com" routeHostnames := []v1beta1.Hostname{"foo.example.com", "bar.example.com"} tests := []struct { @@ -1255,6 +1256,30 @@ func TestFindAcceptedHostnames(t *testing.T) { expected: []string{wildcardHostname}, msg: "both listener and route have empty hostnames", }, + { + listenerHostname: &listenerHostnameWildcard, + routeHostnames: routeHostnames, + expected: []string{"foo.example.com", "bar.example.com"}, + msg: "listener wildcard hostname", + }, + { + listenerHostname: &listenerHostnameFoo, + routeHostnames: []v1beta1.Hostname{"*.example.com"}, + expected: []string{"foo.example.com"}, + msg: "route wildcard hostname; specific listener hostname", + }, + { + listenerHostname: &listenerHostnameWildcard, + routeHostnames: nil, + expected: []string{"*.example.com"}, + msg: "listener wildcard hostname; nil route hostname", + }, + { + listenerHostname: nil, + routeHostnames: []v1beta1.Hostname{"*.example.com"}, + expected: []string{"*.example.com"}, + msg: "route wildcard hostname; nil listener hostname", + }, } for _, test := range tests { diff --git a/internal/state/graph/validation.go b/internal/state/graph/validation.go index 114168001..910353314 100644 --- a/internal/state/graph/validation.go +++ b/internal/state/graph/validation.go @@ -13,8 +13,13 @@ func validateHostname(hostname string) error { return errors.New("cannot be empty string") } - if strings.Contains(hostname, "*") { - return errors.New("wildcards are not supported") + if strings.HasPrefix(hostname, "*.") { + msgs := validation.IsWildcardDNS1123Subdomain(hostname) + if len(msgs) > 0 { + combined := strings.Join(msgs, ",") + return errors.New(combined) + } + return nil } msgs := validation.IsDNS1123Subdomain(hostname) diff --git a/internal/state/graph/validation_test.go b/internal/state/graph/validation_test.go index dc629999b..34fd13a4f 100644 --- a/internal/state/graph/validation_test.go +++ b/internal/state/graph/validation_test.go @@ -24,7 +24,7 @@ func TestValidateHostname(t *testing.T) { }, { hostname: "*.example.com", - expectErr: true, + expectErr: false, name: "wildcard hostname", }, { From 03b4ed4eb13ebd08fb6bfcd5e757ab8735c16bb3 Mon Sep 17 00:00:00 2001 From: Saylor Berman Date: Thu, 22 Jun 2023 10:16:48 -0600 Subject: [PATCH 2/3] Refactor matching and hostname specificity --- internal/state/dataplane/configuration.go | 15 +-- .../state/dataplane/configuration_test.go | 4 +- internal/state/graph/httproute.go | 92 +++++++++++++++---- internal/state/graph/httproute_test.go | 6 ++ internal/state/graph/validation_test.go | 5 + 5 files changed, 89 insertions(+), 33 deletions(-) diff --git a/internal/state/dataplane/configuration.go b/internal/state/dataplane/configuration.go index 664ecefec..46f34131a 100644 --- a/internal/state/dataplane/configuration.go +++ b/internal/state/dataplane/configuration.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "sort" - "strings" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/gateway-api/apis/v1beta1" @@ -569,8 +568,6 @@ func convertPathType(pathType v1beta1.PathMatchType) PathType { } // listenerHostnameMoreSpecific returns true if host1 is more specific than host2. -// -// This function assumes that host1 and host2 match, either exactly or as a substring. func listenerHostnameMoreSpecific(host1, host2 *v1beta1.Hostname) bool { var host1Str, host2Str string if host1 != nil { @@ -581,15 +578,5 @@ func listenerHostnameMoreSpecific(host1, host2 *v1beta1.Hostname) bool { host2Str = string(*host2) } - host1Segments := len(strings.Split(host1Str, ".")) - host2Segments := len(strings.Split(host2Str, ".")) - if host1Segments > host2Segments { - return true - } - - if host2Segments > host1Segments { - return false - } - - return len(host1Str) >= len(host2Str) + return graph.GetMoreSpecificHostname(host1Str, host2Str) == host1Str } diff --git a/internal/state/dataplane/configuration_test.go b/internal/state/dataplane/configuration_test.go index 63ed81c79..a2aba8aa9 100644 --- a/internal/state/dataplane/configuration_test.go +++ b/internal/state/dataplane/configuration_test.go @@ -1996,8 +1996,6 @@ func TestConvertPathType(t *testing.T) { } func TestHostnameMoreSpecific(t *testing.T) { - g := NewGomegaWithT(t) - tests := []struct { host1 *v1beta1.Hostname host2 *v1beta1.Hostname @@ -2050,6 +2048,8 @@ func TestHostnameMoreSpecific(t *testing.T) { for _, tc := range tests { t.Run(tc.msg, func(t *testing.T) { + g := NewGomegaWithT(t) + g.Expect(listenerHostnameMoreSpecific(tc.host1, tc.host2)).To(Equal(tc.host1Wins)) }) } diff --git a/internal/state/graph/httproute.go b/internal/state/graph/httproute.go index 27d7a5517..ef02bc2df 100644 --- a/internal/state/graph/httproute.go +++ b/internal/state/graph/httproute.go @@ -402,33 +402,91 @@ func findAcceptedHostnames(listenerHostname *v1beta1.Hostname, routeHostnames [] return []string{hostname} } - match := func(h v1beta1.Hostname) bool { - if hostname == "" { - return true - } + var result []string + for _, h := range routeHostnames { routeHost := string(h) - return routeHost == hostname || wildcardMatch(hostname, routeHost) || wildcardMatch(routeHost, hostname) + if match(hostname, routeHost) { + result = append(result, GetMoreSpecificHostname(hostname, routeHost)) + } } - var result []string + return result +} - for _, h := range routeHostnames { - if match(h) { - if len(hostname) > len(h) { - result = append(result, hostname) - } else { - result = append(result, string(h)) +func match(listenerHost, routeHost string) bool { + if listenerHost == "" { + return true + } + + if routeHost == listenerHost { + return true + } + + wildcardMatch := func(host1, host2 string) bool { + return strings.HasPrefix(host1, "*.") && strings.HasSuffix(host2, strings.TrimPrefix(host1, "*")) + } + + // check if listenerHost is a wildcard and routeHost matches + if wildcardMatch(listenerHost, routeHost) { + return true + } + + // check if routeHost is a wildcard and listener matchess + return wildcardMatch(routeHost, listenerHost) +} + +// GetMoreSpecificHostname returns the more specific hostname between the two inputs. +// +// This function assumes that the two hostnames match each other, either: +// - Exactly +// - One as a substring of the other +// - Both as substrings of some parent wildcard +func GetMoreSpecificHostname(hostname1, hostname2 string) string { + if hostname1 == hostname2 { + return hostname1 + } + + if hostname1 == "" { + return hostname2 + } else if hostname2 == "" { + return hostname1 + } + + // Compare if wildcards are present + if strings.HasPrefix(hostname1, "*.") { + if strings.HasPrefix(hostname2, "*.") { + subdomains1 := strings.Split(hostname1, ".") + subdomains2 := strings.Split(hostname2, ".") + + // Compare number of subdomains + if len(subdomains1) > len(subdomains2) { + return hostname1 } + + return hostname2 } + + return hostname2 + } else if strings.HasPrefix(hostname2, "*.") { + return hostname1 } - return result -} + subdomains1 := strings.Split(hostname1, ".") + subdomains2 := strings.Split(hostname2, ".") + + // Compare number of subdomains + if len(subdomains1) > len(subdomains2) { + return hostname1 + } else if len(subdomains1) < len(subdomains2) { + return hostname2 + } + + if len(hostname1) > len(hostname2) { + return hostname1 + } -// wildcardMatch checks if host1 is a wildcard host, and if so, checks if host2 is a match for that wildcard. -func wildcardMatch(host1, host2 string) bool { - return strings.HasPrefix(host1, "*.") && strings.HasSuffix(host2, strings.TrimPrefix(host1, "*")) + return hostname2 } func routeAllowedByListener( diff --git a/internal/state/graph/httproute_test.go b/internal/state/graph/httproute_test.go index 8a46237d8..bd83a0876 100644 --- a/internal/state/graph/httproute_test.go +++ b/internal/state/graph/httproute_test.go @@ -1280,6 +1280,12 @@ func TestFindAcceptedHostnames(t *testing.T) { expected: []string{"*.example.com"}, msg: "route wildcard hostname; nil listener hostname", }, + { + listenerHostname: &listenerHostnameWildcard, + routeHostnames: []v1beta1.Hostname{"*.bar.example.com"}, + expected: []string{"*.bar.example.com"}, + msg: "route and listener wildcard hostnames", + }, } for _, test := range tests { diff --git a/internal/state/graph/validation_test.go b/internal/state/graph/validation_test.go index 34fd13a4f..c680aa2e4 100644 --- a/internal/state/graph/validation_test.go +++ b/internal/state/graph/validation_test.go @@ -32,6 +32,11 @@ func TestValidateHostname(t *testing.T) { expectErr: true, name: "invalid hostname", }, + { + hostname: "*.example.*.com", + expectErr: true, + name: "invalid wildcard hostname", + }, } for _, test := range tests { From aa1e79b40f047396d22ab1946feb81525dcd7e17 Mon Sep 17 00:00:00 2001 From: Saylor Berman Date: Thu, 22 Jun 2023 15:46:04 -0600 Subject: [PATCH 3/3] Simplify matching --- .../state/dataplane/configuration_test.go | 22 +++++++++---------- internal/state/graph/httproute.go | 22 ++++--------------- 2 files changed, 15 insertions(+), 29 deletions(-) diff --git a/internal/state/dataplane/configuration_test.go b/internal/state/dataplane/configuration_test.go index a2aba8aa9..c17ddc9da 100644 --- a/internal/state/dataplane/configuration_test.go +++ b/internal/state/dataplane/configuration_test.go @@ -2027,22 +2027,22 @@ func TestHostnameMoreSpecific(t *testing.T) { msg: "host1 has value; host2 empty", }, { - host1: helpers.GetPointer(v1beta1.Hostname("foo.bar.example.com")), - host2: helpers.GetPointer(v1beta1.Hostname("foo.example.com")), - host1Wins: true, - msg: "host1 has more segments than host2", + host1: helpers.GetPointer(v1beta1.Hostname("")), + host2: helpers.GetPointer(v1beta1.Hostname("example.com")), + host1Wins: false, + msg: "host2 has value; host1 empty", }, { - host1: helpers.GetPointer(v1beta1.Hostname("somelongname.example.com")), - host2: helpers.GetPointer(v1beta1.Hostname("foo.bar.example.com")), - host1Wins: false, - msg: "host2 has more segments than host1", + host1: helpers.GetPointer(v1beta1.Hostname("foo.example.com")), + host2: helpers.GetPointer(v1beta1.Hostname("*.example.com")), + host1Wins: true, + msg: "host1 more specific than host2", }, { - host1: helpers.GetPointer(v1beta1.Hostname("example.com")), - host2: helpers.GetPointer(v1beta1.Hostname("longerexample.com")), + host1: helpers.GetPointer(v1beta1.Hostname("*.example.com")), + host2: helpers.GetPointer(v1beta1.Hostname("foo.example.com")), host1Wins: false, - msg: "host2 longer than host1", + msg: "host2 more specific than host1", }, } diff --git a/internal/state/graph/httproute.go b/internal/state/graph/httproute.go index ef02bc2df..470863bee 100644 --- a/internal/state/graph/httproute.go +++ b/internal/state/graph/httproute.go @@ -441,15 +441,14 @@ func match(listenerHost, routeHost string) bool { // This function assumes that the two hostnames match each other, either: // - Exactly // - One as a substring of the other -// - Both as substrings of some parent wildcard func GetMoreSpecificHostname(hostname1, hostname2 string) string { if hostname1 == hostname2 { return hostname1 } - if hostname1 == "" { return hostname2 - } else if hostname2 == "" { + } + if hostname2 == "" { return hostname1 } @@ -468,25 +467,12 @@ func GetMoreSpecificHostname(hostname1, hostname2 string) string { } return hostname2 - } else if strings.HasPrefix(hostname2, "*.") { - return hostname1 } - - subdomains1 := strings.Split(hostname1, ".") - subdomains2 := strings.Split(hostname2, ".") - - // Compare number of subdomains - if len(subdomains1) > len(subdomains2) { - return hostname1 - } else if len(subdomains1) < len(subdomains2) { - return hostname2 - } - - if len(hostname1) > len(hostname2) { + if strings.HasPrefix(hostname2, "*.") { return hostname1 } - return hostname2 + return "" } func routeAllowedByListener(