diff --git a/conformance/README.md b/conformance/README.md index 26eaac8fb2..d5c8d3dfc6 100644 --- a/conformance/README.md +++ b/conformance/README.md @@ -13,13 +13,14 @@ List available commands: ```bash $ make -build-test-image Build conformance test image +build-test-runner-image Build conformance test runner image create-kind-cluster Create a kind cluster delete-kind-cluster Delete kind cluster help Display this help -install-nkg Install NKG on configured kind cluster +install-nkg Install NKG with provisioner on configured kind cluster +prepare-nkg Build and load NKG container on configured kind cluster run-conformance-tests Run conformance tests -uninstall-nkg Uninstall NKG from configured kind cluster +uninstall-nkg Uninstall NKG on configured kind cluster update-test-kind-config Update kind config ``` ### Step 1 - Create a kind Cluster diff --git a/deploy/manifests/service/loadbalancer-aws-nlb.yaml b/deploy/manifests/service/loadbalancer-aws-nlb.yaml index 60937abadd..8bbf959f7e 100644 --- a/deploy/manifests/service/loadbalancer-aws-nlb.yaml +++ b/deploy/manifests/service/loadbalancer-aws-nlb.yaml @@ -7,10 +7,14 @@ metadata: service.beta.kubernetes.io/aws-load-balancer-type: "nlb" spec: type: LoadBalancer - ports: + ports: # Update the following ports to match your Gateway Listener ports - port: 80 targetPort: 80 protocol: TCP name: http + - port: 443 + targetPort: 443 + protocol: TCP + name: https selector: app: nginx-gateway diff --git a/deploy/manifests/service/loadbalancer.yaml b/deploy/manifests/service/loadbalancer.yaml index 9fddd17b1a..e915aeb3e9 100644 --- a/deploy/manifests/service/loadbalancer.yaml +++ b/deploy/manifests/service/loadbalancer.yaml @@ -6,7 +6,7 @@ metadata: spec: externalTrafficPolicy: Local type: LoadBalancer - ports: + ports: # Update the following ports to match your Gateway Listener ports - port: 80 targetPort: 80 protocol: TCP diff --git a/deploy/manifests/service/nodeport.yaml b/deploy/manifests/service/nodeport.yaml index 5e460a7e4a..be7dd2c0e9 100644 --- a/deploy/manifests/service/nodeport.yaml +++ b/deploy/manifests/service/nodeport.yaml @@ -5,10 +5,14 @@ metadata: namespace: nginx-gateway spec: type: NodePort - ports: + ports: # Update the following ports to match your Gateway Listener ports - port: 80 targetPort: 80 protocol: TCP name: http + - port: 443 + targetPort: 443 + protocol: TCP + name: https selector: app: nginx-gateway diff --git a/docs/gateway-api-compatibility.md b/docs/gateway-api-compatibility.md index b90850f927..4c7ca36374 100644 --- a/docs/gateway-api-compatibility.md +++ b/docs/gateway-api-compatibility.md @@ -58,7 +58,7 @@ Fields: * `listeners` * `name` - supported. * `hostname` - partially supported. Wildcard hostnames like `*.example.com` are not yet supported. - * `port` - partially supported. Allowed values: `80` for HTTP listeners and `443` for HTTPS listeners. + * `port` - supported. * `protocol` - partially supported. Allowed values: `HTTP`, `HTTPS`. * `tls` * `mode` - partially supported. Allowed value: `Terminate`. @@ -86,13 +86,12 @@ Fields: * `Accepted/True/Accepted` * `Accepted/False/UnsupportedProtocol` * `Accepted/False/InvalidCertificateRef` - * `Accepted/False/HostnameConflict` - * `Accepted/False/PortUnavailable` + * `Accepted/False/ProtocolConflict` * `Accepted/False/UnsupportedValue`: Custom reason for when a value of a field in a Listener is invalid or not supported. * `Accepted/False/GatewayConflict`: Custom reason for when the Gateway is ignored due to a conflicting Gateway. NKG only supports a single Gateway. * `ResolvedRefs/True/ResolvedRefs` * `ResolvedRefs/False/InvalidCertificateRef` - * `Conflicted/True/HostnameConflict` + * `Conflicted/True/ProtocolConflict` * `Conflicted/False/NoConflicts` ### HTTPRoute diff --git a/docs/installation.md b/docs/installation.md index 75e65bf7a8..c595699da0 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -71,6 +71,11 @@ This guide walks you through how to install NGINX Kubernetes Gateway on a generi You can gain access to NGINX Kubernetes Gateway by creating a `NodePort` Service or a `LoadBalancer` Service. +> Important +> +> The Service manifests expose NGINX Kubernetes Gateway on ports 80 and 443, which exposes any Gateway [Listener](https://gateway-api.sigs.k8s.io/references/spec/#gateway.networking.k8s.io/v1beta1.Listener) configured for those ports. If you'd like to use different ports in your listeners, +> update the manifests accordingly. + ### Create a NodePort Service Create a Service with type `NodePort`: diff --git a/internal/manager/manager.go b/internal/manager/manager.go index e534ea9c52..e777fea353 100644 --- a/internal/manager/manager.go +++ b/internal/manager/manager.go @@ -56,6 +56,9 @@ func Start(cfg config.Config) error { options := manager.Options{ Scheme: scheme, Logger: logger, + // We disable the metrics server because we reserve all ports (1-65535) for the data plane. + // Once we add support for Prometheus, we can make this port configurable by the user. + MetricsBindAddress: "0", } eventCh := make(chan interface{}) diff --git a/internal/nginx/config/generator_test.go b/internal/nginx/config/generator_test.go index 50f916701e..34aff2412d 100644 --- a/internal/nginx/config/generator_test.go +++ b/internal/nginx/config/generator_test.go @@ -1,9 +1,9 @@ package config_test import ( - "strings" "testing" + . "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/types" "github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/config" @@ -26,20 +26,24 @@ func TestGenerate(t *testing.T) { HTTPServers: []dataplane.VirtualServer{ { IsDefault: true, + Port: 80, }, { Hostname: "example.com", + Port: 80, }, }, SSLServers: []dataplane.VirtualServer{ { IsDefault: true, + Port: 443, }, { Hostname: "example.com", SSL: &dataplane.SSL{ CertificatePath: "/etc/nginx/secrets/default", }, + Port: 443, }, }, Upstreams: []dataplane.Upstream{ @@ -50,22 +54,13 @@ func TestGenerate(t *testing.T) { }, BackendGroups: []dataplane.BackendGroup{bg}, } + g := NewGomegaWithT(t) + generator := config.NewGeneratorImpl() cfg := string(generator.Generate(conf)) - if !strings.Contains(cfg, "listen 80") { - t.Errorf("Generate() did not generate a config with a default HTTP server; config: %s", cfg) - } - - if !strings.Contains(cfg, "listen 443") { - t.Errorf("Generate() did not generate a config with an SSL server; config: %s", cfg) - } - - if !strings.Contains(cfg, "upstream") { - t.Errorf("Generate() did not generate a config with an upstream block; config: %s", cfg) - } - - if !strings.Contains(cfg, "split_clients") { - t.Errorf("Generate() did not generate a config with an split_clients block; config: %s", cfg) - } + g.Expect(cfg).To(ContainSubstring("listen 80")) + g.Expect(cfg).To(ContainSubstring("listen 443")) + g.Expect(cfg).To(ContainSubstring("upstream")) + g.Expect(cfg).To(ContainSubstring("split_clients")) } diff --git a/internal/nginx/config/http/config.go b/internal/nginx/config/http/config.go index f1538109af..f61b21de66 100644 --- a/internal/nginx/config/http/config.go +++ b/internal/nginx/config/http/config.go @@ -7,6 +7,7 @@ type Server struct { Locations []Location IsDefaultHTTP bool IsDefaultSSL bool + Port int32 } // Location holds all configuration for an HTTP location. diff --git a/internal/nginx/config/servers.go b/internal/nginx/config/servers.go index 03383b75f3..2418ae122d 100644 --- a/internal/nginx/config/servers.go +++ b/internal/nginx/config/servers.go @@ -42,7 +42,10 @@ func createServers(httpServers, sslServers []dataplane.VirtualServer) []http.Ser func createSSLServer(virtualServer dataplane.VirtualServer) http.Server { if virtualServer.IsDefault { - return createDefaultSSLServer() + return http.Server{ + IsDefaultSSL: true, + Port: virtualServer.Port, + } } return http.Server{ @@ -51,22 +54,27 @@ func createSSLServer(virtualServer dataplane.VirtualServer) http.Server { Certificate: virtualServer.SSL.CertificatePath, CertificateKey: virtualServer.SSL.CertificatePath, }, - Locations: createLocations(virtualServer.PathRules, 443), + Locations: createLocations(virtualServer.PathRules, virtualServer.Port), + Port: virtualServer.Port, } } func createServer(virtualServer dataplane.VirtualServer) http.Server { if virtualServer.IsDefault { - return createDefaultHTTPServer() + return http.Server{ + IsDefaultHTTP: true, + Port: virtualServer.Port, + } } return http.Server{ ServerName: virtualServer.Hostname, - Locations: createLocations(virtualServer.PathRules, 80), + Locations: createLocations(virtualServer.PathRules, virtualServer.Port), + Port: virtualServer.Port, } } -func createLocations(pathRules []dataplane.PathRule, listenerPort int) []http.Location { +func createLocations(pathRules []dataplane.PathRule, listenerPort int32) []http.Location { lenPathRules := len(pathRules) if lenPathRules == 0 { @@ -171,15 +179,7 @@ func createLocations(pathRules []dataplane.PathRule, listenerPort int) []http.Lo return locs } -func createDefaultSSLServer() http.Server { - return http.Server{IsDefaultSSL: true} -} - -func createDefaultHTTPServer() http.Server { - return http.Server{IsDefaultHTTP: true} -} - -func createReturnValForRedirectFilter(filter *v1beta1.HTTPRequestRedirectFilter, listenerPort int) *http.Return { +func createReturnValForRedirectFilter(filter *v1beta1.HTTPRequestRedirectFilter, listenerPort int32) *http.Return { if filter == nil { return nil } @@ -196,7 +196,7 @@ func createReturnValForRedirectFilter(filter *v1beta1.HTTPRequestRedirectFilter, port := listenerPort if filter.Port != nil { - port = int(*filter.Port) + port = int32(*filter.Port) } scheme := "$scheme" diff --git a/internal/nginx/config/servers_template.go b/internal/nginx/config/servers_template.go index 16736564db..24be76de7b 100644 --- a/internal/nginx/config/servers_template.go +++ b/internal/nginx/config/servers_template.go @@ -1,57 +1,59 @@ package config var serversTemplateText = ` -{{ range $s := . -}} - {{ if $s.IsDefaultSSL -}} +{{- range $s := . -}} + {{ if $s.IsDefaultSSL -}} server { - listen 443 ssl default_server; + listen {{ $s.Port }} ssl default_server; - ssl_reject_handshake on; + ssl_reject_handshake on; } - {{- else if $s.IsDefaultHTTP }} + {{- else if $s.IsDefaultHTTP }} server { - listen 80 default_server; + listen {{ $s.Port }} default_server; - default_type text/html; - return 404; + default_type text/html; + return 404; } - {{- else }} + {{- else }} server { - {{- if $s.SSL }} - listen 443 ssl; - ssl_certificate {{ $s.SSL.Certificate }}; - ssl_certificate_key {{ $s.SSL.CertificateKey }}; + {{- if $s.SSL }} + listen {{ $s.Port }} ssl; + ssl_certificate {{ $s.SSL.Certificate }}; + ssl_certificate_key {{ $s.SSL.CertificateKey }}; - if ($ssl_server_name != $host) { - return 421; - } - {{- end }} + if ($ssl_server_name != $host) { + return 421; + } + {{- else }} + listen {{ $s.Port }}; + {{- end }} - server_name {{ $s.ServerName }}; + server_name {{ $s.ServerName }}; - {{ range $l := $s.Locations }} - location {{ if $l.Exact }}= {{ end }}{{ $l.Path }} { - {{ if $l.Internal -}} - internal; - {{ end }} + {{ range $l := $s.Locations }} + location {{ if $l.Exact }}= {{ end }}{{ $l.Path }} { + {{ if $l.Internal -}} + internal; + {{ end }} - {{- if $l.Return -}} - return {{ $l.Return.Code }} "{{ $l.Return.Body }}"; - {{ end }} + {{- if $l.Return -}} + return {{ $l.Return.Code }} "{{ $l.Return.Body }}"; + {{ end }} - {{- if $l.HTTPMatchVar -}} - set $http_matches {{ $l.HTTPMatchVar | printf "%q" }}; - js_content httpmatches.redirect; - {{ end }} + {{- if $l.HTTPMatchVar -}} + set $http_matches {{ $l.HTTPMatchVar | printf "%q" }}; + js_content httpmatches.redirect; + {{ end }} - {{- if $l.ProxyPass -}} - proxy_set_header Host $host; - proxy_pass {{ $l.ProxyPass }}$request_uri; - {{- end }} - } - {{ end }} + {{- if $l.ProxyPass -}} + proxy_set_header Host $host; + proxy_pass {{ $l.ProxyPass }}$request_uri; + {{- end }} + } + {{ end }} } - {{- end }} + {{- end }} {{ end }} server { listen unix:/var/lib/nginx/nginx-502-server.sock; diff --git a/internal/nginx/config/servers_test.go b/internal/nginx/config/servers_test.go index 5a5a8ba71d..ee97d3f710 100644 --- a/internal/nginx/config/servers_test.go +++ b/internal/nginx/config/servers_test.go @@ -22,41 +22,48 @@ func TestExecuteServers(t *testing.T) { HTTPServers: []dataplane.VirtualServer{ { IsDefault: true, + Port: 8080, }, { Hostname: "example.com", + Port: 8080, }, { Hostname: "cafe.example.com", + Port: 8080, }, }, SSLServers: []dataplane.VirtualServer{ { IsDefault: true, + Port: 8443, }, { Hostname: "example.com", SSL: &dataplane.SSL{ CertificatePath: "cert-path", }, + Port: 8443, }, { Hostname: "cafe.example.com", SSL: &dataplane.SSL{ CertificatePath: "cert-path", }, + Port: 8443, }, }, } expSubStrings := map[string]int{ - "listen 80 default_server;": 1, - "listen 443 ssl;": 2, - "listen 443 ssl default_server;": 1, - "server_name example.com;": 2, - "server_name cafe.example.com;": 2, - "ssl_certificate cert-path;": 2, - "ssl_certificate_key cert-path;": 2, + "listen 8080 default_server;": 1, + "listen 8080;": 2, + "listen 8443 ssl;": 2, + "listen 8443 ssl default_server;": 1, + "server_name example.com;": 2, + "server_name cafe.example.com;": 2, + "ssl_certificate cert-path;": 2, + "ssl_certificate_key cert-path;": 2, } servers := string(executeServers(conf)) @@ -74,88 +81,85 @@ func TestExecuteServers(t *testing.T) { func TestExecuteForDefaultServers(t *testing.T) { testcases := []struct { - msg string - conf dataplane.Configuration - httpDefault bool - sslDefault bool + msg string + conf dataplane.Configuration + httpPorts []int + sslPorts []int }{ { - conf: dataplane.Configuration{}, - httpDefault: false, - sslDefault: false, - msg: "no default servers", + conf: dataplane.Configuration{}, + msg: "no default servers", }, { conf: dataplane.Configuration{ HTTPServers: []dataplane.VirtualServer{ { IsDefault: true, + Port: 80, }, }, }, - httpDefault: true, - sslDefault: false, - msg: "only HTTP default server", + httpPorts: []int{80}, + msg: "only HTTP default server", }, { conf: dataplane.Configuration{ SSLServers: []dataplane.VirtualServer{ { IsDefault: true, + Port: 443, }, }, }, - httpDefault: false, - sslDefault: true, - msg: "only HTTPS default server", + sslPorts: []int{443}, + msg: "only HTTPS default server", }, { conf: dataplane.Configuration{ HTTPServers: []dataplane.VirtualServer{ { IsDefault: true, + Port: 80, + }, + { + IsDefault: true, + Port: 8080, }, }, SSLServers: []dataplane.VirtualServer{ { IsDefault: true, + Port: 443, + }, + { + IsDefault: true, + Port: 8443, }, }, }, - httpDefault: true, - sslDefault: true, - msg: "both HTTP and HTTPS default servers", + httpPorts: []int{80, 8080}, + sslPorts: []int{443, 8443}, + msg: "multiple HTTP and HTTPS default servers", }, } - for _, tc := range testcases { - cfg := string(executeServers(tc.conf)) - - defaultSSLExists := strings.Contains(cfg, "listen 443 ssl default_server") - defaultHTTPExists := strings.Contains(cfg, "listen 80 default_server") + sslDefaultFmt := "listen %d ssl default_server" + httpDefaultFmt := "listen %d default_server" - if tc.sslDefault && !defaultSSLExists { - t.Errorf( - "executeServers() did not generate a config with a default TLS termination server for test: %q", - tc.msg, - ) - } - - if !tc.sslDefault && defaultSSLExists { - t.Errorf("executeServers() generated a config with a default TLS termination server for test: %q", tc.msg) - } + for _, tc := range testcases { + t.Run(tc.msg, func(t *testing.T) { + g := NewGomegaWithT(t) - if tc.httpDefault && !defaultHTTPExists { - t.Errorf("executeServers() did not generate a config with a default http server for test: %q", tc.msg) - } + cfg := string(executeServers(tc.conf)) - if !tc.httpDefault && defaultHTTPExists { - t.Errorf("executeServers() generated a config with a default http server for test: %q", tc.msg) - } + for _, expPort := range tc.httpPorts { + g.Expect(cfg).To(ContainSubstring(fmt.Sprintf(httpDefaultFmt, expPort))) + } - if len(cfg) == 0 { - t.Errorf("executeServers() generated empty config for test: %q", tc.msg) - } + for _, expPort := range tc.sslPorts { + g.Expect(cfg).To(ContainSubstring(fmt.Sprintf(sslDefaultFmt, expPort))) + } + }) } } @@ -494,21 +498,25 @@ func TestCreateServers(t *testing.T) { httpServers := []dataplane.VirtualServer{ { IsDefault: true, + Port: 8080, }, { Hostname: "cafe.example.com", PathRules: cafePathRules, + Port: 8080, }, } sslServers := []dataplane.VirtualServer{ { IsDefault: true, + Port: 8443, }, { Hostname: "cafe.example.com", SSL: &dataplane.SSL{CertificatePath: certPath}, PathRules: cafePathRules, + Port: 8443, }, } @@ -541,9 +549,9 @@ func TestCreateServers(t *testing.T) { } getExpectedLocations := func(isHTTPS bool) []http.Location { - port := 80 + port := 8080 if isHTTPS { - port = 443 + port = 8443 } return []http.Location{ @@ -620,18 +628,22 @@ func TestCreateServers(t *testing.T) { expectedServers := []http.Server{ { IsDefaultHTTP: true, + Port: 8080, }, { ServerName: "cafe.example.com", Locations: getExpectedLocations(false), + Port: 8080, }, { IsDefaultSSL: true, + Port: 8443, }, { ServerName: "cafe.example.com", SSL: &http.SSL{Certificate: certPath, CertificateKey: certPath}, Locations: getExpectedLocations(true), + Port: 8443, }, } diff --git a/internal/nginx/config/upstreams_template.go b/internal/nginx/config/upstreams_template.go index 2058e3881a..c0c2ed24c5 100644 --- a/internal/nginx/config/upstreams_template.go +++ b/internal/nginx/config/upstreams_template.go @@ -12,5 +12,5 @@ upstream {{ $u.Name }} { server {{ $server.Address }}; {{- end }} } -{{ end }} +{{ end -}} ` diff --git a/internal/state/change_processor_test.go b/internal/state/change_processor_test.go index dedf884d78..9b8c83bd3e 100644 --- a/internal/state/change_processor_test.go +++ b/internal/state/change_processor_test.go @@ -1590,6 +1590,16 @@ var _ = Describe("ChangeProcessor", func() { } }), ), + Entry("listener hostnames conflict", + createInvalidGateway(func(gw *v1beta1.Gateway) { + gw.Spec.Listeners = append(gw.Spec.Listeners, v1beta1.Listener{ + Name: "listener-80-2", + Hostname: nil, + Port: 80, + Protocol: v1beta1.HTTPProtocolType, + }) + }), + ), ) }) }) diff --git a/internal/state/conditions/conditions.go b/internal/state/conditions/conditions.go index a6fcbe19b8..687d37b4b6 100644 --- a/internal/state/conditions/conditions.go +++ b/internal/state/conditions/conditions.go @@ -258,16 +258,6 @@ func NewDefaultListenerConditions() []Condition { } } -// NewListenerPortUnavailable returns a Condition that indicates a port is unavailable in a Listener. -func NewListenerPortUnavailable(msg string) Condition { - return Condition{ - Type: string(v1beta1.ListenerConditionAccepted), - Status: metav1.ConditionFalse, - Reason: string(v1beta1.ListenerReasonPortUnavailable), - Message: msg, - } -} - // NewListenerAccepted returns a Condition that indicates that the Listener is accepted. func NewListenerAccepted() Condition { return Condition{ @@ -327,19 +317,20 @@ func NewListenerInvalidCertificateRef(msg string) []Condition { } } -// NewListenerConflictedHostname returns Conditions that indicate that a hostname of a Listener is conflicted. -func NewListenerConflictedHostname(msg string) []Condition { +// NewListenerProtocolConflict returns Conditions that indicate multiple Listeners are specified with the same +// Listener port number, but have conflicting protocol specifications. +func NewListenerProtocolConflict(msg string) []Condition { return []Condition{ { Type: string(v1beta1.ListenerConditionAccepted), Status: metav1.ConditionFalse, - Reason: string(v1beta1.ListenerReasonHostnameConflict), + Reason: string(v1beta1.ListenerReasonProtocolConflict), Message: msg, }, { Type: string(v1beta1.ListenerConditionConflicted), Status: metav1.ConditionTrue, - Reason: string(v1beta1.ListenerReasonHostnameConflict), + Reason: string(v1beta1.ListenerReasonProtocolConflict), Message: msg, }, } diff --git a/internal/state/dataplane/configuration.go b/internal/state/dataplane/configuration.go index 6db9253543..0fa404df79 100644 --- a/internal/state/dataplane/configuration.go +++ b/internal/state/dataplane/configuration.go @@ -23,10 +23,8 @@ const ( // Configuration is an intermediate representation of dataplane configuration. type Configuration struct { // HTTPServers holds all HTTPServers. - // We assume that all servers are HTTP and listen on port 80. HTTPServers []VirtualServer // SSLServers holds all SSLServers. - // We assume that all SSL servers listen on port 443. SSLServers []VirtualServer // Upstreams holds all unique Upstreams. Upstreams []Upstream @@ -44,6 +42,8 @@ type VirtualServer struct { PathRules []PathRule // IsDefault indicates whether the server is the default server. IsDefault bool + // Port is the port of the server. + Port int32 } type Upstream struct { @@ -217,14 +217,19 @@ func newBackendGroup(refs []graph.BackendRef, sourceNsName types.NamespacedName, } func buildServers(listeners map[string]*graph.Listener) (http, ssl []VirtualServer) { - rulesForProtocol := map[v1beta1.ProtocolType]*hostPathRules{ - v1beta1.HTTPProtocolType: newHostPathRules(), - v1beta1.HTTPSProtocolType: newHostPathRules(), + rulesForProtocol := map[v1beta1.ProtocolType]portPathRules{ + v1beta1.HTTPProtocolType: make(portPathRules), + v1beta1.HTTPSProtocolType: make(portPathRules), } for _, l := range listeners { if l.Valid { - rules := rulesForProtocol[l.Source.Protocol] + rules := rulesForProtocol[l.Source.Protocol][l.Source.Port] + if rules == nil { + rules = newHostPathRules() + rulesForProtocol[l.Source.Protocol][l.Source.Port] = rules + } + rules.upsertListener(l) } } @@ -235,6 +240,24 @@ func buildServers(listeners map[string]*graph.Listener) (http, ssl []VirtualServ return httpRules.buildServers(), sslRules.buildServers() } +// portPathRules keeps track of hostPathRules per port +type portPathRules map[v1beta1.PortNumber]*hostPathRules + +func (p portPathRules) buildServers() []VirtualServer { + serverCount := 0 + for _, rules := range p { + serverCount += rules.maxServerCount() + } + + servers := make([]VirtualServer, 0, serverCount) + + for _, rules := range p { + servers = append(servers, rules.buildServers()...) + } + + return servers +} + type pathAndType struct { path string pathType v1beta1.PathMatchType @@ -245,6 +268,7 @@ type hostPathRules struct { listenersForHost map[string]*graph.Listener httpsListeners []*graph.Listener listenersExist bool + port int32 } func newHostPathRules() *hostPathRules { @@ -257,6 +281,7 @@ func newHostPathRules() *hostPathRules { func (hpr *hostPathRules) upsertListener(l *graph.Listener) { hpr.listenersExist = true + hpr.port = int32(l.Source.Port) if l.Source.Protocol == v1beta1.HTTPSProtocolType { hpr.httpsListeners = append(hpr.httpsListeners, l) @@ -336,6 +361,7 @@ func (hpr *hostPathRules) buildServers() []VirtualServer { s := VirtualServer{ Hostname: h, PathRules: make([]PathRule, 0, len(rules)), + Port: hpr.port, } l, ok := hpr.listenersForHost[h] @@ -372,6 +398,7 @@ func (hpr *hostPathRules) buildServers() []VirtualServer { if len(l.Routes) == 0 || hostname == wildcardHostname { s := VirtualServer{ Hostname: hostname, + Port: hpr.port, } if l.SecretPath != "" { @@ -384,7 +411,10 @@ func (hpr *hostPathRules) buildServers() []VirtualServer { // if any listeners exist, we need to generate a default server block. if hpr.listenersExist { - servers = append(servers, VirtualServer{IsDefault: true}) + servers = append(servers, VirtualServer{ + IsDefault: true, + Port: hpr.port, + }) } // We sort the servers so the order is preserved after reconfiguration. @@ -395,6 +425,15 @@ func (hpr *hostPathRules) buildServers() []VirtualServer { return servers } +// maxServerCount returns the maximum number of VirtualServers that can be built from the host path rules. +func (hpr *hostPathRules) maxServerCount() int { + // to calculate max # of servers we add up: + // - # of hostnames + // - # of https listeners - this is to account for https wildcard default servers + // - default server - for every hostPathRules we generate 1 default server + return len(hpr.rulesPerHost) + len(hpr.httpsListeners) + 1 +} + func buildUpstreams( ctx context.Context, listeners map[string]*graph.Listener, diff --git a/internal/state/dataplane/configuration_test.go b/internal/state/dataplane/configuration_test.go index 6fefb69206..1c343b67fc 100644 --- a/internal/state/dataplane/configuration_test.go +++ b/internal/state/dataplane/configuration_test.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "sort" "testing" "github.com/google/go-cmp/cmp" @@ -202,6 +201,7 @@ func TestBuildConfiguration(t *testing.T) { pathAndType{path: "/", pathType: prefix}, pathAndType{path: "/third", pathType: prefix}, ) + hr4, expHR4Groups, routeHR4 := createTestResources( "hr-4", "foo.example.com", @@ -238,6 +238,14 @@ func TestBuildConfiguration(t *testing.T) { pathAndType{path: "/valid", pathType: prefix}, pathAndType{path: "/valid", pathType: v1beta1.PathMatchExact}, ) + hr8, expHR8Groups, routeHR8 := createTestResources( + "hr-8", + "foo.example.com", // same as hr3 + "listener-8080", + pathAndType{path: "/", pathType: prefix}, + pathAndType{path: "/third", pathType: prefix}, + ) + httpsHR1, expHTTPSHR1Groups, httpsRouteHR1 := createTestResources( "https-hr-1", "foo.example.com", @@ -282,6 +290,13 @@ func TestBuildConfiguration(t *testing.T) { pathAndType{path: "/valid", pathType: prefix}, pathAndType{path: invalidMatchesPath, pathType: prefix}, ) + httpsHR7, expHTTPSHR7Groups, httpsRouteHR7 := createTestResources( + "https-hr-7", + "foo.example.com", // same as httpsHR3 + "listener-8443", + pathAndType{path: "/", pathType: prefix}, pathAndType{path: "/third", pathType: prefix}, + ) + listener80 := v1beta1.Listener{ Name: "listener-80-1", Hostname: nil, @@ -289,6 +304,13 @@ func TestBuildConfiguration(t *testing.T) { Protocol: v1beta1.HTTPProtocolType, } + listener8080 := v1beta1.Listener{ + Name: "listener-8080", + Hostname: nil, + Port: 8080, + Protocol: v1beta1.HTTPProtocolType, + } + listener443 := v1beta1.Listener{ Name: "listener-443-1", Hostname: nil, @@ -305,6 +327,24 @@ func TestBuildConfiguration(t *testing.T) { }, }, } + + listener8443 := v1beta1.Listener{ + Name: "listener-8443", + Hostname: nil, + Port: 8443, + Protocol: v1beta1.HTTPSProtocolType, + TLS: &v1beta1.GatewayTLSConfig{ + Mode: helpers.GetTLSModePointer(v1beta1.TLSModeTerminate), + CertificateRefs: []v1beta1.SecretObjectReference{ + { + Kind: (*v1beta1.Kind)(helpers.GetStringPointer("Secret")), + Name: "secret", + Namespace: (*v1beta1.Namespace)(helpers.GetStringPointer("test")), + }, + }, + }, + } + hostname := v1beta1.Hostname("example.com") listener443WithHostname := v1beta1.Listener{ @@ -380,6 +420,7 @@ func TestBuildConfiguration(t *testing.T) { HTTPServers: []VirtualServer{ { IsDefault: true, + Port: 80, }, }, SSLServers: []VirtualServer{}, @@ -416,14 +457,17 @@ func TestBuildConfiguration(t *testing.T) { SSLServers: []VirtualServer{ { IsDefault: true, + Port: 443, }, { Hostname: string(hostname), SSL: &SSL{CertificatePath: secretPath}, + Port: 443, }, { Hostname: wildcardHostname, SSL: &SSL{CertificatePath: secretPath}, + Port: 443, }, }, }, @@ -483,6 +527,7 @@ func TestBuildConfiguration(t *testing.T) { HTTPServers: []VirtualServer{ { IsDefault: true, + Port: 80, }, { Hostname: "bar.example.com", @@ -500,6 +545,7 @@ func TestBuildConfiguration(t *testing.T) { }, }, }, + Port: 80, }, { Hostname: "foo.example.com", @@ -517,6 +563,7 @@ func TestBuildConfiguration(t *testing.T) { }, }, }, + Port: 80, }, }, SSLServers: []VirtualServer{}, @@ -564,6 +611,7 @@ func TestBuildConfiguration(t *testing.T) { SSLServers: []VirtualServer{ { IsDefault: true, + Port: 443, }, { Hostname: "bar.example.com", @@ -584,6 +632,7 @@ func TestBuildConfiguration(t *testing.T) { SSL: &SSL{ CertificatePath: secretPath, }, + Port: 443, }, { Hostname: "example.com", @@ -604,6 +653,7 @@ func TestBuildConfiguration(t *testing.T) { SSL: &SSL{ CertificatePath: secretPath, }, + Port: 443, }, { Hostname: "foo.example.com", @@ -624,10 +674,12 @@ func TestBuildConfiguration(t *testing.T) { SSL: &SSL{ CertificatePath: secretPath, }, + Port: 443, }, { Hostname: wildcardHostname, SSL: &SSL{CertificatePath: secretPath}, + Port: 443, }, }, Upstreams: []Upstream{fooUpstream}, @@ -674,6 +726,7 @@ func TestBuildConfiguration(t *testing.T) { HTTPServers: []VirtualServer{ { IsDefault: true, + Port: 80, }, { Hostname: "foo.example.com", @@ -721,11 +774,13 @@ func TestBuildConfiguration(t *testing.T) { }, }, }, + Port: 80, }, }, SSLServers: []VirtualServer{ { IsDefault: true, + Port: 443, }, { Hostname: "foo.example.com", @@ -776,10 +831,12 @@ func TestBuildConfiguration(t *testing.T) { }, }, }, + Port: 443, }, { Hostname: wildcardHostname, SSL: &SSL{CertificatePath: secretPath}, + Port: 443, }, }, Upstreams: []Upstream{fooUpstream}, @@ -796,6 +853,226 @@ func TestBuildConfiguration(t *testing.T) { }, msg: "one http and one https listener with two routes with the same hostname with and without collisions", }, + { + graph: &graph.Graph{ + GatewayClass: &graph.GatewayClass{ + Source: &v1beta1.GatewayClass{}, + Valid: true, + }, + Gateway: &graph.Gateway{ + Source: &v1beta1.Gateway{}, + Listeners: map[string]*graph.Listener{ + "listener-80-1": { + Source: listener80, + Valid: true, + Routes: map[types.NamespacedName]*graph.Route{ + {Namespace: "test", Name: "hr-3"}: routeHR3, + }, + }, + "listener-8080": { + Source: listener8080, + Valid: true, + Routes: map[types.NamespacedName]*graph.Route{ + {Namespace: "test", Name: "hr-8"}: routeHR8, + }, + }, + "listener-443-1": { + Source: listener443, + Valid: true, + SecretPath: secretPath, + Routes: map[types.NamespacedName]*graph.Route{ + {Namespace: "test", Name: "https-hr-3"}: httpsRouteHR3, + }, + }, + "listener-8443": { + Source: listener8443, + Valid: true, + SecretPath: secretPath, + Routes: map[types.NamespacedName]*graph.Route{ + {Namespace: "test", Name: "https-hr-7"}: httpsRouteHR7, + }, + }, + }, + }, + Routes: map[types.NamespacedName]*graph.Route{ + {Namespace: "test", Name: "hr-3"}: routeHR3, + {Namespace: "test", Name: "hr-8"}: routeHR8, + {Namespace: "test", Name: "https-hr-3"}: httpsRouteHR3, + {Namespace: "test", Name: "https-hr-7"}: httpsRouteHR7, + }, + }, + expConf: Configuration{ + HTTPServers: []VirtualServer{ + { + IsDefault: true, + Port: 80, + }, + { + Hostname: "foo.example.com", + PathRules: []PathRule{ + { + Path: "/", + PathType: PathTypePrefix, + MatchRules: []MatchRule{ + { + MatchIdx: 0, + RuleIdx: 0, + BackendGroup: expHR3Groups[0], + Source: hr3, + }, + }, + }, + { + Path: "/third", + PathType: PathTypePrefix, + MatchRules: []MatchRule{ + { + MatchIdx: 0, + RuleIdx: 1, + BackendGroup: expHR3Groups[1], + Source: hr3, + }, + }, + }, + }, + Port: 80, + }, + { + IsDefault: true, + Port: 8080, + }, + { + Hostname: "foo.example.com", + PathRules: []PathRule{ + { + Path: "/", + PathType: PathTypePrefix, + MatchRules: []MatchRule{ + { + MatchIdx: 0, + RuleIdx: 0, + BackendGroup: expHR8Groups[0], + Source: hr8, + }, + }, + }, + { + Path: "/third", + PathType: PathTypePrefix, + MatchRules: []MatchRule{ + { + MatchIdx: 0, + RuleIdx: 1, + BackendGroup: expHR8Groups[1], + Source: hr8, + }, + }, + }, + }, + Port: 8080, + }, + }, + SSLServers: []VirtualServer{ + { + IsDefault: true, + Port: 443, + }, + { + Hostname: "foo.example.com", + SSL: &SSL{ + CertificatePath: secretPath, + }, + PathRules: []PathRule{ + { + Path: "/", + PathType: PathTypePrefix, + MatchRules: []MatchRule{ + { + MatchIdx: 0, + RuleIdx: 0, + BackendGroup: expHTTPSHR3Groups[0], + Source: httpsHR3, + }, + }, + }, + { + Path: "/third", + PathType: PathTypePrefix, + MatchRules: []MatchRule{ + { + MatchIdx: 0, + RuleIdx: 1, + BackendGroup: expHTTPSHR3Groups[1], + Source: httpsHR3, + }, + }, + }, + }, + Port: 443, + }, + { + Hostname: wildcardHostname, + SSL: &SSL{CertificatePath: secretPath}, + Port: 443, + }, + { + IsDefault: true, + Port: 8443, + }, + { + Hostname: "foo.example.com", + SSL: &SSL{ + CertificatePath: secretPath, + }, + PathRules: []PathRule{ + { + Path: "/", + PathType: PathTypePrefix, + MatchRules: []MatchRule{ + { + MatchIdx: 0, + RuleIdx: 0, + BackendGroup: expHTTPSHR7Groups[0], + Source: httpsHR7, + }, + }, + }, + { + Path: "/third", + PathType: PathTypePrefix, + MatchRules: []MatchRule{ + { + MatchIdx: 0, + RuleIdx: 1, + BackendGroup: expHTTPSHR7Groups[1], + Source: httpsHR7, + }, + }, + }, + }, + Port: 8443, + }, + { + Hostname: wildcardHostname, + SSL: &SSL{CertificatePath: secretPath}, + Port: 8443, + }, + }, + Upstreams: []Upstream{fooUpstream}, + BackendGroups: []BackendGroup{ + expHR3Groups[0], + expHR3Groups[1], + expHR8Groups[0], + expHR8Groups[1], + expHTTPSHR3Groups[0], + expHTTPSHR3Groups[1], + expHTTPSHR7Groups[0], + expHTTPSHR7Groups[1], + }, + }, + + msg: "multiple http and https listener; different ports", + }, { graph: &graph.Graph{ GatewayClass: &graph.GatewayClass{ @@ -881,6 +1158,7 @@ func TestBuildConfiguration(t *testing.T) { HTTPServers: []VirtualServer{ { IsDefault: true, + Port: 80, }, { Hostname: "foo.example.com", @@ -916,6 +1194,7 @@ func TestBuildConfiguration(t *testing.T) { }, }, }, + Port: 80, }, }, SSLServers: []VirtualServer{}, @@ -959,6 +1238,7 @@ func TestBuildConfiguration(t *testing.T) { HTTPServers: []VirtualServer{ { IsDefault: true, + Port: 80, }, { Hostname: "foo.example.com", @@ -976,11 +1256,13 @@ func TestBuildConfiguration(t *testing.T) { }, }, }, + Port: 80, }, }, SSLServers: []VirtualServer{ { IsDefault: true, + Port: 443, }, { Hostname: "foo.example.com", @@ -1001,10 +1283,12 @@ func TestBuildConfiguration(t *testing.T) { }, }, }, + Port: 443, }, { Hostname: wildcardHostname, SSL: &SSL{CertificatePath: secretPath}, + Port: 443, }, }, Upstreams: []Upstream{fooUpstream}, @@ -1041,6 +1325,7 @@ func TestBuildConfiguration(t *testing.T) { HTTPServers: []VirtualServer{ { IsDefault: true, + Port: 80, }, { Hostname: "foo.example.com", @@ -1070,6 +1355,7 @@ func TestBuildConfiguration(t *testing.T) { }, }, }, + Port: 80, }, }, SSLServers: []VirtualServer{}, @@ -1114,6 +1400,7 @@ func TestBuildConfiguration(t *testing.T) { SSLServers: []VirtualServer{ { IsDefault: true, + Port: 443, }, { Hostname: "example.com", @@ -1141,10 +1428,12 @@ func TestBuildConfiguration(t *testing.T) { SSL: &SSL{ CertificatePath: "secret-path-https-listener-2", }, + Port: 443, }, { Hostname: wildcardHostname, SSL: &SSL{CertificatePath: secretPath}, + Port: 443, }, }, Upstreams: []Upstream{fooUpstream}, @@ -1156,19 +1445,14 @@ func TestBuildConfiguration(t *testing.T) { for _, test := range tests { t.Run(test.msg, func(t *testing.T) { - result := BuildConfiguration(context.TODO(), test.graph, fakeResolver) - - sort.Slice(result.BackendGroups, func(i, j int) bool { - return result.BackendGroups[i].Name() < result.BackendGroups[j].Name() - }) + g := NewGomegaWithT(t) - sort.Slice(result.Upstreams, func(i, j int) bool { - return result.Upstreams[i].Name < result.Upstreams[j].Name - }) + result := BuildConfiguration(context.TODO(), test.graph, fakeResolver) - if diff := cmp.Diff(test.expConf, result); diff != "" { - t.Errorf("BuildConfiguration() %q mismatch for configuration (-want +got):\n%s", test.msg, diff) - } + g.Expect(result.BackendGroups).To(ConsistOf(test.expConf.BackendGroups)) + g.Expect(result.Upstreams).To(ConsistOf(test.expConf.Upstreams)) + g.Expect(result.HTTPServers).To(ConsistOf(test.expConf.HTTPServers)) + g.Expect(result.SSLServers).To(ConsistOf(test.expConf.SSLServers)) }) } } diff --git a/internal/state/graph/gateway_listener.go b/internal/state/graph/gateway_listener.go index cd1cd8ddec..7c0172172c 100644 --- a/internal/state/graph/gateway_listener.go +++ b/internal/state/graph/gateway_listener.go @@ -1,6 +1,7 @@ package graph import ( + "errors" "fmt" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -67,6 +68,8 @@ func newListenerConfiguratorFactory( gw *v1beta1.Gateway, secretMemoryMgr secrets.SecretDiskMemoryManager, ) *listenerConfiguratorFactory { + sharedPortConflictResolver := createPortConflictResolver() + return &listenerConfiguratorFactory{ unsupportedProtocol: &listenerConfigurator{ validators: []listenerValidator{ @@ -88,7 +91,7 @@ func newListenerConfiguratorFactory( validateHTTPListener, }, conflictResolvers: []listenerConflictResolver{ - createHostnameConflictResolver(), + sharedPortConflictResolver, }, }, https: &listenerConfigurator{ @@ -99,7 +102,7 @@ func newListenerConfiguratorFactory( createHTTPSListenerValidator(gw.Namespace), }, conflictResolvers: []listenerConflictResolver{ - createHostnameConflictResolver(), + sharedPortConflictResolver, }, externalReferenceResolvers: []listenerExternalReferenceResolver{ createExternalReferencesForTLSSecretsResolver(gw.Namespace, secretMemoryMgr), @@ -241,10 +244,10 @@ func validateListenerLabelSelector(listener v1beta1.Listener) []conditions.Condi } func validateHTTPListener(listener v1beta1.Listener) []conditions.Condition { - if listener.Port != 80 { + if err := validateListenerPort(listener.Port); err != nil { path := field.NewPath("port") - valErr := field.NotSupported(path, listener.Port, []string{"80"}) - return []conditions.Condition{conditions.NewListenerPortUnavailable(valErr.Error())} + valErr := field.Invalid(path, listener.Port, err.Error()) + return []conditions.Condition{conditions.NewListenerUnsupportedValue(valErr.Error())} } if listener.TLS != nil { @@ -254,14 +257,22 @@ func validateHTTPListener(listener v1beta1.Listener) []conditions.Condition { return nil } +func validateListenerPort(port v1beta1.PortNumber) error { + if port < 1 || port > 65535 { + return errors.New("port must be between 1-65535") + } + + return nil +} + func createHTTPSListenerValidator(gwNsName string) listenerValidator { return func(listener v1beta1.Listener) []conditions.Condition { var conds []conditions.Condition - if listener.Port != 443 { + if err := validateListenerPort(listener.Port); err != nil { path := field.NewPath("port") - valErr := field.NotSupported(path, listener.Port, []string{"443"}) - conds = append(conds, conditions.NewListenerPortUnavailable(valErr.Error())) + valErr := field.Invalid(path, listener.Port, err.Error()) + conds = append(conds, conditions.NewListenerUnsupportedValue(valErr.Error())) } if listener.TLS == nil { @@ -324,28 +335,47 @@ func createHTTPSListenerValidator(gwNsName string) listenerValidator { } } -func createHostnameConflictResolver() listenerConflictResolver { - usedHostnames := make(map[string]*Listener) +func createPortConflictResolver() listenerConflictResolver { + conflictedPorts := make(map[v1beta1.PortNumber]bool) + portProtocolOwner := make(map[v1beta1.PortNumber]v1beta1.ProtocolType) + listenersByPort := make(map[v1beta1.PortNumber][]*Listener) + + format := "Multiple listeners for the same port %d specify incompatible protocols; " + + "ensure only one protocol per port" return func(l *Listener) { - h := getHostname(l.Source.Hostname) + port := l.Source.Port - if holder, exist := usedHostnames[h]; exist { + // if port is in map of conflictedPorts then we only need to set the current listener to invalid + if conflictedPorts[port] { l.Valid = false - holder.Valid = false // all listeners for the same hostname become conflicted + conflictedConds := conditions.NewListenerProtocolConflict(fmt.Sprintf(format, port)) + l.Conditions = append(l.Conditions, conflictedConds...) + return + } - format := "Multiple listeners for the same port use the same hostname %q; " + - "ensure only one listener uses that hostname" - conflictedConds := conditions.NewListenerConflictedHostname(fmt.Sprintf(format, h)) + // otherwise, we add the listener to the list of listeners for this port + // and then check if the protocol owner for the port is different from the current listener's protocol. - holder.Conditions = append(holder.Conditions, conflictedConds...) - l.Conditions = append(l.Conditions, conflictedConds...) + listenersByPort[port] = append(listenersByPort[port], l) + protocol, ok := portProtocolOwner[port] + if !ok { + portProtocolOwner[port] = l.Source.Protocol return } - usedHostnames[h] = l + // if protocol owner doesn't match the listener's protocol we mark the port as conflicted, + // and invalidate all listeners we've seen for this port. + if protocol != l.Source.Protocol { + conflictedPorts[port] = true + for _, l := range listenersByPort[port] { + l.Valid = false + conflictedConds := conditions.NewListenerProtocolConflict(fmt.Sprintf(format, port)) + l.Conditions = append(l.Conditions, conflictedConds...) + } + } } } @@ -376,7 +406,8 @@ func createExternalReferencesForTLSSecretsResolver( // GetAllowedRouteLabelSelector returns a listener's AllowedRoutes label selector if it exists. func GetAllowedRouteLabelSelector(l v1beta1.Listener) *metav1.LabelSelector { if l.AllowedRoutes != nil && l.AllowedRoutes.Namespaces != nil { - if *l.AllowedRoutes.Namespaces.From == v1beta1.NamespacesFromSelector && l.AllowedRoutes.Namespaces.Selector != nil { + if *l.AllowedRoutes.Namespaces.From == v1beta1.NamespacesFromSelector && + l.AllowedRoutes.Namespaces.Selector != nil { return l.AllowedRoutes.Namespaces.Selector } } diff --git a/internal/state/graph/gateway_listener_test.go b/internal/state/graph/gateway_listener_test.go index 3652ead5ff..f8a9ff3a37 100644 --- a/internal/state/graph/gateway_listener_test.go +++ b/internal/state/graph/gateway_listener_test.go @@ -1,6 +1,7 @@ package graph import ( + "fmt" "testing" . "github.com/onsi/gomega" @@ -26,10 +27,10 @@ func TestValidateHTTPListener(t *testing.T) { }, { l: v1beta1.Listener{ - Port: 81, + Port: 0, }, expected: []conditions.Condition{ - conditions.NewListenerPortUnavailable(`port: Unsupported value: 81: supported values: "80"`), + conditions.NewListenerUnsupportedValue(`port: Invalid value: 0: port must be between 1-65535`), }, name: "invalid port", }, @@ -91,14 +92,14 @@ func TestValidateHTTPSListener(t *testing.T) { }, { l: v1beta1.Listener{ - Port: 80, + Port: 0, TLS: &v1beta1.GatewayTLSConfig{ Mode: helpers.GetTLSModePointer(v1beta1.TLSModeTerminate), CertificateRefs: []v1beta1.SecretObjectReference{validSecretRef}, }, }, expected: []conditions.Condition{ - conditions.NewListenerPortUnavailable(`port: Unsupported value: 80: supported values: "443"`), + conditions.NewListenerUnsupportedValue(`port: Invalid value: 0: port must be between 1-65535`), }, name: "invalid port", }, @@ -364,3 +365,22 @@ func TestValidateListenerLabelSelector(t *testing.T) { }) } } + +func TestValidateListenerPort(t *testing.T) { + validPorts := []v1beta1.PortNumber{1, 80, 443, 1000, 50000, 65535} + invalidPorts := []v1beta1.PortNumber{-1, 0, 65536, 80000} + + for _, p := range validPorts { + t.Run(fmt.Sprintf("valid port %d", p), func(t *testing.T) { + g := NewWithT(t) + g.Expect(validateListenerPort(p)).To(Succeed()) + }) + } + + for _, p := range invalidPorts { + t.Run(fmt.Sprintf("invalid port %d", p), func(t *testing.T) { + g := NewWithT(t) + g.Expect(validateListenerPort(p)).ToNot(Succeed()) + }) + } +} diff --git a/internal/state/graph/gateway_test.go b/internal/state/graph/gateway_test.go index f3c59473ed..584309f7a6 100644 --- a/internal/state/graph/gateway_test.go +++ b/internal/state/graph/gateway_test.go @@ -143,41 +143,6 @@ func TestProcessGateways(t *testing.T) { func TestBuildGateway(t *testing.T) { const gcName = "my-gateway-class" - listener801 := v1beta1.Listener{ - Name: "listener-80-1", - Hostname: (*v1beta1.Hostname)(helpers.GetStringPointer("foo.example.com")), - Port: 80, - Protocol: v1beta1.HTTPProtocolType, - } - listener802 := v1beta1.Listener{ - Name: "listener-80-2", - Hostname: (*v1beta1.Hostname)(helpers.GetStringPointer("bar.example.com")), - Port: 80, - Protocol: v1beta1.TCPProtocolType, // invalid protocol - } - listener803 := v1beta1.Listener{ - Name: "listener-80-3", - Hostname: (*v1beta1.Hostname)(helpers.GetStringPointer("bar.example.com")), - Port: 80, - Protocol: v1beta1.HTTPProtocolType, - } - listener804 := v1beta1.Listener{ - Name: "listener-80-4", - Hostname: (*v1beta1.Hostname)(helpers.GetStringPointer("foo.example.com")), - Port: 80, - Protocol: v1beta1.HTTPProtocolType, - } - listener805 := v1beta1.Listener{ - Name: "listener-80-5", - Port: 81, // invalid port - Protocol: v1beta1.HTTPProtocolType, - } - listener806 := v1beta1.Listener{ - Name: "listener-80-6", - Hostname: (*v1beta1.Hostname)(helpers.GetStringPointer("$example.com")), // invalid hostname - Port: 80, - Protocol: v1beta1.HTTPProtocolType, - } labelSet := map[string]string{ "key": "value", } @@ -225,58 +190,74 @@ func TestBuildGateway(t *testing.T) { }, }, } - // https listeners - listener4431 := v1beta1.Listener{ - Name: "listener-443-1", - Hostname: (*v1beta1.Hostname)(helpers.GetStringPointer("foo.example.com")), - Port: 443, - TLS: gatewayTLSConfig, - Protocol: v1beta1.HTTPSProtocolType, - } - listener4432 := v1beta1.Listener{ - Name: "listener-443-2", - Hostname: (*v1beta1.Hostname)(helpers.GetStringPointer("bar.example.com")), - Port: 443, - TLS: gatewayTLSConfig, - Protocol: v1beta1.HTTPSProtocolType, - } - listener4433 := v1beta1.Listener{ - Name: "listener-443-3", - Hostname: (*v1beta1.Hostname)(helpers.GetStringPointer("foo.example.com")), - Port: 443, - TLS: gatewayTLSConfig, - Protocol: v1beta1.HTTPSProtocolType, + + createListener := func( + name string, + hostname string, + port int, + protocol v1beta1.ProtocolType, + tls *v1beta1.GatewayTLSConfig, + ) v1beta1.Listener { + return v1beta1.Listener{ + Name: v1beta1.SectionName(name), + Hostname: (*v1beta1.Hostname)(helpers.GetStringPointer(hostname)), + Port: v1beta1.PortNumber(port), + Protocol: protocol, + TLS: tls, + } } - listener4434 := v1beta1.Listener{ - Name: "listener-443-4", - Hostname: (*v1beta1.Hostname)(helpers.GetStringPointer("$example.com")), // invalid hostname - Port: 443, - TLS: gatewayTLSConfig, - Protocol: v1beta1.HTTPSProtocolType, + createHTTPListener := func(name, hostname string, port int) v1beta1.Listener { + return createListener(name, hostname, port, v1beta1.HTTPProtocolType, nil) } - listener4435 := v1beta1.Listener{ - Name: "listener-443-5", - Hostname: (*v1beta1.Hostname)(helpers.GetStringPointer("foo.example.com")), - Port: 443, - TLS: tlsConfigInvalidSecret, // invalid https listener; secret does not exist - Protocol: v1beta1.HTTPSProtocolType, + createTCPListener := func(name, hostname string, port int) v1beta1.Listener { + return createListener(name, hostname, port, v1beta1.TCPProtocolType, nil) } - listener4436 := v1beta1.Listener{ - Name: "listener-443-6", - Hostname: (*v1beta1.Hostname)(helpers.GetStringPointer("foo.example.com")), - Port: 444, // invalid port - TLS: gatewayTLSConfig, - Protocol: v1beta1.HTTPSProtocolType, + createHTTPSListener := func(name, hostname string, port int, tls *v1beta1.GatewayTLSConfig) v1beta1.Listener { + return createListener(name, hostname, port, v1beta1.HTTPSProtocolType, tls) } + // foo http listeners + foo80Listener1 := createHTTPListener("foo-80-1", "foo.example.com", 80) + foo8080Listener := createHTTPListener("foo-8080", "foo.example.com", 8080) + foo8081Listener := createHTTPListener("foo-8081", "foo.example.com", 8081) + foo443Listener := createHTTPListener("foo-443", "foo.example.com", 443) + + // foo https listeners + foo80HTTPSListener := createHTTPSListener("foo-80-https", "foo.example.com", 80, gatewayTLSConfig) + foo443HTTPSListener1 := createHTTPSListener("foo-443-https-1", "foo.example.com", 443, gatewayTLSConfig) + foo8443HTTPSListener := createHTTPSListener("foo-8443-https", "foo.example.com", 8443, gatewayTLSConfig) + + // bar http listener + bar80Listener := createHTTPListener("bar-80", "bar.example.com", 80) + + // bar https listeners + bar443HTTPSListener := createHTTPSListener("bar-443-https", "bar.example.com", 443, gatewayTLSConfig) + bar8443HTTPSListener := createHTTPSListener("bar-8443-https", "bar.example.com", 8443, gatewayTLSConfig) + + // invalid listeners + invalidProtocolListener := createTCPListener("invalid-protocol", "bar.example.com", 80) + invalidPortListener := createHTTPListener("invalid-port", "invalid-port", 0) + invalidHostnameListener := createHTTPListener("invalid-hostname", "$example.com", 80) + invalidHTTPSHostnameListener := createHTTPSListener("invalid-https-hostname", "$example.com", 443, gatewayTLSConfig) + invalidTLSConfigListener := createHTTPSListener( + "invalid-tls-config", + "foo.example.com", + 443, + tlsConfigInvalidSecret, + ) + invalidHTTPSPortListener := createHTTPSListener("invalid-https-port", "foo.example.com", 0, gatewayTLSConfig) + const ( invalidHostnameMsg = `hostname: Invalid value: "$example.com": a lowercase RFC 1123 subdomain ` + "must consist of lower case alphanumeric characters, '-' or '.', and must start and end " + "with an alphanumeric character (e.g. 'example.com', regex used for validation is " + `'[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')` - conflictedHostnamesMsg = `Multiple listeners for the same port use the same hostname "foo.example.com"; ` + - "ensure only one listener uses that hostname" + conflict80PortMsg = "Multiple listeners for the same port 80 specify incompatible protocols; " + + "ensure only one protocol per port" + + conflict443PortMsg = "Multiple listeners for the same port 443 specify incompatible protocols; " + + "ensure only one protocol per port" secretPath = "/etc/nginx/secrets/test_secret" ) @@ -318,29 +299,42 @@ func TestBuildGateway(t *testing.T) { name string }{ { - gateway: createGateway(gatewayCfg{listeners: []v1beta1.Listener{listener801}}), + gateway: createGateway(gatewayCfg{listeners: []v1beta1.Listener{foo80Listener1, foo8080Listener}}), gatewayClass: validGC, expected: &Gateway{ Source: getLastCreatedGetaway(), Listeners: map[string]*Listener{ - "listener-80-1": { - Source: listener801, + "foo-80-1": { + Source: foo80Listener1, + Valid: true, + Routes: map[types.NamespacedName]*Route{}, + }, + "foo-8080": { + Source: foo8080Listener, Valid: true, Routes: map[types.NamespacedName]*Route{}, }, }, Valid: true, }, - name: "valid http listener", + name: "valid http listeners", }, { - gateway: createGateway(gatewayCfg{listeners: []v1beta1.Listener{listener4431}}), + gateway: createGateway( + gatewayCfg{listeners: []v1beta1.Listener{foo443HTTPSListener1, foo8443HTTPSListener}}, + ), gatewayClass: validGC, expected: &Gateway{ Source: getLastCreatedGetaway(), Listeners: map[string]*Listener{ - "listener-443-1": { - Source: listener4431, + "foo-443-https-1": { + Source: foo443HTTPSListener1, + Valid: true, + Routes: map[types.NamespacedName]*Route{}, + SecretPath: secretPath, + }, + "foo-8443-https": { + Source: foo8443HTTPSListener, Valid: true, Routes: map[types.NamespacedName]*Route{}, SecretPath: secretPath, @@ -348,7 +342,7 @@ func TestBuildGateway(t *testing.T) { }, Valid: true, }, - name: "valid https listener", + name: "valid https listeners", }, { gateway: createGateway(gatewayCfg{listeners: []v1beta1.Listener{listenerAllowedRoutes}}), @@ -388,13 +382,13 @@ func TestBuildGateway(t *testing.T) { name: "http listener with invalid label selector", }, { - gateway: createGateway(gatewayCfg{listeners: []v1beta1.Listener{listener802}}), + gateway: createGateway(gatewayCfg{listeners: []v1beta1.Listener{invalidProtocolListener}}), gatewayClass: validGC, expected: &Gateway{ Source: getLastCreatedGetaway(), Listeners: map[string]*Listener{ - "listener-80-2": { - Source: listener802, + "invalid-protocol": { + Source: invalidProtocolListener, Valid: false, Conditions: []conditions.Condition{ conditions.NewListenerUnsupportedProtocol( @@ -408,60 +402,53 @@ func TestBuildGateway(t *testing.T) { name: "invalid listener protocol", }, { - gateway: createGateway(gatewayCfg{listeners: []v1beta1.Listener{listener805}}), + gateway: createGateway( + gatewayCfg{listeners: []v1beta1.Listener{invalidPortListener, invalidHTTPSPortListener}}, + ), gatewayClass: validGC, expected: &Gateway{ Source: getLastCreatedGetaway(), Listeners: map[string]*Listener{ - "listener-80-5": { - Source: listener805, + "invalid-port": { + Source: invalidPortListener, Valid: false, Conditions: []conditions.Condition{ - conditions.NewListenerPortUnavailable( - `port: Unsupported value: 81: supported values: "80"`, + conditions.NewListenerUnsupportedValue( + `port: Invalid value: 0: port must be between 1-65535`, ), }, }, - }, - Valid: true, - }, - name: "invalid http listener", - }, - { - gateway: createGateway(gatewayCfg{listeners: []v1beta1.Listener{listener4436}}), - gatewayClass: validGC, - expected: &Gateway{ - Source: getLastCreatedGetaway(), - Listeners: map[string]*Listener{ - "listener-443-6": { - Source: listener4436, + "invalid-https-port": { + Source: invalidHTTPSPortListener, Valid: false, Conditions: []conditions.Condition{ - conditions.NewListenerPortUnavailable( - `port: Unsupported value: 444: supported values: "443"`, + conditions.NewListenerUnsupportedValue( + `port: Invalid value: 0: port must be between 1-65535`, ), }, }, }, Valid: true, }, - name: "invalid https listener", + name: "invalid ports", }, { - gateway: createGateway(gatewayCfg{listeners: []v1beta1.Listener{listener806, listener4434}}), + gateway: createGateway( + gatewayCfg{listeners: []v1beta1.Listener{invalidHostnameListener, invalidHTTPSHostnameListener}}, + ), gatewayClass: validGC, expected: &Gateway{ Source: getLastCreatedGetaway(), Listeners: map[string]*Listener{ - "listener-80-6": { - Source: listener806, + "invalid-hostname": { + Source: invalidHostnameListener, Valid: false, Conditions: []conditions.Condition{ conditions.NewListenerUnsupportedValue(invalidHostnameMsg), }, }, - "listener-443-4": { - Source: listener4434, + "invalid-https-hostname": { + Source: invalidHTTPSHostnameListener, Valid: false, Conditions: []conditions.Condition{ conditions.NewListenerUnsupportedValue(invalidHostnameMsg), @@ -473,13 +460,13 @@ func TestBuildGateway(t *testing.T) { name: "invalid hostnames", }, { - gateway: createGateway(gatewayCfg{listeners: []v1beta1.Listener{listener4435}}), + gateway: createGateway(gatewayCfg{listeners: []v1beta1.Listener{invalidTLSConfigListener}}), gatewayClass: validGC, expected: &Gateway{ Source: getLastCreatedGetaway(), Listeners: map[string]*Listener{ - "listener-443-5": { - Source: listener4435, + "invalid-tls-config": { + Source: invalidTLSConfigListener, Valid: false, Routes: map[types.NamespacedName]*Route{}, Conditions: conditions.NewListenerInvalidCertificateRef( @@ -493,30 +480,63 @@ func TestBuildGateway(t *testing.T) { }, { gateway: createGateway( - gatewayCfg{listeners: []v1beta1.Listener{listener801, listener803, listener4431, listener4432}}, + gatewayCfg{ + listeners: []v1beta1.Listener{ + foo80Listener1, + foo8080Listener, + foo8081Listener, + foo443HTTPSListener1, + foo8443HTTPSListener, + bar80Listener, + bar443HTTPSListener, + bar8443HTTPSListener, + }, + }, ), gatewayClass: validGC, expected: &Gateway{ Source: getLastCreatedGetaway(), Listeners: map[string]*Listener{ - "listener-80-1": { - Source: listener801, + "foo-80-1": { + Source: foo80Listener1, + Valid: true, + Routes: map[types.NamespacedName]*Route{}, + }, + "foo-8080": { + Source: foo8080Listener, Valid: true, Routes: map[types.NamespacedName]*Route{}, }, - "listener-80-3": { - Source: listener803, + "foo-8081": { + Source: foo8081Listener, Valid: true, Routes: map[types.NamespacedName]*Route{}, }, - "listener-443-1": { - Source: listener4431, + "bar-80": { + Source: bar80Listener, + Valid: true, + Routes: map[types.NamespacedName]*Route{}, + }, + "foo-443-https-1": { + Source: foo443HTTPSListener1, + Valid: true, + Routes: map[types.NamespacedName]*Route{}, + SecretPath: secretPath, + }, + "foo-8443-https": { + Source: foo8443HTTPSListener, Valid: true, Routes: map[types.NamespacedName]*Route{}, SecretPath: secretPath, }, - "listener-443-2": { - Source: listener4432, + "bar-443-https": { + Source: bar443HTTPSListener, + Valid: true, + Routes: map[types.NamespacedName]*Route{}, + SecretPath: secretPath, + }, + "bar-8443-https": { + Source: bar8443HTTPSListener, Valid: true, Routes: map[types.NamespacedName]*Route{}, SecretPath: secretPath, @@ -528,55 +548,79 @@ func TestBuildGateway(t *testing.T) { }, { gateway: createGateway( - gatewayCfg{listeners: []v1beta1.Listener{listener801, listener804, listener4431, listener4433}}, + gatewayCfg{ + listeners: []v1beta1.Listener{ + foo80Listener1, + bar80Listener, + foo443Listener, + foo80HTTPSListener, + foo443HTTPSListener1, + bar443HTTPSListener, + }, + }, ), gatewayClass: validGC, expected: &Gateway{ Source: getLastCreatedGetaway(), Listeners: map[string]*Listener{ - "listener-80-1": { - Source: listener801, + "foo-80-1": { + Source: foo80Listener1, + Valid: false, + Routes: map[types.NamespacedName]*Route{}, + Conditions: conditions.NewListenerProtocolConflict(conflict80PortMsg), + }, + "bar-80": { + Source: bar80Listener, + Valid: false, + Routes: map[types.NamespacedName]*Route{}, + Conditions: conditions.NewListenerProtocolConflict(conflict80PortMsg), + }, + "foo-443": { + Source: foo443Listener, Valid: false, Routes: map[types.NamespacedName]*Route{}, - Conditions: conditions.NewListenerConflictedHostname(conflictedHostnamesMsg), + Conditions: conditions.NewListenerProtocolConflict(conflict443PortMsg), }, - "listener-80-4": { - Source: listener804, + "foo-80-https": { + Source: foo80HTTPSListener, Valid: false, Routes: map[types.NamespacedName]*Route{}, - Conditions: conditions.NewListenerConflictedHostname(conflictedHostnamesMsg), + Conditions: conditions.NewListenerProtocolConflict(conflict80PortMsg), + SecretPath: "/etc/nginx/secrets/test_secret", }, - "listener-443-1": { - Source: listener4431, + "foo-443-https-1": { + Source: foo443HTTPSListener1, Valid: false, Routes: map[types.NamespacedName]*Route{}, - Conditions: conditions.NewListenerConflictedHostname(conflictedHostnamesMsg), + Conditions: conditions.NewListenerProtocolConflict(conflict443PortMsg), SecretPath: "/etc/nginx/secrets/test_secret", }, - "listener-443-3": { - Source: listener4433, + "bar-443-https": { + Source: bar443HTTPSListener, Valid: false, Routes: map[types.NamespacedName]*Route{}, - Conditions: conditions.NewListenerConflictedHostname(conflictedHostnamesMsg), + Conditions: conditions.NewListenerProtocolConflict(conflict443PortMsg), SecretPath: "/etc/nginx/secrets/test_secret", }, }, Valid: true, }, - name: "collisions", + name: "port/protocol collisions", }, { - gateway: createGateway(gatewayCfg{ - listeners: []v1beta1.Listener{listener801, listener4431}, - addresses: []v1beta1.GatewayAddress{ - {}, + gateway: createGateway( + gatewayCfg{ + listeners: []v1beta1.Listener{foo80Listener1, foo443HTTPSListener1}, + addresses: []v1beta1.GatewayAddress{{}}, }, - }), + ), gatewayClass: validGC, expected: &Gateway{ - Source: getLastCreatedGetaway(), - Valid: false, - Conditions: conditions.NewGatewayUnsupportedValue("spec.addresses: Forbidden: addresses are not supported"), + Source: getLastCreatedGetaway(), + Valid: false, + Conditions: conditions.NewGatewayUnsupportedValue("spec." + + "addresses: Forbidden: addresses are not supported", + ), }, name: "gateway addresses are not supported", }, @@ -586,7 +630,9 @@ func TestBuildGateway(t *testing.T) { name: "nil gateway", }, { - gateway: createGateway(gatewayCfg{listeners: []v1beta1.Listener{listener801, listener802}}), + gateway: createGateway( + gatewayCfg{listeners: []v1beta1.Listener{foo80Listener1, invalidProtocolListener}}, + ), gatewayClass: invalidGC, expected: &Gateway{ Source: getLastCreatedGetaway(), @@ -596,7 +642,9 @@ func TestBuildGateway(t *testing.T) { name: "invalid gatewayclass", }, { - gateway: createGateway(gatewayCfg{listeners: []v1beta1.Listener{listener801, listener802}}), + gateway: createGateway( + gatewayCfg{listeners: []v1beta1.Listener{foo80Listener1, invalidProtocolListener}}, + ), gatewayClass: nil, expected: &Gateway{ Source: getLastCreatedGetaway(), diff --git a/internal/state/graph/httproute_test.go b/internal/state/graph/httproute_test.go index caea6a9d7b..f7f96271b6 100644 --- a/internal/state/graph/httproute_test.go +++ b/internal/state/graph/httproute_test.go @@ -758,10 +758,8 @@ func TestBindRouteToListeners(t *testing.T) { Source: gw, Valid: true, Listeners: map[string]*Listener{ - "listener-80-1": createListener("listener-80-1"), - "listener-80-2": createModifiedListener("listener-80-2", func(l *Listener) { - l.Source.Hostname = nil - }), + "listener-80": createListener("listener-80"), + "listener-8080": createListener("listener-8080"), }, }, expectedSectionNameRefs: []ParentRef{ @@ -771,26 +769,25 @@ func TestBindRouteToListeners(t *testing.T) { Attachment: &ParentRefAttachmentStatus{ Attached: true, AcceptedHostnames: map[string][]string{ - "listener-80-1": {"foo.example.com"}, - "listener-80-2": {"foo.example.com"}, + "listener-80": {"foo.example.com"}, + "listener-8080": {"foo.example.com"}, }, }, }, }, expectedGatewayListeners: map[string]*Listener{ - "listener-80-1": createModifiedListener("listener-80-1", func(l *Listener) { + "listener-80": createModifiedListener("listener-80", func(l *Listener) { l.Routes = map[types.NamespacedName]*Route{ client.ObjectKeyFromObject(hr): routeWithEmptySectionName, } }), - "listener-80-2": createModifiedListener("listener-80-2", func(l *Listener) { + "listener-8080": createModifiedListener("listener-8080", func(l *Listener) { l.Routes = map[types.NamespacedName]*Route{ client.ObjectKeyFromObject(hr): routeWithEmptySectionName, } - l.Source.Hostname = nil }), }, - name: "section name is empty", + name: "section name is empty; bind to multiple listeners", }, { route: routeWithEmptySectionName, diff --git a/internal/status/statuses_test.go b/internal/status/statuses_test.go index f15edbe582..b9e1427e98 100644 --- a/internal/status/statuses_test.go +++ b/internal/status/statuses_test.go @@ -405,7 +405,7 @@ func TestBuildGatewayStatuses(t *testing.T) { "listener-invalid-1": { Valid: false, Conditions: []conditions.Condition{ - conditions.NewListenerPortUnavailable("port unavailable"), + conditions.NewListenerUnsupportedProtocol("unsupported protocol"), }, }, "listener-invalid-2": { @@ -423,7 +423,7 @@ func TestBuildGatewayStatuses(t *testing.T) { ListenerStatuses: map[string]ListenerStatus{ "listener-invalid-1": { Conditions: []conditions.Condition{ - conditions.NewListenerPortUnavailable("port unavailable"), + conditions.NewListenerUnsupportedProtocol("unsupported protocol"), }, }, "listener-invalid-2": {