From fc6331ba79e546e403f9c05e2aa7caf4ee0c0684 Mon Sep 17 00:00:00 2001 From: Mile <zedsprogramms@gmail.com> Date: Fri, 9 Aug 2024 10:36:04 -0600 Subject: [PATCH] Add support for TLS Passthrough using TLSRoutes (#2356) Problem: As a user of NKG, I want to enable TLS Passthrough for my application's endpoints, so that I can achieve end-to-end encryption for my incoming traffic, and so that I do not have to manage certificates at the Gateway. Solution: Allow users to configure TLS Passthrough for their apps using TLSRoute. Adds basic support for TLSRoute. Cross-namespace routing via ReferenceGrants, traffic splitting, and TLS termination use case will be added in a future release. Note that the stream conf volume are always enabled in the deployment.yaml because our nginx conf reads from it. If the file did not exist then nginx will error. --- .github/workflows/conformance.yml | 2 + .../templates/clusterrole.yaml | 2 + .../templates/deployment.yaml | 6 + config/tests/static-deployment.yaml | 6 + deploy/aws-nlb/deploy.yaml | 6 + deploy/azure/deploy.yaml | 6 + deploy/default/deploy.yaml | 6 + deploy/experimental-nginx-plus/deploy.yaml | 8 + deploy/experimental/deploy.yaml | 8 + deploy/nginx-plus/deploy.yaml | 6 + deploy/nodeport/deploy.yaml | 6 + deploy/openshift/deploy.yaml | 6 + 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 + .../mode/static/nginx/conf/nginx-plus.conf | 14 + internal/mode/static/nginx/conf/nginx.conf | 14 + .../nginx/config/base_http_config_template.go | 21 + .../nginx/config/base_http_config_test.go | 3 + .../mode/static/nginx/config/generator.go | 11 +- .../static/nginx/config/generator_test.go | 31 +- .../mode/static/nginx/config/http/config.go | 26 +- internal/mode/static/nginx/config/maps.go | 117 ++- .../mode/static/nginx/config/maps_template.go | 30 +- .../mode/static/nginx/config/maps_test.go | 204 +++++- internal/mode/static/nginx/config/servers.go | 36 +- .../static/nginx/config/servers_template.go | 24 +- .../mode/static/nginx/config/servers_test.go | 83 ++- .../mode/static/nginx/config/shared/config.go | 21 + internal/mode/static/nginx/config/sockets.go | 17 + .../mode/static/nginx/config/sockets_test.go | 28 + .../mode/static/nginx/config/stream/config.go | 30 + .../static/nginx/config/stream_servers.go | 69 ++ .../nginx/config/stream_servers_template.go | 28 + .../nginx/config/stream_servers_test.go | 263 +++++++ .../mode/static/nginx/config/upstreams.go | 52 ++ .../static/nginx/config/upstreams_template.go | 5 +- .../static/nginx/config/upstreams_test.go | 189 +++++ .../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 | 162 +++- .../state/dataplane/configuration_test.go | 407 ++++++++++- internal/mode/static/state/dataplane/types.go | 18 +- .../mode/static/state/graph/backend_refs.go | 2 +- .../static/state/graph/gateway_listener.go | 160 +++- .../state/graph/gateway_listener_test.go | 150 ++++ .../mode/static/state/graph/gateway_test.go | 198 ++++- internal/mode/static/state/graph/graph.go | 15 +- .../mode/static/state/graph/graph_test.go | 180 ++++- .../mode/static/state/graph/route_common.go | 322 +++++++- .../static/state/graph/route_common_test.go | 690 +++++++++++++++++- 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 | 14 +- tests/conformance/conformance-rbac.yaml | 1 + tests/conformance/conformance_test.go | 9 +- 65 files changed, 4678 insertions(+), 406 deletions(-) create mode 100644 internal/mode/static/nginx/config/shared/config.go create mode 100644 internal/mode/static/nginx/config/sockets.go create mode 100644 internal/mode/static/nginx/config/sockets_test.go create mode 100644 internal/mode/static/nginx/config/stream/config.go create mode 100644 internal/mode/static/nginx/config/stream_servers.go create mode 100644 internal/mode/static/nginx/config/stream_servers_template.go create mode 100644 internal/mode/static/nginx/config/stream_servers_test.go 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 cd89ca6c42..c9a72cb257 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 @@ -146,6 +147,7 @@ jobs: - name: Run conformance tests run: | + if [ ${{ inputs.enable-experimental }} == "true" ]; then export ENABLE_EXPERIMENTAL=true; fi make run-conformance-tests CONFORMANCE_TAG=${{ github.sha }} NGF_VERSION=${{ github.ref_name }} CLUSTER_NAME=${{ github.run_id }} core_result=$(cat conformance-profile.yaml | yq '.profiles[0].core.result') extended_result=$(cat conformance-profile.yaml | yq '.profiles[0].extended.result') 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/charts/nginx-gateway-fabric/templates/deployment.yaml b/charts/nginx-gateway-fabric/templates/deployment.yaml index 107c258990..6ce6240c29 100644 --- a/charts/nginx-gateway-fabric/templates/deployment.yaml +++ b/charts/nginx-gateway-fabric/templates/deployment.yaml @@ -129,6 +129,8 @@ spec: volumeMounts: - name: nginx-conf mountPath: /etc/nginx/conf.d + - name: nginx-stream-conf + mountPath: /etc/nginx/stream-conf.d - name: module-includes mountPath: /etc/nginx/module-includes - name: nginx-secrets @@ -166,6 +168,8 @@ spec: volumeMounts: - name: nginx-conf mountPath: /etc/nginx/conf.d + - name: nginx-stream-conf + mountPath: /etc/nginx/stream-conf.d - name: module-includes mountPath: /etc/nginx/module-includes - name: nginx-secrets @@ -200,6 +204,8 @@ spec: volumes: - name: nginx-conf emptyDir: {} + - name: nginx-stream-conf + emptyDir: {} - name: module-includes emptyDir: {} - name: nginx-secrets diff --git a/config/tests/static-deployment.yaml b/config/tests/static-deployment.yaml index 73ad539084..bb2fb62765 100644 --- a/config/tests/static-deployment.yaml +++ b/config/tests/static-deployment.yaml @@ -72,6 +72,8 @@ spec: volumeMounts: - name: nginx-conf mountPath: /etc/nginx/conf.d + - name: nginx-stream-conf + mountPath: /etc/nginx/stream-conf.d - name: module-includes mountPath: /etc/nginx/module-includes - name: nginx-secrets @@ -102,6 +104,8 @@ spec: volumeMounts: - name: nginx-conf mountPath: /etc/nginx/conf.d + - name: nginx-stream-conf + mountPath: /etc/nginx/stream-conf.d - name: module-includes mountPath: /etc/nginx/module-includes - name: nginx-secrets @@ -121,6 +125,8 @@ spec: volumes: - name: nginx-conf emptyDir: {} + - name: nginx-stream-conf + emptyDir: {} - name: module-includes emptyDir: {} - name: nginx-secrets diff --git a/deploy/aws-nlb/deploy.yaml b/deploy/aws-nlb/deploy.yaml index 00839c0396..49b29bf988 100644 --- a/deploy/aws-nlb/deploy.yaml +++ b/deploy/aws-nlb/deploy.yaml @@ -246,6 +246,8 @@ spec: volumeMounts: - mountPath: /etc/nginx/conf.d name: nginx-conf + - mountPath: /etc/nginx/stream-conf.d + name: nginx-stream-conf - mountPath: /etc/nginx/module-includes name: module-includes - mountPath: /etc/nginx/secrets @@ -276,6 +278,8 @@ spec: volumeMounts: - mountPath: /etc/nginx/conf.d name: nginx-conf + - mountPath: /etc/nginx/stream-conf.d + name: nginx-stream-conf - mountPath: /etc/nginx/module-includes name: module-includes - mountPath: /etc/nginx/secrets @@ -295,6 +299,8 @@ spec: volumes: - emptyDir: {} name: nginx-conf + - emptyDir: {} + name: nginx-stream-conf - emptyDir: {} name: module-includes - emptyDir: {} diff --git a/deploy/azure/deploy.yaml b/deploy/azure/deploy.yaml index 5cfbec8b65..968c1a2926 100644 --- a/deploy/azure/deploy.yaml +++ b/deploy/azure/deploy.yaml @@ -243,6 +243,8 @@ spec: volumeMounts: - mountPath: /etc/nginx/conf.d name: nginx-conf + - mountPath: /etc/nginx/stream-conf.d + name: nginx-stream-conf - mountPath: /etc/nginx/module-includes name: module-includes - mountPath: /etc/nginx/secrets @@ -273,6 +275,8 @@ spec: volumeMounts: - mountPath: /etc/nginx/conf.d name: nginx-conf + - mountPath: /etc/nginx/stream-conf.d + name: nginx-stream-conf - mountPath: /etc/nginx/module-includes name: module-includes - mountPath: /etc/nginx/secrets @@ -294,6 +298,8 @@ spec: volumes: - emptyDir: {} name: nginx-conf + - emptyDir: {} + name: nginx-stream-conf - emptyDir: {} name: module-includes - emptyDir: {} diff --git a/deploy/default/deploy.yaml b/deploy/default/deploy.yaml index 7347443192..6245a2bbc7 100644 --- a/deploy/default/deploy.yaml +++ b/deploy/default/deploy.yaml @@ -243,6 +243,8 @@ spec: volumeMounts: - mountPath: /etc/nginx/conf.d name: nginx-conf + - mountPath: /etc/nginx/stream-conf.d + name: nginx-stream-conf - mountPath: /etc/nginx/module-includes name: module-includes - mountPath: /etc/nginx/secrets @@ -273,6 +275,8 @@ spec: volumeMounts: - mountPath: /etc/nginx/conf.d name: nginx-conf + - mountPath: /etc/nginx/stream-conf.d + name: nginx-stream-conf - mountPath: /etc/nginx/module-includes name: module-includes - mountPath: /etc/nginx/secrets @@ -292,6 +296,8 @@ spec: volumes: - emptyDir: {} name: nginx-conf + - emptyDir: {} + name: nginx-stream-conf - emptyDir: {} name: module-includes - emptyDir: {} diff --git a/deploy/experimental-nginx-plus/deploy.yaml b/deploy/experimental-nginx-plus/deploy.yaml index 2a850aa19a..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: @@ -256,6 +258,8 @@ spec: volumeMounts: - mountPath: /etc/nginx/conf.d name: nginx-conf + - mountPath: /etc/nginx/stream-conf.d + name: nginx-stream-conf - mountPath: /etc/nginx/module-includes name: module-includes - mountPath: /etc/nginx/secrets @@ -286,6 +290,8 @@ spec: volumeMounts: - mountPath: /etc/nginx/conf.d name: nginx-conf + - mountPath: /etc/nginx/stream-conf.d + name: nginx-stream-conf - mountPath: /etc/nginx/module-includes name: module-includes - mountPath: /etc/nginx/secrets @@ -305,6 +311,8 @@ spec: volumes: - emptyDir: {} name: nginx-conf + - emptyDir: {} + name: nginx-stream-conf - emptyDir: {} name: module-includes - emptyDir: {} diff --git a/deploy/experimental/deploy.yaml b/deploy/experimental/deploy.yaml index 5cd1c2b0bb..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: @@ -247,6 +249,8 @@ spec: volumeMounts: - mountPath: /etc/nginx/conf.d name: nginx-conf + - mountPath: /etc/nginx/stream-conf.d + name: nginx-stream-conf - mountPath: /etc/nginx/module-includes name: module-includes - mountPath: /etc/nginx/secrets @@ -277,6 +281,8 @@ spec: volumeMounts: - mountPath: /etc/nginx/conf.d name: nginx-conf + - mountPath: /etc/nginx/stream-conf.d + name: nginx-stream-conf - mountPath: /etc/nginx/module-includes name: module-includes - mountPath: /etc/nginx/secrets @@ -296,6 +302,8 @@ spec: volumes: - emptyDir: {} name: nginx-conf + - emptyDir: {} + name: nginx-stream-conf - emptyDir: {} name: module-includes - emptyDir: {} diff --git a/deploy/nginx-plus/deploy.yaml b/deploy/nginx-plus/deploy.yaml index 9c6a4bd132..76249e80c2 100644 --- a/deploy/nginx-plus/deploy.yaml +++ b/deploy/nginx-plus/deploy.yaml @@ -254,6 +254,8 @@ spec: volumeMounts: - mountPath: /etc/nginx/conf.d name: nginx-conf + - mountPath: /etc/nginx/stream-conf.d + name: nginx-stream-conf - mountPath: /etc/nginx/module-includes name: module-includes - mountPath: /etc/nginx/secrets @@ -284,6 +286,8 @@ spec: volumeMounts: - mountPath: /etc/nginx/conf.d name: nginx-conf + - mountPath: /etc/nginx/stream-conf.d + name: nginx-stream-conf - mountPath: /etc/nginx/module-includes name: module-includes - mountPath: /etc/nginx/secrets @@ -303,6 +307,8 @@ spec: volumes: - emptyDir: {} name: nginx-conf + - emptyDir: {} + name: nginx-stream-conf - emptyDir: {} name: module-includes - emptyDir: {} diff --git a/deploy/nodeport/deploy.yaml b/deploy/nodeport/deploy.yaml index 4f9b78acde..db81fdf259 100644 --- a/deploy/nodeport/deploy.yaml +++ b/deploy/nodeport/deploy.yaml @@ -243,6 +243,8 @@ spec: volumeMounts: - mountPath: /etc/nginx/conf.d name: nginx-conf + - mountPath: /etc/nginx/stream-conf.d + name: nginx-stream-conf - mountPath: /etc/nginx/module-includes name: module-includes - mountPath: /etc/nginx/secrets @@ -273,6 +275,8 @@ spec: volumeMounts: - mountPath: /etc/nginx/conf.d name: nginx-conf + - mountPath: /etc/nginx/stream-conf.d + name: nginx-stream-conf - mountPath: /etc/nginx/module-includes name: module-includes - mountPath: /etc/nginx/secrets @@ -292,6 +296,8 @@ spec: volumes: - emptyDir: {} name: nginx-conf + - emptyDir: {} + name: nginx-stream-conf - emptyDir: {} name: module-includes - emptyDir: {} diff --git a/deploy/openshift/deploy.yaml b/deploy/openshift/deploy.yaml index 213cedcb55..cb78ce0f39 100644 --- a/deploy/openshift/deploy.yaml +++ b/deploy/openshift/deploy.yaml @@ -251,6 +251,8 @@ spec: volumeMounts: - mountPath: /etc/nginx/conf.d name: nginx-conf + - mountPath: /etc/nginx/stream-conf.d + name: nginx-stream-conf - mountPath: /etc/nginx/module-includes name: module-includes - mountPath: /etc/nginx/secrets @@ -281,6 +283,8 @@ spec: volumeMounts: - mountPath: /etc/nginx/conf.d name: nginx-conf + - mountPath: /etc/nginx/stream-conf.d + name: nginx-stream-conf - mountPath: /etc/nginx/module-includes name: module-includes - mountPath: /etc/nginx/secrets @@ -300,6 +304,8 @@ spec: volumes: - emptyDir: {} name: nginx-conf + - emptyDir: {} + name: nginx-stream-conf - emptyDir: {} name: module-includes - emptyDir: {} 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 3c4d642bf7..cf4410f013 100644 --- a/internal/mode/static/handler.go +++ b/internal/mode/static/handler.go @@ -246,6 +246,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 8a16a99eb3..6875f6a927 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)) @@ -489,6 +491,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...) } @@ -663,6 +671,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/conf/nginx-plus.conf b/internal/mode/static/nginx/conf/nginx-plus.conf index d4499652d1..23d97717c9 100644 --- a/internal/mode/static/nginx/conf/nginx-plus.conf +++ b/internal/mode/static/nginx/conf/nginx-plus.conf @@ -54,6 +54,20 @@ http { } } +stream { + variables_hash_bucket_size 512; + variables_hash_max_size 1024; + + map_hash_max_size 2048; + map_hash_bucket_size 256; + + log_format stream-main '$remote_addr [$time_local] ' + '$protocol $status $bytes_sent $bytes_received ' + '$session_time "$ssl_preread_server_name"'; + access_log /dev/stdout stream-main; + include /etc/nginx/stream-conf.d/*.conf; +} + mgmt { usage_report interval=0s; } diff --git a/internal/mode/static/nginx/conf/nginx.conf b/internal/mode/static/nginx/conf/nginx.conf index c253b641eb..2cbc09fa3f 100644 --- a/internal/mode/static/nginx/conf/nginx.conf +++ b/internal/mode/static/nginx/conf/nginx.conf @@ -38,3 +38,17 @@ http { } } } + +stream { + variables_hash_bucket_size 512; + variables_hash_max_size 1024; + + map_hash_max_size 2048; + map_hash_bucket_size 256; + + log_format stream-main '$remote_addr [$time_local] ' + '$protocol $status $bytes_sent $bytes_received ' + '$session_time "$ssl_preread_server_name"'; + access_log /dev/stdout stream-main; + include /etc/nginx/stream-conf.d/*.conf; +} diff --git a/internal/mode/static/nginx/config/base_http_config_template.go b/internal/mode/static/nginx/config/base_http_config_template.go index a909001ab6..bbe35a1018 100644 --- a/internal/mode/static/nginx/config/base_http_config_template.go +++ b/internal/mode/static/nginx/config/base_http_config_template.go @@ -2,4 +2,25 @@ package config const baseHTTPTemplateText = ` {{- if .HTTP2 }}http2 on;{{ end }} + +# Set $gw_api_compliant_host variable to the value of $http_host unless $http_host is empty, then set it to the value +# of $host. We prefer $http_host because it contains the original value of the host header, which is required by the +# Gateway API. However, in an HTTP/1.0 request, it's possible that $http_host can be empty. In this case, we will use +# the value of $host. See http://nginx.org/en/docs/http/ngx_http_core_module.html#var_host. +map $http_host $gw_api_compliant_host { + '' $host; + default $http_host; +} + +# Set $connection_header variable to upgrade when the $http_upgrade header is set, otherwise, set it to close. This +# allows support for websocket connections. See https://nginx.org/en/docs/http/websocket.html. +map $http_upgrade $connection_upgrade { + default upgrade; + '' close; +} + +## Returns just the path from the original request URI. +map $request_uri $request_uri_path { + "~^(?P<path>[^?]*)(\?.*)?$" $path; +} ` diff --git a/internal/mode/static/nginx/config/base_http_config_test.go b/internal/mode/static/nginx/config/base_http_config_test.go index 4eb202ba1b..408118ecbc 100644 --- a/internal/mode/static/nginx/config/base_http_config_test.go +++ b/internal/mode/static/nginx/config/base_http_config_test.go @@ -47,5 +47,8 @@ func TestExecuteBaseHttp(t *testing.T) { res := executeBaseHTTPConfig(test.conf) g.Expect(res).To(HaveLen(1)) g.Expect(test.expCount).To(Equal(strings.Count(string(res[0].data), expSubStr))) + g.Expect(strings.Count(string(res[0].data), "map $http_host $gw_api_compliant_host {")).To(Equal(1)) + g.Expect(strings.Count(string(res[0].data), "map $http_upgrade $connection_upgrade {")).To(Equal(1)) + g.Expect(strings.Count(string(res[0].data), "map $request_uri $request_uri_path {")).To(Equal(1)) } } diff --git a/internal/mode/static/nginx/config/generator.go b/internal/mode/static/nginx/config/generator.go index eac79be34e..3d229c3e1c 100644 --- a/internal/mode/static/nginx/config/generator.go +++ b/internal/mode/static/nginx/config/generator.go @@ -20,6 +20,9 @@ const ( // httpFolder is the folder where NGINX HTTP configuration files are stored. httpFolder = configFolder + "/conf.d" + // streamFolder is the folder where NGINX Stream configuration files are stored. + streamFolder = configFolder + "/stream-conf.d" + // modulesIncludesFolder is the folder where the included "load_module" file is stored. modulesIncludesFolder = configFolder + "/module-includes" @@ -32,6 +35,9 @@ const ( // httpConfigFile is the path to the configuration file with HTTP configuration. httpConfigFile = httpFolder + "/http.conf" + // streamConfigFile is the path to the configuration file with Stream configuration. + streamConfigFile = streamFolder + "/stream.conf" + // configVersionFile is the path to the config version configuration file. configVersionFile = httpFolder + "/config-version.conf" @@ -43,7 +49,7 @@ const ( ) // ConfigFolders is a list of folders where NGINX configuration files are stored. -var ConfigFolders = []string{httpFolder, secretsFolder, includesFolder, modulesIncludesFolder} +var ConfigFolders = []string{httpFolder, secretsFolder, includesFolder, modulesIncludesFolder, streamFolder} // Generator generates NGINX configuration files. // This interface is used for testing purposes only. @@ -168,6 +174,9 @@ func (g GeneratorImpl) getExecuteFuncs(generator policies.Generator) []executeFu executeSplitClients, executeMaps, executeTelemetry, + executeStreamServers, + g.executeStreamUpstreams, + executeStreamMaps, } } diff --git a/internal/mode/static/nginx/config/generator_test.go b/internal/mode/static/nginx/config/generator_test.go index d4e6a5f841..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) { @@ -47,12 +48,30 @@ func TestGenerate(t *testing.T) { Port: 443, }, }, + TLSPassthroughServers: []dataplane.Layer4VirtualServer{ + { + Hostname: "app.example.com", + Port: 443, + UpstreamName: "stream_up", + }, + }, Upstreams: []dataplane.Upstream{ { Name: "up", Endpoints: nil, }, }, + StreamUpstreams: []dataplane.Upstream{ + { + Name: "stream_up", + Endpoints: []resolver.Endpoint{ + { + Address: "1.1.1.1", + Port: 80, + }, + }, + }, + }, BackendGroups: []dataplane.BackendGroup{bg}, SSLKeyPairs: map[dataplane.SSLKeyPairID]dataplane.SSLKeyPair{ "test-keypair": { @@ -81,7 +100,7 @@ func TestGenerate(t *testing.T) { files := generator.Generate(conf) - g.Expect(files).To(HaveLen(6)) + g.Expect(files).To(HaveLen(7)) arrange := func(i, j int) bool { return files[i].Path < files[j].Path } @@ -98,7 +117,7 @@ func TestGenerate(t *testing.T) { // Note: this only verifies that Generate() returns a byte array with upstream, server, and split_client blocks. // It does not test the correctness of those blocks. That functionality is covered by other tests in this package. g.Expect(httpCfg).To(ContainSubstring("listen 80")) - g.Expect(httpCfg).To(ContainSubstring("listen 443")) + g.Expect(httpCfg).To(ContainSubstring("listen unix:/var/run/nginx/https443.sock")) g.Expect(httpCfg).To(ContainSubstring("upstream")) g.Expect(httpCfg).To(ContainSubstring("split_clients")) @@ -127,4 +146,12 @@ func TestGenerate(t *testing.T) { Path: "/etc/nginx/secrets/test-keypair.pem", Content: []byte("test-cert\ntest-key"), })) + + g.Expect(files[6].Path).To(Equal("/etc/nginx/stream-conf.d/stream.conf")) + g.Expect(files[6].Type).To(Equal(file.TypeRegular)) + streamCfg := string(files[6].Content) + g.Expect(streamCfg).To(ContainSubstring("listen unix:/var/run/nginx/app.example.com-443.sock")) + g.Expect(streamCfg).To(ContainSubstring("listen 443")) + g.Expect(streamCfg).To(ContainSubstring("app.example.com unix:/var/run/nginx/app.example.com-443.sock")) + g.Expect(streamCfg).To(ContainSubstring("example.com unix:/var/run/nginx/https443.sock")) } diff --git a/internal/mode/static/nginx/config/http/config.go b/internal/mode/static/nginx/config/http/config.go index 16954c4f3b..f2fb813d6d 100644 --- a/internal/mode/static/nginx/config/http/config.go +++ b/internal/mode/static/nginx/config/http/config.go @@ -1,23 +1,20 @@ package http +import "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/shared" + const InternalRoutePathPrefix = "/_ngf-internal" // Server holds all configuration for an HTTP server. type Server struct { SSL *SSL ServerName string + Listen string Locations []Location Includes []Include - Port int32 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 } type LocationType string @@ -104,19 +101,6 @@ type SplitClientDistribution struct { Value string } -// Map defines an NGINX map. -type Map struct { - Source string - Variable string - Parameters []MapParameter -} - -// MapParameter defines a Value and Result pair in a Map. -type MapParameter struct { - Value string - Result string -} - // ProxySSLVerify holds the proxied HTTPS server verification configuration. type ProxySSLVerify struct { TrustedCertificate string @@ -126,7 +110,7 @@ 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 } // Include defines a file that's included via the include directive. diff --git a/internal/mode/static/nginx/config/maps.go b/internal/mode/static/nginx/config/maps.go index a784390170..f345397264 100644 --- a/internal/mode/static/nginx/config/maps.go +++ b/internal/mode/static/nginx/config/maps.go @@ -5,12 +5,25 @@ import ( gotemplate "text/template" "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" ) var mapsTemplate = gotemplate.Must(gotemplate.New("maps").Parse(mapsTemplateText)) +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...)) result := executeResult{ @@ -21,7 +34,99 @@ func executeMaps(conf dataplane.Configuration) []executeResult { return []executeResult{result} } -func buildAddHeaderMaps(servers []dataplane.VirtualServer) []http.Map { +func executeStreamMaps(conf dataplane.Configuration) []executeResult { + maps := createStreamMaps(conf) + + result := executeResult{ + dest: streamConfigFile, + data: helpers.MustExecuteTemplate(mapsTemplate, maps), + } + + return []executeResult{result} +} + +func createStreamMaps(conf dataplane.Configuration) []shared.Map { + if len(conf.TLSPassthroughServers) == 0 { + 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 u, ok := upstreams[server.UpstreamName]; ok && server.UpstreamName != "" && len(u.Endpoints) > 0 { + socket = getSocketNameTLS(server.Port, server.Hostname) + } + + if server.IsDefault { + socket = connectionClosedStreamServerSocket + } + + if !portInUse { + streamMap = shared.Map{ + Source: "$ssl_preread_server_name", + Variable: getTLSPassthroughVarName(server.Port), + Parameters: make([]shared.MapParameter, 0), + UseHostnames: true, + } + 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 != "" { + mapParam := shared.MapParameter{ + Value: server.Hostname, + Result: socket, + } + streamMap.Parameters = append(streamMap.Parameters, mapParam) + portsToMap[server.Port] = streamMap + } + } + + for _, server := range conf.SSLServers { + streamMap, portInUse := portsToMap[server.Port] + + hostname := server.Hostname + + if server.IsDefault { + hostname = "default" + portHasDefault[server.Port] = struct{}{} + } + + if portInUse { + streamMap.Parameters = append(streamMap.Parameters, shared.MapParameter{ + Value: hostname, + Result: getSocketNameHTTPS(server.Port), + }) + portsToMap[server.Port] = streamMap + } + } + + maps := make([]shared.Map, 0, len(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) + } + + return maps +} + +func buildAddHeaderMaps(servers []dataplane.VirtualServer) []shared.Map { addHeaderNames := make(map[string]struct{}) for _, s := range servers { @@ -39,7 +144,7 @@ func buildAddHeaderMaps(servers []dataplane.VirtualServer) []http.Map { } } - maps := make([]http.Map, 0, len(addHeaderNames)) + maps := make([]shared.Map, 0, len(addHeaderNames)) for m := range addHeaderNames { maps = append(maps, createAddHeadersMap(m)) } @@ -52,11 +157,11 @@ const ( anyStringFmt = `~.*` ) -func createAddHeadersMap(name string) http.Map { +func createAddHeadersMap(name string) shared.Map { underscoreName := convertStringToSafeVariableName(name) httpVarSource := "${http_" + underscoreName + "}" mapVarName := generateAddHeaderMapVariableName(name) - params := []http.MapParameter{ + params := []shared.MapParameter{ { Value: "default", Result: "''", @@ -66,7 +171,7 @@ func createAddHeadersMap(name string) http.Map { Result: httpVarSource + ",", }, } - return http.Map{ + return shared.Map{ Source: httpVarSource, Variable: "$" + mapVarName, Parameters: params, diff --git a/internal/mode/static/nginx/config/maps_template.go b/internal/mode/static/nginx/config/maps_template.go index d11c3f8870..4468b908c7 100644 --- a/internal/mode/static/nginx/config/maps_template.go +++ b/internal/mode/static/nginx/config/maps_template.go @@ -3,30 +3,12 @@ package config const mapsTemplateText = ` {{ range $m := . }} map {{ $m.Source }} {{ $m.Variable }} { - {{ range $p := $m.Parameters }} - {{ $p.Value }} {{ $p.Result }}; - {{ end }} + {{- if $m.UseHostnames }} + hostnames; + {{ end }} + {{ range $p := $m.Parameters }} + {{ $p.Value }} {{ $p.Result }}; + {{ end }} } {{- end }} - -# Set $gw_api_compliant_host variable to the value of $http_host unless $http_host is empty, then set it to the value -# of $host. We prefer $http_host because it contains the original value of the host header, which is required by the -# Gateway API. However, in an HTTP/1.0 request, it's possible that $http_host can be empty. In this case, we will use -# the value of $host. See http://nginx.org/en/docs/http/ngx_http_core_module.html#var_host. -map $http_host $gw_api_compliant_host { - '' $host; - default $http_host; -} - -# Set $connection_header variable to upgrade when the $http_upgrade header is set, otherwise, set it to close. This -# allows support for websocket connections. See https://nginx.org/en/docs/http/websocket.html. -map $http_upgrade $connection_upgrade { - default upgrade; - '' close; -} - -## Returns just the path from the original request URI. -map $request_uri $request_uri_path { - "~^(?P<path>[^?]*)(\?.*)?$" $path; -} ` diff --git a/internal/mode/static/nginx/config/maps_test.go b/internal/mode/static/nginx/config/maps_test.go index b9d0388512..a5227fb9b8 100644 --- a/internal/mode/static/nginx/config/maps_test.go +++ b/internal/mode/static/nginx/config/maps_test.go @@ -6,8 +6,9 @@ import ( . "github.com/onsi/gomega" - "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" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/resolver" ) func TestExecuteMaps(t *testing.T) { @@ -84,9 +85,6 @@ func TestExecuteMaps(t *testing.T) { "map ${http_my_second_add_header} $my_second_add_header_header_var {": 1, "~.* ${http_my_second_add_header},;": 1, "map ${http_my_set_header} $my_set_header_header_var {": 0, - "map $http_host $gw_api_compliant_host {": 1, - "map $http_upgrade $connection_upgrade {": 1, - "map $request_uri $request_uri_path {": 1, } mapResult := executeMaps(conf) @@ -162,11 +160,11 @@ func TestBuildAddHeaderMaps(t *testing.T) { IsDefault: true, }, } - expectedMap := []http.Map{ + expectedMap := []shared.Map{ { Source: "${http_my_add_header}", Variable: "$my_add_header_header_var", - Parameters: []http.MapParameter{ + Parameters: []shared.MapParameter{ {Value: "default", Result: "''"}, { Value: "~.*", @@ -177,7 +175,7 @@ func TestBuildAddHeaderMaps(t *testing.T) { { Source: "${http_my_second_add_header}", Variable: "$my_second_add_header_header_var", - Parameters: []http.MapParameter{ + Parameters: []shared.MapParameter{ {Value: "default", Result: "''"}, { Value: "~.*", @@ -190,3 +188,195 @@ func TestBuildAddHeaderMaps(t *testing.T) { g.Expect(maps).To(ConsistOf(expectedMap)) } + +func TestExecuteStreamMaps(t *testing.T) { + g := NewWithT(t) + conf := dataplane.Configuration{ + TLSPassthroughServers: []dataplane.Layer4VirtualServer{ + { + Hostname: "example.com", + Port: 8081, + UpstreamName: "backend1", + }, + { + Hostname: "example.com", + Port: 8080, + UpstreamName: "backend1", + }, + { + Hostname: "cafe.example.com", + Port: 8080, + UpstreamName: "backend2", + }, + }, + SSLServers: []dataplane.VirtualServer{ + { + Hostname: "app.example.com", + 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{ + "example.com unix:/var/run/nginx/example.com-8081.sock;": 1, + "example.com unix:/var/run/nginx/example.com-8080.sock;": 1, + "cafe.example.com unix:/var/run/nginx/cafe.example.com-8080.sock;": 1, + "app.example.com unix:/var/run/nginx/https8080.sock;": 1, + "hostnames": 2, + "default": 2, + } + + results := executeStreamMaps(conf) + g.Expect(results).To(HaveLen(1)) + result := results[0] + + g.Expect(result.dest).To(Equal(streamConfigFile)) + for expSubStr, expCount := range expSubStrings { + g.Expect(strings.Count(string(result.data), expSubStr)).To(Equal(expCount)) + } +} + +func TestCreateStreamMaps(t *testing.T) { + g := NewWithT(t) + conf := dataplane.Configuration{ + TLSPassthroughServers: []dataplane.Layer4VirtualServer{ + { + Hostname: "example.com", + Port: 8081, + UpstreamName: "backend1", + }, + { + Hostname: "example.com", + Port: 8080, + UpstreamName: "backend1", + }, + { + Hostname: "cafe.example.com", + Port: 8080, + UpstreamName: "backend2", + }, + { + Hostname: "dne.example.com", + Port: 8080, + UpstreamName: "backend-dne", + }, + { + Port: 8082, + Hostname: "", + }, + { + Hostname: "*.example.com", + Port: 8080, + IsDefault: true, + }, + { + Hostname: "no-endpoints.example.com", + Port: 8080, + UpstreamName: "backend3", + }, + }, + SSLServers: []dataplane.VirtualServer{ + { + Hostname: "app.example.com", + Port: 8080, + }, + { + Port: 8080, + 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, + }, + { + Source: "$ssl_preread_server_name", + Variable: getTLSPassthroughVarName(8080), + Parameters: []shared.MapParameter{ + {Value: "example.com", Result: getSocketNameTLS(8080, "example.com")}, + {Value: "cafe.example.com", Result: getSocketNameTLS(8080, "cafe.example.com")}, + {Value: "dne.example.com", Result: emptyStringSocket}, + {Value: "*.example.com", Result: connectionClosedStreamServerSocket}, + {Value: "no-endpoints.example.com", Result: emptyStringSocket}, + {Value: "app.example.com", Result: getSocketNameHTTPS(8080)}, + {Value: "default", Result: getSocketNameHTTPS(8080)}, + }, + UseHostnames: true, + }, + } + + g.Expect(maps).To(ConsistOf(expectedMaps)) +} + +func TestCreateStreamMapsWithEmpty(t *testing.T) { + g := NewWithT(t) + conf := dataplane.Configuration{ + TLSPassthroughServers: nil, + } + + maps := createStreamMaps(conf) + + g.Expect(maps).To(BeNil()) +} diff --git a/internal/mode/static/nginx/config/servers.go b/internal/mode/static/nginx/config/servers.go index f6c666f8a3..6fd3d4610f 100644 --- a/internal/mode/static/nginx/config/servers.go +++ b/internal/mode/static/nginx/config/servers.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/policies" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/shared" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" ) @@ -65,7 +66,7 @@ func newExecuteServersFunc(generator policies.Generator) executeFunc { } func executeServers(conf dataplane.Configuration, generator policies.Generator) []executeResult { - servers, httpMatchPairs := createServers(conf.HTTPServers, conf.SSLServers, generator) + servers, httpMatchPairs := createServers(conf.HTTPServers, conf.SSLServers, conf.TLSPassthroughServers, generator) serverConfig := http.ServerConfig{ Servers: servers, @@ -99,15 +100,15 @@ func executeServers(conf dataplane.Configuration, generator policies.Generator) } // 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 createIncludeFileResults(servers []http.Server) []executeResult { @@ -138,11 +139,18 @@ func createIncludeFileResults(servers []http.Server) []executeResult { } func createServers( - httpServers, sslServers []dataplane.VirtualServer, + httpServers, + sslServers []dataplane.VirtualServer, + tlsPassthroughServers []dataplane.Layer4VirtualServer, generator policies.Generator, ) ([]http.Server, httpMatchPairs) { servers := make([]http.Server, 0, len(httpServers)+len(sslServers)) finalMatchPairs := make(httpMatchPairs) + sharedTLSPorts := make(map[int32]struct{}) + + for _, passthroughServer := range tlsPassthroughServers { + sharedTLSPorts[passthroughServer.Port] = struct{}{} + } for idx, s := range httpServers { serverID := fmt.Sprintf("%d", idx) @@ -153,7 +161,12 @@ func createServers( for idx, s := range sslServers { serverID := fmt.Sprintf("SSL_%d", idx) + sslServer, matchPairs := createSSLServer(s, serverID, generator) + if _, portInUse := sharedTLSPorts[s.Port]; portInUse { + sslServer.Listen = getSocketNameHTTPS(s.Port) + sslServer.IsSocket = true + } servers = append(servers, sslServer) maps.Copy(finalMatchPairs, matchPairs) } @@ -166,10 +179,11 @@ func createSSLServer( serverID string, generator policies.Generator, ) (http.Server, httpMatchPairs) { + listen := fmt.Sprint(virtualServer.Port) if virtualServer.IsDefault { return http.Server{ IsDefaultSSL: true, - Port: virtualServer.Port, + Listen: listen, }, nil } @@ -182,8 +196,8 @@ func createSSLServer( CertificateKey: generatePEMFileName(virtualServer.SSL.KeyPairID), }, Locations: locs, - Port: virtualServer.Port, GRPC: grpc, + Listen: listen, } server.Includes = createIncludesFromPolicyGenerateResult( @@ -197,10 +211,12 @@ func createServer( serverID string, generator policies.Generator, ) (http.Server, httpMatchPairs) { + listen := fmt.Sprint(virtualServer.Port) + if virtualServer.IsDefault { return http.Server{ IsDefaultHTTP: true, - Port: virtualServer.Port, + Listen: listen, }, nil } @@ -209,7 +225,7 @@ func createServer( server := http.Server{ ServerName: virtualServer.Hostname, Locations: locs, - Port: virtualServer.Port, + Listen: listen, GRPC: grpc, } diff --git a/internal/mode/static/nginx/config/servers_template.go b/internal/mode/static/nginx/config/servers_template.go index 6d8bc0c362..80be670045 100644 --- a/internal/mode/static/nginx/config/servers_template.go +++ b/internal/mode/static/nginx/config/servers_template.go @@ -5,11 +5,11 @@ js_preload_object matches from /etc/nginx/conf.d/matches.json; {{- range $s := .Servers -}} {{ if $s.IsDefaultSSL -}} server { - {{- if $.IPFamily.IPv4 }} - listen {{ $s.Port }} ssl default_server; + {{- if or ($.IPFamily.IPv4) ($s.IsSocket) }} + listen {{ $s.Listen }} ssl default_server; {{- end }} - {{- if $.IPFamily.IPv6 }} - listen [::]:{{ $s.Port }} ssl default_server; + {{- if and ($.IPFamily.IPv6) (not $s.IsSocket) }} + listen [::]:{{ $s.Listen }} ssl default_server; {{- end }} ssl_reject_handshake on; @@ -17,10 +17,10 @@ server { {{- else if $s.IsDefaultHTTP }} server { {{- if $.IPFamily.IPv4 }} - listen {{ $s.Port }} default_server; + listen {{ $s.Listen }} default_server; {{- end }} {{- if $.IPFamily.IPv6 }} - listen [::]:{{ $s.Port }} default_server; + listen [::]:{{ $s.Listen }} default_server; {{- end }} default_type text/html; @@ -29,11 +29,11 @@ server { {{- else }} server { {{- if $s.SSL }} - {{- if $.IPFamily.IPv4 }} - listen {{ $s.Port }} ssl; + {{- if or ($.IPFamily.IPv4) ($s.IsSocket) }} + listen {{ $s.Listen }} ssl; {{- end }} - {{- if $.IPFamily.IPv6 }} - listen [::]:{{ $s.Port }} ssl; + {{- if and ($.IPFamily.IPv6) (not $s.IsSocket) }} + listen [::]:{{ $s.Listen }} ssl; {{- end }} ssl_certificate {{ $s.SSL.Certificate }}; ssl_certificate_key {{ $s.SSL.CertificateKey }}; @@ -43,10 +43,10 @@ server { } {{- else }} {{- if $.IPFamily.IPv4 }} - listen {{ $s.Port }}; + listen {{ $s.Listen }}; {{- end }} {{- if $.IPFamily.IPv6 }} - listen [::]:{{ $s.Port }}; + listen [::]:{{ $s.Listen }}; {{- end }} {{- end }} diff --git a/internal/mode/static/nginx/config/servers_test.go b/internal/mode/static/nginx/config/servers_test.go index df517eebe3..2ef832ea09 100644 --- a/internal/mode/static/nginx/config/servers_test.go +++ b/internal/mode/static/nginx/config/servers_test.go @@ -13,6 +13,7 @@ import ( "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/http" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies/policiesfakes" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/shared" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" ) @@ -165,6 +166,26 @@ func TestExecuteServersForIPFamily(t *testing.T) { Port: 8443, }, } + sslServers443 := []dataplane.VirtualServer{ + { + IsDefault: true, + Port: 443, + }, + { + Hostname: "example.com", + SSL: &dataplane.SSL{ + KeyPairID: "test-keypair", + }, + Port: 443, + }, + } + passThroughServers := []dataplane.Layer4VirtualServer{ + { + IsDefault: true, + Hostname: "*.example.com", + Port: 8443, + }, + } tests := []struct { msg string expectedHTTPConfig map[string]int @@ -191,23 +212,26 @@ func TestExecuteServersForIPFamily(t *testing.T) { }, }, { - msg: "http and ssl servers with IPv6 IP family", + msg: "http, ssl servers, and tls servers with IPv6 IP family", config: dataplane.Configuration{ HTTPServers: httpServers, - SSLServers: sslServers, + SSLServers: append(sslServers, sslServers443...), BaseHTTPConfig: dataplane.BaseHTTPConfig{ IPFamily: dataplane.IPv6, }, + TLSPassthroughServers: passThroughServers, }, expectedHTTPConfig: map[string]int{ - "listen [::]:8080 default_server;": 1, - "listen [::]:8080;": 1, - "listen [::]:8443 ssl default_server;": 1, - "listen [::]:8443 ssl;": 1, - "server_name example.com;": 2, - "ssl_certificate /etc/nginx/secrets/test-keypair.pem;": 1, - "ssl_certificate_key /etc/nginx/secrets/test-keypair.pem;": 1, - "ssl_reject_handshake on;": 1, + "listen [::]:8080 default_server;": 1, + "listen [::]:8080;": 1, + "listen [::]:443 ssl default_server;": 1, + "listen [::]:443 ssl;": 1, + "listen unix:/var/run/nginx/https8443.sock ssl;": 1, + "listen unix:/var/run/nginx/https8443.sock ssl default_server;": 1, + "server_name example.com;": 3, + "ssl_certificate /etc/nginx/secrets/test-keypair.pem;": 2, + "ssl_certificate_key /etc/nginx/secrets/test-keypair.pem;": 2, + "ssl_reject_handshake on;": 2, }, }, { @@ -761,6 +785,14 @@ func TestCreateServers(t *testing.T) { }, } + tlsPassthroughServers := []dataplane.Layer4VirtualServer{ + { + Hostname: "app.example.com", + Port: 8443, + UpstreamName: "sup", + }, + } + expMatchPairs := httpMatchPairs{ "1_0": { {Method: "POST", RedirectPath: "/_ngf-internal-rule0-route0"}, @@ -1197,17 +1229,18 @@ func TestCreateServers(t *testing.T) { expectedServers := []http.Server{ { IsDefaultHTTP: true, - Port: 8080, + Listen: "8080", }, { ServerName: "cafe.example.com", Locations: getExpectedLocations(false), - Port: 8080, + Listen: "8080", GRPC: true, }, { IsDefaultSSL: true, - Port: 8443, + Listen: getSocketNameHTTPS(8443), + IsSocket: true, }, { ServerName: "cafe.example.com", @@ -1216,7 +1249,8 @@ func TestCreateServers(t *testing.T) { CertificateKey: expectedPEMPath, }, Locations: getExpectedLocations(true), - Port: 8443, + Listen: getSocketNameHTTPS(8443), + IsSocket: true, GRPC: true, }, } @@ -1237,7 +1271,7 @@ func TestCreateServers(t *testing.T) { }, }) - result, httpMatchPair := createServers(httpServers, sslServers, fakeGenerator) + result, httpMatchPair := createServers(httpServers, sslServers, tlsPassthroughServers, fakeGenerator) g.Expect(httpMatchPair).To(Equal(allExpMatchPair)) g.Expect(helpers.Diff(expectedServers, result)).To(BeEmpty()) @@ -1441,18 +1475,23 @@ func TestCreateServersConflicts(t *testing.T) { expectedServers := []http.Server{ { IsDefaultHTTP: true, - Port: 8080, + Listen: "8080", }, { ServerName: "cafe.example.com", Locations: test.expLocs, - Port: 8080, + Listen: "8080", }, } g := NewWithT(t) - result, _ := createServers(httpServers, []dataplane.VirtualServer{}, &policiesfakes.FakeGenerator{}) + result, _ := createServers( + httpServers, + []dataplane.VirtualServer{}, + []dataplane.Layer4VirtualServer{}, + &policiesfakes.FakeGenerator{}, + ) g.Expect(helpers.Diff(expectedServers, result)).To(BeEmpty()) }) } @@ -2706,22 +2745,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 new file mode 100644 index 0000000000..65c0c873f5 --- /dev/null +++ b/internal/mode/static/nginx/config/shared/config.go @@ -0,0 +1,21 @@ +package shared + +// Map defines an NGINX map. +type Map struct { + Source string + Variable string + Parameters []MapParameter + UseHostnames bool +} + +// MapParameter defines a Value and Result pair in a Map. +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/sockets.go b/internal/mode/static/nginx/config/sockets.go new file mode 100644 index 0000000000..9707ef01a8 --- /dev/null +++ b/internal/mode/static/nginx/config/sockets.go @@ -0,0 +1,17 @@ +package config + +import ( + "fmt" +) + +func getSocketNameTLS(port int32, hostname string) string { + return fmt.Sprintf("unix:/var/run/nginx/%s-%d.sock", hostname, port) +} + +func getSocketNameHTTPS(port int32) string { + return fmt.Sprintf("unix:/var/run/nginx/https%d.sock", port) +} + +func getTLSPassthroughVarName(port int32) string { + return fmt.Sprintf("$dest%d", port) +} diff --git a/internal/mode/static/nginx/config/sockets_test.go b/internal/mode/static/nginx/config/sockets_test.go new file mode 100644 index 0000000000..cbab84aea3 --- /dev/null +++ b/internal/mode/static/nginx/config/sockets_test.go @@ -0,0 +1,28 @@ +package config + +import ( + "testing" + + . "github.com/onsi/gomega" +) + +func TestGetSocketNameTLS(t *testing.T) { + res := getSocketNameTLS(800, "*.cafe.example.com") + + g := NewGomegaWithT(t) + g.Expect(res).To(Equal("unix:/var/run/nginx/*.cafe.example.com-800.sock")) +} + +func TestGetSocketNameHTTPS(t *testing.T) { + res := getSocketNameHTTPS(800) + + g := NewGomegaWithT(t) + g.Expect(res).To(Equal("unix:/var/run/nginx/https800.sock")) +} + +func TestGetTLSPassthroughVarName(t *testing.T) { + res := getTLSPassthroughVarName(800) + + g := NewGomegaWithT(t) + g.Expect(res).To(Equal("$dest800")) +} diff --git a/internal/mode/static/nginx/config/stream/config.go b/internal/mode/static/nginx/config/stream/config.go new file mode 100644 index 0000000000..19a1ae7994 --- /dev/null +++ b/internal/mode/static/nginx/config/stream/config.go @@ -0,0 +1,30 @@ +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. +type Upstream struct { + Name string + ZoneSize string // format: 512k, 1m + Servers []UpstreamServer +} + +// UpstreamServer holds all configuration for a stream upstream server. +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 new file mode 100644 index 0000000000..6c33f5a609 --- /dev/null +++ b/internal/mode/static/nginx/config/stream_servers.go @@ -0,0 +1,69 @@ +package config + +import ( + "fmt" + gotemplate "text/template" + + "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/stream" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" +) + +var streamServersTemplate = gotemplate.Must(gotemplate.New("streamServers").Parse(streamServersTemplateText)) + +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, streamServerConfig), + } + + return []executeResult{ + streamServerResult, + } +} + +func createStreamServers(conf dataplane.Configuration) []stream.Server { + if len(conf.TLSPassthroughServers) == 0 { + return nil + } + + 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 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 { + continue + } + + portSet[server.Port] = struct{}{} + streamServers = append(streamServers, stream.Server{ + Listen: fmt.Sprint(server.Port), + Pass: getTLSPassthroughVarName(server.Port), + SSLPreread: true, + }) + } + + return streamServers +} diff --git a/internal/mode/static/nginx/config/stream_servers_template.go b/internal/mode/static/nginx/config/stream_servers_template.go new file mode 100644 index 0000000000..66f12f5858 --- /dev/null +++ b/internal/mode/static/nginx/config/stream_servers_template.go @@ -0,0 +1,28 @@ +package config + +const streamServersTemplateText = ` +{{- range $s := .Servers }} +server { + {{- 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 }}; + {{- end }} + {{- if $s.Pass }} + pass {{ $s.Pass }}; + {{- end }} + {{- if $s.SSLPreread }} + 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 new file mode 100644 index 0000000000..20f1c201d6 --- /dev/null +++ b/internal/mode/static/nginx/config/stream_servers_test.go @@ -0,0 +1,263 @@ +package config + +import ( + "fmt" + "strings" + "testing" + + . "github.com/onsi/gomega" + + "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) { + conf := dataplane.Configuration{ + TLSPassthroughServers: []dataplane.Layer4VirtualServer{ + { + Hostname: "example.com", + Port: 8081, + UpstreamName: "backend1", + }, + { + Hostname: "example.com", + Port: 8080, + UpstreamName: "backend1", + }, + { + Hostname: "cafe.example.com", + Port: 8080, + 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{ + "pass $dest8081;": 1, + "pass $dest8080;": 1, + "ssl_preread on;": 2, + "proxy_pass": 3, + } + g := NewWithT(t) + + results := executeStreamServers(conf) + g.Expect(results).To(HaveLen(1)) + result := results[0] + + g.Expect(result.dest).To(Equal(streamConfigFile)) + for expSubStr, expCount := range expSubStrings { + g.Expect(strings.Count(string(result.data), expSubStr)).To(Equal(expCount)) + } +} + +func TestCreateStreamServers(t *testing.T) { + conf := dataplane.Configuration{ + TLSPassthroughServers: []dataplane.Layer4VirtualServer{ + { + Hostname: "example.com", + Port: 8081, + UpstreamName: "backend1", + }, + { + Hostname: "example.com", + Port: 8080, + UpstreamName: "backend1", + }, + { + Hostname: "cafe.example.com", + Port: 8080, + UpstreamName: "backend2", + }, + { + Hostname: "blank-upstream.example.com", + Port: 8081, + UpstreamName: "", + }, + { + Hostname: "dne-upstream.example.com", + Port: 8081, + UpstreamName: "dne", + }, + { + Hostname: "no-endpoints.example.com", + Port: 8081, + UpstreamName: "no-endpoints", + }, + }, + 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: "no-endpoints", + Endpoints: nil, + }, + }, + } + + streamServers := createStreamServers(conf) + + g := NewWithT(t) + + expectedStreamServers := []stream.Server{ + { + 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), + Pass: getTLSPassthroughVarName(8081), + SSLPreread: true, + }, + { + Listen: fmt.Sprint(8080), + Pass: getTLSPassthroughVarName(8080), + SSLPreread: true, + }, + } + g.Expect(streamServers).To(ConsistOf(expectedStreamServers)) +} + +func TestExecuteStreamServersForIPFamily(t *testing.T) { + passThroughServers := []dataplane.Layer4VirtualServer{ + { + UpstreamName: "backend1", + Hostname: "cafe.example.com", + Port: 8443, + }, + } + streamUpstreams := []dataplane.Upstream{ + { + Name: "backend1", + Endpoints: []resolver.Endpoint{ + { + Address: "1.1.1.1", + }, + }, + }, + } + tests := []struct { + msg string + expectedServerConfig map[string]int + config dataplane.Configuration + }{ + { + msg: "tls servers with IPv4 IP family", + config: dataplane.Configuration{ + BaseHTTPConfig: dataplane.BaseHTTPConfig{ + IPFamily: dataplane.IPv4, + }, + TLSPassthroughServers: passThroughServers, + StreamUpstreams: streamUpstreams, + }, + expectedServerConfig: map[string]int{ + "listen 8443;": 1, + "listen unix:/var/run/nginx/cafe.example.com-8443.sock;": 1, + }, + }, + { + msg: "tls servers with IPv6 IP family", + config: dataplane.Configuration{ + BaseHTTPConfig: dataplane.BaseHTTPConfig{ + IPFamily: dataplane.IPv6, + }, + TLSPassthroughServers: passThroughServers, + StreamUpstreams: streamUpstreams, + }, + expectedServerConfig: map[string]int{ + "listen [::]:8443;": 1, + "listen unix:/var/run/nginx/cafe.example.com-8443.sock;": 1, + }, + }, + { + msg: "tls servers with dual IP family", + config: dataplane.Configuration{ + BaseHTTPConfig: dataplane.BaseHTTPConfig{ + IPFamily: dataplane.Dual, + }, + TLSPassthroughServers: passThroughServers, + StreamUpstreams: streamUpstreams, + }, + expectedServerConfig: map[string]int{ + "listen 8443;": 1, + "listen [::]:8443;": 1, + "listen unix:/var/run/nginx/cafe.example.com-8443.sock;": 1, + }, + }, + } + + for _, test := range tests { + t.Run(test.msg, func(t *testing.T) { + g := NewWithT(t) + results := executeStreamServers(test.config) + g.Expect(results).To(HaveLen(1)) + serverConf := string(results[0].data) + + for expSubStr, expCount := range test.expectedServerConfig { + g.Expect(strings.Count(serverConf, expSubStr)).To(Equal(expCount)) + } + }) + } +} + +func TestCreateStreamServersWithNone(t *testing.T) { + conf := dataplane.Configuration{ + TLSPassthroughServers: nil, + } + + streamServers := createStreamServers(conf) + + g := NewWithT(t) + + g.Expect(streamServers).To(BeNil()) +} diff --git a/internal/mode/static/nginx/config/upstreams.go b/internal/mode/static/nginx/config/upstreams.go index a76ee23a6a..da52beb07c 100644 --- a/internal/mode/static/nginx/config/upstreams.go +++ b/internal/mode/static/nginx/config/upstreams.go @@ -6,6 +6,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/stream" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" ) @@ -22,6 +23,10 @@ const ( ossZoneSize = "512k" // plusZoneSize is the upstream zone size for nginx plus. plusZoneSize = "1m" + // ossZoneSize is the upstream zone size for nginx open source. + ossZoneSizeStream = "512k" + // plusZoneSize is the upstream zone size for nginx plus. + plusZoneSizeStream = "1m" ) func (g GeneratorImpl) executeUpstreams(conf dataplane.Configuration) []executeResult { @@ -35,6 +40,53 @@ func (g GeneratorImpl) executeUpstreams(conf dataplane.Configuration) []executeR return []executeResult{result} } +func (g GeneratorImpl) executeStreamUpstreams(conf dataplane.Configuration) []executeResult { + upstreams := g.createStreamUpstreams(conf.StreamUpstreams) + + result := executeResult{ + dest: streamConfigFile, + data: helpers.MustExecuteTemplate(upstreamsTemplate, upstreams), + } + + return []executeResult{result} +} + +func (g GeneratorImpl) createStreamUpstreams(upstreams []dataplane.Upstream) []stream.Upstream { + ups := make([]stream.Upstream, 0, len(upstreams)) + + for _, u := range upstreams { + if len(u.Endpoints) != 0 { + ups = append(ups, g.createStreamUpstream(u)) + } + } + + return ups +} + +func (g GeneratorImpl) createStreamUpstream(up dataplane.Upstream) stream.Upstream { + zoneSize := ossZoneSizeStream + if g.plus { + zoneSize = plusZoneSizeStream + } + + 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(format, ep.Address, ep.Port), + } + } + + return stream.Upstream{ + Name: up.Name, + ZoneSize: zoneSize, + Servers: upstreamServers, + } +} + func (g GeneratorImpl) createUpstreams(upstreams []dataplane.Upstream) []http.Upstream { // capacity is the number of upstreams + 1 for the invalid backend ref upstream ups := make([]http.Upstream, 0, len(upstreams)+1) diff --git a/internal/mode/static/nginx/config/upstreams_template.go b/internal/mode/static/nginx/config/upstreams_template.go index fd5130dc2b..a04915bec8 100644 --- a/internal/mode/static/nginx/config/upstreams_template.go +++ b/internal/mode/static/nginx/config/upstreams_template.go @@ -1,8 +1,9 @@ package config // FIXME(kate-osborn): Dynamically calculate upstream zone size based on the number of upstreams. -// 512k will support up to 648 upstream servers for OSS. -// NGINX Plus needs 1m to support roughly the same amount of servers (556 upstream servers). +// 512k will support up to 648 http upstream servers for OSS. +// NGINX Plus needs 1m to support roughly the same amount of http servers (556 upstream servers). +// For stream upstream servers, 512k will support 576 in OSS and 1m will support 991 in NGINX Plus // https://github.com/nginxinc/nginx-gateway-fabric/issues/483 const upstreamsTemplateText = ` {{ range $u := . }} diff --git a/internal/mode/static/nginx/config/upstreams_test.go b/internal/mode/static/nginx/config/upstreams_test.go index 9b6dbbf7ec..8ec1c33386 100644 --- a/internal/mode/static/nginx/config/upstreams_test.go +++ b/internal/mode/static/nginx/config/upstreams_test.go @@ -6,6 +6,7 @@ import ( . "github.com/onsi/gomega" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/http" + "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" ) @@ -307,3 +308,191 @@ func TestCreateUpstreamPlus(t *testing.T) { g := NewWithT(t) g.Expect(result).To(Equal(expectedUpstream)) } + +func TestExecuteStreamUpstreams(t *testing.T) { + gen := GeneratorImpl{} + stateUpstreams := []dataplane.Upstream{ + { + Name: "up1", + Endpoints: []resolver.Endpoint{ + { + Address: "10.0.0.0", + Port: 80, + }, + }, + }, + { + Name: "up2", + Endpoints: []resolver.Endpoint{ + { + Address: "11.0.0.0", + Port: 80, + }, + }, + }, + { + Name: "up3", + Endpoints: []resolver.Endpoint{}, + }, + } + + expectedSubStrings := []string{ + "upstream up1", + "upstream up2", + "server 10.0.0.0:80;", + "server 11.0.0.0:80;", + } + + upstreamResults := gen.executeStreamUpstreams(dataplane.Configuration{StreamUpstreams: stateUpstreams}) + g := NewWithT(t) + g.Expect(upstreamResults).To(HaveLen(1)) + upstreams := string(upstreamResults[0].data) + + g.Expect(upstreamResults[0].dest).To(Equal(streamConfigFile)) + for _, expSubString := range expectedSubStrings { + g.Expect(upstreams).To(ContainSubstring(expSubString)) + } +} + +func TestCreateStreamUpstreams(t *testing.T) { + gen := GeneratorImpl{} + stateUpstreams := []dataplane.Upstream{ + { + Name: "up1", + Endpoints: []resolver.Endpoint{ + { + Address: "10.0.0.0", + Port: 80, + }, + { + Address: "10.0.0.1", + Port: 80, + }, + { + Address: "10.0.0.2", + Port: 80, + }, + { + Address: "2001:db8::1", + IPv6: true, + }, + }, + }, + { + Name: "up2", + Endpoints: []resolver.Endpoint{ + { + Address: "11.0.0.0", + Port: 80, + }, + }, + }, + { + Name: "up3", + Endpoints: []resolver.Endpoint{}, + }, + } + + expUpstreams := []stream.Upstream{ + { + Name: "up1", + ZoneSize: ossZoneSize, + Servers: []stream.UpstreamServer{ + { + Address: "10.0.0.0:80", + }, + { + Address: "10.0.0.1:80", + }, + { + Address: "10.0.0.2:80", + }, + { + Address: "[2001:db8::1]:0", + }, + }, + }, + { + Name: "up2", + ZoneSize: ossZoneSize, + Servers: []stream.UpstreamServer{ + { + Address: "11.0.0.0:80", + }, + }, + }, + } + + g := NewWithT(t) + result := gen.createStreamUpstreams(stateUpstreams) + g.Expect(result).To(Equal(expUpstreams)) +} + +func TestCreateStreamUpstream(t *testing.T) { + gen := GeneratorImpl{} + up := dataplane.Upstream{ + Name: "multiple-endpoints", + Endpoints: []resolver.Endpoint{ + { + Address: "10.0.0.1", + Port: 80, + }, + { + Address: "10.0.0.2", + Port: 80, + }, + { + Address: "10.0.0.3", + Port: 80, + }, + }, + } + + expectedUpstream := stream.Upstream{ + Name: "multiple-endpoints", + ZoneSize: ossZoneSize, + Servers: []stream.UpstreamServer{ + { + Address: "10.0.0.1:80", + }, + { + Address: "10.0.0.2:80", + }, + { + Address: "10.0.0.3:80", + }, + }, + } + + g := NewWithT(t) + result := gen.createStreamUpstream(up) + g.Expect(result).To(Equal(expectedUpstream)) +} + +func TestCreateStreamUpstreamPlus(t *testing.T) { + gen := GeneratorImpl{plus: true} + + stateUpstream := dataplane.Upstream{ + Name: "multiple-endpoints", + Endpoints: []resolver.Endpoint{ + { + Address: "10.0.0.1", + Port: 80, + }, + }, + } + expectedUpstream := stream.Upstream{ + Name: "multiple-endpoints", + ZoneSize: plusZoneSize, + Servers: []stream.UpstreamServer{ + { + Address: "10.0.0.1:80", + }, + }, + } + + result := gen.createStreamUpstream(stateUpstream) + + g := NewWithT(t) + g.Expect(result).To(Equal(expectedUpstream)) +} diff --git a/internal/mode/static/state/change_processor.go b/internal/mode/static/state/change_processor.go index 40a3723dff..8605098b57 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 eff60594b4..0d55189d41 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)) @@ -528,6 +529,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)}, @@ -539,6 +541,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)}, @@ -549,6 +552,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 457f9ad270..026c826787 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" @@ -187,6 +191,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{ @@ -424,6 +439,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 39dc5d63f3..ebaf15bf0b 100644 --- a/internal/mode/static/state/dataplane/configuration.go +++ b/internal/mode/static/state/dataplane/configuration.go @@ -29,7 +29,7 @@ const ( func BuildConfiguration( ctx context.Context, g *graph.Graph, - resolver resolver.ServiceResolver, + serviceResolver resolver.ServiceResolver, configVersion int, ) Configuration { if g.GatewayClass == nil || !g.GatewayClass.Valid { @@ -41,28 +41,164 @@ func BuildConfiguration( } baseHTTPConfig := buildBaseHTTPConfig(g) - upstreams := buildUpstreams(ctx, g.Gateway.Listeners, resolver, baseHTTPConfig.IPFamily) + + upstreams := buildUpstreams(ctx, g.Gateway.Listeners, serviceResolver, baseHTTPConfig.IPFamily) httpServers, sslServers := buildServers(g) + 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) + + passthroughServerCount := 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 + } + + var hostnames []string + + for _, p := range r.ParentRefs { + if val, exist := p.Attachment.AcceptedHostnames[l.Name]; exist { + hostnames = val + break + } + } + + if _, ok := passthroughServersMap[key]; !ok { + passthroughServersMap[key] = make([]Layer4VirtualServer, 0) + } + + passthroughServerCount += len(hostnames) + + 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), + }) + } + } + 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, passthroughServerCount+len(listenerPassthroughServers)) + + 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, + serviceResolver 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 := serviceResolver.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( @@ -210,6 +346,9 @@ func buildServers(g *graph.Graph) (http, ssl []VirtualServer) { } 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 { @@ -313,6 +452,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 61e5742602..edc9ae602e 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" @@ -408,6 +409,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, + } + + invalidBackendRefTR2 := 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 @@ -596,6 +654,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, @@ -1522,11 +1600,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: &invalidBackendRefTR2, + }, + 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: &invalidBackendRefTR2, + }, + 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: &invalidBackendRefTR2, + } g.ReferencedSecrets = map[types.NamespacedName]*graph.Secret{ secret1NsName: secret1, } @@ -1577,9 +1689,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 { @@ -2054,6 +2197,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)) @@ -3141,3 +3285,264 @@ func TestCreateRatioVarName(t *testing.T) { g := NewWithT(t) g.Expect(CreateRatioVarName(25)).To(Equal("$otel_ratio_25")) } + +func TestCreatePassthroughServers(t *testing.T) { + getL4RouteKey := func(name string) graph.L4RouteKey { + return graph.L4RouteKey{ + NamespacedName: types.NamespacedName{ + Namespace: "default", + Name: name, + }, + } + } + secureAppKey := getL4RouteKey("secure-app") + secureApp2Key := getL4RouteKey("secure-app2") + secureApp3Key := getL4RouteKey("secure-app3") + testGraph := graph.Graph{ + Gateway: &graph.Gateway{ + Listeners: []*graph.Listener{ + { + Name: "testingListener", + Valid: true, + 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{ + secureAppKey: { + Valid: true, + Spec: graph.L4RouteSpec{ + Hostnames: []v1.Hostname{"app.example.com", "cafe.example.com"}, + BackendRef: graph.BackendRef{ + Valid: true, + SvcNsName: secureAppKey.NamespacedName, + ServicePort: apiv1.ServicePort{ + Name: "https", + Protocol: "TCP", + Port: 8443, + TargetPort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 8443, + }, + }, + }, + }, + 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, + }, + }, + }, + secureApp2Key: {}, + }, + }, + { + Name: "testingListener2", + Valid: true, + 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{ + secureApp3Key: { + Valid: true, + Spec: graph.L4RouteSpec{ + Hostnames: []v1.Hostname{"app.example.com", "cafe.example.com"}, + BackendRef: graph.BackendRef{ + Valid: true, + SvcNsName: secureAppKey.NamespacedName, + ServicePort: apiv1.ServicePort{ + Name: "https", + Protocol: "TCP", + Port: 8443, + TargetPort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 8443, + }, + }, + }, + }, + }, + }, + }, + { + Name: "httpListener", + Valid: true, + Source: v1.Listener{ + Protocol: v1.HTTPProtocolType, + }, + }, + }, + }, + } + + 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) { + getL4RouteKey := func(name string) graph.L4RouteKey { + return graph.L4RouteKey{ + NamespacedName: types.NamespacedName{ + Namespace: "default", + Name: name, + }, + } + } + secureAppKey := getL4RouteKey("secure-app") + secureApp2Key := getL4RouteKey("secure-app2") + secureApp3Key := getL4RouteKey("secure-app3") + secureApp4Key := getL4RouteKey("secure-app4") + secureApp5Key := getL4RouteKey("secure-app5") + testGraph := graph.Graph{ + Gateway: &graph.Gateway{ + Listeners: []*graph.Listener{ + { + Name: "testingListener", + Valid: true, + Source: v1.Listener{ + Protocol: v1.TLSProtocolType, + Port: 443, + }, + Routes: make(map[graph.RouteKey]*graph.L7Route), + L4Routes: map[graph.L4RouteKey]*graph.L4Route{ + secureAppKey: { + Valid: true, + Spec: graph.L4RouteSpec{ + Hostnames: []v1.Hostname{"app.example.com", "cafe.example.com"}, + BackendRef: graph.BackendRef{ + Valid: true, + SvcNsName: secureAppKey.NamespacedName, + ServicePort: apiv1.ServicePort{ + Name: "https", + Protocol: "TCP", + Port: 8443, + TargetPort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 8443, + }, + }, + }, + }, + }, + secureApp2Key: {}, + secureApp3Key: { + Valid: true, + Spec: graph.L4RouteSpec{ + Hostnames: []v1.Hostname{"test.example.com"}, + BackendRef: graph.BackendRef{}, + }, + }, + secureApp4Key: { + Valid: true, + Spec: graph.L4RouteSpec{ + Hostnames: []v1.Hostname{"app.example.com", "cafe.example.com"}, + BackendRef: graph.BackendRef{ + Valid: true, + SvcNsName: secureAppKey.NamespacedName, + ServicePort: apiv1.ServicePort{ + Name: "https", + Protocol: "TCP", + Port: 8443, + TargetPort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 8443, + }, + }, + }, + }, + }, + secureApp5Key: { + Valid: true, + Spec: graph.L4RouteSpec{ + Hostnames: []v1.Hostname{"app2.example.com"}, + BackendRef: graph.BackendRef{ + Valid: true, + SvcNsName: secureApp5Key.NamespacedName, + ServicePort: apiv1.ServicePort{ + Name: "https", + Protocol: "TCP", + Port: 8443, + TargetPort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 8443, + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + + fakeResolver := resolverfakes.FakeServiceResolver{} + fakeResolver.ResolveReturnsOnCall(0, nil, errors.New("error")) + fakeEndpoints := []resolver.Endpoint{ + {Address: "1.1.1.1", Port: 80}, + } + fakeResolver.ResolveReturnsOnCall( + 1, + fakeEndpoints, + nil, + ) + + streamUpstreams := buildStreamUpstreams(context.Background(), testGraph.Gateway.Listeners, &fakeResolver, Dual) + + expectedStreamUpstreams := []Upstream{ + { + Name: "default_secure-app_8443", + ErrorMsg: "error", + }, + { + Name: "default_secure-app5_8443", + Endpoints: fakeEndpoints, + }, + } + g := NewWithT(t) + + g.Expect(streamUpstreams).To(ConsistOf(expectedStreamUpstreams)) +} diff --git a/internal/mode/static/state/dataplane/types.go b/internal/mode/static/state/dataplane/types.go index c509dbc3ca..3741f131c8 100644 --- a/internal/mode/static/state/dataplane/types.go +++ b/internal/mode/static/state/dataplane/types.go @@ -30,8 +30,12 @@ type Configuration struct { HTTPServers []VirtualServer // SSLServers holds all SSLServers. SSLServers []VirtualServer - // Upstreams holds all unique Upstreams. + // TLSPassthroughServers hold all TLSPassthroughServers + TLSPassthroughServers []Layer4VirtualServer + // Upstreams holds all unique http Upstreams. Upstreams []Upstream + // StreamUpstreams holds all unique stream Upstreams + StreamUpstreams []Upstream // BackendGroups holds all unique BackendGroups. BackendGroups []BackendGroup // BaseHTTPConfig holds the configuration options at the http context. @@ -77,6 +81,18 @@ type VirtualServer struct { IsDefault bool } +// Layer4VirtualServer is a virtual server for Layer 4 traffic. +type Layer4VirtualServer struct { + // Hostname is the hostname of the server. + Hostname string + // UpstreamName refers to the name of the upstream that is used. + 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 +} + // Upstream is a pool of endpoints to be load balanced. type Upstream struct { // Name is the name of the Upstream. Will be unique for each service/port combination. 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..d8e7a60ff5 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, @@ -235,37 +253,51 @@ func getAndValidateListenerSupportedKinds(listener v1.Listener) ( var conds []conditions.Condition var supportedKinds []v1.RouteGroupKind - validRouteKind := func(kind v1.RouteGroupKind) bool { - if kind.Kind != v1.Kind(kinds.HTTPRoute) && kind.Kind != v1.Kind(kinds.GRPCRoute) { - return false + var validKinds []v1.RouteGroupKind + + switch listener.Protocol { + case v1.HTTPProtocolType, v1.HTTPSProtocolType: + validKinds = []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)}, } - if kind.Group == nil || *kind.Group != v1.GroupName { + case v1.TLSProtocolType: + validKinds = []v1.RouteGroupKind{ + {Kind: v1.Kind(kinds.TLSRoute), Group: helpers.GetPointer[v1.Group](v1.GroupName)}, + } + } + + validProtocolRouteKind := func(kind v1.RouteGroupKind) bool { + if kind.Group != nil && *kind.Group != v1.GroupName { return false } - return true + for _, k := range validKinds { + if k.Kind == kind.Kind { + return true + } + } + + return false } if listener.AllowedRoutes != nil && listener.AllowedRoutes.Kinds != nil { supportedKinds = make([]v1.RouteGroupKind, 0, len(listener.AllowedRoutes.Kinds)) for _, kind := range listener.AllowedRoutes.Kinds { - if !validRouteKind(kind) { - msg := fmt.Sprintf("Unsupported route kind \"%s/%s\"", *kind.Group, kind.Kind) + if !validProtocolRouteKind(kind) { + group := v1.GroupName + if kind.Group != nil { + group = string(*kind.Group) + } + msg := fmt.Sprintf("Unsupported route kind for protocol %s \"%s/%s\"", listener.Protocol, group, kind.Kind) conds = append(conds, staticConds.NewListenerInvalidRouteKinds(msg)...) continue } supportedKinds = append(supportedKinds, kind) } - } else { - switch listener.Protocol { - case v1.HTTPProtocolType, v1.HTTPSProtocolType: - 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)}, - } - } + return conds, supportedKinds } - return conds, supportedKinds + return conds, validKinds } func validateListenerAllowedRouteKind(listener v1.Listener) (conds []conditions.Condition, attachable bool) { @@ -321,6 +353,19 @@ 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 +432,25 @@ func createHTTPSListenerValidator(protectedPorts ProtectedPorts) listenerValidat } func createPortConflictResolver() listenerConflictResolver { + const ( + secureProtocolGroup int = 0 + insecureProtocolGroup int = 1 + ) + protocolGroups := 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 +466,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] = protocolGroups[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 != protocolGroups[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 +560,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..712ab867d4 100644 --- a/internal/mode/static/state/graph/gateway_listener_test.go +++ b/internal/mode/static/state/graph/gateway_listener_test.go @@ -303,6 +303,10 @@ func TestGetAndValidateListenerSupportedKinds(t *testing.T) { Group: helpers.GetPointer[v1.Group](v1.GroupName), }, } + TLSRouteGroupKind := v1.RouteGroupKind{ + Kind: kinds.TLSRoute, + Group: helpers.GetPointer[v1.Group](v1.GroupName), + } tests := []struct { protocol v1.ProtocolType name string @@ -357,6 +361,7 @@ func TestGetAndValidateListenerSupportedKinds(t *testing.T) { HTTPRouteGroupKind, GRPCRouteGroupKind, }, }, + { protocol: v1.HTTPProtocolType, kind: []v1.RouteGroupKind{ @@ -365,11 +370,49 @@ func TestGetAndValidateListenerSupportedKinds(t *testing.T) { Kind: "bad-kind", Group: helpers.GetPointer[v1.Group](v1.GroupName), }, + TLSRouteGroupKind, }, expectErr: true, name: "valid and invalid kinds", expected: []v1.RouteGroupKind{HTTPRouteGroupKind}, }, + { + protocol: v1.TLSProtocolType, + kind: []v1.RouteGroupKind{ + HTTPRouteGroupKind, + { + Kind: "bad-kind", + Group: helpers.GetPointer[v1.Group](v1.GroupName), + }, + TLSRouteGroupKind, + GRPCRouteGroupKind, + }, + expectErr: true, + name: "valid and invalid kinds for TLS protocol", + expected: []v1.RouteGroupKind{TLSRouteGroupKind}, + }, + { + protocol: v1.TLSProtocolType, + kind: []v1.RouteGroupKind{ + HTTPRouteGroupKind, + { + Kind: "bad-kind", + Group: helpers.GetPointer[v1.Group](v1.GroupName), + }, + GRPCRouteGroupKind, + }, + expectErr: true, + name: "invalid kinds for TLS protocol", + expected: []v1.RouteGroupKind{}, + }, + { + protocol: v1.TLSProtocolType, + kind: []v1.RouteGroupKind{ + TLSRouteGroupKind, + }, + name: "valid kinds for TLS protocol", + expected: []v1.RouteGroupKind{TLSRouteGroupKind}, + }, } for _, test := range tests { @@ -471,3 +514,110 @@ func TestValidateListenerPort(t *testing.T) { }) } } + +func TestListenerNamesHaveOverlap(t *testing.T) { + tests := []struct { + hostname1 *v1.Hostname + hostname2 *v1.Hostname + msg string + expectResult bool + }{ + { + hostname1: (*v1.Hostname)(helpers.GetPointer("*.example.com")), + hostname2: (*v1.Hostname)(helpers.GetPointer("*.example.com")), + expectResult: true, + msg: "same hostnames with wildcard", + }, + { + hostname1: nil, + hostname2: nil, + expectResult: true, + msg: "two nil hostnames", + }, + { + hostname1: (*v1.Hostname)(helpers.GetPointer("cafe.example.com")), + hostname2: (*v1.Hostname)(helpers.GetPointer("app.example.com")), + expectResult: false, + msg: "two different hostnames no wildcard", + }, + { + hostname1: (*v1.Hostname)(helpers.GetPointer("cafe.example.com")), + hostname2: nil, + expectResult: true, + msg: "hostname1 is nil", + }, + { + hostname1: nil, + hostname2: (*v1.Hostname)(helpers.GetPointer("cafe.example.com")), + expectResult: true, + msg: "hostname2 is nil", + }, + { + hostname1: (*v1.Hostname)(helpers.GetPointer("*.example.com")), + hostname2: (*v1.Hostname)(helpers.GetPointer("*.example.org")), + expectResult: false, + msg: "wildcard hostnames that do not overlap", + }, + { + hostname1: (*v1.Hostname)(helpers.GetPointer("*.example.com")), + hostname2: (*v1.Hostname)(helpers.GetPointer("cafe.example.com")), + expectResult: true, + msg: "one wildcard hostname and one hostname that overlap", + }, + } + + for _, test := range tests { + t.Run(test.msg, func(t *testing.T) { + g := NewWithT(t) + g.Expect(haveOverlap(test.hostname1, test.hostname2)).To(Equal(test.expectResult)) + }) + } +} + +func TestValidateTLSFieldOnTLSListener(t *testing.T) { + tests := []struct { + listener v1.Listener + msg string + expectedCond []conditions.Condition + expectValid bool + }{ + { + listener: v1.Listener{}, + expectedCond: staticConds.NewListenerUnsupportedValue( + "TLS: Required value: tls must be defined for TLS listener", + ), + expectValid: false, + msg: "TLS listener without tls field", + }, + { + listener: v1.Listener{TLS: nil}, + expectedCond: staticConds.NewListenerUnsupportedValue( + "TLS: Required value: tls must be defined for TLS listener", + ), + expectValid: false, + msg: "TLS listener with TLS field nil", + }, + { + listener: v1.Listener{TLS: &v1.GatewayTLSConfig{Mode: helpers.GetPointer(v1.TLSModeTerminate)}}, + expectedCond: staticConds.NewListenerUnsupportedValue( + "TLS.Mode: Required value: Mode must be passthrough for TLS listener", + ), + expectValid: false, + msg: "TLS listener with TLS mode terminate", + }, + { + listener: v1.Listener{TLS: &v1.GatewayTLSConfig{Mode: helpers.GetPointer(v1.TLSModePassthrough)}}, + expectValid: true, + msg: "TLS listener with TLS mode passthrough", + }, + } + for _, test := range tests { + t.Run(test.msg, func(t *testing.T) { + g := NewWithT(t) + cond, valid := validateTLSFieldOnTLSListener(test.listener) + + g.Expect(cond).To(BeEquivalentTo(test.expectedCond)) + g.Expect(valid).To(BeEquivalentTo(test.expectValid)) + }) + } +} diff --git a/internal/mode/static/state/graph/gateway_test.go b/internal/mode/static/state/graph/gateway_test.go index b942c4470f..990568b0a1 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) } @@ -258,12 +267,13 @@ func TestBuildGateway(t *testing.T) { foo80Listener1 := createHTTPListener("foo-80-1", "foo.example.com", 80) foo8080Listener := createHTTPListener("foo-8080", "foo.example.com", 8080) foo8081Listener := createHTTPListener("foo-8081", "foo.example.com", 8081) - foo443Listener := createHTTPListener("foo-443", "foo.example.com", 443) + foo443HTTPListener := createHTTPListener("foo-443-http", "foo.example.com", 443) // foo https listeners 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 { @@ -336,7 +352,7 @@ func TestBuildGateway(t *testing.T) { } return lastCreatedGateway } - getLastCreatedGetaway := func() *v1.Gateway { + getLastCreatedGateway := func() *v1.Gateway { return lastCreatedGateway } @@ -363,7 +379,7 @@ func TestBuildGateway(t *testing.T) { gateway: createGateway(gatewayCfg{listeners: []v1.Listener{foo80Listener1, foo8080Listener}}), gatewayClass: validGC, expected: &Gateway{ - Source: getLastCreatedGetaway(), + Source: getLastCreatedGateway(), Listeners: []*Listener{ { Name: "foo-80-1", @@ -371,6 +387,7 @@ func TestBuildGateway(t *testing.T) { Valid: true, Attachable: true, Routes: map[RouteKey]*L7Route{}, + L4Routes: map[L4RouteKey]*L4Route{}, SupportedKinds: supportedKindsForListeners, }, { @@ -379,6 +396,7 @@ func TestBuildGateway(t *testing.T) { Valid: true, Attachable: true, Routes: map[RouteKey]*L7Route{}, + L4Routes: map[L4RouteKey]*L4Route{}, SupportedKinds: supportedKindsForListeners, }, }, @@ -392,7 +410,7 @@ func TestBuildGateway(t *testing.T) { ), gatewayClass: validGC, expected: &Gateway{ - Source: getLastCreatedGetaway(), + Source: getLastCreatedGateway(), Listeners: []*Listener{ { Name: "foo-443-https-1", @@ -400,6 +418,7 @@ func TestBuildGateway(t *testing.T) { Valid: true, Attachable: true, Routes: map[RouteKey]*L7Route{}, + L4Routes: map[L4RouteKey]*L4Route{}, ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), SupportedKinds: supportedKindsForListeners, }, @@ -409,6 +428,7 @@ func TestBuildGateway(t *testing.T) { Valid: true, Attachable: true, Routes: map[RouteKey]*L7Route{}, + L4Routes: map[L4RouteKey]*L4Route{}, ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), SupportedKinds: supportedKindsForListeners, }, @@ -421,7 +441,7 @@ func TestBuildGateway(t *testing.T) { gateway: createGateway(gatewayCfg{listeners: []v1.Listener{listenerAllowedRoutes}}), gatewayClass: validGC, expected: &Gateway{ - Source: getLastCreatedGetaway(), + Source: getLastCreatedGateway(), Listeners: []*Listener{ { Name: "listener-with-allowed-routes", @@ -430,6 +450,7 @@ func TestBuildGateway(t *testing.T) { Attachable: true, AllowedRouteLabelSelector: labels.SelectorFromSet(labels.Set(labelSet)), Routes: map[RouteKey]*L7Route{}, + L4Routes: map[L4RouteKey]*L4Route{}, SupportedKinds: []v1.RouteGroupKind{ {Kind: kinds.HTTPRoute, Group: helpers.GetPointer[v1.Group](v1.GroupName)}, }, @@ -467,7 +488,7 @@ func TestBuildGateway(t *testing.T) { }, }, expected: &Gateway{ - Source: getLastCreatedGetaway(), + Source: getLastCreatedGateway(), Listeners: []*Listener{ { Name: "listener-cross-ns-secret", @@ -475,6 +496,7 @@ func TestBuildGateway(t *testing.T) { Valid: true, Attachable: true, Routes: map[RouteKey]*L7Route{}, + L4Routes: map[L4RouteKey]*L4Route{}, ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretDiffNamespace)), SupportedKinds: supportedKindsForListeners, }, @@ -487,7 +509,7 @@ func TestBuildGateway(t *testing.T) { gateway: createGateway(gatewayCfg{listeners: []v1.Listener{crossNamespaceSecretListener}}), gatewayClass: validGC, expected: &Gateway{ - Source: getLastCreatedGetaway(), + Source: getLastCreatedGateway(), Listeners: []*Listener{ { Name: "listener-cross-ns-secret", @@ -498,6 +520,7 @@ func TestBuildGateway(t *testing.T) { `Certificate ref to secret diff-ns/secret not permitted by any ReferenceGrant`, ), Routes: map[RouteKey]*L7Route{}, + L4Routes: map[L4RouteKey]*L4Route{}, SupportedKinds: supportedKindsForListeners, }, }, @@ -509,7 +532,7 @@ func TestBuildGateway(t *testing.T) { gateway: createGateway(gatewayCfg{listeners: []v1.Listener{listenerInvalidSelector}}), gatewayClass: validGC, expected: &Gateway{ - Source: getLastCreatedGetaway(), + Source: getLastCreatedGateway(), Listeners: []*Listener{ { Name: "listener-with-invalid-selector", @@ -519,7 +542,8 @@ 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)}, }, @@ -533,7 +557,7 @@ func TestBuildGateway(t *testing.T) { gateway: createGateway(gatewayCfg{listeners: []v1.Listener{invalidProtocolListener}}), gatewayClass: validGC, expected: &Gateway{ - Source: getLastCreatedGetaway(), + Source: getLastCreatedGateway(), Listeners: []*Listener{ { Name: "invalid-protocol", @@ -541,9 +565,10 @@ 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, @@ -562,7 +587,7 @@ func TestBuildGateway(t *testing.T) { ), gatewayClass: validGC, expected: &Gateway{ - Source: getLastCreatedGetaway(), + Source: getLastCreatedGateway(), Listeners: []*Listener{ { Name: "invalid-port", @@ -573,6 +598,7 @@ func TestBuildGateway(t *testing.T) { `port: Invalid value: 0: port must be between 1-65535`, ), Routes: map[RouteKey]*L7Route{}, + L4Routes: map[L4RouteKey]*L4Route{}, SupportedKinds: supportedKindsForListeners, }, { @@ -584,6 +610,7 @@ func TestBuildGateway(t *testing.T) { `port: Invalid value: 65536: port must be between 1-65535`, ), Routes: map[RouteKey]*L7Route{}, + L4Routes: map[L4RouteKey]*L4Route{}, SupportedKinds: supportedKindsForListeners, }, { @@ -594,8 +621,9 @@ 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, @@ -608,7 +636,7 @@ func TestBuildGateway(t *testing.T) { ), gatewayClass: validGC, expected: &Gateway{ - Source: getLastCreatedGetaway(), + Source: getLastCreatedGateway(), Listeners: []*Listener{ { Name: "invalid-hostname", @@ -616,6 +644,7 @@ func TestBuildGateway(t *testing.T) { Valid: false, Conditions: staticConds.NewListenerUnsupportedValue(invalidHostnameMsg), Routes: map[RouteKey]*L7Route{}, + L4Routes: map[L4RouteKey]*L4Route{}, SupportedKinds: supportedKindsForListeners, }, { @@ -624,6 +653,7 @@ func TestBuildGateway(t *testing.T) { Valid: false, Conditions: staticConds.NewListenerUnsupportedValue(invalidHostnameMsg), Routes: map[RouteKey]*L7Route{}, + L4Routes: map[L4RouteKey]*L4Route{}, SupportedKinds: supportedKindsForListeners, }, }, @@ -635,7 +665,7 @@ func TestBuildGateway(t *testing.T) { gateway: createGateway(gatewayCfg{listeners: []v1.Listener{invalidTLSConfigListener}}), gatewayClass: validGC, expected: &Gateway{ - Source: getLastCreatedGetaway(), + Source: getLastCreatedGateway(), Listeners: []*Listener{ { Name: "invalid-tls-config", @@ -643,6 +673,7 @@ func TestBuildGateway(t *testing.T) { Valid: false, Attachable: true, Routes: map[RouteKey]*L7Route{}, + L4Routes: map[L4RouteKey]*L4Route{}, Conditions: staticConds.NewListenerInvalidCertificateRef( `tls.certificateRefs[0]: Invalid value: test/does-not-exist: secret does not exist`, ), @@ -670,7 +701,7 @@ func TestBuildGateway(t *testing.T) { ), gatewayClass: validGC, expected: &Gateway{ - Source: getLastCreatedGetaway(), + Source: getLastCreatedGateway(), Listeners: []*Listener{ { Name: "foo-80-1", @@ -678,6 +709,7 @@ func TestBuildGateway(t *testing.T) { Valid: true, Attachable: true, Routes: map[RouteKey]*L7Route{}, + L4Routes: map[L4RouteKey]*L4Route{}, SupportedKinds: supportedKindsForListeners, }, { @@ -686,6 +718,7 @@ func TestBuildGateway(t *testing.T) { Valid: true, Attachable: true, Routes: map[RouteKey]*L7Route{}, + L4Routes: map[L4RouteKey]*L4Route{}, SupportedKinds: supportedKindsForListeners, }, { @@ -694,6 +727,7 @@ func TestBuildGateway(t *testing.T) { Valid: true, Attachable: true, Routes: map[RouteKey]*L7Route{}, + L4Routes: map[L4RouteKey]*L4Route{}, SupportedKinds: supportedKindsForListeners, }, { @@ -702,6 +736,7 @@ func TestBuildGateway(t *testing.T) { Valid: true, Attachable: true, Routes: map[RouteKey]*L7Route{}, + L4Routes: map[L4RouteKey]*L4Route{}, ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), SupportedKinds: supportedKindsForListeners, }, @@ -711,6 +746,7 @@ func TestBuildGateway(t *testing.T) { Valid: true, Attachable: true, Routes: map[RouteKey]*L7Route{}, + L4Routes: map[L4RouteKey]*L4Route{}, ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), SupportedKinds: supportedKindsForListeners, }, @@ -720,6 +756,7 @@ func TestBuildGateway(t *testing.T) { Valid: true, Attachable: true, Routes: map[RouteKey]*L7Route{}, + L4Routes: map[L4RouteKey]*L4Route{}, SupportedKinds: supportedKindsForListeners, }, { @@ -728,6 +765,7 @@ func TestBuildGateway(t *testing.T) { Valid: true, Attachable: true, Routes: map[RouteKey]*L7Route{}, + L4Routes: map[L4RouteKey]*L4Route{}, ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), SupportedKinds: supportedKindsForListeners, }, @@ -737,6 +775,7 @@ func TestBuildGateway(t *testing.T) { Valid: true, Attachable: true, Routes: map[RouteKey]*L7Route{}, + L4Routes: map[L4RouteKey]*L4Route{}, ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), SupportedKinds: supportedKindsForListeners, }, @@ -751,7 +790,7 @@ func TestBuildGateway(t *testing.T) { listeners: []v1.Listener{ foo80Listener1, bar80Listener, - foo443Listener, + foo443HTTPListener, foo80HTTPSListener, foo443HTTPSListener1, bar443HTTPSListener, @@ -760,7 +799,7 @@ func TestBuildGateway(t *testing.T) { ), gatewayClass: validGC, expected: &Gateway{ - Source: getLastCreatedGetaway(), + Source: getLastCreatedGateway(), Listeners: []*Listener{ { Name: "foo-80-1", @@ -768,6 +807,7 @@ func TestBuildGateway(t *testing.T) { Valid: false, Attachable: true, Routes: map[RouteKey]*L7Route{}, + L4Routes: map[L4RouteKey]*L4Route{}, Conditions: staticConds.NewListenerProtocolConflict(conflict80PortMsg), SupportedKinds: supportedKindsForListeners, }, @@ -777,15 +817,17 @@ func TestBuildGateway(t *testing.T) { Valid: false, Attachable: true, Routes: map[RouteKey]*L7Route{}, + L4Routes: map[L4RouteKey]*L4Route{}, Conditions: staticConds.NewListenerProtocolConflict(conflict80PortMsg), SupportedKinds: supportedKindsForListeners, }, { - Name: "foo-443", - Source: foo443Listener, + Name: "foo-443-http", + Source: foo443HTTPListener, Valid: false, Attachable: true, Routes: map[RouteKey]*L7Route{}, + L4Routes: map[L4RouteKey]*L4Route{}, Conditions: staticConds.NewListenerProtocolConflict(conflict443PortMsg), SupportedKinds: supportedKindsForListeners, }, @@ -795,6 +837,7 @@ func TestBuildGateway(t *testing.T) { Valid: false, Attachable: true, Routes: map[RouteKey]*L7Route{}, + L4Routes: map[L4RouteKey]*L4Route{}, Conditions: staticConds.NewListenerProtocolConflict(conflict80PortMsg), ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), SupportedKinds: supportedKindsForListeners, @@ -805,6 +848,7 @@ func TestBuildGateway(t *testing.T) { Valid: false, Attachable: true, Routes: map[RouteKey]*L7Route{}, + L4Routes: map[L4RouteKey]*L4Route{}, Conditions: staticConds.NewListenerProtocolConflict(conflict443PortMsg), ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), SupportedKinds: supportedKindsForListeners, @@ -815,6 +859,7 @@ func TestBuildGateway(t *testing.T) { Valid: false, Attachable: true, Routes: map[RouteKey]*L7Route{}, + L4Routes: map[L4RouteKey]*L4Route{}, Conditions: staticConds.NewListenerProtocolConflict(conflict443PortMsg), ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), SupportedKinds: supportedKindsForListeners, @@ -833,7 +878,7 @@ func TestBuildGateway(t *testing.T) { ), gatewayClass: validGC, expected: &Gateway{ - Source: getLastCreatedGetaway(), + Source: getLastCreatedGateway(), Valid: false, Conditions: staticConds.NewGatewayUnsupportedValue("spec." + "addresses: Forbidden: addresses are not supported", @@ -852,7 +897,7 @@ func TestBuildGateway(t *testing.T) { ), gatewayClass: invalidGC, expected: &Gateway{ - Source: getLastCreatedGetaway(), + Source: getLastCreatedGateway(), Valid: false, Conditions: staticConds.NewGatewayInvalid("GatewayClass is invalid"), }, @@ -864,12 +909,117 @@ func TestBuildGateway(t *testing.T) { ), gatewayClass: nil, expected: &Gateway{ - Source: getLastCreatedGetaway(), + Source: getLastCreatedGateway(), Valid: false, Conditions: staticConds.NewGatewayInvalid("GatewayClass doesn't exist"), }, name: "nil gatewayclass", }, + { + gateway: createGateway( + gatewayCfg{listeners: []v1.Listener{foo443TLSListener, foo443HTTPListener}}, + ), + gatewayClass: validGC, + expected: &Gateway{ + Source: getLastCreatedGateway(), + 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-http", + Source: foo443HTTPListener, + 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: getLastCreatedGateway(), + 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", + }, + { + gateway: createGateway( + gatewayCfg{listeners: []v1.Listener{foo443TLSListener, bar443HTTPSListener}}, + ), + gatewayClass: validGC, + expected: &Gateway{ + Source: getLastCreatedGateway(), + Valid: true, + Listeners: []*Listener{ + { + Name: "foo-443-tls", + Source: foo443TLSListener, + Valid: true, + Attachable: true, + Routes: map[RouteKey]*L7Route{}, + L4Routes: map[L4RouteKey]*L4Route{}, + SupportedKinds: []v1.RouteGroupKind{ + {Kind: kinds.TLSRoute, Group: helpers.GetPointer[v1.Group](v1.GroupName)}, + }, + }, + { + Name: "bar-443-https", + Source: bar443HTTPSListener, + Valid: true, + Attachable: true, + ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(secretSameNs)), + Routes: map[RouteKey]*L7Route{}, + L4Routes: map[L4RouteKey]*L4Route{}, + SupportedKinds: supportedKindsForListeners, + }, + }, + }, + name: "https listener and tls listener with non overlapping hostnames", + }, } secretResolver := newSecretResolver( diff --git a/internal/mode/static/state/graph/graph.go b/internal/mode/static/state/graph/graph.go index a954d0a898..229af562e4 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 @@ -220,12 +223,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( @@ -240,6 +250,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 4ec03dd32c..3312aea694 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,43 @@ func TestBuildGraph(t *testing.T) { } } + createRouteTLS := func(name string, gatewayName 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 + // These TLS Routes do not specify section names so that they attempt to attach to all listeners. + tr := createRouteTLS("tr", "gateway-1") + tr2 := createRouteTLS("tr2", "gateway-1") + gr := &gatewayv1.GRPCRoute{ ObjectMeta: metav1.ObjectMeta{ Namespace: testNs, @@ -250,7 +293,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 +307,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 +347,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 +529,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, @@ -511,6 +591,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, @@ -584,6 +726,7 @@ func TestBuildGraph(t *testing.T) { CreateRouteKey(gr): routeGR, }, SupportedKinds: supportedKindsForListeners, + L4Routes: map[L4RouteKey]*L4Route{}, AllowedRouteLabelSelector: labels.SelectorFromSet(map[string]string{"app": "allowed"}), }, { @@ -592,9 +735,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}, @@ -607,6 +773,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, @@ -616,7 +786,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): { @@ -685,6 +856,9 @@ func TestBuildGraph(t *testing.T) { t.Run(test.name, func(t *testing.T) { g := NewWithT(t) + // The diffs get very large so the format max length will make sure the output doesn't get truncated. + format.MaxLength = 10000000 + fakePolicyValidator := &validationfakes.FakePolicyValidator{} result := BuildGraph( diff --git a/internal/mode/static/state/graph/route_common.go b/internal/mode/static/state/graph/route_common.go index c096f404c5..b8e9eee4fa 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" + ngfSort "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" ) @@ -57,6 +60,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. @@ -65,6 +74,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. @@ -134,6 +166,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, @@ -247,7 +306,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, ) { @@ -255,70 +315,254 @@ func bindRoutesToListeners( return } + for _, r := range l7Routes { + bindL7RouteToListeners(r, gw, namespaces) + } + + var routes []*L4Route + for _, r := range l4Routes { + routes = append(routes, r) + } + + // Sort the slice by timestamp and name so that we process the routes in the priority order + sort.Slice(routes, func(i, j int) bool { + return ngfSort.LessClientObject(routes[i].Source, routes[j].Source) + }) + + // portHostnamesMap exists to detect duplicate hostnames on the same port + portHostnamesMap := make(map[string]struct{}) + for _, r := range routes { - bindRouteToListeners(r, gw, namespaces) + bindL4RouteToListeners(r, gw, namespaces, portHostnamesMap) } } -func bindRouteToListeners( - route *L7Route, +func validateParentRef( + ref *ParentRef, + gw *Gateway, +) (status *ParentRefAttachmentStatus, attachableListeners []*Listener) { + attachment := &ParentRefAttachmentStatus{ + AcceptedHostnames: make(map[string][]string), + } + + 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() + return attachment, nil + } + + // 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()) + return attachment, attachableListeners + } + + // 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() + return attachment, attachableListeners + } + + // Case 4: Attachment is not possible because Gateway is invalid + + if !gw.Valid { + attachment.FailedCondition = staticConds.NewRouteInvalidGateway() + return attachment, attachableListeners + } + return attachment, attachableListeners +} + +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] + + attachment, attachableListeners := validateParentRef(ref, gw) + + if attachment.FailedCondition != (conditions.Condition{}) { + continue } - ref := &route.ParentRefs[i] - ref.Attachment = attachment - path := field.NewPath("spec").Child("parentRefs").Index(ref.Idx) + // Winning Gateway + // Try to attach Route to all matching listeners - attachableListeners, listenerExists := findAttachableListeners( - getSectionName(ref.SectionName), - gw.Listeners, + cond, attached := tryToAttachL4RouteToListeners( + ref.Attachment, + attachableListeners, + route, + gw, + namespaces, + portHostnamesMap, ) - - // Case 1: Attachment is not possible because the specified SectionName does not match any Listeners in the - // Gateway. - if !listenerExists { - attachment.FailedCondition = staticConds.NewRouteNoMatchingParent() + if !attached { + attachment.FailedCondition = cond continue } + if cond != (conditions.Condition{}) { + route.Conditions = append(route.Conditions, cond) + } - // Case 2: Attachment is not possible due to unsupported configuration + attachment.Attached = true + } +} - if ref.Port != nil { - valErr := field.Forbidden(path.Child("port"), "cannot be set") - attachment.FailedCondition = staticConds.NewRouteUnsupportedValue(valErr.Error()) - continue +// 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) + }) - // Case 3: the parentRef references an ignored Gateway resource. + for _, l := range attachableListeners { + routeAllowed, routeAttached, routeHostnamesUnique := bindToListenerL4( + l, + route, + gw, + namespaces, + portHostnamesMap, + refStatus, + ) + allowed = allowed || routeAllowed + attached = attached || routeAttached + hostnamesUnique = hostnamesUnique || routeHostnamesUnique + attachedToAtLeastOneValidListener = attachedToAtLeastOneValidListener || (routeAttached && l.Valid) + } - referencesWinningGw := ref.Gateway.Namespace == gw.Source.Namespace && ref.Gateway.Name == gw.Source.Name + if !attached { + if !allowed { + return staticConds.NewRouteNotAllowedByListeners(), false + } + if !hostnamesUnique { + return staticConds.NewRouteHostnameConflict(), false + } + return staticConds.NewRouteNoMatchingListenerHostname(), false + } - if !referencesWinningGw { - attachment.FailedCondition = staticConds.NewRouteNotAcceptedGatewayIgnored() - continue + if !attachedToAtLeastOneValidListener { + return staticConds.NewRouteInvalidListener(), true + } + + return conditions.Condition{}, true +} + +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) } + } - // Case 4: Attachment is not possible because Gateway is invalid + // We only add a condition if 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(acceptedListenerHostnames) > 0 { + 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, +) { + if !route.Attachable { + return + } + + for i := range route.ParentRefs { + ref := &(route.ParentRefs)[i] - if !gw.Valid { - attachment.FailedCondition = staticConds.NewRouteInvalidGateway() + attachment, attachableListeners := validateParentRef(ref, gw) + + if attachment.FailedCondition != (conditions.Condition{}) { continue } - // Case 5 - winning Gateway - + // Winning Gateway // Try to attach Route to all matching listeners - cond, attached := tryToAttachRouteToListeners( + cond, attached := tryToAttachL7RouteToListeners( ref.Attachment, attachableListeners, route, @@ -342,7 +586,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, @@ -360,7 +604,7 @@ func tryToAttachRouteToListeners( return false, false } - if !isRouteTypeAllowedByListener(l, route.RouteType) { + if !isRouteTypeAllowedByListener(l, convertRouteType(route.RouteType)) { return false, false } @@ -368,10 +612,12 @@ func tryToAttachRouteToListeners( if len(hostnames) == 0 { return true, false } + refStatus.AcceptedHostnames[string(l.Source.Name)] = hostnames refStatus.ListenerPort = l.Source.Port l.Routes[rk] = route + return true, true } @@ -538,9 +784,9 @@ func isRouteNamespaceAllowedByListener( } // isRouteKindAllowedByListener checks if the route is allowed to attach to the listener. -func isRouteTypeAllowedByListener(listener *Listener, routeType RouteType) bool { - for _, kind := range listener.SupportedKinds { - if kind.Kind == convertRouteType(routeType) { +func isRouteTypeAllowedByListener(listener *Listener, kind v1.Kind) bool { + for _, supportedKind := range listener.SupportedKinds { + if supportedKind.Kind == kind { 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..4d4dee7dc0 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,692 @@ 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 + } + + makeModifiedRoute := func(gateway *gatewayv1.Gateway, m func(r *L4Route)) *L4Route { + normalRoute = createNormalRoute(gateway) + m(normalRoute) + 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 + 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; in different namespace", + }, + { + 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 attachable 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")) + }), + }, + }, + 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")) + }), + }, + name: "route hostname does not match any listener", + }, + { + route: makeModifiedRoute(gw, func(r *L4Route) { + r.ParentRefs[0].SectionName = nil + }), + gateway: &Gateway{ + Source: gw, + Valid: true, + Listeners: []*Listener{ + createListener("listener-443"), + }, + }, + expectedSectionNameRefs: []ParentRef{ + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + 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: "nil section name", + }, + { + route: makeModifiedRoute(gw, func(r *L4Route) { + r.ParentRefs[0].SectionName = helpers.GetPointer[gatewayv1.SectionName]("") + }), + gateway: &Gateway{ + Source: gw, + Valid: true, + Listeners: []*Listener{ + createListener("listener-443"), + }, + }, + expectedSectionNameRefs: []ParentRef{ + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + Attachment: &ParentRefAttachmentStatus{ + Attached: true, + AcceptedHostnames: map[string][]string{ + "listener-443": {"foo.example.com"}, + }, + }, + SectionName: helpers.GetPointer[gatewayv1.SectionName](""), + }, + }, + expectedGatewayListeners: []*Listener{ + createModifiedListener("listener-443", func(l *Listener) { + l.L4Routes = map[L4RouteKey]*L4Route{ + CreateRouteKeyL4(tr): getLastNormalRoute(), + } + }), + }, + name: "empty section name", + }, + { + route: createNormalRoute(gw), + gateway: &Gateway{ + Source: gw, + Valid: true, + Listeners: []*Listener{}, + }, + expectedSectionNameRefs: []ParentRef{ + { + Attachment: &noMatchingParentAttachment, + SectionName: tr.Spec.ParentRefs[0].SectionName, + Gateway: client.ObjectKeyFromObject(gw), + Idx: 0, + }, + }, + expectedGatewayListeners: []*Listener{}, + name: "listener does not exist", + }, + { + route: makeModifiedRoute(gw, func(r *L4Route) { + r.Valid = false + }), + gateway: &Gateway{ + Source: gw, + Valid: true, + Listeners: []*Listener{ + createListener("listener-443"), + }, + }, + expectedSectionNameRefs: []ParentRef{ + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + Attachment: &ParentRefAttachmentStatus{ + Attached: true, + AcceptedHostnames: map[string][]string{ + "listener-443": {"foo.example.com"}, + }, + }, + SectionName: helpers.GetPointer[gatewayv1.SectionName]("listener-443"), + }, + }, + expectedGatewayListeners: []*Listener{ + createModifiedListener("listener-443", func(l *Listener) { + l.L4Routes = map[L4RouteKey]*L4Route{ + CreateRouteKeyL4(tr): getLastNormalRoute(), + } + }), + }, + name: "invalid attachable route", + }, + { + route: createNormalRoute(gw), + gateway: &Gateway{ + Source: gw, + Valid: true, + Listeners: []*Listener{ + createModifiedListener("listener-443", func(l *Listener) { + l.SupportedKinds = nil + }), + }, + }, + expectedSectionNameRefs: []ParentRef{ + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + Attachment: &ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{}, + FailedCondition: staticConds.NewRouteNotAllowedByListeners(), + }, + SectionName: helpers.GetPointer[gatewayv1.SectionName]("listener-443"), + }, + }, + expectedGatewayListeners: []*Listener{ + createModifiedListener("listener-443", func(l *Listener) { + l.SupportedKinds = nil + }), + }, + name: "route kind not allowed", + }, + } + + 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) + + bindL4RouteToListeners( + test.route, + test.gateway, + namespaces, + map[string]struct{}{}, + ) + + 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_NoGateways(t *testing.T) { + g := NewWithT(t) + + nsName := types.NamespacedName{Namespace: testNs, Name: "hi"} + + tlsRoutes := map[types.NamespacedName]*v1alpha2.TLSRoute{ + nsName: { + Spec: v1alpha2.TLSRouteSpec{ + Hostnames: []v1alpha2.Hostname{"app.example.com"}, + }, + }, + } + + services := map[types.NamespacedName]*v1.Service{ + nsName: { + Spec: v1.ServiceSpec{ + Ports: []v1.ServicePort{}, + }, + }, + } + + g.Expect(buildL4RoutesForGateways( + tlsRoutes, + nil, + services, + nil, + )).To(BeNil()) +} + +func TestTryToAttachL4RouteToListeners_NoAttachableListeners(t *testing.T) { + g := NewWithT(t) + + route := &L4Route{ + Spec: L4RouteSpec{ + Hostnames: []v1alpha2.Hostname{"app.example.com"}, + }, + Valid: true, + Attachable: true, + } + + gw := &Gateway{ + Valid: true, + Listeners: []*Listener{ + { + Name: "listener1", + }, + { + Name: "listener2", + }, + }, + } + + cond, attachable := tryToAttachL4RouteToListeners( + nil, + nil, + route, + gw, + nil, + map[string]struct{}{}, + ) + 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..da33e1a5bd --- /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, + } + duplicateParentRefsGtr := createTLSRoute( + "hi.example.com", + nil, + []gatewayv1.ParentReference{ + parentRef, + parentRef, + }, + ) + noParentRefsGtr := createTLSRoute( + "hi.example.com", + nil, + []gatewayv1.ParentReference{}, + ) + invalidHostnameGtr := createTLSRoute( + "hi....com", + nil, + []gatewayv1.ParentReference{ + parentRef, + }, + ) + noRulesGtr := createTLSRoute( + "app.example.com", + nil, + []gatewayv1.ParentReference{ + parentRef, + }, + ) + backedRefDNEGtr := createTLSRoute( + "app.example.com", + []v1alpha2.TLSRouteRule{ + { + BackendRefs: []gatewayv1.BackendRef{ + { + BackendObjectReference: gatewayv1.BackendObjectReference{ + Name: "hi", + Port: helpers.GetPointer[gatewayv1.PortNumber](80), + }, + }, + }, + }, + }, + []gatewayv1.ParentReference{ + parentRef, + }, + ) + + wrongBackendRefGroupGtr := 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, + }, + ) + + wrongBackendRefKindGtr := 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, + }, + ) + + wrongBackendRefNamespaceGtr8 := 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, + }, + ) + + portNilBackendRefGtr := createTLSRoute("app.example.com", + []v1alpha2.TLSRouteRule{ + { + BackendRefs: []gatewayv1.BackendRef{ + { + BackendObjectReference: gatewayv1.BackendObjectReference{ + Name: "hi", + }, + }, + }, + }, + }, + []gatewayv1.ParentReference{ + parentRef, + }, + ) + + ipFamilyMismatchGtr := 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: duplicateParentRefsGtr, + expected: &L4Route{Source: duplicateParentRefsGtr}, + gatewayNsNames: []types.NamespacedName{gatewayNsName}, + services: map[types.NamespacedName]*apiv1.Service{}, + name: "duplicate parent refs", + }, + { + gtr: noParentRefsGtr, + expected: nil, + gatewayNsNames: []types.NamespacedName{gatewayNsName}, + services: map[types.NamespacedName]*apiv1.Service{}, + name: "no parent refs", + }, + { + gtr: invalidHostnameGtr, + expected: &L4Route{ + Source: invalidHostnameGtr, + 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: noRulesGtr, + expected: &L4Route{ + Source: noRulesGtr, + 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: backedRefDNEGtr, + expected: &L4Route{ + Source: backedRefDNEGtr, + 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: wrongBackendRefGroupGtr, + expected: &L4Route{ + Source: wrongBackendRefGroupGtr, + 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: wrongBackendRefKindGtr, + expected: &L4Route{ + Source: wrongBackendRefKindGtr, + 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: wrongBackendRefNamespaceGtr8, + expected: &L4Route{ + Source: wrongBackendRefNamespaceGtr8, + 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: portNilBackendRefGtr, + expected: &L4Route{ + Source: portNilBackendRefGtr, + 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: ipFamilyMismatchGtr, + expected: &L4Route{ + Source: ipFamilyMismatchGtr, + 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 cda4ff6012..dc64490502 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 821d7bcf94..d91c610ff7 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 87e5379fa7..ab63c779b0 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -13,7 +13,17 @@ GW_SVC_GKE_INTERNAL = false NGF_VERSION ?= edge## NGF version to be tested 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_EXTENDED_FEATURES = HTTPRouteQueryParamMatching,HTTPRouteMethodMatching,HTTPRoutePortRedirect,HTTPRouteSchemeRedirect,HTTPRouteHostRewrite,HTTPRoutePathRewrite,GatewayPort8080,HTTPRouteResponseHeaderModification +STANDARD_CONFORMANCE_PROFILES = GATEWAY-HTTP,GATEWAY-GRPC +EXPERIMENTAL_CONFORMANCE_PROFILES = GATEWAY-TLS +CONFORMANCE_PROFILES = $(STANDARD_CONFORMANCE_PROFILES) # by default we use the standard conformance profiles. If experimental is enabled we override this and add the experimental profiles. +SKIP_TESTS = TLSRouteInvalidReferenceGrant + +# Check if ENABLE_EXPERIMENTAL is true +ifeq ($(ENABLE_EXPERIMENTAL),true) + # If true, add the experimental conformance profiles + CONFORMANCE_PROFILES = $(EXPERIMENTAL_CONFORMANCE_PROFILES),$(STANDARD_CONFORMANCE_PROFILES) +endif ifneq ($(GINKGO_LABEL),) override GINKGO_FLAGS += --label-filter "$(GINKGO_LABEL)" @@ -40,7 +50,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_EXTENDED_FEATURES) --version=$(NGF_VERSION) --skip-tests=$(SKIP_TESTS) --conformance-profiles=$(CONFORMANCE_PROFILES) \ --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 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..bb4cd72a40 100644 --- a/tests/conformance/conformance_test.go +++ b/tests/conformance/conformance_test.go @@ -22,7 +22,6 @@ import ( "testing" . "github.com/onsi/gomega" - "k8s.io/apimachinery/pkg/util/sets" "sigs.k8s.io/gateway-api/conformance" conf_v1 "sigs.k8s.io/gateway-api/conformance/apis/v1" "sigs.k8s.io/gateway-api/conformance/tests" @@ -35,9 +34,12 @@ func TestConformance(t *testing.T) { g := NewWithT(t) t.Logf(`Running conformance tests with %s GatewayClass\n cleanup: %t\n`+ - `debug: %t\n enable all features: %t \n supported features: [%v]\n exempt features: [%v]`, + `debug: %t\n enable all features: %t \n supported extended features: [%v]\n exempt features: [%v]\n`+ + `conformance profiles: [%v]\n skip tests: [%v]`, *flags.GatewayClassName, *flags.CleanupBaseResources, *flags.ShowDebug, - *flags.EnableAllSupportedFeatures, *flags.SupportedFeatures, *flags.ExemptFeatures) + *flags.EnableAllSupportedFeatures, *flags.SupportedFeatures, *flags.ExemptFeatures, + *flags.ConformanceProfiles, *flags.SkipTests, + ) opts := conformance.DefaultOptions(t) opts.Implementation = conf_v1.Implementation{ @@ -49,7 +51,6 @@ func TestConformance(t *testing.T) { "https://github.com/nginxinc/nginx-gateway-fabric/discussions/new/choose", }, } - opts.ConformanceProfiles = sets.New(suite.GatewayHTTPConformanceProfileName, suite.GatewayGRPCConformanceProfileName) testSuite, err := suite.NewConformanceTestSuite(opts) g.Expect(err).To(Not(HaveOccurred()))