From d58e9f00cc40a94b92ad4620f7032becb3f90606 Mon Sep 17 00:00:00 2001 From: Sarthak Agrawal Date: Mon, 8 Jul 2024 14:12:24 -0600 Subject: [PATCH] add graph to dataplane conversion Problem: Graph did not support TLS Route to dataplane configuration Solution: Added the code to convert it --- .github/workflows/conformance.yml | 1 + .../templates/clusterrole.yaml | 2 + deploy/experimental-nginx-plus/deploy.yaml | 2 + deploy/experimental/deploy.yaml | 2 + internal/framework/gatewayclass/validate.go | 1 + internal/framework/kinds/kinds.go | 2 + internal/mode/static/handler.go | 1 + internal/mode/static/manager.go | 9 + internal/mode/static/manager_test.go | 2 + .../static/nginx/config/generator_test.go | 10 +- .../mode/static/nginx/config/http/config.go | 11 +- internal/mode/static/nginx/config/maps.go | 57 +- .../mode/static/nginx/config/maps_template.go | 3 +- .../mode/static/nginx/config/maps_test.go | 72 ++- internal/mode/static/nginx/config/servers.go | 15 +- .../static/nginx/config/servers_template.go | 9 +- .../mode/static/nginx/config/servers_test.go | 11 +- .../mode/static/nginx/config/shared/config.go | 6 + .../mode/static/nginx/config/stream/config.go | 9 + .../static/nginx/config/stream_servers.go | 25 +- .../nginx/config/stream_servers_template.go | 23 +- .../nginx/config/stream_servers_test.go | 44 ++ .../mode/static/nginx/config/upstreams.go | 6 +- .../static/nginx/config/upstreams_test.go | 7 + .../mode/static/state/change_processor.go | 7 + .../static/state/change_processor_test.go | 4 + .../static/state/conditions/conditions.go | 35 ++ .../static/state/dataplane/configuration.go | 156 +++++- .../state/dataplane/configuration_test.go | 378 ++++++++++++- internal/mode/static/state/dataplane/types.go | 2 + .../mode/static/state/graph/backend_refs.go | 2 +- .../static/state/graph/gateway_listener.go | 131 ++++- .../state/graph/gateway_listener_test.go | 36 ++ .../mode/static/state/graph/gateway_test.go | 311 +++++++---- internal/mode/static/state/graph/graph.go | 15 +- .../mode/static/state/graph/graph_test.go | 177 +++++- .../mode/static/state/graph/route_common.go | 293 +++++++++- .../static/state/graph/route_common_test.go | 502 +++++++++++++++++- internal/mode/static/state/graph/service.go | 53 +- .../mode/static/state/graph/service_test.go | 340 ++++++------ internal/mode/static/state/graph/tlsroute.go | 133 +++++ .../mode/static/state/graph/tlsroute_test.go | 468 ++++++++++++++++ .../mode/static/status/prepare_requests.go | 26 +- .../static/status/prepare_requests_test.go | 95 +++- internal/mode/static/status/status_setters.go | 21 + .../mode/static/status/status_setters_test.go | 175 ++++++ tests/Makefile | 7 +- tests/conformance/conformance-rbac.yaml | 1 + tests/conformance/conformance_test.go | 6 +- 49 files changed, 3301 insertions(+), 403 deletions(-) create mode 100644 internal/mode/static/state/graph/tlsroute.go create mode 100644 internal/mode/static/state/graph/tlsroute_test.go diff --git a/.github/workflows/conformance.yml b/.github/workflows/conformance.yml index 65ed5cb920..d07b205cad 100644 --- a/.github/workflows/conformance.yml +++ b/.github/workflows/conformance.yml @@ -75,6 +75,7 @@ jobs: run: | ngf_prefix=ghcr.io/nginxinc/nginx-gateway-fabric ngf_tag=${{ steps.ngf-meta.outputs.version }} + if [ ${{ inputs.enable-experimental }} == "true" ]; then export ENABLE_EXPERIMENTAL=true; fi make generate-static-deployment PLUS_ENABLED=${{ inputs.image == 'plus' && 'true' || 'false' }} PREFIX=${ngf_prefix} TAG=${ngf_tag} working-directory: ./tests diff --git a/charts/nginx-gateway-fabric/templates/clusterrole.yaml b/charts/nginx-gateway-fabric/templates/clusterrole.yaml index 01785c2829..65c184ef47 100644 --- a/charts/nginx-gateway-fabric/templates/clusterrole.yaml +++ b/charts/nginx-gateway-fabric/templates/clusterrole.yaml @@ -72,6 +72,7 @@ rules: - grpcroutes {{- if .Values.nginxGateway.gwAPIExperimentalFeatures.enable }} - backendtlspolicies + - tlsroutes {{- end }} verbs: - list @@ -85,6 +86,7 @@ rules: - grpcroutes/status {{- if .Values.nginxGateway.gwAPIExperimentalFeatures.enable }} - backendtlspolicies/status + - tlsroutes/status {{- end }} verbs: - update diff --git a/deploy/experimental-nginx-plus/deploy.yaml b/deploy/experimental-nginx-plus/deploy.yaml index e6cb4a795e..ed9c748e1a 100644 --- a/deploy/experimental-nginx-plus/deploy.yaml +++ b/deploy/experimental-nginx-plus/deploy.yaml @@ -82,6 +82,7 @@ rules: - referencegrants - grpcroutes - backendtlspolicies + - tlsroutes verbs: - list - watch @@ -93,6 +94,7 @@ rules: - gatewayclasses/status - grpcroutes/status - backendtlspolicies/status + - tlsroutes/status verbs: - update - apiGroups: diff --git a/deploy/experimental/deploy.yaml b/deploy/experimental/deploy.yaml index 40c7ad96f6..28cc7b6d19 100644 --- a/deploy/experimental/deploy.yaml +++ b/deploy/experimental/deploy.yaml @@ -74,6 +74,7 @@ rules: - referencegrants - grpcroutes - backendtlspolicies + - tlsroutes verbs: - list - watch @@ -85,6 +86,7 @@ rules: - gatewayclasses/status - grpcroutes/status - backendtlspolicies/status + - tlsroutes/status verbs: - update - apiGroups: diff --git a/internal/framework/gatewayclass/validate.go b/internal/framework/gatewayclass/validate.go index 4c60599a5f..14bef98288 100644 --- a/internal/framework/gatewayclass/validate.go +++ b/internal/framework/gatewayclass/validate.go @@ -23,6 +23,7 @@ var gatewayCRDs = map[string]apiVersion{ "referencegrants.gateway.networking.k8s.io": {}, "backendtlspolicies.gateway.networking.k8s.io": {}, "grpcroutes.gateway.networking.k8s.io": {}, + "tlsroutes.gateway.networking.k8s.io": {}, } type apiVersion struct { diff --git a/internal/framework/kinds/kinds.go b/internal/framework/kinds/kinds.go index f5efa38187..471c526b98 100644 --- a/internal/framework/kinds/kinds.go +++ b/internal/framework/kinds/kinds.go @@ -19,6 +19,8 @@ const ( HTTPRoute = "HTTPRoute" // GRPCRoute is the GRPCRoute kind. GRPCRoute = "GRPCRoute" + // TLSRoute is the TLSRoute kind. + TLSRoute = "TLSRoute" ) // NGINX Gateway Fabric kinds. diff --git a/internal/mode/static/handler.go b/internal/mode/static/handler.go index d90edc337e..3e1f13f37f 100644 --- a/internal/mode/static/handler.go +++ b/internal/mode/static/handler.go @@ -249,6 +249,7 @@ func (h *eventHandlerImpl) updateStatuses(ctx context.Context, logger logr.Logge gcReqs = status.PrepareGatewayClassRequests(graph.GatewayClass, graph.IgnoredGatewayClasses, transitionTime) } routeReqs := status.PrepareRouteRequests( + graph.L4Routes, graph.Routes, transitionTime, h.latestReloadResult, diff --git a/internal/mode/static/manager.go b/internal/mode/static/manager.go index 30b71f7026..96174e4c96 100644 --- a/internal/mode/static/manager.go +++ b/internal/mode/static/manager.go @@ -31,6 +31,7 @@ import ( metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" k8spredicate "sigs.k8s.io/controller-runtime/pkg/predicate" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" gatewayv1alpha3 "sigs.k8s.io/gateway-api/apis/v1alpha3" gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" @@ -73,6 +74,7 @@ func init() { utilruntime.Must(gatewayv1beta1.Install(scheme)) utilruntime.Must(gatewayv1.Install(scheme)) utilruntime.Must(gatewayv1alpha3.Install(scheme)) + utilruntime.Must(gatewayv1alpha2.Install(scheme)) utilruntime.Must(apiv1.AddToScheme(scheme)) utilruntime.Must(discoveryV1.AddToScheme(scheme)) utilruntime.Must(ngfAPI.AddToScheme(scheme)) @@ -492,6 +494,12 @@ func registerControllers( // https://github.com/nginxinc/nginx-gateway-fabric/issues/1545 objectType: &apiv1.ConfigMap{}, }, + { + objectType: &gatewayv1alpha2.TLSRoute{}, + options: []controller.Option{ + controller.WithK8sPredicate(k8spredicate.GenerationChangedPredicate{}), + }, + }, } controllerRegCfgs = append(controllerRegCfgs, gwExpFeatures...) } @@ -666,6 +674,7 @@ func prepareFirstEventBatchPreparerArgs( objectLists, &gatewayv1alpha3.BackendTLSPolicyList{}, &apiv1.ConfigMapList{}, + &gatewayv1alpha2.TLSRouteList{}, ) } diff --git a/internal/mode/static/manager_test.go b/internal/mode/static/manager_test.go index 73cfa7be26..041f5062b6 100644 --- a/internal/mode/static/manager_test.go +++ b/internal/mode/static/manager_test.go @@ -13,6 +13,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" gatewayv1alpha3 "sigs.k8s.io/gateway-api/apis/v1alpha3" gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" @@ -105,6 +106,7 @@ func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) { &ngfAPI.NginxProxyList{}, partialObjectMetadataList, &gatewayv1alpha3.BackendTLSPolicyList{}, + &gatewayv1alpha2.TLSRouteList{}, &gatewayv1.GRPCRouteList{}, &ngfAPI.ClientSettingsPolicyList{}, &ngfAPI.ObservabilityPolicyList{}, diff --git a/internal/mode/static/nginx/config/generator_test.go b/internal/mode/static/nginx/config/generator_test.go index 67311f4223..f3eefa35f5 100644 --- a/internal/mode/static/nginx/config/generator_test.go +++ b/internal/mode/static/nginx/config/generator_test.go @@ -11,6 +11,7 @@ import ( "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/file" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/resolver" ) func TestGenerate(t *testing.T) { @@ -62,8 +63,13 @@ func TestGenerate(t *testing.T) { }, StreamUpstreams: []dataplane.Upstream{ { - Name: "stream_up", - Endpoints: nil, + Name: "stream_up", + Endpoints: []resolver.Endpoint{ + { + Address: "1.1.1.1", + Port: 80, + }, + }, }, }, BackendGroups: []dataplane.BackendGroup{bg}, diff --git a/internal/mode/static/nginx/config/http/config.go b/internal/mode/static/nginx/config/http/config.go index 4e9e4f2f85..ed29b7403d 100644 --- a/internal/mode/static/nginx/config/http/config.go +++ b/internal/mode/static/nginx/config/http/config.go @@ -1,5 +1,7 @@ package http +import "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/shared" + // Server holds all configuration for an HTTP server. type Server struct { SSL *SSL @@ -10,12 +12,7 @@ type Server struct { IsDefaultHTTP bool IsDefaultSSL bool GRPC bool -} - -// IPFamily holds the IP family configuration to be used by NGINX. -type IPFamily struct { - IPv4 bool - IPv6 bool + IsSocket bool } // Location holds all configuration for an HTTP location. @@ -103,5 +100,5 @@ type ProxySSLVerify struct { // ServerConfig holds configuration for an HTTP server and IP family to be used by NGINX. type ServerConfig struct { Servers []Server - IPFamily IPFamily + IPFamily shared.IPFamily } diff --git a/internal/mode/static/nginx/config/maps.go b/internal/mode/static/nginx/config/maps.go index 703b2b7b04..5ad8ac230e 100644 --- a/internal/mode/static/nginx/config/maps.go +++ b/internal/mode/static/nginx/config/maps.go @@ -11,12 +11,18 @@ import ( var mapsTemplate = gotemplate.Must(gotemplate.New("maps").Parse(mapsTemplateText)) -// emptyStringSocket is used when the stream server has an invalid upstream. In this case, we pass the connection -// to the empty socket so that NGINX will close the connection with an error in the error log -- -// no host in pass "" -- and set $status variable to 500 (logged by stream access log), -// which will indicate the problem to the user. -// https://nginx.org/en/docs/stream/ngx_stream_core_module.html#var_status -const emptyStringSocket = `""` +const ( + // emptyStringSocket is used when the stream server has an invalid upstream. In this case, we pass the connection + // to the empty socket so that NGINX will close the connection with an error in the error log -- + // no host in pass "" -- and set $status variable to 500 (logged by stream access log), + // which will indicate the problem to the user. + // https://nginx.org/en/docs/stream/ngx_stream_core_module.html#var_status + emptyStringSocket = `""` + + // connectionClosedStreamServerSocket is used when we want to listen on a port but have no service configured, + // so we pass to this server that just returns an empty string to tell users that we are listening. + connectionClosedStreamServerSocket = "unix:/var/run/nginx/connection-closed-server.sock" +) func executeMaps(conf dataplane.Configuration) []executeResult { maps := buildAddHeaderMaps(append(conf.HTTPServers, conf.SSLServers...)) @@ -44,32 +50,44 @@ func createStreamMaps(conf dataplane.Configuration) []shared.Map { return nil } portsToMap := make(map[int32]shared.Map) + portHasDefault := make(map[int32]struct{}) + upstreams := make(map[string]dataplane.Upstream) + + for _, u := range conf.StreamUpstreams { + upstreams[u.Name] = u + } for _, server := range conf.TLSPassthroughServers { streamMap, portInUse := portsToMap[server.Port] socket := emptyStringSocket - if server.UpstreamName != "" { + if u, ok := upstreams[server.UpstreamName]; ok && server.UpstreamName != "" && len(u.Endpoints) > 0 { socket = getSocketNameTLS(server.Port, server.Hostname) } + if server.IsDefault { + socket = connectionClosedStreamServerSocket + } + mapParam := shared.MapParameter{ Value: server.Hostname, Result: socket, } if !portInUse { - m := shared.Map{ - Source: "$ssl_preread_server_name", - Variable: getTLSPassthroughVarName(server.Port), - Parameters: []shared.MapParameter{ - mapParam, - }, + streamMap = shared.Map{ + Source: "$ssl_preread_server_name", + Variable: getTLSPassthroughVarName(server.Port), + Parameters: make([]shared.MapParameter, 0), UseHostnames: true, } - portsToMap[server.Port] = m - } else { + portsToMap[server.Port] = streamMap + } + + // If the hostname is empty, we don't want to add an entry to the map. This case occurs when + // the gateway listener hostname is not specified + if !(server.Hostname == "") { streamMap.Parameters = append(streamMap.Parameters, mapParam) portsToMap[server.Port] = streamMap } @@ -82,6 +100,7 @@ func createStreamMaps(conf dataplane.Configuration) []shared.Map { if server.IsDefault { hostname = "default" + portHasDefault[server.Port] = struct{}{} } if portInUse { @@ -95,7 +114,13 @@ func createStreamMaps(conf dataplane.Configuration) []shared.Map { maps := make([]shared.Map, 0, len(portsToMap)) - for _, m := range portsToMap { + for p, m := range portsToMap { + if _, ok := portHasDefault[p]; !ok { + m.Parameters = append(m.Parameters, shared.MapParameter{ + Value: "default", + Result: connectionClosedStreamServerSocket, + }) + } maps = append(maps, m) } diff --git a/internal/mode/static/nginx/config/maps_template.go b/internal/mode/static/nginx/config/maps_template.go index 00604410c1..4468b908c7 100644 --- a/internal/mode/static/nginx/config/maps_template.go +++ b/internal/mode/static/nginx/config/maps_template.go @@ -3,10 +3,9 @@ package config const mapsTemplateText = ` {{ range $m := . }} map {{ $m.Source }} {{ $m.Variable }} { - {{- if $m.UseHostnames -}} + {{- if $m.UseHostnames }} hostnames; {{ end }} - {{ range $p := $m.Parameters }} {{ $p.Value }} {{ $p.Result }}; {{ end }} diff --git a/internal/mode/static/nginx/config/maps_test.go b/internal/mode/static/nginx/config/maps_test.go index 96a3c8381a..4cbf47ebde 100644 --- a/internal/mode/static/nginx/config/maps_test.go +++ b/internal/mode/static/nginx/config/maps_test.go @@ -8,6 +8,7 @@ import ( "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/shared" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/resolver" ) func TestExecuteMaps(t *testing.T) { @@ -214,6 +215,26 @@ func TestExecuteStreamMaps(t *testing.T) { Port: 8080, }, }, + StreamUpstreams: []dataplane.Upstream{ + { + Name: "backend1", + Endpoints: []resolver.Endpoint{ + { + Address: "1.1.1.1", + Port: 80, + }, + }, + }, + { + Name: "backend2", + Endpoints: []resolver.Endpoint{ + { + Address: "1.1.1.1", + Port: 80, + }, + }, + }, + }, } expSubStrings := map[string]int{ @@ -256,7 +277,21 @@ func TestCreateStreamMaps(t *testing.T) { { Hostname: "wrong.example.com", Port: 8080, - UpstreamName: "", + UpstreamName: "fake-backend", + }, + { + Port: 8082, + Hostname: "", + }, + { + Hostname: "*.example.com", + Port: 8080, + IsDefault: true, + }, + { + Hostname: "wrong2.example.com", + Port: 8080, + UpstreamName: "backend3", }, }, SSLServers: []dataplane.VirtualServer{ @@ -269,16 +304,49 @@ func TestCreateStreamMaps(t *testing.T) { IsDefault: true, }, }, + StreamUpstreams: []dataplane.Upstream{ + { + Name: "backend1", + Endpoints: []resolver.Endpoint{ + { + Address: "1.1.1.1", + Port: 80, + }, + }, + }, + { + Name: "backend2", + Endpoints: []resolver.Endpoint{ + { + Address: "1.1.1.1", + Port: 80, + }, + }, + }, + { + Name: "backend3", + Endpoints: nil, + }, + }, } maps := createStreamMaps(conf) expectedMaps := []shared.Map{ + { + Source: "$ssl_preread_server_name", + Variable: getTLSPassthroughVarName(8082), + Parameters: []shared.MapParameter{ + {Value: "default", Result: connectionClosedStreamServerSocket}, + }, + UseHostnames: true, + }, { Source: "$ssl_preread_server_name", Variable: getTLSPassthroughVarName(8081), Parameters: []shared.MapParameter{ {Value: "example.com", Result: getSocketNameTLS(8081, "example.com")}, + {Value: "default", Result: connectionClosedStreamServerSocket}, }, UseHostnames: true, }, @@ -289,6 +357,8 @@ func TestCreateStreamMaps(t *testing.T) { {Value: "example.com", Result: getSocketNameTLS(8080, "example.com")}, {Value: "cafe.example.com", Result: getSocketNameTLS(8080, "cafe.example.com")}, {Value: "wrong.example.com", Result: `""`}, + {Value: "*.example.com", Result: connectionClosedStreamServerSocket}, + {Value: "wrong2.example.com", Result: `""`}, {Value: "app.example.com", Result: getSocketNameHTTPS(8080)}, {Value: "default", Result: getSocketNameHTTPS(8080)}, }, diff --git a/internal/mode/static/nginx/config/servers.go b/internal/mode/static/nginx/config/servers.go index b239e378ed..7ce8d6b803 100644 --- a/internal/mode/static/nginx/config/servers.go +++ b/internal/mode/static/nginx/config/servers.go @@ -10,6 +10,7 @@ import ( "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/http" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/shared" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" ) @@ -92,15 +93,15 @@ func executeServers(conf dataplane.Configuration) []executeResult { } // getIPFamily returns whether the server should be configured for IPv4, IPv6, or both. -func getIPFamily(baseHTTPConfig dataplane.BaseHTTPConfig) http.IPFamily { +func getIPFamily(baseHTTPConfig dataplane.BaseHTTPConfig) shared.IPFamily { switch baseHTTPConfig.IPFamily { case dataplane.IPv4: - return http.IPFamily{IPv4: true} + return shared.IPFamily{IPv4: true} case dataplane.IPv6: - return http.IPFamily{IPv6: true} + return shared.IPFamily{IPv6: true} } - return http.IPFamily{IPv4: true, IPv6: true} + return shared.IPFamily{IPv4: true, IPv6: true} } func createAdditionFileResults(conf dataplane.Configuration) []executeResult { @@ -180,11 +181,11 @@ func createServers( for serverID, s := range sslServers { listen := fmt.Sprint(s.Port) - + sslServer, matchPair := createSSLServer(s, serverID, listen) if _, portInUse := sharedTLSPorts[s.Port]; portInUse { - listen = getSocketNameHTTPS(s.Port) + sslServer.Listen = getSocketNameHTTPS(s.Port) + sslServer.IsSocket = true } - sslServer, matchPair := createSSLServer(s, serverID, listen) servers = append(servers, sslServer) maps.Copy(finalMatchPairs, matchPair) } diff --git a/internal/mode/static/nginx/config/servers_template.go b/internal/mode/static/nginx/config/servers_template.go index 44d9231fb3..b92d1a157c 100644 --- a/internal/mode/static/nginx/config/servers_template.go +++ b/internal/mode/static/nginx/config/servers_template.go @@ -5,10 +5,10 @@ js_preload_object matches from /etc/nginx/conf.d/matches.json; {{- range $s := .Servers -}} {{ if $s.IsDefaultSSL -}} server { - {{- if $.IPFamily.IPv4 }} + {{- if or ($.IPFamily.IPv4) ($s.IsSocket) }} listen {{ $s.Listen }} ssl default_server; {{- end }} - {{- if $.IPFamily.IPv6 }} + {{- if and ($.IPFamily.IPv6) (not $s.IsSocket) }} listen [::]:{{ $s.Listen }} ssl default_server; {{- end }} @@ -29,13 +29,12 @@ server { {{- else }} server { {{- if $s.SSL }} - {{- if $.IPFamily.IPv4 }} + {{- if or ($.IPFamily.IPv4) ($s.IsSocket) }} listen {{ $s.Listen }} ssl; {{- end }} - {{- if $.IPFamily.IPv6 }} + {{- if and ($.IPFamily.IPv6) (not $s.IsSocket) }} listen [::]:{{ $s.Listen }} ssl; {{- end }} - ssl_certificate {{ $s.SSL.Certificate }}; ssl_certificate_key {{ $s.SSL.CertificateKey }}; diff --git a/internal/mode/static/nginx/config/servers_test.go b/internal/mode/static/nginx/config/servers_test.go index ec203235a9..9c4c3ff0b1 100644 --- a/internal/mode/static/nginx/config/servers_test.go +++ b/internal/mode/static/nginx/config/servers_test.go @@ -11,6 +11,7 @@ import ( "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/http" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/shared" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" ) @@ -1158,6 +1159,7 @@ func TestCreateServers(t *testing.T) { { IsDefaultSSL: true, Listen: getSocketNameHTTPS(8443), + IsSocket: true, }, { ServerName: "cafe.example.com", @@ -1167,6 +1169,7 @@ func TestCreateServers(t *testing.T) { }, Locations: getExpectedLocations(true), Listen: getSocketNameHTTPS(8443), + IsSocket: true, GRPC: true, Includes: []string{ includesFolder + "/server-addition-1.conf", @@ -2639,22 +2642,22 @@ func TestGetIPFamily(t *testing.T) { test := []struct { msg string baseHTTPConfig dataplane.BaseHTTPConfig - expected http.IPFamily + expected shared.IPFamily }{ { msg: "ipv4", baseHTTPConfig: dataplane.BaseHTTPConfig{IPFamily: dataplane.IPv4}, - expected: http.IPFamily{IPv4: true, IPv6: false}, + expected: shared.IPFamily{IPv4: true, IPv6: false}, }, { msg: "ipv6", baseHTTPConfig: dataplane.BaseHTTPConfig{IPFamily: dataplane.IPv6}, - expected: http.IPFamily{IPv4: false, IPv6: true}, + expected: shared.IPFamily{IPv4: false, IPv6: true}, }, { msg: "dual", baseHTTPConfig: dataplane.BaseHTTPConfig{IPFamily: dataplane.Dual}, - expected: http.IPFamily{IPv4: true, IPv6: true}, + expected: shared.IPFamily{IPv4: true, IPv6: true}, }, } diff --git a/internal/mode/static/nginx/config/shared/config.go b/internal/mode/static/nginx/config/shared/config.go index baab86c73a..65c0c873f5 100644 --- a/internal/mode/static/nginx/config/shared/config.go +++ b/internal/mode/static/nginx/config/shared/config.go @@ -13,3 +13,9 @@ type MapParameter struct { Value string Result string } + +// IPFamily holds the IP family configuration to be used by NGINX. +type IPFamily struct { + IPv4 bool + IPv6 bool +} diff --git a/internal/mode/static/nginx/config/stream/config.go b/internal/mode/static/nginx/config/stream/config.go index 93f16b22cc..19a1ae7994 100644 --- a/internal/mode/static/nginx/config/stream/config.go +++ b/internal/mode/static/nginx/config/stream/config.go @@ -1,11 +1,14 @@ package stream +import "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/shared" + // Server holds all configuration for a stream server. type Server struct { Listen string ProxyPass string Pass string SSLPreread bool + IsSocket bool } // Upstream holds all configuration for a stream upstream. @@ -19,3 +22,9 @@ type Upstream struct { type UpstreamServer struct { Address string } + +// ServerConfig holds configuration for a stream server and IP family to be used by NGINX. +type ServerConfig struct { + Servers []Server + IPFamily shared.IPFamily +} diff --git a/internal/mode/static/nginx/config/stream_servers.go b/internal/mode/static/nginx/config/stream_servers.go index 29f0991cf0..a83fb5de05 100644 --- a/internal/mode/static/nginx/config/stream_servers.go +++ b/internal/mode/static/nginx/config/stream_servers.go @@ -14,9 +14,14 @@ var streamServersTemplate = gotemplate.Must(gotemplate.New("streamServers").Pars func executeStreamServers(conf dataplane.Configuration) []executeResult { streamServers := createStreamServers(conf) + streamServerConfig := stream.ServerConfig{ + Servers: streamServers, + IPFamily: getIPFamily(conf.BaseHTTPConfig), + } + streamServerResult := executeResult{ dest: streamConfigFile, - data: helpers.MustExecuteTemplate(streamServersTemplate, streamServers), + data: helpers.MustExecuteTemplate(streamServersTemplate, streamServerConfig), } return []executeResult{ @@ -31,13 +36,21 @@ func createStreamServers(conf dataplane.Configuration) []stream.Server { streamServers := make([]stream.Server, 0, len(conf.TLSPassthroughServers)*2) portSet := make(map[int32]struct{}) + upstreams := make(map[string]dataplane.Upstream) + + for _, u := range conf.StreamUpstreams { + upstreams[u.Name] = u + } for _, server := range conf.TLSPassthroughServers { - if server.UpstreamName != "" { - streamServers = append(streamServers, stream.Server{ - Listen: getSocketNameTLS(server.Port, server.Hostname), - ProxyPass: server.UpstreamName, - }) + if u, ok := upstreams[server.UpstreamName]; ok && server.UpstreamName != "" { + if !(server.Hostname == "") && len(u.Endpoints) > 0 { + streamServers = append(streamServers, stream.Server{ + Listen: getSocketNameTLS(server.Port, server.Hostname), + ProxyPass: server.UpstreamName, + IsSocket: true, + }) + } } if _, inPortSet := portSet[server.Port]; inPortSet { diff --git a/internal/mode/static/nginx/config/stream_servers_template.go b/internal/mode/static/nginx/config/stream_servers_template.go index e0e1c00ba8..66f12f5858 100644 --- a/internal/mode/static/nginx/config/stream_servers_template.go +++ b/internal/mode/static/nginx/config/stream_servers_template.go @@ -1,21 +1,28 @@ package config const streamServersTemplateText = ` -{{- range $s := . }} +{{- range $s := .Servers }} server { - listen {{ $s.Listen }}; - + {{- if or ($.IPFamily.IPv4) ($s.IsSocket) }} + listen {{ $s.Listen }}; + {{- end }} + {{- if and ($.IPFamily.IPv6) (not $s.IsSocket) }} + listen [::]:{{ $s.Listen }}; + {{- end }} {{- if $s.ProxyPass }} - proxy_pass {{ $s.ProxyPass }}; + proxy_pass {{ $s.ProxyPass }}; {{- end }} - {{- if $s.Pass }} - pass {{ $s.Pass }}; + pass {{ $s.Pass }}; {{- end }} - {{- if $s.SSLPreread }} - ssl_preread on; + ssl_preread on; {{- end }} } {{- end }} + +server { + listen unix:/var/run/nginx/connection-closed-server.sock; + return ""; +} ` diff --git a/internal/mode/static/nginx/config/stream_servers_test.go b/internal/mode/static/nginx/config/stream_servers_test.go index 1f6a94d9b7..6ec335ab8d 100644 --- a/internal/mode/static/nginx/config/stream_servers_test.go +++ b/internal/mode/static/nginx/config/stream_servers_test.go @@ -9,6 +9,7 @@ import ( "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/stream" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/resolver" ) func TestExecuteStreamServers(t *testing.T) { @@ -30,6 +31,26 @@ func TestExecuteStreamServers(t *testing.T) { UpstreamName: "backend2", }, }, + StreamUpstreams: []dataplane.Upstream{ + { + Name: "backend1", + Endpoints: []resolver.Endpoint{ + { + Address: "1.1.1.1", + Port: 80, + }, + }, + }, + { + Name: "backend2", + Endpoints: []resolver.Endpoint{ + { + Address: "1.1.1.1", + Port: 80, + }, + }, + }, + }, } expSubStrings := map[string]int{ @@ -74,6 +95,26 @@ func TestCreateStreamServers(t *testing.T) { UpstreamName: "", }, }, + StreamUpstreams: []dataplane.Upstream{ + { + Name: "backend1", + Endpoints: []resolver.Endpoint{ + { + Address: "1.1.1.1", + Port: 80, + }, + }, + }, + { + Name: "backend2", + Endpoints: []resolver.Endpoint{ + { + Address: "1.1.1.1", + Port: 80, + }, + }, + }, + }, } streamServers := createStreamServers(conf) @@ -85,16 +126,19 @@ func TestCreateStreamServers(t *testing.T) { Listen: getSocketNameTLS(conf.TLSPassthroughServers[0].Port, conf.TLSPassthroughServers[0].Hostname), ProxyPass: conf.TLSPassthroughServers[0].UpstreamName, SSLPreread: false, + IsSocket: true, }, { Listen: getSocketNameTLS(conf.TLSPassthroughServers[1].Port, conf.TLSPassthroughServers[1].Hostname), ProxyPass: conf.TLSPassthroughServers[1].UpstreamName, SSLPreread: false, + IsSocket: true, }, { Listen: getSocketNameTLS(conf.TLSPassthroughServers[2].Port, conf.TLSPassthroughServers[2].Hostname), ProxyPass: conf.TLSPassthroughServers[2].UpstreamName, SSLPreread: false, + IsSocket: true, }, { Listen: fmt.Sprint(8081), diff --git a/internal/mode/static/nginx/config/upstreams.go b/internal/mode/static/nginx/config/upstreams.go index f15a89d5d8..da52beb07c 100644 --- a/internal/mode/static/nginx/config/upstreams.go +++ b/internal/mode/static/nginx/config/upstreams.go @@ -71,8 +71,12 @@ func (g GeneratorImpl) createStreamUpstream(up dataplane.Upstream) stream.Upstre upstreamServers := make([]stream.UpstreamServer, len(up.Endpoints)) for idx, ep := range up.Endpoints { + format := "%s:%d" + if ep.IPv6 { + format = "[%s]:%d" + } upstreamServers[idx] = stream.UpstreamServer{ - Address: fmt.Sprintf("%s:%d", ep.Address, ep.Port), + Address: fmt.Sprintf(format, ep.Address, ep.Port), } } diff --git a/internal/mode/static/nginx/config/upstreams_test.go b/internal/mode/static/nginx/config/upstreams_test.go index eb7b123542..8ec1c33386 100644 --- a/internal/mode/static/nginx/config/upstreams_test.go +++ b/internal/mode/static/nginx/config/upstreams_test.go @@ -372,6 +372,10 @@ func TestCreateStreamUpstreams(t *testing.T) { Address: "10.0.0.2", Port: 80, }, + { + Address: "2001:db8::1", + IPv6: true, + }, }, }, { @@ -403,6 +407,9 @@ func TestCreateStreamUpstreams(t *testing.T) { { Address: "10.0.0.2:80", }, + { + Address: "[2001:db8::1]:0", + }, }, }, { diff --git a/internal/mode/static/state/change_processor.go b/internal/mode/static/state/change_processor.go index 3fdcd55d97..5ee2e0c09d 100644 --- a/internal/mode/static/state/change_processor.go +++ b/internal/mode/static/state/change_processor.go @@ -12,6 +12,7 @@ import ( "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client" v1 "sigs.k8s.io/gateway-api/apis/v1" + "sigs.k8s.io/gateway-api/apis/v1alpha2" "sigs.k8s.io/gateway-api/apis/v1alpha3" "sigs.k8s.io/gateway-api/apis/v1beta1" @@ -107,6 +108,7 @@ func NewChangeProcessorImpl(cfg ChangeProcessorConfig) *ChangeProcessorImpl { ConfigMaps: make(map[types.NamespacedName]*apiv1.ConfigMap), NginxProxies: make(map[types.NamespacedName]*ngfAPI.NginxProxy), GRPCRoutes: make(map[types.NamespacedName]*v1.GRPCRoute), + TLSRoutes: make(map[types.NamespacedName]*v1alpha2.TLSRoute), NGFPolicies: make(map[graph.PolicyKey]policies.Policy), } @@ -211,6 +213,11 @@ func NewChangeProcessorImpl(cfg ChangeProcessorConfig) *ChangeProcessorImpl { store: commonPolicyObjectStore, predicate: funcPredicate{stateChanged: isNGFPolicyRelevant}, }, + { + gvk: cfg.MustExtractGVK(&v1alpha2.TLSRoute{}), + store: newObjectStoreMapAdapter(clusterStore.TLSRoutes), + predicate: nil, + }, }, ) diff --git a/internal/mode/static/state/change_processor_test.go b/internal/mode/static/state/change_processor_test.go index 3e1455b2ca..398738b167 100644 --- a/internal/mode/static/state/change_processor_test.go +++ b/internal/mode/static/state/change_processor_test.go @@ -199,6 +199,7 @@ func createScheme() *runtime.Scheme { utilruntime.Must(v1.Install(scheme)) utilruntime.Must(v1beta1.Install(scheme)) + utilruntime.Must(v1alpha2.Install(scheme)) utilruntime.Must(v1alpha3.Install(scheme)) utilruntime.Must(apiv1.AddToScheme(scheme)) utilruntime.Must(discoveryV1.AddToScheme(scheme)) @@ -524,6 +525,7 @@ var _ = Describe("ChangeProcessor", func() { Valid: true, Attachable: true, Routes: map[graph.RouteKey]*graph.L7Route{routeKey1: expRouteHR1}, + L4Routes: map[graph.L4RouteKey]*graph.L4Route{}, SupportedKinds: []v1.RouteGroupKind{ {Kind: v1.Kind(kinds.HTTPRoute), Group: helpers.GetPointer[v1.Group](v1.GroupName)}, {Kind: v1.Kind(kinds.GRPCRoute), Group: helpers.GetPointer[v1.Group](v1.GroupName)}, @@ -535,6 +537,7 @@ var _ = Describe("ChangeProcessor", func() { Valid: true, Attachable: true, Routes: map[graph.RouteKey]*graph.L7Route{routeKey1: expRouteHR1}, + L4Routes: map[graph.L4RouteKey]*graph.L4Route{}, ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(diffNsTLSSecret)), SupportedKinds: []v1.RouteGroupKind{ {Kind: v1.Kind(kinds.HTTPRoute), Group: helpers.GetPointer[v1.Group](v1.GroupName)}, @@ -545,6 +548,7 @@ var _ = Describe("ChangeProcessor", func() { Valid: true, }, IgnoredGateways: map[types.NamespacedName]*v1.Gateway{}, + L4Routes: map[graph.L4RouteKey]*graph.L4Route{}, Routes: map[graph.RouteKey]*graph.L7Route{routeKey1: expRouteHR1}, ReferencedSecrets: map[types.NamespacedName]*graph.Secret{}, ReferencedServices: map[types.NamespacedName]struct{}{ diff --git a/internal/mode/static/state/conditions/conditions.go b/internal/mode/static/state/conditions/conditions.go index 5b075a6446..6d364bd1d7 100644 --- a/internal/mode/static/state/conditions/conditions.go +++ b/internal/mode/static/state/conditions/conditions.go @@ -32,6 +32,10 @@ const ( // RouteReasonInvalidListener is used with the "Accepted" condition when the Route references an invalid listener. RouteReasonInvalidListener v1.RouteConditionReason = "InvalidListener" + // RouteReasonHostnameConflict is used with the "Accepted" condition when a route has the exact same hostname + // as another route. + RouteReasonHostnameConflict v1.RouteConditionReason = "HostnameConflict" + // RouteReasonGatewayNotProgrammed is used when the associated Gateway is not programmed. // Used with Accepted (false). RouteReasonGatewayNotProgrammed v1.RouteConditionReason = "GatewayNotProgrammed" @@ -183,6 +187,17 @@ func NewRouteInvalidListener() conditions.Condition { } } +// NewRouteHostnameConflict returns a Condition that indicates that the Route is not accepted because of a +// conflicting hostname on the same port. +func NewRouteHostnameConflict() conditions.Condition { + return conditions.Condition{ + Type: string(v1.RouteConditionAccepted), + Status: metav1.ConditionFalse, + Reason: string(RouteReasonHostnameConflict), + Message: "Hostname(s) conflict with another route of the same kind on the same port.", + } +} + // NewRouteResolvedRefs returns a Condition that indicates that all the references on the Route are resolved. func NewRouteResolvedRefs() conditions.Condition { return conditions.Condition{ @@ -420,6 +435,26 @@ func NewListenerProtocolConflict(msg string) []conditions.Condition { } } +// NewListenerHostnameConflict returns Conditions that indicate multiple Listeners are specified with the same +// Listener port, but are HTTPS and TLS and have overlapping hostnames. +func NewListenerHostnameConflict(msg string) []conditions.Condition { + return []conditions.Condition{ + { + Type: string(v1.ListenerConditionAccepted), + Status: metav1.ConditionFalse, + Reason: string(v1.ListenerReasonHostnameConflict), + Message: msg, + }, + { + Type: string(v1.ListenerConditionConflicted), + Status: metav1.ConditionTrue, + Reason: string(v1.ListenerReasonHostnameConflict), + Message: msg, + }, + NewListenerNotProgrammedInvalid(msg), + } +} + // NewListenerUnsupportedProtocol returns Conditions that indicate that the protocol of a Listener is unsupported. func NewListenerUnsupportedProtocol(msg string) []conditions.Condition { return []conditions.Condition{ diff --git a/internal/mode/static/state/dataplane/configuration.go b/internal/mode/static/state/dataplane/configuration.go index 1edfcf7c70..49cc1cf4e9 100644 --- a/internal/mode/static/state/dataplane/configuration.go +++ b/internal/mode/static/state/dataplane/configuration.go @@ -45,26 +45,160 @@ func BuildConfiguration( baseHTTPConfig := buildBaseHTTPConfig(g) upstreams := buildUpstreams(ctx, g.Gateway.Listeners, serviceResolver, baseHTTPConfig.IPFamily) httpServers, sslServers := buildServers(g, generator) + passthroughServers := buildPassthroughServers(g) + streamUpstreams := buildStreamUpstreams(ctx, g.Gateway.Listeners, serviceResolver, baseHTTPConfig.IPFamily) backendGroups := buildBackendGroups(append(httpServers, sslServers...)) keyPairs := buildSSLKeyPairs(g.ReferencedSecrets, g.Gateway.Listeners) certBundles := buildCertBundles(g.ReferencedCaCertConfigMaps, backendGroups) telemetry := buildTelemetry(g) config := Configuration{ - HTTPServers: httpServers, - SSLServers: sslServers, - Upstreams: upstreams, - BackendGroups: backendGroups, - SSLKeyPairs: keyPairs, - Version: configVersion, - CertBundles: certBundles, - Telemetry: telemetry, - BaseHTTPConfig: baseHTTPConfig, + HTTPServers: httpServers, + SSLServers: sslServers, + TLSPassthroughServers: passthroughServers, + Upstreams: upstreams, + StreamUpstreams: streamUpstreams, + BackendGroups: backendGroups, + SSLKeyPairs: keyPairs, + Version: configVersion, + CertBundles: certBundles, + Telemetry: telemetry, + BaseHTTPConfig: baseHTTPConfig, } return config } +// buildPassthroughServers builds TLSPassthroughServers from TLSRoutes attaches to listeners. +func buildPassthroughServers(g *graph.Graph) []Layer4VirtualServer { + passthroughServersMap := make(map[graph.L4RouteKey][]Layer4VirtualServer) + listenerPassthroughServers := make([]Layer4VirtualServer, 0) + + count := 0 + + for _, l := range g.Gateway.Listeners { + if !l.Valid || l.Source.Protocol != v1.TLSProtocolType { + continue + } + foundRouteMatchingListenerHostname := false + for key, r := range l.L4Routes { + if !r.Valid { + continue + } + + if _, ok := passthroughServersMap[key]; !ok { + passthroughServersMap[key] = []Layer4VirtualServer{} + } + + var hostnames []string + + for _, p := range r.ParentRefs { + if val, exist := p.Attachment.AcceptedHostnames[l.Name]; exist { + hostnames = val + break + } + } + + for _, h := range hostnames { + if l.Source.Hostname != nil && h == string(*l.Source.Hostname) { + foundRouteMatchingListenerHostname = true + } + passthroughServersMap[key] = append(passthroughServersMap[key], Layer4VirtualServer{ + Hostname: h, + UpstreamName: r.Spec.BackendRef.ServicePortReference(), + Port: int32(l.Source.Port), + }) + count++ + } + } + if !foundRouteMatchingListenerHostname { + if l.Source.Hostname != nil { + listenerPassthroughServers = append(listenerPassthroughServers, Layer4VirtualServer{ + Hostname: string(*l.Source.Hostname), + IsDefault: true, + Port: int32(l.Source.Port), + }) + } else { + listenerPassthroughServers = append(listenerPassthroughServers, Layer4VirtualServer{ + Hostname: "", + Port: int32(l.Source.Port), + }) + } + } + } + passthroughServers := make([]Layer4VirtualServer, 0, count) + + for _, r := range passthroughServersMap { + passthroughServers = append(passthroughServers, r...) + } + + passthroughServers = append(passthroughServers, listenerPassthroughServers...) + + return passthroughServers +} + +// buildStreamUpstreams builds all stream upstreams. +func buildStreamUpstreams( + ctx context.Context, + listeners []*graph.Listener, + resolver resolver.ServiceResolver, + ipFamily IPFamilyType, +) []Upstream { + // There can be duplicate upstreams if multiple routes reference the same upstream. + // We use a map to deduplicate them. + uniqueUpstreams := make(map[string]Upstream) + + for _, l := range listeners { + if !l.Valid || l.Source.Protocol != v1.TLSProtocolType { + continue + } + + for _, route := range l.L4Routes { + if !route.Valid { + continue + } + + br := route.Spec.BackendRef + + if !br.Valid { + continue + } + + upstreamName := br.ServicePortReference() + + if _, exist := uniqueUpstreams[upstreamName]; exist { + continue + } + + var errMsg string + + allowedAddressType := getAllowedAddressType(ipFamily) + + eps, err := resolver.Resolve(ctx, br.SvcNsName, br.ServicePort, allowedAddressType) + if err != nil { + errMsg = err.Error() + } + + uniqueUpstreams[upstreamName] = Upstream{ + Name: upstreamName, + Endpoints: eps, + ErrorMsg: errMsg, + } + } + } + + if len(uniqueUpstreams) == 0 { + return nil + } + + upstreams := make([]Upstream, 0, len(uniqueUpstreams)) + + for _, up := range uniqueUpstreams { + upstreams = append(upstreams, up) + } + return upstreams +} + // buildSSLKeyPairs builds the SSLKeyPairs from the Secrets. It will only include Secrets that are referenced by // valid listeners, so that we don't include unused Secrets in the configuration of the data plane. func buildSSLKeyPairs( @@ -212,6 +346,9 @@ func buildServers(g *graph.Graph, generator policies.ConfigGenerator) (http, ssl } for _, l := range g.Gateway.Listeners { + if l.Source.Protocol == v1.TLSProtocolType { + continue + } if l.Valid { rules := rulesForProtocol[l.Source.Protocol][l.Source.Port] if rules == nil { @@ -318,6 +455,7 @@ func (hpr *hostPathRules) upsertRoute( for _, p := range route.ParentRefs { if val, exist := p.Attachment.AcceptedHostnames[string(listener.Source.Name)]; exist { hostnames = val + break } } diff --git a/internal/mode/static/state/dataplane/configuration_test.go b/internal/mode/static/state/dataplane/configuration_test.go index 83d5873ce0..b8c1a9d5f1 100644 --- a/internal/mode/static/state/dataplane/configuration_test.go +++ b/internal/mode/static/state/dataplane/configuration_test.go @@ -13,6 +13,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/intstr" "sigs.k8s.io/controller-runtime/pkg/client" v1 "sigs.k8s.io/gateway-api/apis/v1" "sigs.k8s.io/gateway-api/apis/v1alpha2" @@ -406,6 +407,63 @@ func TestBuildConfiguration(t *testing.T) { pathAndType{path: "/valid", pathType: prefix}, pathAndType{path: invalidMatchesPath, pathType: prefix}, ) + tlsTR1 := graph.L4Route{ + Spec: graph.L4RouteSpec{ + Hostnames: []v1.Hostname{"app.example.com", "cafe.example.com"}, + BackendRef: graph.BackendRef{ + SvcNsName: types.NamespacedName{ + Namespace: "default", + Name: "secure-app", + }, + ServicePort: apiv1.ServicePort{ + Name: "https", + Protocol: "TCP", + Port: 8443, + TargetPort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 8443, + }, + }, + Valid: true, + }, + }, + ParentRefs: []graph.ParentRef{ + { + Attachment: &graph.ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{ + "listener-443-2": {"app.example.com"}, + }, + }, + }, + { + Attachment: &graph.ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{ + "listener-444-3": {"app.example.com"}, + }, + }, + }, + }, + Valid: true, + } + + tlsTR2 := graph.L4Route{ + Spec: graph.L4RouteSpec{ + Hostnames: []v1.Hostname{"test.example.com"}, + BackendRef: graph.BackendRef{}, + }, + Valid: true, + } + + TR1Key := graph.L4RouteKey{NamespacedName: types.NamespacedName{ + Namespace: "default", + Name: "secure-app", + }} + + TR2Key := graph.L4RouteKey{NamespacedName: types.NamespacedName{ + Namespace: "default", + Name: "secure-app2", + }} + httpsHR7, expHTTPSHR7Groups, httpsRouteHR7 := createTestResources( "https-hr-7", "foo.example.com", // same as httpsHR3 @@ -594,6 +652,26 @@ func TestBuildConfiguration(t *testing.T) { }, } + listener443_2 := v1.Listener{ + Name: "listener-443-2", + Hostname: (*v1.Hostname)(helpers.GetPointer("*.example.com")), + Port: 443, + Protocol: v1.TLSProtocolType, + } + + listener444_3 := v1.Listener{ + Name: "listener-444-3", + Hostname: (*v1.Hostname)(helpers.GetPointer("app.example.com")), + Port: 444, + Protocol: v1.TLSProtocolType, + } + + listener443_4 := v1.Listener{ + Name: "listener-443-4", + Port: 443, + Protocol: v1.TLSProtocolType, + } + listener8443 := v1.Listener{ Name: "listener-8443", Hostname: nil, @@ -1520,11 +1598,45 @@ func TestBuildConfiguration(t *testing.T) { }, ResolvedSecret: &secret1NsName, }, + { + Name: "listener-443-2", + Source: listener443_2, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{}, + L4Routes: map[graph.L4RouteKey]*graph.L4Route{ + TR1Key: &tlsTR1, + TR2Key: &tlsTR2, + }, + ResolvedSecret: &secret1NsName, + }, + { + Name: "listener-444-3", + Source: listener444_3, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{}, + L4Routes: map[graph.L4RouteKey]*graph.L4Route{ + TR1Key: &tlsTR1, + TR2Key: &tlsTR2, + }, + ResolvedSecret: &secret1NsName, + }, + { + Name: "listener-443-4", + Source: listener443_4, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{}, + L4Routes: map[graph.L4RouteKey]*graph.L4Route{}, + ResolvedSecret: &secret1NsName, + }, }...) g.Routes = map[graph.RouteKey]*graph.L7Route{ graph.CreateRouteKey(hr6): routeHR6, graph.CreateRouteKey(httpsHR6): httpsRouteHR6, } + g.L4Routes = map[graph.L4RouteKey]*graph.L4Route{ + TR1Key: &tlsTR1, + TR2Key: &tlsTR2, + } g.ReferencedSecrets = map[types.NamespacedName]*graph.Secret{ secret1NsName: secret1, } @@ -1575,9 +1687,40 @@ func TestBuildConfiguration(t *testing.T) { }...) conf.Upstreams = []Upstream{fooUpstream} conf.BackendGroups = []BackendGroup{expHR6Groups[0], expHTTPSHR6Groups[0]} + conf.StreamUpstreams = []Upstream{ + { + Endpoints: fooEndpoints, + Name: "default_secure-app_8443", + }, + } + conf.TLSPassthroughServers = []Layer4VirtualServer{ + { + Hostname: "app.example.com", + UpstreamName: "default_secure-app_8443", + Port: 443, + }, + { + Hostname: "*.example.com", + UpstreamName: "", + Port: 443, + IsDefault: true, + }, + { + Hostname: "app.example.com", + UpstreamName: "default_secure-app_8443", + Port: 444, + IsDefault: false, + }, + { + Hostname: "", + UpstreamName: "", + Port: 443, + IsDefault: false, + }, + } return conf }), - msg: "one http and one https listener with routes with valid and invalid rules", + msg: "one http, one https listener, and three tls listeners with routes with valid and invalid rules", }, { graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { @@ -2124,6 +2267,7 @@ func TestBuildConfiguration(t *testing.T) { 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)) + g.Expect(result.TLSPassthroughServers).To(ConsistOf(test.expConf.TLSPassthroughServers)) g.Expect(result.SSLKeyPairs).To(Equal(test.expConf.SSLKeyPairs)) g.Expect(result.Version).To(Equal(1)) g.Expect(result.CertBundles).To(Equal(test.expConf.CertBundles)) @@ -3215,3 +3359,235 @@ func TestGetAllowedAddressType(t *testing.T) { }) } } + +func TestCreatePassthroughServers(t *testing.T) { + testGraph := graph.Graph{ + Gateway: &graph.Gateway{ + Listeners: []*graph.Listener{ + { + Name: "testingListener", + Source: v1.Listener{ + Protocol: v1.TLSProtocolType, + Port: 443, + Hostname: helpers.GetPointer[v1.Hostname]("*.example.com"), + }, + Routes: make(map[graph.RouteKey]*graph.L7Route), + L4Routes: map[graph.L4RouteKey]*graph.L4Route{ + {NamespacedName: types.NamespacedName{ + Namespace: "default", + Name: "secure-app", + }}: { + Spec: graph.L4RouteSpec{ + Hostnames: []v1.Hostname{"app.example.com", "cafe.example.com"}, + BackendRef: graph.BackendRef{ + SvcNsName: types.NamespacedName{ + Namespace: "default", + Name: "secure-app", + }, + ServicePort: apiv1.ServicePort{ + Name: "https", + Protocol: "TCP", + Port: 8443, + TargetPort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 8443, + }, + }, + Valid: true, + }, + }, + ParentRefs: []graph.ParentRef{ + { + Attachment: &graph.ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{ + "testingListener": {"app.example.com", "cafe.example.com"}, + }, + }, + SectionName: nil, + Port: nil, + Gateway: types.NamespacedName{}, + Idx: 0, + }, + }, + Valid: true, + }, + {NamespacedName: types.NamespacedName{ + Namespace: "default", + Name: "secure-app2", + }}: {}, + }, + Valid: true, + }, + { + Name: "testingListener2", + Source: v1.Listener{ + Protocol: v1.TLSProtocolType, + Port: 443, + Hostname: helpers.GetPointer[v1.Hostname]("cafe.example.com"), + }, + Routes: make(map[graph.RouteKey]*graph.L7Route), + L4Routes: map[graph.L4RouteKey]*graph.L4Route{ + {NamespacedName: types.NamespacedName{ + Namespace: "default", + Name: "secure-app3", + }}: { + Spec: graph.L4RouteSpec{ + Hostnames: []v1.Hostname{"app.example.com", "cafe.example.com"}, + BackendRef: graph.BackendRef{ + SvcNsName: types.NamespacedName{ + Namespace: "default", + Name: "secure-app", + }, + ServicePort: apiv1.ServicePort{ + Name: "https", + Protocol: "TCP", + Port: 8443, + TargetPort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 8443, + }, + }, + Valid: true, + }, + }, + Valid: true, + }, + }, + Valid: true, + }, + }, + }, + } + + passthroughServers := buildPassthroughServers(&testGraph) + + expectedPassthroughServers := []Layer4VirtualServer{ + { + Hostname: "app.example.com", + UpstreamName: "default_secure-app_8443", + Port: 443, + IsDefault: false, + }, + { + Hostname: "cafe.example.com", + UpstreamName: "default_secure-app_8443", + Port: 443, + IsDefault: false, + }, + { + Hostname: "*.example.com", + UpstreamName: "", + Port: 443, + IsDefault: true, + }, + { + Hostname: "cafe.example.com", + UpstreamName: "", + Port: 443, + IsDefault: true, + }, + } + + g := NewWithT(t) + + g.Expect(passthroughServers).To(Equal(expectedPassthroughServers)) +} + +func TestBuildStreamUpstreams(t *testing.T) { + testGraph := graph.Graph{ + Gateway: &graph.Gateway{ + Listeners: []*graph.Listener{ + { + Name: "testingListener", + Source: v1.Listener{ + Protocol: v1.TLSProtocolType, + Port: 443, + }, + Routes: make(map[graph.RouteKey]*graph.L7Route), + L4Routes: map[graph.L4RouteKey]*graph.L4Route{ + {NamespacedName: types.NamespacedName{ + Namespace: "default", + Name: "secure-app", + }}: { + Spec: graph.L4RouteSpec{ + Hostnames: []v1.Hostname{"app.example.com", "cafe.example.com"}, + BackendRef: graph.BackendRef{ + SvcNsName: types.NamespacedName{ + Namespace: "default", + Name: "secure-app", + }, + ServicePort: apiv1.ServicePort{ + Name: "https", + Protocol: "TCP", + Port: 8443, + TargetPort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 8443, + }, + }, + Valid: true, + }, + }, + Valid: true, + }, + {NamespacedName: types.NamespacedName{ + Namespace: "default", + Name: "secure-app2", + }}: {}, + {NamespacedName: types.NamespacedName{ + Namespace: "default", + Name: "secure-app3", + }}: { + Valid: true, + Spec: graph.L4RouteSpec{ + Hostnames: []v1.Hostname{"test.example.com"}, + BackendRef: graph.BackendRef{}, + }, + }, + {NamespacedName: types.NamespacedName{ + Namespace: "default", + Name: "secure-app4", + }}: { + Spec: graph.L4RouteSpec{ + Hostnames: []v1.Hostname{"app.example.com", "cafe.example.com"}, + BackendRef: graph.BackendRef{ + SvcNsName: types.NamespacedName{ + Namespace: "default", + Name: "secure-app", + }, + ServicePort: apiv1.ServicePort{ + Name: "https", + Protocol: "TCP", + Port: 8443, + TargetPort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 8443, + }, + }, + Valid: true, + }, + }, + Valid: true, + }, + }, + Valid: true, + }, + }, + }, + } + + fakeResolver := resolverfakes.FakeServiceResolver{} + fakeResolver.ResolveReturns(nil, errors.New("error")) + + streamUpstreams := buildStreamUpstreams(context.Background(), testGraph.Gateway.Listeners, &fakeResolver, Dual) + + expectedStreamUpstreams := []Upstream{ + { + Name: "default_secure-app_8443", + ErrorMsg: "error", + }, + } + g := NewWithT(t) + + g.Expect(streamUpstreams).To(Equal(expectedStreamUpstreams)) +} diff --git a/internal/mode/static/state/dataplane/types.go b/internal/mode/static/state/dataplane/types.go index 5eef247340..bc58f06b3d 100644 --- a/internal/mode/static/state/dataplane/types.go +++ b/internal/mode/static/state/dataplane/types.go @@ -88,6 +88,8 @@ type Layer4VirtualServer struct { UpstreamName string // Port is the port of the server. Port int32 + // IsDefault refers to whether this server is created for the default listener hostname. + IsDefault bool } // Addition holds additional configuration. diff --git a/internal/mode/static/state/graph/backend_refs.go b/internal/mode/static/state/graph/backend_refs.go index b90fa2ebc4..2d441fca38 100644 --- a/internal/mode/static/state/graph/backend_refs.go +++ b/internal/mode/static/state/graph/backend_refs.go @@ -19,7 +19,7 @@ import ( staticConds "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/conditions" ) -// BackendRef is an internal representation of a backendRef in an HTTP/GRPCRoute. +// BackendRef is an internal representation of a backendRef in an HTTP/GRPC/TLSRoute. type BackendRef struct { // BackendTLSPolicy is the BackendTLSPolicy of the Service which is referenced by the backendRef. BackendTLSPolicy *BackendTLSPolicy diff --git a/internal/mode/static/state/graph/gateway_listener.go b/internal/mode/static/state/graph/gateway_listener.go index f1f4e6bd9f..109a70bc64 100644 --- a/internal/mode/static/state/graph/gateway_listener.go +++ b/internal/mode/static/state/graph/gateway_listener.go @@ -3,6 +3,7 @@ package graph import ( "errors" "fmt" + "strings" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" @@ -25,6 +26,8 @@ type Listener struct { // Routes holds the GRPC/HTTPRoutes attached to the Listener. // Only valid routes are attached. Routes map[RouteKey]*L7Route + // L4Routes holds the TLSRoutes attached to the Listener. + L4Routes map[L4RouteKey]*L4Route // AllowedRouteLabelSelector is the label selector for this Listener's allowed routes, if defined. AllowedRouteLabelSelector labels.Selector // ResolvedSecret is the namespaced name of the Secret resolved for this listener. @@ -61,7 +64,7 @@ func buildListeners( } type listenerConfiguratorFactory struct { - http, https, unsupportedProtocol *listenerConfigurator + http, https, tls, unsupportedProtocol *listenerConfigurator } func (f *listenerConfiguratorFactory) getConfiguratorForListener(l v1.Listener) *listenerConfigurator { @@ -70,6 +73,8 @@ func (f *listenerConfiguratorFactory) getConfiguratorForListener(l v1.Listener) return f.http case v1.HTTPSProtocolType: return f.https + case v1.TLSProtocolType: + return f.tls default: return f.unsupportedProtocol } @@ -90,7 +95,7 @@ func newListenerConfiguratorFactory( valErr := field.NotSupported( field.NewPath("protocol"), listener.Protocol, - []string{string(v1.HTTPProtocolType), string(v1.HTTPSProtocolType)}, + []string{string(v1.HTTPProtocolType), string(v1.HTTPSProtocolType), string(v1.TLSProtocolType)}, ) return staticConds.NewListenerUnsupportedProtocol(valErr.Error()), false /* not attachable */ }, @@ -121,6 +126,18 @@ func newListenerConfiguratorFactory( createExternalReferencesForTLSSecretsResolver(gw.Namespace, secretResolver, refGrantResolver), }, }, + tls: &listenerConfigurator{ + validators: []listenerValidator{ + validateListenerAllowedRouteKind, + validateListenerLabelSelector, + validateListenerHostname, + validateTLSFieldOnTLSListener, + }, + conflictResolvers: []listenerConflictResolver{ + sharedPortConflictResolver, + }, + externalReferenceResolvers: []listenerExternalReferenceResolver{}, + }, } } @@ -184,6 +201,7 @@ func (c *listenerConfigurator) configure(listener v1.Listener) *Listener { Conditions: conds, AllowedRouteLabelSelector: allowedRouteSelector, Routes: make(map[RouteKey]*L7Route), + L4Routes: make(map[L4RouteKey]*L4Route), Valid: valid, Attachable: attachable, SupportedKinds: supportedKinds, @@ -195,7 +213,8 @@ func (c *listenerConfigurator) configure(listener v1.Listener) *Listener { // resolvers might add different conditions to the listener, so we run them all. - for _, resolver := range c.conflictResolvers { + for _, resolver := range c. + conflictResolvers { resolver(l) } @@ -235,8 +254,14 @@ func getAndValidateListenerSupportedKinds(listener v1.Listener) ( var conds []conditions.Condition var supportedKinds []v1.RouteGroupKind + ngfSupportedKinds := map[v1.Kind]struct{}{ + kinds.TLSRoute: {}, + kinds.HTTPRoute: {}, + kinds.GRPCRoute: {}, + } + validRouteKind := func(kind v1.RouteGroupKind) bool { - if kind.Kind != v1.Kind(kinds.HTTPRoute) && kind.Kind != v1.Kind(kinds.GRPCRoute) { + if _, ok := ngfSupportedKinds[kind.Kind]; !ok { return false } if kind.Group == nil || *kind.Group != v1.GroupName { @@ -262,6 +287,10 @@ func getAndValidateListenerSupportedKinds(listener v1.Listener) ( {Kind: v1.Kind(kinds.HTTPRoute), Group: helpers.GetPointer[v1.Group](v1.GroupName)}, {Kind: v1.Kind(kinds.GRPCRoute), Group: helpers.GetPointer[v1.Group](v1.GroupName)}, } + case v1.TLSProtocolType: + supportedKinds = []v1.RouteGroupKind{ + {Kind: v1.Kind(kinds.TLSRoute), Group: helpers.GetPointer[v1.Group](v1.GroupName)}, + } } } @@ -321,6 +350,21 @@ func validateListenerPort(port v1.PortNumber, protectedPorts ProtectedPorts) err return nil } +func validateTLSFieldOnTLSListener(listener v1.Listener) (conds []conditions.Condition, attachable bool) { + tlspath := field.NewPath("TLS") + if listener.TLS == nil { + valErr := field.Required(tlspath, "tls must be defined for TLS listener") + return staticConds.NewListenerUnsupportedValue(valErr.Error()), false + } + if listener.TLS.Mode == nil || *listener.TLS.Mode != v1.TLSModePassthrough { + valErr := field.Required(tlspath.Child("Mode"), + "Mode must be passthrough for TLS listener", + ) + return staticConds.NewListenerUnsupportedValue(valErr.Error()), false + } + return nil, true +} + func createHTTPSListenerValidator(protectedPorts ProtectedPorts) listenerValidator { return func(listener v1.Listener) (conds []conditions.Condition, attachable bool) { if err := validateListenerPort(listener.Port, protectedPorts); err != nil { @@ -387,13 +431,25 @@ func createHTTPSListenerValidator(protectedPorts ProtectedPorts) listenerValidat } func createPortConflictResolver() listenerConflictResolver { + const ( + SecureProtocolGroup int = 0 + InsecureProtocolGroup int = 1 + ) + protocolGoups := map[v1.ProtocolType]int{ + v1.TLSProtocolType: SecureProtocolGroup, + v1.HTTPProtocolType: InsecureProtocolGroup, + v1.HTTPSProtocolType: SecureProtocolGroup, + } conflictedPorts := make(map[v1.PortNumber]bool) - portProtocolOwner := make(map[v1.PortNumber]v1.ProtocolType) + portProtocolOwner := make(map[v1.PortNumber]int) listenersByPort := make(map[v1.PortNumber][]*Listener) format := "Multiple listeners for the same port %d specify incompatible protocols; " + "ensure only one protocol per port" + formatHostname := "HTTPS and TLS listeners for the same port %d specify overlapping hostnames; " + + "ensure no overlapping hostnames for HTTPS and TLS listeners for the same port" + return func(l *Listener) { port := l.Source.Port @@ -409,24 +465,45 @@ func createPortConflictResolver() listenerConflictResolver { // 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. - listenersByPort[port] = append(listenersByPort[port], l) - - protocol, ok := portProtocolOwner[port] + protocolGroup, ok := portProtocolOwner[port] if !ok { - portProtocolOwner[port] = l.Source.Protocol + portProtocolOwner[port] = protocolGoups[l.Source.Protocol] + listenersByPort[port] = append(listenersByPort[port], l) return } - // if protocol owner doesn't match the listener's protocol we mark the port as conflicted, + // if protocol group owner doesn't match the listener's protocol group we mark the port as conflicted, // and invalidate all listeners we've seen for this port. - if protocol != l.Source.Protocol { + if protocolGroup != protocolGoups[l.Source.Protocol] { conflictedPorts[port] = true - for _, l := range listenersByPort[port] { - l.Valid = false + for _, listener := range listenersByPort[port] { + listener.Valid = false conflictedConds := staticConds.NewListenerProtocolConflict(fmt.Sprintf(format, port)) + listener.Conditions = append(listener.Conditions, conflictedConds...) + } + l.Valid = false + conflictedConds := staticConds.NewListenerProtocolConflict(fmt.Sprintf(format, port)) + l.Conditions = append(l.Conditions, conflictedConds...) + } else { + foundConflict := false + for _, listener := range listenersByPort[port] { + if listener.Source.Protocol != l.Source.Protocol && + haveOverlap(l.Source.Hostname, listener.Source.Hostname) { + listener.Valid = false + conflictedConds := staticConds.NewListenerHostnameConflict(fmt.Sprintf(formatHostname, port)) + listener.Conditions = append(listener.Conditions, conflictedConds...) + foundConflict = true + } + } + + if foundConflict { + l.Valid = false + conflictedConds := staticConds.NewListenerHostnameConflict(fmt.Sprintf(formatHostname, port)) l.Conditions = append(l.Conditions, conflictedConds...) } } + + listenersByPort[port] = append(listenersByPort[port], l) } } @@ -482,3 +559,31 @@ func GetAllowedRouteLabelSelector(l v1.Listener) *metav1.LabelSelector { return nil } + +// matchesWildcard checks if hostname2 matches the wildcard pattern of hostname1. +func matchesWildcard(hostname1, hostname2 string) bool { + matchesWildcard := func(h1, h2 string) bool { + if strings.HasPrefix(h1, "*.") { + // Remove the "*." from h1 + h1 = h1[2:] + // Check if h2 ends with h1 + return strings.HasSuffix(h2, h1) + } + return false + } + return matchesWildcard(hostname1, hostname2) || matchesWildcard(hostname2, hostname1) +} + +// haveOverlap checks for overlap between two hostnames. +func haveOverlap(hostname1, hostname2 *v1.Hostname) bool { + // Check if hostname1 matches wildcard pattern of hostname2 or vice versa + if hostname1 == nil || hostname2 == nil { + return true + } + h1, h2 := string(*hostname1), string(*hostname2) + + if h1 == h2 { + return true + } + return matchesWildcard(h1, h2) +} diff --git a/internal/mode/static/state/graph/gateway_listener_test.go b/internal/mode/static/state/graph/gateway_listener_test.go index d60ad7321b..5814c8b597 100644 --- a/internal/mode/static/state/graph/gateway_listener_test.go +++ b/internal/mode/static/state/graph/gateway_listener_test.go @@ -357,6 +357,7 @@ func TestGetAndValidateListenerSupportedKinds(t *testing.T) { HTTPRouteGroupKind, GRPCRouteGroupKind, }, }, + { protocol: v1.HTTPProtocolType, kind: []v1.RouteGroupKind{ @@ -471,3 +472,38 @@ func TestValidateListenerPort(t *testing.T) { }) } } + +func TestListenerNamesHaveOverlap(t *testing.T) { + g := NewWithT(t) + g.Expect(haveOverlap(nil, nil)).To(BeTrue()) + g.Expect(haveOverlap( + (*v1.Hostname)(helpers.GetPointer("*.example.com")), + (*v1.Hostname)(helpers.GetPointer("*.example.com")), + )).To(BeTrue()) + g.Expect(haveOverlap( + (*v1.Hostname)(helpers.GetPointer("cafe.example.com")), + (*v1.Hostname)(helpers.GetPointer("app.example.com")), + )).To(BeFalse()) +} + +func TestValidateTLSFieldOnTLSListener(t *testing.T) { + g := NewWithT(t) + cond, valid := validateTLSFieldOnTLSListener(v1.Listener{}) + g.Expect(valid).To(BeFalse()) + g.Expect(cond).To(Equal(staticConds.NewListenerUnsupportedValue( + "TLS: Required value: tls must be defined for TLS listener", + ))) + + cond, valid = validateTLSFieldOnTLSListener(v1.Listener{TLS: nil}) + g.Expect(valid).To(BeFalse()) + g.Expect(cond).To(Equal(staticConds.NewListenerUnsupportedValue( + "TLS: Required value: tls must be defined for TLS listener", + ))) + + mode := v1.GatewayTLSConfig{Mode: helpers.GetPointer(v1.TLSModeTerminate)} + cond, valid = validateTLSFieldOnTLSListener(v1.Listener{TLS: &mode}) + g.Expect(valid).To(BeFalse()) + g.Expect(cond).To(Equal(staticConds.NewListenerUnsupportedValue( + "TLS.Mode: Required value: Mode must be passthrough for TLS listener", + ))) +} diff --git a/internal/mode/static/state/graph/gateway_test.go b/internal/mode/static/state/graph/gateway_test.go index b942c4470f..60692d2401 100644 --- a/internal/mode/static/state/graph/gateway_test.go +++ b/internal/mode/static/state/graph/gateway_test.go @@ -250,6 +250,15 @@ func TestBuildGateway(t *testing.T) { createTCPListener := func(name, hostname string, port int) v1.Listener { return createListener(name, hostname, port, v1.TCPProtocolType, nil) } + createTLSListener := func(name, hostname string, port int) v1.Listener { + return createListener( + name, + hostname, + port, + v1.TLSProtocolType, + &v1.GatewayTLSConfig{Mode: helpers.GetPointer(v1.TLSModePassthrough)}, + ) + } createHTTPSListener := func(name, hostname string, port int, tls *v1.GatewayTLSConfig) v1.Listener { return createListener(name, hostname, port, v1.HTTPSProtocolType, tls) } @@ -264,6 +273,7 @@ func TestBuildGateway(t *testing.T) { foo80HTTPSListener := createHTTPSListener("foo-80-https", "foo.example.com", 80, gatewayTLSConfigSameNs) foo443HTTPSListener1 := createHTTPSListener("foo-443-https-1", "foo.example.com", 443, gatewayTLSConfigSameNs) foo8443HTTPSListener := createHTTPSListener("foo-8443-https", "foo.example.com", 8443, gatewayTLSConfigSameNs) + splat443HTTPSListener := createHTTPSListener("splat-443-https", "*.example.com", 443, gatewayTLSConfigSameNs) // bar http listener bar80Listener := createHTTPListener("bar-80", "bar.example.com", 80) @@ -280,6 +290,9 @@ func TestBuildGateway(t *testing.T) { gatewayTLSConfigDiffNs, ) + // tls listeners + foo443TLSListener := createTLSListener("foo-443-tls", "foo.example.com", 443) + // invalid listeners invalidProtocolListener := createTCPListener("invalid-protocol", "bar.example.com", 80) invalidPortListener := createHTTPListener("invalid-port", "invalid-port", 0) @@ -315,6 +328,9 @@ func TestBuildGateway(t *testing.T) { conflict443PortMsg = "Multiple listeners for the same port 443 specify incompatible protocols; " + "ensure only one protocol per port" + + conflict443HostnameMsg = "HTTPS and TLS listeners for the same port 443 specify overlapping hostnames; " + + "ensure no overlapping hostnames for HTTPS and TLS listeners for the same port" ) type gatewayCfg struct { @@ -366,19 +382,19 @@ func TestBuildGateway(t *testing.T) { Source: getLastCreatedGetaway(), Listeners: []*Listener{ { - Name: "foo-80-1", - Source: foo80Listener1, - Valid: true, - Attachable: true, - Routes: map[RouteKey]*L7Route{}, + Name: "foo-80-1", + Source: foo80Listener1, + Valid: true, + Attachable: true, + Routes: map[RouteKey]*L7Route{}, L4Routes: map[L4RouteKey]*L4Route{}, SupportedKinds: supportedKindsForListeners, }, { - Name: "foo-8080", - Source: foo8080Listener, - Valid: true, - Attachable: true, - Routes: map[RouteKey]*L7Route{}, + Name: "foo-8080", + Source: foo8080Listener, + Valid: true, + Attachable: true, + Routes: map[RouteKey]*L7Route{}, L4Routes: map[L4RouteKey]*L4Route{}, SupportedKinds: supportedKindsForListeners, }, }, @@ -395,20 +411,20 @@ func TestBuildGateway(t *testing.T) { Source: getLastCreatedGetaway(), Listeners: []*Listener{ { - Name: "foo-443-https-1", - Source: foo443HTTPSListener1, - Valid: true, - Attachable: true, - Routes: map[RouteKey]*L7Route{}, + Name: "foo-443-https-1", + Source: foo443HTTPSListener1, + Valid: true, + Attachable: true, + Routes: map[RouteKey]*L7Route{}, L4Routes: map[L4RouteKey]*L4Route{}, ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), SupportedKinds: supportedKindsForListeners, }, { - Name: "foo-8443-https", - Source: foo8443HTTPSListener, - Valid: true, - Attachable: true, - Routes: map[RouteKey]*L7Route{}, + Name: "foo-8443-https", + Source: foo8443HTTPSListener, + Valid: true, + Attachable: true, + Routes: map[RouteKey]*L7Route{}, L4Routes: map[L4RouteKey]*L4Route{}, ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), SupportedKinds: supportedKindsForListeners, }, @@ -429,7 +445,7 @@ func TestBuildGateway(t *testing.T) { Valid: true, Attachable: true, AllowedRouteLabelSelector: labels.SelectorFromSet(labels.Set(labelSet)), - Routes: map[RouteKey]*L7Route{}, + Routes: map[RouteKey]*L7Route{}, L4Routes: map[L4RouteKey]*L4Route{}, SupportedKinds: []v1.RouteGroupKind{ {Kind: kinds.HTTPRoute, Group: helpers.GetPointer[v1.Group](v1.GroupName)}, }, @@ -470,11 +486,11 @@ func TestBuildGateway(t *testing.T) { Source: getLastCreatedGetaway(), Listeners: []*Listener{ { - Name: "listener-cross-ns-secret", - Source: crossNamespaceSecretListener, - Valid: true, - Attachable: true, - Routes: map[RouteKey]*L7Route{}, + Name: "listener-cross-ns-secret", + Source: crossNamespaceSecretListener, + Valid: true, + Attachable: true, + Routes: map[RouteKey]*L7Route{}, L4Routes: map[L4RouteKey]*L4Route{}, ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretDiffNamespace)), SupportedKinds: supportedKindsForListeners, }, @@ -497,7 +513,7 @@ func TestBuildGateway(t *testing.T) { Conditions: staticConds.NewListenerRefNotPermitted( `Certificate ref to secret diff-ns/secret not permitted by any ReferenceGrant`, ), - Routes: map[RouteKey]*L7Route{}, + Routes: map[RouteKey]*L7Route{}, L4Routes: map[L4RouteKey]*L4Route{}, SupportedKinds: supportedKindsForListeners, }, }, @@ -519,7 +535,7 @@ func TestBuildGateway(t *testing.T) { Conditions: staticConds.NewListenerUnsupportedValue( `invalid label selector: "invalid" is not a valid label selector operator`, ), - Routes: map[RouteKey]*L7Route{}, + Routes: map[RouteKey]*L7Route{}, L4Routes: map[L4RouteKey]*L4Route{}, SupportedKinds: []v1.RouteGroupKind{ {Kind: kinds.HTTPRoute, Group: helpers.GetPointer[v1.Group](v1.GroupName)}, }, @@ -541,9 +557,9 @@ func TestBuildGateway(t *testing.T) { Valid: false, Attachable: false, Conditions: staticConds.NewListenerUnsupportedProtocol( - `protocol: Unsupported value: "TCP": supported values: "HTTP", "HTTPS"`, + `protocol: Unsupported value: "TCP": supported values: "HTTP", "HTTPS", "TLS"`, ), - Routes: map[RouteKey]*L7Route{}, + Routes: map[RouteKey]*L7Route{}, L4Routes: map[L4RouteKey]*L4Route{}, }, }, Valid: true, @@ -572,7 +588,7 @@ func TestBuildGateway(t *testing.T) { Conditions: staticConds.NewListenerUnsupportedValue( `port: Invalid value: 0: port must be between 1-65535`, ), - Routes: map[RouteKey]*L7Route{}, + Routes: map[RouteKey]*L7Route{}, L4Routes: map[L4RouteKey]*L4Route{}, SupportedKinds: supportedKindsForListeners, }, { @@ -583,7 +599,7 @@ func TestBuildGateway(t *testing.T) { Conditions: staticConds.NewListenerUnsupportedValue( `port: Invalid value: 65536: port must be between 1-65535`, ), - Routes: map[RouteKey]*L7Route{}, + Routes: map[RouteKey]*L7Route{}, L4Routes: map[L4RouteKey]*L4Route{}, SupportedKinds: supportedKindsForListeners, }, { @@ -594,8 +610,8 @@ func TestBuildGateway(t *testing.T) { Conditions: staticConds.NewListenerUnsupportedValue( `port: Invalid value: 9113: port is already in use as MetricsPort`, ), - Routes: map[RouteKey]*L7Route{}, SupportedKinds: supportedKindsForListeners, + Routes: map[RouteKey]*L7Route{}, L4Routes: map[L4RouteKey]*L4Route{}, }, }, Valid: true, @@ -611,19 +627,19 @@ func TestBuildGateway(t *testing.T) { Source: getLastCreatedGetaway(), Listeners: []*Listener{ { - Name: "invalid-hostname", - Source: invalidHostnameListener, - Valid: false, - Conditions: staticConds.NewListenerUnsupportedValue(invalidHostnameMsg), - Routes: map[RouteKey]*L7Route{}, + Name: "invalid-hostname", + Source: invalidHostnameListener, + Valid: false, + Conditions: staticConds.NewListenerUnsupportedValue(invalidHostnameMsg), + Routes: map[RouteKey]*L7Route{}, L4Routes: map[L4RouteKey]*L4Route{}, SupportedKinds: supportedKindsForListeners, }, { - Name: "invalid-https-hostname", - Source: invalidHTTPSHostnameListener, - Valid: false, - Conditions: staticConds.NewListenerUnsupportedValue(invalidHostnameMsg), - Routes: map[RouteKey]*L7Route{}, + Name: "invalid-https-hostname", + Source: invalidHTTPSHostnameListener, + Valid: false, + Conditions: staticConds.NewListenerUnsupportedValue(invalidHostnameMsg), + Routes: map[RouteKey]*L7Route{}, L4Routes: map[L4RouteKey]*L4Route{}, SupportedKinds: supportedKindsForListeners, }, }, @@ -642,7 +658,7 @@ func TestBuildGateway(t *testing.T) { Source: invalidTLSConfigListener, Valid: false, Attachable: true, - Routes: map[RouteKey]*L7Route{}, + Routes: map[RouteKey]*L7Route{}, L4Routes: map[L4RouteKey]*L4Route{}, Conditions: staticConds.NewListenerInvalidCertificateRef( `tls.certificateRefs[0]: Invalid value: test/does-not-exist: secret does not exist`, ), @@ -673,70 +689,70 @@ func TestBuildGateway(t *testing.T) { Source: getLastCreatedGetaway(), Listeners: []*Listener{ { - Name: "foo-80-1", - Source: foo80Listener1, - Valid: true, - Attachable: true, - Routes: map[RouteKey]*L7Route{}, + Name: "foo-80-1", + Source: foo80Listener1, + Valid: true, + Attachable: true, + Routes: map[RouteKey]*L7Route{}, L4Routes: map[L4RouteKey]*L4Route{}, SupportedKinds: supportedKindsForListeners, }, { - Name: "foo-8080", - Source: foo8080Listener, - Valid: true, - Attachable: true, - Routes: map[RouteKey]*L7Route{}, + Name: "foo-8080", + Source: foo8080Listener, + Valid: true, + Attachable: true, + Routes: map[RouteKey]*L7Route{}, L4Routes: map[L4RouteKey]*L4Route{}, SupportedKinds: supportedKindsForListeners, }, { - Name: "foo-8081", - Source: foo8081Listener, - Valid: true, - Attachable: true, - Routes: map[RouteKey]*L7Route{}, + Name: "foo-8081", + Source: foo8081Listener, + Valid: true, + Attachable: true, + Routes: map[RouteKey]*L7Route{}, L4Routes: map[L4RouteKey]*L4Route{}, SupportedKinds: supportedKindsForListeners, }, { - Name: "foo-443-https-1", - Source: foo443HTTPSListener1, - Valid: true, - Attachable: true, - Routes: map[RouteKey]*L7Route{}, + Name: "foo-443-https-1", + Source: foo443HTTPSListener1, + Valid: true, + Attachable: true, + Routes: map[RouteKey]*L7Route{}, L4Routes: map[L4RouteKey]*L4Route{}, ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), SupportedKinds: supportedKindsForListeners, }, { - Name: "foo-8443-https", - Source: foo8443HTTPSListener, - Valid: true, - Attachable: true, - Routes: map[RouteKey]*L7Route{}, + Name: "foo-8443-https", + Source: foo8443HTTPSListener, + Valid: true, + Attachable: true, + Routes: map[RouteKey]*L7Route{}, L4Routes: map[L4RouteKey]*L4Route{}, ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), SupportedKinds: supportedKindsForListeners, }, { - Name: "bar-80", - Source: bar80Listener, - Valid: true, - Attachable: true, - Routes: map[RouteKey]*L7Route{}, + Name: "bar-80", + Source: bar80Listener, + Valid: true, + Attachable: true, + Routes: map[RouteKey]*L7Route{}, L4Routes: map[L4RouteKey]*L4Route{}, SupportedKinds: supportedKindsForListeners, }, { - Name: "bar-443-https", - Source: bar443HTTPSListener, - Valid: true, - Attachable: true, - Routes: map[RouteKey]*L7Route{}, + Name: "bar-443-https", + Source: bar443HTTPSListener, + Valid: true, + Attachable: true, + Routes: map[RouteKey]*L7Route{}, L4Routes: map[L4RouteKey]*L4Route{}, ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), SupportedKinds: supportedKindsForListeners, }, { - Name: "bar-8443-https", - Source: bar8443HTTPSListener, - Valid: true, - Attachable: true, - Routes: map[RouteKey]*L7Route{}, + Name: "bar-8443-https", + Source: bar8443HTTPSListener, + Valid: true, + Attachable: true, + Routes: map[RouteKey]*L7Route{}, L4Routes: map[L4RouteKey]*L4Route{}, ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), SupportedKinds: supportedKindsForListeners, }, @@ -763,58 +779,58 @@ func TestBuildGateway(t *testing.T) { Source: getLastCreatedGetaway(), Listeners: []*Listener{ { - Name: "foo-80-1", - Source: foo80Listener1, - Valid: false, - Attachable: true, - Routes: map[RouteKey]*L7Route{}, + Name: "foo-80-1", + Source: foo80Listener1, + Valid: false, + Attachable: true, + Routes: map[RouteKey]*L7Route{}, L4Routes: map[L4RouteKey]*L4Route{}, Conditions: staticConds.NewListenerProtocolConflict(conflict80PortMsg), SupportedKinds: supportedKindsForListeners, }, { - Name: "bar-80", - Source: bar80Listener, - Valid: false, - Attachable: true, - Routes: map[RouteKey]*L7Route{}, + Name: "bar-80", + Source: bar80Listener, + Valid: false, + Attachable: true, + Routes: map[RouteKey]*L7Route{}, L4Routes: map[L4RouteKey]*L4Route{}, Conditions: staticConds.NewListenerProtocolConflict(conflict80PortMsg), SupportedKinds: supportedKindsForListeners, }, { - Name: "foo-443", - Source: foo443Listener, - Valid: false, - Attachable: true, - Routes: map[RouteKey]*L7Route{}, + Name: "foo-443", + Source: foo443Listener, + Valid: false, + Attachable: true, + Routes: map[RouteKey]*L7Route{}, L4Routes: map[L4RouteKey]*L4Route{}, Conditions: staticConds.NewListenerProtocolConflict(conflict443PortMsg), SupportedKinds: supportedKindsForListeners, }, { - Name: "foo-80-https", - Source: foo80HTTPSListener, - Valid: false, - Attachable: true, - Routes: map[RouteKey]*L7Route{}, + Name: "foo-80-https", + Source: foo80HTTPSListener, + Valid: false, + Attachable: true, + Routes: map[RouteKey]*L7Route{}, L4Routes: map[L4RouteKey]*L4Route{}, Conditions: staticConds.NewListenerProtocolConflict(conflict80PortMsg), ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), SupportedKinds: supportedKindsForListeners, }, { - Name: "foo-443-https-1", - Source: foo443HTTPSListener1, - Valid: false, - Attachable: true, - Routes: map[RouteKey]*L7Route{}, + Name: "foo-443-https-1", + Source: foo443HTTPSListener1, + Valid: false, + Attachable: true, + Routes: map[RouteKey]*L7Route{}, L4Routes: map[L4RouteKey]*L4Route{}, Conditions: staticConds.NewListenerProtocolConflict(conflict443PortMsg), ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), SupportedKinds: supportedKindsForListeners, }, { - Name: "bar-443-https", - Source: bar443HTTPSListener, - Valid: false, - Attachable: true, - Routes: map[RouteKey]*L7Route{}, + Name: "bar-443-https", + Source: bar443HTTPSListener, + Valid: false, + Attachable: true, + Routes: map[RouteKey]*L7Route{}, L4Routes: map[L4RouteKey]*L4Route{}, Conditions: staticConds.NewListenerProtocolConflict(conflict443PortMsg), ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), SupportedKinds: supportedKindsForListeners, @@ -870,6 +886,73 @@ func TestBuildGateway(t *testing.T) { }, name: "nil gatewayclass", }, + { + gateway: createGateway( + gatewayCfg{listeners: []v1.Listener{foo443TLSListener, foo443Listener}}, + ), + gatewayClass: validGC, + expected: &Gateway{ + Source: getLastCreatedGetaway(), + Valid: true, + Listeners: []*Listener{ + { + Name: "foo-443-tls", + Source: foo443TLSListener, + Valid: false, + Attachable: true, + Routes: map[RouteKey]*L7Route{}, L4Routes: map[L4RouteKey]*L4Route{}, + Conditions: staticConds.NewListenerProtocolConflict(conflict443PortMsg), + SupportedKinds: []v1.RouteGroupKind{ + {Kind: kinds.TLSRoute, Group: helpers.GetPointer[v1.Group](v1.GroupName)}, + }, + }, + { + Name: "foo-443", + Source: foo443Listener, + Valid: false, + Attachable: true, + Routes: map[RouteKey]*L7Route{}, L4Routes: map[L4RouteKey]*L4Route{}, + Conditions: staticConds.NewListenerProtocolConflict(conflict443PortMsg), + SupportedKinds: supportedKindsForListeners, + }, + }, + }, + name: "http listener and tls listener port conflicting", + }, + { + gateway: createGateway( + gatewayCfg{listeners: []v1.Listener{foo443TLSListener, splat443HTTPSListener}}, + ), + gatewayClass: validGC, + expected: &Gateway{ + Source: getLastCreatedGetaway(), + Valid: true, + Listeners: []*Listener{ + { + Name: "foo-443-tls", + Source: foo443TLSListener, + Valid: false, + Attachable: true, + Routes: map[RouteKey]*L7Route{}, L4Routes: map[L4RouteKey]*L4Route{}, + Conditions: staticConds.NewListenerHostnameConflict(conflict443HostnameMsg), + SupportedKinds: []v1.RouteGroupKind{ + {Kind: kinds.TLSRoute, Group: helpers.GetPointer[v1.Group](v1.GroupName)}, + }, + }, + { + Name: "splat-443-https", + Source: splat443HTTPSListener, + Valid: false, + Attachable: true, + ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), + Routes: map[RouteKey]*L7Route{}, L4Routes: map[L4RouteKey]*L4Route{}, + Conditions: staticConds.NewListenerHostnameConflict(conflict443HostnameMsg), + SupportedKinds: supportedKindsForListeners, + }, + }, + }, + name: "https listener and tls listener with overlapping hostnames", + }, } secretResolver := newSecretResolver( diff --git a/internal/mode/static/state/graph/graph.go b/internal/mode/static/state/graph/graph.go index d874d5d2c7..8752c52420 100644 --- a/internal/mode/static/state/graph/graph.go +++ b/internal/mode/static/state/graph/graph.go @@ -25,6 +25,7 @@ type ClusterState struct { GatewayClasses map[types.NamespacedName]*gatewayv1.GatewayClass Gateways map[types.NamespacedName]*gatewayv1.Gateway HTTPRoutes map[types.NamespacedName]*gatewayv1.HTTPRoute + TLSRoutes map[types.NamespacedName]*v1alpha2.TLSRoute Services map[types.NamespacedName]*v1.Service Namespaces map[types.NamespacedName]*v1.Namespace ReferenceGrants map[types.NamespacedName]*v1beta1.ReferenceGrant @@ -53,6 +54,8 @@ type Graph struct { IgnoredGateways map[types.NamespacedName]*gatewayv1.Gateway // Routes hold Route resources. Routes map[RouteKey]*L7Route + // L4Routes hold L4Route resources. + L4Routes map[L4RouteKey]*L4Route // ReferencedSecrets includes Secrets referenced by Gateway Listeners, including invalid ones. // It is different from the other maps, because it includes entries for Secrets that do not exist // in the cluster. We need such entries so that we can query the Graph to determine if a Secret is referenced @@ -224,12 +227,19 @@ func BuildGraph( npCfg, ) - bindRoutesToListeners(routes, gw, state.Namespaces) + l4routes := buildL4RoutesForGateways( + state.TLSRoutes, + processedGws.GetAllNsNames(), + state.Services, + npCfg, + ) + + bindRoutesToListeners(routes, l4routes, gw, state.Namespaces) addBackendRefsToRouteRules(routes, refGrantResolver, state.Services, processedBackendTLSPolicies, npCfg) referencedNamespaces := buildReferencedNamespaces(state.Namespaces, gw) - referencedServices := buildReferencedServices(routes) + referencedServices := buildReferencedServices(routes, l4routes) // policies must be processed last because they rely on the state of the other resources in the graph processedPolicies := processPolicies( @@ -244,6 +254,7 @@ func BuildGraph( GatewayClass: gc, Gateway: gw, Routes: routes, + L4Routes: l4routes, IgnoredGatewayClasses: processedGwClasses.Ignored, IgnoredGateways: processedGws.Ignored, ReferencedSecrets: secretResolver.getResolvedSecrets(), diff --git a/internal/mode/static/state/graph/graph_test.go b/internal/mode/static/state/graph/graph_test.go index ea8de6c551..0053ed6677 100644 --- a/internal/mode/static/state/graph/graph_test.go +++ b/internal/mode/static/state/graph/graph_test.go @@ -4,6 +4,7 @@ import ( "testing" . "github.com/onsi/gomega" + "github.com/onsi/gomega/format" v1 "k8s.io/api/core/v1" discoveryV1 "k8s.io/api/discovery/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -101,6 +102,15 @@ func TestBuildGraph(t *testing.T) { }, } + commonTLSBackendRef := gatewayv1.BackendRef{ + BackendObjectReference: gatewayv1.BackendObjectReference{ + Kind: (*gatewayv1.Kind)(helpers.GetPointer("Service")), + Name: "foo2", + Namespace: (*gatewayv1.Namespace)(helpers.GetPointer("test")), + Port: (*gatewayv1.PortNumber)(helpers.GetPointer[int32](80)), + }, + } + createValidRuleWithBackendRefs := func(matches []gatewayv1.HTTPRouteMatch) RouteRule { refs := []BackendRef{ { @@ -167,10 +177,42 @@ func TestBuildGraph(t *testing.T) { } } + createRouteTLS := func(name string, gatewayName string, _ string) *v1alpha2.TLSRoute { + return &v1alpha2.TLSRoute{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testNs, + Name: name, + }, + Spec: v1alpha2.TLSRouteSpec{ + CommonRouteSpec: gatewayv1.CommonRouteSpec{ + ParentRefs: []gatewayv1.ParentReference{ + { + Namespace: (*gatewayv1.Namespace)(helpers.GetPointer(testNs)), + Name: gatewayv1.ObjectName(gatewayName), + }, + }, + }, + Hostnames: []gatewayv1.Hostname{ + "fizz.example.org", + }, + Rules: []v1alpha2.TLSRouteRule{ + { + BackendRefs: []v1alpha2.BackendRef{ + commonTLSBackendRef, + }, + }, + }, + }, + } + } + hr1 := createRoute("hr-1", "gateway-1", "listener-80-1") hr2 := createRoute("hr-2", "wrong-gateway", "listener-80-1") hr3 := createRoute("hr-3", "gateway-1", "listener-443-1") // https listener; should not conflict with hr1 + tr := createRouteTLS("tr", "gateway-1", "listener-443-2") + tr2 := createRouteTLS("tr2", "gateway-1", "listener-443-2") + gr := &gatewayv1.GRPCRoute{ ObjectMeta: metav1.ObjectMeta{ Namespace: testNs, @@ -250,7 +292,7 @@ func TestBuildGraph(t *testing.T) { { Name: "listener-443-1", - Hostname: nil, + Hostname: (*gatewayv1.Hostname)(helpers.GetPointer("*.example.com")), Port: 443, TLS: &gatewayv1.GatewayTLSConfig{ Mode: helpers.GetPointer(gatewayv1.TLSModeTerminate), @@ -264,6 +306,25 @@ func TestBuildGraph(t *testing.T) { }, Protocol: gatewayv1.HTTPSProtocolType, }, + { + Name: "listener-443-2", + Hostname: (*gatewayv1.Hostname)(helpers.GetPointer("*.example.org")), + Port: 443, + Protocol: gatewayv1.TLSProtocolType, + TLS: &gatewayv1.GatewayTLSConfig{Mode: helpers.GetPointer(gatewayv1.TLSModePassthrough)}, + AllowedRoutes: &gatewayv1.AllowedRoutes{ + Kinds: []gatewayv1.RouteGroupKind{ + {Kind: kinds.TLSRoute, Group: helpers.GetPointer[gatewayv1.Group](gatewayv1.GroupName)}, + }, + }, + }, + { + Name: "listener-8443", + Hostname: (*gatewayv1.Hostname)(helpers.GetPointer("*.example.org")), + Port: 8443, + Protocol: gatewayv1.TLSProtocolType, + TLS: &gatewayv1.GatewayTLSConfig{Mode: helpers.GetPointer(gatewayv1.TLSModePassthrough)}, + }, }, }, } @@ -285,6 +346,19 @@ func TestBuildGraph(t *testing.T) { }, } + svc1 := &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", Name: "foo2", + }, + Spec: v1.ServiceSpec{ + Ports: []v1.ServicePort{ + { + Port: 80, + }, + }, + }, + } + rgSecret := &v1beta1.ReferenceGrant{ ObjectMeta: metav1.ObjectMeta{ Name: "rg-secret", @@ -454,11 +528,16 @@ func TestBuildGraph(t *testing.T) { client.ObjectKeyFromObject(hr2): hr2, client.ObjectKeyFromObject(hr3): hr3, }, + TLSRoutes: map[types.NamespacedName]*v1alpha2.TLSRoute{ + client.ObjectKeyFromObject(tr): tr, + client.ObjectKeyFromObject(tr2): tr2, + }, GRPCRoutes: map[types.NamespacedName]*gatewayv1.GRPCRoute{ client.ObjectKeyFromObject(gr): gr, }, Services: map[types.NamespacedName]*v1.Service{ - client.ObjectKeyFromObject(svc): svc, + client.ObjectKeyFromObject(svc): svc, + client.ObjectKeyFromObject(svc1): svc1, }, Namespaces: map[types.NamespacedName]*v1.Namespace{ client.ObjectKeyFromObject(ns): ns, @@ -510,6 +589,68 @@ func TestBuildGraph(t *testing.T) { Policies: []*Policy{processedRoutePolicy}, } + routeTR := &L4Route{ + Valid: true, + Attachable: true, + Source: tr, + ParentRefs: []ParentRef{ + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw1), + Attachment: &ParentRefAttachmentStatus{ + Attached: true, + AcceptedHostnames: map[string][]string{ + "listener-443-2": {"fizz.example.org"}, + "listener-8443": {"fizz.example.org"}, + }, + }, + }, + }, + Spec: L4RouteSpec{ + Hostnames: tr.Spec.Hostnames, + BackendRef: BackendRef{ + SvcNsName: types.NamespacedName{ + Namespace: "test", + Name: "foo2", + }, + ServicePort: v1.ServicePort{ + Port: 80, + }, + Valid: true, + }, + }, + } + + routeTR2 := &L4Route{ + Valid: true, + Attachable: true, + Source: tr2, + ParentRefs: []ParentRef{ + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw1), + Attachment: &ParentRefAttachmentStatus{ + Attached: false, + AcceptedHostnames: map[string][]string{}, + FailedCondition: staticConds.NewRouteHostnameConflict(), + }, + }, + }, + Spec: L4RouteSpec{ + Hostnames: tr.Spec.Hostnames, + BackendRef: BackendRef{ + SvcNsName: types.NamespacedName{ + Namespace: "test", + Name: "foo2", + }, + ServicePort: v1.ServicePort{ + Port: 80, + }, + Valid: true, + }, + }, + } + routeGR := &L7Route{ RouteType: RouteTypeGRPC, Valid: true, @@ -581,6 +722,7 @@ func TestBuildGraph(t *testing.T) { CreateRouteKey(gr): routeGR, }, SupportedKinds: supportedKindsForListeners, + L4Routes: map[L4RouteKey]*L4Route{}, AllowedRouteLabelSelector: labels.SelectorFromSet(map[string]string{"app": "allowed"}), }, { @@ -589,9 +731,32 @@ func TestBuildGraph(t *testing.T) { Valid: true, Attachable: true, Routes: map[RouteKey]*L7Route{CreateRouteKey(hr3): routeHR3}, + L4Routes: map[L4RouteKey]*L4Route{}, ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secret)), SupportedKinds: supportedKindsForListeners, }, + { + Name: "listener-443-2", + Source: gw1.Spec.Listeners[2], + Valid: true, + Attachable: true, + L4Routes: map[L4RouteKey]*L4Route{CreateRouteKeyL4(tr): routeTR}, + Routes: map[RouteKey]*L7Route{}, + SupportedKinds: []gatewayv1.RouteGroupKind{ + {Kind: kinds.TLSRoute, Group: helpers.GetPointer[gatewayv1.Group](gatewayv1.GroupName)}, + }, + }, + { + Name: "listener-8443", + Source: gw1.Spec.Listeners[3], + Valid: true, + Attachable: true, + L4Routes: map[L4RouteKey]*L4Route{CreateRouteKeyL4(tr): routeTR}, + Routes: map[RouteKey]*L7Route{}, + SupportedKinds: []gatewayv1.RouteGroupKind{ + {Kind: kinds.TLSRoute, Group: helpers.GetPointer[gatewayv1.Group](gatewayv1.GroupName)}, + }, + }, }, Valid: true, Policies: []*Policy{processedGwPolicy}, @@ -604,6 +769,10 @@ func TestBuildGraph(t *testing.T) { CreateRouteKey(hr3): routeHR3, CreateRouteKey(gr): routeGR, }, + L4Routes: map[L4RouteKey]*L4Route{ + CreateRouteKeyL4(tr): routeTR, + CreateRouteKeyL4(tr2): routeTR2, + }, ReferencedSecrets: map[types.NamespacedName]*Secret{ client.ObjectKeyFromObject(secret): { Source: secret, @@ -613,7 +782,8 @@ func TestBuildGraph(t *testing.T) { client.ObjectKeyFromObject(ns): ns, }, ReferencedServices: map[types.NamespacedName]struct{}{ - client.ObjectKeyFromObject(svc): {}, + client.ObjectKeyFromObject(svc): {}, + client.ObjectKeyFromObject(svc1): {}, }, ReferencedCaCertConfigMaps: map[types.NamespacedName]*CaCertConfigMap{ client.ObjectKeyFromObject(cm): { @@ -684,6 +854,7 @@ func TestBuildGraph(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { g := NewWithT(t) + format.MaxLength = 10000000 fakePolicyValidator := &validationfakes.FakePolicyValidator{} diff --git a/internal/mode/static/state/graph/route_common.go b/internal/mode/static/state/graph/route_common.go index 36d14a11f3..a4c266dab4 100644 --- a/internal/mode/static/state/graph/route_common.go +++ b/internal/mode/static/state/graph/route_common.go @@ -2,6 +2,7 @@ package graph import ( "fmt" + "sort" "strings" apiv1 "k8s.io/api/core/v1" @@ -10,9 +11,11 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" "sigs.k8s.io/controller-runtime/pkg/client" v1 "sigs.k8s.io/gateway-api/apis/v1" + v1alpha "sigs.k8s.io/gateway-api/apis/v1alpha2" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/kinds" + sort2 "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/sort" staticConds "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/conditions" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/validation" ) @@ -55,6 +58,12 @@ const ( RouteTypeGRPC RouteType = "grpc" ) +// L4RouteKey is the unique identifier for a L4Route. +type L4RouteKey struct { + // NamespacedName is the NamespacedName of the Route. + NamespacedName types.NamespacedName +} + // RouteKey is the unique identifier for a L7Route. type RouteKey struct { // NamespacedName is the NamespacedName of the Route. @@ -63,6 +72,29 @@ type RouteKey struct { RouteType RouteType } +type L4Route struct { + // Source is the source Gateway API object of the Route. + Source client.Object + // ParentRefs describe the references to the parents in a Route. + ParentRefs []ParentRef + // Conditions define the conditions to be reported in the status of the Route. + Conditions []conditions.Condition + // Spec is the L4RouteSpec of the Route + Spec L4RouteSpec + // Valid indicates if the Route is valid. + Valid bool + // Attachable indicates if the Route is attachable to any Listener. + Attachable bool +} + +type L4RouteSpec struct { + // Hostnames defines a set of hostnames used to select a Route used to process the request. + Hostnames []v1.Hostname + // FIXME (sarthyparty): change to slice of BackendRef, as for now we are only supporting one BackendRef. + // We will eventually support multiple BackendRef https://github.com/nginxinc/nginx-gateway-fabric/issues/2184 + BackendRef BackendRef +} + // L7Route is the generic type for the layer 7 routes, HTTPRoute and GRPCRoute. type L7Route struct { // Source is the source Gateway API object of the Route. @@ -132,6 +164,33 @@ func CreateRouteKey(obj client.Object) RouteKey { } } +// CreateRouteKeyL4 takes a client.Object and creates a L4RouteKey. +func CreateRouteKeyL4(obj client.Object) L4RouteKey { + return L4RouteKey{ + NamespacedName: client.ObjectKeyFromObject(obj), + } +} + +func buildL4RoutesForGateways( + tlsRoutes map[types.NamespacedName]*v1alpha.TLSRoute, + gatewayNsNames []types.NamespacedName, + services map[types.NamespacedName]*apiv1.Service, + npCfg *NginxProxy, +) map[L4RouteKey]*L4Route { + if len(gatewayNsNames) == 0 { + return nil + } + + routes := make(map[L4RouteKey]*L4Route) + for _, route := range tlsRoutes { + r := buildTLSRoute(route, gatewayNsNames, services, npCfg) + if r != nil { + routes[CreateRouteKeyL4(route)] = r + } + } + return routes +} + // buildGRPCRoutesForGateways builds routes from HTTP/GRPCRoutes that reference any of the specified Gateways. func buildRoutesForGateways( validator validation.HTTPFieldsValidator, @@ -245,7 +304,8 @@ func findGatewayForParentRef( } func bindRoutesToListeners( - routes map[RouteKey]*L7Route, + l7Routes map[RouteKey]*L7Route, + l4Routes map[L4RouteKey]*L4Route, gw *Gateway, namespaces map[types.NamespacedName]*apiv1.Namespace, ) { @@ -253,12 +313,220 @@ func bindRoutesToListeners( return } + portHostnamesMap := make(map[string]struct{}) + + for _, r := range l7Routes { + bindL7RouteToListeners(r, gw, namespaces) + } + + var routes []*L4Route + for _, r := range l4Routes { + routes = append(routes, r) + } + + // Sort the slice + sort.Slice(routes, func(i, j int) bool { + return sort2.LessClientObject(routes[i].Source, routes[j].Source) + }) + for _, r := range routes { - bindRouteToListeners(r, gw, namespaces) + bindL4RouteToListeners(r, gw, namespaces, portHostnamesMap) + } +} + +func bindL4RouteToListeners( + route *L4Route, + gw *Gateway, + namespaces map[types.NamespacedName]*apiv1.Namespace, + portHostnamesMap map[string]struct{}, +) { + if !route.Attachable { + return + } + + for i := range route.ParentRefs { + attachment := &ParentRefAttachmentStatus{ + AcceptedHostnames: make(map[string][]string), + } + ref := &(route.ParentRefs)[i] + + ref.Attachment = attachment + + path := field.NewPath("spec").Child("parentRefs").Index(ref.Idx) + + attachableListeners, listenerExists := findAttachableListeners( + getSectionName(ref.SectionName), + gw.Listeners, + ) + + // Case 1: Attachment is not possible because the specified SectionName does not match any Listeners in the + // Gateway. + if !listenerExists { + attachment.FailedCondition = staticConds.NewRouteNoMatchingParent() + continue + } + + // Case 2: Attachment is not possible due to unsupported configuration. + + if ref.Port != nil { + valErr := field.Forbidden(path.Child("port"), "cannot be set") + attachment.FailedCondition = staticConds.NewRouteUnsupportedValue(valErr.Error()) + continue + } + + // Case 3: the parentRef references an ignored Gateway resource. + + referencesWinningGw := ref.Gateway.Namespace == gw.Source.Namespace && ref.Gateway.Name == gw.Source.Name + + if !referencesWinningGw { + attachment.FailedCondition = staticConds.NewRouteNotAcceptedGatewayIgnored() + continue + } + + // Case 4: Attachment is not possible because Gateway is invalid + + if !gw.Valid { + attachment.FailedCondition = staticConds.NewRouteInvalidGateway() + continue + } + + // Case 5 - winning Gateway + + // Try to attach Route to all matching listeners + + cond, attached := tryToAttachL4RouteToListeners( + ref.Attachment, + attachableListeners, + route, + gw, + namespaces, + portHostnamesMap, + ) + if !attached { + attachment.FailedCondition = cond + continue + } + if cond != (conditions.Condition{}) { + route.Conditions = append(route.Conditions, cond) + } + + attachment.Attached = true + } +} + +// tryToAttachL4RouteToListeners tries to attach the L4Route to listeners that match the parentRef and the hostnames. +// There are two cases: +// (1) If it succeeds in attaching at least one listener it will return true. The returned condition will be empty if +// at least one of the listeners is valid. Otherwise, it will return the failure condition. +// (2) If it fails to attach the route, it will return false and the failure condition. +func tryToAttachL4RouteToListeners( + refStatus *ParentRefAttachmentStatus, + attachableListeners []*Listener, + route *L4Route, + gw *Gateway, + namespaces map[types.NamespacedName]*apiv1.Namespace, + portHostnamesMap map[string]struct{}, +) (conditions.Condition, bool) { + if len(attachableListeners) == 0 { + return staticConds.NewRouteInvalidListener(), false + } + + var ( + attachedToAtLeastOneValidListener bool + allowed, attached, hostnamesUnique bool + ) + + // Sorting the listeners from most specific hostname to least specific hostname + sort.Slice(attachableListeners, func(i, j int) bool { + h1 := "" + h2 := "" + if attachableListeners[i].Source.Hostname != nil { + h1 = string(*attachableListeners[i].Source.Hostname) + } + if attachableListeners[j].Source.Hostname != nil { + h2 = string(*attachableListeners[j].Source.Hostname) + } + return h1 == GetMoreSpecificHostname(h1, h2) + }) + + for _, l := range attachableListeners { + routeAllowed, routeAttached, hostnameUnique := bindToListenerL4( + l, + route, + gw, + namespaces, + portHostnamesMap, + refStatus, + ) + allowed = allowed || routeAllowed + attached = attached || routeAttached + hostnamesUnique = hostnamesUnique || hostnameUnique + attachedToAtLeastOneValidListener = attachedToAtLeastOneValidListener || (routeAttached && l.Valid) } + + if !attached { + if !allowed { + return staticConds.NewRouteNotAllowedByListeners(), false + } + if !hostnamesUnique { + return staticConds.NewRouteHostnameConflict(), false + } + return staticConds.NewRouteNoMatchingListenerHostname(), false + } + + if !attachedToAtLeastOneValidListener { + return staticConds.NewRouteInvalidListener(), true + } + + return conditions.Condition{}, true } -func bindRouteToListeners( +func bindToListenerL4( + l *Listener, + route *L4Route, + gw *Gateway, + namespaces map[types.NamespacedName]*apiv1.Namespace, + portHostnamesMap map[string]struct{}, + refStatus *ParentRefAttachmentStatus, +) (allowed, attached, notConflicting bool) { + if !isRouteNamespaceAllowedByListener(l, route.Source.GetNamespace(), gw.Source.Namespace, namespaces) { + return false, false, false + } + + if !isRouteTypeAllowedByListener(l, kinds.TLSRoute) { + return false, false, false + } + + acceptedListenerHostnames := findAcceptedHostnames(l.Source.Hostname, route.Spec.Hostnames) + + hostnames := make([]string, 0) + + for _, h := range acceptedListenerHostnames { + portHostname := fmt.Sprintf("%s:%d", h, l.Source.Port) + _, ok := portHostnamesMap[portHostname] + if !ok { + portHostnamesMap[portHostname] = struct{}{} + hostnames = append(hostnames, h) + } + } + + // We only add a condition is there are no valid hostnames left. If there are none left, then we will want to check + // if any hostnames were removed because of conflicts first, and add that condition first. Otherwise, we know that + // the hostnames were all removed because they didn't match the listener hostname, so we add that condition. + if len(hostnames) == 0 && len(hostnames) < len(acceptedListenerHostnames) { + return true, false, false + } + if len(hostnames) == 0 { + return true, false, true + } + + refStatus.AcceptedHostnames[string(l.Source.Name)] = hostnames + l.L4Routes[CreateRouteKeyL4(route.Source)] = route + + return true, true, true +} + +func bindL7RouteToListeners( route *L7Route, gw *Gateway, namespaces map[types.NamespacedName]*apiv1.Namespace, @@ -271,7 +539,8 @@ func bindRouteToListeners( attachment := &ParentRefAttachmentStatus{ AcceptedHostnames: make(map[string][]string), } - ref := &route.ParentRefs[i] + ref := &(route.ParentRefs)[i] + ref.Attachment = attachment path := field.NewPath("spec").Child("parentRefs").Index(ref.Idx) @@ -316,7 +585,7 @@ func bindRouteToListeners( // Try to attach Route to all matching listeners - cond, attached := tryToAttachRouteToListeners( + cond, attached := tryToAttachL7RouteToListeners( ref.Attachment, attachableListeners, route, @@ -340,7 +609,7 @@ func bindRouteToListeners( // (1) If it succeeds in attaching at least one listener it will return true. The returned condition will be empty if // at least one of the listeners is valid. Otherwise, it will return the failure condition. // (2) If it fails to attach the route, it will return false and the failure condition. -func tryToAttachRouteToListeners( +func tryToAttachL7RouteToListeners( refStatus *ParentRefAttachmentStatus, attachableListeners []*Listener, route *L7Route, @@ -351,14 +620,12 @@ func tryToAttachRouteToListeners( return staticConds.NewRouteInvalidListener(), false } - rk := CreateRouteKey(route.Source) - bind := func(l *Listener) (allowed, attached bool) { if !isRouteNamespaceAllowedByListener(l, route.Source.GetNamespace(), gw.Source.Namespace, namespaces) { return false, false } - if !isRouteTypeAllowedByListener(l, route.RouteType) { + if !isRouteTypeAllowedByListener(l, convertRouteType(route.RouteType)) { return false, false } @@ -366,9 +633,11 @@ func tryToAttachRouteToListeners( if len(hostnames) == 0 { return true, false } + refStatus.AcceptedHostnames[string(l.Source.Name)] = hostnames - l.Routes[rk] = route + l.Routes[CreateRouteKey(route.Source)] = route + return true, true } @@ -535,9 +804,9 @@ func isRouteNamespaceAllowedByListener( } // isRouteKindAllowedByListener checks if the route is allowed to attach to the listener. -func isRouteTypeAllowedByListener(listener *Listener, routeType RouteType) bool { +func isRouteTypeAllowedByListener(listener *Listener, k v1.Kind) bool { for _, kind := range listener.SupportedKinds { - if kind.Kind == convertRouteType(routeType) { + if kind.Kind == k { return true } } diff --git a/internal/mode/static/state/graph/route_common_test.go b/internal/mode/static/state/graph/route_common_test.go index 36b32318a5..4d07b674a2 100644 --- a/internal/mode/static/state/graph/route_common_test.go +++ b/internal/mode/static/state/graph/route_common_test.go @@ -12,6 +12,7 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" "sigs.k8s.io/controller-runtime/pkg/client" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + "sigs.k8s.io/gateway-api/apis/v1alpha2" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" @@ -1220,7 +1221,7 @@ func TestBindRouteToListeners(t *testing.T) { t.Run(test.name, func(t *testing.T) { g := NewWithT(t) - bindRouteToListeners( + bindL7RouteToListeners( test.route, test.gateway, namespaces, @@ -1788,7 +1789,504 @@ func TestAllowedRouteType(t *testing.T) { for _, test := range test { t.Run(test.name, func(t *testing.T) { g := NewWithT(t) - g.Expect(isRouteTypeAllowedByListener(test.listener, test.routeType)).To(Equal(test.expResult)) + g.Expect(isRouteTypeAllowedByListener(test.listener, convertRouteType(test.routeType))).To(Equal(test.expResult)) }) } } +func TestBindL4RouteToListeners(t *testing.T) { + // we create a new listener each time because the function under test can modify it + createListener := func(name string) *Listener { + return &Listener{ + Name: name, + Source: gatewayv1.Listener{ + Name: gatewayv1.SectionName(name), + Hostname: (*gatewayv1.Hostname)(helpers.GetPointer("foo.example.com")), + Protocol: gatewayv1.TLSProtocolType, + TLS: helpers.GetPointer(gatewayv1.GatewayTLSConfig{ + Mode: helpers.GetPointer(gatewayv1.TLSModeTerminate), + }), + }, + SupportedKinds: []gatewayv1.RouteGroupKind{ + {Kind: kinds.TLSRoute, Group: helpers.GetPointer[gatewayv1.Group](gatewayv1.GroupName)}, + }, + Valid: true, + Attachable: true, + Routes: map[RouteKey]*L7Route{}, + L4Routes: map[L4RouteKey]*L4Route{}, + } + } + createModifiedListener := func(name string, m func(*Listener)) *Listener { + l := createListener(name) + m(l) + return l + } + + gw := &gatewayv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "gateway", + }, + } + + gwWrongNamespace := &gatewayv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "wrong", + Name: "gateway", + }, + } + + createTLSRouteWithSectionNameAndPort := func( + sectionName *gatewayv1.SectionName, + port *gatewayv1.PortNumber, + ns string, + ) *v1alpha2.TLSRoute { + return &v1alpha2.TLSRoute{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + Name: "hr", + }, + Spec: v1alpha2.TLSRouteSpec{ + CommonRouteSpec: gatewayv1.CommonRouteSpec{ + ParentRefs: []gatewayv1.ParentReference{ + { + Name: gatewayv1.ObjectName(gw.Name), + SectionName: sectionName, + Port: port, + }, + }, + }, + Hostnames: []gatewayv1.Hostname{ + "foo.example.com", + }, + }, + } + } + + tr := createTLSRouteWithSectionNameAndPort(helpers.GetPointer[gatewayv1.SectionName]("listener-443"), nil, "test") + + var normalRoute *L4Route + createNormalRoute := func(gateway *gatewayv1.Gateway) *L4Route { + normalRoute = &L4Route{ + Source: tr, + Spec: L4RouteSpec{ + Hostnames: tr.Spec.Hostnames, + }, + Valid: true, + Attachable: true, + ParentRefs: []ParentRef{ + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gateway), + SectionName: tr.Spec.ParentRefs[0].SectionName, + }, + }, + } + return normalRoute + } + getLastNormalRoute := func() *L4Route { + return normalRoute + } + + noMatchingParentAttachment := ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{}, + FailedCondition: staticConds.NewRouteNoMatchingParent(), + } + + notAttachableRoute := &L4Route{ + Source: tr, + Spec: L4RouteSpec{ + Hostnames: tr.Spec.Hostnames, + }, + Valid: true, + ParentRefs: []ParentRef{ + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + SectionName: tr.Spec.ParentRefs[0].SectionName, + }, + }, + } + notAttachableRoutePort := &L4Route{ + Source: tr, + Spec: L4RouteSpec{ + Hostnames: tr.Spec.Hostnames, + }, + Valid: true, + ParentRefs: []ParentRef{ + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + SectionName: tr.Spec.ParentRefs[0].SectionName, + Port: helpers.GetPointer[gatewayv1.PortNumber](80), + }, + }, + Attachable: true, + } + routeReferencesWrongNamespace := &L4Route{ + Source: tr, + Spec: L4RouteSpec{ + Hostnames: tr.Spec.Hostnames, + }, + Valid: true, + ParentRefs: []ParentRef{ + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gwWrongNamespace), + SectionName: tr.Spec.ParentRefs[0].SectionName, + }, + }, + Attachable: true, + } + + tests := []struct { + route *L4Route + gateway *Gateway + portHostnamesMap map[string]struct{} + expectedGatewayListeners []*Listener + name string + expectedSectionNameRefs []ParentRef + expectedConditions []conditions.Condition + }{ + { + route: createNormalRoute(gw), + gateway: &Gateway{ + Source: gw, + Valid: true, + Listeners: []*Listener{ + createListener("listener-443"), + }, + }, + expectedSectionNameRefs: []ParentRef{ + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + SectionName: tr.Spec.ParentRefs[0].SectionName, + Attachment: &ParentRefAttachmentStatus{ + Attached: true, + AcceptedHostnames: map[string][]string{ + "listener-443": {"foo.example.com"}, + }, + }, + }, + }, + expectedGatewayListeners: []*Listener{ + createModifiedListener("listener-443", func(l *Listener) { + l.L4Routes = map[L4RouteKey]*L4Route{ + CreateRouteKeyL4(tr): getLastNormalRoute(), + } + }), + }, + name: "normal case", + }, + { + route: notAttachableRoute, + gateway: &Gateway{ + Source: gw, + Valid: true, + Listeners: []*Listener{ + createListener("listener-443"), + }, + }, + expectedSectionNameRefs: []ParentRef{ + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + SectionName: tr.Spec.ParentRefs[0].SectionName, + }, + }, + expectedGatewayListeners: []*Listener{ + createListener("listener-443"), + }, + name: "route is not attachable", + }, + { + route: createNormalRoute(gw), + gateway: &Gateway{ + Source: gw, + Valid: true, + Listeners: []*Listener{ + createListener("listener-444"), + }, + }, + expectedSectionNameRefs: []ParentRef{ + { + Attachment: &noMatchingParentAttachment, + SectionName: tr.Spec.ParentRefs[0].SectionName, + Gateway: client.ObjectKeyFromObject(gw), + Idx: 0, + }, + }, + expectedGatewayListeners: []*Listener{ + createListener("listener-444"), + }, + name: "section name is wrong", + }, + { + route: notAttachableRoutePort, + gateway: &Gateway{ + Source: gw, + Valid: true, + Listeners: []*Listener{ + createListener("listener-443"), + }, + }, + expectedSectionNameRefs: []ParentRef{ + { + Attachment: &ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{}, + FailedCondition: conditions.Condition{ + Type: "Accepted", + Status: "False", + Reason: "UnsupportedValue", + Message: "spec.parentRefs[0].port: Forbidden: cannot be set", + }, + Attached: false, + }, + SectionName: tr.Spec.ParentRefs[0].SectionName, + Gateway: client.ObjectKeyFromObject(gw), + Idx: 0, + Port: helpers.GetPointer[gatewayv1.PortNumber](80), + }, + }, + expectedGatewayListeners: []*Listener{ + createListener("listener-443"), + }, + name: "port is not nil", + }, + { + route: routeReferencesWrongNamespace, + gateway: &Gateway{ + Source: gw, + Valid: true, + Listeners: []*Listener{ + createListener("listener-443"), + }, + }, + expectedSectionNameRefs: []ParentRef{ + { + Attachment: &ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{}, + FailedCondition: conditions.Condition{ + Type: "Accepted", + Status: "False", + Reason: "GatewayIgnored", + Message: "The Gateway is ignored by the controller", + }, + Attached: false, + }, + SectionName: tr.Spec.ParentRefs[0].SectionName, + Gateway: client.ObjectKeyFromObject(gwWrongNamespace), + Idx: 0, + }, + }, + expectedGatewayListeners: []*Listener{ + createListener("listener-443"), + }, + name: "ignored gateway", + }, + { + route: createNormalRoute(gw), + gateway: &Gateway{ + Source: gw, + Valid: false, + Listeners: []*Listener{ + createListener("listener-443"), + }, + }, + expectedSectionNameRefs: []ParentRef{ + { + Attachment: &ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{}, + FailedCondition: conditions.Condition{ + Type: "Accepted", + Status: "False", + Reason: "InvalidGateway", + Message: "Gateway is invalid", + }, + Attached: false, + }, + SectionName: tr.Spec.ParentRefs[0].SectionName, + Gateway: client.ObjectKeyFromObject(gw), + Idx: 0, + }, + }, + expectedGatewayListeners: []*Listener{ + createListener("listener-443"), + }, + name: "invalid gateway", + }, + { + route: createNormalRoute(gwWrongNamespace), + gateway: &Gateway{ + Source: gwWrongNamespace, + Valid: true, + Listeners: []*Listener{ + createModifiedListener("listener-443", func(l *Listener) { + l.Source.AllowedRoutes = &gatewayv1.AllowedRoutes{ + Namespaces: &gatewayv1.RouteNamespaces{From: helpers.GetPointer( + gatewayv1.FromNamespaces("Same"), + )}, + } + }), + }, + }, + expectedSectionNameRefs: []ParentRef{ + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gwWrongNamespace), + SectionName: tr.Spec.ParentRefs[0].SectionName, + Attachment: &ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{}, + FailedCondition: conditions.Condition{ + Type: "Accepted", + Status: "False", + Reason: "NotAllowedByListeners", + Message: "Route is not allowed by any listener", + }, + }, + }, + }, + expectedGatewayListeners: []*Listener{ + createModifiedListener("listener-443", func(l *Listener) { + l.Source.AllowedRoutes = &gatewayv1.AllowedRoutes{ + Namespaces: &gatewayv1.RouteNamespaces{From: helpers.GetPointer( + gatewayv1.FromNamespaces("Same"), + )}, + } + }), + }, + name: "route not allowed by listener", + }, + { + route: createNormalRoute(gw), + gateway: &Gateway{ + Source: gw, + Valid: true, + Listeners: []*Listener{ + createModifiedListener("listener-443", func(l *Listener) { + l.Valid = false + }), + }, + }, + expectedSectionNameRefs: []ParentRef{ + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + SectionName: tr.Spec.ParentRefs[0].SectionName, + Attachment: &ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{ + "listener-443": {"foo.example.com"}, + }, + Attached: true, + }, + }, + }, + expectedGatewayListeners: []*Listener{ + createModifiedListener("listener-443", func(l *Listener) { + l.Valid = false + r := createNormalRoute(gw) + r.Conditions = append(r.Conditions, staticConds.NewRouteInvalidListener()) + r.ParentRefs = []ParentRef{ + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + SectionName: tr.Spec.ParentRefs[0].SectionName, + Attachment: &ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{ + "listener-443": {"foo.example.com"}, + }, + Attached: true, + }, + }, + } + l.L4Routes = map[L4RouteKey]*L4Route{ + CreateRouteKeyL4(tr): r, + } + }), + }, + expectedConditions: []conditions.Condition{staticConds.NewRouteInvalidListener()}, + name: "invalid listener", + }, + { + route: createNormalRoute(gw), + gateway: &Gateway{ + Source: gw, + Valid: true, + Listeners: []*Listener{ + createModifiedListener("listener-443", func(l *Listener) { + l.Source.Hostname = (*gatewayv1.Hostname)(helpers.GetPointer("*.example.org")) + helpers.GetPointer("*.example.com") + }), + }, + }, + expectedSectionNameRefs: []ParentRef{ + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + SectionName: tr.Spec.ParentRefs[0].SectionName, + Attachment: &ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{}, + FailedCondition: staticConds.NewRouteNoMatchingListenerHostname(), + }, + }, + }, + expectedGatewayListeners: []*Listener{ + createModifiedListener("listener-443", func(l *Listener) { + l.Source.Hostname = (*gatewayv1.Hostname)(helpers.GetPointer("*.example.org")) + helpers.GetPointer("*.example.com") + }), + }, + name: "hostname does not match", + }, + } + + namespaces := map[types.NamespacedName]*v1.Namespace{ + {Name: "test"}: { + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Labels: map[string]string{"app": "allowed"}, + }, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewWithT(t) + + if test.portHostnamesMap == nil { + test.portHostnamesMap = map[string]struct{}{} + } + + bindL4RouteToListeners( + test.route, + test.gateway, + namespaces, + test.portHostnamesMap, + ) + + g.Expect(test.route.ParentRefs).To(Equal(test.expectedSectionNameRefs)) + g.Expect(helpers.Diff(test.gateway.Listeners, test.expectedGatewayListeners)).To(BeEmpty()) + g.Expect(helpers.Diff(test.route.Conditions, test.expectedConditions)).To(BeEmpty()) + }) + } +} + +func TestBuildL4RoutesForGateways(t *testing.T) { + g := NewWithT(t) + + g.Expect(buildL4RoutesForGateways(nil, nil, nil, nil)).To(BeNil()) +} + +func TestTryToAttachL4RouteToListeners(t *testing.T) { + g := NewWithT(t) + + cond, attachable := tryToAttachL4RouteToListeners( + nil, + nil, + nil, + nil, + nil, + nil, + ) + g.Expect(cond).To(Equal(staticConds.NewRouteInvalidListener())) + g.Expect(attachable).To(BeFalse()) +} diff --git a/internal/mode/static/state/graph/service.go b/internal/mode/static/state/graph/service.go index 08a5fe497c..ad33579f1e 100644 --- a/internal/mode/static/state/graph/service.go +++ b/internal/mode/static/state/graph/service.go @@ -5,27 +5,27 @@ import ( ) func buildReferencedServices( - routes map[RouteKey]*L7Route, + l7routes map[RouteKey]*L7Route, + l4Routes map[L4RouteKey]*L4Route, ) map[types.NamespacedName]struct{} { svcNames := make(map[types.NamespacedName]struct{}) - getServiceNamesFromRoute := func(parentRefs []ParentRef, routeRules []RouteRule) { - // If none of the ParentRefs are attached to the Gateway, we want to skip the route. - attached := false + attached := func(parentRefs []ParentRef) bool { for _, ref := range parentRefs { if ref.Attachment.Attached { - attached = true - break + return true } } - if !attached { - return - } + return false + } + + // Processes both valid and invalid BackendRefs as invalid ones still have referenced services + // we may want to track. + + populateServiceNamesForL7Routes := func(routeRules []RouteRule) { for _, rule := range routeRules { for _, ref := range rule.BackendRefs { - // Processes both valid and invalid BackendRefs as invalid ones still have referenced services - // we may want to track. if ref.SvcNsName != (types.NamespacedName{}) { svcNames[ref.SvcNsName] = struct{}{} } @@ -33,15 +33,40 @@ func buildReferencedServices( } } + populateServiceNamesForL4Routes := func(route *L4Route) { + nsname := route.Spec.BackendRef.SvcNsName + if nsname != (types.NamespacedName{}) { + svcNames[nsname] = struct{}{} + } + } + // routes all have populated ParentRefs from when they were created. // - // Get all the service names referenced from all the Routes. - for _, route := range routes { + // Get all the service names referenced from all the l7 and l4 routes. + for _, route := range l7routes { + if !route.Valid { + continue + } + + // If none of the ParentRefs are attached to the Gateway, we want to skip the route. + if !attached(route.ParentRefs) { + continue + } + + populateServiceNamesForL7Routes(route.Spec.Rules) + } + + for _, route := range l4Routes { if !route.Valid { continue } - getServiceNamesFromRoute(route.ParentRefs, route.Spec.Rules) + // If none of the ParentRefs are attached to the Gateway, we want to skip the route. + if !attached(route.ParentRefs) { + continue + } + + populateServiceNamesForL4Routes(route) } if len(svcNames) == 0 { diff --git a/internal/mode/static/state/graph/service_test.go b/internal/mode/static/state/graph/service_test.go index 73e4851947..3f233c84fd 100644 --- a/internal/mode/static/state/graph/service_test.go +++ b/internal/mode/static/state/graph/service_test.go @@ -8,148 +8,126 @@ import ( ) func TestBuildReferencedServices(t *testing.T) { - normalRoute := &L7Route{ - ParentRefs: []ParentRef{ - { - Attachment: &ParentRefAttachmentStatus{ - Attached: true, - }, - }, - }, - Valid: true, - Spec: L7RouteSpec{ - Rules: []RouteRule{ + getNormalL7Route := func() *L7Route { + return &L7Route{ + ParentRefs: []ParentRef{ { - BackendRefs: []BackendRef{ - { - SvcNsName: types.NamespacedName{Namespace: "banana-ns", Name: "service"}, - Weight: 1, - }, + Attachment: &ParentRefAttachmentStatus{ + Attached: true, }, - ValidMatches: true, - ValidFilters: true, - }, - }, - }, - RouteType: RouteTypeHTTP, - } - - validRouteTwoServicesOneRule := &L7Route{ - ParentRefs: []ParentRef{ - { - Attachment: &ParentRefAttachmentStatus{ - Attached: true, }, }, - }, - Valid: true, - Spec: L7RouteSpec{ - Rules: []RouteRule{ - { - BackendRefs: []BackendRef{ - { - SvcNsName: types.NamespacedName{Namespace: "service-ns", Name: "service"}, - Weight: 1, - }, - { - SvcNsName: types.NamespacedName{Namespace: "service-ns2", Name: "service2"}, - Weight: 1, + Valid: true, + Spec: L7RouteSpec{ + Rules: []RouteRule{ + { + BackendRefs: []BackendRef{ + { + SvcNsName: types.NamespacedName{Namespace: "banana-ns", Name: "service"}, + }, }, }, - ValidMatches: true, - ValidFilters: true, }, }, - }, + RouteType: RouteTypeHTTP, + } } - validRouteTwoServicesTwoRules := &L7Route{ - ParentRefs: []ParentRef{ - { - Attachment: &ParentRefAttachmentStatus{ - Attached: true, + getModifiedL7Route := func(mod func(route *L7Route) *L7Route) *L7Route { + return mod(getNormalL7Route()) + } + + getNormalL4Route := func() *L4Route { + return &L4Route{ + Spec: L4RouteSpec{ + BackendRef: BackendRef{ + SvcNsName: types.NamespacedName{Namespace: "tlsroute-ns", Name: "service"}, }, }, - }, - Valid: true, - Spec: L7RouteSpec{ - Rules: []RouteRule{ - { - BackendRefs: []BackendRef{ - { - SvcNsName: types.NamespacedName{Namespace: "service-ns", Name: "service"}, - Weight: 1, - }, - }, - ValidMatches: true, - ValidFilters: true, - }, + Valid: true, + ParentRefs: []ParentRef{ { - BackendRefs: []BackendRef{ - { - SvcNsName: types.NamespacedName{Namespace: "service-ns2", Name: "service2"}, - Weight: 1, - }, + Attachment: &ParentRefAttachmentStatus{ + Attached: true, }, - ValidMatches: true, - ValidFilters: true, }, }, - }, + } + } + + getModifiedL4Route := func(mod func(route *L4Route) *L4Route) *L4Route { + return mod(getNormalL4Route()) } - invalidRoute := &L7Route{ - ParentRefs: []ParentRef{ + normalRoute := getNormalL7Route() + normalL4Route := getNormalL4Route() + + validRouteTwoServicesOneRule := getModifiedL7Route(func(route *L7Route) *L7Route { + route.Spec.Rules[0].BackendRefs = []BackendRef{ { - Attachment: &ParentRefAttachmentStatus{ - Attached: true, - }, + SvcNsName: types.NamespacedName{Namespace: "service-ns", Name: "service"}, }, - }, - Valid: false, - Spec: L7RouteSpec{ - Rules: []RouteRule{ - { - BackendRefs: []BackendRef{ - { - SvcNsName: types.NamespacedName{Namespace: "service-ns", Name: "service"}, - Weight: 1, - }, - }, - ValidMatches: true, - ValidFilters: true, - }, + { + SvcNsName: types.NamespacedName{Namespace: "service-ns2", Name: "service2"}, }, - }, - } + } - unattachedRoute := &L7Route{ - ParentRefs: []ParentRef{ + return route + }) + + validRouteTwoServicesTwoRules := getModifiedL7Route(func(route *L7Route) *L7Route { + route.Spec.Rules = []RouteRule{ { - Attachment: &ParentRefAttachmentStatus{ - Attached: false, + BackendRefs: []BackendRef{ + { + SvcNsName: types.NamespacedName{Namespace: "service-ns", Name: "service"}, + }, }, }, - }, - Valid: true, - Spec: L7RouteSpec{ - Rules: []RouteRule{ - { - BackendRefs: []BackendRef{ - { - SvcNsName: types.NamespacedName{Namespace: "service-ns", Name: "service"}, - Weight: 1, - }, + { + BackendRefs: []BackendRef{ + { + SvcNsName: types.NamespacedName{Namespace: "service-ns2", Name: "service2"}, }, - ValidMatches: true, - ValidFilters: true, }, }, - }, - } + } + + return route + }) + + normalL4Route2 := getModifiedL4Route(func(route *L4Route) *L4Route { + route.Spec.BackendRef.SvcNsName = types.NamespacedName{Namespace: "tlsroute-ns", Name: "service2"} + return route + }) + + normalL4RouteWithSameSvcAsL7Route := getModifiedL4Route(func(route *L4Route) *L4Route { + route.Spec.BackendRef.SvcNsName = types.NamespacedName{Namespace: "service-ns", Name: "service"} + return route + }) + + invalidRoute := getModifiedL7Route(func(route *L7Route) *L7Route { + route.Valid = false + return route + }) - attachedRouteWithManyParentRefs := &L7Route{ - ParentRefs: []ParentRef{ + invalidL4Route := getModifiedL4Route(func(route *L4Route) *L4Route { + route.Valid = false + return route + }) + + unattachedRoute := getModifiedL7Route(func(route *L7Route) *L7Route { + route.ParentRefs[0].Attachment.Attached = false + return route + }) + + unattachedL4Route := getModifiedL4Route(func(route *L4Route) *L4Route { + route.ParentRefs[0].Attachment.Attached = false + return route + }) + + attachedRouteWithManyParentRefs := getModifiedL7Route(func(route *L7Route) *L7Route { + route.ParentRefs = []ParentRef{ { Attachment: &ParentRefAttachmentStatus{ Attached: false, @@ -165,64 +143,65 @@ func TestBuildReferencedServices(t *testing.T) { Attached: true, }, }, - }, - Valid: true, - Spec: L7RouteSpec{ - Rules: []RouteRule{ - { - BackendRefs: []BackendRef{ - { - SvcNsName: types.NamespacedName{Namespace: "service-ns", Name: "service"}, - Weight: 1, - }, - }, - ValidMatches: true, - ValidFilters: true, + } + + return route + }) + + attachedL4RoutesWithManyParentRefs := getModifiedL4Route(func(route *L4Route) *L4Route { + route.ParentRefs = []ParentRef{ + { + Attachment: &ParentRefAttachmentStatus{ + Attached: false, }, }, - }, - } - validRouteNoServiceNsName := &L7Route{ - ParentRefs: []ParentRef{ { Attachment: &ParentRefAttachmentStatus{ Attached: true, }, }, - }, - Valid: true, - Spec: L7RouteSpec{ - Rules: []RouteRule{ - { - BackendRefs: []BackendRef{ - { - Weight: 1, - }, - }, - ValidMatches: true, - ValidFilters: true, + { + Attachment: &ParentRefAttachmentStatus{ + Attached: false, }, }, - }, - } + } + + return route + }) + + validRouteNoServiceNsName := getModifiedL7Route(func(route *L7Route) *L7Route { + route.Spec.Rules[0].BackendRefs[0].SvcNsName = types.NamespacedName{} + return route + }) + + validL4RouteNoServiceNsName := getModifiedL4Route(func(route *L4Route) *L4Route { + route.Spec.BackendRef.SvcNsName = types.NamespacedName{} + return route + }) tests := []struct { - routes map[RouteKey]*L7Route - exp map[types.NamespacedName]struct{} - name string + l7Routes map[RouteKey]*L7Route + l4Routes map[L4RouteKey]*L4Route + exp map[types.NamespacedName]struct{} + name string }{ { - name: "normal route", - routes: map[RouteKey]*L7Route{ + name: "normal routes", + l7Routes: map[RouteKey]*L7Route{ {NamespacedName: types.NamespacedName{Name: "normal-route"}}: normalRoute, }, + l4Routes: map[L4RouteKey]*L4Route{ + {NamespacedName: types.NamespacedName{Name: "normal-l4-route"}}: normalL4Route, + }, exp: map[types.NamespacedName]struct{}{ - {Namespace: "banana-ns", Name: "service"}: {}, + {Namespace: "banana-ns", Name: "service"}: {}, + {Namespace: "tlsroute-ns", Name: "service"}: {}, }, }, { - name: "route with two services in one Rule", - routes: map[RouteKey]*L7Route{ + name: "l7 route with two services in one Rule", // l4 routes don't support multiple services right now + l7Routes: map[RouteKey]*L7Route{ {NamespacedName: types.NamespacedName{Name: "two-svc-one-rule"}}: validRouteTwoServicesOneRule, }, exp: map[types.NamespacedName]struct{}{ @@ -231,8 +210,8 @@ func TestBuildReferencedServices(t *testing.T) { }, }, { - name: "route with one service per rule", - routes: map[RouteKey]*L7Route{ + name: "route with one service per rule", // l4 routes don't support multiple rules right now + l7Routes: map[RouteKey]*L7Route{ {NamespacedName: types.NamespacedName{Name: "one-svc-per-rule"}}: validRouteTwoServicesTwoRules, }, exp: map[types.NamespacedName]struct{}{ @@ -241,66 +220,95 @@ func TestBuildReferencedServices(t *testing.T) { }, }, { - name: "two valid routes with same services", - routes: map[RouteKey]*L7Route{ + name: "multiple valid routes with same services", + l7Routes: map[RouteKey]*L7Route{ {NamespacedName: types.NamespacedName{Name: "one-svc-per-rule"}}: validRouteTwoServicesTwoRules, {NamespacedName: types.NamespacedName{Name: "two-svc-one-rule"}}: validRouteTwoServicesOneRule, }, + l4Routes: map[L4RouteKey]*L4Route{ + {NamespacedName: types.NamespacedName{Name: "l4-route-1"}}: normalL4Route, + {NamespacedName: types.NamespacedName{Name: "l4-route-2"}}: normalL4Route2, + {NamespacedName: types.NamespacedName{Name: "l4-route-same-svc-as-l7-route"}}: normalL4RouteWithSameSvcAsL7Route, + }, exp: map[types.NamespacedName]struct{}{ {Namespace: "service-ns", Name: "service"}: {}, {Namespace: "service-ns2", Name: "service2"}: {}, + {Namespace: "tlsroute-ns", Name: "service"}: {}, + {Namespace: "tlsroute-ns", Name: "service2"}: {}, }, }, { - name: "two valid routes with different services", - routes: map[RouteKey]*L7Route{ + name: "valid routes with different services", + l7Routes: map[RouteKey]*L7Route{ {NamespacedName: types.NamespacedName{Name: "one-svc-per-rule"}}: validRouteTwoServicesTwoRules, {NamespacedName: types.NamespacedName{Name: "normal-route"}}: normalRoute, }, + l4Routes: map[L4RouteKey]*L4Route{ + {NamespacedName: types.NamespacedName{Name: "normal-l4-route"}}: normalL4Route, + }, exp: map[types.NamespacedName]struct{}{ {Namespace: "service-ns", Name: "service"}: {}, {Namespace: "service-ns2", Name: "service2"}: {}, {Namespace: "banana-ns", Name: "service"}: {}, + {Namespace: "tlsroute-ns", Name: "service"}: {}, }, }, { name: "invalid routes", - routes: map[RouteKey]*L7Route{ + l7Routes: map[RouteKey]*L7Route{ {NamespacedName: types.NamespacedName{Name: "invalid-route"}}: invalidRoute, }, + l4Routes: map[L4RouteKey]*L4Route{ + {NamespacedName: types.NamespacedName{Name: "invalid-l4-route"}}: invalidL4Route, + }, exp: nil, }, { name: "unattached route", - routes: map[RouteKey]*L7Route{ + l7Routes: map[RouteKey]*L7Route{ {NamespacedName: types.NamespacedName{Name: "unattached-route"}}: unattachedRoute, }, + l4Routes: map[L4RouteKey]*L4Route{ + {NamespacedName: types.NamespacedName{Name: "unattached-l4-route"}}: unattachedL4Route, + }, exp: nil, }, { name: "combination of valid and invalid routes", - routes: map[RouteKey]*L7Route{ + l7Routes: map[RouteKey]*L7Route{ {NamespacedName: types.NamespacedName{Name: "normal-route"}}: normalRoute, {NamespacedName: types.NamespacedName{Name: "invalid-route"}}: invalidRoute, }, + l4Routes: map[L4RouteKey]*L4Route{ + {NamespacedName: types.NamespacedName{Name: "invalid-l4-route"}}: invalidL4Route, + {NamespacedName: types.NamespacedName{Name: "normal-l4-route"}}: normalL4Route, + }, exp: map[types.NamespacedName]struct{}{ - {Namespace: "banana-ns", Name: "service"}: {}, + {Namespace: "banana-ns", Name: "service"}: {}, + {Namespace: "tlsroute-ns", Name: "service"}: {}, }, }, { name: "route with many parentRefs and one is attached", - routes: map[RouteKey]*L7Route{ + l7Routes: map[RouteKey]*L7Route{ {NamespacedName: types.NamespacedName{Name: "multiple-parent-ref-route"}}: attachedRouteWithManyParentRefs, }, + l4Routes: map[L4RouteKey]*L4Route{ + {NamespacedName: types.NamespacedName{Name: "multiple-parent-ref-l4-route"}}: attachedL4RoutesWithManyParentRefs, + }, exp: map[types.NamespacedName]struct{}{ - {Namespace: "service-ns", Name: "service"}: {}, + {Namespace: "banana-ns", Name: "service"}: {}, + {Namespace: "tlsroute-ns", Name: "service"}: {}, }, }, { name: "valid route no service nsname", - routes: map[RouteKey]*L7Route{ + l7Routes: map[RouteKey]*L7Route{ {NamespacedName: types.NamespacedName{Name: "no-service-nsname"}}: validRouteNoServiceNsName, }, + l4Routes: map[L4RouteKey]*L4Route{ + {NamespacedName: types.NamespacedName{Name: "no-service-nsname-l4"}}: validL4RouteNoServiceNsName, + }, exp: nil, }, } @@ -308,7 +316,7 @@ func TestBuildReferencedServices(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { g := NewWithT(t) - g.Expect(buildReferencedServices(test.routes)).To(Equal(test.exp)) + g.Expect(buildReferencedServices(test.l7Routes, test.l4Routes)).To(Equal(test.exp)) }) } } diff --git a/internal/mode/static/state/graph/tlsroute.go b/internal/mode/static/state/graph/tlsroute.go new file mode 100644 index 0000000000..f4ca1d1e68 --- /dev/null +++ b/internal/mode/static/state/graph/tlsroute.go @@ -0,0 +1,133 @@ +package graph + +import ( + apiv1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/gateway-api/apis/v1alpha2" + + "github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions" + "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" + staticConds "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/conditions" +) + +func buildTLSRoute( + gtr *v1alpha2.TLSRoute, + gatewayNsNames []types.NamespacedName, + services map[types.NamespacedName]*apiv1.Service, + npCfg *NginxProxy, +) *L4Route { + r := &L4Route{ + Source: gtr, + } + + sectionNameRefs, err := buildSectionNameRefs(gtr.Spec.ParentRefs, gtr.Namespace, gatewayNsNames) + if err != nil { + r.Valid = false + + return r + } + // route doesn't belong to any of the Gateways + if len(sectionNameRefs) == 0 { + return nil + } + r.ParentRefs = sectionNameRefs + + if err := validateHostnames( + gtr.Spec.Hostnames, + field.NewPath("spec").Child("hostnames"), + ); err != nil { + r.Valid = false + r.Conditions = append(r.Conditions, staticConds.NewRouteUnsupportedValue(err.Error())) + return r + } + + r.Spec.Hostnames = gtr.Spec.Hostnames + + if len(gtr.Spec.Rules) != 1 || len(gtr.Spec.Rules[0].BackendRefs) != 1 { + r.Valid = false + cond := staticConds.NewRouteBackendRefUnsupportedValue( + "Must have exactly one Rule and BackendRef", + ) + r.Conditions = append(r.Conditions, cond) + return r + } + + cond, br := validateBackendRefTLSRoute(gtr, services, npCfg) + + r.Spec.BackendRef = br + r.Valid = true + r.Attachable = true + + if cond != nil { + r.Conditions = append(r.Conditions, *cond) + r.Valid = false + } + + return r +} + +func validateBackendRefTLSRoute(gtr *v1alpha2.TLSRoute, + services map[types.NamespacedName]*apiv1.Service, + npCfg *NginxProxy, +) (*conditions.Condition, BackendRef) { + // Length of BackendRefs and Rules is guaranteed to be one due to earlier check in buildTLSRoute + refPath := field.NewPath("spec").Child("rules").Index(0).Child("backendRefs").Index(0) + + ref := gtr.Spec.Rules[0].BackendRefs[0] + + ns := gtr.Namespace + if ref.Namespace != nil { + ns = string(*ref.Namespace) + } + + svcNsName := types.NamespacedName{ + Namespace: ns, + Name: string(gtr.Spec.Rules[0].BackendRefs[0].Name), + } + + backendRef := BackendRef{ + Valid: true, + } + var cond *conditions.Condition + + if ref.Port == nil { + valErr := field.Required(refPath.Child("port"), "port cannot be nil") + backendRef.Valid = false + cond = helpers.GetPointer(staticConds.NewRouteBackendRefUnsupportedValue(valErr.Error())) + + return cond, backendRef + } + + svcIPFamily, svcPort, err := getIPFamilyAndPortFromRef( + ref, + svcNsName, + services, + refPath, + ) + + backendRef.ServicePort = svcPort + backendRef.SvcNsName = svcNsName + + if err != nil { + backendRef.Valid = false + cond = helpers.GetPointer(staticConds.NewRouteBackendRefRefBackendNotFound(err.Error())) + } else if err := verifyIPFamily(npCfg, svcIPFamily); err != nil { + backendRef.Valid = false + cond = helpers.GetPointer(staticConds.NewRouteInvalidIPFamily(err.Error())) + } else if ref.Group != nil && !(*ref.Group == "core" || *ref.Group == "") { + valErr := field.NotSupported(refPath.Child("group"), *ref.Group, []string{"core", ""}) + backendRef.Valid = false + cond = helpers.GetPointer(staticConds.NewRouteBackendRefInvalidKind(valErr.Error())) + } else if ref.Kind != nil && *ref.Kind != "Service" { + valErr := field.NotSupported(refPath.Child("kind"), *ref.Kind, []string{"Service"}) + backendRef.Valid = false + cond = helpers.GetPointer(staticConds.NewRouteBackendRefInvalidKind(valErr.Error())) + } else if ref.Namespace != nil && string(*ref.Namespace) != gtr.Namespace { + msg := "Cross-namespace routing is not supported" + backendRef.Valid = false + cond = helpers.GetPointer(staticConds.NewRouteBackendRefUnsupportedValue(msg)) + } + // FIXME(sarthyparty): Add check for invalid weights, we removed checks to pass the conformance test + return cond, backendRef +} diff --git a/internal/mode/static/state/graph/tlsroute_test.go b/internal/mode/static/state/graph/tlsroute_test.go new file mode 100644 index 0000000000..74d76ddae3 --- /dev/null +++ b/internal/mode/static/state/graph/tlsroute_test.go @@ -0,0 +1,468 @@ +package graph + +import ( + "testing" + + . "github.com/onsi/gomega" + apiv1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + "sigs.k8s.io/gateway-api/apis/v1alpha2" + + ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" + "github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions" + "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" + staticConds "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/conditions" +) + +func createTLSRoute( + hostname gatewayv1.Hostname, + rules []v1alpha2.TLSRouteRule, + parentRefs []gatewayv1.ParentReference, +) *v1alpha2.TLSRoute { + return &v1alpha2.TLSRoute{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "tr", + }, + Spec: v1alpha2.TLSRouteSpec{ + CommonRouteSpec: gatewayv1.CommonRouteSpec{ + ParentRefs: parentRefs, + }, + Hostnames: []gatewayv1.Hostname{hostname}, + Rules: rules, + }, + } +} + +func TestBuildTLSRoute(t *testing.T) { + parentRef := gatewayv1.ParentReference{ + Namespace: helpers.GetPointer[gatewayv1.Namespace]("test"), + Name: "gateway", + SectionName: helpers.GetPointer[gatewayv1.SectionName]("l1"), + } + gatewayNsName := types.NamespacedName{ + Namespace: "test", + Name: "gateway", + } + parentRefGraph := ParentRef{ + SectionName: helpers.GetPointer[gatewayv1.SectionName]("l1"), + Gateway: gatewayNsName, + } + gtr1 := createTLSRoute( + "hi.example.com", + nil, + []gatewayv1.ParentReference{ + parentRef, + parentRef, + }, + ) + gtr2 := createTLSRoute( + "hi.example.com", + nil, + []gatewayv1.ParentReference{}, + ) + gtr3 := createTLSRoute( + "hi....com", + nil, + []gatewayv1.ParentReference{ + parentRef, + }, + ) + gtr4 := createTLSRoute( + "app.example.com", + nil, + []gatewayv1.ParentReference{ + parentRef, + }, + ) + gtr5 := createTLSRoute( + "app.example.com", + []v1alpha2.TLSRouteRule{ + { + BackendRefs: []gatewayv1.BackendRef{ + { + BackendObjectReference: gatewayv1.BackendObjectReference{ + Name: "hi", + Port: helpers.GetPointer[gatewayv1.PortNumber](80), + }, + }, + }, + }, + }, + []gatewayv1.ParentReference{ + parentRef, + }, + ) + + gtr6 := createTLSRoute( + "app.example.com", + []v1alpha2.TLSRouteRule{ + { + BackendRefs: []gatewayv1.BackendRef{ + { + BackendObjectReference: gatewayv1.BackendObjectReference{ + Name: "hi", + Port: helpers.GetPointer[gatewayv1.PortNumber](80), + Group: helpers.GetPointer[gatewayv1.Group]("wrong"), + }, + }, + }, + }, + }, + []gatewayv1.ParentReference{ + parentRef, + }, + ) + + gtr7 := createTLSRoute( + "app.example.com", + []v1alpha2.TLSRouteRule{ + { + BackendRefs: []gatewayv1.BackendRef{ + { + BackendObjectReference: gatewayv1.BackendObjectReference{ + Name: "hi", + Port: helpers.GetPointer[gatewayv1.PortNumber](80), + Kind: helpers.GetPointer[gatewayv1.Kind]("not service"), + }, + }, + }, + }, + }, + []gatewayv1.ParentReference{ + parentRef, + }, + ) + + gtr8 := createTLSRoute("app.example.com", + []v1alpha2.TLSRouteRule{ + { + BackendRefs: []gatewayv1.BackendRef{ + { + BackendObjectReference: gatewayv1.BackendObjectReference{ + Name: "hi", + Port: helpers.GetPointer[gatewayv1.PortNumber](80), + Namespace: helpers.GetPointer[gatewayv1.Namespace]("wrong"), + }, + }, + }, + }, + }, + []gatewayv1.ParentReference{ + parentRef, + }, + ) + + gtr9 := createTLSRoute("app.example.com", + []v1alpha2.TLSRouteRule{ + { + BackendRefs: []gatewayv1.BackendRef{ + { + BackendObjectReference: gatewayv1.BackendObjectReference{ + Name: "hi", + }, + }, + }, + }, + }, + []gatewayv1.ParentReference{ + parentRef, + }, + ) + + gtr10 := createTLSRoute( + "app.example.com", + []v1alpha2.TLSRouteRule{ + { + BackendRefs: []gatewayv1.BackendRef{ + { + BackendObjectReference: gatewayv1.BackendObjectReference{ + Name: "hi", + Port: helpers.GetPointer[gatewayv1.PortNumber](80), + }, + }, + }, + }, + }, + []gatewayv1.ParentReference{ + parentRef, + }, + ) + svcNsName := types.NamespacedName{ + Namespace: "test", + Name: "hi", + } + + svcNsNameWrong := types.NamespacedName{ + Namespace: "wrong", + Name: "hi", + } + + createSvc := func(name string, port int32) *apiv1.Service { + return &apiv1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: name, + }, + Spec: apiv1.ServiceSpec{ + Ports: []apiv1.ServicePort{ + {Port: port}, + }, + }, + } + } + + badNsSvc := &apiv1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "wrong", + Name: "hi", + }, + Spec: apiv1.ServiceSpec{ + Ports: []apiv1.ServicePort{ + {Port: 80}, + }, + }, + } + + ipv4Svc := &apiv1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "hi", + }, + Spec: apiv1.ServiceSpec{ + IPFamilies: []apiv1.IPFamily{ + apiv1.IPv4Protocol, + }, + Ports: []apiv1.ServicePort{ + {Port: 80}, + }, + }, + } + + tests := []struct { + expected *L4Route + gtr *v1alpha2.TLSRoute + services map[types.NamespacedName]*apiv1.Service + name string + gatewayNsNames []types.NamespacedName + npCfg NginxProxy + }{ + { + gtr: gtr1, + expected: &L4Route{Source: gtr1}, + gatewayNsNames: []types.NamespacedName{gatewayNsName}, + services: map[types.NamespacedName]*apiv1.Service{}, + name: "duplicate section names", + }, + { + gtr: gtr2, + expected: nil, + gatewayNsNames: []types.NamespacedName{gatewayNsName}, + services: map[types.NamespacedName]*apiv1.Service{}, + name: "no section names", + }, + { + gtr: gtr3, + expected: &L4Route{ + Source: gtr3, + ParentRefs: []ParentRef{parentRefGraph}, + Conditions: []conditions.Condition{staticConds.NewRouteUnsupportedValue( + "spec.hostnames[0]: Invalid value: \"hi....com\": a lowercase RFC 1" + + "123 subdomain must consist of lower case alphanumeric characters" + + ", '-' or '.', and must start and end with an alphanumeric charac" + + "ter (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])?)*')", + )}, + }, + gatewayNsNames: []types.NamespacedName{gatewayNsName}, + services: map[types.NamespacedName]*apiv1.Service{}, + name: "invalid hostname", + }, + { + gtr: gtr4, + expected: &L4Route{ + Source: gtr4, + ParentRefs: []ParentRef{parentRefGraph}, + Spec: L4RouteSpec{ + Hostnames: []gatewayv1.Hostname{ + "app.example.com", + }, + }, + Conditions: []conditions.Condition{staticConds.NewRouteBackendRefUnsupportedValue( + "Must have exactly one Rule and BackendRef", + )}, + }, + gatewayNsNames: []types.NamespacedName{gatewayNsName}, + services: map[types.NamespacedName]*apiv1.Service{}, + name: "invalid rule", + }, + { + gtr: gtr5, + expected: &L4Route{ + Source: gtr5, + ParentRefs: []ParentRef{parentRefGraph}, + Spec: L4RouteSpec{ + Hostnames: []gatewayv1.Hostname{ + "app.example.com", + }, + BackendRef: BackendRef{ + SvcNsName: types.NamespacedName{ + Namespace: "test", + Name: "hi", + }, + }, + }, + Conditions: []conditions.Condition{staticConds.NewRouteBackendRefRefBackendNotFound( + "spec.rules[0].backendRefs[0].name: Not found: \"hi\"", + )}, + Attachable: true, + }, + gatewayNsNames: []types.NamespacedName{gatewayNsName}, + services: map[types.NamespacedName]*apiv1.Service{}, + name: "BackendRef not found", + }, + { + gtr: gtr6, + expected: &L4Route{ + Source: gtr6, + ParentRefs: []ParentRef{parentRefGraph}, + Spec: L4RouteSpec{ + Hostnames: []gatewayv1.Hostname{ + "app.example.com", + }, + BackendRef: BackendRef{ + SvcNsName: svcNsName, + ServicePort: apiv1.ServicePort{Port: 80}, + }, + }, + Conditions: []conditions.Condition{staticConds.NewRouteBackendRefInvalidKind( + "spec.rules[0].backendRefs[0].group:" + + " Unsupported value: \"wrong\": supported values: \"core\", \"\"", + )}, + Attachable: true, + }, + gatewayNsNames: []types.NamespacedName{gatewayNsName}, + services: map[types.NamespacedName]*apiv1.Service{ + svcNsName: createSvc("hi", 80), + }, + name: "BackendRef group wrong", + }, + { + gtr: gtr7, + expected: &L4Route{ + Source: gtr7, + ParentRefs: []ParentRef{parentRefGraph}, + Spec: L4RouteSpec{ + Hostnames: []gatewayv1.Hostname{ + "app.example.com", + }, + BackendRef: BackendRef{ + SvcNsName: svcNsName, + ServicePort: apiv1.ServicePort{Port: 80}, + }, + }, + Conditions: []conditions.Condition{staticConds.NewRouteBackendRefInvalidKind( + "spec.rules[0].backendRefs[0].kind:" + + " Unsupported value: \"not service\": supported values: \"Service\"", + )}, + Attachable: true, + }, + gatewayNsNames: []types.NamespacedName{gatewayNsName}, + services: map[types.NamespacedName]*apiv1.Service{ + svcNsName: createSvc("hi", 80), + }, + name: "BackendRef kind wrong", + }, + { + gtr: gtr8, + expected: &L4Route{ + Source: gtr8, + ParentRefs: []ParentRef{parentRefGraph}, + Spec: L4RouteSpec{ + Hostnames: []gatewayv1.Hostname{ + "app.example.com", + }, + BackendRef: BackendRef{ + SvcNsName: svcNsNameWrong, + ServicePort: apiv1.ServicePort{Port: 80}, + }, + }, + Conditions: []conditions.Condition{staticConds.NewRouteBackendRefUnsupportedValue( + "Cross-namespace routing is not supported", + )}, + Attachable: true, + }, + gatewayNsNames: []types.NamespacedName{gatewayNsName}, + services: map[types.NamespacedName]*apiv1.Service{ + svcNsNameWrong: badNsSvc, + }, + name: "BackendRef namespace wrong", + }, + { + gtr: gtr9, + expected: &L4Route{ + Source: gtr9, + ParentRefs: []ParentRef{parentRefGraph}, + Spec: L4RouteSpec{ + Hostnames: []gatewayv1.Hostname{ + "app.example.com", + }, + BackendRef: BackendRef{}, + }, + Conditions: []conditions.Condition{staticConds.NewRouteBackendRefUnsupportedValue( + "spec.rules[0].backendRefs[0].port: Required value: port cannot be nil", + )}, + Attachable: true, + }, + gatewayNsNames: []types.NamespacedName{gatewayNsName}, + services: map[types.NamespacedName]*apiv1.Service{ + svcNsNameWrong: createSvc("hi", 80), + }, + name: "BackendRef port nil", + }, + { + gtr: gtr10, + expected: &L4Route{ + Source: gtr10, + ParentRefs: []ParentRef{parentRefGraph}, + Spec: L4RouteSpec{ + Hostnames: []gatewayv1.Hostname{ + "app.example.com", + }, + BackendRef: BackendRef{ + SvcNsName: svcNsName, + ServicePort: apiv1.ServicePort{Port: 80}, + }, + }, + Conditions: []conditions.Condition{staticConds.NewRouteInvalidIPFamily( + "Service configured with IPv4 family but NginxProxy is configured with IPv6", + )}, + Attachable: true, + }, + gatewayNsNames: []types.NamespacedName{gatewayNsName}, + services: map[types.NamespacedName]*apiv1.Service{ + svcNsName: ipv4Svc, + }, + name: "service and npcfg ip family mismatch", + npCfg: NginxProxy{ + Source: &ngfAPI.NginxProxy{Spec: ngfAPI.NginxProxySpec{IPFamily: helpers.GetPointer(ngfAPI.IPv6)}}, + Valid: true, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewWithT(t) + r := buildTLSRoute( + test.gtr, + test.gatewayNsNames, + test.services, + &test.npCfg, + ) + g.Expect(helpers.Diff(test.expected, r)).To(BeEmpty()) + }) + } +} diff --git a/internal/mode/static/status/prepare_requests.go b/internal/mode/static/status/prepare_requests.go index c8bde98cec..78d39327bf 100644 --- a/internal/mode/static/status/prepare_requests.go +++ b/internal/mode/static/status/prepare_requests.go @@ -27,6 +27,7 @@ type NginxReloadResult struct { // PrepareRouteRequests prepares status UpdateRequests for the given Routes. func PrepareRouteRequests( + l4routes map[graph.L4RouteKey]*graph.L4Route, routes map[graph.RouteKey]*graph.L7Route, transitionTime metav1.Time, nginxReloadRes NginxReloadResult, @@ -34,6 +35,29 @@ func PrepareRouteRequests( ) []frameworkStatus.UpdateRequest { reqs := make([]frameworkStatus.UpdateRequest, 0, len(routes)) + for routeKey, r := range l4routes { + routeStatus := prepareRouteStatus( + gatewayCtlrName, + r.ParentRefs, + r.Conditions, + nginxReloadRes, + transitionTime, + r.Source.GetGeneration(), + ) + + status := v1alpha2.TLSRouteStatus{ + RouteStatus: routeStatus, + } + + req := frameworkStatus.UpdateRequest{ + NsName: routeKey.NamespacedName, + ResourceType: &v1alpha2.TLSRoute{}, + Setter: newTLSRouteStatusSetter(status, gatewayCtlrName), + } + + reqs = append(reqs, req) + } + for routeKey, r := range routes { routeStatus := prepareRouteStatus( gatewayCtlrName, @@ -260,7 +284,7 @@ func prepareGatewayRequest( listenerStatuses = append(listenerStatuses, v1.ListenerStatus{ Name: v1.SectionName(l.Name), SupportedKinds: l.SupportedKinds, - AttachedRoutes: int32(len(l.Routes)), + AttachedRoutes: int32(len(l.Routes)) + int32(len(l.L4Routes)), Conditions: apiConds, }) } diff --git a/internal/mode/static/status/prepare_requests_test.go b/internal/mode/static/status/prepare_requests_test.go index c0629a18f1..3e98e777c1 100644 --- a/internal/mode/static/status/prepare_requests_test.go +++ b/internal/mode/static/status/prepare_requests_test.go @@ -33,6 +33,7 @@ func createK8sClientFor(resourceType ngftypes.ObjectType) client.Client { // for simplicity, we add all used schemes here utilruntime.Must(v1.Install(scheme)) + utilruntime.Must(v1alpha2.Install(scheme)) utilruntime.Must(v1alpha3.Install(scheme)) utilruntime.Must(ngfAPI.AddToScheme(scheme)) @@ -221,6 +222,7 @@ func TestBuildHTTPRouteStatuses(t *testing.T) { CommonRouteSpec: commonRouteSpecValid, }, } + hrInvalid := &v1.HTTPRoute{ ObjectMeta: metav1.ObjectMeta{ Namespace: "test", @@ -267,7 +269,13 @@ func TestBuildHTTPRouteStatuses(t *testing.T) { updater := statusFramework.NewUpdater(k8sClient, zap.New()) - reqs := PrepareRouteRequests(routes, transitionTime, NginxReloadResult{}, gatewayCtlrName) + reqs := PrepareRouteRequests( + map[graph.L4RouteKey]*graph.L4Route{}, + routes, + transitionTime, + NginxReloadResult{}, + gatewayCtlrName, + ) updater.Update(context.Background(), reqs...) @@ -339,7 +347,13 @@ func TestBuildGRPCRouteStatuses(t *testing.T) { updater := statusFramework.NewUpdater(k8sClient, zap.New()) - reqs := PrepareRouteRequests(routes, transitionTime, NginxReloadResult{}, gatewayCtlrName) + reqs := PrepareRouteRequests( + map[graph.L4RouteKey]*graph.L4Route{}, + routes, + transitionTime, + NginxReloadResult{}, + gatewayCtlrName, + ) updater.Update(context.Background(), reqs...) @@ -354,6 +368,82 @@ func TestBuildGRPCRouteStatuses(t *testing.T) { } } +func TestBuildTLSRouteStatuses(t *testing.T) { + trValid := &v1alpha2.TLSRoute{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "tr-valid", + Generation: 3, + }, + Spec: v1alpha2.TLSRouteSpec{ + CommonRouteSpec: commonRouteSpecValid, + }, + } + trInvalid := &v1alpha2.TLSRoute{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "tr-invalid", + Generation: 3, + }, + Spec: v1alpha2.TLSRouteSpec{ + CommonRouteSpec: commonRouteSpecInvalid, + }, + } + routes := map[graph.L4RouteKey]*graph.L4Route{ + graph.CreateRouteKeyL4(trValid): { + Valid: true, + Source: trValid, + ParentRefs: parentRefsValid, + }, + graph.CreateRouteKeyL4(trInvalid): { + Valid: false, + Conditions: []conditions.Condition{invalidRouteCondition}, + Source: trInvalid, + ParentRefs: parentRefsInvalid, + }, + } + + expectedStatuses := map[types.NamespacedName]v1alpha2.TLSRouteStatus{ + {Namespace: "test", Name: "tr-valid"}: { + RouteStatus: routeStatusValid, + }, + {Namespace: "test", Name: "tr-invalid"}: { + RouteStatus: routeStatusInvalid, + }, + } + + g := NewWithT(t) + + k8sClient := createK8sClientFor(&v1alpha2.TLSRoute{}) + + for _, r := range routes { + err := k8sClient.Create(context.Background(), r.Source) + g.Expect(err).ToNot(HaveOccurred()) + } + + updater := statusFramework.NewUpdater(k8sClient, zap.New()) + + reqs := PrepareRouteRequests( + routes, + map[graph.RouteKey]*graph.L7Route{}, + transitionTime, + NginxReloadResult{}, + gatewayCtlrName, + ) + + updater.Update(context.Background(), reqs...) + + g.Expect(reqs).To(HaveLen(len(expectedStatuses))) + + for nsname, expected := range expectedStatuses { + var hr v1alpha2.TLSRoute + + err := k8sClient.Get(context.Background(), nsname, &hr) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(helpers.Diff(expected, hr.Status)).To(BeEmpty()) + } +} + func TestBuildRouteStatusesNginxErr(t *testing.T) { const gatewayCtlrName = "controller" @@ -437,6 +527,7 @@ func TestBuildRouteStatusesNginxErr(t *testing.T) { updater := statusFramework.NewUpdater(k8sClient, zap.New()) reqs := PrepareRouteRequests( + map[graph.L4RouteKey]*graph.L4Route{}, routes, transitionTime, NginxReloadResult{Error: errors.New("test error")}, diff --git a/internal/mode/static/status/status_setters.go b/internal/mode/static/status/status_setters.go index 7a23f56f73..44fab45255 100644 --- a/internal/mode/static/status/status_setters.go +++ b/internal/mode/static/status/status_setters.go @@ -101,6 +101,27 @@ func newHTTPRouteStatusSetter(status gatewayv1.HTTPRouteStatus, gatewayCtlrName } } +func newTLSRouteStatusSetter(status v1alpha2.TLSRouteStatus, gatewayCtlrName string) frameworkStatus.Setter { + return func(object client.Object) (wasSet bool) { + tr := object.(*v1alpha2.TLSRoute) + + // keep all the parent statuses that belong to other controllers + for _, os := range tr.Status.Parents { + if string(os.ControllerName) != gatewayCtlrName { + status.Parents = append(status.Parents, os) + } + } + + if routeStatusEqual(gatewayCtlrName, tr.Status.Parents, status.Parents) { + return false + } + + tr.Status = status + + return true + } +} + func newGRPCRouteStatusSetter(status gatewayv1.GRPCRouteStatus, gatewayCtlrName string) frameworkStatus.Setter { return func(object client.Object) (wasSet bool) { gr := object.(*gatewayv1.GRPCRoute) diff --git a/internal/mode/static/status/status_setters_test.go b/internal/mode/static/status/status_setters_test.go index a7b838342d..8e4282e8f4 100644 --- a/internal/mode/static/status/status_setters_test.go +++ b/internal/mode/static/status/status_setters_test.go @@ -477,6 +477,181 @@ func TestNewGRPCRouteStatusSetter(t *testing.T) { } } +func TestNewTLSRouteStatusSetter(t *testing.T) { + const ( + controllerName = "controller" + otherControllerName = "different" + ) + + tests := []struct { + name string + status, newStatus, expStatus v1alpha2.TLSRouteStatus + expStatusSet bool + }{ + { + name: "TLSRoute has no status", + newStatus: v1alpha2.TLSRouteStatus{ + RouteStatus: v1alpha2.RouteStatus{ + Parents: []v1alpha2.RouteParentStatus{ + { + ParentRef: gatewayv1.ParentReference{}, + ControllerName: gatewayv1.GatewayController(controllerName), + Conditions: []metav1.Condition{{Message: "new condition"}}, + }, + }, + }, + }, + expStatus: v1alpha2.TLSRouteStatus{ + RouteStatus: gatewayv1.RouteStatus{ + Parents: []gatewayv1.RouteParentStatus{ + { + ParentRef: gatewayv1.ParentReference{}, + ControllerName: gatewayv1.GatewayController(controllerName), + Conditions: []metav1.Condition{{Message: "new condition"}}, + }, + }, + }, + }, + expStatusSet: true, + }, + { + name: "TLSRoute has old status", + newStatus: v1alpha2.TLSRouteStatus{ + RouteStatus: gatewayv1.RouteStatus{ + Parents: []gatewayv1.RouteParentStatus{ + { + ParentRef: gatewayv1.ParentReference{}, + ControllerName: gatewayv1.GatewayController(controllerName), + Conditions: []metav1.Condition{{Message: "new condition"}}, + }, + }, + }, + }, + status: v1alpha2.TLSRouteStatus{ + RouteStatus: gatewayv1.RouteStatus{ + Parents: []gatewayv1.RouteParentStatus{ + { + ParentRef: gatewayv1.ParentReference{}, + ControllerName: gatewayv1.GatewayController(controllerName), + Conditions: []metav1.Condition{{Message: "old condition"}}, + }, + }, + }, + }, + expStatus: v1alpha2.TLSRouteStatus{ + RouteStatus: gatewayv1.RouteStatus{ + Parents: []gatewayv1.RouteParentStatus{ + { + ParentRef: gatewayv1.ParentReference{}, + ControllerName: gatewayv1.GatewayController(controllerName), + Conditions: []metav1.Condition{{Message: "new condition"}}, + }, + }, + }, + }, + expStatusSet: true, + }, + { + name: "TLSRoute has old status, keep other controller statuses", + newStatus: v1alpha2.TLSRouteStatus{ + RouteStatus: gatewayv1.RouteStatus{ + Parents: []gatewayv1.RouteParentStatus{ + { + ParentRef: gatewayv1.ParentReference{}, + ControllerName: gatewayv1.GatewayController(controllerName), + Conditions: []metav1.Condition{{Message: "new condition"}}, + }, + }, + }, + }, + status: v1alpha2.TLSRouteStatus{ + RouteStatus: gatewayv1.RouteStatus{ + Parents: []gatewayv1.RouteParentStatus{ + { + ParentRef: gatewayv1.ParentReference{}, + ControllerName: gatewayv1.GatewayController(otherControllerName), + Conditions: []metav1.Condition{{Message: "some condition"}}, + }, + { + ParentRef: gatewayv1.ParentReference{}, + ControllerName: gatewayv1.GatewayController(controllerName), + Conditions: []metav1.Condition{{Message: "old condition"}}, + }, + }, + }, + }, + expStatus: v1alpha2.TLSRouteStatus{ + RouteStatus: gatewayv1.RouteStatus{ + Parents: []gatewayv1.RouteParentStatus{ + { + ParentRef: gatewayv1.ParentReference{}, + ControllerName: gatewayv1.GatewayController(controllerName), + Conditions: []metav1.Condition{{Message: "new condition"}}, + }, + { + ParentRef: gatewayv1.ParentReference{}, + ControllerName: gatewayv1.GatewayController(otherControllerName), + Conditions: []metav1.Condition{{Message: "some condition"}}, + }, + }, + }, + }, + expStatusSet: true, + }, + { + name: "TLSRoute has same status", + newStatus: v1alpha2.TLSRouteStatus{ + RouteStatus: gatewayv1.RouteStatus{ + Parents: []gatewayv1.RouteParentStatus{ + { + ParentRef: gatewayv1.ParentReference{}, + ControllerName: gatewayv1.GatewayController(controllerName), + Conditions: []metav1.Condition{{Message: "same condition"}}, + }, + }, + }, + }, + status: v1alpha2.TLSRouteStatus{ + RouteStatus: gatewayv1.RouteStatus{ + Parents: []gatewayv1.RouteParentStatus{ + { + ParentRef: gatewayv1.ParentReference{}, + ControllerName: gatewayv1.GatewayController(controllerName), + Conditions: []metav1.Condition{{Message: "same condition"}}, + }, + }, + }, + }, + expStatus: v1alpha2.TLSRouteStatus{ + RouteStatus: gatewayv1.RouteStatus{ + Parents: []gatewayv1.RouteParentStatus{ + { + ParentRef: gatewayv1.ParentReference{}, + ControllerName: gatewayv1.GatewayController(controllerName), + Conditions: []metav1.Condition{{Message: "same condition"}}, + }, + }, + }, + }, + expStatusSet: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewWithT(t) + + setter := newTLSRouteStatusSetter(test.newStatus, controllerName) + obj := &v1alpha2.TLSRoute{Status: test.status} + + statusSet := setter(obj) + + g.Expect(statusSet).To(Equal(test.expStatusSet)) + g.Expect(obj.Status).To(Equal(test.expStatus)) + }) + } +} + func TestNewGatewayClassStatusSetter(t *testing.T) { tests := []struct { name string diff --git a/tests/Makefile b/tests/Makefile index b75c2050de..0c92721fad 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -12,7 +12,7 @@ GW_SVC_GKE_INTERNAL = false NGF_VERSION ?= $(shell git describe --tags $(shell git rev-list --tags --max-count=1))## NGF version to be tested (defaults to latest tag) PULL_POLICY = Never## Pull policy for the images PROVISIONER_MANIFEST = conformance/provisioner/provisioner.yaml -SUPPORTED_FEATURES = HTTPRouteQueryParamMatching,HTTPRouteMethodMatching,HTTPRoutePortRedirect,HTTPRouteSchemeRedirect,HTTPRouteHostRewrite,HTTPRoutePathRewrite,GatewayPort8080,HTTPRouteResponseHeaderModification,GRPCExactMethodMatching,GRPCRouteListenerHostnameMatching,GRPCRouteHeaderMatching +SUPPORTED_FEATURES = HTTPRouteQueryParamMatching,HTTPRouteMethodMatching,HTTPRoutePortRedirect,HTTPRouteSchemeRedirect,HTTPRouteHostRewrite,HTTPRoutePathRewrite,GatewayPort8080,HTTPRouteResponseHeaderModification,GRPCExactMethodMatching,GRPCRouteListenerHostnameMatching,GRPCRouteHeaderMatching,TLSRoute ifneq ($(GINKGO_LABEL),) override GINKGO_FLAGS += --label-filter "$(GINKGO_LABEL)" @@ -35,7 +35,7 @@ run-conformance-tests: ## Run conformance tests --image=$(CONFORMANCE_PREFIX):$(CONFORMANCE_TAG) --image-pull-policy=Never \ --overrides='{ "spec": { "serviceAccountName": "conformance" } }' \ --restart=Never -- sh -c "go test -v . -tags conformance,experimental -args --gateway-class=$(GATEWAY_CLASS) \ - --supported-features=$(SUPPORTED_FEATURES) --version=$(NGF_VERSION) \ + --supported-features=$(SUPPORTED_FEATURES) --version=$(NGF_VERSION) --skip-tests=TLSRouteInvalidReferenceGrant \ --report-output=output.txt; cat output.txt" | tee output.txt ./scripts/check-pod-exit-code.sh sed -e '1,/CONFORMANCE PROFILE/d' output.txt > conformance-profile.yaml @@ -43,7 +43,8 @@ run-conformance-tests: ## Run conformance tests grpc_core_result=`yq '.profiles[0].core.result' conformance-profile.yaml`; \ http_core_result=`yq '.profiles[1].core.result' conformance-profile.yaml`; \ http_extended_result=`yq '.profiles[1].extended.result' conformance-profile.yaml`; \ - if [ "$$grpc_core_result" != "failure" ] && [ "$$http_core_result" != "failure" ] && [ "$$http_extended_result" != "failure" ] ; then \ + tls_core_result=`yq '.profiles[2].core.result' conformance-profile.yaml`; \ + if [ "$$grpc_core_result" != "failure" ] && [ "$$http_core_result" != "failure" ] && [ "$$http_extended_result" != "failure" ] && [ "$$tls_core_result" != "failure" ]; then \ exit 0; \ else \ exit 2; \ diff --git a/tests/conformance/conformance-rbac.yaml b/tests/conformance/conformance-rbac.yaml index 1c7db22f30..c1c0d54185 100644 --- a/tests/conformance/conformance-rbac.yaml +++ b/tests/conformance/conformance-rbac.yaml @@ -39,6 +39,7 @@ rules: - grpcroutes - referencegrants - gatewayclasses + - tlsroutes verbs: - create - delete diff --git a/tests/conformance/conformance_test.go b/tests/conformance/conformance_test.go index 2bf8739984..54acfe5dbc 100644 --- a/tests/conformance/conformance_test.go +++ b/tests/conformance/conformance_test.go @@ -49,7 +49,11 @@ func TestConformance(t *testing.T) { "https://github.com/nginxinc/nginx-gateway-fabric/discussions/new/choose", }, } - opts.ConformanceProfiles = sets.New(suite.GatewayHTTPConformanceProfileName, suite.GatewayGRPCConformanceProfileName) + opts.ConformanceProfiles = sets.New( + suite.GatewayHTTPConformanceProfileName, + suite.GatewayGRPCConformanceProfileName, + suite.GatewayTLSConformanceProfileName, + ) testSuite, err := suite.NewConformanceTestSuite(opts) g.Expect(err).To(Not(HaveOccurred()))