From 5afe0693debef39acc9d14577f567ef7a094cec4 Mon Sep 17 00:00:00 2001 From: Saloni Date: Mon, 1 Jul 2024 18:56:35 -0600 Subject: [PATCH 01/28] add support for ipv6 --- apis/v1alpha1/nginxproxy_types.go | 21 +- charts/nginx-gateway-fabric/values.yaml | 1 + .../bases/gateway.nginx.org_nginxproxies.yaml | 9 + deploy/crds.yaml | 9 + docs/developer/quickstart.md | 19 ++ .../mode/static/nginx/config/http/config.go | 7 + internal/mode/static/nginx/config/servers.go | 42 ++- .../static/nginx/config/servers_template.go | 20 ++ .../mode/static/nginx/config/servers_test.go | 261 +++++++++++------- .../mode/static/nginx/config/upstreams.go | 5 + .../static/nginx/config/upstreams_test.go | 53 ++++ .../static/state/dataplane/configuration.go | 10 +- .../state/dataplane/configuration_test.go | 142 ++++++++-- internal/mode/static/state/dataplane/types.go | 7 +- .../mode/static/state/resolver/resolver.go | 10 +- .../static/state/resolver/resolver_test.go | 26 +- .../state/resolver/service_resolver_test.go | 7 +- site/content/reference/api.md | 58 ++++ 18 files changed, 574 insertions(+), 133 deletions(-) diff --git a/apis/v1alpha1/nginxproxy_types.go b/apis/v1alpha1/nginxproxy_types.go index 2c2a125946..50818c7425 100644 --- a/apis/v1alpha1/nginxproxy_types.go +++ b/apis/v1alpha1/nginxproxy_types.go @@ -10,7 +10,7 @@ import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" // NginxProxy is a configuration object that is attached to a GatewayClass parametersRef. It provides a way // to configure global settings for all Gateways defined from the GatewayClass. -type NginxProxy struct { //nolint:govet // standard field alignment, don't change it +type NginxProxy struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` @@ -27,12 +27,31 @@ type NginxProxyList struct { Items []NginxProxy `json:"items"` } +// IPFamilyType specifies the IP family to be used by the server. +// +// +kubebuilder:validation:Enum=dual;ipv4;ipv6 +type IPFamilyType string + +const ( + // Dual specifies that the server will use both IPv4 and IPv6. + Dual IPFamilyType = "dual" + // IPv4 specifies that the server will use only IPv4. + IPv4 IPFamilyType = "ipv4" + // IPv6 specifies that the server will use only IPv6. + IPv6 IPFamilyType = "ipv6" +) + // NginxProxySpec defines the desired state of the NginxProxy. type NginxProxySpec struct { // Telemetry specifies the OpenTelemetry configuration. // // +optional Telemetry *Telemetry `json:"telemetry,omitempty"` + // IPFamily specifies the IP family to be used by the server. + // Default is "dual", meaning the server will use both IPv4 and IPv6. + // + // +optional + IPFamily IPFamilyType `json:"ipFamily,omitempty"` // DisableHTTP2 defines if http2 should be disabled for all servers. // Default is false, meaning http2 will be enabled for all servers. // diff --git a/charts/nginx-gateway-fabric/values.yaml b/charts/nginx-gateway-fabric/values.yaml index 827d414fab..9cfc2064b2 100644 --- a/charts/nginx-gateway-fabric/values.yaml +++ b/charts/nginx-gateway-fabric/values.yaml @@ -92,6 +92,7 @@ nginx: config: {} # disableHTTP2: false + # ipFamily: dual # telemetry: # exporter: # endpoint: otel-collector.default.svc:4317 diff --git a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml index b42d562f75..a4534f4145 100644 --- a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml +++ b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml @@ -52,6 +52,15 @@ spec: DisableHTTP2 defines if http2 should be disabled for all servers. Default is false, meaning http2 will be enabled for all servers. type: boolean + ipFamily: + description: |- + IPFamily specifies the IP family to be used by the server. + Default is "dual", meaning the server will use both IPv4 and IPv6. + enum: + - dual + - ipv4 + - ipv6 + type: string telemetry: description: Telemetry specifies the OpenTelemetry configuration. properties: diff --git a/deploy/crds.yaml b/deploy/crds.yaml index 51ae8b34bf..6a8d14c83e 100644 --- a/deploy/crds.yaml +++ b/deploy/crds.yaml @@ -697,6 +697,15 @@ spec: DisableHTTP2 defines if http2 should be disabled for all servers. Default is false, meaning http2 will be enabled for all servers. type: boolean + ipFamily: + description: |- + IPFamily specifies the IP family to be used by the server. + Default is "dual", meaning the server will use both IPv4 and IPv6. + enum: + - dual + - ipv4 + - ipv6 + type: string telemetry: description: Telemetry specifies the OpenTelemetry configuration. properties: diff --git a/docs/developer/quickstart.md b/docs/developer/quickstart.md index 448123a973..76a13ee476 100644 --- a/docs/developer/quickstart.md +++ b/docs/developer/quickstart.md @@ -134,10 +134,29 @@ This will build the docker images `nginx-gateway-fabric:` and `nginx- 1. Create a `kind` cluster: + To create a `kind` cluster with IPv4 enabled: + ```makefile make create-kind-cluster ``` + To create a `kind` cluster with IPv6 or Dual IPFamily enabled, use this `config.yaml`: + + ```yaml + kind: Cluster + apiVersion: kind.x-k8s.io/v1alpha4 + nodes: + - role: control-plane + networking: + ipFamily: dual + # ipFamily: ipv6 + apiServerAddress: 127.0.0.1 + ``` + + ```shell + kind create cluster --config ~/cluster.yaml + ``` + 2. Load the previously built images onto your `kind` cluster: ```shell diff --git a/internal/mode/static/nginx/config/http/config.go b/internal/mode/static/nginx/config/http/config.go index 19e7db223d..198d295cc2 100644 --- a/internal/mode/static/nginx/config/http/config.go +++ b/internal/mode/static/nginx/config/http/config.go @@ -10,6 +10,13 @@ type Server struct { IsDefaultHTTP bool IsDefaultSSL bool GRPC bool + IPFamily IPFamily +} + +// IPFamily holds the IP family configuration for all servers. +type IPFamily struct { + IPv4 bool + IPv6 bool } // Location holds all configuration for an HTTP location. diff --git a/internal/mode/static/nginx/config/servers.go b/internal/mode/static/nginx/config/servers.go index 54bbe709ed..e61ff5bd31 100644 --- a/internal/mode/static/nginx/config/servers.go +++ b/internal/mode/static/nginx/config/servers.go @@ -8,6 +8,7 @@ import ( "strings" gotemplate "text/template" + ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" "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/state/dataplane" @@ -58,7 +59,8 @@ var grpcBaseHeaders = []http.Header{ } func executeServers(conf dataplane.Configuration) []executeResult { - servers, httpMatchPairs := createServers(conf.HTTPServers, conf.SSLServers) + ipFamily := getIPFamily(conf.BaseHTTPConfig) + servers, httpMatchPairs := createServers(conf.HTTPServers, conf.SSLServers, ipFamily) serverResult := executeResult{ dest: httpConfigFile, @@ -86,6 +88,18 @@ func executeServers(conf dataplane.Configuration) []executeResult { return allResults } +// getIPFamily returns whether the server should be configured for IPv4, IPv6, or both. +func getIPFamily(baseHTTPConfig dataplane.BaseHTTPConfig) http.IPFamily { + switch ip := baseHTTPConfig.IPFamily; ip { + case ngfAPI.IPv4: + return http.IPFamily{IPv4: true} + case ngfAPI.IPv6: + return http.IPFamily{IPv6: true} + } + + return http.IPFamily{IPv4: true, IPv6: true} +} + func createAdditionFileResults(conf dataplane.Configuration) []executeResult { uniqueAdditions := make(map[string][]byte) @@ -141,18 +155,22 @@ func createIncludes(additions []dataplane.Addition) []string { return includes } -func createServers(httpServers, sslServers []dataplane.VirtualServer) ([]http.Server, httpMatchPairs) { +func createServers( + httpServers, + sslServers []dataplane.VirtualServer, + ipFamily http.IPFamily, +) ([]http.Server, httpMatchPairs) { servers := make([]http.Server, 0, len(httpServers)+len(sslServers)) finalMatchPairs := make(httpMatchPairs) for serverID, s := range httpServers { - httpServer, matchPairs := createServer(s, serverID) + httpServer, matchPairs := createServer(s, serverID, ipFamily) servers = append(servers, httpServer) maps.Copy(finalMatchPairs, matchPairs) } for serverID, s := range sslServers { - sslServer, matchPair := createSSLServer(s, serverID) + sslServer, matchPair := createSSLServer(s, serverID, ipFamily) servers = append(servers, sslServer) maps.Copy(finalMatchPairs, matchPair) } @@ -160,11 +178,16 @@ func createServers(httpServers, sslServers []dataplane.VirtualServer) ([]http.Se return servers, finalMatchPairs } -func createSSLServer(virtualServer dataplane.VirtualServer, serverID int) (http.Server, httpMatchPairs) { +func createSSLServer( + virtualServer dataplane.VirtualServer, + serverID int, + ipFamily http.IPFamily, +) (http.Server, httpMatchPairs) { if virtualServer.IsDefault { return http.Server{ IsDefaultSSL: true, Port: virtualServer.Port, + IPFamily: ipFamily, }, nil } @@ -180,14 +203,20 @@ func createSSLServer(virtualServer dataplane.VirtualServer, serverID int) (http. Port: virtualServer.Port, GRPC: grpc, Includes: createIncludes(virtualServer.Additions), + IPFamily: ipFamily, }, matchPairs } -func createServer(virtualServer dataplane.VirtualServer, serverID int) (http.Server, httpMatchPairs) { +func createServer( + virtualServer dataplane.VirtualServer, + serverID int, + ipFamily http.IPFamily, +) (http.Server, httpMatchPairs) { if virtualServer.IsDefault { return http.Server{ IsDefaultHTTP: true, Port: virtualServer.Port, + IPFamily: ipFamily, }, nil } @@ -199,6 +228,7 @@ func createServer(virtualServer dataplane.VirtualServer, serverID int) (http.Ser Port: virtualServer.Port, GRPC: grpc, Includes: createIncludes(virtualServer.Additions), + IPFamily: ipFamily, }, matchPairs } diff --git a/internal/mode/static/nginx/config/servers_template.go b/internal/mode/static/nginx/config/servers_template.go index 465bd6fc81..22cd357ff5 100644 --- a/internal/mode/static/nginx/config/servers_template.go +++ b/internal/mode/static/nginx/config/servers_template.go @@ -5,13 +5,23 @@ js_preload_object matches from /etc/nginx/conf.d/matches.json; {{- range $s := . -}} {{ if $s.IsDefaultSSL -}} server { + {{- if $s.IPFamily.IPv4 }} listen {{ $s.Port }} ssl default_server; + {{- end }} + {{- if $s.IPFamily.IPv6 }} + listen [::]:{{ $s.Port }} ssl default_server; + {{- end }} ssl_reject_handshake on; } {{- else if $s.IsDefaultHTTP }} server { + {{- if $s.IPFamily.IPv4 }} listen {{ $s.Port }} default_server; + {{- end }} + {{- if $s.IPFamily.IPv6 }} + listen [::]:{{ $s.Port }} default_server; + {{- end }} default_type text/html; return 404; @@ -19,7 +29,12 @@ server { {{- else }} server { {{- if $s.SSL }} + {{- if $s.IPFamily.IPv4 }} listen {{ $s.Port }} ssl; + {{- end }} + {{- if $s.IPFamily.IPv6 }} + listen [::]:{{ $s.Port }} ssl; + {{- end }} ssl_certificate {{ $s.SSL.Certificate }}; ssl_certificate_key {{ $s.SSL.CertificateKey }}; @@ -27,8 +42,13 @@ server { return 421; } {{- else }} + {{- if $s.IPFamily.IPv4 }} listen {{ $s.Port }}; {{- end }} + {{- if $s.IPFamily.IPv6 }} + listen [::]:{{ $s.Port }}; + {{- end }} + {{- end }} server_name {{ $s.ServerName }}; diff --git a/internal/mode/static/nginx/config/servers_test.go b/internal/mode/static/nginx/config/servers_test.go index ae458e8f03..e8eeff37f0 100644 --- a/internal/mode/static/nginx/config/servers_test.go +++ b/internal/mode/static/nginx/config/servers_test.go @@ -9,131 +9,198 @@ import ( . "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/types" + ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" "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/state/dataplane" ) func TestExecuteServers(t *testing.T) { - conf := dataplane.Configuration{ - HTTPServers: []dataplane.VirtualServer{ - { - IsDefault: true, - Port: 8080, - }, - { - Hostname: "example.com", - Port: 8080, - }, - { - Hostname: "cafe.example.com", - Port: 8080, - Additions: []dataplane.Addition{ + tests := []struct { + msg string + expectedHTTPConfig map[string]int + expectedHTTPMatchVars string + expectedIncludedFileConfigs map[string]string + config dataplane.Configuration + }{ + { + msg: "http and ssl servers with IPv4 IP family", + config: dataplane.Configuration{ + HTTPServers: []dataplane.VirtualServer{ + { + IsDefault: true, + Port: 8080, + }, { - Bytes: []byte("addition-1"), - Identifier: "addition-1", + Hostname: "example.com", + Port: 8080, }, }, - }, - }, - SSLServers: []dataplane.VirtualServer{ - { - IsDefault: true, - Port: 8443, - }, - { - Hostname: "example.com", - SSL: &dataplane.SSL{ - KeyPairID: "test-keypair", + SSLServers: []dataplane.VirtualServer{ + { + IsDefault: true, + Port: 8443, + }, + { + Hostname: "example.com", + SSL: &dataplane.SSL{ + KeyPairID: "test-keypair", + }, + Port: 8443, + }, + }, + BaseHTTPConfig: dataplane.BaseHTTPConfig{ + IPFamily: ngfAPI.IPv4, }, - Port: 8443, }, - { - Hostname: "cafe.example.com", - SSL: &dataplane.SSL{ - KeyPairID: "test-keypair", + 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, + }, + expectedHTTPMatchVars: "{}", + }, + { + msg: "http and ssl servers with dual IP family", + config: dataplane.Configuration{ + HTTPServers: []dataplane.VirtualServer{ + { + IsDefault: true, + Port: 8080, + }, + { + Hostname: "example.com", + Port: 8080, + }, + { + Hostname: "cafe.example.com", + Port: 8080, + Additions: []dataplane.Addition{ + { + Bytes: []byte("addition-1"), + Identifier: "addition-1", + }, + }, + }, }, - Port: 8443, - PathRules: []dataplane.PathRule{ + SSLServers: []dataplane.VirtualServer{ { - Path: "/", - PathType: dataplane.PathTypePrefix, - MatchRules: []dataplane.MatchRule{ + IsDefault: true, + Port: 8443, + }, + { + Hostname: "example.com", + SSL: &dataplane.SSL{ + KeyPairID: "test-keypair", + }, + Port: 8443, + }, + { + Hostname: "cafe.example.com", + SSL: &dataplane.SSL{ + KeyPairID: "test-keypair", + }, + Port: 8443, + PathRules: []dataplane.PathRule{ { - Match: dataplane.Match{}, - BackendGroup: dataplane.BackendGroup{ - Source: types.NamespacedName{Namespace: "test", Name: "route1"}, - RuleIdx: 0, - Backends: []dataplane.Backend{ - { - UpstreamName: "test_foo_443", - Valid: true, - Weight: 1, - VerifyTLS: &dataplane.VerifyTLS{ - CertBundleID: "test-foo", - Hostname: "test-foo.example.com", + Path: "/", + PathType: dataplane.PathTypePrefix, + MatchRules: []dataplane.MatchRule{ + { + Match: dataplane.Match{}, + BackendGroup: dataplane.BackendGroup{ + Source: types.NamespacedName{Namespace: "test", Name: "route1"}, + RuleIdx: 0, + Backends: []dataplane.Backend{ + { + UpstreamName: "test_foo_443", + Valid: true, + Weight: 1, + VerifyTLS: &dataplane.VerifyTLS{ + CertBundleID: "test-foo", + Hostname: "test-foo.example.com", + }, + }, }, }, }, }, }, }, + Additions: []dataplane.Addition{ + { + Bytes: []byte("addition-1"), + Identifier: "addition-1", // duplicate + }, + { + Bytes: []byte("addition-2"), + Identifier: "addition-2", + }, + }, }, }, - Additions: []dataplane.Addition{ - { - Bytes: []byte("addition-1"), - Identifier: "addition-1", // duplicate - }, - { - Bytes: []byte("addition-2"), - Identifier: "addition-2", - }, + BaseHTTPConfig: dataplane.BaseHTTPConfig{ + IPFamily: ngfAPI.Dual, }, }, + expectedHTTPConfig: map[string]int{ + "listen 8080 default_server;": 1, + "listen [::]:8080 default_server;": 1, + "listen 8080;": 2, + "listen [::]:8080;": 2, + "listen 8443 ssl;": 2, + "listen [::]:8443 ssl;": 2, + "listen 8443 ssl default_server;": 1, + "listen [::]:8443 ssl default_server;": 1, + "server_name example.com;": 2, + "server_name cafe.example.com;": 2, + "ssl_certificate /etc/nginx/secrets/test-keypair.pem;": 2, + "ssl_certificate_key /etc/nginx/secrets/test-keypair.pem;": 2, + "proxy_ssl_server_name on;": 1, + }, + expectedHTTPMatchVars: "{}", + expectedIncludedFileConfigs: map[string]string{ + includesFolder + "/addition-1.conf": "addition-1", + includesFolder + "/addition-2.conf": "addition-2", + }, }, } - expSubStrings := map[string]int{ - "listen 8080 default_server;": 1, - "listen 8080;": 2, - "listen 8443 ssl;": 2, - "listen 8443 ssl default_server;": 1, - "server_name example.com;": 2, - "server_name cafe.example.com;": 2, - "ssl_certificate /etc/nginx/secrets/test-keypair.pem;": 2, - "ssl_certificate_key /etc/nginx/secrets/test-keypair.pem;": 2, - "proxy_ssl_server_name on;": 1, - } - type assertion func(g *WithT, data string) - - expectedResults := map[string]assertion{ - httpConfigFile: func(g *WithT, data string) { - for expSubStr, expCount := range expSubStrings { - g.Expect(strings.Count(data, expSubStr)).To(Equal(expCount)) + for _, test := range tests { + t.Run(test.msg, func(t *testing.T) { + g := NewWithT(t) + results := executeServers(test.config) + expectedResults := map[string]assertion{ + httpConfigFile: func(g *WithT, data string) { + for expSubStr, expCount := range test.expectedHTTPConfig { + g.Expect(strings.Count(data, expSubStr)).To(Equal(expCount)) + } + }, + httpMatchVarsFile: func(g *WithT, data string) { + g.Expect(data).To(Equal(test.expectedHTTPMatchVars)) + }, } - }, - httpMatchVarsFile: func(g *WithT, data string) { - g.Expect(data).To(Equal("{}")) - }, - includesFolder + "/addition-1.conf": func(g *WithT, data string) { - g.Expect(data).To(Equal("addition-1")) - }, - includesFolder + "/addition-2.conf": func(g *WithT, data string) { - g.Expect(data).To(Equal("addition-2")) - }, - } - g := NewWithT(t) - results := executeServers(conf) - g.Expect(results).To(HaveLen(len(expectedResults))) + for file, assertData := range test.expectedIncludedFileConfigs { + expectedResults[file] = func(g *WithT, data string) { + g.Expect(data).To(Equal(assertData)) + } + } - for _, res := range results { - g.Expect(expectedResults).To(HaveKey(res.dest), "executeServers returned unexpected result destination") + g.Expect(results).To(HaveLen(len(expectedResults))) - assertData := expectedResults[res.dest] - assertData(g, string(res.data)) + for _, res := range results { + g.Expect(expectedResults).To(HaveKey(res.dest), "executeServers returned unexpected result destination") + assertData := expectedResults[res.dest] + assertData(g, string(res.data)) + } + }) } } @@ -1025,6 +1092,7 @@ func TestCreateServers(t *testing.T) { { IsDefaultHTTP: true, Port: 8080, + IPFamily: http.IPFamily{}, }, { ServerName: "cafe.example.com", @@ -1035,10 +1103,12 @@ func TestCreateServers(t *testing.T) { includesFolder + "/server-addition-1.conf", includesFolder + "/server-addition-2.conf", }, + IPFamily: http.IPFamily{}, }, { IsDefaultSSL: true, Port: 8443, + IPFamily: http.IPFamily{}, }, { ServerName: "cafe.example.com", @@ -1053,12 +1123,13 @@ func TestCreateServers(t *testing.T) { includesFolder + "/server-addition-1.conf", includesFolder + "/server-addition-3.conf", }, + IPFamily: http.IPFamily{}, }, } g := NewWithT(t) - result, httpMatchPair := createServers(httpServers, sslServers) + result, httpMatchPair := createServers(httpServers, sslServers, http.IPFamily{}) g.Expect(httpMatchPair).To(Equal(allExpMatchPair)) g.Expect(helpers.Diff(expectedServers, result)).To(BeEmpty()) @@ -1267,7 +1338,7 @@ func TestCreateServersConflicts(t *testing.T) { g := NewWithT(t) - result, _ := createServers(httpServers, []dataplane.VirtualServer{}) + result, _ := createServers(httpServers, []dataplane.VirtualServer{}, http.IPFamily{}) g.Expect(helpers.Diff(expectedServers, result)).To(BeEmpty()) }) } diff --git a/internal/mode/static/nginx/config/upstreams.go b/internal/mode/static/nginx/config/upstreams.go index 2e1906234e..e1c6a7ae42 100644 --- a/internal/mode/static/nginx/config/upstreams.go +++ b/internal/mode/static/nginx/config/upstreams.go @@ -71,6 +71,11 @@ func (g GeneratorImpl) createUpstream(up dataplane.Upstream) http.Upstream { upstreamServers[idx] = http.UpstreamServer{ Address: fmt.Sprintf("%s:%d", ep.Address, ep.Port), } + if ep.IPv6 { + upstreamServers[idx] = http.UpstreamServer{ + Address: fmt.Sprintf("[%s]:%d", ep.Address, ep.Port), + } + } } return http.Upstream{ diff --git a/internal/mode/static/nginx/config/upstreams_test.go b/internal/mode/static/nginx/config/upstreams_test.go index b0a631858d..351345a21f 100644 --- a/internal/mode/static/nginx/config/upstreams_test.go +++ b/internal/mode/static/nginx/config/upstreams_test.go @@ -35,15 +35,27 @@ func TestExecuteUpstreams(t *testing.T) { Name: "up3", Endpoints: []resolver.Endpoint{}, }, + { + Name: "up4", + Endpoints: []resolver.Endpoint{ + { + Address: "2001:db8::1", + Port: 80, + IPv6: true, + }, + }, + }, } expectedSubStrings := []string{ "upstream up1", "upstream up2", "upstream up3", + "upstream up4", "upstream invalid-backend-ref", "server 10.0.0.0:80;", "server 11.0.0.0:80;", + "server [2001:db8::1]:80", "server unix:/var/run/nginx/nginx-502-server.sock;", } @@ -91,6 +103,16 @@ func TestCreateUpstreams(t *testing.T) { Name: "up3", Endpoints: []resolver.Endpoint{}, }, + { + Name: "up4", + Endpoints: []resolver.Endpoint{ + { + Address: "fd00:10:244:1::7", + Port: 80, + IPv6: true, + }, + }, + }, } expUpstreams := []http.Upstream{ @@ -127,6 +149,15 @@ func TestCreateUpstreams(t *testing.T) { }, }, }, + { + Name: "up4", + ZoneSize: ossZoneSize, + Servers: []http.UpstreamServer{ + { + Address: "[fd00:10:244:1::7]:80", + }, + }, + }, { Name: invalidBackendRef, Servers: []http.UpstreamServer{ @@ -216,6 +247,28 @@ func TestCreateUpstream(t *testing.T) { }, msg: "multiple endpoints", }, + { + stateUpstream: dataplane.Upstream{ + Name: "endpoint-ipv6", + Endpoints: []resolver.Endpoint{ + { + Address: "fd00:10:244:1::7", + Port: 80, + IPv6: true, + }, + }, + }, + expectedUpstream: http.Upstream{ + Name: "endpoint-ipv6", + ZoneSize: ossZoneSize, + Servers: []http.UpstreamServer{ + { + Address: "[fd00:10:244:1::7]:80", + }, + }, + }, + msg: "endpoint ipv6", + }, } for _, test := range tests { diff --git a/internal/mode/static/state/dataplane/configuration.go b/internal/mode/static/state/dataplane/configuration.go index 56569aa432..a43f338f57 100644 --- a/internal/mode/static/state/dataplane/configuration.go +++ b/internal/mode/static/state/dataplane/configuration.go @@ -658,7 +658,8 @@ func buildTelemetry(g *graph.Graph) Telemetry { func buildBaseHTTPConfig(g *graph.Graph) BaseHTTPConfig { baseConfig := BaseHTTPConfig{ // HTTP2 should be enabled by default - HTTP2: true, + HTTP2: true, + IPFamily: ngfAPI.Dual, } if g.NginxProxy == nil || !g.NginxProxy.Valid { return baseConfig @@ -668,6 +669,13 @@ func buildBaseHTTPConfig(g *graph.Graph) BaseHTTPConfig { baseConfig.HTTP2 = false } + switch ipFamily := g.NginxProxy.Source.Spec.IPFamily; ipFamily { + case ngfAPI.IPv4: + baseConfig.IPFamily = ngfAPI.IPv4 + case ngfAPI.IPv6: + baseConfig.IPFamily = ngfAPI.IPv6 + } + return baseConfig } diff --git a/internal/mode/static/state/dataplane/configuration_test.go b/internal/mode/static/state/dataplane/configuration_test.go index e053a42d96..8580b01b73 100644 --- a/internal/mode/static/state/dataplane/configuration_test.go +++ b/internal/mode/static/state/dataplane/configuration_test.go @@ -633,6 +633,27 @@ func TestBuildConfiguration(t *testing.T) { ServiceName: helpers.GetPointer("my-svc"), }, DisableHTTP2: true, + IPFamily: ngfAPI.Dual, + }, + }, + Valid: true, + } + + nginxProxyIPv4 := &graph.NginxProxy{ + Source: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + Telemetry: &ngfAPI.Telemetry{}, + IPFamily: ngfAPI.IPv4, + }, + }, + Valid: true, + } + + nginxProxyIPv6 := &graph.NginxProxy{ + Source: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + Telemetry: &ngfAPI.Telemetry{}, + IPFamily: ngfAPI.IPv6, }, }, Valid: true, @@ -660,7 +681,7 @@ func TestBuildConfiguration(t *testing.T) { SSLServers: []VirtualServer{}, SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{}, CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.Dual}, }, msg: "no listeners and routes", }, @@ -693,7 +714,7 @@ func TestBuildConfiguration(t *testing.T) { SSLServers: []VirtualServer{}, SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{}, CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.Dual}, }, msg: "http listener with no routes", }, @@ -757,7 +778,7 @@ func TestBuildConfiguration(t *testing.T) { }, }, CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.Dual}, }, msg: "http and https listeners with no valid routes", }, @@ -821,7 +842,7 @@ func TestBuildConfiguration(t *testing.T) { }, }, CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.Dual}, }, msg: "https listeners with no routes", }, @@ -855,7 +876,7 @@ func TestBuildConfiguration(t *testing.T) { SSLServers: []VirtualServer{}, SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{}, CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.Dual}, }, msg: "invalid https listener with resolved secret", }, @@ -928,7 +949,7 @@ func TestBuildConfiguration(t *testing.T) { BackendGroups: []BackendGroup{expHR1Groups[0], expHR2Groups[0]}, SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{}, CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.Dual}, }, msg: "one http listener with two routes for different hostnames", }, @@ -984,7 +1005,7 @@ func TestBuildConfiguration(t *testing.T) { BackendGroups: []BackendGroup{expGRGroups[0]}, SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{}, CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.Dual}, }, msg: "one http listener with one grpc route", }, @@ -1105,7 +1126,7 @@ func TestBuildConfiguration(t *testing.T) { }, }, CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.Dual}, }, msg: "two https listeners each with routes for different hostnames", }, @@ -1266,7 +1287,7 @@ func TestBuildConfiguration(t *testing.T) { }, }, CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.Dual}, }, msg: "one http and one https listener with two routes with the same hostname with and without collisions", }, @@ -1480,7 +1501,7 @@ func TestBuildConfiguration(t *testing.T) { }, }, CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.Dual}, }, msg: "multiple http and https listener; different ports", @@ -1613,7 +1634,7 @@ func TestBuildConfiguration(t *testing.T) { BackendGroups: []BackendGroup{expHR5Groups[0], expHR5Groups[1]}, SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{}, CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.Dual}, }, msg: "one http listener with one route with filters", }, @@ -1716,7 +1737,7 @@ func TestBuildConfiguration(t *testing.T) { }, }, CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.Dual}, }, msg: "one http and one https listener with routes with valid and invalid rules", }, @@ -1781,7 +1802,7 @@ func TestBuildConfiguration(t *testing.T) { BackendGroups: []BackendGroup{expHR7Groups[0], expHR7Groups[1]}, SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{}, CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.Dual}, }, msg: "duplicate paths with different types", }, @@ -1870,7 +1891,7 @@ func TestBuildConfiguration(t *testing.T) { }, }, CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.Dual}, }, msg: "two https listeners with different hostnames but same route; chooses listener with more specific hostname", }, @@ -1947,7 +1968,7 @@ func TestBuildConfiguration(t *testing.T) { CertBundles: map[CertBundleID]CertBundle{ "cert_bundle_test_configmap-1": []byte("cert-1"), }, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.Dual}, }, msg: "https listener with httproute with backend that has a backend TLS policy attached", }, @@ -2024,7 +2045,7 @@ func TestBuildConfiguration(t *testing.T) { CertBundles: map[CertBundleID]CertBundle{ "cert_bundle_test_configmap-2": []byte("cert-2"), }, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.Dual}, }, msg: "https listener with httproute with backend that has a backend TLS policy with binaryData attached", }, @@ -2063,7 +2084,7 @@ func TestBuildConfiguration(t *testing.T) { SSLServers: []VirtualServer{}, SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{}, CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: false}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: false, IPFamily: ngfAPI.Dual}, Telemetry: Telemetry{ Endpoint: "my-otel.svc:4563", Interval: "5s", @@ -2103,6 +2124,7 @@ func TestBuildConfiguration(t *testing.T) { Source: &ngfAPI.NginxProxy{ Spec: ngfAPI.NginxProxySpec{ DisableHTTP2: true, + IPFamily: ngfAPI.Dual, Telemetry: &ngfAPI.Telemetry{ Exporter: &ngfAPI.TelemetryExporter{ Endpoint: "some-endpoint", @@ -2122,7 +2144,7 @@ func TestBuildConfiguration(t *testing.T) { SSLServers: []VirtualServer{}, SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{}, CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.Dual}, Telemetry: Telemetry{}, }, msg: "invalid NginxProxy", @@ -2289,12 +2311,90 @@ func TestBuildConfiguration(t *testing.T) { expHRWithPolicyGroups[0], expHTTPSHRWithPolicyGroups[0], }, - BaseHTTPConfig: BaseHTTPConfig{ - HTTP2: true, - }, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.Dual}, }, msg: "Simple Gateway and HTTPRoute with policies attached", }, + { + graph: &graph.Graph{ + GatewayClass: &graph.GatewayClass{ + Source: &v1.GatewayClass{}, + Valid: true, + }, + Gateway: &graph.Gateway{ + Source: &v1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gw", + Namespace: "ns", + }, + }, + Listeners: []*graph.Listener{ + { + Name: "listener-80-1", + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{}, + }, + }, + }, + Routes: map[graph.RouteKey]*graph.L7Route{}, + NginxProxy: nginxProxyIPv4, + }, + expConf: Configuration{ + HTTPServers: []VirtualServer{ + { + IsDefault: true, + Port: 80, + }, + }, + SSLServers: []VirtualServer{}, + SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{}, + CertBundles: map[CertBundleID]CertBundle{}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.IPv4}, + Telemetry: Telemetry{}, + }, + msg: "NginxProxy with IPv4 IPFamily and no routes", + }, + { + graph: &graph.Graph{ + GatewayClass: &graph.GatewayClass{ + Source: &v1.GatewayClass{}, + Valid: true, + }, + Gateway: &graph.Gateway{ + Source: &v1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gw", + Namespace: "ns", + }, + }, + Listeners: []*graph.Listener{ + { + Name: "listener-80-1", + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{}, + }, + }, + }, + Routes: map[graph.RouteKey]*graph.L7Route{}, + NginxProxy: nginxProxyIPv6, + }, + expConf: Configuration{ + HTTPServers: []VirtualServer{ + { + IsDefault: true, + Port: 80, + }, + }, + SSLServers: []VirtualServer{}, + SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{}, + CertBundles: map[CertBundleID]CertBundle{}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.IPv6}, + Telemetry: Telemetry{}, + }, + msg: "NginxProxy with IPv6 IPFamily", + }, } for _, test := range tests { diff --git a/internal/mode/static/state/dataplane/types.go b/internal/mode/static/state/dataplane/types.go index 77cd66c008..3fb99da50d 100644 --- a/internal/mode/static/state/dataplane/types.go +++ b/internal/mode/static/state/dataplane/types.go @@ -6,6 +6,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/resolver" ) @@ -33,10 +34,10 @@ type Configuration struct { Upstreams []Upstream // BackendGroups holds all unique BackendGroups. BackendGroups []BackendGroup - // Telemetry holds the Otel configuration. - Telemetry Telemetry // BaseHTTPConfig holds the configuration options at the http context. BaseHTTPConfig BaseHTTPConfig + // Telemetry holds the Otel configuration. + Telemetry Telemetry // Version represents the version of the generated configuration. Version int } @@ -296,6 +297,8 @@ type SpanAttribute struct { // BaseHTTPConfig holds the configuration options at the http context. type BaseHTTPConfig struct { + // IPFamily specifies the IP family for all servers. + IPFamily ngfAPI.IPFamilyType // HTTP2 specifies whether http2 should be enabled for all servers. HTTP2 bool } diff --git a/internal/mode/static/state/resolver/resolver.go b/internal/mode/static/state/resolver/resolver.go index 1200add7f3..16d62189a6 100644 --- a/internal/mode/static/state/resolver/resolver.go +++ b/internal/mode/static/state/resolver/resolver.go @@ -28,6 +28,8 @@ type Endpoint struct { Address string // Port is the port of the endpoint. Port int32 + // IPv6 is true if the endpoint is an IPv6 address. + IPv6 bool } // ServiceResolverImpl implements ServiceResolver. @@ -109,6 +111,10 @@ func resolveEndpoints( endpointSet := initEndpointsSet(filteredSlices) for _, eps := range filteredSlices { + var ipv6 bool + if eps.AddressType == discoveryV1.AddressTypeIPv6 { + ipv6 = true + } for _, endpoint := range eps.Endpoints { if !endpointReady(endpoint) { continue @@ -119,7 +125,7 @@ func resolveEndpoints( endpointPort := findPort(eps.Ports, svcPort) for _, address := range endpoint.Addresses { - ep := Endpoint{Address: address, Port: endpointPort} + ep := Endpoint{Address: address, Port: endpointPort, IPv6: ipv6} endpointSet[ep] = struct{}{} } } @@ -149,7 +155,7 @@ func getDefaultPort(svcPort v1.ServicePort) int32 { } func ignoreEndpointSlice(endpointSlice discoveryV1.EndpointSlice, port v1.ServicePort) bool { - if endpointSlice.AddressType != discoveryV1.AddressTypeIPv4 { + if endpointSlice.AddressType == discoveryV1.AddressTypeFQDN { return true } diff --git a/internal/mode/static/state/resolver/resolver_test.go b/internal/mode/static/state/resolver/resolver_test.go index 1b7c248621..c6ff068633 100644 --- a/internal/mode/static/state/resolver/resolver_test.go +++ b/internal/mode/static/state/resolver/resolver_test.go @@ -16,13 +16,19 @@ import ( var ( svcPortName = "svc-port" - addresses = []string{"10.0.0.1", "10.0.0.2", "10.0.0.3"} + addresses = []string{"10.0.0.1", "10.0.0.2", "10.0.0.3"} + addressesIPv6 = []string{"2001:db8::1", "2001:db8::2", "2001:db8::3"} readyEndpoint1 = discoveryV1.Endpoint{ Addresses: addresses, Conditions: discoveryV1.EndpointConditions{Ready: helpers.GetPointer(true)}, } + readyEndpoint2 = discoveryV1.Endpoint{ + Addresses: addressesIPv6, + Conditions: discoveryV1.EndpointConditions{Ready: helpers.GetPointer(true)}, + } + notReadyEndpoint = discoveryV1.Endpoint{ Addresses: addresses, Conditions: discoveryV1.EndpointConditions{Ready: helpers.GetPointer(false)}, @@ -59,8 +65,19 @@ var ( }, } - invalidAddressTypeEndpointSlice = discoveryV1.EndpointSlice{ + validIPv6EndpointSlice = discoveryV1.EndpointSlice{ AddressType: discoveryV1.AddressTypeIPv6, + Endpoints: []discoveryV1.Endpoint{readyEndpoint2}, + Ports: []discoveryV1.EndpointPort{ + { + Name: &svcPortName, + Port: helpers.GetPointer[int32](80), + }, + }, + } + + invalidAddressTypeEndpointSlice = discoveryV1.EndpointSlice{ + AddressType: discoveryV1.AddressTypeFQDN, Endpoints: []discoveryV1.Endpoint{readyEndpoint1}, Ports: []discoveryV1.EndpointPort{ { @@ -86,6 +103,7 @@ func TestFilterEndpointSliceList(t *testing.T) { sliceList := discoveryV1.EndpointSliceList{ Items: []discoveryV1.EndpointSlice{ validEndpointSlice, + validIPv6EndpointSlice, invalidAddressTypeEndpointSlice, invalidPortEndpointSlice, nilEndpoints, @@ -99,7 +117,7 @@ func TestFilterEndpointSliceList(t *testing.T) { TargetPort: intstr.FromInt(80), } - expFilteredList := []discoveryV1.EndpointSlice{validEndpointSlice, mixedValidityEndpointSlice} + expFilteredList := []discoveryV1.EndpointSlice{validEndpointSlice, validIPv6EndpointSlice, mixedValidityEndpointSlice} filteredSliceList := filterEndpointSliceList(sliceList, svcPort) g := NewWithT(t) @@ -171,7 +189,7 @@ func TestIgnoreEndpointSlice(t *testing.T) { Port: 80, TargetPort: intstr.FromInt(8080), }, - ignore: true, + ignore: false, }, { msg: "FQDN address type", diff --git a/internal/mode/static/state/resolver/service_resolver_test.go b/internal/mode/static/state/resolver/service_resolver_test.go index ef605eb5ba..83573d51c5 100644 --- a/internal/mode/static/state/resolver/service_resolver_test.go +++ b/internal/mode/static/state/resolver/service_resolver_test.go @@ -196,6 +196,11 @@ var _ = Describe("ServiceResolver", func() { Address: "12.0.0.1", Port: 8080, }, + { + Address: "FE80:CD00:0:CDE:1257:0:211E:729C", + Port: 8080, + IPv6: true, + }, } endpoints, err := serviceResolver.Resolve(context.TODO(), svcNsName, svcPort) @@ -207,6 +212,7 @@ var _ = Describe("ServiceResolver", func() { Expect(fakeK8sClient.Delete(context.TODO(), slice1)).To(Succeed()) Expect(fakeK8sClient.Delete(context.TODO(), slice2)).To(Succeed()) Expect(fakeK8sClient.Delete(context.TODO(), dupeEndpointSlice)).To(Succeed()) + Expect(fakeK8sClient.Delete(context.TODO(), sliceIPV6)).To(Succeed()) endpoints, err := serviceResolver.Resolve(context.TODO(), svcNsName, svcPort) Expect(err).To(HaveOccurred()) @@ -214,7 +220,6 @@ var _ = Describe("ServiceResolver", func() { }) It("returns an error if there are no endpoint slices for the service", func() { // delete remaining endpoint slices - Expect(fakeK8sClient.Delete(context.TODO(), sliceIPV6)).To(Succeed()) Expect(fakeK8sClient.Delete(context.TODO(), sliceNoMatchingPortName)).To(Succeed()) endpoints, err := serviceResolver.Resolve(context.TODO(), svcNsName, svcPort) diff --git a/site/content/reference/api.md b/site/content/reference/api.md index fef67d181f..1f33b3419b 100644 --- a/site/content/reference/api.md +++ b/site/content/reference/api.md @@ -313,6 +313,21 @@ Telemetry +ipFamily
+ + +IPFamilyType + + + + +(Optional) +

IPFamily specifies the IP family to be used by the server. +Default is “dual”, meaning the server will use both IPv4 and IPv6.

+ + + + disableHTTP2
bool @@ -715,6 +730,34 @@ Support: Gateway, HTTPRoute, GRPCRoute.

Duration can be specified in milliseconds (ms) or seconds (s) A value without a suffix is seconds. Examples: 120s, 50ms.

+

IPFamilyType +(string alias)

+

+

+(Appears on: +NginxProxySpec) +

+

+

IPFamilyType specifies the IP family to be used by the server.

+

+ + + + + + + + + + + + + + +
ValueDescription

"dual"

Dual specifies that the server will use both IPv4 and IPv6.

+

"ipv4"

IPv4 specifies that the server will use only IPv4.

+

"ipv6"

IPv6 specifies that the server will use only IPv6.

+

Logging

@@ -892,6 +935,21 @@ Telemetry +ipFamily
+ + +IPFamilyType + + + + +(Optional) +

IPFamily specifies the IP family to be used by the server. +Default is “dual”, meaning the server will use both IPv4 and IPv6.

+ + + + disableHTTP2
bool From 1fe7bb162f7d2c220e327b6bf6b05786cf91db19 Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Mon, 8 Jul 2024 16:12:54 -0600 Subject: [PATCH 02/28] Update apis/v1alpha1/nginxproxy_types.go Co-authored-by: Michael Pleshakov --- apis/v1alpha1/nginxproxy_types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apis/v1alpha1/nginxproxy_types.go b/apis/v1alpha1/nginxproxy_types.go index 50818c7425..7d60cfae91 100644 --- a/apis/v1alpha1/nginxproxy_types.go +++ b/apis/v1alpha1/nginxproxy_types.go @@ -27,7 +27,7 @@ type NginxProxyList struct { Items []NginxProxy `json:"items"` } -// IPFamilyType specifies the IP family to be used by the server. +// IPFamilyType specifies the IP family to be used by NGINX. // // +kubebuilder:validation:Enum=dual;ipv4;ipv6 type IPFamilyType string From caafe3340a82265eeeeb410780839ced3c3729de Mon Sep 17 00:00:00 2001 From: Saloni Date: Tue, 9 Jul 2024 13:45:13 -0600 Subject: [PATCH 03/28] updates based on reviews --- apis/v1alpha1/nginxproxy_types.go | 15 +++--- apis/v1alpha1/zz_generated.deepcopy.go | 7 ++- .../mode/static/nginx/config/http/config.go | 1 - internal/mode/static/nginx/config/servers.go | 45 +++++++---------- .../static/nginx/config/servers_template.go | 18 +++---- .../mode/static/nginx/config/servers_test.go | 13 ++--- .../mode/static/nginx/config/upstreams.go | 11 ++-- .../static/state/dataplane/configuration.go | 8 +-- .../state/dataplane/configuration_test.go | 50 +++++++++---------- internal/mode/static/state/dataplane/types.go | 15 +++++- .../mode/static/state/resolver/resolver.go | 5 +- .../static/state/resolver/resolver_test.go | 30 +++++------ site/content/reference/api.md | 42 ++++++++-------- 13 files changed, 130 insertions(+), 130 deletions(-) diff --git a/apis/v1alpha1/nginxproxy_types.go b/apis/v1alpha1/nginxproxy_types.go index 7d60cfae91..5445eb945f 100644 --- a/apis/v1alpha1/nginxproxy_types.go +++ b/apis/v1alpha1/nginxproxy_types.go @@ -10,12 +10,13 @@ import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" // NginxProxy is a configuration object that is attached to a GatewayClass parametersRef. It provides a way // to configure global settings for all Gateways defined from the GatewayClass. -type NginxProxy struct { - metav1.TypeMeta `json:",inline"` +type NginxProxy struct { //nolint:govet // standard field alignment, don't change it metav1.ObjectMeta `json:"metadata,omitempty"` // Spec defines the desired state of the NginxProxy. Spec NginxProxySpec `json:"spec"` + + metav1.TypeMeta `json:",inline"` } // +kubebuilder:object:root=true @@ -43,15 +44,15 @@ const ( // NginxProxySpec defines the desired state of the NginxProxy. type NginxProxySpec struct { - // Telemetry specifies the OpenTelemetry configuration. - // - // +optional - Telemetry *Telemetry `json:"telemetry,omitempty"` // IPFamily specifies the IP family to be used by the server. // Default is "dual", meaning the server will use both IPv4 and IPv6. // // +optional - IPFamily IPFamilyType `json:"ipFamily,omitempty"` + IPFamily *IPFamilyType `json:"ipFamily,omitempty"` + // Telemetry specifies the OpenTelemetry configuration. + // + // +optional + Telemetry *Telemetry `json:"telemetry,omitempty"` // DisableHTTP2 defines if http2 should be disabled for all servers. // Default is false, meaning http2 will be enabled for all servers. // diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go index a341140561..0c61370b0c 100644 --- a/apis/v1alpha1/zz_generated.deepcopy.go +++ b/apis/v1alpha1/zz_generated.deepcopy.go @@ -299,9 +299,9 @@ func (in *NginxGatewayStatus) DeepCopy() *NginxGatewayStatus { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NginxProxy) DeepCopyInto(out *NginxProxy) { *out = *in - out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) in.Spec.DeepCopyInto(&out.Spec) + out.TypeMeta = in.TypeMeta } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NginxProxy. @@ -362,6 +362,11 @@ func (in *NginxProxySpec) DeepCopyInto(out *NginxProxySpec) { *out = new(Telemetry) (*in).DeepCopyInto(*out) } + if in.IPFamily != nil { + in, out := &in.IPFamily, &out.IPFamily + *out = new(IPFamilyType) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NginxProxySpec. diff --git a/internal/mode/static/nginx/config/http/config.go b/internal/mode/static/nginx/config/http/config.go index 198d295cc2..98594b25c4 100644 --- a/internal/mode/static/nginx/config/http/config.go +++ b/internal/mode/static/nginx/config/http/config.go @@ -10,7 +10,6 @@ type Server struct { IsDefaultHTTP bool IsDefaultSSL bool GRPC bool - IPFamily IPFamily } // IPFamily holds the IP family configuration for all servers. diff --git a/internal/mode/static/nginx/config/servers.go b/internal/mode/static/nginx/config/servers.go index e61ff5bd31..a9aa55521f 100644 --- a/internal/mode/static/nginx/config/servers.go +++ b/internal/mode/static/nginx/config/servers.go @@ -8,7 +8,6 @@ import ( "strings" gotemplate "text/template" - ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" "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/state/dataplane" @@ -58,13 +57,23 @@ var grpcBaseHeaders = []http.Header{ }, } +type serverConfig struct { + Servers []http.Server + IPFamily http.IPFamily +} + func executeServers(conf dataplane.Configuration) []executeResult { ipFamily := getIPFamily(conf.BaseHTTPConfig) - servers, httpMatchPairs := createServers(conf.HTTPServers, conf.SSLServers, ipFamily) + servers, httpMatchPairs := createServers(conf.HTTPServers, conf.SSLServers) + + serverConfig := serverConfig{ + Servers: servers, + IPFamily: ipFamily, + } serverResult := executeResult{ dest: httpConfigFile, - data: helpers.MustExecuteTemplate(serversTemplate, servers), + data: helpers.MustExecuteTemplate(serversTemplate, serverConfig), } // create httpMatchPair conf @@ -91,9 +100,9 @@ func executeServers(conf dataplane.Configuration) []executeResult { // getIPFamily returns whether the server should be configured for IPv4, IPv6, or both. func getIPFamily(baseHTTPConfig dataplane.BaseHTTPConfig) http.IPFamily { switch ip := baseHTTPConfig.IPFamily; ip { - case ngfAPI.IPv4: + case dataplane.IPv4: return http.IPFamily{IPv4: true} - case ngfAPI.IPv6: + case dataplane.IPv6: return http.IPFamily{IPv6: true} } @@ -155,22 +164,18 @@ func createIncludes(additions []dataplane.Addition) []string { return includes } -func createServers( - httpServers, - sslServers []dataplane.VirtualServer, - ipFamily http.IPFamily, -) ([]http.Server, httpMatchPairs) { +func createServers(httpServers, sslServers []dataplane.VirtualServer) ([]http.Server, httpMatchPairs) { servers := make([]http.Server, 0, len(httpServers)+len(sslServers)) finalMatchPairs := make(httpMatchPairs) for serverID, s := range httpServers { - httpServer, matchPairs := createServer(s, serverID, ipFamily) + httpServer, matchPairs := createServer(s, serverID) servers = append(servers, httpServer) maps.Copy(finalMatchPairs, matchPairs) } for serverID, s := range sslServers { - sslServer, matchPair := createSSLServer(s, serverID, ipFamily) + sslServer, matchPair := createSSLServer(s, serverID) servers = append(servers, sslServer) maps.Copy(finalMatchPairs, matchPair) } @@ -178,16 +183,11 @@ func createServers( return servers, finalMatchPairs } -func createSSLServer( - virtualServer dataplane.VirtualServer, - serverID int, - ipFamily http.IPFamily, -) (http.Server, httpMatchPairs) { +func createSSLServer(virtualServer dataplane.VirtualServer, serverID int) (http.Server, httpMatchPairs) { if virtualServer.IsDefault { return http.Server{ IsDefaultSSL: true, Port: virtualServer.Port, - IPFamily: ipFamily, }, nil } @@ -203,20 +203,14 @@ func createSSLServer( Port: virtualServer.Port, GRPC: grpc, Includes: createIncludes(virtualServer.Additions), - IPFamily: ipFamily, }, matchPairs } -func createServer( - virtualServer dataplane.VirtualServer, - serverID int, - ipFamily http.IPFamily, -) (http.Server, httpMatchPairs) { +func createServer(virtualServer dataplane.VirtualServer, serverID int) (http.Server, httpMatchPairs) { if virtualServer.IsDefault { return http.Server{ IsDefaultHTTP: true, Port: virtualServer.Port, - IPFamily: ipFamily, }, nil } @@ -228,7 +222,6 @@ func createServer( Port: virtualServer.Port, GRPC: grpc, Includes: createIncludes(virtualServer.Additions), - IPFamily: ipFamily, }, matchPairs } diff --git a/internal/mode/static/nginx/config/servers_template.go b/internal/mode/static/nginx/config/servers_template.go index 22cd357ff5..9334d97263 100644 --- a/internal/mode/static/nginx/config/servers_template.go +++ b/internal/mode/static/nginx/config/servers_template.go @@ -2,13 +2,13 @@ package config const serversTemplateText = ` js_preload_object matches from /etc/nginx/conf.d/matches.json; -{{- range $s := . -}} +{{- range $s := .Servers -}} {{ if $s.IsDefaultSSL -}} server { - {{- if $s.IPFamily.IPv4 }} + {{- if $.IPFamily.IPv4 }} listen {{ $s.Port }} ssl default_server; {{- end }} - {{- if $s.IPFamily.IPv6 }} + {{- if $.IPFamily.IPv6 }} listen [::]:{{ $s.Port }} ssl default_server; {{- end }} @@ -16,10 +16,10 @@ server { } {{- else if $s.IsDefaultHTTP }} server { - {{- if $s.IPFamily.IPv4 }} + {{- if $.IPFamily.IPv4 }} listen {{ $s.Port }} default_server; {{- end }} - {{- if $s.IPFamily.IPv6 }} + {{- if $.IPFamily.IPv6 }} listen [::]:{{ $s.Port }} default_server; {{- end }} @@ -29,10 +29,10 @@ server { {{- else }} server { {{- if $s.SSL }} - {{- if $s.IPFamily.IPv4 }} + {{- if $.IPFamily.IPv4 }} listen {{ $s.Port }} ssl; {{- end }} - {{- if $s.IPFamily.IPv6 }} + {{- if $.IPFamily.IPv6 }} listen [::]:{{ $s.Port }} ssl; {{- end }} ssl_certificate {{ $s.SSL.Certificate }}; @@ -42,10 +42,10 @@ server { return 421; } {{- else }} - {{- if $s.IPFamily.IPv4 }} + {{- if $.IPFamily.IPv4 }} listen {{ $s.Port }}; {{- end }} - {{- if $s.IPFamily.IPv6 }} + {{- if $.IPFamily.IPv6 }} listen [::]:{{ $s.Port }}; {{- end }} {{- end }} diff --git a/internal/mode/static/nginx/config/servers_test.go b/internal/mode/static/nginx/config/servers_test.go index e8eeff37f0..c954deeb03 100644 --- a/internal/mode/static/nginx/config/servers_test.go +++ b/internal/mode/static/nginx/config/servers_test.go @@ -9,7 +9,6 @@ import ( . "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/types" - ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" "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/state/dataplane" @@ -50,7 +49,7 @@ func TestExecuteServers(t *testing.T) { }, }, BaseHTTPConfig: dataplane.BaseHTTPConfig{ - IPFamily: ngfAPI.IPv4, + IPFamily: dataplane.IPv4, }, }, expectedHTTPConfig: map[string]int{ @@ -145,7 +144,7 @@ func TestExecuteServers(t *testing.T) { }, }, BaseHTTPConfig: dataplane.BaseHTTPConfig{ - IPFamily: ngfAPI.Dual, + IPFamily: dataplane.Dual, }, }, expectedHTTPConfig: map[string]int{ @@ -1092,7 +1091,6 @@ func TestCreateServers(t *testing.T) { { IsDefaultHTTP: true, Port: 8080, - IPFamily: http.IPFamily{}, }, { ServerName: "cafe.example.com", @@ -1103,12 +1101,10 @@ func TestCreateServers(t *testing.T) { includesFolder + "/server-addition-1.conf", includesFolder + "/server-addition-2.conf", }, - IPFamily: http.IPFamily{}, }, { IsDefaultSSL: true, Port: 8443, - IPFamily: http.IPFamily{}, }, { ServerName: "cafe.example.com", @@ -1123,13 +1119,12 @@ func TestCreateServers(t *testing.T) { includesFolder + "/server-addition-1.conf", includesFolder + "/server-addition-3.conf", }, - IPFamily: http.IPFamily{}, }, } g := NewWithT(t) - result, httpMatchPair := createServers(httpServers, sslServers, http.IPFamily{}) + result, httpMatchPair := createServers(httpServers, sslServers) g.Expect(httpMatchPair).To(Equal(allExpMatchPair)) g.Expect(helpers.Diff(expectedServers, result)).To(BeEmpty()) @@ -1338,7 +1333,7 @@ func TestCreateServersConflicts(t *testing.T) { g := NewWithT(t) - result, _ := createServers(httpServers, []dataplane.VirtualServer{}, http.IPFamily{}) + result, _ := createServers(httpServers, []dataplane.VirtualServer{}) g.Expect(helpers.Diff(expectedServers, result)).To(BeEmpty()) }) } diff --git a/internal/mode/static/nginx/config/upstreams.go b/internal/mode/static/nginx/config/upstreams.go index e1c6a7ae42..a76ee23a6a 100644 --- a/internal/mode/static/nginx/config/upstreams.go +++ b/internal/mode/static/nginx/config/upstreams.go @@ -68,13 +68,12 @@ func (g GeneratorImpl) createUpstream(up dataplane.Upstream) http.Upstream { upstreamServers := make([]http.UpstreamServer, len(up.Endpoints)) for idx, ep := range up.Endpoints { - upstreamServers[idx] = http.UpstreamServer{ - Address: fmt.Sprintf("%s:%d", ep.Address, ep.Port), - } + format := "%s:%d" if ep.IPv6 { - upstreamServers[idx] = http.UpstreamServer{ - Address: fmt.Sprintf("[%s]:%d", ep.Address, ep.Port), - } + format = "[%s]:%d" + } + upstreamServers[idx] = http.UpstreamServer{ + Address: fmt.Sprintf(format, ep.Address, ep.Port), } } diff --git a/internal/mode/static/state/dataplane/configuration.go b/internal/mode/static/state/dataplane/configuration.go index a43f338f57..9a9384e783 100644 --- a/internal/mode/static/state/dataplane/configuration.go +++ b/internal/mode/static/state/dataplane/configuration.go @@ -659,7 +659,7 @@ func buildBaseHTTPConfig(g *graph.Graph) BaseHTTPConfig { baseConfig := BaseHTTPConfig{ // HTTP2 should be enabled by default HTTP2: true, - IPFamily: ngfAPI.Dual, + IPFamily: Dual, } if g.NginxProxy == nil || !g.NginxProxy.Valid { return baseConfig @@ -669,11 +669,11 @@ func buildBaseHTTPConfig(g *graph.Graph) BaseHTTPConfig { baseConfig.HTTP2 = false } - switch ipFamily := g.NginxProxy.Source.Spec.IPFamily; ipFamily { + switch ipFamily := g.NginxProxy.Source.Spec.IPFamily; *ipFamily { case ngfAPI.IPv4: - baseConfig.IPFamily = ngfAPI.IPv4 + baseConfig.IPFamily = IPv4 case ngfAPI.IPv6: - baseConfig.IPFamily = ngfAPI.IPv6 + baseConfig.IPFamily = IPv6 } return baseConfig diff --git a/internal/mode/static/state/dataplane/configuration_test.go b/internal/mode/static/state/dataplane/configuration_test.go index 8580b01b73..7562c0f8b5 100644 --- a/internal/mode/static/state/dataplane/configuration_test.go +++ b/internal/mode/static/state/dataplane/configuration_test.go @@ -633,7 +633,7 @@ func TestBuildConfiguration(t *testing.T) { ServiceName: helpers.GetPointer("my-svc"), }, DisableHTTP2: true, - IPFamily: ngfAPI.Dual, + IPFamily: helpers.GetPointer(ngfAPI.IPv4), }, }, Valid: true, @@ -643,7 +643,7 @@ func TestBuildConfiguration(t *testing.T) { Source: &ngfAPI.NginxProxy{ Spec: ngfAPI.NginxProxySpec{ Telemetry: &ngfAPI.Telemetry{}, - IPFamily: ngfAPI.IPv4, + IPFamily: helpers.GetPointer(ngfAPI.IPv4), }, }, Valid: true, @@ -653,7 +653,7 @@ func TestBuildConfiguration(t *testing.T) { Source: &ngfAPI.NginxProxy{ Spec: ngfAPI.NginxProxySpec{ Telemetry: &ngfAPI.Telemetry{}, - IPFamily: ngfAPI.IPv6, + IPFamily: helpers.GetPointer(ngfAPI.IPv6), }, }, Valid: true, @@ -681,7 +681,7 @@ func TestBuildConfiguration(t *testing.T) { SSLServers: []VirtualServer{}, SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{}, CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.Dual}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, }, msg: "no listeners and routes", }, @@ -714,7 +714,7 @@ func TestBuildConfiguration(t *testing.T) { SSLServers: []VirtualServer{}, SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{}, CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.Dual}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, }, msg: "http listener with no routes", }, @@ -778,7 +778,7 @@ func TestBuildConfiguration(t *testing.T) { }, }, CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.Dual}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, }, msg: "http and https listeners with no valid routes", }, @@ -842,7 +842,7 @@ func TestBuildConfiguration(t *testing.T) { }, }, CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.Dual}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, }, msg: "https listeners with no routes", }, @@ -876,7 +876,7 @@ func TestBuildConfiguration(t *testing.T) { SSLServers: []VirtualServer{}, SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{}, CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.Dual}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, }, msg: "invalid https listener with resolved secret", }, @@ -949,7 +949,7 @@ func TestBuildConfiguration(t *testing.T) { BackendGroups: []BackendGroup{expHR1Groups[0], expHR2Groups[0]}, SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{}, CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.Dual}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, }, msg: "one http listener with two routes for different hostnames", }, @@ -1005,7 +1005,7 @@ func TestBuildConfiguration(t *testing.T) { BackendGroups: []BackendGroup{expGRGroups[0]}, SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{}, CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.Dual}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, }, msg: "one http listener with one grpc route", }, @@ -1126,7 +1126,7 @@ func TestBuildConfiguration(t *testing.T) { }, }, CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.Dual}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, }, msg: "two https listeners each with routes for different hostnames", }, @@ -1287,7 +1287,7 @@ func TestBuildConfiguration(t *testing.T) { }, }, CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.Dual}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, }, msg: "one http and one https listener with two routes with the same hostname with and without collisions", }, @@ -1501,7 +1501,7 @@ func TestBuildConfiguration(t *testing.T) { }, }, CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.Dual}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, }, msg: "multiple http and https listener; different ports", @@ -1634,7 +1634,7 @@ func TestBuildConfiguration(t *testing.T) { BackendGroups: []BackendGroup{expHR5Groups[0], expHR5Groups[1]}, SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{}, CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.Dual}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, }, msg: "one http listener with one route with filters", }, @@ -1737,7 +1737,7 @@ func TestBuildConfiguration(t *testing.T) { }, }, CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.Dual}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, }, msg: "one http and one https listener with routes with valid and invalid rules", }, @@ -1802,7 +1802,7 @@ func TestBuildConfiguration(t *testing.T) { BackendGroups: []BackendGroup{expHR7Groups[0], expHR7Groups[1]}, SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{}, CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.Dual}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, }, msg: "duplicate paths with different types", }, @@ -1891,7 +1891,7 @@ func TestBuildConfiguration(t *testing.T) { }, }, CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.Dual}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, }, msg: "two https listeners with different hostnames but same route; chooses listener with more specific hostname", }, @@ -1968,7 +1968,7 @@ func TestBuildConfiguration(t *testing.T) { CertBundles: map[CertBundleID]CertBundle{ "cert_bundle_test_configmap-1": []byte("cert-1"), }, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.Dual}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, }, msg: "https listener with httproute with backend that has a backend TLS policy attached", }, @@ -2045,7 +2045,7 @@ func TestBuildConfiguration(t *testing.T) { CertBundles: map[CertBundleID]CertBundle{ "cert_bundle_test_configmap-2": []byte("cert-2"), }, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.Dual}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, }, msg: "https listener with httproute with backend that has a backend TLS policy with binaryData attached", }, @@ -2084,7 +2084,7 @@ func TestBuildConfiguration(t *testing.T) { SSLServers: []VirtualServer{}, SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{}, CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: false, IPFamily: ngfAPI.Dual}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: false, IPFamily: IPv4}, Telemetry: Telemetry{ Endpoint: "my-otel.svc:4563", Interval: "5s", @@ -2124,7 +2124,7 @@ func TestBuildConfiguration(t *testing.T) { Source: &ngfAPI.NginxProxy{ Spec: ngfAPI.NginxProxySpec{ DisableHTTP2: true, - IPFamily: ngfAPI.Dual, + IPFamily: helpers.GetPointer(ngfAPI.Dual), Telemetry: &ngfAPI.Telemetry{ Exporter: &ngfAPI.TelemetryExporter{ Endpoint: "some-endpoint", @@ -2144,7 +2144,7 @@ func TestBuildConfiguration(t *testing.T) { SSLServers: []VirtualServer{}, SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{}, CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.Dual}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, Telemetry: Telemetry{}, }, msg: "invalid NginxProxy", @@ -2311,7 +2311,7 @@ func TestBuildConfiguration(t *testing.T) { expHRWithPolicyGroups[0], expHTTPSHRWithPolicyGroups[0], }, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.Dual}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, }, msg: "Simple Gateway and HTTPRoute with policies attached", }, @@ -2350,7 +2350,7 @@ func TestBuildConfiguration(t *testing.T) { SSLServers: []VirtualServer{}, SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{}, CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.IPv4}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: IPv4}, Telemetry: Telemetry{}, }, msg: "NginxProxy with IPv4 IPFamily and no routes", @@ -2390,7 +2390,7 @@ func TestBuildConfiguration(t *testing.T) { SSLServers: []VirtualServer{}, SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{}, CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: ngfAPI.IPv6}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: IPv6}, Telemetry: Telemetry{}, }, msg: "NginxProxy with IPv6 IPFamily", diff --git a/internal/mode/static/state/dataplane/types.go b/internal/mode/static/state/dataplane/types.go index 3fb99da50d..d342ff3b0c 100644 --- a/internal/mode/static/state/dataplane/types.go +++ b/internal/mode/static/state/dataplane/types.go @@ -6,7 +6,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/resolver" ) @@ -298,11 +297,23 @@ type SpanAttribute struct { // BaseHTTPConfig holds the configuration options at the http context. type BaseHTTPConfig struct { // IPFamily specifies the IP family for all servers. - IPFamily ngfAPI.IPFamilyType + IPFamily IPFamilyType // HTTP2 specifies whether http2 should be enabled for all servers. HTTP2 bool } +// IPFamilyType specifies the IP family to be used by NGINX. +type IPFamilyType string + +const ( + // Dual specifies that the server will use both IPv4 and IPv6. + Dual IPFamilyType = "dual" + // IPv4 specifies that the server will use only IPv4. + IPv4 IPFamilyType = "ipv4" + // IPv6 specifies that the server will use only IPv6. + IPv6 IPFamilyType = "ipv6" +) + // Ratio represents a tracing sampling ratio used in an nginx config with the otel_module. type Ratio struct { // Name is based on the associated ObservabilityPolicy's NamespacedName, diff --git a/internal/mode/static/state/resolver/resolver.go b/internal/mode/static/state/resolver/resolver.go index 16d62189a6..79dec3b338 100644 --- a/internal/mode/static/state/resolver/resolver.go +++ b/internal/mode/static/state/resolver/resolver.go @@ -111,10 +111,7 @@ func resolveEndpoints( endpointSet := initEndpointsSet(filteredSlices) for _, eps := range filteredSlices { - var ipv6 bool - if eps.AddressType == discoveryV1.AddressTypeIPv6 { - ipv6 = true - } + ipv6 := eps.AddressType == discoveryV1.AddressTypeIPv6 for _, endpoint := range eps.Endpoints { if !endpointReady(endpoint) { continue diff --git a/internal/mode/static/state/resolver/resolver_test.go b/internal/mode/static/state/resolver/resolver_test.go index c6ff068633..2763893f2c 100644 --- a/internal/mode/static/state/resolver/resolver_test.go +++ b/internal/mode/static/state/resolver/resolver_test.go @@ -174,9 +174,9 @@ func TestIgnoreEndpointSlice(t *testing.T) { ignore bool }{ { - msg: "IPV6 address type", + msg: "FQDN address type", slice: discoveryV1.EndpointSlice{ - AddressType: discoveryV1.AddressTypeIPv6, + AddressType: discoveryV1.AddressTypeFQDN, Ports: []discoveryV1.EndpointPort{ { Name: &svcPortName, @@ -189,16 +189,16 @@ func TestIgnoreEndpointSlice(t *testing.T) { Port: 80, TargetPort: intstr.FromInt(8080), }, - ignore: false, + ignore: true, }, { - msg: "FQDN address type", + msg: "no matching port", slice: discoveryV1.EndpointSlice{ - AddressType: discoveryV1.AddressTypeFQDN, + AddressType: discoveryV1.AddressTypeIPv4, Ports: []discoveryV1.EndpointPort{ { - Name: &svcPortName, - Port: &port8080, + Name: helpers.GetPointer("other-svc-port"), + Port: &port4000, }, }, }, @@ -210,13 +210,12 @@ func TestIgnoreEndpointSlice(t *testing.T) { ignore: true, }, { - msg: "no matching port", + msg: "nil endpoint port", slice: discoveryV1.EndpointSlice{ AddressType: discoveryV1.AddressTypeIPv4, Ports: []discoveryV1.EndpointPort{ { - Name: helpers.GetPointer("other-svc-port"), - Port: &port4000, + Port: nil, }, }, }, @@ -225,15 +224,16 @@ func TestIgnoreEndpointSlice(t *testing.T) { Port: 80, TargetPort: intstr.FromInt(8080), }, - ignore: true, + ignore: false, }, { - msg: "nil endpoint port", + msg: "normal", slice: discoveryV1.EndpointSlice{ AddressType: discoveryV1.AddressTypeIPv4, Ports: []discoveryV1.EndpointPort{ { - Port: nil, + Name: &svcPortName, + Port: &port8080, }, }, }, @@ -245,9 +245,9 @@ func TestIgnoreEndpointSlice(t *testing.T) { ignore: false, }, { - msg: "normal", + msg: "normal IPV6 address type", slice: discoveryV1.EndpointSlice{ - AddressType: discoveryV1.AddressTypeIPv4, + AddressType: discoveryV1.AddressTypeIPv6, Ports: []discoveryV1.EndpointPort{ { Name: &svcPortName, diff --git a/site/content/reference/api.md b/site/content/reference/api.md index 1f33b3419b..10d794dd1c 100644 --- a/site/content/reference/api.md +++ b/site/content/reference/api.md @@ -313,30 +313,30 @@ Telemetry -ipFamily
+disableHTTP2
- -IPFamilyType - +bool (Optional) -

IPFamily specifies the IP family to be used by the server. -Default is “dual”, meaning the server will use both IPv4 and IPv6.

+

DisableHTTP2 defines if http2 should be disabled for all servers. +Default is false, meaning http2 will be enabled for all servers.

-disableHTTP2
+ipFamily
-bool + +IPFamilyType + (Optional) -

DisableHTTP2 defines if http2 should be disabled for all servers. -Default is false, meaning http2 will be enabled for all servers.

+

IPFamily specifies the IP family to be used by the server. +Default is “dual”, meaning the server will use both IPv4 and IPv6.

@@ -738,7 +738,7 @@ Examples: 120s, 50ms.

NginxProxySpec)

-

IPFamilyType specifies the IP family to be used by the server.

+

IPFamilyType specifies the IP family to be used by NGINX.

@@ -935,30 +935,30 @@ Telemetry From 6c96e949a70c1c25942770840c5dbc7111be9d27 Mon Sep 17 00:00:00 2001 From: Saloni Date: Tue, 9 Jul 2024 16:59:27 -0600 Subject: [PATCH 04/28] edit kind cluster creation with dual stack --- Makefile | 3 +- apis/v1alpha1/nginxproxy_types.go | 1 + apis/v1alpha1/zz_generated.deepcopy.go | 10 ++--- config/cluster/kind-cluster.yaml | 9 +++++ docs/developer/quickstart.md | 11 ++--- site/content/reference/api.md | 56 +++++++++++++------------- 6 files changed, 48 insertions(+), 42 deletions(-) create mode 100644 config/cluster/kind-cluster.yaml diff --git a/Makefile b/Makefile index f26c1aef5e..af7cda6c20 100644 --- a/Makefile +++ b/Makefile @@ -7,6 +7,7 @@ MANIFEST_DIR = $(CURDIR)/deploy/manifests CHART_DIR = $(SELF_DIR)charts/nginx-gateway-fabric NGINX_CONF_DIR = internal/mode/static/nginx/conf NJS_DIR = internal/mode/static/nginx/modules/src +KIND_CONFIG_FILE = $(SELF_DIR)config/cluster/kind-cluster.yaml NGINX_DOCKER_BUILD_PLUS_ARGS = --secret id=nginx-repo.crt,src=nginx-repo.crt --secret id=nginx-repo.key,src=nginx-repo.key BUILD_AGENT=local PLUS_ENABLED ?= false @@ -160,7 +161,7 @@ deps: ## Add missing and remove unused modules, verify deps and download them to .PHONY: create-kind-cluster create-kind-cluster: ## Create a kind cluster $(eval KIND_IMAGE=$(shell grep -m1 'FROM kindest/node' <$(SELF_DIR)tests/Dockerfile | awk -F'[ ]' '{print $$2}')) - kind create cluster --image $(KIND_IMAGE) + kind create cluster --image $(KIND_IMAGE) --config $(KIND_CONFIG_FILE) .PHONY: delete-kind-cluster delete-kind-cluster: ## Delete kind cluster diff --git a/apis/v1alpha1/nginxproxy_types.go b/apis/v1alpha1/nginxproxy_types.go index 5445eb945f..eab725b6d2 100644 --- a/apis/v1alpha1/nginxproxy_types.go +++ b/apis/v1alpha1/nginxproxy_types.go @@ -31,6 +31,7 @@ type NginxProxyList struct { // IPFamilyType specifies the IP family to be used by NGINX. // // +kubebuilder:validation:Enum=dual;ipv4;ipv6 +// +kubebuilder:default:=dual type IPFamilyType string const ( diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go index 0c61370b0c..58e50ed64a 100644 --- a/apis/v1alpha1/zz_generated.deepcopy.go +++ b/apis/v1alpha1/zz_generated.deepcopy.go @@ -357,16 +357,16 @@ func (in *NginxProxyList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NginxProxySpec) DeepCopyInto(out *NginxProxySpec) { *out = *in - if in.Telemetry != nil { - in, out := &in.Telemetry, &out.Telemetry - *out = new(Telemetry) - (*in).DeepCopyInto(*out) - } if in.IPFamily != nil { in, out := &in.IPFamily, &out.IPFamily *out = new(IPFamilyType) **out = **in } + if in.Telemetry != nil { + in, out := &in.Telemetry, &out.Telemetry + *out = new(Telemetry) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NginxProxySpec. diff --git a/config/cluster/kind-cluster.yaml b/config/cluster/kind-cluster.yaml new file mode 100644 index 0000000000..ce0de55cf1 --- /dev/null +++ b/config/cluster/kind-cluster.yaml @@ -0,0 +1,9 @@ +kind: Cluster +apiVersion: kind.x-k8s.io/v1alpha4 +nodes: +- role: control-plane +networking: + ipFamily: dual + # ipFamily: ipv6 + # ipFamily: ipv4 + apiServerAddress: 127.0.0.1 diff --git a/docs/developer/quickstart.md b/docs/developer/quickstart.md index 76a13ee476..e64706b174 100644 --- a/docs/developer/quickstart.md +++ b/docs/developer/quickstart.md @@ -134,13 +134,13 @@ This will build the docker images `nginx-gateway-fabric:` and `nginx- 1. Create a `kind` cluster: - To create a `kind` cluster with IPv4 enabled: + To create a `kind` cluster with dual (IPv4 and IPv6) enabled: ```makefile make create-kind-cluster ``` - To create a `kind` cluster with IPv6 or Dual IPFamily enabled, use this `config.yaml`: + To create a `kind` cluster with IPv6 or IPv4 only, edit kind cluster config located at `nginx-gateway-fabric/config/cluster/kind-cluster.yaml`: ```yaml kind: Cluster @@ -148,15 +148,10 @@ This will build the docker images `nginx-gateway-fabric:` and `nginx- nodes: - role: control-plane networking: - ipFamily: dual - # ipFamily: ipv6 + ipFamily: ipv6 apiServerAddress: 127.0.0.1 ``` - ```shell - kind create cluster --config ~/cluster.yaml - ``` - 2. Load the previously built images onto your `kind` cluster: ```shell diff --git a/site/content/reference/api.md b/site/content/reference/api.md index 10d794dd1c..e16aac444d 100644 --- a/site/content/reference/api.md +++ b/site/content/reference/api.md @@ -299,44 +299,44 @@ NginxProxySpec
-ipFamily
+disableHTTP2
- -IPFamilyType - +bool 		(Optional) -

IPFamily specifies the IP family to be used by the server. -Default is “dual”, meaning the server will use both IPv4 and IPv6.

+

DisableHTTP2 defines if http2 should be disabled for all servers. +Default is false, meaning http2 will be enabled for all servers.
-disableHTTP2
+ipFamily
-bool + +IPFamilyType + 		(Optional) -

DisableHTTP2 defines if http2 should be disabled for all servers. -Default is false, meaning http2 will be enabled for all servers.

+

IPFamily specifies the IP family to be used by the server. +Default is “dual”, meaning the server will use both IPv4 and IPv6.
-telemetry
+ipFamily
- -Telemetry + +IPFamilyType 		(Optional) -

Telemetry specifies the OpenTelemetry configuration.

+

IPFamily specifies the IP family to be used by the server. +Default is “dual”, meaning the server will use both IPv4 and IPv6.
-disableHTTP2
+telemetry
-bool + +Telemetry + 		(Optional) -

DisableHTTP2 defines if http2 should be disabled for all servers. -Default is false, meaning http2 will be enabled for all servers.

+

Telemetry specifies the OpenTelemetry configuration.
-ipFamily
+disableHTTP2
- -IPFamilyType - +bool 		(Optional) -

IPFamily specifies the IP family to be used by the server. -Default is “dual”, meaning the server will use both IPv4 and IPv6.

+

DisableHTTP2 defines if http2 should be disabled for all servers. +Default is false, meaning http2 will be enabled for all servers.
@@ -921,44 +921,44 @@ Logging -telemetry
+ipFamily
- -Telemetry + +IPFamilyType (Optional) -

Telemetry specifies the OpenTelemetry configuration.

+

IPFamily specifies the IP family to be used by the server. +Default is “dual”, meaning the server will use both IPv4 and IPv6.

-disableHTTP2
+telemetry
-bool + +Telemetry + (Optional) -

DisableHTTP2 defines if http2 should be disabled for all servers. -Default is false, meaning http2 will be enabled for all servers.

+

Telemetry specifies the OpenTelemetry configuration.

-ipFamily
+disableHTTP2
- -IPFamilyType - +bool (Optional) -

IPFamily specifies the IP family to be used by the server. -Default is “dual”, meaning the server will use both IPv4 and IPv6.

+

DisableHTTP2 defines if http2 should be disabled for all servers. +Default is false, meaning http2 will be enabled for all servers.

From 0a5ead0c8a00759dd6a696bf6091ec2032f81d3c Mon Sep 17 00:00:00 2001 From: Saloni Date: Tue, 9 Jul 2024 17:01:18 -0600 Subject: [PATCH 05/28] fix fieldalignment --- apis/v1alpha1/nginxproxy_types.go | 3 +-- apis/v1alpha1/zz_generated.deepcopy.go | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/apis/v1alpha1/nginxproxy_types.go b/apis/v1alpha1/nginxproxy_types.go index eab725b6d2..1dce407c2c 100644 --- a/apis/v1alpha1/nginxproxy_types.go +++ b/apis/v1alpha1/nginxproxy_types.go @@ -11,12 +11,11 @@ import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" // NginxProxy is a configuration object that is attached to a GatewayClass parametersRef. It provides a way // to configure global settings for all Gateways defined from the GatewayClass. type NginxProxy struct { //nolint:govet // standard field alignment, don't change it + metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` // Spec defines the desired state of the NginxProxy. Spec NginxProxySpec `json:"spec"` - - metav1.TypeMeta `json:",inline"` } // +kubebuilder:object:root=true diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go index 58e50ed64a..90cdca7a41 100644 --- a/apis/v1alpha1/zz_generated.deepcopy.go +++ b/apis/v1alpha1/zz_generated.deepcopy.go @@ -299,9 +299,9 @@ func (in *NginxGatewayStatus) DeepCopy() *NginxGatewayStatus { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NginxProxy) DeepCopyInto(out *NginxProxy) { *out = *in + out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) in.Spec.DeepCopyInto(&out.Spec) - out.TypeMeta = in.TypeMeta } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NginxProxy. From 8a930d72875fe206574782291fb9499e1bcf4b8f Mon Sep 17 00:00:00 2001 From: Saloni Date: Wed, 10 Jul 2024 13:28:52 -0600 Subject: [PATCH 06/28] updates based on reviews --- apis/v1alpha1/nginxproxy_types.go | 2 +- .../bases/gateway.nginx.org_nginxproxies.yaml | 2 +- deploy/crds.yaml | 2 +- .../mode/static/nginx/config/http/config.go | 8 ++++- internal/mode/static/nginx/config/servers.go | 7 +--- .../static/nginx/config/servers_template.go | 16 ++++----- .../mode/static/nginx/config/servers_test.go | 33 +++++++++++++++++++ site/content/reference/api.md | 4 +-- 8 files changed, 54 insertions(+), 20 deletions(-) diff --git a/apis/v1alpha1/nginxproxy_types.go b/apis/v1alpha1/nginxproxy_types.go index 1dce407c2c..881c4cccbc 100644 --- a/apis/v1alpha1/nginxproxy_types.go +++ b/apis/v1alpha1/nginxproxy_types.go @@ -44,7 +44,7 @@ const ( // NginxProxySpec defines the desired state of the NginxProxy. type NginxProxySpec struct { - // IPFamily specifies the IP family to be used by the server. + // IPFamily specifies the IP family to be used by the NGINX. // Default is "dual", meaning the server will use both IPv4 and IPv6. // // +optional diff --git a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml index a4534f4145..bed5896b82 100644 --- a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml +++ b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml @@ -54,7 +54,7 @@ spec: type: boolean ipFamily: description: |- - IPFamily specifies the IP family to be used by the server. + IPFamily specifies the IP family to be used by the NGINX. Default is "dual", meaning the server will use both IPv4 and IPv6. enum: - dual diff --git a/deploy/crds.yaml b/deploy/crds.yaml index 6a8d14c83e..9f2fec33e8 100644 --- a/deploy/crds.yaml +++ b/deploy/crds.yaml @@ -699,7 +699,7 @@ spec: type: boolean ipFamily: description: |- - IPFamily specifies the IP family to be used by the server. + IPFamily specifies the IP family to be used by the NGINX. Default is "dual", meaning the server will use both IPv4 and IPv6. enum: - dual diff --git a/internal/mode/static/nginx/config/http/config.go b/internal/mode/static/nginx/config/http/config.go index 98594b25c4..9326ebb439 100644 --- a/internal/mode/static/nginx/config/http/config.go +++ b/internal/mode/static/nginx/config/http/config.go @@ -12,7 +12,7 @@ type Server struct { GRPC bool } -// IPFamily holds the IP family configuration for all servers. +// IPFamily holds the IP family configuration to be used by NGINX. type IPFamily struct { IPv4 bool IPv6 bool @@ -112,3 +112,9 @@ type ProxySSLVerify struct { TrustedCertificate string Name string } + +// ServerConfig holds configuration for an HTTP server and IP family to be used by NGINX. +type ServerConfig struct { + Servers []Server + IPFamily IPFamily +} diff --git a/internal/mode/static/nginx/config/servers.go b/internal/mode/static/nginx/config/servers.go index a9aa55521f..f56bdcf60b 100644 --- a/internal/mode/static/nginx/config/servers.go +++ b/internal/mode/static/nginx/config/servers.go @@ -57,16 +57,11 @@ var grpcBaseHeaders = []http.Header{ }, } -type serverConfig struct { - Servers []http.Server - IPFamily http.IPFamily -} - func executeServers(conf dataplane.Configuration) []executeResult { ipFamily := getIPFamily(conf.BaseHTTPConfig) servers, httpMatchPairs := createServers(conf.HTTPServers, conf.SSLServers) - serverConfig := serverConfig{ + serverConfig := http.ServerConfig{ Servers: servers, IPFamily: ipFamily, } diff --git a/internal/mode/static/nginx/config/servers_template.go b/internal/mode/static/nginx/config/servers_template.go index 9334d97263..4d8196b180 100644 --- a/internal/mode/static/nginx/config/servers_template.go +++ b/internal/mode/static/nginx/config/servers_template.go @@ -29,12 +29,12 @@ server { {{- else }} server { {{- if $s.SSL }} - {{- if $.IPFamily.IPv4 }} + {{- if $.IPFamily.IPv4 }} listen {{ $s.Port }} ssl; - {{- end }} - {{- if $.IPFamily.IPv6 }} + {{- end }} + {{- if $.IPFamily.IPv6 }} listen [::]:{{ $s.Port }} ssl; - {{- end }} + {{- end }} ssl_certificate {{ $s.SSL.Certificate }}; ssl_certificate_key {{ $s.SSL.CertificateKey }}; @@ -42,12 +42,12 @@ server { return 421; } {{- else }} - {{- if $.IPFamily.IPv4 }} + {{- if $.IPFamily.IPv4 }} listen {{ $s.Port }}; - {{- end }} - {{- if $.IPFamily.IPv6 }} + {{- end }} + {{- if $.IPFamily.IPv6 }} listen [::]:{{ $s.Port }}; - {{- end }} + {{- end }} {{- end }} server_name {{ $s.ServerName }}; diff --git a/internal/mode/static/nginx/config/servers_test.go b/internal/mode/static/nginx/config/servers_test.go index c954deeb03..cedba6c24c 100644 --- a/internal/mode/static/nginx/config/servers_test.go +++ b/internal/mode/static/nginx/config/servers_test.go @@ -2581,3 +2581,36 @@ func TestAdditionFilename(t *testing.T) { name := createAdditionFileName(dataplane.Addition{Identifier: "my-addition"}) g.Expect(name).To(Equal(includesFolder + "/" + "my-addition.conf")) } + +func TestGetIPFamily(t *testing.T) { + test := []struct { + msg string + baseHTTPConfig dataplane.BaseHTTPConfig + expected http.IPFamily + }{ + { + msg: "ipv4", + baseHTTPConfig: dataplane.BaseHTTPConfig{IPFamily: dataplane.IPv4}, + expected: http.IPFamily{IPv4: true, IPv6: false}, + }, + { + msg: "ipv6", + baseHTTPConfig: dataplane.BaseHTTPConfig{IPFamily: dataplane.IPv6}, + expected: http.IPFamily{IPv4: false, IPv6: true}, + }, + { + msg: "dual", + baseHTTPConfig: dataplane.BaseHTTPConfig{IPFamily: dataplane.Dual}, + expected: http.IPFamily{IPv4: true, IPv6: true}, + }, + } + + for _, tc := range test { + t.Run(tc.msg, func(t *testing.T) { + g := NewWithT(t) + + result := getIPFamily(tc.baseHTTPConfig) + g.Expect(result).To(Equal(tc.expected)) + }) + } +} diff --git a/site/content/reference/api.md b/site/content/reference/api.md index e16aac444d..9c509b34a5 100644 --- a/site/content/reference/api.md +++ b/site/content/reference/api.md @@ -308,7 +308,7 @@ IPFamilyType (Optional) -

IPFamily specifies the IP family to be used by the server. +

IPFamily specifies the IP family to be used by the NGINX. Default is “dual”, meaning the server will use both IPv4 and IPv6.

@@ -930,7 +930,7 @@ IPFamilyType (Optional) -

IPFamily specifies the IP family to be used by the server. +

IPFamily specifies the IP family to be used by the NGINX. Default is “dual”, meaning the server will use both IPv4 and IPv6.

From 2f03c33d80c6ffe1bff1b879565afc71e160ad39 Mon Sep 17 00:00:00 2001 From: Saloni Date: Wed, 10 Jul 2024 13:38:15 -0600 Subject: [PATCH 07/28] update unit test --- internal/mode/static/nginx/config/servers_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/mode/static/nginx/config/servers_test.go b/internal/mode/static/nginx/config/servers_test.go index cedba6c24c..1e991a337e 100644 --- a/internal/mode/static/nginx/config/servers_test.go +++ b/internal/mode/static/nginx/config/servers_test.go @@ -2608,7 +2608,6 @@ func TestGetIPFamily(t *testing.T) { for _, tc := range test { t.Run(tc.msg, func(t *testing.T) { g := NewWithT(t) - result := getIPFamily(tc.baseHTTPConfig) g.Expect(result).To(Equal(tc.expected)) }) From a19793cc499c452e5aa33cd22a8d89c55b890dfc Mon Sep 17 00:00:00 2001 From: Saloni Date: Thu, 11 Jul 2024 11:00:42 -0600 Subject: [PATCH 08/28] update endpoint verification based on NGINX IP Family --- apis/v1alpha1/nginxproxy_types.go | 8 +- .../bases/gateway.nginx.org_nginxproxies.yaml | 1 + deploy/crds.yaml | 1 + .../static/state/dataplane/configuration.go | 29 +++++-- .../state/dataplane/configuration_test.go | 51 +++++++++++- .../mode/static/state/resolver/resolver.go | 39 ++++++++-- .../static/state/resolver/resolver_test.go | 78 +++++++++++++++---- .../resolverfakes/fake_service_resolver.go | 20 +++-- .../state/resolver/service_resolver_test.go | 10 +-- site/content/reference/api.md | 6 +- 10 files changed, 192 insertions(+), 51 deletions(-) diff --git a/apis/v1alpha1/nginxproxy_types.go b/apis/v1alpha1/nginxproxy_types.go index 881c4cccbc..018911da7e 100644 --- a/apis/v1alpha1/nginxproxy_types.go +++ b/apis/v1alpha1/nginxproxy_types.go @@ -30,15 +30,14 @@ type NginxProxyList struct { // IPFamilyType specifies the IP family to be used by NGINX. // // +kubebuilder:validation:Enum=dual;ipv4;ipv6 -// +kubebuilder:default:=dual type IPFamilyType string const ( - // Dual specifies that the server will use both IPv4 and IPv6. + // Dual specifies that NGINX will use both IPv4 and IPv6. Dual IPFamilyType = "dual" - // IPv4 specifies that the server will use only IPv4. + // IPv4 specifies that NGINX will use only IPv4. IPv4 IPFamilyType = "ipv4" - // IPv6 specifies that the server will use only IPv6. + // IPv6 specifies that NGINX will use only IPv6. IPv6 IPFamilyType = "ipv6" ) @@ -48,6 +47,7 @@ type NginxProxySpec struct { // Default is "dual", meaning the server will use both IPv4 and IPv6. // // +optional + // +kubebuilder:default:=dual IPFamily *IPFamilyType `json:"ipFamily,omitempty"` // Telemetry specifies the OpenTelemetry configuration. // diff --git a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml index bed5896b82..73acae5e84 100644 --- a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml +++ b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml @@ -53,6 +53,7 @@ spec: Default is false, meaning http2 will be enabled for all servers. type: boolean ipFamily: + default: dual description: |- IPFamily specifies the IP family to be used by the NGINX. Default is "dual", meaning the server will use both IPv4 and IPv6. diff --git a/deploy/crds.yaml b/deploy/crds.yaml index 9f2fec33e8..547c912748 100644 --- a/deploy/crds.yaml +++ b/deploy/crds.yaml @@ -698,6 +698,7 @@ spec: Default is false, meaning http2 will be enabled for all servers. type: boolean ipFamily: + default: dual description: |- IPFamily specifies the IP family to be used by the NGINX. Default is "dual", meaning the server will use both IPv4 and IPv6. diff --git a/internal/mode/static/state/dataplane/configuration.go b/internal/mode/static/state/dataplane/configuration.go index 9a9384e783..b64155d0f6 100644 --- a/internal/mode/static/state/dataplane/configuration.go +++ b/internal/mode/static/state/dataplane/configuration.go @@ -41,13 +41,13 @@ func BuildConfiguration( return Configuration{Version: configVersion} } - upstreams := buildUpstreams(ctx, g.Gateway.Listeners, resolver) + baseHTTPConfig := buildBaseHTTPConfig(g) + upstreams := buildUpstreams(ctx, g.Gateway.Listeners, resolver, baseHTTPConfig) httpServers, sslServers := buildServers(g, generator) backendGroups := buildBackendGroups(append(httpServers, sslServers...)) keyPairs := buildSSLKeyPairs(g.ReferencedSecrets, g.Gateway.Listeners) certBundles := buildCertBundles(g.ReferencedCaCertConfigMaps, backendGroups) telemetry := buildTelemetry(g) - baseHTTPConfig := buildBaseHTTPConfig(g) config := Configuration{ HTTPServers: httpServers, @@ -472,11 +472,15 @@ func buildUpstreams( ctx context.Context, listeners []*graph.Listener, resolver resolver.ServiceResolver, + baseHTTPConfig BaseHTTPConfig, ) []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) + // We need to build endpoints based on the IPFamily of NGINX. + ipv4, ipv6 := getIPFamily(baseHTTPConfig) + for _, l := range listeners { if !l.Valid { continue @@ -503,7 +507,7 @@ func buildUpstreams( var errMsg string - eps, err := resolver.Resolve(ctx, br.SvcNsName, br.ServicePort) + eps, err := resolver.Resolve(ctx, br.SvcNsName, br.ServicePort, ipv4, ipv6) if err != nil { errMsg = err.Error() } @@ -531,6 +535,13 @@ func buildUpstreams( return upstreams } +func getIPFamily(config BaseHTTPConfig) (bool, bool) { + ipv4 := config.IPFamily == IPv4 || config.IPFamily == Dual + ipv6 := config.IPFamily == IPv6 || config.IPFamily == Dual + + return ipv4, ipv6 +} + func getListenerHostname(h *v1.Hostname) string { if h == nil || *h == "" { return wildcardHostname @@ -669,11 +680,13 @@ func buildBaseHTTPConfig(g *graph.Graph) BaseHTTPConfig { baseConfig.HTTP2 = false } - switch ipFamily := g.NginxProxy.Source.Spec.IPFamily; *ipFamily { - case ngfAPI.IPv4: - baseConfig.IPFamily = IPv4 - case ngfAPI.IPv6: - baseConfig.IPFamily = IPv6 + if g.NginxProxy.Source.Spec.IPFamily != nil { + switch ipFamily := g.NginxProxy.Source.Spec.IPFamily; *ipFamily { + case ngfAPI.IPv4: + baseConfig.IPFamily = IPv4 + case ngfAPI.IPv6: + baseConfig.IPFamily = IPv6 + } } return baseConfig diff --git a/internal/mode/static/state/dataplane/configuration_test.go b/internal/mode/static/state/dataplane/configuration_test.go index 7562c0f8b5..13695f4c25 100644 --- a/internal/mode/static/state/dataplane/configuration_test.go +++ b/internal/mode/static/state/dataplane/configuration_test.go @@ -2707,6 +2707,10 @@ func TestBuildUpstreams(t *testing.T) { Address: "10.0.0.2", Port: 8080, }, + { + Address: "fd00:10:244::6", + Port: 8080, + }, } barEndpoints := []resolver.Endpoint{ @@ -2733,6 +2737,10 @@ func TestBuildUpstreams(t *testing.T) { Address: "12.0.0.0", Port: 80, }, + { + Address: "fd00:10:244::9", + Port: 80, + }, } baz2Endpoints := []resolver.Endpoint{ @@ -2749,6 +2757,21 @@ func TestBuildUpstreams(t *testing.T) { }, } + ipv6Endpoints := []resolver.Endpoint{ + { + Address: "fd00:10:244::7", + Port: 80, + }, + { + Address: "fd00:10:244::8", + Port: 80, + }, + { + Address: "fd00:10:244::9", + Port: 80, + }, + } + createBackendRefs := func(serviceNames ...string) []graph.BackendRef { var backends []graph.BackendRef for _, name := range serviceNames { @@ -2775,6 +2798,8 @@ func TestBuildUpstreams(t *testing.T) { hr4Refs1 := createBackendRefs("baz2") + hr5Refs0 := createBackendRefs("ipv6-endpoints") + nonExistingRefs := createBackendRefs("non-existing") invalidHRRefs := createBackendRefs("abc") @@ -2809,6 +2834,15 @@ func TestBuildUpstreams(t *testing.T) { }, } + routes3 := map[graph.RouteKey]*graph.L7Route{ + {NamespacedName: types.NamespacedName{Name: "hr4", Namespace: "test"}}: { + Valid: true, + Spec: graph.L7RouteSpec{ + Rules: refsToValidRules(hr5Refs0, hr2Refs1), + }, + }, + } + routesWithNonExistingRefs := map[graph.RouteKey]*graph.L7Route{ {NamespacedName: types.NamespacedName{Name: "non-existing", Namespace: "test"}}: { Valid: true, @@ -2848,6 +2882,11 @@ func TestBuildUpstreams(t *testing.T) { Valid: true, Routes: invalidRoutes, // shouldn't be included since routes are invalid }, + { + Name: "listener-4", + Valid: true, + Routes: routes3, + }, } emptyEndpointsErrMsg := "empty endpoints error" @@ -2880,6 +2919,10 @@ func TestBuildUpstreams(t *testing.T) { Endpoints: nil, ErrorMsg: nilEndpointsErrMsg, }, + { + Name: "test_ipv6-endpoints_80", + Endpoints: ipv6Endpoints, + }, } fakeResolver := &resolverfakes.FakeServiceResolver{} @@ -2887,6 +2930,8 @@ func TestBuildUpstreams(t *testing.T) { _ context.Context, svcNsName types.NamespacedName, _ apiv1.ServicePort, + _ bool, + _ bool, ) ([]resolver.Endpoint, error) { switch svcNsName.Name { case "bar": @@ -2903,6 +2948,8 @@ func TestBuildUpstreams(t *testing.T) { return nil, errors.New(nilEndpointsErrMsg) case "abc": return abcEndpoints, nil + case "ipv6-endpoints": + return ipv6Endpoints, nil default: return nil, fmt.Errorf("unexpected service %s", svcNsName.Name) } @@ -2910,7 +2957,9 @@ func TestBuildUpstreams(t *testing.T) { g := NewWithT(t) - upstreams := buildUpstreams(context.TODO(), listeners, fakeResolver) + upstreams := buildUpstreams(context.TODO(), listeners, fakeResolver, BaseHTTPConfig{ + IPFamily: Dual, + }) g.Expect(upstreams).To(ConsistOf(expUpstreams)) } diff --git a/internal/mode/static/state/resolver/resolver.go b/internal/mode/static/state/resolver/resolver.go index 79dec3b338..05e8beb378 100644 --- a/internal/mode/static/state/resolver/resolver.go +++ b/internal/mode/static/state/resolver/resolver.go @@ -19,7 +19,13 @@ import ( // ServiceResolver resolves a Service's NamespacedName and ServicePort to a list of Endpoints. // Returns an error if the Service or Service Port cannot be resolved. type ServiceResolver interface { - Resolve(ctx context.Context, svcNsName types.NamespacedName, svcPort v1.ServicePort) ([]Endpoint, error) + Resolve( + ctx context.Context, + svcNsName types.NamespacedName, + svcPort v1.ServicePort, + ipv4Enabled bool, + ipv6Enabled bool, + ) ([]Endpoint, error) } // Endpoint is the internal representation of a Kubernetes endpoint. @@ -48,6 +54,7 @@ func (e *ServiceResolverImpl) Resolve( ctx context.Context, svcNsName types.NamespacedName, svcPort v1.ServicePort, + ipv4Enabled, ipv6Enabled bool, ) ([]Endpoint, error) { if svcPort.Port == 0 || svcNsName.Name == "" || svcNsName.Namespace == "" { panic(fmt.Errorf("expected the following fields to be non-empty: name: %s, ns: %s, port: %d", @@ -68,7 +75,14 @@ func (e *ServiceResolverImpl) Resolve( return nil, fmt.Errorf("no endpoints found for Service %s", svcNsName) } - return resolveEndpoints(svcNsName, svcPort, endpointSliceList, initEndpointSetWithCalculatedSize) + return resolveEndpoints( + svcNsName, + svcPort, + endpointSliceList, + initEndpointSetWithCalculatedSize, + ipv4Enabled, + ipv6Enabled, + ) } type initEndpointSetFunc func([]discoveryV1.EndpointSlice) map[Endpoint]struct{} @@ -99,8 +113,9 @@ func resolveEndpoints( svcPort v1.ServicePort, endpointSliceList discoveryV1.EndpointSliceList, initEndpointsSet initEndpointSetFunc, + ipv4Enabled, ipv6Enabled bool, ) ([]Endpoint, error) { - filteredSlices := filterEndpointSliceList(endpointSliceList, svcPort) + filteredSlices := filterEndpointSliceList(endpointSliceList, svcPort, ipv4Enabled, ipv6Enabled) if len(filteredSlices) == 0 { return nil, fmt.Errorf("no valid endpoints found for Service %s and port %d", svcNsName, svcPort.Port) @@ -151,11 +166,23 @@ func getDefaultPort(svcPort v1.ServicePort) int32 { return svcPort.Port } -func ignoreEndpointSlice(endpointSlice discoveryV1.EndpointSlice, port v1.ServicePort) bool { +func ignoreEndpointSlice( + endpointSlice discoveryV1.EndpointSlice, + port v1.ServicePort, + ipv4Enabled, ipv6Enabled bool, +) bool { if endpointSlice.AddressType == discoveryV1.AddressTypeFQDN { return true } + if endpointSlice.AddressType == discoveryV1.AddressTypeIPv4 && !ipv4Enabled { + return true + } + + if endpointSlice.AddressType == discoveryV1.AddressTypeIPv6 && !ipv6Enabled { + return true + } + // ignore endpoint slices that don't have a matching port. return findPort(endpointSlice.Ports, port) == 0 } @@ -168,11 +195,13 @@ func endpointReady(endpoint discoveryV1.Endpoint) bool { func filterEndpointSliceList( endpointSliceList discoveryV1.EndpointSliceList, port v1.ServicePort, + ipv4Enabled bool, + ipv6Enabled bool, ) []discoveryV1.EndpointSlice { filtered := make([]discoveryV1.EndpointSlice, 0, len(endpointSliceList.Items)) for _, endpointSlice := range endpointSliceList.Items { - if !ignoreEndpointSlice(endpointSlice, port) { + if !ignoreEndpointSlice(endpointSlice, port, ipv4Enabled, ipv6Enabled) { filtered = append(filtered, endpointSlice) } } diff --git a/internal/mode/static/state/resolver/resolver_test.go b/internal/mode/static/state/resolver/resolver_test.go index 2763893f2c..b8baa688af 100644 --- a/internal/mode/static/state/resolver/resolver_test.go +++ b/internal/mode/static/state/resolver/resolver_test.go @@ -50,7 +50,7 @@ var ( Endpoints: nil, } - validEndpointSlice = discoveryV1.EndpointSlice{ + validIPv4EndpontSlice = discoveryV1.EndpointSlice{ AddressType: discoveryV1.AddressTypeIPv4, Endpoints: []discoveryV1.Endpoint{ readyEndpoint1, @@ -100,14 +100,58 @@ var ( ) func TestFilterEndpointSliceList(t *testing.T) { - sliceList := discoveryV1.EndpointSliceList{ - Items: []discoveryV1.EndpointSlice{ - validEndpointSlice, - validIPv6EndpointSlice, - invalidAddressTypeEndpointSlice, - invalidPortEndpointSlice, - nilEndpoints, - mixedValidityEndpointSlice, + test := []struct { + msg string + sliceList discoveryV1.EndpointSliceList + expList []discoveryV1.EndpointSlice + ipv4 bool + ipv6 bool + }{ + { + msg: "only ipv4 enabled", + sliceList: discoveryV1.EndpointSliceList{ + Items: []discoveryV1.EndpointSlice{ + validIPv4EndpontSlice, + validIPv6EndpointSlice, + }, + }, + expList: []discoveryV1.EndpointSlice{ + validIPv4EndpontSlice, + }, + ipv4: true, + ipv6: false, + }, + { + msg: "only ipv6 enabled", + sliceList: discoveryV1.EndpointSliceList{ + Items: []discoveryV1.EndpointSlice{ + validIPv4EndpontSlice, + validIPv6EndpointSlice, + }, + }, + expList: []discoveryV1.EndpointSlice{ + validIPv6EndpointSlice, + }, + ipv4: false, + ipv6: true, + }, + { + msg: "ipv4 and ipv6 enabled", + sliceList: discoveryV1.EndpointSliceList{ + Items: []discoveryV1.EndpointSlice{ + validIPv4EndpontSlice, + validIPv6EndpointSlice, + invalidAddressTypeEndpointSlice, + invalidPortEndpointSlice, + nilEndpoints, + mixedValidityEndpointSlice, + }, + }, + expList: []discoveryV1.EndpointSlice{ + validIPv4EndpontSlice, validIPv6EndpointSlice, mixedValidityEndpointSlice, + }, + ipv4: true, + ipv6: true, }, } @@ -117,11 +161,11 @@ func TestFilterEndpointSliceList(t *testing.T) { TargetPort: intstr.FromInt(80), } - expFilteredList := []discoveryV1.EndpointSlice{validEndpointSlice, validIPv6EndpointSlice, mixedValidityEndpointSlice} - - filteredSliceList := filterEndpointSliceList(sliceList, svcPort) - g := NewWithT(t) - g.Expect(filteredSliceList).To(Equal(expFilteredList)) + for _, tc := range test { + filteredSliceList := filterEndpointSliceList(tc.sliceList, svcPort, tc.ipv4, tc.ipv6) + g := NewWithT(t) + g.Expect(filteredSliceList).To(Equal(tc.expList)) + } } func TestGetDefaultPort(t *testing.T) { @@ -227,7 +271,7 @@ func TestIgnoreEndpointSlice(t *testing.T) { ignore: false, }, { - msg: "normal", + msg: "normal IPV4 address type", slice: discoveryV1.EndpointSlice{ AddressType: discoveryV1.AddressTypeIPv4, Ports: []discoveryV1.EndpointPort{ @@ -265,7 +309,7 @@ func TestIgnoreEndpointSlice(t *testing.T) { } for _, tc := range testcases { g := NewWithT(t) - g.Expect(ignoreEndpointSlice(tc.slice, tc.servicePort)).To(Equal(tc.ignore)) + g.Expect(ignoreEndpointSlice(tc.slice, tc.servicePort, true, true)).To(Equal(tc.ignore)) } } @@ -560,7 +604,7 @@ func bench(b *testing.B, svcNsName types.NamespacedName, ) { b.Helper() for i := 0; i < b.N; i++ { - res, err := resolveEndpoints(svcNsName, v1.ServicePort{Port: 80}, list, initSet) + res, err := resolveEndpoints(svcNsName, v1.ServicePort{Port: 80}, list, initSet, true, true) if len(res) != n { b.Fatalf("expected %d endpoints, got %d", n, len(res)) } diff --git a/internal/mode/static/state/resolver/resolverfakes/fake_service_resolver.go b/internal/mode/static/state/resolver/resolverfakes/fake_service_resolver.go index 8fa8a9fcdc..152d495d52 100644 --- a/internal/mode/static/state/resolver/resolverfakes/fake_service_resolver.go +++ b/internal/mode/static/state/resolver/resolverfakes/fake_service_resolver.go @@ -11,12 +11,14 @@ import ( ) type FakeServiceResolver struct { - ResolveStub func(context.Context, types.NamespacedName, v1.ServicePort) ([]resolver.Endpoint, error) + ResolveStub func(context.Context, types.NamespacedName, v1.ServicePort, bool, bool) ([]resolver.Endpoint, error) resolveMutex sync.RWMutex resolveArgsForCall []struct { arg1 context.Context arg2 types.NamespacedName arg3 v1.ServicePort + arg4 bool + arg5 bool } resolveReturns struct { result1 []resolver.Endpoint @@ -30,20 +32,22 @@ type FakeServiceResolver struct { invocationsMutex sync.RWMutex } -func (fake *FakeServiceResolver) Resolve(arg1 context.Context, arg2 types.NamespacedName, arg3 v1.ServicePort) ([]resolver.Endpoint, error) { +func (fake *FakeServiceResolver) Resolve(arg1 context.Context, arg2 types.NamespacedName, arg3 v1.ServicePort, arg4 bool, arg5 bool) ([]resolver.Endpoint, error) { fake.resolveMutex.Lock() ret, specificReturn := fake.resolveReturnsOnCall[len(fake.resolveArgsForCall)] fake.resolveArgsForCall = append(fake.resolveArgsForCall, struct { arg1 context.Context arg2 types.NamespacedName arg3 v1.ServicePort - }{arg1, arg2, arg3}) + arg4 bool + arg5 bool + }{arg1, arg2, arg3, arg4, arg5}) stub := fake.ResolveStub fakeReturns := fake.resolveReturns - fake.recordInvocation("Resolve", []interface{}{arg1, arg2, arg3}) + fake.recordInvocation("Resolve", []interface{}{arg1, arg2, arg3, arg4, arg5}) fake.resolveMutex.Unlock() if stub != nil { - return stub(arg1, arg2, arg3) + return stub(arg1, arg2, arg3, arg4, arg5) } if specificReturn { return ret.result1, ret.result2 @@ -57,17 +61,17 @@ func (fake *FakeServiceResolver) ResolveCallCount() int { return len(fake.resolveArgsForCall) } -func (fake *FakeServiceResolver) ResolveCalls(stub func(context.Context, types.NamespacedName, v1.ServicePort) ([]resolver.Endpoint, error)) { +func (fake *FakeServiceResolver) ResolveCalls(stub func(context.Context, types.NamespacedName, v1.ServicePort, bool, bool) ([]resolver.Endpoint, error)) { fake.resolveMutex.Lock() defer fake.resolveMutex.Unlock() fake.ResolveStub = stub } -func (fake *FakeServiceResolver) ResolveArgsForCall(i int) (context.Context, types.NamespacedName, v1.ServicePort) { +func (fake *FakeServiceResolver) ResolveArgsForCall(i int) (context.Context, types.NamespacedName, v1.ServicePort, bool, bool) { fake.resolveMutex.RLock() defer fake.resolveMutex.RUnlock() argsForCall := fake.resolveArgsForCall[i] - return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3, argsForCall.arg4, argsForCall.arg5 } func (fake *FakeServiceResolver) ResolveReturns(result1 []resolver.Endpoint, result2 error) { diff --git a/internal/mode/static/state/resolver/service_resolver_test.go b/internal/mode/static/state/resolver/service_resolver_test.go index 83573d51c5..1a2ac046db 100644 --- a/internal/mode/static/state/resolver/service_resolver_test.go +++ b/internal/mode/static/state/resolver/service_resolver_test.go @@ -203,7 +203,7 @@ var _ = Describe("ServiceResolver", func() { }, } - endpoints, err := serviceResolver.Resolve(context.TODO(), svcNsName, svcPort) + endpoints, err := serviceResolver.Resolve(context.TODO(), svcNsName, svcPort, true, true) Expect(err).ToNot(HaveOccurred()) Expect(endpoints).To(ConsistOf(expectedEndpoints)) }) @@ -214,7 +214,7 @@ var _ = Describe("ServiceResolver", func() { Expect(fakeK8sClient.Delete(context.TODO(), dupeEndpointSlice)).To(Succeed()) Expect(fakeK8sClient.Delete(context.TODO(), sliceIPV6)).To(Succeed()) - endpoints, err := serviceResolver.Resolve(context.TODO(), svcNsName, svcPort) + endpoints, err := serviceResolver.Resolve(context.TODO(), svcNsName, svcPort, true, true) Expect(err).To(HaveOccurred()) Expect(endpoints).To(BeNil()) }) @@ -222,19 +222,19 @@ var _ = Describe("ServiceResolver", func() { // delete remaining endpoint slices Expect(fakeK8sClient.Delete(context.TODO(), sliceNoMatchingPortName)).To(Succeed()) - endpoints, err := serviceResolver.Resolve(context.TODO(), svcNsName, svcPort) + endpoints, err := serviceResolver.Resolve(context.TODO(), svcNsName, svcPort, true, true) Expect(err).To(HaveOccurred()) Expect(endpoints).To(BeNil()) }) It("panics if the service NamespacedName is empty", func() { resolve := func() { - _, _ = serviceResolver.Resolve(context.TODO(), types.NamespacedName{}, svcPort) + _, _ = serviceResolver.Resolve(context.TODO(), types.NamespacedName{}, svcPort, true, true) } Expect(resolve).Should(Panic()) }) It("panics if the ServicePort is empty", func() { resolve := func() { - _, _ = serviceResolver.Resolve(context.TODO(), types.NamespacedName{}, v1.ServicePort{}) + _, _ = serviceResolver.Resolve(context.TODO(), types.NamespacedName{}, v1.ServicePort{}, true, true) } Expect(resolve).Should(Panic()) }) diff --git a/site/content/reference/api.md b/site/content/reference/api.md index 9c509b34a5..b597e1a39e 100644 --- a/site/content/reference/api.md +++ b/site/content/reference/api.md @@ -748,13 +748,13 @@ Examples: 120s, 50ms.

"dual"

-

Dual specifies that the server will use both IPv4 and IPv6.

+

Dual specifies that NGINX will use both IPv4 and IPv6.

"ipv4"

-

IPv4 specifies that the server will use only IPv4.

+

IPv4 specifies that NGINX will use only IPv4.

"ipv6"

-

IPv6 specifies that the server will use only IPv6.

+

IPv6 specifies that NGINX will use only IPv6.

From 56007263ca10ec63364054ac370bca031206d572 Mon Sep 17 00:00:00 2001 From: Saloni Date: Thu, 11 Jul 2024 16:32:48 -0600 Subject: [PATCH 09/28] update resolver --- .../static/state/dataplane/configuration.go | 22 +++++++++----- .../state/dataplane/configuration_test.go | 8 ++--- .../mode/static/state/resolver/resolver.go | 26 +++++++--------- .../static/state/resolver/resolver_test.go | 30 +++++++++---------- .../resolverfakes/fake_service_resolver.go | 28 +++++++++-------- .../state/resolver/service_resolver_test.go | 14 +++++---- 6 files changed, 67 insertions(+), 61 deletions(-) diff --git a/internal/mode/static/state/dataplane/configuration.go b/internal/mode/static/state/dataplane/configuration.go index b64155d0f6..5429698155 100644 --- a/internal/mode/static/state/dataplane/configuration.go +++ b/internal/mode/static/state/dataplane/configuration.go @@ -7,6 +7,7 @@ import ( "sort" apiv1 "k8s.io/api/core/v1" + discoveryV1 "k8s.io/api/discovery/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -42,7 +43,7 @@ func BuildConfiguration( } baseHTTPConfig := buildBaseHTTPConfig(g) - upstreams := buildUpstreams(ctx, g.Gateway.Listeners, resolver, baseHTTPConfig) + upstreams := buildUpstreams(ctx, g.Gateway.Listeners, resolver, baseHTTPConfig.IPFamily) httpServers, sslServers := buildServers(g, generator) backendGroups := buildBackendGroups(append(httpServers, sslServers...)) keyPairs := buildSSLKeyPairs(g.ReferencedSecrets, g.Gateway.Listeners) @@ -472,14 +473,14 @@ func buildUpstreams( ctx context.Context, listeners []*graph.Listener, resolver resolver.ServiceResolver, - baseHTTPConfig BaseHTTPConfig, + 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) // We need to build endpoints based on the IPFamily of NGINX. - ipv4, ipv6 := getIPFamily(baseHTTPConfig) + allowedAddressType := getAllowedAddressType(ipFamily) for _, l := range listeners { if !l.Valid { @@ -507,7 +508,7 @@ func buildUpstreams( var errMsg string - eps, err := resolver.Resolve(ctx, br.SvcNsName, br.ServicePort, ipv4, ipv6) + eps, err := resolver.Resolve(ctx, br.SvcNsName, br.ServicePort, allowedAddressType) if err != nil { errMsg = err.Error() } @@ -535,11 +536,16 @@ func buildUpstreams( return upstreams } -func getIPFamily(config BaseHTTPConfig) (bool, bool) { - ipv4 := config.IPFamily == IPv4 || config.IPFamily == Dual - ipv6 := config.IPFamily == IPv6 || config.IPFamily == Dual +func getAllowedAddressType(ipFamily IPFamilyType) []discoveryV1.AddressType { + allowedAddressType := make([]discoveryV1.AddressType, 0) + if ipFamily == IPv4 || ipFamily == Dual { + allowedAddressType = append(allowedAddressType, discoveryV1.AddressTypeIPv4) + } + if ipFamily == IPv6 || ipFamily == Dual { + allowedAddressType = append(allowedAddressType, discoveryV1.AddressTypeIPv6) + } - return ipv4, ipv6 + return allowedAddressType } func getListenerHostname(h *v1.Hostname) string { diff --git a/internal/mode/static/state/dataplane/configuration_test.go b/internal/mode/static/state/dataplane/configuration_test.go index 13695f4c25..7409b9e630 100644 --- a/internal/mode/static/state/dataplane/configuration_test.go +++ b/internal/mode/static/state/dataplane/configuration_test.go @@ -9,6 +9,7 @@ import ( . "github.com/onsi/gomega" apiv1 "k8s.io/api/core/v1" + discoveryV1 "k8s.io/api/discovery/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" @@ -2930,8 +2931,7 @@ func TestBuildUpstreams(t *testing.T) { _ context.Context, svcNsName types.NamespacedName, _ apiv1.ServicePort, - _ bool, - _ bool, + _ []discoveryV1.AddressType, ) ([]resolver.Endpoint, error) { switch svcNsName.Name { case "bar": @@ -2957,9 +2957,7 @@ func TestBuildUpstreams(t *testing.T) { g := NewWithT(t) - upstreams := buildUpstreams(context.TODO(), listeners, fakeResolver, BaseHTTPConfig{ - IPFamily: Dual, - }) + upstreams := buildUpstreams(context.TODO(), listeners, fakeResolver, Dual) g.Expect(upstreams).To(ConsistOf(expUpstreams)) } diff --git a/internal/mode/static/state/resolver/resolver.go b/internal/mode/static/state/resolver/resolver.go index 05e8beb378..6ceb9637cd 100644 --- a/internal/mode/static/state/resolver/resolver.go +++ b/internal/mode/static/state/resolver/resolver.go @@ -3,6 +3,7 @@ package resolver import ( "context" "fmt" + "slices" v1 "k8s.io/api/core/v1" discoveryV1 "k8s.io/api/discovery/v1" @@ -23,8 +24,7 @@ type ServiceResolver interface { ctx context.Context, svcNsName types.NamespacedName, svcPort v1.ServicePort, - ipv4Enabled bool, - ipv6Enabled bool, + allowedAddressType []discoveryV1.AddressType, ) ([]Endpoint, error) } @@ -54,7 +54,7 @@ func (e *ServiceResolverImpl) Resolve( ctx context.Context, svcNsName types.NamespacedName, svcPort v1.ServicePort, - ipv4Enabled, ipv6Enabled bool, + allowedAddressType []discoveryV1.AddressType, ) ([]Endpoint, error) { if svcPort.Port == 0 || svcNsName.Name == "" || svcNsName.Namespace == "" { panic(fmt.Errorf("expected the following fields to be non-empty: name: %s, ns: %s, port: %d", @@ -80,8 +80,7 @@ func (e *ServiceResolverImpl) Resolve( svcPort, endpointSliceList, initEndpointSetWithCalculatedSize, - ipv4Enabled, - ipv6Enabled, + allowedAddressType, ) } @@ -113,9 +112,9 @@ func resolveEndpoints( svcPort v1.ServicePort, endpointSliceList discoveryV1.EndpointSliceList, initEndpointsSet initEndpointSetFunc, - ipv4Enabled, ipv6Enabled bool, + allowedAddressType []discoveryV1.AddressType, ) ([]Endpoint, error) { - filteredSlices := filterEndpointSliceList(endpointSliceList, svcPort, ipv4Enabled, ipv6Enabled) + filteredSlices := filterEndpointSliceList(endpointSliceList, svcPort, allowedAddressType) if len(filteredSlices) == 0 { return nil, fmt.Errorf("no valid endpoints found for Service %s and port %d", svcNsName, svcPort.Port) @@ -169,17 +168,13 @@ func getDefaultPort(svcPort v1.ServicePort) int32 { func ignoreEndpointSlice( endpointSlice discoveryV1.EndpointSlice, port v1.ServicePort, - ipv4Enabled, ipv6Enabled bool, + allowedAddressType []discoveryV1.AddressType, ) bool { if endpointSlice.AddressType == discoveryV1.AddressTypeFQDN { return true } - if endpointSlice.AddressType == discoveryV1.AddressTypeIPv4 && !ipv4Enabled { - return true - } - - if endpointSlice.AddressType == discoveryV1.AddressTypeIPv6 && !ipv6Enabled { + if !slices.Contains(allowedAddressType, endpointSlice.AddressType) { return true } @@ -195,13 +190,12 @@ func endpointReady(endpoint discoveryV1.Endpoint) bool { func filterEndpointSliceList( endpointSliceList discoveryV1.EndpointSliceList, port v1.ServicePort, - ipv4Enabled bool, - ipv6Enabled bool, + allowedAddressType []discoveryV1.AddressType, ) []discoveryV1.EndpointSlice { filtered := make([]discoveryV1.EndpointSlice, 0, len(endpointSliceList.Items)) for _, endpointSlice := range endpointSliceList.Items { - if !ignoreEndpointSlice(endpointSlice, port, ipv4Enabled, ipv6Enabled) { + if !ignoreEndpointSlice(endpointSlice, port, allowedAddressType) { filtered = append(filtered, endpointSlice) } } diff --git a/internal/mode/static/state/resolver/resolver_test.go b/internal/mode/static/state/resolver/resolver_test.go index b8baa688af..adc2051175 100644 --- a/internal/mode/static/state/resolver/resolver_test.go +++ b/internal/mode/static/state/resolver/resolver_test.go @@ -14,7 +14,11 @@ import ( ) var ( - svcPortName = "svc-port" + svcPortName = "svc-port" + dualAddressType = []discoveryV1.AddressType{ + discoveryV1.AddressTypeIPv4, + discoveryV1.AddressTypeIPv6, + } addresses = []string{"10.0.0.1", "10.0.0.2", "10.0.0.3"} addressesIPv6 = []string{"2001:db8::1", "2001:db8::2", "2001:db8::3"} @@ -101,11 +105,10 @@ var ( func TestFilterEndpointSliceList(t *testing.T) { test := []struct { - msg string - sliceList discoveryV1.EndpointSliceList - expList []discoveryV1.EndpointSlice - ipv4 bool - ipv6 bool + msg string + sliceList discoveryV1.EndpointSliceList + expList []discoveryV1.EndpointSlice + allowedAddressType []discoveryV1.AddressType }{ { msg: "only ipv4 enabled", @@ -118,8 +121,7 @@ func TestFilterEndpointSliceList(t *testing.T) { expList: []discoveryV1.EndpointSlice{ validIPv4EndpontSlice, }, - ipv4: true, - ipv6: false, + allowedAddressType: []discoveryV1.AddressType{discoveryV1.AddressTypeIPv4}, }, { msg: "only ipv6 enabled", @@ -132,8 +134,7 @@ func TestFilterEndpointSliceList(t *testing.T) { expList: []discoveryV1.EndpointSlice{ validIPv6EndpointSlice, }, - ipv4: false, - ipv6: true, + allowedAddressType: []discoveryV1.AddressType{discoveryV1.AddressTypeIPv6}, }, { msg: "ipv4 and ipv6 enabled", @@ -150,8 +151,7 @@ func TestFilterEndpointSliceList(t *testing.T) { expList: []discoveryV1.EndpointSlice{ validIPv4EndpontSlice, validIPv6EndpointSlice, mixedValidityEndpointSlice, }, - ipv4: true, - ipv6: true, + allowedAddressType: dualAddressType, }, } @@ -162,7 +162,7 @@ func TestFilterEndpointSliceList(t *testing.T) { } for _, tc := range test { - filteredSliceList := filterEndpointSliceList(tc.sliceList, svcPort, tc.ipv4, tc.ipv6) + filteredSliceList := filterEndpointSliceList(tc.sliceList, svcPort, tc.allowedAddressType) g := NewWithT(t) g.Expect(filteredSliceList).To(Equal(tc.expList)) } @@ -309,7 +309,7 @@ func TestIgnoreEndpointSlice(t *testing.T) { } for _, tc := range testcases { g := NewWithT(t) - g.Expect(ignoreEndpointSlice(tc.slice, tc.servicePort, true, true)).To(Equal(tc.ignore)) + g.Expect(ignoreEndpointSlice(tc.slice, tc.servicePort, dualAddressType)).To(Equal(tc.ignore)) } } @@ -604,7 +604,7 @@ func bench(b *testing.B, svcNsName types.NamespacedName, ) { b.Helper() for i := 0; i < b.N; i++ { - res, err := resolveEndpoints(svcNsName, v1.ServicePort{Port: 80}, list, initSet, true, true) + res, err := resolveEndpoints(svcNsName, v1.ServicePort{Port: 80}, list, initSet, dualAddressType) if len(res) != n { b.Fatalf("expected %d endpoints, got %d", n, len(res)) } diff --git a/internal/mode/static/state/resolver/resolverfakes/fake_service_resolver.go b/internal/mode/static/state/resolver/resolverfakes/fake_service_resolver.go index 152d495d52..09e624cfe5 100644 --- a/internal/mode/static/state/resolver/resolverfakes/fake_service_resolver.go +++ b/internal/mode/static/state/resolver/resolverfakes/fake_service_resolver.go @@ -7,18 +7,18 @@ import ( "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/resolver" v1 "k8s.io/api/core/v1" + v1a "k8s.io/api/discovery/v1" "k8s.io/apimachinery/pkg/types" ) type FakeServiceResolver struct { - ResolveStub func(context.Context, types.NamespacedName, v1.ServicePort, bool, bool) ([]resolver.Endpoint, error) + ResolveStub func(context.Context, types.NamespacedName, v1.ServicePort, []v1a.AddressType) ([]resolver.Endpoint, error) resolveMutex sync.RWMutex resolveArgsForCall []struct { arg1 context.Context arg2 types.NamespacedName arg3 v1.ServicePort - arg4 bool - arg5 bool + arg4 []v1a.AddressType } resolveReturns struct { result1 []resolver.Endpoint @@ -32,22 +32,26 @@ type FakeServiceResolver struct { invocationsMutex sync.RWMutex } -func (fake *FakeServiceResolver) Resolve(arg1 context.Context, arg2 types.NamespacedName, arg3 v1.ServicePort, arg4 bool, arg5 bool) ([]resolver.Endpoint, error) { +func (fake *FakeServiceResolver) Resolve(arg1 context.Context, arg2 types.NamespacedName, arg3 v1.ServicePort, arg4 []v1a.AddressType) ([]resolver.Endpoint, error) { + var arg4Copy []v1a.AddressType + if arg4 != nil { + arg4Copy = make([]v1a.AddressType, len(arg4)) + copy(arg4Copy, arg4) + } fake.resolveMutex.Lock() ret, specificReturn := fake.resolveReturnsOnCall[len(fake.resolveArgsForCall)] fake.resolveArgsForCall = append(fake.resolveArgsForCall, struct { arg1 context.Context arg2 types.NamespacedName arg3 v1.ServicePort - arg4 bool - arg5 bool - }{arg1, arg2, arg3, arg4, arg5}) + arg4 []v1a.AddressType + }{arg1, arg2, arg3, arg4Copy}) stub := fake.ResolveStub fakeReturns := fake.resolveReturns - fake.recordInvocation("Resolve", []interface{}{arg1, arg2, arg3, arg4, arg5}) + fake.recordInvocation("Resolve", []interface{}{arg1, arg2, arg3, arg4Copy}) fake.resolveMutex.Unlock() if stub != nil { - return stub(arg1, arg2, arg3, arg4, arg5) + return stub(arg1, arg2, arg3, arg4) } if specificReturn { return ret.result1, ret.result2 @@ -61,17 +65,17 @@ func (fake *FakeServiceResolver) ResolveCallCount() int { return len(fake.resolveArgsForCall) } -func (fake *FakeServiceResolver) ResolveCalls(stub func(context.Context, types.NamespacedName, v1.ServicePort, bool, bool) ([]resolver.Endpoint, error)) { +func (fake *FakeServiceResolver) ResolveCalls(stub func(context.Context, types.NamespacedName, v1.ServicePort, []v1a.AddressType) ([]resolver.Endpoint, error)) { fake.resolveMutex.Lock() defer fake.resolveMutex.Unlock() fake.ResolveStub = stub } -func (fake *FakeServiceResolver) ResolveArgsForCall(i int) (context.Context, types.NamespacedName, v1.ServicePort, bool, bool) { +func (fake *FakeServiceResolver) ResolveArgsForCall(i int) (context.Context, types.NamespacedName, v1.ServicePort, []v1a.AddressType) { fake.resolveMutex.RLock() defer fake.resolveMutex.RUnlock() argsForCall := fake.resolveArgsForCall[i] - return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3, argsForCall.arg4, argsForCall.arg5 + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3, argsForCall.arg4 } func (fake *FakeServiceResolver) ResolveReturns(result1 []resolver.Endpoint, result2 error) { diff --git a/internal/mode/static/state/resolver/service_resolver_test.go b/internal/mode/static/state/resolver/service_resolver_test.go index 1a2ac046db..51cdd29485 100644 --- a/internal/mode/static/state/resolver/service_resolver_test.go +++ b/internal/mode/static/state/resolver/service_resolver_test.go @@ -150,6 +150,10 @@ var _ = Describe("ServiceResolver", func() { "other-svc-port", discoveryV1.AddressTypeIPv4, ) + dualAddressType = []discoveryV1.AddressType{ + discoveryV1.AddressTypeIPv4, + discoveryV1.AddressTypeIPv6, + } ) var ( @@ -203,7 +207,7 @@ var _ = Describe("ServiceResolver", func() { }, } - endpoints, err := serviceResolver.Resolve(context.TODO(), svcNsName, svcPort, true, true) + endpoints, err := serviceResolver.Resolve(context.TODO(), svcNsName, svcPort, dualAddressType) Expect(err).ToNot(HaveOccurred()) Expect(endpoints).To(ConsistOf(expectedEndpoints)) }) @@ -214,7 +218,7 @@ var _ = Describe("ServiceResolver", func() { Expect(fakeK8sClient.Delete(context.TODO(), dupeEndpointSlice)).To(Succeed()) Expect(fakeK8sClient.Delete(context.TODO(), sliceIPV6)).To(Succeed()) - endpoints, err := serviceResolver.Resolve(context.TODO(), svcNsName, svcPort, true, true) + endpoints, err := serviceResolver.Resolve(context.TODO(), svcNsName, svcPort, dualAddressType) Expect(err).To(HaveOccurred()) Expect(endpoints).To(BeNil()) }) @@ -222,19 +226,19 @@ var _ = Describe("ServiceResolver", func() { // delete remaining endpoint slices Expect(fakeK8sClient.Delete(context.TODO(), sliceNoMatchingPortName)).To(Succeed()) - endpoints, err := serviceResolver.Resolve(context.TODO(), svcNsName, svcPort, true, true) + endpoints, err := serviceResolver.Resolve(context.TODO(), svcNsName, svcPort, dualAddressType) Expect(err).To(HaveOccurred()) Expect(endpoints).To(BeNil()) }) It("panics if the service NamespacedName is empty", func() { resolve := func() { - _, _ = serviceResolver.Resolve(context.TODO(), types.NamespacedName{}, svcPort, true, true) + _, _ = serviceResolver.Resolve(context.TODO(), types.NamespacedName{}, svcPort, dualAddressType) } Expect(resolve).Should(Panic()) }) It("panics if the ServicePort is empty", func() { resolve := func() { - _, _ = serviceResolver.Resolve(context.TODO(), types.NamespacedName{}, v1.ServicePort{}, true, true) + _, _ = serviceResolver.Resolve(context.TODO(), types.NamespacedName{}, v1.ServicePort{}, dualAddressType) } Expect(resolve).Should(Panic()) }) From ec8632211446ddc5900c443972a71d1102f58b42 Mon Sep 17 00:00:00 2001 From: Saloni Date: Thu, 11 Jul 2024 17:19:02 -0600 Subject: [PATCH 10/28] update nginx proxy validator for ipFamily --- internal/mode/static/state/graph/nginxproxy.go | 16 ++++++++++++++++ .../mode/static/state/graph/nginxproxy_test.go | 13 +++++++++++++ 2 files changed, 29 insertions(+) diff --git a/internal/mode/static/state/graph/nginxproxy.go b/internal/mode/static/state/graph/nginxproxy.go index 29cebdbea2..b79f63b627 100644 --- a/internal/mode/static/state/graph/nginxproxy.go +++ b/internal/mode/static/state/graph/nginxproxy.go @@ -108,5 +108,21 @@ func validateNginxProxy( } } + if npCfg.Spec.IPFamily != nil { + ipFamily := npCfg.Spec.IPFamily + ipFamilyPath := spec.Child("ipFamily") + switch *ipFamily { + case ngfAPI.Dual, + ngfAPI.IPv4, + ngfAPI.IPv6: + default: + allErrs = append(allErrs, field.NotSupported( + ipFamilyPath, + ipFamily, + []string{string(ngfAPI.Dual), string(ngfAPI.IPv4), string(ngfAPI.IPv6)}), + ) + } + } + return allErrs } diff --git a/internal/mode/static/state/graph/nginxproxy_test.go b/internal/mode/static/state/graph/nginxproxy_test.go index 1aed0913fc..a664fa80d2 100644 --- a/internal/mode/static/state/graph/nginxproxy_test.go +++ b/internal/mode/static/state/graph/nginxproxy_test.go @@ -262,6 +262,7 @@ func TestValidateNginxProxy(t *testing.T) { {Key: "key", Value: "value"}, }, }, + IPFamily: helpers.GetPointer[ngfAPI.IPFamilyType](ngfAPI.Dual), }, }, expectErrCount: 0, @@ -326,6 +327,18 @@ func TestValidateNginxProxy(t *testing.T) { expErrSubstring: "telemetry.spanAttributes", expectErrCount: 2, }, + { + name: "invalid ipFamily type", + validator: createInvalidValidator(), + np: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + Telemetry: &ngfAPI.Telemetry{}, + IPFamily: helpers.GetPointer[ngfAPI.IPFamilyType]("invalid"), + }, + }, + expErrSubstring: "spec.ipFamily", + expectErrCount: 1, + }, } for _, test := range tests { From 5f6f583833df12140f840a28c513d14b0b076c87 Mon Sep 17 00:00:00 2001 From: Saloni Date: Thu, 11 Jul 2024 22:17:23 -0600 Subject: [PATCH 11/28] add conditions to service --- .../static/state/conditions/conditions.go | 13 ++++ .../mode/static/state/graph/backend_refs.go | 54 ++++++++++++-- .../static/state/graph/backend_refs_test.go | 70 ++++++++++++++++++- internal/mode/static/state/graph/graph.go | 2 +- 4 files changed, 130 insertions(+), 9 deletions(-) diff --git a/internal/mode/static/state/conditions/conditions.go b/internal/mode/static/state/conditions/conditions.go index bb53854c68..a85dc4835f 100644 --- a/internal/mode/static/state/conditions/conditions.go +++ b/internal/mode/static/state/conditions/conditions.go @@ -40,6 +40,10 @@ const ( // Used with Accepted (false). RouteReasonUnsupportedConfiguration v1.RouteConditionReason = "UnsupportedConfiguration" + // RouteReasonInvalidIPFamily is used when the Service associated with the Route is not configured with + // the same IP family as the NGINX server. + RouteReasonInvalidIPFamily v1.RouteConditionReason = "InvalidIPFamily" + // GatewayReasonGatewayConflict indicates there are multiple Gateway resources to choose from, // and we ignored the resource in question and picked another Gateway as the winner. // This reason is used with GatewayConditionAccepted (false). @@ -276,6 +280,15 @@ func NewRouteGatewayNotProgrammed(msg string) conditions.Condition { } } +func NewRouteInvalidIPFamily(msg string) conditions.Condition { + return conditions.Condition{ + Type: string(v1.RouteConditionAccepted), + Status: metav1.ConditionFalse, + Reason: string(RouteReasonInvalidIPFamily), + Message: msg, + } +} + // NewDefaultListenerConditions returns the default Conditions that must be present in the status of a Listener. func NewDefaultListenerConditions() []conditions.Condition { return []conditions.Condition{ diff --git a/internal/mode/static/state/graph/backend_refs.go b/internal/mode/static/state/graph/backend_refs.go index 2673fa4365..fef67e2600 100644 --- a/internal/mode/static/state/graph/backend_refs.go +++ b/internal/mode/static/state/graph/backend_refs.go @@ -10,6 +10,8 @@ import ( gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" "sigs.k8s.io/gateway-api/apis/v1alpha3" + 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" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/sort" @@ -44,9 +46,10 @@ func addBackendRefsToRouteRules( refGrantResolver *referenceGrantResolver, services map[types.NamespacedName]*v1.Service, backendTLSPolicies map[types.NamespacedName]*BackendTLSPolicy, + npCfg *NginxProxy, ) { for _, r := range routes { - addBackendRefsToRules(r, refGrantResolver, services, backendTLSPolicies) + addBackendRefsToRules(r, refGrantResolver, services, backendTLSPolicies, npCfg) } } @@ -57,6 +60,7 @@ func addBackendRefsToRules( refGrantResolver *referenceGrantResolver, services map[types.NamespacedName]*v1.Service, backendTLSPolicies map[types.NamespacedName]*BackendTLSPolicy, + npCfg *NginxProxy, ) { if !route.Valid { return @@ -87,6 +91,7 @@ func addBackendRefsToRules( services, refPath, backendTLSPolicies, + npCfg, ) backendRefs = append(backendRefs, ref) @@ -116,6 +121,7 @@ func createBackendRef( services map[types.NamespacedName]*v1.Service, refPath *field.Path, backendTLSPolicies map[types.NamespacedName]*BackendTLSPolicy, + npCfg *NginxProxy, ) (BackendRef, *conditions.Condition) { // Data plane will handle invalid ref by responding with 500. // Because of that, we always need to add a BackendRef to group.Backends, even if the ref is invalid. @@ -142,7 +148,7 @@ func createBackendRef( return backendRef, &cond } - svcNsName, svcPort, err := getServiceAndPortFromRef(ref.BackendRef, sourceNamespace, services, refPath) + svcNsName, svcPort, svcIPFamilies, err := getServiceAndPortFromRef(ref.BackendRef, sourceNamespace, services, refPath) if err != nil { backendRef = BackendRef{ SvcNsName: svcNsName, @@ -155,6 +161,19 @@ func createBackendRef( return backendRef, &cond } + err = verifyIPFamily(npCfg, svcIPFamilies) + if err != nil { + backendRef = BackendRef{ + SvcNsName: svcNsName, + ServicePort: svcPort, + Weight: weight, + Valid: false, + } + + cond := staticConds.NewRouteInvalidIPFamily(err.Error()) + return backendRef, &cond + } + backendTLSPolicy, err := findBackendTLSPolicyForService( backendTLSPolicies, ref.Namespace, @@ -278,7 +297,7 @@ func getServiceAndPortFromRef( routeNamespace string, services map[types.NamespacedName]*v1.Service, refPath *field.Path, -) (types.NamespacedName, v1.ServicePort, error) { +) (types.NamespacedName, v1.ServicePort, []v1.IPFamily, error) { ns := routeNamespace if ref.Namespace != nil { ns = string(*ref.Namespace) @@ -288,16 +307,39 @@ func getServiceAndPortFromRef( svc, ok := services[svcNsName] if !ok { - return svcNsName, v1.ServicePort{}, field.NotFound(refPath.Child("name"), ref.Name) + return svcNsName, v1.ServicePort{}, []v1.IPFamily{}, field.NotFound(refPath.Child("name"), ref.Name) } // safe to dereference port here because we already validated that the port is not nil in validateBackendRef. svcPort, err := getServicePort(svc, int32(*ref.Port)) if err != nil { - return svcNsName, v1.ServicePort{}, err + return svcNsName, v1.ServicePort{}, []v1.IPFamily{}, err } - return svcNsName, svcPort, nil + svcIPFamilies := svc.Spec.IPFamilies + + return svcNsName, svcPort, svcIPFamilies, nil +} + +func verifyIPFamily(npCfg *NginxProxy, svcIPFamily []v1.IPFamily) error { + if npCfg.Source == nil { + return nil + } + + // we can access this field since we have already validated that ipFamily is not nil in validateNginxProxy. + npIPFamily := npCfg.Source.Spec.IPFamily + if *npIPFamily == ngfAPI.IPv4 { + if slices.Contains(svcIPFamily, v1.IPv6Protocol) { + return fmt.Errorf("service configured with IPv6 stack but NGINX only supports IPv4") + } + } + if *npIPFamily == ngfAPI.IPv6 { + if slices.Contains(svcIPFamily, v1.IPv4Protocol) { + return fmt.Errorf("service configured with IPv4 stack but NGINX only supports IPv6") + } + } + + return nil } func validateRouteBackendRef( diff --git a/internal/mode/static/state/graph/backend_refs_test.go b/internal/mode/static/state/graph/backend_refs_test.go index 140d718864..5086f97fba 100644 --- a/internal/mode/static/state/graph/backend_refs_test.go +++ b/internal/mode/static/state/graph/backend_refs_test.go @@ -1,6 +1,7 @@ package graph import ( + "fmt" "testing" . "github.com/onsi/gomega" @@ -14,6 +15,7 @@ import ( "sigs.k8s.io/gateway-api/apis/v1alpha3" "sigs.k8s.io/gateway-api/apis/v1beta1" + 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" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/kinds" @@ -266,6 +268,7 @@ func TestGetServiceAndPortFromRef(t *testing.T) { Port: 80, }, }, + IPFamilies: []v1.IPFamily{v1.IPv4Protocol}, }, } svc1NsName := types.NamespacedName{ @@ -285,6 +288,7 @@ func TestGetServiceAndPortFromRef(t *testing.T) { expServiceNsName types.NamespacedName name string expServicePort v1.ServicePort + expSvcIPFamily []v1.IPFamily expErr bool }{ { @@ -292,6 +296,7 @@ func TestGetServiceAndPortFromRef(t *testing.T) { ref: getNormalRef(), expServiceNsName: svc1NsName, expServicePort: v1.ServicePort{Port: 80}, + expSvcIPFamily: []v1.IPFamily{v1.IPv4Protocol}, }, { name: "service does not exist", @@ -302,6 +307,7 @@ func TestGetServiceAndPortFromRef(t *testing.T) { expErr: true, expServiceNsName: types.NamespacedName{Name: "does-not-exist", Namespace: "test"}, expServicePort: v1.ServicePort{}, + expSvcIPFamily: []v1.IPFamily{}, }, { name: "no matching port for service and port", @@ -312,6 +318,7 @@ func TestGetServiceAndPortFromRef(t *testing.T) { expErr: true, expServiceNsName: svc1NsName, expServicePort: v1.ServicePort{}, + expSvcIPFamily: []v1.IPFamily{}, }, } @@ -326,11 +333,69 @@ func TestGetServiceAndPortFromRef(t *testing.T) { t.Run(test.name, func(t *testing.T) { g := NewWithT(t) - svcNsName, servicePort, err := getServiceAndPortFromRef(test.ref, "test", services, refPath) + svcNsName, servicePort, svcIPFamily, err := getServiceAndPortFromRef(test.ref, "test", services, refPath) g.Expect(err != nil).To(Equal(test.expErr)) g.Expect(svcNsName).To(Equal(test.expServiceNsName)) g.Expect(servicePort).To(Equal(test.expServicePort)) + g.Expect(svcIPFamily).To(Equal(test.expSvcIPFamily)) + }) + } +} + +func TestVerifyIPFamily(t *testing.T) { + test := []struct { + name string + expErr error + npCfg *NginxProxy + svcIPFamily []v1.IPFamily + }{ + { + name: "Valid - IPv6 and IPv4 configured for NGINX, service has only IPv4", + npCfg: &NginxProxy{ + Source: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + IPFamily: helpers.GetPointer(ngfAPI.Dual), + }, + }, + }, + svcIPFamily: []v1.IPFamily{v1.IPv4Protocol}, + }, + { + name: "Invalid - IPv4 configured for NGINX, service has only IPv6", + npCfg: &NginxProxy{ + Source: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + IPFamily: helpers.GetPointer(ngfAPI.IPv4), + }, + }, + }, + svcIPFamily: []v1.IPFamily{v1.IPv6Protocol}, + expErr: fmt.Errorf("service configured with IPv6 stack but NGINX only supports IPv4"), + }, + { + name: "Invalid - IPv6 configured for NGINX, service has only IPv4", + npCfg: &NginxProxy{ + Source: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + IPFamily: helpers.GetPointer(ngfAPI.IPv6), + }, + }, + }, + svcIPFamily: []v1.IPFamily{v1.IPv4Protocol}, + expErr: fmt.Errorf("service configured with IPv4 stack but NGINX only supports IPv6"), + }, + } + + for _, test := range test { + t.Run(test.name, func(t *testing.T) { + g := NewWithT(t) + err := verifyIPFamily(test.npCfg, test.svcIPFamily) + if test.expErr != nil { + g.Expect(err).To(Equal(test.expErr)) + } else { + g.Expect(err).ToNot(HaveOccurred()) + } }) } } @@ -680,7 +745,7 @@ func TestAddBackendRefsToRulesTest(t *testing.T) { t.Run(test.name, func(t *testing.T) { g := NewWithT(t) resolver := newReferenceGrantResolver(nil) - addBackendRefsToRules(test.route, resolver, services, test.policies) + addBackendRefsToRules(test.route, resolver, services, test.policies, &NginxProxy{}) var actual []BackendRef if test.route.Spec.Rules != nil { @@ -940,6 +1005,7 @@ func TestCreateBackend(t *testing.T) { services, refPath, policies, + &NginxProxy{}, ) g.Expect(helpers.Diff(test.expectedBackend, backend)).To(BeEmpty()) diff --git a/internal/mode/static/state/graph/graph.go b/internal/mode/static/state/graph/graph.go index 4fc86119cb..d874d5d2c7 100644 --- a/internal/mode/static/state/graph/graph.go +++ b/internal/mode/static/state/graph/graph.go @@ -225,7 +225,7 @@ func BuildGraph( ) bindRoutesToListeners(routes, gw, state.Namespaces) - addBackendRefsToRouteRules(routes, refGrantResolver, state.Services, processedBackendTLSPolicies) + addBackendRefsToRouteRules(routes, refGrantResolver, state.Services, processedBackendTLSPolicies, npCfg) referencedNamespaces := buildReferencedNamespaces(state.Namespaces, gw) From 5b38ff987d09165370f1ac602730180ec526ebaf Mon Sep 17 00:00:00 2001 From: Saloni Date: Fri, 12 Jul 2024 09:58:59 -0600 Subject: [PATCH 12/28] improve verify IPFamily --- .../static/state/conditions/conditions.go | 4 ++-- .../mode/static/state/graph/backend_refs.go | 6 ++--- .../static/state/graph/backend_refs_test.go | 24 +++++++++++++++++-- .../mode/static/state/graph/nginxproxy.go | 3 +++ .../static/state/graph/nginxproxy_test.go | 3 +++ 5 files changed, 33 insertions(+), 7 deletions(-) diff --git a/internal/mode/static/state/conditions/conditions.go b/internal/mode/static/state/conditions/conditions.go index a85dc4835f..c0b354670d 100644 --- a/internal/mode/static/state/conditions/conditions.go +++ b/internal/mode/static/state/conditions/conditions.go @@ -42,7 +42,7 @@ const ( // RouteReasonInvalidIPFamily is used when the Service associated with the Route is not configured with // the same IP family as the NGINX server. - RouteReasonInvalidIPFamily v1.RouteConditionReason = "InvalidIPFamily" + RouteReasonInvalidIPFamily v1.RouteConditionReason = "InvalidServiceIPFamily" // GatewayReasonGatewayConflict indicates there are multiple Gateway resources to choose from, // and we ignored the resource in question and picked another Gateway as the winner. @@ -282,7 +282,7 @@ func NewRouteGatewayNotProgrammed(msg string) conditions.Condition { func NewRouteInvalidIPFamily(msg string) conditions.Condition { return conditions.Condition{ - Type: string(v1.RouteConditionAccepted), + Type: string(v1.RouteConditionResolvedRefs), Status: metav1.ConditionFalse, Reason: string(RouteReasonInvalidIPFamily), Message: msg, diff --git a/internal/mode/static/state/graph/backend_refs.go b/internal/mode/static/state/graph/backend_refs.go index fef67e2600..2aa1dcce5e 100644 --- a/internal/mode/static/state/graph/backend_refs.go +++ b/internal/mode/static/state/graph/backend_refs.go @@ -322,7 +322,7 @@ func getServiceAndPortFromRef( } func verifyIPFamily(npCfg *NginxProxy, svcIPFamily []v1.IPFamily) error { - if npCfg.Source == nil { + if npCfg == nil || npCfg.Source == nil || !npCfg.Valid { return nil } @@ -330,12 +330,12 @@ func verifyIPFamily(npCfg *NginxProxy, svcIPFamily []v1.IPFamily) error { npIPFamily := npCfg.Source.Spec.IPFamily if *npIPFamily == ngfAPI.IPv4 { if slices.Contains(svcIPFamily, v1.IPv6Protocol) { - return fmt.Errorf("service configured with IPv6 stack but NGINX only supports IPv4") + return fmt.Errorf("service configured with IPv6 family but NginxProxy is configured with IPv4") } } if *npIPFamily == ngfAPI.IPv6 { if slices.Contains(svcIPFamily, v1.IPv4Protocol) { - return fmt.Errorf("service configured with IPv4 stack but NGINX only supports IPv6") + return fmt.Errorf("service configured with IPv4 family but NginxProxy is configured with IPv6") } } diff --git a/internal/mode/static/state/graph/backend_refs_test.go b/internal/mode/static/state/graph/backend_refs_test.go index 5086f97fba..d767a95053 100644 --- a/internal/mode/static/state/graph/backend_refs_test.go +++ b/internal/mode/static/state/graph/backend_refs_test.go @@ -358,9 +358,22 @@ func TestVerifyIPFamily(t *testing.T) { IPFamily: helpers.GetPointer(ngfAPI.Dual), }, }, + Valid: true, }, svcIPFamily: []v1.IPFamily{v1.IPv4Protocol}, }, + { + name: "Valid - IPv6 and IPv4 configured for NGINX, service has only IPv6", + npCfg: &NginxProxy{ + Source: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + IPFamily: helpers.GetPointer(ngfAPI.Dual), + }, + }, + Valid: true, + }, + svcIPFamily: []v1.IPFamily{v1.IPv6Protocol}, + }, { name: "Invalid - IPv4 configured for NGINX, service has only IPv6", npCfg: &NginxProxy{ @@ -369,9 +382,10 @@ func TestVerifyIPFamily(t *testing.T) { IPFamily: helpers.GetPointer(ngfAPI.IPv4), }, }, + Valid: true, }, svcIPFamily: []v1.IPFamily{v1.IPv6Protocol}, - expErr: fmt.Errorf("service configured with IPv6 stack but NGINX only supports IPv4"), + expErr: fmt.Errorf("service configured with IPv6 family but NginxProxy is configured with IPv4"), }, { name: "Invalid - IPv6 configured for NGINX, service has only IPv4", @@ -381,9 +395,15 @@ func TestVerifyIPFamily(t *testing.T) { IPFamily: helpers.GetPointer(ngfAPI.IPv6), }, }, + Valid: true, }, svcIPFamily: []v1.IPFamily{v1.IPv4Protocol}, - expErr: fmt.Errorf("service configured with IPv4 stack but NGINX only supports IPv6"), + expErr: fmt.Errorf("service configured with IPv4 family but NginxProxy is configured with IPv6"), + }, + { + name: "Invalid - When NginxProxy is nil", + npCfg: &NginxProxy{}, + svcIPFamily: []v1.IPFamily{v1.IPv4Protocol}, }, } diff --git a/internal/mode/static/state/graph/nginxproxy.go b/internal/mode/static/state/graph/nginxproxy.go index b79f63b627..a3449d711e 100644 --- a/internal/mode/static/state/graph/nginxproxy.go +++ b/internal/mode/static/state/graph/nginxproxy.go @@ -6,6 +6,7 @@ import ( v1 "sigs.k8s.io/gateway-api/apis/v1" ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" + "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/kinds" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/validation" ) @@ -122,6 +123,8 @@ func validateNginxProxy( []string{string(ngfAPI.Dual), string(ngfAPI.IPv4), string(ngfAPI.IPv6)}), ) } + } else { + npCfg.Spec.IPFamily = helpers.GetPointer[ngfAPI.IPFamilyType](ngfAPI.Dual) } return allErrs diff --git a/internal/mode/static/state/graph/nginxproxy_test.go b/internal/mode/static/state/graph/nginxproxy_test.go index a664fa80d2..efec06e865 100644 --- a/internal/mode/static/state/graph/nginxproxy_test.go +++ b/internal/mode/static/state/graph/nginxproxy_test.go @@ -71,6 +71,9 @@ func TestGetNginxProxy(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "np2", }, + Spec: ngfAPI.NginxProxySpec{ + IPFamily: helpers.GetPointer(ngfAPI.Dual), + }, }, Valid: true, }, From b3dcc2c1876acbc601d7d1a5330e7486e4091719 Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Fri, 12 Jul 2024 15:34:01 -0600 Subject: [PATCH 13/28] Update internal/mode/static/state/resolver/resolver_test.go Co-authored-by: Kate Osborn <50597707+kate-osborn@users.noreply.github.com> --- internal/mode/static/state/resolver/resolver_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/mode/static/state/resolver/resolver_test.go b/internal/mode/static/state/resolver/resolver_test.go index adc2051175..4ba6a8f85c 100644 --- a/internal/mode/static/state/resolver/resolver_test.go +++ b/internal/mode/static/state/resolver/resolver_test.go @@ -28,7 +28,7 @@ var ( Conditions: discoveryV1.EndpointConditions{Ready: helpers.GetPointer(true)}, } - readyEndpoint2 = discoveryV1.Endpoint{ + readyIPv6Endpoint2 = discoveryV1.Endpoint{ Addresses: addressesIPv6, Conditions: discoveryV1.EndpointConditions{Ready: helpers.GetPointer(true)}, } From c80427420b3dbddd3b3ab088e1f700a83b80119a Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Fri, 12 Jul 2024 15:34:31 -0600 Subject: [PATCH 14/28] Update internal/mode/static/nginx/config/upstreams_test.go Co-authored-by: Kate Osborn <50597707+kate-osborn@users.noreply.github.com> --- internal/mode/static/nginx/config/upstreams_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/mode/static/nginx/config/upstreams_test.go b/internal/mode/static/nginx/config/upstreams_test.go index 351345a21f..f45b942224 100644 --- a/internal/mode/static/nginx/config/upstreams_test.go +++ b/internal/mode/static/nginx/config/upstreams_test.go @@ -36,7 +36,7 @@ func TestExecuteUpstreams(t *testing.T) { Endpoints: []resolver.Endpoint{}, }, { - Name: "up4", + Name: "up-ipv6", Endpoints: []resolver.Endpoint{ { Address: "2001:db8::1", From a578387a4d4a93d37e991a1d33ef4fb2ba44159d Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Fri, 12 Jul 2024 15:34:53 -0600 Subject: [PATCH 15/28] Update internal/mode/static/nginx/config/servers.go Co-authored-by: Kate Osborn <50597707+kate-osborn@users.noreply.github.com> --- internal/mode/static/nginx/config/servers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/mode/static/nginx/config/servers.go b/internal/mode/static/nginx/config/servers.go index f56bdcf60b..df42fba802 100644 --- a/internal/mode/static/nginx/config/servers.go +++ b/internal/mode/static/nginx/config/servers.go @@ -63,7 +63,7 @@ func executeServers(conf dataplane.Configuration) []executeResult { serverConfig := http.ServerConfig{ Servers: servers, - IPFamily: ipFamily, + IPFamily: getIPFamily(conf.BaseHTTPConfig), } serverResult := executeResult{ From 624fd81c544eeea106821389468d34018de672c2 Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Fri, 12 Jul 2024 15:35:11 -0600 Subject: [PATCH 16/28] Update internal/mode/static/state/dataplane/configuration_test.go Co-authored-by: Kate Osborn <50597707+kate-osborn@users.noreply.github.com> --- internal/mode/static/state/dataplane/configuration_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/mode/static/state/dataplane/configuration_test.go b/internal/mode/static/state/dataplane/configuration_test.go index 7409b9e630..f5c1ca4674 100644 --- a/internal/mode/static/state/dataplane/configuration_test.go +++ b/internal/mode/static/state/dataplane/configuration_test.go @@ -2394,7 +2394,7 @@ func TestBuildConfiguration(t *testing.T) { BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: IPv6}, Telemetry: Telemetry{}, }, - msg: "NginxProxy with IPv6 IPFamily", + msg: "NginxProxy with IPv6 IPFamily and no routes", }, } From de05457088036f598b78a0393c2897dd15fde145 Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Fri, 12 Jul 2024 15:35:27 -0600 Subject: [PATCH 17/28] Update internal/mode/static/state/graph/backend_refs.go Co-authored-by: Kate Osborn <50597707+kate-osborn@users.noreply.github.com> --- internal/mode/static/state/graph/backend_refs.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/mode/static/state/graph/backend_refs.go b/internal/mode/static/state/graph/backend_refs.go index 2aa1dcce5e..063dcbc4f1 100644 --- a/internal/mode/static/state/graph/backend_refs.go +++ b/internal/mode/static/state/graph/backend_refs.go @@ -162,7 +162,7 @@ func createBackendRef( } err = verifyIPFamily(npCfg, svcIPFamilies) - if err != nil { + if err := verifyIPFamily(npCfg, svcIPFamilies); err != nil { backendRef = BackendRef{ SvcNsName: svcNsName, ServicePort: svcPort, From fe42fa56f1ef57b434bfaa8626c49ad5bb1e7e82 Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Fri, 12 Jul 2024 15:36:05 -0600 Subject: [PATCH 18/28] Update internal/mode/static/state/graph/backend_refs.go Co-authored-by: Kate Osborn <50597707+kate-osborn@users.noreply.github.com> --- internal/mode/static/state/graph/backend_refs.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/mode/static/state/graph/backend_refs.go b/internal/mode/static/state/graph/backend_refs.go index 063dcbc4f1..01090ed6bf 100644 --- a/internal/mode/static/state/graph/backend_refs.go +++ b/internal/mode/static/state/graph/backend_refs.go @@ -330,7 +330,7 @@ func verifyIPFamily(npCfg *NginxProxy, svcIPFamily []v1.IPFamily) error { npIPFamily := npCfg.Source.Spec.IPFamily if *npIPFamily == ngfAPI.IPv4 { if slices.Contains(svcIPFamily, v1.IPv6Protocol) { - return fmt.Errorf("service configured with IPv6 family but NginxProxy is configured with IPv4") + return errors.New("service configured with IPv6 family but NginxProxy is configured with IPv4") } } if *npIPFamily == ngfAPI.IPv6 { From 82584b2e5004539138594ae855a8c6bd8df65ca6 Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Fri, 12 Jul 2024 15:36:23 -0600 Subject: [PATCH 19/28] Update internal/mode/static/state/graph/backend_refs.go Co-authored-by: Kate Osborn <50597707+kate-osborn@users.noreply.github.com> --- internal/mode/static/state/graph/backend_refs.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/mode/static/state/graph/backend_refs.go b/internal/mode/static/state/graph/backend_refs.go index 01090ed6bf..d72a7cbe9d 100644 --- a/internal/mode/static/state/graph/backend_refs.go +++ b/internal/mode/static/state/graph/backend_refs.go @@ -335,7 +335,7 @@ func verifyIPFamily(npCfg *NginxProxy, svcIPFamily []v1.IPFamily) error { } if *npIPFamily == ngfAPI.IPv6 { if slices.Contains(svcIPFamily, v1.IPv4Protocol) { - return fmt.Errorf("service configured with IPv4 family but NginxProxy is configured with IPv6") + return errors.New("service configured with IPv4 family but NginxProxy is configured with IPv6") } } From 931c61359b5e4422016df77fb7889f0c21f158ab Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Fri, 12 Jul 2024 15:36:41 -0600 Subject: [PATCH 20/28] Update internal/mode/static/state/graph/backend_refs_test.go Co-authored-by: Kate Osborn <50597707+kate-osborn@users.noreply.github.com> --- internal/mode/static/state/graph/backend_refs_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/mode/static/state/graph/backend_refs_test.go b/internal/mode/static/state/graph/backend_refs_test.go index d767a95053..e78f8c8c3d 100644 --- a/internal/mode/static/state/graph/backend_refs_test.go +++ b/internal/mode/static/state/graph/backend_refs_test.go @@ -385,7 +385,7 @@ func TestVerifyIPFamily(t *testing.T) { Valid: true, }, svcIPFamily: []v1.IPFamily{v1.IPv6Protocol}, - expErr: fmt.Errorf("service configured with IPv6 family but NginxProxy is configured with IPv4"), + expErr: errors.New("service configured with IPv6 family but NginxProxy is configured with IPv4"), }, { name: "Invalid - IPv6 configured for NGINX, service has only IPv4", From 2d1938cc6d8356c8178b396c6ca65342d6bbad4a Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Fri, 12 Jul 2024 15:36:59 -0600 Subject: [PATCH 21/28] Update internal/mode/static/state/graph/backend_refs_test.go Co-authored-by: Kate Osborn <50597707+kate-osborn@users.noreply.github.com> --- internal/mode/static/state/graph/backend_refs_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/mode/static/state/graph/backend_refs_test.go b/internal/mode/static/state/graph/backend_refs_test.go index e78f8c8c3d..044738e16c 100644 --- a/internal/mode/static/state/graph/backend_refs_test.go +++ b/internal/mode/static/state/graph/backend_refs_test.go @@ -398,7 +398,7 @@ func TestVerifyIPFamily(t *testing.T) { Valid: true, }, svcIPFamily: []v1.IPFamily{v1.IPv4Protocol}, - expErr: fmt.Errorf("service configured with IPv4 family but NginxProxy is configured with IPv6"), + expErr: errors.New("service configured with IPv4 family but NginxProxy is configured with IPv6"), }, { name: "Invalid - When NginxProxy is nil", From efe7fa1e4e3f87c2f6fd8277f77300397190a453 Mon Sep 17 00:00:00 2001 From: Saloni Date: Fri, 12 Jul 2024 15:41:48 -0600 Subject: [PATCH 22/28] address comments --- config/cluster/kind-cluster.yaml | 2 - internal/mode/static/nginx/config/servers.go | 1 - .../mode/static/nginx/config/servers_test.go | 321 ++++++++++-------- .../static/nginx/config/upstreams_test.go | 2 +- .../static/state/conditions/conditions.go | 3 + .../static/state/dataplane/configuration.go | 15 +- .../state/dataplane/configuration_test.go | 35 +- .../mode/static/state/graph/backend_refs.go | 16 +- .../static/state/graph/backend_refs_test.go | 7 +- .../mode/static/state/graph/nginxproxy.go | 15 +- .../static/state/resolver/resolver_test.go | 2 +- 11 files changed, 245 insertions(+), 174 deletions(-) diff --git a/config/cluster/kind-cluster.yaml b/config/cluster/kind-cluster.yaml index ce0de55cf1..d61c10ca6f 100644 --- a/config/cluster/kind-cluster.yaml +++ b/config/cluster/kind-cluster.yaml @@ -4,6 +4,4 @@ nodes: - role: control-plane networking: ipFamily: dual - # ipFamily: ipv6 - # ipFamily: ipv4 apiServerAddress: 127.0.0.1 diff --git a/internal/mode/static/nginx/config/servers.go b/internal/mode/static/nginx/config/servers.go index df42fba802..80d7bf3de0 100644 --- a/internal/mode/static/nginx/config/servers.go +++ b/internal/mode/static/nginx/config/servers.go @@ -58,7 +58,6 @@ var grpcBaseHeaders = []http.Header{ } func executeServers(conf dataplane.Configuration) []executeResult { - ipFamily := getIPFamily(conf.BaseHTTPConfig) servers, httpMatchPairs := createServers(conf.HTTPServers, conf.SSLServers) serverConfig := http.ServerConfig{ diff --git a/internal/mode/static/nginx/config/servers_test.go b/internal/mode/static/nginx/config/servers_test.go index 1e991a337e..effb0099ab 100644 --- a/internal/mode/static/nginx/config/servers_test.go +++ b/internal/mode/static/nginx/config/servers_test.go @@ -15,39 +15,162 @@ import ( ) func TestExecuteServers(t *testing.T) { - tests := []struct { - msg string - expectedHTTPConfig map[string]int - expectedHTTPMatchVars string - expectedIncludedFileConfigs map[string]string - config dataplane.Configuration - }{ - { - msg: "http and ssl servers with IPv4 IP family", - config: dataplane.Configuration{ - HTTPServers: []dataplane.VirtualServer{ + conf := dataplane.Configuration{ + HTTPServers: []dataplane.VirtualServer{ + { + IsDefault: true, + Port: 8080, + }, + { + Hostname: "example.com", + Port: 8080, + }, + { + Hostname: "cafe.example.com", + Port: 8080, + Additions: []dataplane.Addition{ { - IsDefault: true, - Port: 8080, + Bytes: []byte("addition-1"), + Identifier: "addition-1", }, + }, + }, + }, + SSLServers: []dataplane.VirtualServer{ + { + IsDefault: true, + Port: 8443, + }, + { + Hostname: "example.com", + SSL: &dataplane.SSL{ + KeyPairID: "test-keypair", + }, + Port: 8443, + }, + { + Hostname: "cafe.example.com", + SSL: &dataplane.SSL{ + KeyPairID: "test-keypair", + }, + Port: 8443, + PathRules: []dataplane.PathRule{ { - Hostname: "example.com", - Port: 8080, + Path: "/", + PathType: dataplane.PathTypePrefix, + MatchRules: []dataplane.MatchRule{ + { + Match: dataplane.Match{}, + BackendGroup: dataplane.BackendGroup{ + Source: types.NamespacedName{Namespace: "test", Name: "route1"}, + RuleIdx: 0, + Backends: []dataplane.Backend{ + { + UpstreamName: "test_foo_443", + Valid: true, + Weight: 1, + VerifyTLS: &dataplane.VerifyTLS{ + CertBundleID: "test-foo", + Hostname: "test-foo.example.com", + }, + }, + }, + }, + }, + }, }, }, - SSLServers: []dataplane.VirtualServer{ + Additions: []dataplane.Addition{ { - IsDefault: true, - Port: 8443, + Bytes: []byte("addition-1"), + Identifier: "addition-1", // duplicate }, { - Hostname: "example.com", - SSL: &dataplane.SSL{ - KeyPairID: "test-keypair", - }, - Port: 8443, + Bytes: []byte("addition-2"), + Identifier: "addition-2", }, }, + }, + }, + } + + expSubStrings := map[string]int{ + "listen 8080 default_server;": 1, + "listen 8080;": 2, + "listen 8443 ssl;": 2, + "listen 8443 ssl default_server;": 1, + "server_name example.com;": 2, + "server_name cafe.example.com;": 2, + "ssl_certificate /etc/nginx/secrets/test-keypair.pem;": 2, + "ssl_certificate_key /etc/nginx/secrets/test-keypair.pem;": 2, + "proxy_ssl_server_name on;": 1, + } + + type assertion func(g *WithT, data string) + + expectedResults := map[string]assertion{ + httpConfigFile: func(g *WithT, data string) { + for expSubStr, expCount := range expSubStrings { + g.Expect(strings.Count(data, expSubStr)).To(Equal(expCount)) + } + }, + httpMatchVarsFile: func(g *WithT, data string) { + g.Expect(data).To(Equal("{}")) + }, + includesFolder + "/addition-1.conf": func(g *WithT, data string) { + g.Expect(data).To(Equal("addition-1")) + }, + includesFolder + "/addition-2.conf": func(g *WithT, data string) { + g.Expect(data).To(Equal("addition-2")) + }, + } + g := NewWithT(t) + + results := executeServers(conf) + g.Expect(results).To(HaveLen(len(expectedResults))) + + for _, res := range results { + g.Expect(expectedResults).To(HaveKey(res.dest), "executeServers returned unexpected result destination") + + assertData := expectedResults[res.dest] + assertData(g, string(res.data)) + } +} + +func TestExecuteServersForIPFamily(t *testing.T) { + httpServers := []dataplane.VirtualServer{ + { + IsDefault: true, + Port: 8080, + }, + { + Hostname: "example.com", + Port: 8080, + }, + } + sslServers := []dataplane.VirtualServer{ + { + IsDefault: true, + Port: 8443, + }, + { + Hostname: "example.com", + SSL: &dataplane.SSL{ + KeyPairID: "test-keypair", + }, + Port: 8443, + }, + } + tests := []struct { + msg string + expectedHTTPConfig map[string]int + config dataplane.Configuration + }{ + { + msg: "http and ssl servers with IPv4 IP family", + config: dataplane.Configuration{ + HTTPServers: httpServers, + SSLServers: sslServers, BaseHTTPConfig: dataplane.BaseHTTPConfig{ IPFamily: dataplane.IPv4, }, @@ -62,142 +185,64 @@ func TestExecuteServers(t *testing.T) { "ssl_certificate_key /etc/nginx/secrets/test-keypair.pem;": 1, "ssl_reject_handshake on;": 1, }, - expectedHTTPMatchVars: "{}", }, { - msg: "http and ssl servers with dual IP family", + msg: "http and ssl servers with IPv6 IP family", config: dataplane.Configuration{ - HTTPServers: []dataplane.VirtualServer{ - { - IsDefault: true, - Port: 8080, - }, - { - Hostname: "example.com", - Port: 8080, - }, - { - Hostname: "cafe.example.com", - Port: 8080, - Additions: []dataplane.Addition{ - { - Bytes: []byte("addition-1"), - Identifier: "addition-1", - }, - }, - }, - }, - SSLServers: []dataplane.VirtualServer{ - { - IsDefault: true, - Port: 8443, - }, - { - Hostname: "example.com", - SSL: &dataplane.SSL{ - KeyPairID: "test-keypair", - }, - Port: 8443, - }, - { - Hostname: "cafe.example.com", - SSL: &dataplane.SSL{ - KeyPairID: "test-keypair", - }, - Port: 8443, - PathRules: []dataplane.PathRule{ - { - Path: "/", - PathType: dataplane.PathTypePrefix, - MatchRules: []dataplane.MatchRule{ - { - Match: dataplane.Match{}, - BackendGroup: dataplane.BackendGroup{ - Source: types.NamespacedName{Namespace: "test", Name: "route1"}, - RuleIdx: 0, - Backends: []dataplane.Backend{ - { - UpstreamName: "test_foo_443", - Valid: true, - Weight: 1, - VerifyTLS: &dataplane.VerifyTLS{ - CertBundleID: "test-foo", - Hostname: "test-foo.example.com", - }, - }, - }, - }, - }, - }, - }, - }, - Additions: []dataplane.Addition{ - { - Bytes: []byte("addition-1"), - Identifier: "addition-1", // duplicate - }, - { - Bytes: []byte("addition-2"), - Identifier: "addition-2", - }, - }, - }, + HTTPServers: httpServers, + SSLServers: sslServers, + BaseHTTPConfig: dataplane.BaseHTTPConfig{ + IPFamily: dataplane.IPv6, }, + }, + 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, + }, + }, + { + msg: "http and ssl servers with Dual IP family", + config: dataplane.Configuration{ + HTTPServers: httpServers, + SSLServers: sslServers, BaseHTTPConfig: dataplane.BaseHTTPConfig{ IPFamily: dataplane.Dual, }, }, expectedHTTPConfig: map[string]int{ "listen 8080 default_server;": 1, - "listen [::]:8080 default_server;": 1, - "listen 8080;": 2, - "listen [::]:8080;": 2, - "listen 8443 ssl;": 2, - "listen [::]:8443 ssl;": 2, + "listen 8080;": 1, "listen 8443 ssl default_server;": 1, - "listen [::]:8443 ssl default_server;": 1, + "listen 8443 ssl;": 1, "server_name example.com;": 2, - "server_name cafe.example.com;": 2, - "ssl_certificate /etc/nginx/secrets/test-keypair.pem;": 2, - "ssl_certificate_key /etc/nginx/secrets/test-keypair.pem;": 2, - "proxy_ssl_server_name on;": 1, - }, - expectedHTTPMatchVars: "{}", - expectedIncludedFileConfigs: map[string]string{ - includesFolder + "/addition-1.conf": "addition-1", - includesFolder + "/addition-2.conf": "addition-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 [::]:8443 ssl default_server;": 1, + "listen [::]:8443 ssl;": 1, }, }, } - type assertion func(g *WithT, data string) for _, test := range tests { t.Run(test.msg, func(t *testing.T) { g := NewWithT(t) results := executeServers(test.config) - expectedResults := map[string]assertion{ - httpConfigFile: func(g *WithT, data string) { - for expSubStr, expCount := range test.expectedHTTPConfig { - g.Expect(strings.Count(data, expSubStr)).To(Equal(expCount)) - } - }, - httpMatchVarsFile: func(g *WithT, data string) { - g.Expect(data).To(Equal(test.expectedHTTPMatchVars)) - }, - } - - for file, assertData := range test.expectedIncludedFileConfigs { - expectedResults[file] = func(g *WithT, data string) { - g.Expect(data).To(Equal(assertData)) - } - } - - g.Expect(results).To(HaveLen(len(expectedResults))) + g.Expect(results).To(HaveLen(2)) + serverConf := string(results[0].data) + httpMatchConf := string(results[1].data) + g.Expect(httpMatchConf).To(Equal("{}")) - for _, res := range results { - g.Expect(expectedResults).To(HaveKey(res.dest), "executeServers returned unexpected result destination") - assertData := expectedResults[res.dest] - assertData(g, string(res.data)) + for expSubStr, expCount := range test.expectedHTTPConfig { + g.Expect(strings.Count(serverConf, expSubStr)).To(Equal(expCount)) } }) } diff --git a/internal/mode/static/nginx/config/upstreams_test.go b/internal/mode/static/nginx/config/upstreams_test.go index f45b942224..c7db4f5756 100644 --- a/internal/mode/static/nginx/config/upstreams_test.go +++ b/internal/mode/static/nginx/config/upstreams_test.go @@ -104,7 +104,7 @@ func TestCreateUpstreams(t *testing.T) { Endpoints: []resolver.Endpoint{}, }, { - Name: "up4", + Name: "up4-ipv6", Endpoints: []resolver.Endpoint{ { Address: "fd00:10:244:1::7", diff --git a/internal/mode/static/state/conditions/conditions.go b/internal/mode/static/state/conditions/conditions.go index c0b354670d..5b075a6446 100644 --- a/internal/mode/static/state/conditions/conditions.go +++ b/internal/mode/static/state/conditions/conditions.go @@ -42,6 +42,7 @@ const ( // RouteReasonInvalidIPFamily is used when the Service associated with the Route is not configured with // the same IP family as the NGINX server. + // Used with ResolvedRefs (false). RouteReasonInvalidIPFamily v1.RouteConditionReason = "InvalidServiceIPFamily" // GatewayReasonGatewayConflict indicates there are multiple Gateway resources to choose from, @@ -280,6 +281,8 @@ func NewRouteGatewayNotProgrammed(msg string) conditions.Condition { } } +// NewRouteInvalidIPFamily returns a Condition that indicates that the Service associated with the Route +// is not configured with the same IP family as the NGINX server. func NewRouteInvalidIPFamily(msg string) conditions.Condition { return conditions.Condition{ Type: string(v1.RouteConditionResolvedRefs), diff --git a/internal/mode/static/state/dataplane/configuration.go b/internal/mode/static/state/dataplane/configuration.go index 5429698155..536213a1d6 100644 --- a/internal/mode/static/state/dataplane/configuration.go +++ b/internal/mode/static/state/dataplane/configuration.go @@ -537,15 +537,14 @@ func buildUpstreams( } func getAllowedAddressType(ipFamily IPFamilyType) []discoveryV1.AddressType { - allowedAddressType := make([]discoveryV1.AddressType, 0) - if ipFamily == IPv4 || ipFamily == Dual { - allowedAddressType = append(allowedAddressType, discoveryV1.AddressTypeIPv4) + switch ipFamily { + case IPv4: + return []discoveryV1.AddressType{discoveryV1.AddressTypeIPv4} + case IPv6: + return []discoveryV1.AddressType{discoveryV1.AddressTypeIPv6} + default: + return []discoveryV1.AddressType{discoveryV1.AddressTypeIPv4, discoveryV1.AddressTypeIPv6} } - if ipFamily == IPv6 || ipFamily == Dual { - allowedAddressType = append(allowedAddressType, discoveryV1.AddressTypeIPv6) - } - - return allowedAddressType } func getListenerHostname(h *v1.Hostname) string { diff --git a/internal/mode/static/state/dataplane/configuration_test.go b/internal/mode/static/state/dataplane/configuration_test.go index f5c1ca4674..b0c1295d18 100644 --- a/internal/mode/static/state/dataplane/configuration_test.go +++ b/internal/mode/static/state/dataplane/configuration_test.go @@ -634,7 +634,7 @@ func TestBuildConfiguration(t *testing.T) { ServiceName: helpers.GetPointer("my-svc"), }, DisableHTTP2: true, - IPFamily: helpers.GetPointer(ngfAPI.IPv4), + IPFamily: helpers.GetPointer(ngfAPI.Dual), }, }, Valid: true, @@ -2085,7 +2085,7 @@ func TestBuildConfiguration(t *testing.T) { SSLServers: []VirtualServer{}, SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{}, CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: false, IPFamily: IPv4}, + BaseHTTPConfig: BaseHTTPConfig{HTTP2: false, IPFamily: Dual}, Telemetry: Telemetry{ Endpoint: "my-otel.svc:4563", Interval: "5s", @@ -3486,3 +3486,34 @@ func TestBuildAdditions(t *testing.T) { }) } } + +func TestGetAllowedAddressType(t *testing.T) { + test := []struct { + msg string + ipFamily IPFamilyType + expected []discoveryV1.AddressType + }{ + { + msg: "dual ip family", + ipFamily: Dual, + expected: []discoveryV1.AddressType{discoveryV1.AddressTypeIPv4, discoveryV1.AddressTypeIPv6}, + }, + { + msg: "ipv4 ip family", + ipFamily: IPv4, + expected: []discoveryV1.AddressType{discoveryV1.AddressTypeIPv4}, + }, + { + msg: "ipv6 ip family", + ipFamily: IPv6, + expected: []discoveryV1.AddressType{discoveryV1.AddressTypeIPv6}, + }, + } + + for _, tc := range test { + t.Run(tc.msg, func(t *testing.T) { + g := NewWithT(t) + g.Expect(getAllowedAddressType(tc.ipFamily)).To(Equal(tc.expected)) + }) + } +} diff --git a/internal/mode/static/state/graph/backend_refs.go b/internal/mode/static/state/graph/backend_refs.go index d72a7cbe9d..9edd49c6ff 100644 --- a/internal/mode/static/state/graph/backend_refs.go +++ b/internal/mode/static/state/graph/backend_refs.go @@ -1,6 +1,7 @@ package graph import ( + "errors" "fmt" "slices" @@ -148,7 +149,8 @@ func createBackendRef( return backendRef, &cond } - svcNsName, svcPort, svcIPFamilies, err := getServiceAndPortFromRef(ref.BackendRef, sourceNamespace, services, refPath) + svcIPFamilies, svcPort, err := getServiceAndPortFromRef(ref.BackendRef, sourceNamespace, services, refPath) + svcNsName := types.NamespacedName{Namespace: sourceNamespace, Name: string(ref.Name)} if err != nil { backendRef = BackendRef{ SvcNsName: svcNsName, @@ -161,7 +163,6 @@ func createBackendRef( return backendRef, &cond } - err = verifyIPFamily(npCfg, svcIPFamilies) if err := verifyIPFamily(npCfg, svcIPFamilies); err != nil { backendRef = BackendRef{ SvcNsName: svcNsName, @@ -297,28 +298,25 @@ func getServiceAndPortFromRef( routeNamespace string, services map[types.NamespacedName]*v1.Service, refPath *field.Path, -) (types.NamespacedName, v1.ServicePort, []v1.IPFamily, error) { +) ([]v1.IPFamily, v1.ServicePort, error) { ns := routeNamespace if ref.Namespace != nil { ns = string(*ref.Namespace) } svcNsName := types.NamespacedName{Name: string(ref.Name), Namespace: ns} - svc, ok := services[svcNsName] if !ok { - return svcNsName, v1.ServicePort{}, []v1.IPFamily{}, field.NotFound(refPath.Child("name"), ref.Name) + return []v1.IPFamily{}, v1.ServicePort{}, field.NotFound(refPath.Child("name"), ref.Name) } // safe to dereference port here because we already validated that the port is not nil in validateBackendRef. svcPort, err := getServicePort(svc, int32(*ref.Port)) if err != nil { - return svcNsName, v1.ServicePort{}, []v1.IPFamily{}, err + return []v1.IPFamily{}, v1.ServicePort{}, err } - svcIPFamilies := svc.Spec.IPFamilies - - return svcNsName, svcPort, svcIPFamilies, nil + return svc.Spec.IPFamilies, svcPort, nil } func verifyIPFamily(npCfg *NginxProxy, svcIPFamily []v1.IPFamily) error { diff --git a/internal/mode/static/state/graph/backend_refs_test.go b/internal/mode/static/state/graph/backend_refs_test.go index 044738e16c..9712dbe8d6 100644 --- a/internal/mode/static/state/graph/backend_refs_test.go +++ b/internal/mode/static/state/graph/backend_refs_test.go @@ -1,7 +1,7 @@ package graph import ( - "fmt" + "errors" "testing" . "github.com/onsi/gomega" @@ -333,10 +333,9 @@ func TestGetServiceAndPortFromRef(t *testing.T) { t.Run(test.name, func(t *testing.T) { g := NewWithT(t) - svcNsName, servicePort, svcIPFamily, err := getServiceAndPortFromRef(test.ref, "test", services, refPath) + svcIPFamily, servicePort, err := getServiceAndPortFromRef(test.ref, "test", services, refPath) g.Expect(err != nil).To(Equal(test.expErr)) - g.Expect(svcNsName).To(Equal(test.expServiceNsName)) g.Expect(servicePort).To(Equal(test.expServicePort)) g.Expect(svcIPFamily).To(Equal(test.expSvcIPFamily)) }) @@ -401,7 +400,7 @@ func TestVerifyIPFamily(t *testing.T) { expErr: errors.New("service configured with IPv4 family but NginxProxy is configured with IPv6"), }, { - name: "Invalid - When NginxProxy is nil", + name: "Valid - When NginxProxy is nil", npCfg: &NginxProxy{}, svcIPFamily: []v1.IPFamily{v1.IPv4Protocol}, }, diff --git a/internal/mode/static/state/graph/nginxproxy.go b/internal/mode/static/state/graph/nginxproxy.go index a3449d711e..d36133308b 100644 --- a/internal/mode/static/state/graph/nginxproxy.go +++ b/internal/mode/static/state/graph/nginxproxy.go @@ -113,15 +113,14 @@ func validateNginxProxy( ipFamily := npCfg.Spec.IPFamily ipFamilyPath := spec.Child("ipFamily") switch *ipFamily { - case ngfAPI.Dual, - ngfAPI.IPv4, - ngfAPI.IPv6: + case ngfAPI.Dual, ngfAPI.IPv4, ngfAPI.IPv6: default: - allErrs = append(allErrs, field.NotSupported( - ipFamilyPath, - ipFamily, - []string{string(ngfAPI.Dual), string(ngfAPI.IPv4), string(ngfAPI.IPv6)}), - ) + allErrs = append( + allErrs, + field.NotSupported( + ipFamilyPath, + ipFamily, + []string{string(ngfAPI.Dual), string(ngfAPI.IPv4), string(ngfAPI.IPv6)})) } } else { npCfg.Spec.IPFamily = helpers.GetPointer[ngfAPI.IPFamilyType](ngfAPI.Dual) diff --git a/internal/mode/static/state/resolver/resolver_test.go b/internal/mode/static/state/resolver/resolver_test.go index 4ba6a8f85c..adb87bb1ac 100644 --- a/internal/mode/static/state/resolver/resolver_test.go +++ b/internal/mode/static/state/resolver/resolver_test.go @@ -71,7 +71,7 @@ var ( validIPv6EndpointSlice = discoveryV1.EndpointSlice{ AddressType: discoveryV1.AddressTypeIPv6, - Endpoints: []discoveryV1.Endpoint{readyEndpoint2}, + Endpoints: []discoveryV1.Endpoint{readyIPv6Endpoint2}, Ports: []discoveryV1.EndpointPort{ { Name: &svcPortName, From 4175da4e2ea2261b3a5445ca359fe09cacbe4f3e Mon Sep 17 00:00:00 2001 From: Saloni Date: Sun, 14 Jul 2024 17:51:09 -0600 Subject: [PATCH 23/28] update compat doc and troubleshooting guide --- .../static/nginx/config/upstreams_test.go | 6 +-- .../mode/static/state/graph/backend_refs.go | 18 ++++--- .../static/state/graph/backend_refs_test.go | 48 ++++++++++++++++--- .../how-to/monitoring/troubleshooting.md | 27 +++++++++++ .../overview/gateway-api-compatibility.md | 1 + 5 files changed, 83 insertions(+), 17 deletions(-) diff --git a/internal/mode/static/nginx/config/upstreams_test.go b/internal/mode/static/nginx/config/upstreams_test.go index c7db4f5756..9b6dbbf7ec 100644 --- a/internal/mode/static/nginx/config/upstreams_test.go +++ b/internal/mode/static/nginx/config/upstreams_test.go @@ -36,7 +36,7 @@ func TestExecuteUpstreams(t *testing.T) { Endpoints: []resolver.Endpoint{}, }, { - Name: "up-ipv6", + Name: "up4-ipv6", Endpoints: []resolver.Endpoint{ { Address: "2001:db8::1", @@ -51,7 +51,7 @@ func TestExecuteUpstreams(t *testing.T) { "upstream up1", "upstream up2", "upstream up3", - "upstream up4", + "upstream up4-ipv6", "upstream invalid-backend-ref", "server 10.0.0.0:80;", "server 11.0.0.0:80;", @@ -150,7 +150,7 @@ func TestCreateUpstreams(t *testing.T) { }, }, { - Name: "up4", + Name: "up4-ipv6", ZoneSize: ossZoneSize, Servers: []http.UpstreamServer{ { diff --git a/internal/mode/static/state/graph/backend_refs.go b/internal/mode/static/state/graph/backend_refs.go index 9edd49c6ff..7cb4b494d1 100644 --- a/internal/mode/static/state/graph/backend_refs.go +++ b/internal/mode/static/state/graph/backend_refs.go @@ -6,8 +6,10 @@ import ( "slices" v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "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/v1alpha3" @@ -149,8 +151,8 @@ func createBackendRef( return backendRef, &cond } - svcIPFamilies, svcPort, err := getServiceAndPortFromRef(ref.BackendRef, sourceNamespace, services, refPath) - svcNsName := types.NamespacedName{Namespace: sourceNamespace, Name: string(ref.Name)} + svc, svcPort, err := getServiceAndPortFromRef(ref.BackendRef, sourceNamespace, services, refPath) + svcNsName := client.ObjectKeyFromObject(svc) if err != nil { backendRef = BackendRef{ SvcNsName: svcNsName, @@ -163,7 +165,7 @@ func createBackendRef( return backendRef, &cond } - if err := verifyIPFamily(npCfg, svcIPFamilies); err != nil { + if err := verifyIPFamily(npCfg, svc.Spec.IPFamilies); err != nil { backendRef = BackendRef{ SvcNsName: svcNsName, ServicePort: svcPort, @@ -298,7 +300,7 @@ func getServiceAndPortFromRef( routeNamespace string, services map[types.NamespacedName]*v1.Service, refPath *field.Path, -) ([]v1.IPFamily, v1.ServicePort, error) { +) (*v1.Service, v1.ServicePort, error) { ns := routeNamespace if ref.Namespace != nil { ns = string(*ref.Namespace) @@ -307,16 +309,18 @@ func getServiceAndPortFromRef( svcNsName := types.NamespacedName{Name: string(ref.Name), Namespace: ns} svc, ok := services[svcNsName] if !ok { - return []v1.IPFamily{}, v1.ServicePort{}, field.NotFound(refPath.Child("name"), ref.Name) + return &v1.Service{ObjectMeta: metav1.ObjectMeta{Namespace: ns, Name: string(ref.Name)}}, + v1.ServicePort{}, + field.NotFound(refPath.Child("name"), ref.Name) } // safe to dereference port here because we already validated that the port is not nil in validateBackendRef. svcPort, err := getServicePort(svc, int32(*ref.Port)) if err != nil { - return []v1.IPFamily{}, v1.ServicePort{}, err + return &v1.Service{ObjectMeta: metav1.ObjectMeta{Namespace: ns, Name: string(ref.Name)}}, v1.ServicePort{}, err } - return svc.Spec.IPFamilies, svcPort, nil + return svc, svcPort, nil } func verifyIPFamily(npCfg *NginxProxy, svcIPFamily []v1.IPFamily) error { diff --git a/internal/mode/static/state/graph/backend_refs_test.go b/internal/mode/static/state/graph/backend_refs_test.go index 9712dbe8d6..f889e1b088 100644 --- a/internal/mode/static/state/graph/backend_refs_test.go +++ b/internal/mode/static/state/graph/backend_refs_test.go @@ -284,11 +284,11 @@ func TestGetServiceAndPortFromRef(t *testing.T) { } tests := []struct { + expSvc *v1.Service ref gatewayv1.BackendRef expServiceNsName types.NamespacedName name string expServicePort v1.ServicePort - expSvcIPFamily []v1.IPFamily expErr bool }{ { @@ -296,7 +296,7 @@ func TestGetServiceAndPortFromRef(t *testing.T) { ref: getNormalRef(), expServiceNsName: svc1NsName, expServicePort: v1.ServicePort{Port: 80}, - expSvcIPFamily: []v1.IPFamily{v1.IPv4Protocol}, + expSvc: svc1, }, { name: "service does not exist", @@ -307,7 +307,11 @@ func TestGetServiceAndPortFromRef(t *testing.T) { expErr: true, expServiceNsName: types.NamespacedName{Name: "does-not-exist", Namespace: "test"}, expServicePort: v1.ServicePort{}, - expSvcIPFamily: []v1.IPFamily{}, + expSvc: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "does-not-exist", Namespace: "test", + }, + }, }, { name: "no matching port for service and port", @@ -318,7 +322,11 @@ func TestGetServiceAndPortFromRef(t *testing.T) { expErr: true, expServiceNsName: svc1NsName, expServicePort: v1.ServicePort{}, - expSvcIPFamily: []v1.IPFamily{}, + expSvc: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "service1", Namespace: "test", + }, + }, }, } @@ -333,11 +341,11 @@ func TestGetServiceAndPortFromRef(t *testing.T) { t.Run(test.name, func(t *testing.T) { g := NewWithT(t) - svcIPFamily, servicePort, err := getServiceAndPortFromRef(test.ref, "test", services, refPath) + svc, servicePort, err := getServiceAndPortFromRef(test.ref, "test", services, refPath) g.Expect(err != nil).To(Equal(test.expErr)) g.Expect(servicePort).To(Equal(test.expServicePort)) - g.Expect(svcIPFamily).To(Equal(test.expSvcIPFamily)) + g.Expect(svc).To(Equal(test.expSvc)) }) } } @@ -790,6 +798,7 @@ func TestCreateBackend(t *testing.T) { Port: 80, }, }, + IPFamilies: []v1.IPFamily{v1.IPv4Protocol}, }, } } @@ -855,6 +864,7 @@ func TestCreateBackend(t *testing.T) { tests := []struct { expectedCondition *conditions.Condition + nginxProxy *NginxProxy name string expectedServicePortReference string ref gatewayv1.HTTPBackendRef @@ -952,6 +962,30 @@ func TestCreateBackend(t *testing.T) { ), name: "service doesn't exist", }, + { + ref: gatewayv1.HTTPBackendRef{ + BackendRef: getModifiedRef(func(backend gatewayv1.BackendRef) gatewayv1.BackendRef { + backend.Name = "service2" + return backend + }), + }, + expectedBackend: BackendRef{ + SvcNsName: svc2NamespacedName, + ServicePort: svc1.Spec.Ports[0], + Weight: 5, + Valid: false, + }, + nginxProxy: &NginxProxy{ + Source: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{IPFamily: helpers.GetPointer(ngfAPI.IPv6)}, + }, + Valid: true, + }, + expectedCondition: helpers.GetPointer( + staticConds.NewRouteInvalidIPFamily(`service configured with IPv4 family but NginxProxy is configured with IPv6`), + ), + name: "service IPFamily doesn't match NginxProxy IPFamily", + }, { ref: gatewayv1.HTTPBackendRef{ BackendRef: getModifiedRef(func(backend gatewayv1.BackendRef) gatewayv1.BackendRef { @@ -1024,7 +1058,7 @@ func TestCreateBackend(t *testing.T) { services, refPath, policies, - &NginxProxy{}, + test.nginxProxy, ) g.Expect(helpers.Diff(test.expectedBackend, backend)).To(BeEmpty()) diff --git a/site/content/how-to/monitoring/troubleshooting.md b/site/content/how-to/monitoring/troubleshooting.md index 6de1f8f76e..ac4b657e09 100644 --- a/site/content/how-to/monitoring/troubleshooting.md +++ b/site/content/how-to/monitoring/troubleshooting.md @@ -405,6 +405,33 @@ Or view the following error message in the NGINX logs: The request body exceeds the [client_max_body_size](https://nginx.org/en/docs/http/ngx_http_core_module.html#client_max_body_size). To **resolve** this, you can configure the `client_max_body_size` using the `ClientSettingsPolicy` API. Read the [Client Settings Policy]({{< relref "how-to/traffic-management/client-settings.md" >}}) documentation for more information. +##### IP Family Mismatch Errors + +If you `describe` your HTTPRoute and see the following error: + +```text + Conditions: + Last Transition Time: 2024-07-14T23:36:37Z + Message: The route is accepted + Observed Generation: 1 + Reason: Accepted + Status: True + Type: Accepted + Last Transition Time: 2024-07-14T23:36:37Z + Message: service configured with IPv4 family but NginxProxy is configured with IPv6 + Observed Generation: 1 + Reason: InvalidServiceIPFamily + Status: False + Type: ResolvedRefs + Controller Name: gateway.nginx.org/nginx-gateway-controller +``` + +The Service associated with your HTTPRoute is configured with a IP Family different than the one specified in NginxProxy configuration. +To **resolve** this, you can do the following: + +- Change the IPFamily by modifying the field `nginx.config.ipFamily` in the `values.yaml` or add the `--set nginx.config.ipFamily=` flag to the `helm install` command. The supported IPFamilies are `ipv4`, `ipv6` and `dual` (default). + +- Adjust the IPFamily of your Service to match that of NginxProxy. ### Further reading diff --git a/site/content/overview/gateway-api-compatibility.md b/site/content/overview/gateway-api-compatibility.md index 50c118a41e..c7dd0e0a9b 100644 --- a/site/content/overview/gateway-api-compatibility.md +++ b/site/content/overview/gateway-api-compatibility.md @@ -179,6 +179,7 @@ See the [static-mode]({{< relref "/reference/cli-help.md#static-mode">}}) comman - `ResolvedRefs/False/RefNotPermitted` - `ResolvedRefs/False/BackendNotFound` - `ResolvedRefs/False/UnsupportedValue`: Custom reason for when one of the HTTPRoute rules has a backendRef with an unsupported value. + - `ResolvedRefs/False/InvalidIPFamily`: Custom reason for when one of the HTTPRoute rules has a backendRef that has unsupported IPFamily. - `PartiallyInvalid/True/UnsupportedValue` --- From e3c1b57ed7870ab0ba394ca1924fd2c3dcf28ff3 Mon Sep 17 00:00:00 2001 From: Saloni Date: Mon, 15 Jul 2024 09:20:49 -0600 Subject: [PATCH 24/28] update doc --- site/content/overview/gateway-api-compatibility.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/content/overview/gateway-api-compatibility.md b/site/content/overview/gateway-api-compatibility.md index c7dd0e0a9b..c106ec85af 100644 --- a/site/content/overview/gateway-api-compatibility.md +++ b/site/content/overview/gateway-api-compatibility.md @@ -179,7 +179,7 @@ See the [static-mode]({{< relref "/reference/cli-help.md#static-mode">}}) comman - `ResolvedRefs/False/RefNotPermitted` - `ResolvedRefs/False/BackendNotFound` - `ResolvedRefs/False/UnsupportedValue`: Custom reason for when one of the HTTPRoute rules has a backendRef with an unsupported value. - - `ResolvedRefs/False/InvalidIPFamily`: Custom reason for when one of the HTTPRoute rules has a backendRef that has unsupported IPFamily. + - `ResolvedRefs/False/InvalidIPFamily`: Custom reason for when one of the HTTPRoute rules has a backendRef that has invalid IPFamily. - `PartiallyInvalid/True/UnsupportedValue` --- From 2ea4db67c74fd96bf7aa8affcff97ff93bd5eaf1 Mon Sep 17 00:00:00 2001 From: Saloni Date: Mon, 15 Jul 2024 09:23:37 -0600 Subject: [PATCH 25/28] update troubleshooting guide --- site/content/how-to/monitoring/troubleshooting.md | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/site/content/how-to/monitoring/troubleshooting.md b/site/content/how-to/monitoring/troubleshooting.md index ac4b657e09..a7c15ed8c6 100644 --- a/site/content/how-to/monitoring/troubleshooting.md +++ b/site/content/how-to/monitoring/troubleshooting.md @@ -426,12 +426,18 @@ If you `describe` your HTTPRoute and see the following error: Controller Name: gateway.nginx.org/nginx-gateway-controller ``` -The Service associated with your HTTPRoute is configured with a IP Family different than the one specified in NginxProxy configuration. -To **resolve** this, you can do the following: +The Service associated with your HTTPRoute is configured with a IP Family different than the one specified in the NginxProxy configuration. +To **resolve** this, you can do one of the following: -- Change the IPFamily by modifying the field `nginx.config.ipFamily` in the `values.yaml` or add the `--set nginx.config.ipFamily=` flag to the `helm install` command. The supported IPFamilies are `ipv4`, `ipv6` and `dual` (default). +- Update the NginxProxy configuration with the proper [`ipFamily`](({{< relref "reference/api.md" >}})) field. You can edit the NginxProxy configuration using `kubectl edit`. For example: -- Adjust the IPFamily of your Service to match that of NginxProxy. + ```shell + kubectl edit -n nginx-gateway nginxproxies.gateway.nginx.org ngf-proxy-config + ``` + +- When installing NGINX Gateway Fabric, change the IPFamily by modifying the field `nginx.config.ipFamily` in the `values.yaml` or add the `--set nginx.config.ipFamily=` flag to the `helm install` command. The supported IPFamilies are `ipv4`, `ipv6` and `dual` (default). + +- Adjust the IPFamily of your Service to match that of the NginxProxy configuration. ### Further reading From b52b90d440f749ec5999965789d82d101322dee6 Mon Sep 17 00:00:00 2001 From: Saloni Date: Mon, 15 Jul 2024 15:30:35 -0600 Subject: [PATCH 26/28] update based on reviews --- internal/mode/static/nginx/config/servers.go | 2 +- .../static/state/dataplane/configuration.go | 6 +- .../state/dataplane/configuration_test.go | 1646 +++++++---------- .../mode/static/state/graph/backend_refs.go | 17 +- .../static/state/graph/backend_refs_test.go | 12 +- .../overview/gateway-api-compatibility.md | 2 +- 6 files changed, 709 insertions(+), 976 deletions(-) diff --git a/internal/mode/static/nginx/config/servers.go b/internal/mode/static/nginx/config/servers.go index 80d7bf3de0..3aeefa47c7 100644 --- a/internal/mode/static/nginx/config/servers.go +++ b/internal/mode/static/nginx/config/servers.go @@ -93,7 +93,7 @@ func executeServers(conf dataplane.Configuration) []executeResult { // getIPFamily returns whether the server should be configured for IPv4, IPv6, or both. func getIPFamily(baseHTTPConfig dataplane.BaseHTTPConfig) http.IPFamily { - switch ip := baseHTTPConfig.IPFamily; ip { + switch baseHTTPConfig.IPFamily { case dataplane.IPv4: return http.IPFamily{IPv4: true} case dataplane.IPv6: diff --git a/internal/mode/static/state/dataplane/configuration.go b/internal/mode/static/state/dataplane/configuration.go index 536213a1d6..b81dbfe7a0 100644 --- a/internal/mode/static/state/dataplane/configuration.go +++ b/internal/mode/static/state/dataplane/configuration.go @@ -542,8 +542,10 @@ func getAllowedAddressType(ipFamily IPFamilyType) []discoveryV1.AddressType { return []discoveryV1.AddressType{discoveryV1.AddressTypeIPv4} case IPv6: return []discoveryV1.AddressType{discoveryV1.AddressTypeIPv6} - default: + case Dual: return []discoveryV1.AddressType{discoveryV1.AddressTypeIPv4, discoveryV1.AddressTypeIPv6} + default: + return []discoveryV1.AddressType{} } } @@ -686,7 +688,7 @@ func buildBaseHTTPConfig(g *graph.Graph) BaseHTTPConfig { } if g.NginxProxy.Source.Spec.IPFamily != nil { - switch ipFamily := g.NginxProxy.Source.Spec.IPFamily; *ipFamily { + switch *g.NginxProxy.Source.Spec.IPFamily { case ngfAPI.IPv4: baseConfig.IPFamily = IPv4 case ngfAPI.IPv6: diff --git a/internal/mode/static/state/dataplane/configuration_test.go b/internal/mode/static/state/dataplane/configuration_test.go index b0c1295d18..273bd2e071 100644 --- a/internal/mode/static/state/dataplane/configuration_test.go +++ b/internal/mode/static/state/dataplane/configuration_test.go @@ -27,6 +27,68 @@ import ( "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/resolver/resolverfakes" ) +func getNormalBackendRef() graph.BackendRef { + return graph.BackendRef{ + SvcNsName: types.NamespacedName{Name: "foo", Namespace: "test"}, + ServicePort: apiv1.ServicePort{Port: 80}, + Valid: true, + Weight: 1, + } +} + +func getModifiedBackendRef(mod func(ref graph.BackendRef) graph.BackendRef) graph.BackendRef { + return mod(getNormalBackendRef()) +} + +func getExpectedConfiguration() Configuration { + return Configuration{ + BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, + HTTPServers: []VirtualServer{{ + IsDefault: true, + Port: 80, + }}, + SSLServers: []VirtualServer{ + { + IsDefault: true, + Port: 443, + }, + }, + Upstreams: []Upstream{}, + BackendGroups: []BackendGroup{}, + SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{ + "ssl_keypair_test_secret-1": { + Cert: []byte("cert-1"), + Key: []byte("privateKey-1"), + }, + }, + CertBundles: map[CertBundleID]CertBundle{}, + } +} + +func getNormalGraph() *graph.Graph { + return &graph.Graph{ + GatewayClass: &graph.GatewayClass{ + Source: &v1.GatewayClass{}, + Valid: true, + }, + Gateway: &graph.Gateway{ + Source: &v1.Gateway{}, + Listeners: []*graph.Listener{}, + }, + Routes: map[graph.RouteKey]*graph.L7Route{}, + ReferencedSecrets: map[types.NamespacedName]*graph.Secret{}, + ReferencedCaCertConfigMaps: map[types.NamespacedName]*graph.CaCertConfigMap{}, + } +} + +func getModifiedGraph(mod func(g *graph.Graph) *graph.Graph) *graph.Graph { + return mod(getNormalGraph()) +} + +func getModifiedExpectedConfiguration(mod func(conf Configuration) Configuration) Configuration { + return mod(getExpectedConfiguration()) +} + func createFakePolicy(name string, kind string) policies.Policy { fakeKind := &policiesfakes.FakeObjectKind{ GroupVersionKindStub: func() schema.GroupVersionKind { @@ -121,12 +183,9 @@ func TestBuildConfiguration(t *testing.T) { fakeResolver := &resolverfakes.FakeServiceResolver{} fakeResolver.ResolveReturns(fooEndpoints, nil) - validBackendRef := graph.BackendRef{ - SvcNsName: types.NamespacedName{Name: "foo", Namespace: "test"}, - ServicePort: apiv1.ServicePort{Port: 80}, - Valid: true, - Weight: 1, - } + validBackendRef := getModifiedBackendRef(func(backend graph.BackendRef) graph.BackendRef { + return backend + }) expValidBackend := Backend{ UpstreamName: fooUpstreamName, @@ -666,161 +725,97 @@ func TestBuildConfiguration(t *testing.T) { expConf Configuration }{ { - graph: &graph.Graph{ - GatewayClass: &graph.GatewayClass{ - Source: &v1.GatewayClass{}, - Valid: true, - }, - Gateway: &graph.Gateway{ - Source: &v1.Gateway{}, - Listeners: []*graph.Listener{}, - }, - Routes: map[graph.RouteKey]*graph.L7Route{}, - }, - expConf: Configuration{ - HTTPServers: []VirtualServer{}, - SSLServers: []VirtualServer{}, - SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{}, - CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, - }, + graph: getNormalGraph(), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.HTTPServers = []VirtualServer{} + conf.SSLServers = []VirtualServer{} + conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} + return conf + }), msg: "no listeners and routes", }, { - graph: &graph.Graph{ - GatewayClass: &graph.GatewayClass{ - Source: &v1.GatewayClass{}, + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{ + Name: "listener-80-1", + Source: listener80, Valid: true, - }, - Gateway: &graph.Gateway{ - Source: &v1.Gateway{}, - Listeners: []*graph.Listener{ - { - Name: "listener-80-1", - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{}, - }, - }, - }, - Routes: map[graph.RouteKey]*graph.L7Route{}, - }, - expConf: Configuration{ - HTTPServers: []VirtualServer{ - { - IsDefault: true, - Port: 80, - }, - }, - SSLServers: []VirtualServer{}, - SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{}, - CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, - }, + }) + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.SSLServers = []VirtualServer{} + conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} + return conf + }), msg: "http listener with no routes", }, { - graph: &graph.Graph{ - GatewayClass: &graph.GatewayClass{ - Source: &v1.GatewayClass{}, - Valid: true, - }, - Gateway: &graph.Gateway{ - Source: &v1.Gateway{}, - Listeners: []*graph.Listener{ - { - Name: "listener-80-1", - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(hr1Invalid): routeHR1Invalid, - }, - }, - { - Name: "listener-443-1", - Source: listener443, // nil hostname - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(httpsHR1Invalid): httpsRouteHR1Invalid, - }, - ResolvedSecret: &secret1NsName, - }, - }, - }, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(hr1Invalid): routeHR1Invalid, - }, - ReferencedSecrets: map[types.NamespacedName]*graph.Secret{ - secret1NsName: secret1, - }, - }, - expConf: Configuration{ - HTTPServers: []VirtualServer{ + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{ { - IsDefault: true, - Port: 80, - }, - }, - SSLServers: []VirtualServer{ - { - IsDefault: true, - Port: 443, + Name: "listener-80-1", + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(hr1Invalid): routeHR1Invalid, + }, }, { - Hostname: wildcardHostname, - SSL: &SSL{KeyPairID: "ssl_keypair_test_secret-1"}, - Port: 443, - }, - }, - SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{ - "ssl_keypair_test_secret-1": { - Cert: []byte("cert-1"), - Key: []byte("privateKey-1"), - }, - }, - CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, - }, + Name: "listener-443-1", + Source: listener443, // nil hostname + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(httpsHR1Invalid): httpsRouteHR1Invalid, + }, + ResolvedSecret: &secret1NsName, + }, + }...) + g.Routes[graph.CreateRouteKey(hr1Invalid)] = routeHR1Invalid + g.ReferencedSecrets[secret1NsName] = secret1 + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.HTTPServers = []VirtualServer{{ + IsDefault: true, + Port: 80, + }} + conf.SSLServers = append(conf.SSLServers, VirtualServer{ + Hostname: wildcardHostname, + SSL: &SSL{KeyPairID: "ssl_keypair_test_secret-1"}, + Port: 443, + }) + return conf + }), msg: "http and https listeners with no valid routes", }, { - graph: &graph.Graph{ - GatewayClass: &graph.GatewayClass{ - Source: &v1.GatewayClass{}, - Valid: true, - }, - Gateway: &graph.Gateway{ - Source: &v1.Gateway{}, - Listeners: []*graph.Listener{ - { - Name: "listener-443-1", - Source: listener443, // nil hostname - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{}, - ResolvedSecret: &secret1NsName, - }, - { - Name: "listener-443-with-hostname", - Source: listener443WithHostname, // non-nil hostname - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{}, - ResolvedSecret: &secret2NsName, - }, + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{ + { + Name: "listener-443-1", + Source: listener443, // nil hostname + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{}, + ResolvedSecret: &secret1NsName, }, - }, - Routes: map[graph.RouteKey]*graph.L7Route{}, - ReferencedSecrets: map[types.NamespacedName]*graph.Secret{ + { + Name: "listener-443-with-hostname", + Source: listener443WithHostname, // non-nil hostname + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{}, + ResolvedSecret: &secret2NsName, + }, + }...) + g.ReferencedSecrets = map[types.NamespacedName]*graph.Secret{ secret1NsName: secret1, secret2NsName: secret2, - }, - }, - expConf: Configuration{ - HTTPServers: []VirtualServer{}, - SSLServers: []VirtualServer{ - { - IsDefault: true, - Port: 443, - }, + } + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.HTTPServers = []VirtualServer{} + conf.SSLServers = append(conf.SSLServers, []VirtualServer{ { Hostname: string(hostname), SSL: &SSL{KeyPairID: "ssl_keypair_test_secret-2"}, @@ -831,87 +826,61 @@ func TestBuildConfiguration(t *testing.T) { SSL: &SSL{KeyPairID: "ssl_keypair_test_secret-1"}, Port: 443, }, - }, - SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{ - "ssl_keypair_test_secret-1": { - Cert: []byte("cert-1"), - Key: []byte("privateKey-1"), - }, - "ssl_keypair_test_secret-2": { - Cert: []byte("cert-2"), - Key: []byte("privateKey-2"), - }, - }, - CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, - }, + }...) + conf.SSLKeyPairs["ssl_keypair_test_secret-2"] = SSLKeyPair{ + Cert: []byte("cert-2"), + Key: []byte("privateKey-2"), + } + return conf + }), msg: "https listeners with no routes", }, { - graph: &graph.Graph{ - GatewayClass: &graph.GatewayClass{ - Source: &v1.GatewayClass{}, - Valid: true, - }, - Gateway: &graph.Gateway{ - Source: &v1.Gateway{}, - Listeners: []*graph.Listener{ - { - Name: "invalid-listener", - Source: invalidListener, - Valid: false, - ResolvedSecret: &secret1NsName, - }, + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{ + { + Name: "invalid-listener", + Source: invalidListener, + Valid: false, + ResolvedSecret: &secret1NsName, }, - }, - Routes: map[graph.RouteKey]*graph.L7Route{ + }...) + g.Routes = map[graph.RouteKey]*graph.L7Route{ graph.CreateRouteKey(httpsHR1): httpsRouteHR1, graph.CreateRouteKey(httpsHR2): httpsRouteHR2, - }, - ReferencedSecrets: map[types.NamespacedName]*graph.Secret{ - secret1NsName: secret1, - }, - }, - expConf: Configuration{ - HTTPServers: []VirtualServer{}, - SSLServers: []VirtualServer{}, - SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{}, - CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, - }, + } + g.ReferencedSecrets[secret1NsName] = secret1 + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.HTTPServers = []VirtualServer{} + conf.SSLServers = []VirtualServer{} + conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} + return conf + }), msg: "invalid https listener with resolved secret", }, { - graph: &graph.Graph{ - GatewayClass: &graph.GatewayClass{ - Source: &v1.GatewayClass{}, - Valid: true, - }, - Gateway: &graph.Gateway{ - Source: &v1.Gateway{}, - Listeners: []*graph.Listener{ - { - Name: "listener-80-1", - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(hr1): routeHR1, - graph.CreateRouteKey(hr2): routeHR2, - }, + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{ + { + Name: "listener-80-1", + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(hr1): routeHR1, + graph.CreateRouteKey(hr2): routeHR2, }, }, - }, - Routes: map[graph.RouteKey]*graph.L7Route{ + }...) + g.Routes = map[graph.RouteKey]*graph.L7Route{ graph.CreateRouteKey(hr1): routeHR1, graph.CreateRouteKey(hr2): routeHR2, - }, - }, - expConf: Configuration{ - HTTPServers: []VirtualServer{ - { - IsDefault: true, - Port: 80, - }, + } + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.HTTPServers = append(conf.HTTPServers, []VirtualServer{ { Hostname: "bar.example.com", PathRules: []PathRule{ @@ -944,119 +913,95 @@ func TestBuildConfiguration(t *testing.T) { }, Port: 80, }, - }, - SSLServers: []VirtualServer{}, - Upstreams: []Upstream{fooUpstream}, - BackendGroups: []BackendGroup{expHR1Groups[0], expHR2Groups[0]}, - SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{}, - CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, - }, + }...) + conf.SSLServers = []VirtualServer{} + conf.Upstreams = []Upstream{fooUpstream} + conf.BackendGroups = []BackendGroup{expHR1Groups[0], expHR2Groups[0]} + conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} + + return conf + }), msg: "one http listener with two routes for different hostnames", }, { - graph: &graph.Graph{ - GatewayClass: &graph.GatewayClass{ - Source: &v1.GatewayClass{}, - Valid: true, - }, - Gateway: &graph.Gateway{ - Source: &v1.Gateway{}, - Listeners: []*graph.Listener{ - { - Name: "listener-80-1", - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(gr): routeGR, - }, - }, - }, - }, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(gr): routeGR, - }, - }, - expConf: Configuration{ - HTTPServers: []VirtualServer{ + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{ { - IsDefault: true, - Port: 80, + Name: "listener-80-1", + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(gr): routeGR, + }, }, - { - Hostname: "foo.example.com", - PathRules: []PathRule{ - { - Path: "/", - PathType: PathTypePrefix, - GRPC: true, - MatchRules: []MatchRule{ - { - BackendGroup: expGRGroups[0], - Source: &gr.ObjectMeta, - }, + }...) + g.Routes[graph.CreateRouteKey(gr)] = routeGR + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.HTTPServers = append(conf.HTTPServers, VirtualServer{ + Hostname: "foo.example.com", + PathRules: []PathRule{ + { + Path: "/", + PathType: PathTypePrefix, + GRPC: true, + MatchRules: []MatchRule{ + { + BackendGroup: expGRGroups[0], + Source: &gr.ObjectMeta, }, }, }, - Port: 80, }, + Port: 80, }, - SSLServers: []VirtualServer{}, - Upstreams: []Upstream{fooUpstream}, - BackendGroups: []BackendGroup{expGRGroups[0]}, - SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{}, - CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, - }, + ) + conf.SSLServers = []VirtualServer{} + conf.Upstreams = append(conf.Upstreams, fooUpstream) + conf.BackendGroups = []BackendGroup{expGRGroups[0]} + conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} + return conf + }), msg: "one http listener with one grpc route", }, { - graph: &graph.Graph{ - GatewayClass: &graph.GatewayClass{ - Source: &v1.GatewayClass{}, - Valid: true, - }, - Gateway: &graph.Gateway{ - Source: &v1.Gateway{}, - Listeners: []*graph.Listener{ - { - Name: "listener-443-1", - Source: listener443, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(httpsHR1): httpsRouteHR1, - graph.CreateRouteKey(httpsHR2): httpsRouteHR2, - }, - ResolvedSecret: &secret1NsName, + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{ + { + Name: "listener-443-1", + Source: listener443, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(httpsHR1): httpsRouteHR1, + graph.CreateRouteKey(httpsHR2): httpsRouteHR2, }, - { - Name: "listener-443-with-hostname", - Source: listener443WithHostname, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(httpsHR5): httpsRouteHR5, - }, - ResolvedSecret: &secret2NsName, + ResolvedSecret: &secret1NsName, + }, + { + Name: "listener-443-with-hostname", + Source: listener443WithHostname, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(httpsHR5): httpsRouteHR5, }, + ResolvedSecret: &secret2NsName, }, - }, - Routes: map[graph.RouteKey]*graph.L7Route{ + }...) + g.Routes = map[graph.RouteKey]*graph.L7Route{ graph.CreateRouteKey(hr1): httpsRouteHR1, graph.CreateRouteKey(hr2): httpsRouteHR2, graph.CreateRouteKey(httpsHR5): httpsRouteHR5, - }, - ReferencedSecrets: map[types.NamespacedName]*graph.Secret{ + } + g.ReferencedSecrets = map[types.NamespacedName]*graph.Secret{ secret1NsName: secret1, secret2NsName: secret2, - }, - }, - expConf: Configuration{ - HTTPServers: []VirtualServer{}, - SSLServers: []VirtualServer{ - { - IsDefault: true, - Port: 443, - }, + } + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.HTTPServers = []VirtualServer{} + conf.SSLServers = append(conf.SSLServers, []VirtualServer{ { Hostname: "bar.example.com", PathRules: []PathRule{ @@ -1113,10 +1058,10 @@ func TestBuildConfiguration(t *testing.T) { SSL: &SSL{KeyPairID: "ssl_keypair_test_secret-1"}, Port: 443, }, - }, - Upstreams: []Upstream{fooUpstream}, - BackendGroups: []BackendGroup{expHTTPSHR1Groups[0], expHTTPSHR2Groups[0], expHTTPSHR5Groups[0]}, - SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{ + }...) + conf.Upstreams = []Upstream{fooUpstream} + conf.BackendGroups = []BackendGroup{expHTTPSHR1Groups[0], expHTTPSHR2Groups[0], expHTTPSHR5Groups[0]} + conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{ "ssl_keypair_test_secret-1": { Cert: []byte("cert-1"), Key: []byte("privateKey-1"), @@ -1125,58 +1070,47 @@ func TestBuildConfiguration(t *testing.T) { Cert: []byte("cert-2"), Key: []byte("privateKey-2"), }, - }, - CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, - }, + } + return conf + }), msg: "two https listeners each with routes for different hostnames", }, { - graph: &graph.Graph{ - GatewayClass: &graph.GatewayClass{ - Source: &v1.GatewayClass{}, - Valid: true, - }, - Gateway: &graph.Gateway{ - Source: &v1.Gateway{}, - Listeners: []*graph.Listener{ - { - Name: "listener-80-1", - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(hr3): routeHR3, - graph.CreateRouteKey(hr4): routeHR4, - }, + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{ + { + Name: "listener-80-1", + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(hr3): routeHR3, + graph.CreateRouteKey(hr4): routeHR4, }, - { - Name: "listener-443-1", - Source: listener443, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(httpsHR3): httpsRouteHR3, - graph.CreateRouteKey(httpsHR4): httpsRouteHR4, - }, - ResolvedSecret: &secret1NsName, + }, + { + Name: "listener-443-1", + Source: listener443, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(httpsHR3): httpsRouteHR3, + graph.CreateRouteKey(httpsHR4): httpsRouteHR4, }, + ResolvedSecret: &secret1NsName, }, - }, - Routes: map[graph.RouteKey]*graph.L7Route{ + }...) + g.Routes = map[graph.RouteKey]*graph.L7Route{ graph.CreateRouteKey(hr3): routeHR3, graph.CreateRouteKey(hr4): routeHR4, graph.CreateRouteKey(httpsHR3): httpsRouteHR3, graph.CreateRouteKey(httpsHR4): httpsRouteHR4, - }, - ReferencedSecrets: map[types.NamespacedName]*graph.Secret{ + } + g.ReferencedSecrets = map[types.NamespacedName]*graph.Secret{ secret1NsName: secret1, - }, - }, - expConf: Configuration{ - HTTPServers: []VirtualServer{ - { - IsDefault: true, - Port: 80, - }, + } + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.HTTPServers = append(conf.HTTPServers, []VirtualServer{ { Hostname: "foo.example.com", PathRules: []PathRule{ @@ -1217,12 +1151,8 @@ func TestBuildConfiguration(t *testing.T) { }, Port: 80, }, - }, - SSLServers: []VirtualServer{ - { - IsDefault: true, - Port: 443, - }, + }...) + conf.SSLServers = append(conf.SSLServers, []VirtualServer{ { Hostname: "foo.example.com", SSL: &SSL{KeyPairID: "ssl_keypair_test_secret-1"}, @@ -1269,9 +1199,9 @@ func TestBuildConfiguration(t *testing.T) { SSL: &SSL{KeyPairID: "ssl_keypair_test_secret-1"}, Port: 443, }, - }, - Upstreams: []Upstream{fooUpstream}, - BackendGroups: []BackendGroup{ + }...) + conf.Upstreams = append(conf.Upstreams, fooUpstream) + conf.BackendGroups = []BackendGroup{ expHR3Groups[0], expHR3Groups[1], expHR4Groups[0], @@ -1280,79 +1210,62 @@ func TestBuildConfiguration(t *testing.T) { expHTTPSHR3Groups[1], expHTTPSHR4Groups[0], expHTTPSHR4Groups[1], - }, - SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{ - "ssl_keypair_test_secret-1": { - Cert: []byte("cert-1"), - Key: []byte("privateKey-1"), - }, - }, - CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, - }, + } + return conf + }), msg: "one http and one https listener with two routes with the same hostname with and without collisions", }, { - graph: &graph.Graph{ - GatewayClass: &graph.GatewayClass{ - Source: &v1.GatewayClass{}, - Valid: true, - }, - Gateway: &graph.Gateway{ - Source: &v1.Gateway{}, - Listeners: []*graph.Listener{ - { - Name: "listener-80-1", - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(hr3): routeHR3, - }, + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{ + { + Name: "listener-80-1", + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(hr3): routeHR3, }, - { - Name: "listener-8080", - Source: listener8080, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(hr8): routeHR8, - }, + }, + { + Name: "listener-8080", + Source: listener8080, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(hr8): routeHR8, }, - { - Name: "listener-443-1", - Source: listener443, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(httpsHR3): httpsRouteHR3, - }, - ResolvedSecret: &secret1NsName, + }, + { + Name: "listener-443-1", + Source: listener443, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(httpsHR3): httpsRouteHR3, }, - { - Name: "listener-8443", - Source: listener8443, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(httpsHR7): httpsRouteHR7, - }, - ResolvedSecret: &secret1NsName, + ResolvedSecret: &secret1NsName, + }, + { + Name: "listener-8443", + Source: listener8443, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(httpsHR7): httpsRouteHR7, }, + ResolvedSecret: &secret1NsName, }, - }, - Routes: map[graph.RouteKey]*graph.L7Route{ + }...) + g.Routes = map[graph.RouteKey]*graph.L7Route{ graph.CreateRouteKey(hr3): routeHR3, graph.CreateRouteKey(hr8): routeHR8, graph.CreateRouteKey(httpsHR3): httpsRouteHR3, graph.CreateRouteKey(httpsHR7): httpsRouteHR7, - }, - ReferencedSecrets: map[types.NamespacedName]*graph.Secret{ + } + g.ReferencedSecrets = map[types.NamespacedName]*graph.Secret{ secret1NsName: secret1, - }, - }, - expConf: Configuration{ - HTTPServers: []VirtualServer{ - { - IsDefault: true, - Port: 80, - }, + } + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.HTTPServers = append(conf.HTTPServers, []VirtualServer{ { Hostname: "foo.example.com", PathRules: []PathRule{ @@ -1409,12 +1322,8 @@ func TestBuildConfiguration(t *testing.T) { }, Port: 8080, }, - }, - SSLServers: []VirtualServer{ - { - IsDefault: true, - Port: 443, - }, + }...) + conf.SSLServers = append(conf.SSLServers, []VirtualServer{ { Hostname: "foo.example.com", SSL: &SSL{KeyPairID: "ssl_keypair_test_secret-1"}, @@ -1483,9 +1392,9 @@ func TestBuildConfiguration(t *testing.T) { SSL: &SSL{KeyPairID: "ssl_keypair_test_secret-1"}, Port: 8443, }, - }, - Upstreams: []Upstream{fooUpstream}, - BackendGroups: []BackendGroup{ + }...) + conf.Upstreams = append(conf.Upstreams, fooUpstream) + conf.BackendGroups = []BackendGroup{ expHR3Groups[0], expHR3Groups[1], expHR8Groups[0], @@ -1494,109 +1403,80 @@ func TestBuildConfiguration(t *testing.T) { expHTTPSHR3Groups[1], expHTTPSHR7Groups[0], expHTTPSHR7Groups[1], - }, - SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{ - "ssl_keypair_test_secret-1": { - Cert: []byte("cert-1"), - Key: []byte("privateKey-1"), - }, - }, - CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, - }, - + } + return conf + }), msg: "multiple http and https listener; different ports", }, { - graph: &graph.Graph{ - GatewayClass: &graph.GatewayClass{ - Source: &v1.GatewayClass{}, - Valid: false, - }, - Gateway: &graph.Gateway{ - Source: &v1.Gateway{}, - Listeners: []*graph.Listener{ - { - Name: "listener-80-1", - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(hr1): routeHR1, - }, + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + g.GatewayClass.Valid = false + g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{ + { + Name: "listener-80-1", + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(hr1): routeHR1, }, }, - }, - Routes: map[graph.RouteKey]*graph.L7Route{ + }...) + g.Routes = map[graph.RouteKey]*graph.L7Route{ graph.CreateRouteKey(hr1): routeHR1, - }, - }, + } + return g + }), expConf: Configuration{}, msg: "invalid gatewayclass", }, { - graph: &graph.Graph{ - GatewayClass: nil, - Gateway: &graph.Gateway{ - Source: &v1.Gateway{}, - Listeners: []*graph.Listener{ - { - Name: "listener-80-1", - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(hr1): routeHR1, - }, + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + g.GatewayClass.Valid = false + g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{ + { + Name: "listener-80-1", + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(hr1): routeHR1, }, }, - }, - Routes: map[graph.RouteKey]*graph.L7Route{ + }...) + g.Routes = map[graph.RouteKey]*graph.L7Route{ graph.CreateRouteKey(hr1): routeHR1, - }, - }, + } + return g + }), expConf: Configuration{}, msg: "missing gatewayclass", }, { - graph: &graph.Graph{ - GatewayClass: &graph.GatewayClass{ - Source: &v1.GatewayClass{}, - Valid: true, - }, - Gateway: nil, - Routes: map[graph.RouteKey]*graph.L7Route{}, - }, + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + g.Gateway = nil + return g + }), expConf: Configuration{}, msg: "missing gateway", }, { - graph: &graph.Graph{ - GatewayClass: &graph.GatewayClass{ - Source: &v1.GatewayClass{}, - Valid: true, - }, - Gateway: &graph.Gateway{ - Source: &v1.Gateway{}, - Listeners: []*graph.Listener{ - { - Name: "listener-80-1", - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(hr5): routeHR5, - }, + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{ + { + Name: "listener-80-1", + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(hr5): routeHR5, }, }, - }, - Routes: map[graph.RouteKey]*graph.L7Route{ + }...) + g.Routes = map[graph.RouteKey]*graph.L7Route{ graph.CreateRouteKey(hr5): routeHR5, - }, - }, - expConf: Configuration{ - HTTPServers: []VirtualServer{ - { - IsDefault: true, - Port: 80, - }, + } + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.HTTPServers = append(conf.HTTPServers, []VirtualServer{ { Hostname: "foo.example.com", PathRules: []PathRule{ @@ -1629,58 +1509,47 @@ func TestBuildConfiguration(t *testing.T) { }, Port: 80, }, - }, - SSLServers: []VirtualServer{}, - Upstreams: []Upstream{fooUpstream}, - BackendGroups: []BackendGroup{expHR5Groups[0], expHR5Groups[1]}, - SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{}, - CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, - }, + }...) + conf.SSLServers = []VirtualServer{} + conf.Upstreams = []Upstream{fooUpstream} + conf.BackendGroups = []BackendGroup{expHR5Groups[0], expHR5Groups[1]} + conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} + return conf + }), msg: "one http listener with one route with filters", }, { - graph: &graph.Graph{ - GatewayClass: &graph.GatewayClass{ - Source: &v1.GatewayClass{}, - Valid: true, - }, - Gateway: &graph.Gateway{ - Source: &v1.Gateway{}, - Listeners: []*graph.Listener{ - { - Name: "listener-80-1", - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(hr6): routeHR6, - }, + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{ + { + Name: "listener-80-1", + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(hr6): routeHR6, }, - { - Name: "listener-443-1", - Source: listener443, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(httpsHR6): httpsRouteHR6, - }, - ResolvedSecret: &secret1NsName, + }, + { + Name: "listener-443-1", + Source: listener443, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(httpsHR6): httpsRouteHR6, }, + ResolvedSecret: &secret1NsName, }, - }, - Routes: map[graph.RouteKey]*graph.L7Route{ + }...) + g.Routes = map[graph.RouteKey]*graph.L7Route{ graph.CreateRouteKey(hr6): routeHR6, graph.CreateRouteKey(httpsHR6): httpsRouteHR6, - }, - ReferencedSecrets: map[types.NamespacedName]*graph.Secret{ + } + g.ReferencedSecrets = map[types.NamespacedName]*graph.Secret{ secret1NsName: secret1, - }, - }, - expConf: Configuration{ - HTTPServers: []VirtualServer{ - { - IsDefault: true, - Port: 80, - }, + } + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.HTTPServers = append(conf.HTTPServers, []VirtualServer{ { Hostname: "foo.example.com", PathRules: []PathRule{ @@ -1697,12 +1566,8 @@ func TestBuildConfiguration(t *testing.T) { }, Port: 80, }, - }, - SSLServers: []VirtualServer{ - { - IsDefault: true, - Port: 443, - }, + }...) + conf.SSLServers = append(conf.SSLServers, []VirtualServer{ { Hostname: "foo.example.com", SSL: &SSL{KeyPairID: "ssl_keypair_test_secret-1"}, @@ -1725,52 +1590,32 @@ func TestBuildConfiguration(t *testing.T) { SSL: &SSL{KeyPairID: "ssl_keypair_test_secret-1"}, Port: 443, }, - }, - Upstreams: []Upstream{fooUpstream}, - BackendGroups: []BackendGroup{ - expHR6Groups[0], - expHTTPSHR6Groups[0], - }, - SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{ - "ssl_keypair_test_secret-1": { - Cert: []byte("cert-1"), - Key: []byte("privateKey-1"), - }, - }, - CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, - }, + }...) + conf.Upstreams = []Upstream{fooUpstream} + conf.BackendGroups = []BackendGroup{expHR6Groups[0], expHTTPSHR6Groups[0]} + return conf + }), msg: "one http and one https listener with routes with valid and invalid rules", }, { - graph: &graph.Graph{ - GatewayClass: &graph.GatewayClass{ - Source: &v1.GatewayClass{}, - Valid: true, - }, - Gateway: &graph.Gateway{ - Source: &v1.Gateway{}, - Listeners: []*graph.Listener{ - { - Name: "listener-80-1", - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(hr7): routeHR7, - }, + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{ + { + Name: "listener-80-1", + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(hr7): routeHR7, }, }, - }, - Routes: map[graph.RouteKey]*graph.L7Route{ + }...) + g.Routes = map[graph.RouteKey]*graph.L7Route{ graph.CreateRouteKey(hr7): routeHR7, - }, - }, - expConf: Configuration{ - HTTPServers: []VirtualServer{ - { - IsDefault: true, - Port: 80, - }, + } + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.HTTPServers = append(conf.HTTPServers, []VirtualServer{ { Hostname: "foo.example.com", PathRules: []PathRule{ @@ -1797,60 +1642,48 @@ func TestBuildConfiguration(t *testing.T) { }, Port: 80, }, - }, - SSLServers: []VirtualServer{}, - Upstreams: []Upstream{fooUpstream}, - BackendGroups: []BackendGroup{expHR7Groups[0], expHR7Groups[1]}, - SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{}, - CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, - }, + }...) + conf.SSLServers = []VirtualServer{} + conf.Upstreams = []Upstream{fooUpstream} + conf.BackendGroups = []BackendGroup{expHR7Groups[0], expHR7Groups[1]} + conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} + return conf + }), msg: "duplicate paths with different types", }, { - graph: &graph.Graph{ - GatewayClass: &graph.GatewayClass{ - Source: &v1.GatewayClass{}, - Valid: true, - }, - Gateway: &graph.Gateway{ - Source: &v1.Gateway{}, - Listeners: []*graph.Listener{ - { - Name: "listener-443-with-hostname", - Source: listener443WithHostname, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(httpsHR5): httpsRouteHR5, - }, - ResolvedSecret: &secret2NsName, + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{ + { + Name: "listener-443-with-hostname", + Source: listener443WithHostname, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(httpsHR5): httpsRouteHR5, }, - { - Name: "listener-443-1", - Source: listener443, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(httpsHR5): httpsRouteHR5, - }, - ResolvedSecret: &secret1NsName, + ResolvedSecret: &secret2NsName, + }, + { + Name: "listener-443-1", + Source: listener443, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(httpsHR5): httpsRouteHR5, }, + ResolvedSecret: &secret1NsName, }, - }, - Routes: map[graph.RouteKey]*graph.L7Route{ + }...) + g.Routes = map[graph.RouteKey]*graph.L7Route{ graph.CreateRouteKey(httpsHR5): httpsRouteHR5, - }, - ReferencedSecrets: map[types.NamespacedName]*graph.Secret{ + } + g.ReferencedSecrets = map[types.NamespacedName]*graph.Secret{ secret1NsName: secret1, secret2NsName: secret2, - }, - }, - expConf: Configuration{ - HTTPServers: []VirtualServer{}, - SSLServers: []VirtualServer{ - { - IsDefault: true, - Port: 443, - }, + } + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.SSLServers = append(conf.SSLServers, []VirtualServer{ { Hostname: "example.com", PathRules: []PathRule{ @@ -1878,10 +1711,11 @@ func TestBuildConfiguration(t *testing.T) { SSL: &SSL{KeyPairID: "ssl_keypair_test_secret-1"}, Port: 443, }, - }, - Upstreams: []Upstream{fooUpstream}, - BackendGroups: []BackendGroup{expHTTPSHR5Groups[0]}, - SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{ + }...) + conf.HTTPServers = []VirtualServer{} + conf.Upstreams = []Upstream{fooUpstream} + conf.BackendGroups = []BackendGroup{expHTTPSHR5Groups[0]} + conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{ "ssl_keypair_test_secret-1": { Cert: []byte("cert-1"), Key: []byte("privateKey-1"), @@ -1890,47 +1724,35 @@ func TestBuildConfiguration(t *testing.T) { Cert: []byte("cert-2"), Key: []byte("privateKey-2"), }, - }, - CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, - }, + } + return conf + }), msg: "two https listeners with different hostnames but same route; chooses listener with more specific hostname", }, { - graph: &graph.Graph{ - GatewayClass: &graph.GatewayClass{ - Source: &v1.GatewayClass{}, - Valid: true, - }, - Gateway: &graph.Gateway{ - Source: &v1.Gateway{}, - Listeners: []*graph.Listener{ - { - Name: "listener-443", - Source: listener443, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(httpsHR8): httpsRouteHR8, - }, - ResolvedSecret: &secret1NsName, + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{ + { + Name: "listener-443", + Source: listener443, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(httpsHR8): httpsRouteHR8, }, + ResolvedSecret: &secret1NsName, }, - }, - Routes: map[graph.RouteKey]*graph.L7Route{ + }...) + g.Routes = map[graph.RouteKey]*graph.L7Route{ graph.CreateRouteKey(httpsHR8): httpsRouteHR8, - }, - ReferencedSecrets: map[types.NamespacedName]*graph.Secret{ + } + g.ReferencedSecrets = map[types.NamespacedName]*graph.Secret{ secret1NsName: secret1, - }, - ReferencedCaCertConfigMaps: referencedConfigMaps, - }, - expConf: Configuration{ - HTTPServers: []VirtualServer{}, - SSLServers: []VirtualServer{ - { - IsDefault: true, - Port: 443, - }, + } + g.ReferencedCaCertConfigMaps = referencedConfigMaps + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.SSLServers = append(conf.SSLServers, []VirtualServer{ { Hostname: "foo.example.com", PathRules: []PathRule{ @@ -1957,57 +1779,41 @@ func TestBuildConfiguration(t *testing.T) { SSL: &SSL{KeyPairID: "ssl_keypair_test_secret-1"}, Port: 443, }, - }, - Upstreams: []Upstream{fooUpstream}, - BackendGroups: []BackendGroup{expHTTPSHR8Groups[0], expHTTPSHR8Groups[1]}, - SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{ - "ssl_keypair_test_secret-1": { - Cert: []byte("cert-1"), - Key: []byte("privateKey-1"), - }, - }, - CertBundles: map[CertBundleID]CertBundle{ + }...) + conf.HTTPServers = []VirtualServer{} + conf.Upstreams = []Upstream{fooUpstream} + conf.BackendGroups = []BackendGroup{expHTTPSHR8Groups[0], expHTTPSHR8Groups[1]} + conf.CertBundles = map[CertBundleID]CertBundle{ "cert_bundle_test_configmap-1": []byte("cert-1"), - }, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, - }, + } + return conf + }), msg: "https listener with httproute with backend that has a backend TLS policy attached", }, { - graph: &graph.Graph{ - GatewayClass: &graph.GatewayClass{ - Source: &v1.GatewayClass{}, - Valid: true, - }, - Gateway: &graph.Gateway{ - Source: &v1.Gateway{}, - Listeners: []*graph.Listener{ - { - Name: "listener-443", - Source: listener443, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(httpsHR9): httpsRouteHR9, - }, - ResolvedSecret: &secret1NsName, + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{ + { + Name: "listener-443", + Source: listener443, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(httpsHR9): httpsRouteHR9, }, + ResolvedSecret: &secret1NsName, }, - }, - Routes: map[graph.RouteKey]*graph.L7Route{ + }...) + g.Routes = map[graph.RouteKey]*graph.L7Route{ graph.CreateRouteKey(httpsHR9): httpsRouteHR9, - }, - ReferencedSecrets: map[types.NamespacedName]*graph.Secret{ + } + g.ReferencedSecrets = map[types.NamespacedName]*graph.Secret{ secret1NsName: secret1, - }, - ReferencedCaCertConfigMaps: referencedConfigMaps, - }, - expConf: Configuration{ - HTTPServers: []VirtualServer{}, - SSLServers: []VirtualServer{ - { - IsDefault: true, - Port: 443, - }, + } + g.ReferencedCaCertConfigMaps = referencedConfigMaps + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.SSLServers = append(conf.SSLServers, []VirtualServer{ { Hostname: "foo.example.com", PathRules: []PathRule{ @@ -2034,93 +1840,65 @@ func TestBuildConfiguration(t *testing.T) { SSL: &SSL{KeyPairID: "ssl_keypair_test_secret-1"}, Port: 443, }, - }, - Upstreams: []Upstream{fooUpstream}, - BackendGroups: []BackendGroup{expHTTPSHR9Groups[0], expHTTPSHR9Groups[1]}, - SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{ - "ssl_keypair_test_secret-1": { - Cert: []byte("cert-1"), - Key: []byte("privateKey-1"), - }, - }, - CertBundles: map[CertBundleID]CertBundle{ + }...) + conf.HTTPServers = []VirtualServer{} + conf.Upstreams = []Upstream{fooUpstream} + conf.BackendGroups = []BackendGroup{expHTTPSHR9Groups[0], expHTTPSHR9Groups[1]} + conf.CertBundles = map[CertBundleID]CertBundle{ "cert_bundle_test_configmap-2": []byte("cert-2"), - }, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, - }, + } + return conf + }), msg: "https listener with httproute with backend that has a backend TLS policy with binaryData attached", }, { - graph: &graph.Graph{ - GatewayClass: &graph.GatewayClass{ - Source: &v1.GatewayClass{}, - Valid: true, - }, - Gateway: &graph.Gateway{ - Source: &v1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Name: "gw", - Namespace: "ns", - }, - }, - Listeners: []*graph.Listener{ - { - Name: "listener-80-1", - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{}, - }, - }, - }, - Routes: map[graph.RouteKey]*graph.L7Route{}, - NginxProxy: nginxProxy, - }, - expConf: Configuration{ - HTTPServers: []VirtualServer{ + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + g.Gateway.Source.ObjectMeta = metav1.ObjectMeta{ + Name: "gw", + Namespace: "ns", + } + g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{ { - IsDefault: true, - Port: 80, - }, - }, - SSLServers: []VirtualServer{}, - SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{}, - CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: false, IPFamily: Dual}, - Telemetry: Telemetry{ + Name: "listener-80-1", + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{}, + }, + }...) + g.NginxProxy = nginxProxy + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.SSLServers = []VirtualServer{} + conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} + conf.Telemetry = Telemetry{ Endpoint: "my-otel.svc:4563", Interval: "5s", BatchSize: 512, BatchCount: 4, ServiceName: "ngf:ns:gw:my-svc", Ratios: []Ratio{}, - }, - }, + } + conf.BaseHTTPConfig = BaseHTTPConfig{HTTP2: false, IPFamily: Dual} + return conf + }), msg: "NginxProxy with tracing config and http2 disabled", }, { - graph: &graph.Graph{ - GatewayClass: &graph.GatewayClass{ - Source: &v1.GatewayClass{}, - Valid: true, - }, - Gateway: &graph.Gateway{ - Source: &v1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Name: "gw", - Namespace: "ns", - }, - }, - Listeners: []*graph.Listener{ - { - Name: "listener-80-1", - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{}, - }, + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + g.Gateway.Source.ObjectMeta = metav1.ObjectMeta{ + Name: "gw", + Namespace: "ns", + } + g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{ + { + Name: "listener-80-1", + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{}, }, - }, - Routes: map[graph.RouteKey]*graph.L7Route{}, - NginxProxy: &graph.NginxProxy{ + }...) + g.NginxProxy = &graph.NginxProxy{ Valid: false, Source: &ngfAPI.NginxProxy{ Spec: ngfAPI.NginxProxySpec{ @@ -2133,72 +1911,52 @@ func TestBuildConfiguration(t *testing.T) { }, }, }, - }, - }, - expConf: Configuration{ - HTTPServers: []VirtualServer{ - { - IsDefault: true, - Port: 80, - }, - }, - SSLServers: []VirtualServer{}, - SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{}, - CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, - Telemetry: Telemetry{}, - }, + } + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.SSLServers = []VirtualServer{} + conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} + return conf + }), msg: "invalid NginxProxy", }, { - graph: &graph.Graph{ - GatewayClass: &graph.GatewayClass{ - Source: &v1.GatewayClass{}, - Valid: true, - }, - Gateway: &graph.Gateway{ - Source: &v1.Gateway{}, - Listeners: []*graph.Listener{ - { - Name: "listener-80-1", - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(hrWithPolicy): l7RouteWithPolicy, - }, + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{ + { + Name: "listener-80-1", + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(hrWithPolicy): l7RouteWithPolicy, }, - { - Name: "listener-443", - Source: listener443, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(httpsHRWithPolicy): l7HTTPSRouteWithPolicy, - }, - ResolvedSecret: &secret1NsName, + }, + { + Name: "listener-443", + Source: listener443, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(httpsHRWithPolicy): l7HTTPSRouteWithPolicy, }, + ResolvedSecret: &secret1NsName, }, - Policies: []*graph.Policy{gwPolicy1, gwPolicy2}, - }, - Routes: map[graph.RouteKey]*graph.L7Route{ + }...) + g.Gateway.Policies = []*graph.Policy{gwPolicy1, gwPolicy2} + g.Routes = map[graph.RouteKey]*graph.L7Route{ graph.CreateRouteKey(hrWithPolicy): l7RouteWithPolicy, graph.CreateRouteKey(httpsHRWithPolicy): l7HTTPSRouteWithPolicy, - }, - ReferencedSecrets: map[types.NamespacedName]*graph.Secret{ + } + g.ReferencedSecrets = map[types.NamespacedName]*graph.Secret{ secret1NsName: secret1, - }, - }, - expConf: Configuration{ - SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{ - "ssl_keypair_test_secret-1": { - Cert: []byte("cert-1"), - Key: []byte("privateKey-1"), - }, - }, - CertBundles: map[CertBundleID]CertBundle{}, - HTTPServers: []VirtualServer{ + } + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.SSLServers = []VirtualServer{ { IsDefault: true, - Port: 80, + Port: 443, Additions: []Addition{ { Bytes: []byte("apple"), @@ -2218,19 +1976,20 @@ func TestBuildConfiguration(t *testing.T) { PathType: PathTypePrefix, MatchRules: []MatchRule{ { - Source: &hrWithPolicy.ObjectMeta, - BackendGroup: expHRWithPolicyGroups[0], + BackendGroup: expHTTPSHRWithPolicyGroups[0], + Source: &httpsHRWithPolicy.ObjectMeta, Additions: []Addition{ { - Bytes: []byte("lemon"), - Identifier: "LemonPolicy_default_attach-hr", + Bytes: []byte("lime"), + Identifier: "LimePolicy_default_attach-hr", }, }, }, }, }, }, - Port: 80, + SSL: &SSL{KeyPairID: "ssl_keypair_test_secret-1"}, + Port: 443, Additions: []Addition{ { Bytes: []byte("apple"), @@ -2242,11 +2001,26 @@ func TestBuildConfiguration(t *testing.T) { }, }, }, - }, - SSLServers: []VirtualServer{ + { + Hostname: wildcardHostname, + SSL: &SSL{KeyPairID: "ssl_keypair_test_secret-1"}, + Port: 443, + Additions: []Addition{ + { + Bytes: []byte("apple"), + Identifier: "ApplePolicy_default_attach-gw", + }, + { + Bytes: []byte("orange"), + Identifier: "OrangePolicy_default_attach-gw", + }, + }, + }, + } + conf.HTTPServers = []VirtualServer{ { IsDefault: true, - Port: 443, + Port: 80, Additions: []Addition{ { Bytes: []byte("apple"), @@ -2266,35 +2040,19 @@ func TestBuildConfiguration(t *testing.T) { PathType: PathTypePrefix, MatchRules: []MatchRule{ { - BackendGroup: expHTTPSHRWithPolicyGroups[0], - Source: &httpsHRWithPolicy.ObjectMeta, + Source: &hrWithPolicy.ObjectMeta, + BackendGroup: expHRWithPolicyGroups[0], Additions: []Addition{ { - Bytes: []byte("lime"), - Identifier: "LimePolicy_default_attach-hr", + Bytes: []byte("lemon"), + Identifier: "LemonPolicy_default_attach-hr", }, }, }, }, }, }, - SSL: &SSL{KeyPairID: "ssl_keypair_test_secret-1"}, - Port: 443, - Additions: []Addition{ - { - Bytes: []byte("apple"), - Identifier: "ApplePolicy_default_attach-gw", - }, - { - Bytes: []byte("orange"), - Identifier: "OrangePolicy_default_attach-gw", - }, - }, - }, - { - Hostname: wildcardHostname, - SSL: &SSL{KeyPairID: "ssl_keypair_test_secret-1"}, - Port: 443, + Port: 80, Additions: []Addition{ { Bytes: []byte("apple"), @@ -2306,94 +2064,61 @@ func TestBuildConfiguration(t *testing.T) { }, }, }, - }, - Upstreams: []Upstream{fooUpstream}, - BackendGroups: []BackendGroup{ - expHRWithPolicyGroups[0], - expHTTPSHRWithPolicyGroups[0], - }, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, - }, + } + conf.Upstreams = []Upstream{fooUpstream} + conf.BackendGroups = []BackendGroup{expHRWithPolicyGroups[0], expHTTPSHRWithPolicyGroups[0]} + return conf + }), msg: "Simple Gateway and HTTPRoute with policies attached", }, { - graph: &graph.Graph{ - GatewayClass: &graph.GatewayClass{ - Source: &v1.GatewayClass{}, - Valid: true, - }, - Gateway: &graph.Gateway{ - Source: &v1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Name: "gw", - Namespace: "ns", - }, - }, - Listeners: []*graph.Listener{ - { - Name: "listener-80-1", - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{}, - }, - }, - }, - Routes: map[graph.RouteKey]*graph.L7Route{}, - NginxProxy: nginxProxyIPv4, - }, - expConf: Configuration{ - HTTPServers: []VirtualServer{ + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + g.Gateway.Source.ObjectMeta = metav1.ObjectMeta{ + Name: "gw", + Namespace: "ns", + } + g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{ { - IsDefault: true, - Port: 80, - }, - }, - SSLServers: []VirtualServer{}, - SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{}, - CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: IPv4}, - Telemetry: Telemetry{}, - }, + Name: "listener-80-1", + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{}, + }, + }...) + g.NginxProxy = nginxProxyIPv4 + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.SSLServers = []VirtualServer{} + conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} + conf.BaseHTTPConfig = BaseHTTPConfig{HTTP2: true, IPFamily: IPv4} + return conf + }), msg: "NginxProxy with IPv4 IPFamily and no routes", }, { - graph: &graph.Graph{ - GatewayClass: &graph.GatewayClass{ - Source: &v1.GatewayClass{}, - Valid: true, - }, - Gateway: &graph.Gateway{ - Source: &v1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Name: "gw", - Namespace: "ns", - }, - }, - Listeners: []*graph.Listener{ - { - Name: "listener-80-1", - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{}, - }, - }, - }, - Routes: map[graph.RouteKey]*graph.L7Route{}, - NginxProxy: nginxProxyIPv6, - }, - expConf: Configuration{ - HTTPServers: []VirtualServer{ + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + g.Gateway.Source.ObjectMeta = metav1.ObjectMeta{ + Name: "gw", + Namespace: "ns", + } + g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{ { - IsDefault: true, - Port: 80, - }, - }, - SSLServers: []VirtualServer{}, - SSLKeyPairs: map[SSLKeyPairID]SSLKeyPair{}, - CertBundles: map[CertBundleID]CertBundle{}, - BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: IPv6}, - Telemetry: Telemetry{}, - }, + Name: "listener-80-1", + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{}, + }, + }...) + g.NginxProxy = nginxProxyIPv6 + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.SSLServers = []VirtualServer{} + conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} + conf.BaseHTTPConfig = BaseHTTPConfig{HTTP2: true, IPFamily: IPv6} + return conf + }), msg: "NginxProxy with IPv6 IPFamily and no routes", }, } @@ -3508,6 +3233,11 @@ func TestGetAllowedAddressType(t *testing.T) { ipFamily: IPv6, expected: []discoveryV1.AddressType{discoveryV1.AddressTypeIPv6}, }, + { + msg: "unknown ip family", + ipFamily: "unknown", + expected: []discoveryV1.AddressType{}, + }, } for _, tc := range test { diff --git a/internal/mode/static/state/graph/backend_refs.go b/internal/mode/static/state/graph/backend_refs.go index 7cb4b494d1..9a87dc368d 100644 --- a/internal/mode/static/state/graph/backend_refs.go +++ b/internal/mode/static/state/graph/backend_refs.go @@ -152,19 +152,18 @@ func createBackendRef( } svc, svcPort, err := getServiceAndPortFromRef(ref.BackendRef, sourceNamespace, services, refPath) - svcNsName := client.ObjectKeyFromObject(svc) if err != nil { backendRef = BackendRef{ - SvcNsName: svcNsName, - ServicePort: svcPort, - Weight: weight, - Valid: false, + Weight: weight, + Valid: false, } cond := staticConds.NewRouteBackendRefRefBackendNotFound(err.Error()) return backendRef, &cond } + svcNsName := client.ObjectKeyFromObject(svc) + if err := verifyIPFamily(npCfg, svc.Spec.IPFamilies); err != nil { backendRef = BackendRef{ SvcNsName: svcNsName, @@ -332,12 +331,16 @@ func verifyIPFamily(npCfg *NginxProxy, svcIPFamily []v1.IPFamily) error { npIPFamily := npCfg.Source.Spec.IPFamily if *npIPFamily == ngfAPI.IPv4 { if slices.Contains(svcIPFamily, v1.IPv6Protocol) { - return errors.New("service configured with IPv6 family but NginxProxy is configured with IPv4") + // capitalizing error message to match the rest of the error messages associated with a condition + return errors.New( //nolint: stylecheck + "Service configured with IPv6 family but NginxProxy is configured with IPv4") } } if *npIPFamily == ngfAPI.IPv6 { if slices.Contains(svcIPFamily, v1.IPv4Protocol) { - return errors.New("service configured with IPv4 family but NginxProxy is configured with IPv6") + // capitalizing error message to match the rest of the error messages associated with a condition + return errors.New( //nolint: stylecheck + "Service configured with IPv4 family but NginxProxy is configured with IPv6") } } diff --git a/internal/mode/static/state/graph/backend_refs_test.go b/internal/mode/static/state/graph/backend_refs_test.go index f889e1b088..21b987244c 100644 --- a/internal/mode/static/state/graph/backend_refs_test.go +++ b/internal/mode/static/state/graph/backend_refs_test.go @@ -392,7 +392,7 @@ func TestVerifyIPFamily(t *testing.T) { Valid: true, }, svcIPFamily: []v1.IPFamily{v1.IPv6Protocol}, - expErr: errors.New("service configured with IPv6 family but NginxProxy is configured with IPv4"), + expErr: errors.New("Service configured with IPv6 family but NginxProxy is configured with IPv4"), }, { name: "Invalid - IPv6 configured for NGINX, service has only IPv4", @@ -405,7 +405,7 @@ func TestVerifyIPFamily(t *testing.T) { Valid: true, }, svcIPFamily: []v1.IPFamily{v1.IPv4Protocol}, - expErr: errors.New("service configured with IPv4 family but NginxProxy is configured with IPv6"), + expErr: errors.New("Service configured with IPv4 family but NginxProxy is configured with IPv6"), }, { name: "Valid - When NginxProxy is nil", @@ -951,10 +951,8 @@ func TestCreateBackend(t *testing.T) { }), }, expectedBackend: BackendRef{ - SvcNsName: types.NamespacedName{Name: "not-exist", Namespace: "test"}, - ServicePort: v1.ServicePort{}, - Weight: 5, - Valid: false, + Weight: 5, + Valid: false, }, expectedServicePortReference: "", expectedCondition: helpers.GetPointer( @@ -982,7 +980,7 @@ func TestCreateBackend(t *testing.T) { Valid: true, }, expectedCondition: helpers.GetPointer( - staticConds.NewRouteInvalidIPFamily(`service configured with IPv4 family but NginxProxy is configured with IPv6`), + staticConds.NewRouteInvalidIPFamily(`Service configured with IPv4 family but NginxProxy is configured with IPv6`), ), name: "service IPFamily doesn't match NginxProxy IPFamily", }, diff --git a/site/content/overview/gateway-api-compatibility.md b/site/content/overview/gateway-api-compatibility.md index c106ec85af..567b3c5af0 100644 --- a/site/content/overview/gateway-api-compatibility.md +++ b/site/content/overview/gateway-api-compatibility.md @@ -179,7 +179,7 @@ See the [static-mode]({{< relref "/reference/cli-help.md#static-mode">}}) comman - `ResolvedRefs/False/RefNotPermitted` - `ResolvedRefs/False/BackendNotFound` - `ResolvedRefs/False/UnsupportedValue`: Custom reason for when one of the HTTPRoute rules has a backendRef with an unsupported value. - - `ResolvedRefs/False/InvalidIPFamily`: Custom reason for when one of the HTTPRoute rules has a backendRef that has invalid IPFamily. + - `ResolvedRefs/False/InvalidIPFamily`: Custom reason for when one of the HTTPRoute rules has a backendRef that has an invalid IPFamily. - `PartiallyInvalid/True/UnsupportedValue` --- From 9ef486828ddc9a810b25466aaa38bddcccf46648 Mon Sep 17 00:00:00 2001 From: Saloni Date: Mon, 15 Jul 2024 19:04:24 -0600 Subject: [PATCH 27/28] move service name creation out of function --- .../mode/static/state/graph/backend_refs.go | 29 ++++++------ .../static/state/graph/backend_refs_test.go | 45 +++++++++---------- 2 files changed, 37 insertions(+), 37 deletions(-) diff --git a/internal/mode/static/state/graph/backend_refs.go b/internal/mode/static/state/graph/backend_refs.go index 9a87dc368d..200dc71a08 100644 --- a/internal/mode/static/state/graph/backend_refs.go +++ b/internal/mode/static/state/graph/backend_refs.go @@ -6,10 +6,8 @@ import ( "slices" v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "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/v1alpha3" @@ -151,20 +149,25 @@ func createBackendRef( return backendRef, &cond } - svc, svcPort, err := getServiceAndPortFromRef(ref.BackendRef, sourceNamespace, services, refPath) + ns := sourceNamespace + if ref.BackendRef.Namespace != nil { + ns = string(*ref.Namespace) + } + svcNsName := types.NamespacedName{Name: string(ref.BackendRef.Name), Namespace: ns} + svcIPFamily, svcPort, err := getServiceAndPortFromRef(ref.BackendRef, sourceNamespace, services, refPath) if err != nil { backendRef = BackendRef{ - Weight: weight, - Valid: false, + Weight: weight, + Valid: false, + SvcNsName: svcNsName, + ServicePort: v1.ServicePort{}, } cond := staticConds.NewRouteBackendRefRefBackendNotFound(err.Error()) return backendRef, &cond } - svcNsName := client.ObjectKeyFromObject(svc) - - if err := verifyIPFamily(npCfg, svc.Spec.IPFamilies); err != nil { + if err := verifyIPFamily(npCfg, svcIPFamily); err != nil { backendRef = BackendRef{ SvcNsName: svcNsName, ServicePort: svcPort, @@ -299,7 +302,7 @@ func getServiceAndPortFromRef( routeNamespace string, services map[types.NamespacedName]*v1.Service, refPath *field.Path, -) (*v1.Service, v1.ServicePort, error) { +) ([]v1.IPFamily, v1.ServicePort, error) { ns := routeNamespace if ref.Namespace != nil { ns = string(*ref.Namespace) @@ -308,18 +311,16 @@ func getServiceAndPortFromRef( svcNsName := types.NamespacedName{Name: string(ref.Name), Namespace: ns} svc, ok := services[svcNsName] if !ok { - return &v1.Service{ObjectMeta: metav1.ObjectMeta{Namespace: ns, Name: string(ref.Name)}}, - v1.ServicePort{}, - field.NotFound(refPath.Child("name"), ref.Name) + return []v1.IPFamily{}, v1.ServicePort{}, field.NotFound(refPath.Child("name"), ref.Name) } // safe to dereference port here because we already validated that the port is not nil in validateBackendRef. svcPort, err := getServicePort(svc, int32(*ref.Port)) if err != nil { - return &v1.Service{ObjectMeta: metav1.ObjectMeta{Namespace: ns, Name: string(ref.Name)}}, v1.ServicePort{}, err + return []v1.IPFamily{}, v1.ServicePort{}, err } - return svc, svcPort, nil + return svc.Spec.IPFamilies, svcPort, nil } func verifyIPFamily(npCfg *NginxProxy, svcIPFamily []v1.IPFamily) error { diff --git a/internal/mode/static/state/graph/backend_refs_test.go b/internal/mode/static/state/graph/backend_refs_test.go index 21b987244c..caafae5f28 100644 --- a/internal/mode/static/state/graph/backend_refs_test.go +++ b/internal/mode/static/state/graph/backend_refs_test.go @@ -271,10 +271,6 @@ func TestGetServiceAndPortFromRef(t *testing.T) { IPFamilies: []v1.IPFamily{v1.IPv4Protocol}, }, } - svc1NsName := types.NamespacedName{ - Namespace: "test", - Name: "service1", - } svc2 := &v1.Service{ ObjectMeta: metav1.ObjectMeta{ @@ -284,19 +280,18 @@ func TestGetServiceAndPortFromRef(t *testing.T) { } tests := []struct { - expSvc *v1.Service - ref gatewayv1.BackendRef - expServiceNsName types.NamespacedName - name string - expServicePort v1.ServicePort - expErr bool + expSvc *v1.Service + ref gatewayv1.BackendRef + expSvcIPFamily []v1.IPFamily + name string + expServicePort v1.ServicePort + expErr bool }{ { - name: "normal case", - ref: getNormalRef(), - expServiceNsName: svc1NsName, - expServicePort: v1.ServicePort{Port: 80}, - expSvc: svc1, + name: "normal case", + ref: getNormalRef(), + expServicePort: v1.ServicePort{Port: 80}, + expSvcIPFamily: []v1.IPFamily{v1.IPv4Protocol}, }, { name: "service does not exist", @@ -304,14 +299,14 @@ func TestGetServiceAndPortFromRef(t *testing.T) { backend.Name = "does-not-exist" return backend }), - expErr: true, - expServiceNsName: types.NamespacedName{Name: "does-not-exist", Namespace: "test"}, - expServicePort: v1.ServicePort{}, + expErr: true, + expServicePort: v1.ServicePort{}, expSvc: &v1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: "does-not-exist", Namespace: "test", }, }, + expSvcIPFamily: []v1.IPFamily{}, }, { name: "no matching port for service and port", @@ -319,14 +314,14 @@ func TestGetServiceAndPortFromRef(t *testing.T) { backend.Port = helpers.GetPointer[gatewayv1.PortNumber](504) return backend }), - expErr: true, - expServiceNsName: svc1NsName, - expServicePort: v1.ServicePort{}, + expErr: true, + expServicePort: v1.ServicePort{}, expSvc: &v1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: "service1", Namespace: "test", }, }, + expSvcIPFamily: []v1.IPFamily{}, }, } @@ -341,11 +336,11 @@ func TestGetServiceAndPortFromRef(t *testing.T) { t.Run(test.name, func(t *testing.T) { g := NewWithT(t) - svc, servicePort, err := getServiceAndPortFromRef(test.ref, "test", services, refPath) + svcIPFamily, servicePort, err := getServiceAndPortFromRef(test.ref, "test", services, refPath) g.Expect(err != nil).To(Equal(test.expErr)) g.Expect(servicePort).To(Equal(test.expServicePort)) - g.Expect(svc).To(Equal(test.expSvc)) + g.Expect(svcIPFamily).To(Equal(test.expSvcIPFamily)) }) } } @@ -953,6 +948,10 @@ func TestCreateBackend(t *testing.T) { expectedBackend: BackendRef{ Weight: 5, Valid: false, + SvcNsName: types.NamespacedName{ + Namespace: "test", + Name: "not-exist", + }, }, expectedServicePortReference: "", expectedCondition: helpers.GetPointer( From e802e820d60afe3bdea9ffc7340756946a6dd6f6 Mon Sep 17 00:00:00 2001 From: Saloni Date: Tue, 16 Jul 2024 14:28:12 -0600 Subject: [PATCH 28/28] address comments --- .../state/dataplane/configuration_test.go | 212 ++++++++---------- .../mode/static/state/graph/backend_refs.go | 20 +- .../static/state/graph/backend_refs_test.go | 22 +- .../how-to/monitoring/troubleshooting.md | 2 +- 4 files changed, 106 insertions(+), 150 deletions(-) diff --git a/internal/mode/static/state/dataplane/configuration_test.go b/internal/mode/static/state/dataplane/configuration_test.go index 273bd2e071..83d5873ce0 100644 --- a/internal/mode/static/state/dataplane/configuration_test.go +++ b/internal/mode/static/state/dataplane/configuration_test.go @@ -36,10 +36,6 @@ func getNormalBackendRef() graph.BackendRef { } } -func getModifiedBackendRef(mod func(ref graph.BackendRef) graph.BackendRef) graph.BackendRef { - return mod(getNormalBackendRef()) -} - func getExpectedConfiguration() Configuration { return Configuration{ BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual}, @@ -183,9 +179,7 @@ func TestBuildConfiguration(t *testing.T) { fakeResolver := &resolverfakes.FakeServiceResolver{} fakeResolver.ResolveReturns(fooEndpoints, nil) - validBackendRef := getModifiedBackendRef(func(backend graph.BackendRef) graph.BackendRef { - return backend - }) + validBackendRef := getNormalBackendRef() expValidBackend := Backend{ UpstreamName: fooUpstreamName, @@ -837,14 +831,12 @@ func TestBuildConfiguration(t *testing.T) { }, { graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { - g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{ - { - Name: "invalid-listener", - Source: invalidListener, - Valid: false, - ResolvedSecret: &secret1NsName, - }, - }...) + g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{ + Name: "invalid-listener", + Source: invalidListener, + Valid: false, + ResolvedSecret: &secret1NsName, + }) g.Routes = map[graph.RouteKey]*graph.L7Route{ graph.CreateRouteKey(httpsHR1): httpsRouteHR1, graph.CreateRouteKey(httpsHR2): httpsRouteHR2, @@ -862,17 +854,15 @@ func TestBuildConfiguration(t *testing.T) { }, { graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { - g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{ - { - Name: "listener-80-1", - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(hr1): routeHR1, - graph.CreateRouteKey(hr2): routeHR2, - }, + g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{ + Name: "listener-80-1", + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(hr1): routeHR1, + graph.CreateRouteKey(hr2): routeHR2, }, - }...) + }) g.Routes = map[graph.RouteKey]*graph.L7Route{ graph.CreateRouteKey(hr1): routeHR1, graph.CreateRouteKey(hr2): routeHR2, @@ -925,16 +915,14 @@ func TestBuildConfiguration(t *testing.T) { }, { graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { - g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{ - { - Name: "listener-80-1", - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(gr): routeGR, - }, + g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{ + Name: "listener-80-1", + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(gr): routeGR, }, - }...) + }) g.Routes[graph.CreateRouteKey(gr)] = routeGR return g }), @@ -1411,16 +1399,14 @@ func TestBuildConfiguration(t *testing.T) { { graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { g.GatewayClass.Valid = false - g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{ - { - Name: "listener-80-1", - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(hr1): routeHR1, - }, + g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{ + Name: "listener-80-1", + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(hr1): routeHR1, }, - }...) + }) g.Routes = map[graph.RouteKey]*graph.L7Route{ graph.CreateRouteKey(hr1): routeHR1, } @@ -1432,16 +1418,14 @@ func TestBuildConfiguration(t *testing.T) { { graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { g.GatewayClass.Valid = false - g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{ - { - Name: "listener-80-1", - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(hr1): routeHR1, - }, + g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{ + Name: "listener-80-1", + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(hr1): routeHR1, }, - }...) + }) g.Routes = map[graph.RouteKey]*graph.L7Route{ graph.CreateRouteKey(hr1): routeHR1, } @@ -1460,16 +1444,14 @@ func TestBuildConfiguration(t *testing.T) { }, { graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { - g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{ - { - Name: "listener-80-1", - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(hr5): routeHR5, - }, + g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{ + Name: "listener-80-1", + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(hr5): routeHR5, }, - }...) + }) g.Routes = map[graph.RouteKey]*graph.L7Route{ graph.CreateRouteKey(hr5): routeHR5, } @@ -1599,16 +1581,14 @@ func TestBuildConfiguration(t *testing.T) { }, { graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { - g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{ - { - Name: "listener-80-1", - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(hr7): routeHR7, - }, + g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{ + Name: "listener-80-1", + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(hr7): routeHR7, }, - }...) + }) g.Routes = map[graph.RouteKey]*graph.L7Route{ graph.CreateRouteKey(hr7): routeHR7, } @@ -1731,17 +1711,15 @@ func TestBuildConfiguration(t *testing.T) { }, { graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { - g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{ - { - Name: "listener-443", - Source: listener443, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(httpsHR8): httpsRouteHR8, - }, - ResolvedSecret: &secret1NsName, + g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{ + Name: "listener-443", + Source: listener443, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(httpsHR8): httpsRouteHR8, }, - }...) + ResolvedSecret: &secret1NsName, + }) g.Routes = map[graph.RouteKey]*graph.L7Route{ graph.CreateRouteKey(httpsHR8): httpsRouteHR8, } @@ -1792,17 +1770,15 @@ func TestBuildConfiguration(t *testing.T) { }, { graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { - g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{ - { - Name: "listener-443", - Source: listener443, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(httpsHR9): httpsRouteHR9, - }, - ResolvedSecret: &secret1NsName, + g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{ + Name: "listener-443", + Source: listener443, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(httpsHR9): httpsRouteHR9, }, - }...) + ResolvedSecret: &secret1NsName, + }) g.Routes = map[graph.RouteKey]*graph.L7Route{ graph.CreateRouteKey(httpsHR9): httpsRouteHR9, } @@ -1857,14 +1833,12 @@ func TestBuildConfiguration(t *testing.T) { Name: "gw", Namespace: "ns", } - g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{ - { - Name: "listener-80-1", - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{}, - }, - }...) + g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{ + Name: "listener-80-1", + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{}, + }) g.NginxProxy = nginxProxy return g }), @@ -1890,14 +1864,12 @@ func TestBuildConfiguration(t *testing.T) { Name: "gw", Namespace: "ns", } - g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{ - { - Name: "listener-80-1", - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{}, - }, - }...) + g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{ + Name: "listener-80-1", + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{}, + }) g.NginxProxy = &graph.NginxProxy{ Valid: false, Source: &ngfAPI.NginxProxy{ @@ -2077,14 +2049,12 @@ func TestBuildConfiguration(t *testing.T) { Name: "gw", Namespace: "ns", } - g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{ - { - Name: "listener-80-1", - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{}, - }, - }...) + g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{ + Name: "listener-80-1", + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{}, + }) g.NginxProxy = nginxProxyIPv4 return g }), @@ -2102,14 +2072,12 @@ func TestBuildConfiguration(t *testing.T) { Name: "gw", Namespace: "ns", } - g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{ - { - Name: "listener-80-1", - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{}, - }, - }...) + g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{ + Name: "listener-80-1", + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{}, + }) g.NginxProxy = nginxProxyIPv6 return g }), diff --git a/internal/mode/static/state/graph/backend_refs.go b/internal/mode/static/state/graph/backend_refs.go index 200dc71a08..2b79569dc3 100644 --- a/internal/mode/static/state/graph/backend_refs.go +++ b/internal/mode/static/state/graph/backend_refs.go @@ -154,7 +154,7 @@ func createBackendRef( ns = string(*ref.Namespace) } svcNsName := types.NamespacedName{Name: string(ref.BackendRef.Name), Namespace: ns} - svcIPFamily, svcPort, err := getServiceAndPortFromRef(ref.BackendRef, sourceNamespace, services, refPath) + svcIPFamily, svcPort, err := getIPFamilyAndPortFromRef(ref.BackendRef, svcNsName, services, refPath) if err != nil { backendRef = BackendRef{ Weight: weight, @@ -293,22 +293,16 @@ func findBackendTLSPolicyForService( return beTLSPolicy, err } -// getServiceAndPortFromRef extracts the NamespacedName of the Service and the port from a BackendRef. +// getIPFamilyAndPortFromRef extracts the IPFamily of the Service and the port from a BackendRef. // It can return an error and an empty v1.ServicePort in two cases: // 1. The Service referenced from the BackendRef does not exist in the cluster/state. // 2. The Port on the BackendRef does not match any of the ServicePorts on the Service. -func getServiceAndPortFromRef( +func getIPFamilyAndPortFromRef( ref gatewayv1.BackendRef, - routeNamespace string, + svcNsName types.NamespacedName, services map[types.NamespacedName]*v1.Service, refPath *field.Path, ) ([]v1.IPFamily, v1.ServicePort, error) { - ns := routeNamespace - if ref.Namespace != nil { - ns = string(*ref.Namespace) - } - - svcNsName := types.NamespacedName{Name: string(ref.Name), Namespace: ns} svc, ok := services[svcNsName] if !ok { return []v1.IPFamily{}, v1.ServicePort{}, field.NotFound(refPath.Child("name"), ref.Name) @@ -334,14 +328,16 @@ func verifyIPFamily(npCfg *NginxProxy, svcIPFamily []v1.IPFamily) error { if slices.Contains(svcIPFamily, v1.IPv6Protocol) { // capitalizing error message to match the rest of the error messages associated with a condition return errors.New( //nolint: stylecheck - "Service configured with IPv6 family but NginxProxy is configured with IPv4") + "Service configured with IPv6 family but NginxProxy is configured with IPv4", + ) } } if *npIPFamily == ngfAPI.IPv6 { if slices.Contains(svcIPFamily, v1.IPv4Protocol) { // capitalizing error message to match the rest of the error messages associated with a condition return errors.New( //nolint: stylecheck - "Service configured with IPv4 family but NginxProxy is configured with IPv6") + "Service configured with IPv4 family but NginxProxy is configured with IPv6", + ) } } diff --git a/internal/mode/static/state/graph/backend_refs_test.go b/internal/mode/static/state/graph/backend_refs_test.go index caafae5f28..c260408c43 100644 --- a/internal/mode/static/state/graph/backend_refs_test.go +++ b/internal/mode/static/state/graph/backend_refs_test.go @@ -256,7 +256,7 @@ func TestValidateWeight(t *testing.T) { } } -func TestGetServiceAndPortFromRef(t *testing.T) { +func TestGetIPFamilyAndPortFromRef(t *testing.T) { svc1 := &v1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: "service1", @@ -280,8 +280,8 @@ func TestGetServiceAndPortFromRef(t *testing.T) { } tests := []struct { - expSvc *v1.Service ref gatewayv1.BackendRef + svcNsName types.NamespacedName expSvcIPFamily []v1.IPFamily name string expServicePort v1.ServicePort @@ -292,6 +292,7 @@ func TestGetServiceAndPortFromRef(t *testing.T) { ref: getNormalRef(), expServicePort: v1.ServicePort{Port: 80}, expSvcIPFamily: []v1.IPFamily{v1.IPv4Protocol}, + svcNsName: types.NamespacedName{Namespace: "test", Name: "service1"}, }, { name: "service does not exist", @@ -301,12 +302,8 @@ func TestGetServiceAndPortFromRef(t *testing.T) { }), expErr: true, expServicePort: v1.ServicePort{}, - expSvc: &v1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: "does-not-exist", Namespace: "test", - }, - }, expSvcIPFamily: []v1.IPFamily{}, + svcNsName: types.NamespacedName{Namespace: "test", Name: "does-not-exist"}, }, { name: "no matching port for service and port", @@ -316,12 +313,8 @@ func TestGetServiceAndPortFromRef(t *testing.T) { }), expErr: true, expServicePort: v1.ServicePort{}, - expSvc: &v1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: "service1", Namespace: "test", - }, - }, expSvcIPFamily: []v1.IPFamily{}, + svcNsName: types.NamespacedName{Namespace: "test", Name: "service1"}, }, } @@ -336,7 +329,7 @@ func TestGetServiceAndPortFromRef(t *testing.T) { t.Run(test.name, func(t *testing.T) { g := NewWithT(t) - svcIPFamily, servicePort, err := getServiceAndPortFromRef(test.ref, "test", services, refPath) + svcIPFamily, servicePort, err := getIPFamilyAndPortFromRef(test.ref, test.svcNsName, services, refPath) g.Expect(err != nil).To(Equal(test.expErr)) g.Expect(servicePort).To(Equal(test.expServicePort)) @@ -404,7 +397,6 @@ func TestVerifyIPFamily(t *testing.T) { }, { name: "Valid - When NginxProxy is nil", - npCfg: &NginxProxy{}, svcIPFamily: []v1.IPFamily{v1.IPv4Protocol}, }, } @@ -767,7 +759,7 @@ func TestAddBackendRefsToRulesTest(t *testing.T) { t.Run(test.name, func(t *testing.T) { g := NewWithT(t) resolver := newReferenceGrantResolver(nil) - addBackendRefsToRules(test.route, resolver, services, test.policies, &NginxProxy{}) + addBackendRefsToRules(test.route, resolver, services, test.policies, nil) var actual []BackendRef if test.route.Spec.Rules != nil { diff --git a/site/content/how-to/monitoring/troubleshooting.md b/site/content/how-to/monitoring/troubleshooting.md index a7c15ed8c6..e15bee6a1c 100644 --- a/site/content/how-to/monitoring/troubleshooting.md +++ b/site/content/how-to/monitoring/troubleshooting.md @@ -418,7 +418,7 @@ If you `describe` your HTTPRoute and see the following error: Status: True Type: Accepted Last Transition Time: 2024-07-14T23:36:37Z - Message: service configured with IPv4 family but NginxProxy is configured with IPv6 + Message: Service configured with IPv4 family but NginxProxy is configured with IPv6 Observed Generation: 1 Reason: InvalidServiceIPFamily Status: False