From 9631d542f473826abc4d0ee0518142c06dc6d913 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Thu, 7 Aug 2025 11:25:05 +0100 Subject: [PATCH 01/19] Add tests for kubernetes validation --- tests/cel/nginxproxy_test.go | 104 +++++++++++++++++++++++++++++++++++ 1 file changed, 104 insertions(+) create mode 100644 tests/cel/nginxproxy_test.go diff --git a/tests/cel/nginxproxy_test.go b/tests/cel/nginxproxy_test.go new file mode 100644 index 0000000000..df7cbac3bb --- /dev/null +++ b/tests/cel/nginxproxy_test.go @@ -0,0 +1,104 @@ +package cel + +import ( + "context" + "testing" + + . "github.com/onsi/gomega" + controllerruntime "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + + ngfAPIv1alpha2 "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha2" + "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/helpers" + "github.com/nginx/nginx-gateway-fabric/v2/tests/framework" +) + +func TestNginxProxyKubernetes(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + k8sClient, err := getKubernetesClient(t) + g.Expect(err).ToNot(HaveOccurred()) + + tests := []struct { + policySpec ngfAPIv1alpha2.NginxProxySpec + name string + wantErrors []string + }{ + { + name: "Validate NginxProxy with both Deployment and DaemonSet is invalid", + wantErrors: []string{"only one of deployment or daemonSet can be set"}, + policySpec: ngfAPIv1alpha2.NginxProxySpec{ + Kubernetes: &ngfAPIv1alpha2.KubernetesSpec{ + Deployment: &ngfAPIv1alpha2.DeploymentSpec{ + Replicas: helpers.GetPointer[int32](3), + }, + DaemonSet: &ngfAPIv1alpha2.DaemonSetSpec{}, + }, + }, + }, + { + name: "Validate NginxProxy with Deployment only is valid", + policySpec: ngfAPIv1alpha2.NginxProxySpec{ + Kubernetes: &ngfAPIv1alpha2.KubernetesSpec{ + Deployment: &ngfAPIv1alpha2.DeploymentSpec{ + Replicas: helpers.GetPointer[int32](3), + }, + }, + }, + }, + { + name: "Validate NginxProxy with DaemonSet only is valid", + policySpec: ngfAPIv1alpha2.NginxProxySpec{ + Kubernetes: &ngfAPIv1alpha2.KubernetesSpec{ + DaemonSet: &ngfAPIv1alpha2.DaemonSetSpec{}, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + validateNginxProxy(t, tt, g, k8sClient) + }) + } +} + +func validateNginxProxy(t *testing.T, tt struct { + policySpec ngfAPIv1alpha2.NginxProxySpec + name string + wantErrors []string +}, g *WithT, k8sClient client.Client, +) { + t.Helper() + + policySpec := tt.policySpec + policyName := uniqueResourceName(testPolicyName) + + nginxProxy := &ngfAPIv1alpha2.NginxProxy{ + ObjectMeta: controllerruntime.ObjectMeta{ + Name: policyName, + Namespace: defaultNamespace, + }, + Spec: policySpec, + } + timeoutConfig := framework.DefaultTimeoutConfig() + ctx, cancel := context.WithTimeout(context.Background(), timeoutConfig.KubernetesClientTimeout) + err := k8sClient.Create(ctx, nginxProxy) + defer cancel() + + // Clean up after test + defer func() { + _ = k8sClient.Delete(context.Background(), nginxProxy) + }() + + if len(tt.wantErrors) == 0 { + g.Expect(err).ToNot(HaveOccurred()) + } else { + g.Expect(err).To(HaveOccurred()) + for _, wantError := range tt.wantErrors { + g.Expect(err.Error()).To(ContainSubstring(wantError), "Expected error '%s' not found in: %s", wantError, err.Error()) + } + } +} From 8455a14c78f636e75eac9cfff85ba5aa8b5b94f8 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Thu, 7 Aug 2025 11:38:33 +0100 Subject: [PATCH 02/19] Add tests for rewriteClientIP --- tests/cel/nginxproxy_test.go | 45 ++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/tests/cel/nginxproxy_test.go b/tests/cel/nginxproxy_test.go index df7cbac3bb..e71edb67f3 100644 --- a/tests/cel/nginxproxy_test.go +++ b/tests/cel/nginxproxy_test.go @@ -65,6 +65,51 @@ func TestNginxProxyKubernetes(t *testing.T) { } } +func TestNginxProxyRewriteClientIP(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + k8sClient, err := getKubernetesClient(t) + g.Expect(err).ToNot(HaveOccurred()) + + tests := []struct { + policySpec ngfAPIv1alpha2.NginxProxySpec + name string + wantErrors []string + }{ + { + name: "Validate NginxProxy is invalid when trustedAddresses is not set and mode is set", + wantErrors: []string{"if mode is set, trustedAddresses is a required field"}, + policySpec: ngfAPIv1alpha2.NginxProxySpec{ + RewriteClientIP: &ngfAPIv1alpha2.RewriteClientIP{ + Mode: helpers.GetPointer[ngfAPIv1alpha2.RewriteClientIPModeType]("XForwardedFor"), + }, + }, + }, + { + name: "Validate NginxProxy is valid when both mode and trustedAddresses are set", + policySpec: ngfAPIv1alpha2.NginxProxySpec{ + RewriteClientIP: &ngfAPIv1alpha2.RewriteClientIP{ + Mode: helpers.GetPointer[ngfAPIv1alpha2.RewriteClientIPModeType]("XForwardedFor"), + TrustedAddresses: []ngfAPIv1alpha2.RewriteClientIPAddress{ + { + Type: ngfAPIv1alpha2.RewriteClientIPAddressType("CIDR"), + Value: "10.0.0.0/8", + }, + }, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + validateNginxProxy(t, tt, g, k8sClient) + }) + } +} + func validateNginxProxy(t *testing.T, tt struct { policySpec ngfAPIv1alpha2.NginxProxySpec name string From 8404b7d899d7bb96271d1761fd29a7a71ec04770 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Thu, 7 Aug 2025 12:20:15 +0100 Subject: [PATCH 03/19] Move validation function to common.go --- tests/cel/clientsettingspolicy_test.go | 75 +++++++++++--------------- tests/cel/common.go | 26 +++++++++ tests/cel/nginxproxy_test.go | 65 ++++++++-------------- 3 files changed, 78 insertions(+), 88 deletions(-) diff --git a/tests/cel/clientsettingspolicy_test.go b/tests/cel/clientsettingspolicy_test.go index 7e7ea5c4b1..73ec2f0dfb 100644 --- a/tests/cel/clientsettingspolicy_test.go +++ b/tests/cel/clientsettingspolicy_test.go @@ -1,17 +1,14 @@ package cel import ( - "context" "testing" . "github.com/onsi/gomega" controllerruntime "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" ngfAPIv1alpha1 "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1" "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/helpers" - "github.com/nginx/nginx-gateway-fabric/v2/tests/framework" ) func TestClientSettingsPoliciesTargetRefKind(t *testing.T) { @@ -78,7 +75,16 @@ func TestClientSettingsPoliciesTargetRefKind(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - validateClientSettingsPolicy(t, tt, g, k8sClient) + policySpec := tt.policySpec + policySpec.TargetRef.Name = gatewayv1alpha2.ObjectName(uniqueResourceName(testTargetRefName)) + clientSettingsPolicy := &ngfAPIv1alpha1.ClientSettingsPolicy{ + ObjectMeta: controllerruntime.ObjectMeta{ + Name: uniqueResourceName(testPolicyName), + Namespace: defaultNamespace, + }, + Spec: policySpec, + } + validateCrd(t, tt.wantErrors, g, clientSettingsPolicy, k8sClient) }) } } @@ -129,7 +135,16 @@ func TestClientSettingsPoliciesTargetRefGroup(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - validateClientSettingsPolicy(t, tt, g, k8sClient) + policySpec := tt.policySpec + policySpec.TargetRef.Name = gatewayv1alpha2.ObjectName(uniqueResourceName(testTargetRefName)) + clientSettingsPolicy := &ngfAPIv1alpha1.ClientSettingsPolicy{ + ObjectMeta: controllerruntime.ObjectMeta{ + Name: uniqueResourceName(testPolicyName), + Namespace: defaultNamespace, + }, + Spec: policySpec, + } + validateCrd(t, tt.wantErrors, g, clientSettingsPolicy, k8sClient) }) } } @@ -191,46 +206,16 @@ func TestClientSettingsPoliciesKeepAliveTimeout(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - validateClientSettingsPolicy(t, tt, g, k8sClient) + policySpec := tt.policySpec + policySpec.TargetRef.Name = gatewayv1alpha2.ObjectName(uniqueResourceName(testTargetRefName)) + clientSettingsPolicy := &ngfAPIv1alpha1.ClientSettingsPolicy{ + ObjectMeta: controllerruntime.ObjectMeta{ + Name: uniqueResourceName(testPolicyName), + Namespace: defaultNamespace, + }, + Spec: policySpec, + } + validateCrd(t, tt.wantErrors, g, clientSettingsPolicy, k8sClient) }) } } - -func validateClientSettingsPolicy(t *testing.T, tt struct { - policySpec ngfAPIv1alpha1.ClientSettingsPolicySpec - name string - wantErrors []string -}, g *WithT, k8sClient client.Client, -) { - t.Helper() - - policySpec := tt.policySpec - policySpec.TargetRef.Name = gatewayv1alpha2.ObjectName(uniqueResourceName(testTargetRefName)) - policyName := uniqueResourceName(testPolicyName) - - clientSettingsPolicy := &ngfAPIv1alpha1.ClientSettingsPolicy{ - ObjectMeta: controllerruntime.ObjectMeta{ - Name: policyName, - Namespace: defaultNamespace, - }, - Spec: policySpec, - } - timeoutConfig := framework.DefaultTimeoutConfig() - ctx, cancel := context.WithTimeout(context.Background(), timeoutConfig.KubernetesClientTimeout) - err := k8sClient.Create(ctx, clientSettingsPolicy) - defer cancel() - - // Clean up after test - defer func() { - _ = k8sClient.Delete(context.Background(), clientSettingsPolicy) - }() - - if len(tt.wantErrors) == 0 { - g.Expect(err).ToNot(HaveOccurred()) - } else { - g.Expect(err).To(HaveOccurred()) - for _, wantError := range tt.wantErrors { - g.Expect(err.Error()).To(ContainSubstring(wantError), "Expected error '%s' not found in: %s", wantError, err.Error()) - } - } -} diff --git a/tests/cel/common.go b/tests/cel/common.go index 2f25b1f7af..878ddd2dca 100644 --- a/tests/cel/common.go +++ b/tests/cel/common.go @@ -1,16 +1,19 @@ package cel import ( + "context" "crypto/rand" "fmt" "testing" + . "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/runtime" controllerruntime "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" ngfAPIv1alpha1 "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1" ngfAPIv1alpha2 "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha2" + "github.com/nginx/nginx-gateway-fabric/v2/tests/framework" ) const ( @@ -77,3 +80,26 @@ func randomPrimeNumber() int64 { func uniqueResourceName(name string) string { return fmt.Sprintf("%s-%d", name, randomPrimeNumber()) } + +func validateCrd(t *testing.T, wantErrors []string, g *WithT, crd client.Object, k8sClient client.Client) { + t.Helper() + + timeoutConfig := framework.DefaultTimeoutConfig() + ctx, cancel := context.WithTimeout(context.Background(), timeoutConfig.KubernetesClientTimeout) + err := k8sClient.Create(ctx, crd) + defer cancel() + + // Clean up after test + defer func() { + _ = k8sClient.Delete(context.Background(), crd) + }() + + if len(wantErrors) == 0 { + g.Expect(err).ToNot(HaveOccurred()) + } else { + g.Expect(err).To(HaveOccurred()) + for _, wantError := range wantErrors { + g.Expect(err.Error()).To(ContainSubstring(wantError), "Expected error '%s' not found in: %s", wantError, err.Error()) + } + } +} diff --git a/tests/cel/nginxproxy_test.go b/tests/cel/nginxproxy_test.go index e71edb67f3..fc154fcd89 100644 --- a/tests/cel/nginxproxy_test.go +++ b/tests/cel/nginxproxy_test.go @@ -1,16 +1,13 @@ package cel import ( - "context" "testing" . "github.com/onsi/gomega" controllerruntime "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" ngfAPIv1alpha2 "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha2" "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/helpers" - "github.com/nginx/nginx-gateway-fabric/v2/tests/framework" ) func TestNginxProxyKubernetes(t *testing.T) { @@ -60,7 +57,17 @@ func TestNginxProxyKubernetes(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - validateNginxProxy(t, tt, g, k8sClient) + policySpec := tt.policySpec + policyName := uniqueResourceName(testPolicyName) + + nginxProxy := &ngfAPIv1alpha2.NginxProxy{ + ObjectMeta: controllerruntime.ObjectMeta{ + Name: policyName, + Namespace: defaultNamespace, + }, + Spec: policySpec, + } + validateCrd(t, tt.wantErrors, g, nginxProxy, k8sClient) }) } } @@ -105,45 +112,17 @@ func TestNginxProxyRewriteClientIP(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - validateNginxProxy(t, tt, g, k8sClient) - }) - } -} - -func validateNginxProxy(t *testing.T, tt struct { - policySpec ngfAPIv1alpha2.NginxProxySpec - name string - wantErrors []string -}, g *WithT, k8sClient client.Client, -) { - t.Helper() - - policySpec := tt.policySpec - policyName := uniqueResourceName(testPolicyName) + policySpec := tt.policySpec + policyName := uniqueResourceName(testPolicyName) - nginxProxy := &ngfAPIv1alpha2.NginxProxy{ - ObjectMeta: controllerruntime.ObjectMeta{ - Name: policyName, - Namespace: defaultNamespace, - }, - Spec: policySpec, - } - timeoutConfig := framework.DefaultTimeoutConfig() - ctx, cancel := context.WithTimeout(context.Background(), timeoutConfig.KubernetesClientTimeout) - err := k8sClient.Create(ctx, nginxProxy) - defer cancel() - - // Clean up after test - defer func() { - _ = k8sClient.Delete(context.Background(), nginxProxy) - }() - - if len(tt.wantErrors) == 0 { - g.Expect(err).ToNot(HaveOccurred()) - } else { - g.Expect(err).To(HaveOccurred()) - for _, wantError := range tt.wantErrors { - g.Expect(err.Error()).To(ContainSubstring(wantError), "Expected error '%s' not found in: %s", wantError, err.Error()) - } + nginxProxy := &ngfAPIv1alpha2.NginxProxy{ + ObjectMeta: controllerruntime.ObjectMeta{ + Name: policyName, + Namespace: defaultNamespace, + }, + Spec: policySpec, + } + validateCrd(t, tt.wantErrors, g, nginxProxy, k8sClient) + }) } } From 0ba4b7c185df40153d050e5960d2f4ee7c49d6e1 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Thu, 7 Aug 2025 14:03:39 +0100 Subject: [PATCH 04/19] Add comment to `validateCrd` function --- tests/cel/common.go | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/cel/common.go b/tests/cel/common.go index 878ddd2dca..4fa43534e7 100644 --- a/tests/cel/common.go +++ b/tests/cel/common.go @@ -81,6 +81,7 @@ func uniqueResourceName(name string) string { return fmt.Sprintf("%s-%d", name, randomPrimeNumber()) } +// validateCrd creates a k8s resource and validates it against the expected errors. func validateCrd(t *testing.T, wantErrors []string, g *WithT, crd client.Object, k8sClient client.Client) { t.Helper() From b1da49813e3881e3f15db2bdf4a74817b7557c15 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Mon, 11 Aug 2025 08:00:01 +0100 Subject: [PATCH 05/19] Move expected error strings to common.go --- tests/cel/common.go | 8 +++++--- tests/cel/nginxproxy_test.go | 14 +++++--------- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/tests/cel/common.go b/tests/cel/common.go index 4fa43534e7..838cc7d2dd 100644 --- a/tests/cel/common.go +++ b/tests/cel/common.go @@ -31,9 +31,11 @@ const ( ) const ( - expectedTargetRefKindError = `TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute` - expectedTargetRefGroupError = `TargetRef Group must be gateway.networking.k8s.io.` - expectedHeaderWithoutServerError = `header can only be specified if server is specified` + expectedTargetRefKindError = `TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute` + expectedTargetRefGroupError = `TargetRef Group must be gateway.networking.k8s.io.` + expectedHeaderWithoutServerError = `header can only be specified if server is specified` + expectedOneOfDeploymentOrDaemonSetError = `only one of deployment or daemonSet can be set` + expectedIfModeSetTrustedAddressesError = `if mode is set, trustedAddresses is a required field` ) const ( diff --git a/tests/cel/nginxproxy_test.go b/tests/cel/nginxproxy_test.go index fc154fcd89..433a6c591d 100644 --- a/tests/cel/nginxproxy_test.go +++ b/tests/cel/nginxproxy_test.go @@ -24,13 +24,11 @@ func TestNginxProxyKubernetes(t *testing.T) { }{ { name: "Validate NginxProxy with both Deployment and DaemonSet is invalid", - wantErrors: []string{"only one of deployment or daemonSet can be set"}, + wantErrors: []string{expectedOneOfDeploymentOrDaemonSetError}, policySpec: ngfAPIv1alpha2.NginxProxySpec{ Kubernetes: &ngfAPIv1alpha2.KubernetesSpec{ - Deployment: &ngfAPIv1alpha2.DeploymentSpec{ - Replicas: helpers.GetPointer[int32](3), - }, - DaemonSet: &ngfAPIv1alpha2.DaemonSetSpec{}, + Deployment: &ngfAPIv1alpha2.DeploymentSpec{}, + DaemonSet: &ngfAPIv1alpha2.DaemonSetSpec{}, }, }, }, @@ -38,9 +36,7 @@ func TestNginxProxyKubernetes(t *testing.T) { name: "Validate NginxProxy with Deployment only is valid", policySpec: ngfAPIv1alpha2.NginxProxySpec{ Kubernetes: &ngfAPIv1alpha2.KubernetesSpec{ - Deployment: &ngfAPIv1alpha2.DeploymentSpec{ - Replicas: helpers.GetPointer[int32](3), - }, + Deployment: &ngfAPIv1alpha2.DeploymentSpec{}, }, }, }, @@ -86,7 +82,7 @@ func TestNginxProxyRewriteClientIP(t *testing.T) { }{ { name: "Validate NginxProxy is invalid when trustedAddresses is not set and mode is set", - wantErrors: []string{"if mode is set, trustedAddresses is a required field"}, + wantErrors: []string{expectedIfModeSetTrustedAddressesError}, policySpec: ngfAPIv1alpha2.NginxProxySpec{ RewriteClientIP: &ngfAPIv1alpha2.RewriteClientIP{ Mode: helpers.GetPointer[ngfAPIv1alpha2.RewriteClientIPModeType]("XForwardedFor"), From 80e121ac4e25b0d3e389482aac9e1c3d0a33eb8c Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Mon, 11 Aug 2025 08:11:39 +0100 Subject: [PATCH 06/19] Add tests for autoscaling validation --- tests/cel/common.go | 1 + tests/cel/nginxproxy_test.go | 93 +++++++++++++++++++++++++++++++----- 2 files changed, 83 insertions(+), 11 deletions(-) diff --git a/tests/cel/common.go b/tests/cel/common.go index 838cc7d2dd..d9f33ca064 100644 --- a/tests/cel/common.go +++ b/tests/cel/common.go @@ -36,6 +36,7 @@ const ( expectedHeaderWithoutServerError = `header can only be specified if server is specified` expectedOneOfDeploymentOrDaemonSetError = `only one of deployment or daemonSet can be set` expectedIfModeSetTrustedAddressesError = `if mode is set, trustedAddresses is a required field` + expectedMinReplicasLessThanOrEqualError = `minReplicas must be less than or equal to maxReplicas` ) const ( diff --git a/tests/cel/nginxproxy_test.go b/tests/cel/nginxproxy_test.go index 433a6c591d..45515980b7 100644 --- a/tests/cel/nginxproxy_test.go +++ b/tests/cel/nginxproxy_test.go @@ -18,14 +18,14 @@ func TestNginxProxyKubernetes(t *testing.T) { g.Expect(err).ToNot(HaveOccurred()) tests := []struct { - policySpec ngfAPIv1alpha2.NginxProxySpec + spec ngfAPIv1alpha2.NginxProxySpec name string wantErrors []string }{ { name: "Validate NginxProxy with both Deployment and DaemonSet is invalid", wantErrors: []string{expectedOneOfDeploymentOrDaemonSetError}, - policySpec: ngfAPIv1alpha2.NginxProxySpec{ + spec: ngfAPIv1alpha2.NginxProxySpec{ Kubernetes: &ngfAPIv1alpha2.KubernetesSpec{ Deployment: &ngfAPIv1alpha2.DeploymentSpec{}, DaemonSet: &ngfAPIv1alpha2.DaemonSetSpec{}, @@ -34,7 +34,7 @@ func TestNginxProxyKubernetes(t *testing.T) { }, { name: "Validate NginxProxy with Deployment only is valid", - policySpec: ngfAPIv1alpha2.NginxProxySpec{ + spec: ngfAPIv1alpha2.NginxProxySpec{ Kubernetes: &ngfAPIv1alpha2.KubernetesSpec{ Deployment: &ngfAPIv1alpha2.DeploymentSpec{}, }, @@ -42,7 +42,7 @@ func TestNginxProxyKubernetes(t *testing.T) { }, { name: "Validate NginxProxy with DaemonSet only is valid", - policySpec: ngfAPIv1alpha2.NginxProxySpec{ + spec: ngfAPIv1alpha2.NginxProxySpec{ Kubernetes: &ngfAPIv1alpha2.KubernetesSpec{ DaemonSet: &ngfAPIv1alpha2.DaemonSetSpec{}, }, @@ -53,7 +53,7 @@ func TestNginxProxyKubernetes(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - policySpec := tt.policySpec + spec := tt.spec policyName := uniqueResourceName(testPolicyName) nginxProxy := &ngfAPIv1alpha2.NginxProxy{ @@ -61,7 +61,7 @@ func TestNginxProxyKubernetes(t *testing.T) { Name: policyName, Namespace: defaultNamespace, }, - Spec: policySpec, + Spec: spec, } validateCrd(t, tt.wantErrors, g, nginxProxy, k8sClient) }) @@ -76,14 +76,14 @@ func TestNginxProxyRewriteClientIP(t *testing.T) { g.Expect(err).ToNot(HaveOccurred()) tests := []struct { - policySpec ngfAPIv1alpha2.NginxProxySpec + spec ngfAPIv1alpha2.NginxProxySpec name string wantErrors []string }{ { name: "Validate NginxProxy is invalid when trustedAddresses is not set and mode is set", wantErrors: []string{expectedIfModeSetTrustedAddressesError}, - policySpec: ngfAPIv1alpha2.NginxProxySpec{ + spec: ngfAPIv1alpha2.NginxProxySpec{ RewriteClientIP: &ngfAPIv1alpha2.RewriteClientIP{ Mode: helpers.GetPointer[ngfAPIv1alpha2.RewriteClientIPModeType]("XForwardedFor"), }, @@ -91,7 +91,7 @@ func TestNginxProxyRewriteClientIP(t *testing.T) { }, { name: "Validate NginxProxy is valid when both mode and trustedAddresses are set", - policySpec: ngfAPIv1alpha2.NginxProxySpec{ + spec: ngfAPIv1alpha2.NginxProxySpec{ RewriteClientIP: &ngfAPIv1alpha2.RewriteClientIP{ Mode: helpers.GetPointer[ngfAPIv1alpha2.RewriteClientIPModeType]("XForwardedFor"), TrustedAddresses: []ngfAPIv1alpha2.RewriteClientIPAddress{ @@ -108,7 +108,7 @@ func TestNginxProxyRewriteClientIP(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - policySpec := tt.policySpec + spec := tt.spec policyName := uniqueResourceName(testPolicyName) nginxProxy := &ngfAPIv1alpha2.NginxProxy{ @@ -116,7 +116,78 @@ func TestNginxProxyRewriteClientIP(t *testing.T) { Name: policyName, Namespace: defaultNamespace, }, - Spec: policySpec, + Spec: spec, + } + validateCrd(t, tt.wantErrors, g, nginxProxy, k8sClient) + }) + } +} + +func TestNginxProxyAutoscaling(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + k8sClient, err := getKubernetesClient(t) + g.Expect(err).ToNot(HaveOccurred()) + + tests := []struct { + spec ngfAPIv1alpha2.NginxProxySpec + name string + wantErrors []string + }{ + { + name: "Validate NginxProxy is invalid when MinReplicas not less than, or equal to MaxReplicas", + wantErrors: []string{expectedMinReplicasLessThanOrEqualError}, + spec: ngfAPIv1alpha2.NginxProxySpec{ + Kubernetes: &ngfAPIv1alpha2.KubernetesSpec{ + Deployment: &ngfAPIv1alpha2.DeploymentSpec{ + Autoscaling: &ngfAPIv1alpha2.AutoscalingSpec{ + MinReplicas: helpers.GetPointer[int32](10), + MaxReplicas: 5, + }, + }, + }, + }, + }, + { + name: "Validate NginxProxy is valid when MinReplicas is less than MaxReplicas", + spec: ngfAPIv1alpha2.NginxProxySpec{ + Kubernetes: &ngfAPIv1alpha2.KubernetesSpec{ + Deployment: &ngfAPIv1alpha2.DeploymentSpec{ + Autoscaling: &ngfAPIv1alpha2.AutoscalingSpec{ + MinReplicas: helpers.GetPointer[int32](1), + MaxReplicas: 5, + }, + }, + }, + }, + }, + { + name: "Validate NginxProxy is valid when MinReplicas is equal to MaxReplicas", + spec: ngfAPIv1alpha2.NginxProxySpec{ + Kubernetes: &ngfAPIv1alpha2.KubernetesSpec{ + Deployment: &ngfAPIv1alpha2.DeploymentSpec{ + Autoscaling: &ngfAPIv1alpha2.AutoscalingSpec{ + MinReplicas: helpers.GetPointer[int32](5), + MaxReplicas: 5, + }, + }, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + spec := tt.spec + policyName := uniqueResourceName(testPolicyName) + + nginxProxy := &ngfAPIv1alpha2.NginxProxy{ + ObjectMeta: controllerruntime.ObjectMeta{ + Name: policyName, + Namespace: defaultNamespace, + }, + Spec: spec, } validateCrd(t, tt.wantErrors, g, nginxProxy, k8sClient) }) From 9bbe4cd8f3f5e571c7752b270de33dbaca66c5d9 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Mon, 11 Aug 2025 10:36:13 +0100 Subject: [PATCH 07/19] Refactor tests --- tests/cel/clientsettingspolicy_test.go | 74 +++++++++++--------------- tests/cel/common.go | 31 +++++------ tests/cel/nginxproxy_test.go | 28 ++++------ 3 files changed, 57 insertions(+), 76 deletions(-) diff --git a/tests/cel/clientsettingspolicy_test.go b/tests/cel/clientsettingspolicy_test.go index 73ec2f0dfb..d7de613416 100644 --- a/tests/cel/clientsettingspolicy_test.go +++ b/tests/cel/clientsettingspolicy_test.go @@ -3,7 +3,6 @@ package cel import ( "testing" - . "github.com/onsi/gomega" controllerruntime "sigs.k8s.io/controller-runtime" gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" @@ -13,19 +12,16 @@ import ( func TestClientSettingsPoliciesTargetRefKind(t *testing.T) { t.Parallel() - g := NewWithT(t) - - k8sClient, err := getKubernetesClient(t) - g.Expect(err).ToNot(HaveOccurred()) + k8sClient := getKubernetesClient(t) tests := []struct { - policySpec ngfAPIv1alpha1.ClientSettingsPolicySpec + spec ngfAPIv1alpha1.ClientSettingsPolicySpec name string wantErrors []string }{ { name: "Validate TargetRef of kind Gateway is allowed", - policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ + spec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ Kind: gatewayKind, Group: gatewayGroup, @@ -34,7 +30,7 @@ func TestClientSettingsPoliciesTargetRefKind(t *testing.T) { }, { name: "Validate TargetRef of kind HTTPRoute is allowed", - policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ + spec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ Kind: httpRouteKind, Group: gatewayGroup, @@ -43,7 +39,7 @@ func TestClientSettingsPoliciesTargetRefKind(t *testing.T) { }, { name: "Validate TargetRef of kind GRPCRoute is allowed", - policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ + spec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ Kind: grpcRouteKind, Group: gatewayGroup, @@ -53,7 +49,7 @@ func TestClientSettingsPoliciesTargetRefKind(t *testing.T) { { name: "Validate Invalid TargetRef Kind is not allowed", wantErrors: []string{expectedTargetRefKindError}, - policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ + spec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ Kind: invalidKind, Group: gatewayGroup, @@ -63,7 +59,7 @@ func TestClientSettingsPoliciesTargetRefKind(t *testing.T) { { name: "Validate TCPRoute TargetRef Kind is not allowed", wantErrors: []string{expectedTargetRefKindError}, - policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ + spec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ Kind: tcpRouteKind, Group: gatewayGroup, @@ -75,35 +71,32 @@ func TestClientSettingsPoliciesTargetRefKind(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - policySpec := tt.policySpec - policySpec.TargetRef.Name = gatewayv1alpha2.ObjectName(uniqueResourceName(testTargetRefName)) + spec := tt.spec + spec.TargetRef.Name = gatewayv1alpha2.ObjectName(uniqueResourceName(testTargetRefName)) clientSettingsPolicy := &ngfAPIv1alpha1.ClientSettingsPolicy{ ObjectMeta: controllerruntime.ObjectMeta{ - Name: uniqueResourceName(testPolicyName), + Name: uniqueResourceName(testResourceName), Namespace: defaultNamespace, }, - Spec: policySpec, + Spec: spec, } - validateCrd(t, tt.wantErrors, g, clientSettingsPolicy, k8sClient) + validateCrd(t, tt.wantErrors, clientSettingsPolicy, k8sClient) }) } } func TestClientSettingsPoliciesTargetRefGroup(t *testing.T) { t.Parallel() - g := NewWithT(t) - - k8sClient, err := getKubernetesClient(t) - g.Expect(err).ToNot(HaveOccurred()) + k8sClient := getKubernetesClient(t) tests := []struct { - policySpec ngfAPIv1alpha1.ClientSettingsPolicySpec + spec ngfAPIv1alpha1.ClientSettingsPolicySpec name string wantErrors []string }{ { name: "Validate gateway.networking.k8s.io TargetRef Group is allowed", - policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ + spec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ Kind: gatewayKind, Group: gatewayGroup, @@ -113,7 +106,7 @@ func TestClientSettingsPoliciesTargetRefGroup(t *testing.T) { { name: "Validate invalid.networking.k8s.io TargetRef Group is not allowed", wantErrors: []string{expectedTargetRefGroupError}, - policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ + spec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ Kind: gatewayKind, Group: invalidGroup, @@ -123,7 +116,7 @@ func TestClientSettingsPoliciesTargetRefGroup(t *testing.T) { { name: "Validate discovery.k8s.io/v1 TargetRef Group is not allowed", wantErrors: []string{expectedTargetRefGroupError}, - policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ + spec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ Kind: gatewayKind, Group: discoveryGroup, @@ -135,35 +128,32 @@ func TestClientSettingsPoliciesTargetRefGroup(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - policySpec := tt.policySpec - policySpec.TargetRef.Name = gatewayv1alpha2.ObjectName(uniqueResourceName(testTargetRefName)) + spec := tt.spec + spec.TargetRef.Name = gatewayv1alpha2.ObjectName(uniqueResourceName(testTargetRefName)) clientSettingsPolicy := &ngfAPIv1alpha1.ClientSettingsPolicy{ ObjectMeta: controllerruntime.ObjectMeta{ - Name: uniqueResourceName(testPolicyName), + Name: uniqueResourceName(testResourceName), Namespace: defaultNamespace, }, - Spec: policySpec, + Spec: spec, } - validateCrd(t, tt.wantErrors, g, clientSettingsPolicy, k8sClient) + validateCrd(t, tt.wantErrors, clientSettingsPolicy, k8sClient) }) } } func TestClientSettingsPoliciesKeepAliveTimeout(t *testing.T) { t.Parallel() - g := NewWithT(t) - - k8sClient, err := getKubernetesClient(t) - g.Expect(err).ToNot(HaveOccurred()) + k8sClient := getKubernetesClient(t) tests := []struct { - policySpec ngfAPIv1alpha1.ClientSettingsPolicySpec + spec ngfAPIv1alpha1.ClientSettingsPolicySpec name string wantErrors []string }{ { name: "Validate KeepAliveTimeout is not set", - policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ + spec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ Kind: gatewayKind, Group: gatewayGroup, @@ -173,7 +163,7 @@ func TestClientSettingsPoliciesKeepAliveTimeout(t *testing.T) { }, { name: "Validate KeepAlive is set", - policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ + spec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ Kind: gatewayKind, Group: gatewayGroup, @@ -189,7 +179,7 @@ func TestClientSettingsPoliciesKeepAliveTimeout(t *testing.T) { { name: "Validate Header cannot be set without Server", wantErrors: []string{expectedHeaderWithoutServerError}, - policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ + spec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ Kind: gatewayKind, Group: gatewayGroup, @@ -206,16 +196,16 @@ func TestClientSettingsPoliciesKeepAliveTimeout(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - policySpec := tt.policySpec - policySpec.TargetRef.Name = gatewayv1alpha2.ObjectName(uniqueResourceName(testTargetRefName)) + spec := tt.spec + spec.TargetRef.Name = gatewayv1alpha2.ObjectName(uniqueResourceName(testTargetRefName)) clientSettingsPolicy := &ngfAPIv1alpha1.ClientSettingsPolicy{ ObjectMeta: controllerruntime.ObjectMeta{ - Name: uniqueResourceName(testPolicyName), + Name: uniqueResourceName(testResourceName), Namespace: defaultNamespace, }, - Spec: policySpec, + Spec: spec, } - validateCrd(t, tt.wantErrors, g, clientSettingsPolicy, k8sClient) + validateCrd(t, tt.wantErrors, clientSettingsPolicy, k8sClient) }) } } diff --git a/tests/cel/common.go b/tests/cel/common.go index d9f33ca064..c8effe0f06 100644 --- a/tests/cel/common.go +++ b/tests/cel/common.go @@ -44,29 +44,30 @@ const ( ) const ( - testPolicyName = "test-policy" + testResourceName = "test-policy" testTargetRefName = "test-targetRef" ) // getKubernetesClient returns a client connected to a real Kubernetes cluster. -func getKubernetesClient(t *testing.T) (k8sClient client.Client, err error) { +func getKubernetesClient(t *testing.T) (k8sClient client.Client) { t.Helper() + g := NewWithT(t) // Use controller-runtime to get cluster connection k8sConfig, err := controllerruntime.GetConfig() - if err != nil { - return nil, err - } + g.Expect(err).ToNot(HaveOccurred()) // Set up scheme with NGF types scheme := runtime.NewScheme() - if err = ngfAPIv1alpha1.AddToScheme(scheme); err != nil { - return nil, err - } - if err = ngfAPIv1alpha2.AddToScheme(scheme); err != nil { - return nil, err - } - // Create a new client with the scheme and return it - return client.New(k8sConfig, client.Options{Scheme: scheme}) + err = ngfAPIv1alpha1.AddToScheme(scheme) + g.Expect(err).ToNot(HaveOccurred()) + + err = ngfAPIv1alpha2.AddToScheme(scheme) + g.Expect(err).ToNot(HaveOccurred()) + + k8sClient, err = client.New(k8sConfig, client.Options{Scheme: scheme}) + g.Expect(err).ToNot(HaveOccurred()) + + return k8sClient } // randomPrimeNumber generates a random prime number of 64 bits. @@ -85,9 +86,9 @@ func uniqueResourceName(name string) string { } // validateCrd creates a k8s resource and validates it against the expected errors. -func validateCrd(t *testing.T, wantErrors []string, g *WithT, crd client.Object, k8sClient client.Client) { +func validateCrd(t *testing.T, wantErrors []string, crd client.Object, k8sClient client.Client) { t.Helper() - + g := NewWithT(t) timeoutConfig := framework.DefaultTimeoutConfig() ctx, cancel := context.WithTimeout(context.Background(), timeoutConfig.KubernetesClientTimeout) err := k8sClient.Create(ctx, crd) diff --git a/tests/cel/nginxproxy_test.go b/tests/cel/nginxproxy_test.go index 45515980b7..fe3ce144e9 100644 --- a/tests/cel/nginxproxy_test.go +++ b/tests/cel/nginxproxy_test.go @@ -3,7 +3,6 @@ package cel import ( "testing" - . "github.com/onsi/gomega" controllerruntime "sigs.k8s.io/controller-runtime" ngfAPIv1alpha2 "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha2" @@ -12,10 +11,7 @@ import ( func TestNginxProxyKubernetes(t *testing.T) { t.Parallel() - g := NewWithT(t) - - k8sClient, err := getKubernetesClient(t) - g.Expect(err).ToNot(HaveOccurred()) + k8sClient := getKubernetesClient(t) tests := []struct { spec ngfAPIv1alpha2.NginxProxySpec @@ -54,7 +50,7 @@ func TestNginxProxyKubernetes(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() spec := tt.spec - policyName := uniqueResourceName(testPolicyName) + policyName := uniqueResourceName(testResourceName) nginxProxy := &ngfAPIv1alpha2.NginxProxy{ ObjectMeta: controllerruntime.ObjectMeta{ @@ -63,17 +59,14 @@ func TestNginxProxyKubernetes(t *testing.T) { }, Spec: spec, } - validateCrd(t, tt.wantErrors, g, nginxProxy, k8sClient) + validateCrd(t, tt.wantErrors, nginxProxy, k8sClient) }) } } func TestNginxProxyRewriteClientIP(t *testing.T) { t.Parallel() - g := NewWithT(t) - - k8sClient, err := getKubernetesClient(t) - g.Expect(err).ToNot(HaveOccurred()) + k8sClient := getKubernetesClient(t) tests := []struct { spec ngfAPIv1alpha2.NginxProxySpec @@ -109,7 +102,7 @@ func TestNginxProxyRewriteClientIP(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() spec := tt.spec - policyName := uniqueResourceName(testPolicyName) + policyName := uniqueResourceName(testResourceName) nginxProxy := &ngfAPIv1alpha2.NginxProxy{ ObjectMeta: controllerruntime.ObjectMeta{ @@ -118,17 +111,14 @@ func TestNginxProxyRewriteClientIP(t *testing.T) { }, Spec: spec, } - validateCrd(t, tt.wantErrors, g, nginxProxy, k8sClient) + validateCrd(t, tt.wantErrors, nginxProxy, k8sClient) }) } } func TestNginxProxyAutoscaling(t *testing.T) { t.Parallel() - g := NewWithT(t) - - k8sClient, err := getKubernetesClient(t) - g.Expect(err).ToNot(HaveOccurred()) + k8sClient := getKubernetesClient(t) tests := []struct { spec ngfAPIv1alpha2.NginxProxySpec @@ -180,7 +170,7 @@ func TestNginxProxyAutoscaling(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() spec := tt.spec - policyName := uniqueResourceName(testPolicyName) + policyName := uniqueResourceName(testResourceName) nginxProxy := &ngfAPIv1alpha2.NginxProxy{ ObjectMeta: controllerruntime.ObjectMeta{ @@ -189,7 +179,7 @@ func TestNginxProxyAutoscaling(t *testing.T) { }, Spec: spec, } - validateCrd(t, tt.wantErrors, g, nginxProxy, k8sClient) + validateCrd(t, tt.wantErrors, nginxProxy, k8sClient) }) } } From 8612c211c1378be7fc76062547b0c74896524bc3 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Mon, 11 Aug 2025 10:37:45 +0100 Subject: [PATCH 08/19] Change name of `testResourceName` const --- tests/cel/common.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/cel/common.go b/tests/cel/common.go index c8effe0f06..9e4cc09ff9 100644 --- a/tests/cel/common.go +++ b/tests/cel/common.go @@ -44,7 +44,7 @@ const ( ) const ( - testResourceName = "test-policy" + testResourceName = "test-resource" testTargetRefName = "test-targetRef" ) From 45302dcafc3938b99c9d9b99efe9d7f209786f3a Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Mon, 11 Aug 2025 14:24:11 +0100 Subject: [PATCH 09/19] Group expexted error consts --- tests/cel/common.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tests/cel/common.go b/tests/cel/common.go index 9e4cc09ff9..67b88f221f 100644 --- a/tests/cel/common.go +++ b/tests/cel/common.go @@ -30,10 +30,15 @@ const ( discoveryGroup = "discovery.k8s.io/v1" ) +// ClientSettingsPolicy validation errors. +const ( + expectedTargetRefKindError = `TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute` + expectedTargetRefGroupError = `TargetRef Group must be gateway.networking.k8s.io.` + expectedHeaderWithoutServerError = `header can only be specified if server is specified` +) + +// NginxProxy validation errors. const ( - expectedTargetRefKindError = `TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute` - expectedTargetRefGroupError = `TargetRef Group must be gateway.networking.k8s.io.` - expectedHeaderWithoutServerError = `header can only be specified if server is specified` expectedOneOfDeploymentOrDaemonSetError = `only one of deployment or daemonSet can be set` expectedIfModeSetTrustedAddressesError = `if mode is set, trustedAddresses is a required field` expectedMinReplicasLessThanOrEqualError = `minReplicas must be less than or equal to maxReplicas` From 9bb4bdb5656fe512d30347b95956174ec4fbba2a Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Mon, 11 Aug 2025 14:46:46 +0100 Subject: [PATCH 10/19] Ensure gateway CRDs are installed when running `test-cel-validation` --- tests/Makefile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/Makefile b/tests/Makefile index 6771828ceb..9c3be8064a 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -18,6 +18,7 @@ EXPERIMENTAL_CONFORMANCE_PROFILES = GATEWAY-TLS CONFORMANCE_PROFILES = $(STANDARD_CONFORMANCE_PROFILES) # by default we use the standard conformance profiles. If experimental is enabled we override this and add the experimental profiles. SKIP_TESTS = CEL_TEST_TARGET = +BRANCH = ?= main # Check if ENABLE_EXPERIMENTAL is true ifeq ($(ENABLE_EXPERIMENTAL),true) @@ -172,7 +173,7 @@ HELM_PARAMETERS += --set nginxGateway.name=nginx-gateway --set nginx.service.typ # it overrides the target in the main Makefile when the GW_API_VERSION is set to main ifeq ($(GW_API_VERSION),main) install-gateway-crds: - kubectl kustomize "https://github.com/kubernetes-sigs/gateway-api/config/crd/$(if $(filter true,$(ENABLE_EXPERIMENTAL)),experimental,)?timeout=120&ref=main" | kubectl apply -f - + kubectl kustomize "https://github.com/kubernetes-sigs/gateway-api/config/crd/$(if $(filter true,$(ENABLE_EXPERIMENTAL)),experimental,)?timeout=120&ref=$(BRANCH)" | kubectl apply -f - endif .PHONY: install-ngf-local-no-build @@ -191,6 +192,7 @@ uninstall-ngf: ## Uninstall NGF on configured kind cluster # Run CEL validation integration tests against a real cluster .PHONY: test-cel-validation test-cel-validation: + -make install-gateway-crds || true; @if [ -z "$(CEL_TEST_TARGET)" ]; then \ echo "Running all tests in ./cel"; \ go test -v ./cel; \ From 531850d58d37914024d0548073b3a0eed8ebd65e Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Mon, 11 Aug 2025 14:49:50 +0100 Subject: [PATCH 11/19] Undo changes to `install-gateway-crds` command --- tests/Makefile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/Makefile b/tests/Makefile index 9c3be8064a..fe41af2b2e 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -18,7 +18,6 @@ EXPERIMENTAL_CONFORMANCE_PROFILES = GATEWAY-TLS CONFORMANCE_PROFILES = $(STANDARD_CONFORMANCE_PROFILES) # by default we use the standard conformance profiles. If experimental is enabled we override this and add the experimental profiles. SKIP_TESTS = CEL_TEST_TARGET = -BRANCH = ?= main # Check if ENABLE_EXPERIMENTAL is true ifeq ($(ENABLE_EXPERIMENTAL),true) @@ -173,7 +172,7 @@ HELM_PARAMETERS += --set nginxGateway.name=nginx-gateway --set nginx.service.typ # it overrides the target in the main Makefile when the GW_API_VERSION is set to main ifeq ($(GW_API_VERSION),main) install-gateway-crds: - kubectl kustomize "https://github.com/kubernetes-sigs/gateway-api/config/crd/$(if $(filter true,$(ENABLE_EXPERIMENTAL)),experimental,)?timeout=120&ref=$(BRANCH)" | kubectl apply -f - + kubectl kustomize "https://github.com/kubernetes-sigs/gateway-api/config/crd/$(if $(filter true,$(ENABLE_EXPERIMENTAL)),experimental,)?timeout=120&ref=main" | kubectl apply -f - endif .PHONY: install-ngf-local-no-build From d74246799ea75003dface3256cbecd15240eb704 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Mon, 11 Aug 2025 15:04:55 +0100 Subject: [PATCH 12/19] Change `policyName` to `resourceName` --- tests/cel/nginxproxy_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/cel/nginxproxy_test.go b/tests/cel/nginxproxy_test.go index fe3ce144e9..94100e9aed 100644 --- a/tests/cel/nginxproxy_test.go +++ b/tests/cel/nginxproxy_test.go @@ -50,11 +50,11 @@ func TestNginxProxyKubernetes(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() spec := tt.spec - policyName := uniqueResourceName(testResourceName) + resourceName := uniqueResourceName(testResourceName) nginxProxy := &ngfAPIv1alpha2.NginxProxy{ ObjectMeta: controllerruntime.ObjectMeta{ - Name: policyName, + Name: resourceName, Namespace: defaultNamespace, }, Spec: spec, @@ -102,11 +102,11 @@ func TestNginxProxyRewriteClientIP(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() spec := tt.spec - policyName := uniqueResourceName(testResourceName) + resourceName := uniqueResourceName(testResourceName) nginxProxy := &ngfAPIv1alpha2.NginxProxy{ ObjectMeta: controllerruntime.ObjectMeta{ - Name: policyName, + Name: resourceName, Namespace: defaultNamespace, }, Spec: spec, @@ -170,11 +170,11 @@ func TestNginxProxyAutoscaling(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() spec := tt.spec - policyName := uniqueResourceName(testResourceName) + resourceName := uniqueResourceName(testResourceName) nginxProxy := &ngfAPIv1alpha2.NginxProxy{ ObjectMeta: controllerruntime.ObjectMeta{ - Name: policyName, + Name: resourceName, Namespace: defaultNamespace, }, Spec: spec, From 216406ca1e3d0dda61105cd57065bee199c64af6 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Tue, 12 Aug 2025 08:19:20 +0100 Subject: [PATCH 13/19] Add test for when `minReplicas` is nil --- tests/Makefile | 1 + tests/cel/common.go | 9 +++------ tests/cel/nginxproxy_test.go | 13 +++++++++++++ 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/tests/Makefile b/tests/Makefile index fe41af2b2e..3b6a0cdc20 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -191,6 +191,7 @@ uninstall-ngf: ## Uninstall NGF on configured kind cluster # Run CEL validation integration tests against a real cluster .PHONY: test-cel-validation test-cel-validation: + -make install-crds || true; -make install-gateway-crds || true; @if [ -z "$(CEL_TEST_TARGET)" ]; then \ echo "Running all tests in ./cel"; \ diff --git a/tests/cel/common.go b/tests/cel/common.go index 67b88f221f..29926275b9 100644 --- a/tests/cel/common.go +++ b/tests/cel/common.go @@ -63,11 +63,8 @@ func getKubernetesClient(t *testing.T) (k8sClient client.Client) { // Set up scheme with NGF types scheme := runtime.NewScheme() - err = ngfAPIv1alpha1.AddToScheme(scheme) - g.Expect(err).ToNot(HaveOccurred()) - - err = ngfAPIv1alpha2.AddToScheme(scheme) - g.Expect(err).ToNot(HaveOccurred()) + g.Expect(ngfAPIv1alpha1.AddToScheme(scheme)).To(Succeed()) + g.Expect(ngfAPIv1alpha2.AddToScheme(scheme)).To(Succeed()) k8sClient, err = client.New(k8sConfig, client.Options{Scheme: scheme}) g.Expect(err).ToNot(HaveOccurred()) @@ -96,8 +93,8 @@ func validateCrd(t *testing.T, wantErrors []string, crd client.Object, k8sClient g := NewWithT(t) timeoutConfig := framework.DefaultTimeoutConfig() ctx, cancel := context.WithTimeout(context.Background(), timeoutConfig.KubernetesClientTimeout) - err := k8sClient.Create(ctx, crd) defer cancel() + err := k8sClient.Create(ctx, crd) // Clean up after test defer func() { diff --git a/tests/cel/nginxproxy_test.go b/tests/cel/nginxproxy_test.go index 94100e9aed..022c870d63 100644 --- a/tests/cel/nginxproxy_test.go +++ b/tests/cel/nginxproxy_test.go @@ -165,6 +165,19 @@ func TestNginxProxyAutoscaling(t *testing.T) { }, }, }, + { + name: "Validate NginxProxy is valid when MinReplicas is nil", + spec: ngfAPIv1alpha2.NginxProxySpec{ + Kubernetes: &ngfAPIv1alpha2.KubernetesSpec{ + Deployment: &ngfAPIv1alpha2.DeploymentSpec{ + Autoscaling: &ngfAPIv1alpha2.AutoscalingSpec{ + MinReplicas: nil, + MaxReplicas: 5, + }, + }, + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From 960db801ff037b3d71ba3b652a1e17c9fd962aea Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Tue, 12 Aug 2025 10:42:57 +0100 Subject: [PATCH 14/19] Have `k8sClient.Delete` use same context and `k8sClient.Create` --- tests/cel/common.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/cel/common.go b/tests/cel/common.go index 29926275b9..5671404fad 100644 --- a/tests/cel/common.go +++ b/tests/cel/common.go @@ -91,6 +91,7 @@ func uniqueResourceName(name string) string { func validateCrd(t *testing.T, wantErrors []string, crd client.Object, k8sClient client.Client) { t.Helper() g := NewWithT(t) + var deleteErr error timeoutConfig := framework.DefaultTimeoutConfig() ctx, cancel := context.WithTimeout(context.Background(), timeoutConfig.KubernetesClientTimeout) defer cancel() @@ -98,9 +99,9 @@ func validateCrd(t *testing.T, wantErrors []string, crd client.Object, k8sClient // Clean up after test defer func() { - _ = k8sClient.Delete(context.Background(), crd) + deleteErr = k8sClient.Delete(ctx, crd) }() - + g.Expect(deleteErr).ToNot(HaveOccurred()) if len(wantErrors) == 0 { g.Expect(err).ToNot(HaveOccurred()) } else { From 38e20ee48bbf3ff9e8db65c14724a2372af14fd3 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Tue, 12 Aug 2025 11:12:01 +0100 Subject: [PATCH 15/19] Add spacing --- tests/cel/common.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/cel/common.go b/tests/cel/common.go index 5671404fad..542d72f4fd 100644 --- a/tests/cel/common.go +++ b/tests/cel/common.go @@ -102,6 +102,8 @@ func validateCrd(t *testing.T, wantErrors []string, crd client.Object, k8sClient deleteErr = k8sClient.Delete(ctx, crd) }() g.Expect(deleteErr).ToNot(HaveOccurred()) + + // Check for expected errors if len(wantErrors) == 0 { g.Expect(err).ToNot(HaveOccurred()) } else { From aba710a74bbbc37a001762f360ca5422d3c057af Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Tue, 12 Aug 2025 11:13:27 +0100 Subject: [PATCH 16/19] Add additional comments --- tests/cel/common.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/cel/common.go b/tests/cel/common.go index 542d72f4fd..d5e875dcf4 100644 --- a/tests/cel/common.go +++ b/tests/cel/common.go @@ -91,6 +91,9 @@ func uniqueResourceName(name string) string { func validateCrd(t *testing.T, wantErrors []string, crd client.Object, k8sClient client.Client) { t.Helper() g := NewWithT(t) + + // Create the resource in the cluster + // Use a context with a timeout to avoid hanging tests var deleteErr error timeoutConfig := framework.DefaultTimeoutConfig() ctx, cancel := context.WithTimeout(context.Background(), timeoutConfig.KubernetesClientTimeout) From 14ee3f7f73227ac868d383f450885c9a52912faa Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Tue, 12 Aug 2025 16:11:01 +0100 Subject: [PATCH 17/19] Ensure installed resources are only delete if an error is NOT expected --- tests/Makefile | 1 - tests/cel/common.go | 11 +++++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/tests/Makefile b/tests/Makefile index 3b6a0cdc20..4072702f4b 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -192,7 +192,6 @@ uninstall-ngf: ## Uninstall NGF on configured kind cluster .PHONY: test-cel-validation test-cel-validation: -make install-crds || true; - -make install-gateway-crds || true; @if [ -z "$(CEL_TEST_TARGET)" ]; then \ echo "Running all tests in ./cel"; \ go test -v ./cel; \ diff --git a/tests/cel/common.go b/tests/cel/common.go index d5e875dcf4..a0e2ddead7 100644 --- a/tests/cel/common.go +++ b/tests/cel/common.go @@ -100,15 +100,14 @@ func validateCrd(t *testing.T, wantErrors []string, crd client.Object, k8sClient defer cancel() err := k8sClient.Create(ctx, crd) - // Clean up after test - defer func() { - deleteErr = k8sClient.Delete(ctx, crd) - }() - g.Expect(deleteErr).ToNot(HaveOccurred()) - // Check for expected errors if len(wantErrors) == 0 { g.Expect(err).ToNot(HaveOccurred()) + // Clean up after test + defer func() { + deleteErr = k8sClient.Delete(ctx, crd) + g.Expect(deleteErr).ToNot(HaveOccurred()) + }() } else { g.Expect(err).To(HaveOccurred()) for _, wantError := range wantErrors { From ac12babf2a94a70178c4ac1905003dcf5b9b395b Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Tue, 12 Aug 2025 16:18:43 +0100 Subject: [PATCH 18/19] Expected k8sClient.Delete to succeed --- tests/cel/common.go | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/tests/cel/common.go b/tests/cel/common.go index a0e2ddead7..0fc13eee73 100644 --- a/tests/cel/common.go +++ b/tests/cel/common.go @@ -92,9 +92,6 @@ func validateCrd(t *testing.T, wantErrors []string, crd client.Object, k8sClient t.Helper() g := NewWithT(t) - // Create the resource in the cluster - // Use a context with a timeout to avoid hanging tests - var deleteErr error timeoutConfig := framework.DefaultTimeoutConfig() ctx, cancel := context.WithTimeout(context.Background(), timeoutConfig.KubernetesClientTimeout) defer cancel() @@ -104,10 +101,7 @@ func validateCrd(t *testing.T, wantErrors []string, crd client.Object, k8sClient if len(wantErrors) == 0 { g.Expect(err).ToNot(HaveOccurred()) // Clean up after test - defer func() { - deleteErr = k8sClient.Delete(ctx, crd) - g.Expect(deleteErr).ToNot(HaveOccurred()) - }() + g.Expect(k8sClient.Delete(ctx, crd)).To(Succeed()) } else { g.Expect(err).To(HaveOccurred()) for _, wantError := range wantErrors { From 51d49f827d2c8bcfeef2153c04d8616b401d63f2 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Wed, 13 Aug 2025 09:12:19 +0100 Subject: [PATCH 19/19] Add additional comments --- tests/cel/common.go | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/cel/common.go b/tests/cel/common.go index 0fc13eee73..fcbb85366c 100644 --- a/tests/cel/common.go +++ b/tests/cel/common.go @@ -101,6 +101,7 @@ func validateCrd(t *testing.T, wantErrors []string, crd client.Object, k8sClient if len(wantErrors) == 0 { g.Expect(err).ToNot(HaveOccurred()) // Clean up after test + // Resources only need to be deleted if they were created successfully g.Expect(k8sClient.Delete(ctx, crd)).To(Succeed()) } else { g.Expect(err).To(HaveOccurred())