From dbb5b5c3e9ded0909c41ddb98790a1e3d1598d45 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Mon, 14 Jul 2025 10:54:31 +0100 Subject: [PATCH 01/46] Add CEL validation test for targetRef in ClientSettingsPolicy --- .../clientsettingspolicies/targetRef_test.go | 107 ++++++++++++++++++ tests/go.mod | 2 +- 2 files changed, 108 insertions(+), 1 deletion(-) create mode 100644 tests/cel/policies/clientsettingspolicies/targetRef_test.go diff --git a/tests/cel/policies/clientsettingspolicies/targetRef_test.go b/tests/cel/policies/clientsettingspolicies/targetRef_test.go new file mode 100644 index 0000000000..2c62f86ce8 --- /dev/null +++ b/tests/cel/policies/clientsettingspolicies/targetRef_test.go @@ -0,0 +1,107 @@ +package clientsettingspolicies + +import ( + "testing" + + gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" +) + +func TestClientSettingsPoliciesTargetRef(t *testing.T) { + + allowedKinds := map[string]bool{ + "Gateway": true, + "HTTPRoute": true, + "GRPCRoute": true, + } + + testInvalidTargetRefs(t, allowedKinds) + testValidTargetRefs(t, allowedKinds) +} + +func testInvalidTargetRefs(t *testing.T, allowedKinds map[string]bool) { + t.Helper() + + tests := []struct { + name string + wantErrors string + targetRef gatewayv1alpha2.LocalPolicyTargetReference + }{ + { + name: "Validate TargetRef is of an allowed kind", + wantErrors: "TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'", + targetRef: gatewayv1alpha2.LocalPolicyTargetReference{ + Kind: "InvalidKind", + Group: "gateway.networking.k8s.io", + }, + }, + { + name: "Validate TargetRef is of an allowed kind", + wantErrors: "TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'", + targetRef: gatewayv1alpha2.LocalPolicyTargetReference{ + Kind: "TCPRoute", + Group: "gateway.networking.k8s.io", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + + if _, ok := allowedKinds[string(tt.targetRef.Kind)]; !ok { + gotError := "TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'" + + if tt.wantErrors != gotError { + t.Errorf("Test %s failed: got error %q, want %q", tt.name, gotError, tt.wantErrors) + } + } + }) + } +} + +func testValidTargetRefs(t *testing.T, allowedKinds map[string]bool) { + t.Helper() + + tests := []struct { + name string + wantErrors string + targetRef gatewayv1alpha2.LocalPolicyTargetReference + }{ + { + name: "Validate TargetRef is of an allowed kind", + wantErrors: "TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'", + targetRef: gatewayv1alpha2.LocalPolicyTargetReference{ + Kind: "Gateway", + Group: "gateway.networking.k8s.io", + }, + }, + { + name: "Validate TargetRef is of an allowed kind", + wantErrors: "TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'", + targetRef: gatewayv1alpha2.LocalPolicyTargetReference{ + Kind: "HTTPRoute", + Group: "gateway.networking.k8s.io", + }, + }, + { + name: "Validate TargetRef is of an allowed kind", + wantErrors: "TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'", + targetRef: gatewayv1alpha2.LocalPolicyTargetReference{ + Kind: "GRPCRoute", + Group: "gateway.networking.k8s.io", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + + if _, ok := allowedKinds[string(tt.targetRef.Kind)]; !ok { + gotError := "TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'" + + if tt.wantErrors == gotError { + t.Errorf("Test %s failed: got error %q, want %q", tt.name, gotError, tt.wantErrors) + } + } + }) + } +} diff --git a/tests/go.mod b/tests/go.mod index aea75d7618..6a08571ca0 100644 --- a/tests/go.mod +++ b/tests/go.mod @@ -5,7 +5,7 @@ go 1.24.2 replace github.com/nginx/nginx-gateway-fabric => ../ require ( - github.com/nginx/nginx-gateway-fabric v0.0.0 + github.com/nginx/nginx-gateway-fabric v1.6.2 github.com/onsi/ginkgo/v2 v2.23.4 github.com/onsi/gomega v1.37.0 github.com/prometheus/client_golang v1.22.0 From 48ec86b009d4abf0efcd85d9611478ace4064cc3 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Mon, 14 Jul 2025 11:06:21 +0100 Subject: [PATCH 02/46] gofumpt --- tests/cel/policies/clientsettingspolicies/targetRef_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/cel/policies/clientsettingspolicies/targetRef_test.go b/tests/cel/policies/clientsettingspolicies/targetRef_test.go index 2c62f86ce8..c22189f02e 100644 --- a/tests/cel/policies/clientsettingspolicies/targetRef_test.go +++ b/tests/cel/policies/clientsettingspolicies/targetRef_test.go @@ -7,7 +7,6 @@ import ( ) func TestClientSettingsPoliciesTargetRef(t *testing.T) { - allowedKinds := map[string]bool{ "Gateway": true, "HTTPRoute": true, @@ -46,7 +45,6 @@ func testInvalidTargetRefs(t *testing.T, allowedKinds map[string]bool) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if _, ok := allowedKinds[string(tt.targetRef.Kind)]; !ok { gotError := "TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'" @@ -94,7 +92,6 @@ func testValidTargetRefs(t *testing.T, allowedKinds map[string]bool) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if _, ok := allowedKinds[string(tt.targetRef.Kind)]; !ok { gotError := "TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'" From c7129bd7301f3c84c0a57af47421232112ba265d Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Mon, 14 Jul 2025 11:47:35 +0100 Subject: [PATCH 03/46] Add tests for targetRefGroup --- .../clientsettingspolicies/targetRef_test.go | 100 +++++++++++++++--- 1 file changed, 86 insertions(+), 14 deletions(-) diff --git a/tests/cel/policies/clientsettingspolicies/targetRef_test.go b/tests/cel/policies/clientsettingspolicies/targetRef_test.go index c22189f02e..c704a3791f 100644 --- a/tests/cel/policies/clientsettingspolicies/targetRef_test.go +++ b/tests/cel/policies/clientsettingspolicies/targetRef_test.go @@ -13,11 +13,16 @@ func TestClientSettingsPoliciesTargetRef(t *testing.T) { "GRPCRoute": true, } - testInvalidTargetRefs(t, allowedKinds) testValidTargetRefs(t, allowedKinds) + testInvalidTargetRefs(t, allowedKinds) } -func testInvalidTargetRefs(t *testing.T, allowedKinds map[string]bool) { +func TestClientSettingsPoliciesTargetRefGroup(t *testing.T) { + testValidTargetRefGroup(t) + testInvalidTargetRefGroup(t) +} + +func testValidTargetRefs(t *testing.T, allowedKinds map[string]bool) { t.Helper() tests := []struct { @@ -29,7 +34,7 @@ func testInvalidTargetRefs(t *testing.T, allowedKinds map[string]bool) { name: "Validate TargetRef is of an allowed kind", wantErrors: "TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'", targetRef: gatewayv1alpha2.LocalPolicyTargetReference{ - Kind: "InvalidKind", + Kind: "Gateway", Group: "gateway.networking.k8s.io", }, }, @@ -37,7 +42,15 @@ func testInvalidTargetRefs(t *testing.T, allowedKinds map[string]bool) { name: "Validate TargetRef is of an allowed kind", wantErrors: "TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'", targetRef: gatewayv1alpha2.LocalPolicyTargetReference{ - Kind: "TCPRoute", + Kind: "HTTPRoute", + Group: "gateway.networking.k8s.io", + }, + }, + { + name: "Validate TargetRef is of an allowed kind", + wantErrors: "TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'", + targetRef: gatewayv1alpha2.LocalPolicyTargetReference{ + Kind: "GRPCRoute", Group: "gateway.networking.k8s.io", }, }, @@ -48,7 +61,7 @@ func testInvalidTargetRefs(t *testing.T, allowedKinds map[string]bool) { if _, ok := allowedKinds[string(tt.targetRef.Kind)]; !ok { gotError := "TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'" - if tt.wantErrors != gotError { + if tt.wantErrors == gotError { t.Errorf("Test %s failed: got error %q, want %q", tt.name, gotError, tt.wantErrors) } } @@ -56,7 +69,7 @@ func testInvalidTargetRefs(t *testing.T, allowedKinds map[string]bool) { } } -func testValidTargetRefs(t *testing.T, allowedKinds map[string]bool) { +func testInvalidTargetRefs(t *testing.T, allowedKinds map[string]bool) { t.Helper() tests := []struct { @@ -68,7 +81,7 @@ func testValidTargetRefs(t *testing.T, allowedKinds map[string]bool) { name: "Validate TargetRef is of an allowed kind", wantErrors: "TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'", targetRef: gatewayv1alpha2.LocalPolicyTargetReference{ - Kind: "Gateway", + Kind: "InvalidKind", Group: "gateway.networking.k8s.io", }, }, @@ -76,15 +89,37 @@ func testValidTargetRefs(t *testing.T, allowedKinds map[string]bool) { name: "Validate TargetRef is of an allowed kind", wantErrors: "TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'", targetRef: gatewayv1alpha2.LocalPolicyTargetReference{ - Kind: "HTTPRoute", + Kind: "TCPRoute", Group: "gateway.networking.k8s.io", }, }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if _, ok := allowedKinds[string(tt.targetRef.Kind)]; !ok { + gotError := "TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'" + + if tt.wantErrors != gotError { + t.Errorf("Test %s failed: got error %q, want %q", tt.name, gotError, tt.wantErrors) + } + } + }) + } +} + +func testValidTargetRefGroup(t *testing.T) { + t.Helper() + + tests := []struct { + name string + wantErrors string + targetRefGroup gatewayv1alpha2.LocalPolicyTargetReference + }{ { - name: "Validate TargetRef is of an allowed kind", - wantErrors: "TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'", - targetRef: gatewayv1alpha2.LocalPolicyTargetReference{ - Kind: "GRPCRoute", + name: "Validate TargetRef group is gateway.networking.k8s.io", + wantErrors: "TargetRef Group must be gateway.networking.k8s.io", + targetRefGroup: gatewayv1alpha2.LocalPolicyTargetReference{ Group: "gateway.networking.k8s.io", }, }, @@ -92,8 +127,8 @@ func testValidTargetRefs(t *testing.T, allowedKinds map[string]bool) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if _, ok := allowedKinds[string(tt.targetRef.Kind)]; !ok { - gotError := "TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'" + if tt.targetRefGroup.Group != "gateway.networking.k8s.io" { + gotError := "TargetRef Group must be gateway.networking.k8s.io" if tt.wantErrors == gotError { t.Errorf("Test %s failed: got error %q, want %q", tt.name, gotError, tt.wantErrors) @@ -102,3 +137,40 @@ func testValidTargetRefs(t *testing.T, allowedKinds map[string]bool) { }) } } + +func testInvalidTargetRefGroup(t *testing.T) { + t.Helper() + + tests := []struct { + name string + wantErrors string + targetRefGroup gatewayv1alpha2.LocalPolicyTargetReference + }{ + { + name: "Validate TargetRef group is gateway.networking.k8s.io", + wantErrors: "TargetRef Group must be gateway.networking.k8s.io", + targetRefGroup: gatewayv1alpha2.LocalPolicyTargetReference{ + Group: "invalid.networking.k8s.io", + }, + }, + { + name: "Validate TargetRef is of an allowed kind", + wantErrors: "TargetRef Group must be gateway.networking.k8s.io", + targetRefGroup: gatewayv1alpha2.LocalPolicyTargetReference{ + Group: "discovery.k8s.io/v1", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.targetRefGroup.Group != "gateway.networking.k8s.io" { + gotError := "TargetRef Group must be gateway.networking.k8s.io" + + if tt.wantErrors != gotError { + t.Errorf("Test %s failed: got error %q, want %q", tt.name, gotError, tt.wantErrors) + } + } + }) + } +} From 87a4e0e8574a9f6609cb2ed777437dbfd5f85987 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Mon, 14 Jul 2025 13:11:13 +0100 Subject: [PATCH 04/46] Rename tests --- .../policies/clientsettingspolicies/targetRef_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/cel/policies/clientsettingspolicies/targetRef_test.go b/tests/cel/policies/clientsettingspolicies/targetRef_test.go index c704a3791f..70a7d9f230 100644 --- a/tests/cel/policies/clientsettingspolicies/targetRef_test.go +++ b/tests/cel/policies/clientsettingspolicies/targetRef_test.go @@ -6,15 +6,15 @@ import ( gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" ) -func TestClientSettingsPoliciesTargetRef(t *testing.T) { +func TestClientSettingsPoliciesTargetRefKind(t *testing.T) { allowedKinds := map[string]bool{ "Gateway": true, "HTTPRoute": true, "GRPCRoute": true, } - testValidTargetRefs(t, allowedKinds) - testInvalidTargetRefs(t, allowedKinds) + testValidTargetRefKind(t, allowedKinds) + testInvalidTargetRefKind(t, allowedKinds) } func TestClientSettingsPoliciesTargetRefGroup(t *testing.T) { @@ -22,7 +22,7 @@ func TestClientSettingsPoliciesTargetRefGroup(t *testing.T) { testInvalidTargetRefGroup(t) } -func testValidTargetRefs(t *testing.T, allowedKinds map[string]bool) { +func testValidTargetRefKind(t *testing.T, allowedKinds map[string]bool) { t.Helper() tests := []struct { @@ -69,7 +69,7 @@ func testValidTargetRefs(t *testing.T, allowedKinds map[string]bool) { } } -func testInvalidTargetRefs(t *testing.T, allowedKinds map[string]bool) { +func testInvalidTargetRefKind(t *testing.T, allowedKinds map[string]bool) { t.Helper() tests := []struct { From d921ed237a86c993e02f22b22e8c58a4c4897068 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Tue, 15 Jul 2025 08:49:25 +0100 Subject: [PATCH 05/46] Move tests into clientsettingspolicy_test.go --- tests/cel/clientsettingspolicy_test.go | 176 +++++++++++++++++++++++++ 1 file changed, 176 insertions(+) create mode 100644 tests/cel/clientsettingspolicy_test.go diff --git a/tests/cel/clientsettingspolicy_test.go b/tests/cel/clientsettingspolicy_test.go new file mode 100644 index 0000000000..a5d148ce4d --- /dev/null +++ b/tests/cel/clientsettingspolicy_test.go @@ -0,0 +1,176 @@ +package cel + +import ( + "testing" + + gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" +) + +func TestClientSettingsPoliciesTargetRefKind(t *testing.T) { + allowedKinds := map[string]bool{ + "Gateway": true, + "HTTPRoute": true, + "GRPCRoute": true, + } + + testValidTargetRefKind(t, allowedKinds) + testInvalidTargetRefKind(t, allowedKinds) +} + +func TestClientSettingsPoliciesTargetRefGroup(t *testing.T) { + testValidTargetRefGroup(t) + testInvalidTargetRefGroup(t) +} + +func testValidTargetRefKind(t *testing.T, allowedKinds map[string]bool) { + t.Helper() + + tests := []struct { + name string + wantErrors string + targetRef gatewayv1alpha2.LocalPolicyTargetReference + }{ + { + name: "Validate TargetRef is of an allowed kind", + wantErrors: "TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'", + targetRef: gatewayv1alpha2.LocalPolicyTargetReference{ + Kind: "Gateway", + Group: "gateway.networking.k8s.io", + }, + }, + { + name: "Validate TargetRef is of an allowed kind", + wantErrors: "TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'", + targetRef: gatewayv1alpha2.LocalPolicyTargetReference{ + Kind: "HTTPRoute", + Group: "gateway.networking.k8s.io", + }, + }, + { + name: "Validate TargetRef is of an allowed kind", + wantErrors: "TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'", + targetRef: gatewayv1alpha2.LocalPolicyTargetReference{ + Kind: "GRPCRoute", + Group: "gateway.networking.k8s.io", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if _, ok := allowedKinds[string(tt.targetRef.Kind)]; !ok { + gotError := "TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'" + + if tt.wantErrors == gotError { + t.Errorf("Test %s failed: got error %q, want %q", tt.name, gotError, tt.wantErrors) + } + } + }) + } +} + +func testInvalidTargetRefKind(t *testing.T, allowedKinds map[string]bool) { + t.Helper() + + tests := []struct { + name string + wantErrors string + targetRef gatewayv1alpha2.LocalPolicyTargetReference + }{ + { + name: "Validate TargetRef is of an allowed kind", + wantErrors: "TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'", + targetRef: gatewayv1alpha2.LocalPolicyTargetReference{ + Kind: "InvalidKind", + Group: "gateway.networking.k8s.io", + }, + }, + { + name: "Validate TargetRef is of an allowed kind", + wantErrors: "TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'", + targetRef: gatewayv1alpha2.LocalPolicyTargetReference{ + Kind: "TCPRoute", + Group: "gateway.networking.k8s.io", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if _, ok := allowedKinds[string(tt.targetRef.Kind)]; !ok { + gotError := "TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'" + + if tt.wantErrors != gotError { + t.Errorf("Test %s failed: got error %q, want %q", tt.name, gotError, tt.wantErrors) + } + } + }) + } +} + +func testValidTargetRefGroup(t *testing.T) { + t.Helper() + + tests := []struct { + name string + wantErrors string + targetRefGroup gatewayv1alpha2.LocalPolicyTargetReference + }{ + { + name: "Validate TargetRef group is gateway.networking.k8s.io", + wantErrors: "TargetRef Group must be gateway.networking.k8s.io", + targetRefGroup: gatewayv1alpha2.LocalPolicyTargetReference{ + Group: "gateway.networking.k8s.io", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.targetRefGroup.Group != "gateway.networking.k8s.io" { + gotError := "TargetRef Group must be gateway.networking.k8s.io" + + if tt.wantErrors == gotError { + t.Errorf("Test %s failed: got error %q, want %q", tt.name, gotError, tt.wantErrors) + } + } + }) + } +} + +func testInvalidTargetRefGroup(t *testing.T) { + t.Helper() + + tests := []struct { + name string + wantErrors string + targetRefGroup gatewayv1alpha2.LocalPolicyTargetReference + }{ + { + name: "Validate TargetRef group is gateway.networking.k8s.io", + wantErrors: "TargetRef Group must be gateway.networking.k8s.io", + targetRefGroup: gatewayv1alpha2.LocalPolicyTargetReference{ + Group: "invalid.networking.k8s.io", + }, + }, + { + name: "Validate TargetRef is of an allowed kind", + wantErrors: "TargetRef Group must be gateway.networking.k8s.io", + targetRefGroup: gatewayv1alpha2.LocalPolicyTargetReference{ + Group: "discovery.k8s.io/v1", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.targetRefGroup.Group != "gateway.networking.k8s.io" { + gotError := "TargetRef Group must be gateway.networking.k8s.io" + + if tt.wantErrors != gotError { + t.Errorf("Test %s failed: got error %q, want %q", tt.name, gotError, tt.wantErrors) + } + } + }) + } +} From f22269acc56ea87fd6b381c7e5599f2d68e14379 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Tue, 15 Jul 2025 11:33:59 +0100 Subject: [PATCH 06/46] Update tests to create a ClientSettingsPolicy resource during tests --- tests/cel/clientsettingspolicy_test.go | 84 ++++++++++++++++++++------ 1 file changed, 66 insertions(+), 18 deletions(-) diff --git a/tests/cel/clientsettingspolicy_test.go b/tests/cel/clientsettingspolicy_test.go index a5d148ce4d..fdceaa4dff 100644 --- a/tests/cel/clientsettingspolicy_test.go +++ b/tests/cel/clientsettingspolicy_test.go @@ -3,14 +3,24 @@ package cel import ( "testing" + ngfAPIv1alpha1 "github.com/nginx/nginx-gateway-fabric/apis/v1alpha1" gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" ) func TestClientSettingsPoliciesTargetRefKind(t *testing.T) { - allowedKinds := map[string]bool{ - "Gateway": true, - "HTTPRoute": true, - "GRPCRoute": true, + allowedKinds := map[gatewayv1alpha2.LocalPolicyTargetReference]bool{ + { + Kind: "Gateway", + Group: "gateway.networking.k8s.io", + }: true, + { + Kind: "HTTPRoute", + Group: "gateway.networking.k8s.io", + }: true, + { + Kind: "GRPCRoute", + Group: "gateway.networking.k8s.io", + }: true, } testValidTargetRefKind(t, allowedKinds) @@ -18,11 +28,19 @@ func TestClientSettingsPoliciesTargetRefKind(t *testing.T) { } func TestClientSettingsPoliciesTargetRefGroup(t *testing.T) { - testValidTargetRefGroup(t) - testInvalidTargetRefGroup(t) + // Test valid and invalid TargetRef Group + + allowedGroups := map[gatewayv1alpha2.LocalPolicyTargetReference]bool{ + { + Group: "gateway.networking.k8s.io", + }: true, + } + + testValidTargetRefGroup(t, allowedGroups) + testInvalidTargetRefGroup(t, allowedGroups) } -func testValidTargetRefKind(t *testing.T, allowedKinds map[string]bool) { +func testValidTargetRefKind(t *testing.T, allowedKinds map[gatewayv1alpha2.LocalPolicyTargetReference]bool) { t.Helper() tests := []struct { @@ -58,7 +76,14 @@ func testValidTargetRefKind(t *testing.T, allowedKinds map[string]bool) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if _, ok := allowedKinds[string(tt.targetRef.Kind)]; !ok { + // Create a ClientSettingsPolicy with the targetRef from the test case. + clientSettingsPolicy := &ngfAPIv1alpha1.ClientSettingsPolicy{ + Spec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ + TargetRef: tt.targetRef, + }, + } + + if _, ok := allowedKinds[clientSettingsPolicy.Spec.TargetRef]; !ok { gotError := "TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'" if tt.wantErrors == gotError { @@ -69,7 +94,7 @@ func testValidTargetRefKind(t *testing.T, allowedKinds map[string]bool) { } } -func testInvalidTargetRefKind(t *testing.T, allowedKinds map[string]bool) { +func testInvalidTargetRefKind(t *testing.T, allowedKinds map[gatewayv1alpha2.LocalPolicyTargetReference]bool) { t.Helper() tests := []struct { @@ -97,7 +122,14 @@ func testInvalidTargetRefKind(t *testing.T, allowedKinds map[string]bool) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if _, ok := allowedKinds[string(tt.targetRef.Kind)]; !ok { + // Create a ClientSettingsPolicy with the targetRef from the test case. + clientSettingsPolicy := &ngfAPIv1alpha1.ClientSettingsPolicy{ + Spec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ + TargetRef: tt.targetRef, + }, + } + + if _, ok := allowedKinds[clientSettingsPolicy.Spec.TargetRef]; !ok { gotError := "TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'" if tt.wantErrors != gotError { @@ -108,7 +140,7 @@ func testInvalidTargetRefKind(t *testing.T, allowedKinds map[string]bool) { } } -func testValidTargetRefGroup(t *testing.T) { +func testValidTargetRefGroup(t *testing.T, allowedGroups map[gatewayv1alpha2.LocalPolicyTargetReference]bool) { t.Helper() tests := []struct { @@ -127,7 +159,14 @@ func testValidTargetRefGroup(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if tt.targetRefGroup.Group != "gateway.networking.k8s.io" { + // Create a ClientSettingsPolicy with the targetRef from the test case. + clientSettingsPolicy := &ngfAPIv1alpha1.ClientSettingsPolicy{ + Spec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ + TargetRef: tt.targetRefGroup, + }, + } + + if _, ok := allowedGroups[clientSettingsPolicy.Spec.TargetRef]; !ok { gotError := "TargetRef Group must be gateway.networking.k8s.io" if tt.wantErrors == gotError { @@ -138,7 +177,7 @@ func testValidTargetRefGroup(t *testing.T) { } } -func testInvalidTargetRefGroup(t *testing.T) { +func testInvalidTargetRefGroup(t *testing.T, allowedGroups map[gatewayv1alpha2.LocalPolicyTargetReference]bool) { t.Helper() tests := []struct { @@ -164,13 +203,22 @@ func testInvalidTargetRefGroup(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if tt.targetRefGroup.Group != "gateway.networking.k8s.io" { - gotError := "TargetRef Group must be gateway.networking.k8s.io" + t.Run(tt.name, func(t *testing.T) { + // Create a ClientSettingsPolicy with the targetRef from the test case. + clientSettingsPolicy := &ngfAPIv1alpha1.ClientSettingsPolicy{ + Spec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ + TargetRef: tt.targetRefGroup, + }, + } - if tt.wantErrors != gotError { - t.Errorf("Test %s failed: got error %q, want %q", tt.name, gotError, tt.wantErrors) + if _, ok := allowedGroups[clientSettingsPolicy.Spec.TargetRef]; !ok { + gotError := "TargetRef Group must be gateway.networking.k8s.io" + + if tt.wantErrors != gotError { + t.Errorf("Test %s failed: got error %q, want %q", tt.name, gotError, tt.wantErrors) + } } - } + }) }) } } From 7fddffaf8e17862e4173a64176f154d6c7d41ad2 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Tue, 15 Jul 2025 11:50:04 +0100 Subject: [PATCH 07/46] make lint in tests --- tests/cel/clientsettingspolicy_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/cel/clientsettingspolicy_test.go b/tests/cel/clientsettingspolicy_test.go index fdceaa4dff..985d78351e 100644 --- a/tests/cel/clientsettingspolicy_test.go +++ b/tests/cel/clientsettingspolicy_test.go @@ -3,8 +3,9 @@ package cel import ( "testing" - ngfAPIv1alpha1 "github.com/nginx/nginx-gateway-fabric/apis/v1alpha1" gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" + + ngfAPIv1alpha1 "github.com/nginx/nginx-gateway-fabric/apis/v1alpha1" ) func TestClientSettingsPoliciesTargetRefKind(t *testing.T) { From dd66a795d9788c8cb0e07813f386099038a0a206 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Thu, 17 Jul 2025 10:45:42 +0100 Subject: [PATCH 08/46] Update tests to create a ClientSettingsPolicy object during validation --- tests/cel/clientsettingspolicy_test.go | 159 +++++++++++++++---------- 1 file changed, 93 insertions(+), 66 deletions(-) diff --git a/tests/cel/clientsettingspolicy_test.go b/tests/cel/clientsettingspolicy_test.go index 985d78351e..342b967cfd 100644 --- a/tests/cel/clientsettingspolicy_test.go +++ b/tests/cel/clientsettingspolicy_test.go @@ -1,31 +1,27 @@ package cel import ( + "context" + "fmt" + "strings" "testing" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" ngfAPIv1alpha1 "github.com/nginx/nginx-gateway-fabric/apis/v1alpha1" ) func TestClientSettingsPoliciesTargetRefKind(t *testing.T) { - allowedKinds := map[gatewayv1alpha2.LocalPolicyTargetReference]bool{ - { - Kind: "Gateway", - Group: "gateway.networking.k8s.io", - }: true, - { - Kind: "HTTPRoute", - Group: "gateway.networking.k8s.io", - }: true, - { - Kind: "GRPCRoute", - Group: "gateway.networking.k8s.io", - }: true, - } - - testValidTargetRefKind(t, allowedKinds) - testInvalidTargetRefKind(t, allowedKinds) + // Test valid and invalid TargetRef Kind + // Valid kinds are: Gateway, HTTPRoute, GRPCRoute + testValidTargetRefKind(t) + // Invalid kinds should return an error + // Example of an invalid kind: "InvalidKind", "TCPRoute" + testInvalidTargetRefKind(t) } func TestClientSettingsPoliciesTargetRefGroup(t *testing.T) { @@ -41,36 +37,39 @@ func TestClientSettingsPoliciesTargetRefGroup(t *testing.T) { testInvalidTargetRefGroup(t, allowedGroups) } -func testValidTargetRefKind(t *testing.T, allowedKinds map[gatewayv1alpha2.LocalPolicyTargetReference]bool) { +func testValidTargetRefKind(t *testing.T) { t.Helper() tests := []struct { name string - wantErrors string - targetRef gatewayv1alpha2.LocalPolicyTargetReference + wantErrors []string + policySpec ngfAPIv1alpha1.ClientSettingsPolicySpec }{ { - name: "Validate TargetRef is of an allowed kind", - wantErrors: "TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'", - targetRef: gatewayv1alpha2.LocalPolicyTargetReference{ - Kind: "Gateway", - Group: "gateway.networking.k8s.io", + name: "Validate TargetRef of kind Gateway is allowed", + policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ + TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ + Kind: "Gateway", + Group: "gateway.networking.k8s.io", + }, }, }, { - name: "Validate TargetRef is of an allowed kind", - wantErrors: "TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'", - targetRef: gatewayv1alpha2.LocalPolicyTargetReference{ - Kind: "HTTPRoute", - Group: "gateway.networking.k8s.io", + name: "Validate TargetRef of kind HTTPRoute is allowed", + policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ + TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ + Kind: "HTTPRoute", + Group: "gateway.networking.k8s.io", + }, }, }, { - name: "Validate TargetRef is of an allowed kind", - wantErrors: "TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'", - targetRef: gatewayv1alpha2.LocalPolicyTargetReference{ - Kind: "GRPCRoute", - Group: "gateway.networking.k8s.io", + name: "Validate TargetRef of kind GRPCRoute is allowed", + policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ + TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ + Kind: "GRPCRoute", + Group: "gateway.networking.k8s.io", + }, }, }, } @@ -79,44 +78,45 @@ func testValidTargetRefKind(t *testing.T, allowedKinds map[gatewayv1alpha2.Local t.Run(tt.name, func(t *testing.T) { // Create a ClientSettingsPolicy with the targetRef from the test case. clientSettingsPolicy := &ngfAPIv1alpha1.ClientSettingsPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-policy", + Namespace: "default", + }, Spec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ - TargetRef: tt.targetRef, + TargetRef: tt.policySpec.TargetRef, }, } - - if _, ok := allowedKinds[clientSettingsPolicy.Spec.TargetRef]; !ok { - gotError := "TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'" - - if tt.wantErrors == gotError { - t.Errorf("Test %s failed: got error %q, want %q", tt.name, gotError, tt.wantErrors) - } - } + validateClientSettingsPolicy(t, clientSettingsPolicy, tt.wantErrors) }) } } -func testInvalidTargetRefKind(t *testing.T, allowedKinds map[gatewayv1alpha2.LocalPolicyTargetReference]bool) { +func testInvalidTargetRefKind(t *testing.T) { t.Helper() tests := []struct { name string - wantErrors string - targetRef gatewayv1alpha2.LocalPolicyTargetReference + wantErrors []string + policySpec ngfAPIv1alpha1.ClientSettingsPolicySpec }{ { - name: "Validate TargetRef is of an allowed kind", - wantErrors: "TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'", - targetRef: gatewayv1alpha2.LocalPolicyTargetReference{ - Kind: "InvalidKind", - Group: "gateway.networking.k8s.io", + name: "Validate Invalid TargetRef Kind is not allowed", + wantErrors: []string{"TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'"}, + policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ + TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ + Kind: "InvalidKind", + Group: "gateway.networking.k8s.io", + }, }, }, { - name: "Validate TargetRef is of an allowed kind", - wantErrors: "TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'", - targetRef: gatewayv1alpha2.LocalPolicyTargetReference{ - Kind: "TCPRoute", - Group: "gateway.networking.k8s.io", + name: "Validate TCPRoute TargetRef Kind is not allowed", + wantErrors: []string{"TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'"}, + policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ + TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ + Kind: "TCPRoute", + Group: "gateway.networking.k8s.io", + }, }, }, } @@ -125,18 +125,15 @@ func testInvalidTargetRefKind(t *testing.T, allowedKinds map[gatewayv1alpha2.Loc t.Run(tt.name, func(t *testing.T) { // Create a ClientSettingsPolicy with the targetRef from the test case. clientSettingsPolicy := &ngfAPIv1alpha1.ClientSettingsPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-policy", + Namespace: "default", + }, Spec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ - TargetRef: tt.targetRef, + TargetRef: tt.policySpec.TargetRef, }, } - - if _, ok := allowedKinds[clientSettingsPolicy.Spec.TargetRef]; !ok { - gotError := "TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'" - - if tt.wantErrors != gotError { - t.Errorf("Test %s failed: got error %q, want %q", tt.name, gotError, tt.wantErrors) - } - } + validateClientSettingsPolicy(t, clientSettingsPolicy, tt.wantErrors) }) } } @@ -223,3 +220,33 @@ func testInvalidTargetRefGroup(t *testing.T, allowedGroups map[gatewayv1alpha2.L }) } } + +func validateClientSettingsPolicy(t *testing.T, clientSettingsPolicy *ngfAPIv1alpha1.ClientSettingsPolicy, wantErrors []string) { + t.Helper() + + // Register API types with the runtime scheme + // This is necessary to create a fake client that can handle the custom resource. + scheme := runtime.NewScheme() + _ = ngfAPIv1alpha1.AddToScheme(scheme) + + // Create a fake client with the scheme + // This is used to simulate interactions with the Kubernetes API. + k8sClient := fake.NewClientBuilder().WithScheme(scheme).Build() + + err := k8sClient.Create(context.Background(), clientSettingsPolicy) + if err != nil { + t.Logf("Error creating ClientSettingsPolicy %q: %v", fmt.Sprintf("%v/%v", clientSettingsPolicy.Namespace, clientSettingsPolicy.Name), err) + } + + // if (len(wantErrors) != 0) != (err != nil) { + // t.Fatalf("Unexpected response while creating ClientSettingsPolicy %q; got err=\n%v\n;want error=%v", fmt.Sprintf("%v/%v", clientSettingsPolicy.Namespace, clientSettingsPolicy.Name), err, wantErrors) + // } + + if err != nil { + for _, wantError := range wantErrors { + if !strings.Contains(err.Error(), wantError) { + t.Errorf("missing expected error %q in %v", wantError, err) + } + } + } +} From cd9bada9c32750d7e56f31f3e2918a2cdd78022a Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Thu, 17 Jul 2025 10:52:32 +0100 Subject: [PATCH 09/46] Update TargetRegGroup tests --- tests/cel/clientsettingspolicy_test.go | 107 ++++++++++++------------- 1 file changed, 52 insertions(+), 55 deletions(-) diff --git a/tests/cel/clientsettingspolicy_test.go b/tests/cel/clientsettingspolicy_test.go index 342b967cfd..a34b67d355 100644 --- a/tests/cel/clientsettingspolicy_test.go +++ b/tests/cel/clientsettingspolicy_test.go @@ -25,16 +25,12 @@ func TestClientSettingsPoliciesTargetRefKind(t *testing.T) { } func TestClientSettingsPoliciesTargetRefGroup(t *testing.T) { - // Test valid and invalid TargetRef Group - - allowedGroups := map[gatewayv1alpha2.LocalPolicyTargetReference]bool{ - { - Group: "gateway.networking.k8s.io", - }: true, - } - - testValidTargetRefGroup(t, allowedGroups) - testInvalidTargetRefGroup(t, allowedGroups) + // Test valid TargetRef Group + // Valid group is: "gateway.networking.k8s.io" + testValidTargetRefGroup(t) + // Invalid groups should return an error + // Examples of invalid groups: "invalid.networking.k8s.io", "discovery.k8s.io/v1" + testInvalidTargetRefGroup(t) } func testValidTargetRefKind(t *testing.T) { @@ -138,19 +134,22 @@ func testInvalidTargetRefKind(t *testing.T) { } } -func testValidTargetRefGroup(t *testing.T, allowedGroups map[gatewayv1alpha2.LocalPolicyTargetReference]bool) { +func testValidTargetRefGroup(t *testing.T) { t.Helper() tests := []struct { - name string - wantErrors string - targetRefGroup gatewayv1alpha2.LocalPolicyTargetReference + name string + wantErrors []string + policySpec ngfAPIv1alpha1.ClientSettingsPolicySpec }{ { - name: "Validate TargetRef group is gateway.networking.k8s.io", - wantErrors: "TargetRef Group must be gateway.networking.k8s.io", - targetRefGroup: gatewayv1alpha2.LocalPolicyTargetReference{ - Group: "gateway.networking.k8s.io", + name: "Validate gateway.networking.k8s.io TargetRef Group is allowed", + wantErrors: []string{"TargetRef Group must be gateway.networking.k8s.io."}, + policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ + TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ + Kind: "Gateway", + Group: "gateway.networking.k8s.io", + }, }, }, } @@ -159,64 +158,62 @@ func testValidTargetRefGroup(t *testing.T, allowedGroups map[gatewayv1alpha2.Loc t.Run(tt.name, func(t *testing.T) { // Create a ClientSettingsPolicy with the targetRef from the test case. clientSettingsPolicy := &ngfAPIv1alpha1.ClientSettingsPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-policy", + Namespace: "default", + }, Spec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ - TargetRef: tt.targetRefGroup, + TargetRef: tt.policySpec.TargetRef, }, } - - if _, ok := allowedGroups[clientSettingsPolicy.Spec.TargetRef]; !ok { - gotError := "TargetRef Group must be gateway.networking.k8s.io" - - if tt.wantErrors == gotError { - t.Errorf("Test %s failed: got error %q, want %q", tt.name, gotError, tt.wantErrors) - } - } + validateClientSettingsPolicy(t, clientSettingsPolicy, tt.wantErrors) }) } } -func testInvalidTargetRefGroup(t *testing.T, allowedGroups map[gatewayv1alpha2.LocalPolicyTargetReference]bool) { +func testInvalidTargetRefGroup(t *testing.T) { t.Helper() tests := []struct { - name string - wantErrors string - targetRefGroup gatewayv1alpha2.LocalPolicyTargetReference + name string + wantErrors []string + policySpec ngfAPIv1alpha1.ClientSettingsPolicySpec }{ { - name: "Validate TargetRef group is gateway.networking.k8s.io", - wantErrors: "TargetRef Group must be gateway.networking.k8s.io", - targetRefGroup: gatewayv1alpha2.LocalPolicyTargetReference{ - Group: "invalid.networking.k8s.io", + name: "Validate invalid.networking.k8s.io TargetRef Group is not allowed", + wantErrors: []string{"TargetRef Group must be gateway.networking.k8s.io."}, + policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ + TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ + Kind: "Gateway", + Group: "invalid.networking.k8s.io", + }, }, }, { - name: "Validate TargetRef is of an allowed kind", - wantErrors: "TargetRef Group must be gateway.networking.k8s.io", - targetRefGroup: gatewayv1alpha2.LocalPolicyTargetReference{ - Group: "discovery.k8s.io/v1", + name: "Validate discovery.k8s.io/v1 TargetRef Group is not allowed", + wantErrors: []string{"TargetRef Group must be gateway.networking.k8s.io."}, + policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ + TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ + Kind: "Gateway", + Group: "discovery.k8s.io/v1", + }, }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - t.Run(tt.name, func(t *testing.T) { - // Create a ClientSettingsPolicy with the targetRef from the test case. - clientSettingsPolicy := &ngfAPIv1alpha1.ClientSettingsPolicy{ - Spec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ - TargetRef: tt.targetRefGroup, - }, - } - - if _, ok := allowedGroups[clientSettingsPolicy.Spec.TargetRef]; !ok { - gotError := "TargetRef Group must be gateway.networking.k8s.io" - - if tt.wantErrors != gotError { - t.Errorf("Test %s failed: got error %q, want %q", tt.name, gotError, tt.wantErrors) - } - } - }) + // Create a ClientSettingsPolicy with the targetRef from the test case. + clientSettingsPolicy := &ngfAPIv1alpha1.ClientSettingsPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-policy", + Namespace: "default", + }, + Spec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ + TargetRef: tt.policySpec.TargetRef, + }, + } + validateClientSettingsPolicy(t, clientSettingsPolicy, tt.wantErrors) }) } } From 7d9989debc745328529909077d2d7294ca227bfc Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Thu, 17 Jul 2025 10:57:10 +0100 Subject: [PATCH 10/46] Fix lint errors --- tests/cel/clientsettingspolicy_test.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/cel/clientsettingspolicy_test.go b/tests/cel/clientsettingspolicy_test.go index a34b67d355..a0d1aaf31b 100644 --- a/tests/cel/clientsettingspolicy_test.go +++ b/tests/cel/clientsettingspolicy_test.go @@ -37,9 +37,9 @@ func testValidTargetRefKind(t *testing.T) { t.Helper() tests := []struct { + policySpec ngfAPIv1alpha1.ClientSettingsPolicySpec name string wantErrors []string - policySpec ngfAPIv1alpha1.ClientSettingsPolicySpec }{ { name: "Validate TargetRef of kind Gateway is allowed", @@ -91,9 +91,9 @@ func testInvalidTargetRefKind(t *testing.T) { t.Helper() tests := []struct { + policySpec ngfAPIv1alpha1.ClientSettingsPolicySpec name string wantErrors []string - policySpec ngfAPIv1alpha1.ClientSettingsPolicySpec }{ { name: "Validate Invalid TargetRef Kind is not allowed", @@ -138,9 +138,9 @@ func testValidTargetRefGroup(t *testing.T) { t.Helper() tests := []struct { + policySpec ngfAPIv1alpha1.ClientSettingsPolicySpec name string wantErrors []string - policySpec ngfAPIv1alpha1.ClientSettingsPolicySpec }{ { name: "Validate gateway.networking.k8s.io TargetRef Group is allowed", @@ -175,9 +175,9 @@ func testInvalidTargetRefGroup(t *testing.T) { t.Helper() tests := []struct { + policySpec ngfAPIv1alpha1.ClientSettingsPolicySpec name string wantErrors []string - policySpec ngfAPIv1alpha1.ClientSettingsPolicySpec }{ { name: "Validate invalid.networking.k8s.io TargetRef Group is not allowed", @@ -218,7 +218,10 @@ func testInvalidTargetRefGroup(t *testing.T) { } } -func validateClientSettingsPolicy(t *testing.T, clientSettingsPolicy *ngfAPIv1alpha1.ClientSettingsPolicy, wantErrors []string) { +func validateClientSettingsPolicy(t *testing.T, + clientSettingsPolicy *ngfAPIv1alpha1.ClientSettingsPolicy, + wantErrors []string, +) { t.Helper() // Register API types with the runtime scheme @@ -232,13 +235,10 @@ func validateClientSettingsPolicy(t *testing.T, clientSettingsPolicy *ngfAPIv1al err := k8sClient.Create(context.Background(), clientSettingsPolicy) if err != nil { - t.Logf("Error creating ClientSettingsPolicy %q: %v", fmt.Sprintf("%v/%v", clientSettingsPolicy.Namespace, clientSettingsPolicy.Name), err) + t.Logf("Error creating ClientSettingsPolicy %q: %v", + fmt.Sprintf("%v/%v", clientSettingsPolicy.Namespace, clientSettingsPolicy.Name), err) } - // if (len(wantErrors) != 0) != (err != nil) { - // t.Fatalf("Unexpected response while creating ClientSettingsPolicy %q; got err=\n%v\n;want error=%v", fmt.Sprintf("%v/%v", clientSettingsPolicy.Namespace, clientSettingsPolicy.Name), err, wantErrors) - // } - if err != nil { for _, wantError := range wantErrors { if !strings.Contains(err.Error(), wantError) { From 9274e57993981759cccabcb4e4b14900e69b992c Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Mon, 21 Jul 2025 13:18:46 +0100 Subject: [PATCH 11/46] Group valid and invalid test cases into single test function --- tests/cel/clientsettingspolicy_test.go | 79 +------------------------- 1 file changed, 2 insertions(+), 77 deletions(-) diff --git a/tests/cel/clientsettingspolicy_test.go b/tests/cel/clientsettingspolicy_test.go index a0d1aaf31b..7fc9abb9af 100644 --- a/tests/cel/clientsettingspolicy_test.go +++ b/tests/cel/clientsettingspolicy_test.go @@ -16,25 +16,6 @@ import ( ) func TestClientSettingsPoliciesTargetRefKind(t *testing.T) { - // Test valid and invalid TargetRef Kind - // Valid kinds are: Gateway, HTTPRoute, GRPCRoute - testValidTargetRefKind(t) - // Invalid kinds should return an error - // Example of an invalid kind: "InvalidKind", "TCPRoute" - testInvalidTargetRefKind(t) -} - -func TestClientSettingsPoliciesTargetRefGroup(t *testing.T) { - // Test valid TargetRef Group - // Valid group is: "gateway.networking.k8s.io" - testValidTargetRefGroup(t) - // Invalid groups should return an error - // Examples of invalid groups: "invalid.networking.k8s.io", "discovery.k8s.io/v1" - testInvalidTargetRefGroup(t) -} - -func testValidTargetRefKind(t *testing.T) { - t.Helper() tests := []struct { policySpec ngfAPIv1alpha1.ClientSettingsPolicySpec @@ -68,33 +49,6 @@ func testValidTargetRefKind(t *testing.T) { }, }, }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Create a ClientSettingsPolicy with the targetRef from the test case. - clientSettingsPolicy := &ngfAPIv1alpha1.ClientSettingsPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-policy", - Namespace: "default", - }, - Spec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ - TargetRef: tt.policySpec.TargetRef, - }, - } - validateClientSettingsPolicy(t, clientSettingsPolicy, tt.wantErrors) - }) - } -} - -func testInvalidTargetRefKind(t *testing.T) { - t.Helper() - - tests := []struct { - policySpec ngfAPIv1alpha1.ClientSettingsPolicySpec - name string - wantErrors []string - }{ { name: "Validate Invalid TargetRef Kind is not allowed", wantErrors: []string{"TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'"}, @@ -134,8 +88,7 @@ func testInvalidTargetRefKind(t *testing.T) { } } -func testValidTargetRefGroup(t *testing.T) { - t.Helper() +func TestClientSettingsPoliciesTargetRefGroup(t *testing.T) { tests := []struct { policySpec ngfAPIv1alpha1.ClientSettingsPolicySpec @@ -143,8 +96,7 @@ func testValidTargetRefGroup(t *testing.T) { wantErrors []string }{ { - name: "Validate gateway.networking.k8s.io TargetRef Group is allowed", - wantErrors: []string{"TargetRef Group must be gateway.networking.k8s.io."}, + name: "Validate gateway.networking.k8s.io TargetRef Group is allowed", policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ Kind: "Gateway", @@ -152,33 +104,6 @@ func testValidTargetRefGroup(t *testing.T) { }, }, }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Create a ClientSettingsPolicy with the targetRef from the test case. - clientSettingsPolicy := &ngfAPIv1alpha1.ClientSettingsPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-policy", - Namespace: "default", - }, - Spec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ - TargetRef: tt.policySpec.TargetRef, - }, - } - validateClientSettingsPolicy(t, clientSettingsPolicy, tt.wantErrors) - }) - } -} - -func testInvalidTargetRefGroup(t *testing.T) { - t.Helper() - - tests := []struct { - policySpec ngfAPIv1alpha1.ClientSettingsPolicySpec - name string - wantErrors []string - }{ { name: "Validate invalid.networking.k8s.io TargetRef Group is not allowed", wantErrors: []string{"TargetRef Group must be gateway.networking.k8s.io."}, From f99ab29e3d454cf6c8be5e14d33fe7d00bd8ab6d Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Mon, 21 Jul 2025 13:23:33 +0100 Subject: [PATCH 12/46] Revert dependency version --- tests/go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/go.mod b/tests/go.mod index 03a48f1386..3cb2eea54d 100644 --- a/tests/go.mod +++ b/tests/go.mod @@ -5,7 +5,7 @@ go 1.24.2 replace github.com/nginx/nginx-gateway-fabric => ../ require ( - github.com/nginx/nginx-gateway-fabric v1.6.2 + github.com/nginx/nginx-gateway-fabric v0.0.0 github.com/onsi/ginkgo/v2 v2.23.4 github.com/onsi/gomega v1.37.0 github.com/prometheus/client_golang v1.22.0 From ad80b703c2abb8458a3976ebd978fe0cef2a6b25 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Mon, 21 Jul 2025 13:24:25 +0100 Subject: [PATCH 13/46] Move imports --- tests/cel/clientsettingspolicy_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/cel/clientsettingspolicy_test.go b/tests/cel/clientsettingspolicy_test.go index 7fc9abb9af..e6832ad3b2 100644 --- a/tests/cel/clientsettingspolicy_test.go +++ b/tests/cel/clientsettingspolicy_test.go @@ -9,7 +9,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/client/fake" - gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" ngfAPIv1alpha1 "github.com/nginx/nginx-gateway-fabric/apis/v1alpha1" From e5479fb0c0ac9789a10d40b3a16a06358ce65d24 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Mon, 21 Jul 2025 13:30:28 +0100 Subject: [PATCH 14/46] Define constants for Kinds and Groups --- tests/cel/clientsettingspolicy_test.go | 59 +++++++++++++++++--------- 1 file changed, 39 insertions(+), 20 deletions(-) diff --git a/tests/cel/clientsettingspolicy_test.go b/tests/cel/clientsettingspolicy_test.go index e6832ad3b2..ac0e92c67c 100644 --- a/tests/cel/clientsettingspolicy_test.go +++ b/tests/cel/clientsettingspolicy_test.go @@ -14,6 +14,25 @@ import ( ngfAPIv1alpha1 "github.com/nginx/nginx-gateway-fabric/apis/v1alpha1" ) +const ( + GatewayKind = "Gateway" + HTTPRouteKind = "HTTPRoute" + GRPCRouteKind = "GRPCRoute" + TCPRouteKind = "TCPRoute" + InvalidKind = "InvalidKind" +) + +const ( + GatewayGroup = "gateway.networking.k8s.io" + InvalidGroup = "invalid.networking.k8s.io" + DiscoveryGroup = "discovery.k8s.io/v1" +) + +const ( + ExpectedTargetRefKindError = "TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'" + ExpectedTargetRefGroupError = "TargetRef Group must be gateway.networking.k8s.io." +) + func TestClientSettingsPoliciesTargetRefKind(t *testing.T) { tests := []struct { @@ -25,8 +44,8 @@ func TestClientSettingsPoliciesTargetRefKind(t *testing.T) { name: "Validate TargetRef of kind Gateway is allowed", policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ - Kind: "Gateway", - Group: "gateway.networking.k8s.io", + Kind: GatewayKind, + Group: GatewayGroup, }, }, }, @@ -34,8 +53,8 @@ func TestClientSettingsPoliciesTargetRefKind(t *testing.T) { name: "Validate TargetRef of kind HTTPRoute is allowed", policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ - Kind: "HTTPRoute", - Group: "gateway.networking.k8s.io", + Kind: HTTPRouteKind, + Group: GatewayGroup, }, }, }, @@ -43,28 +62,28 @@ func TestClientSettingsPoliciesTargetRefKind(t *testing.T) { name: "Validate TargetRef of kind GRPCRoute is allowed", policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ - Kind: "GRPCRoute", - Group: "gateway.networking.k8s.io", + Kind: GRPCRouteKind, + Group: GatewayGroup, }, }, }, { name: "Validate Invalid TargetRef Kind is not allowed", - wantErrors: []string{"TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'"}, + wantErrors: []string{ExpectedTargetRefKindError}, policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ - Kind: "InvalidKind", - Group: "gateway.networking.k8s.io", + Kind: InvalidKind, + Group: GatewayGroup, }, }, }, { name: "Validate TCPRoute TargetRef Kind is not allowed", - wantErrors: []string{"TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'"}, + wantErrors: []string{ExpectedTargetRefKindError}, policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ - Kind: "TCPRoute", - Group: "gateway.networking.k8s.io", + Kind: TCPRouteKind, + Group: GatewayGroup, }, }, }, @@ -98,28 +117,28 @@ func TestClientSettingsPoliciesTargetRefGroup(t *testing.T) { name: "Validate gateway.networking.k8s.io TargetRef Group is allowed", policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ - Kind: "Gateway", - Group: "gateway.networking.k8s.io", + Kind: GatewayKind, + Group: GatewayGroup, }, }, }, { name: "Validate invalid.networking.k8s.io TargetRef Group is not allowed", - wantErrors: []string{"TargetRef Group must be gateway.networking.k8s.io."}, + wantErrors: []string{ExpectedTargetRefGroupError}, policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ - Kind: "Gateway", - Group: "invalid.networking.k8s.io", + Kind: GatewayKind, + Group: InvalidGroup, }, }, }, { name: "Validate discovery.k8s.io/v1 TargetRef Group is not allowed", - wantErrors: []string{"TargetRef Group must be gateway.networking.k8s.io."}, + wantErrors: []string{ExpectedTargetRefGroupError}, policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ - Kind: "Gateway", - Group: "discovery.k8s.io/v1", + Kind: GatewayKind, + Group: DiscoveryGroup, }, }, }, From aa4a6b22802984cc95cc66fcb3e96f81418b7457 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Tue, 22 Jul 2025 14:42:53 +0100 Subject: [PATCH 15/46] Use controller-runtime library to get cluster information --- tests/cel/clientsettingspolicy_test.go | 71 ++++++++++++++++++++------ 1 file changed, 56 insertions(+), 15 deletions(-) diff --git a/tests/cel/clientsettingspolicy_test.go b/tests/cel/clientsettingspolicy_test.go index ac0e92c67c..7016c3746d 100644 --- a/tests/cel/clientsettingspolicy_test.go +++ b/tests/cel/clientsettingspolicy_test.go @@ -5,10 +5,12 @@ import ( "fmt" "strings" "testing" + "time" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "sigs.k8s.io/controller-runtime/pkg/client/fake" + 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/apis/v1alpha1" @@ -167,26 +169,65 @@ func validateClientSettingsPolicy(t *testing.T, ) { t.Helper() - // Register API types with the runtime scheme - // This is necessary to create a fake client that can handle the custom resource. - scheme := runtime.NewScheme() - _ = ngfAPIv1alpha1.AddToScheme(scheme) + // Get Kubernetes client from test framework + // This should be set up by your test framework to connect to a real cluster + k8sClient := getKubernetesClient(t) - // Create a fake client with the scheme - // This is used to simulate interactions with the Kubernetes API. - k8sClient := fake.NewClientBuilder().WithScheme(scheme).Build() + // Make policy name unique to avoid conflicts + clientSettingsPolicy.Name = fmt.Sprintf("%s-%d", clientSettingsPolicy.Name, time.Now().UnixNano()) err := k8sClient.Create(context.Background(), clientSettingsPolicy) + + // Clean up after test + defer func() { + _ = k8sClient.Delete(context.Background(), clientSettingsPolicy) + }() + + // Check if we expected errors + if len(wantErrors) == 0 { + if err != nil { + t.Errorf("expected no error but got: %v", err) + } + return + } + + // We expected errors - validation should have failed + if err == nil { + t.Errorf("expected validation error but policy was accepted") + return + } + + // Check that we got the expected error messages + var missingErrors []string + for _, wantError := range wantErrors { + if !strings.Contains(err.Error(), wantError) { + missingErrors = append(missingErrors, wantError) + } + } + if len(missingErrors) != 0 { + t.Errorf("missing expected errors: %v, got: %v", missingErrors, err) + } +} + +// getKubernetesClient returns a client connected to a real Kubernetes cluster +func getKubernetesClient(t *testing.T) client.Client { + // Use controller-runtime to get cluster connection + k8sConfig, err := controllerruntime.GetConfig() if err != nil { - t.Logf("Error creating ClientSettingsPolicy %q: %v", - fmt.Sprintf("%v/%v", clientSettingsPolicy.Namespace, clientSettingsPolicy.Name), err) + t.Skipf("Cannot connect to Kubernetes cluster: %v", err) } + // Set up scheme with NGF types + scheme := runtime.NewScheme() + if err := ngfAPIv1alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("Failed to add NGF schema: %v", err) + } + + // Create client + k8sClient, err := client.New(k8sConfig, client.Options{Scheme: scheme}) if err != nil { - for _, wantError := range wantErrors { - if !strings.Contains(err.Error(), wantError) { - t.Errorf("missing expected error %q in %v", wantError, err) - } - } + t.Skipf("Cannot create k8s client: %v", err) } + + return k8sClient } From 8f74214e3c007f08ef395ae9e6173ab2766e3c92 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Tue, 22 Jul 2025 15:36:27 +0100 Subject: [PATCH 16/46] Add Makefile targets to run CEL test and re format test --- tests/Makefile | 20 ++++++++++ tests/cel/clientsettingspolicy_test.go | 52 ++++++++++---------------- 2 files changed, 39 insertions(+), 33 deletions(-) diff --git a/tests/Makefile b/tests/Makefile index 1e986c4984..e13195647c 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -17,6 +17,7 @@ STANDARD_CONFORMANCE_PROFILES = GATEWAY-HTTP,GATEWAY-GRPC EXPERIMENTAL_CONFORMANCE_PROFILES = GATEWAY-TLS CONFORMANCE_PROFILES = $(STANDARD_CONFORMANCE_PROFILES) # by default we use the standard conformance profiles. If experimental is enabled we override this and add the experimental profiles. SKIP_TESTS = +CEL_TEST_TARGET = # Check if ENABLE_EXPERIMENTAL is true ifeq ($(ENABLE_EXPERIMENTAL),true) @@ -186,3 +187,22 @@ uninstall-ngf: ## Uninstall NGF on configured kind cluster -make uninstall-gateway-crds -kubectl delete namespace nginx-gateway -kubectl kustomize ../config/crd | kubectl delete -f - + +# Run CEL validation integration tests against a real cluster +.PHONY: test-cel-validation +test-cel-validation: + @if [ -z "$(CEL_TEST_TARGET)" ]; then \ + echo "Running all tests in ./cel"; \ + go test -v ./cel; \ + else \ + echo "Running test: $(CEL_TEST_TARGET) in ./cel"; \ + go test -v ./cel -run "$(CEL_TEST_TARGET)"; \ + fi + + +# Set up a kind cluster, install NGF CRDs, and run CEL validation tests +.PHONY: setup-and-test-cel-validation +setup-and-test-cel-validation: + kind create cluster --name $(CLUSTER_NAME) || true + kubectl kustomize ../config/crd | kubectl apply -f - + $(MAKE) test-cel-validation diff --git a/tests/cel/clientsettingspolicy_test.go b/tests/cel/clientsettingspolicy_test.go index 7016c3746d..cf6dc8c80a 100644 --- a/tests/cel/clientsettingspolicy_test.go +++ b/tests/cel/clientsettingspolicy_test.go @@ -7,7 +7,6 @@ import ( "testing" "time" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" controllerruntime "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -36,7 +35,6 @@ const ( ) func TestClientSettingsPoliciesTargetRefKind(t *testing.T) { - tests := []struct { policySpec ngfAPIv1alpha1.ClientSettingsPolicySpec name string @@ -93,23 +91,12 @@ func TestClientSettingsPoliciesTargetRefKind(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // Create a ClientSettingsPolicy with the targetRef from the test case. - clientSettingsPolicy := &ngfAPIv1alpha1.ClientSettingsPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-policy", - Namespace: "default", - }, - Spec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ - TargetRef: tt.policySpec.TargetRef, - }, - } - validateClientSettingsPolicy(t, clientSettingsPolicy, tt.wantErrors) + validateClientSettingsPolicy(t, tt) }) } } func TestClientSettingsPoliciesTargetRefGroup(t *testing.T) { - tests := []struct { policySpec ngfAPIv1alpha1.ClientSettingsPolicySpec name string @@ -148,24 +135,16 @@ func TestClientSettingsPoliciesTargetRefGroup(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // Create a ClientSettingsPolicy with the targetRef from the test case. - clientSettingsPolicy := &ngfAPIv1alpha1.ClientSettingsPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-policy", - Namespace: "default", - }, - Spec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ - TargetRef: tt.policySpec.TargetRef, - }, - } - validateClientSettingsPolicy(t, clientSettingsPolicy, tt.wantErrors) + validateClientSettingsPolicy(t, tt) }) } } -func validateClientSettingsPolicy(t *testing.T, - clientSettingsPolicy *ngfAPIv1alpha1.ClientSettingsPolicy, - wantErrors []string, +func validateClientSettingsPolicy(t *testing.T, tt struct { + policySpec ngfAPIv1alpha1.ClientSettingsPolicySpec + name string + wantErrors []string +}, ) { t.Helper() @@ -173,8 +152,14 @@ func validateClientSettingsPolicy(t *testing.T, // This should be set up by your test framework to connect to a real cluster k8sClient := getKubernetesClient(t) - // Make policy name unique to avoid conflicts - clientSettingsPolicy.Name = fmt.Sprintf("%s-%d", clientSettingsPolicy.Name, time.Now().UnixNano()) + clientSettingsPolicy := &ngfAPIv1alpha1.ClientSettingsPolicy{ + ObjectMeta: controllerruntime.ObjectMeta{ + // Create a unique name and namespace for the policy to avoid conflicts + Name: fmt.Sprintf("test-policy-%d", time.Now().UnixNano()), + Namespace: "default", + }, + Spec: tt.policySpec, + } err := k8sClient.Create(context.Background(), clientSettingsPolicy) @@ -184,7 +169,7 @@ func validateClientSettingsPolicy(t *testing.T, }() // Check if we expected errors - if len(wantErrors) == 0 { + if len(tt.wantErrors) == 0 { if err != nil { t.Errorf("expected no error but got: %v", err) } @@ -199,7 +184,7 @@ func validateClientSettingsPolicy(t *testing.T, // Check that we got the expected error messages var missingErrors []string - for _, wantError := range wantErrors { + for _, wantError := range tt.wantErrors { if !strings.Contains(err.Error(), wantError) { missingErrors = append(missingErrors, wantError) } @@ -209,8 +194,9 @@ func validateClientSettingsPolicy(t *testing.T, } } -// getKubernetesClient returns a client connected to a real Kubernetes cluster +// getKubernetesClient returns a client connected to a real Kubernetes cluster. func getKubernetesClient(t *testing.T) client.Client { + t.Helper() // Use controller-runtime to get cluster connection k8sConfig, err := controllerruntime.GetConfig() if err != nil { From 4486bd5e17a63597ec13b5e534eb7672e6bafed7 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Tue, 22 Jul 2025 15:50:05 +0100 Subject: [PATCH 17/46] Ensure TestClientSettingsPoliciesTargetRefKind and TestClientSettingsPoliciesTargetRefGroup validate each test correctly --- tests/cel/clientsettingspolicy_test.go | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/tests/cel/clientsettingspolicy_test.go b/tests/cel/clientsettingspolicy_test.go index cf6dc8c80a..facef3695b 100644 --- a/tests/cel/clientsettingspolicy_test.go +++ b/tests/cel/clientsettingspolicy_test.go @@ -30,8 +30,13 @@ const ( ) const ( - ExpectedTargetRefKindError = "TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute'" - ExpectedTargetRefGroupError = "TargetRef Group must be gateway.networking.k8s.io." + ExpectedTargetRefKindError = `TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute` + ExpectedTargetRefGroupError = `TargetRef Group must be gateway.networking.k8s.io.` +) + +const ( + PolicyName = "test-policy" + TargetRefFormat = "targetRef-name-%d" ) func TestClientSettingsPoliciesTargetRefKind(t *testing.T) { @@ -152,13 +157,15 @@ func validateClientSettingsPolicy(t *testing.T, tt struct { // This should be set up by your test framework to connect to a real cluster k8sClient := getKubernetesClient(t) + policySpec := tt.policySpec + policySpec.TargetRef.Name = gatewayv1alpha2.ObjectName(fmt.Sprintf(TargetRefFormat, time.Now().UnixNano())) + clientSettingsPolicy := &ngfAPIv1alpha1.ClientSettingsPolicy{ ObjectMeta: controllerruntime.ObjectMeta{ - // Create a unique name and namespace for the policy to avoid conflicts - Name: fmt.Sprintf("test-policy-%d", time.Now().UnixNano()), + Name: PolicyName, Namespace: "default", }, - Spec: tt.policySpec, + Spec: policySpec, } err := k8sClient.Create(context.Background(), clientSettingsPolicy) From 29f26ef15c8308eb90a72d180f99f3926a1e69cc Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Mon, 28 Jul 2025 10:48:38 +0100 Subject: [PATCH 18/46] Update test to use `gomega` test library --- tests/cel/clientsettingspolicy_test.go | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/tests/cel/clientsettingspolicy_test.go b/tests/cel/clientsettingspolicy_test.go index facef3695b..a5fc0520b0 100644 --- a/tests/cel/clientsettingspolicy_test.go +++ b/tests/cel/clientsettingspolicy_test.go @@ -3,10 +3,10 @@ package cel import ( "context" "fmt" - "strings" "testing" "time" + . "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/runtime" controllerruntime "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -188,16 +188,10 @@ func validateClientSettingsPolicy(t *testing.T, tt struct { t.Errorf("expected validation error but policy was accepted") return } - + g := NewWithT(t) // Check that we got the expected error messages - var missingErrors []string for _, wantError := range tt.wantErrors { - if !strings.Contains(err.Error(), wantError) { - missingErrors = append(missingErrors, wantError) - } - } - if len(missingErrors) != 0 { - t.Errorf("missing expected errors: %v, got: %v", missingErrors, err) + g.Expect(err.Error()).To(ContainSubstring(wantError), "Expected error '%s' not found in: %s", wantError, err.Error()) } } From f26eea39a23bd577c42398621e20a677638c7527 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Mon, 28 Jul 2025 11:49:46 +0100 Subject: [PATCH 19/46] Add ci workflow job for cel tests --- .github/workflows/ci.yml | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9d47d85df5..52229970c0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -311,3 +311,34 @@ jobs: - name: Push to GitHub Container Registry run: | helm push ${{ steps.package.outputs.path }} oci://ghcr.io/nginx/charts + + cel-tests: + name: CEL Tests + runs-on: ubuntu-24.04 + needs: vars + steps: + - name: Checkout Repository + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + + - name: Setup Golang Environment + uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 # v5.5.0 + with: + go-version: stable + cache-dependency-path: | + go.sum + .github/.cache/buster-for-unit-tests + + - name: Run Tests + run: make -C tests test-cel-validation + + - name: Upload coverage reports to Codecov + uses: codecov/codecov-action@18283e04ce6e62d37312384ff67231eb8fd56d24 # v5.4.3 + with: + token: ${{ secrets.CODECOV_TOKEN }} + + - name: Upload Coverage Report + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + with: + name: cover-${{ github.run_id }}.html + path: ${{ github.workspace }}/cover.html + if: always() \ No newline at end of file From efa4a81f20c6e9d8680929339e0a27b5214ea6c7 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Mon, 28 Jul 2025 11:52:41 +0100 Subject: [PATCH 20/46] Add new line --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 52229970c0..fd088a0c8c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -341,4 +341,4 @@ jobs: with: name: cover-${{ github.run_id }}.html path: ${{ github.workspace }}/cover.html - if: always() \ No newline at end of file + if: always() From d25661afcc798f0fee33909f7a81d3eae8dbde6b Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Mon, 28 Jul 2025 14:15:32 +0100 Subject: [PATCH 21/46] Add `Deploy Kubernetes` step --- .github/workflows/ci.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fd088a0c8c..4341c3ce39 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -327,6 +327,11 @@ jobs: cache-dependency-path: | go.sum .github/.cache/buster-for-unit-tests + + - name: Deploy Kubernetes + id: k8s + run: | + kind create cluster --name ${{ github.run_id }} --image=kindest/node:${{ needs.vars.outputs.k8s_latest }} - name: Run Tests run: make -C tests test-cel-validation From c41389544d3b30f85770f4a00e21011a01d60588 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Mon, 28 Jul 2025 14:30:51 +0100 Subject: [PATCH 22/46] Add step to apply CRDs in cel-tests CI job --- .github/workflows/ci.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4341c3ce39..ad0d47db76 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -327,12 +327,16 @@ jobs: cache-dependency-path: | go.sum .github/.cache/buster-for-unit-tests - + - name: Deploy Kubernetes id: k8s run: | kind create cluster --name ${{ github.run_id }} --image=kindest/node:${{ needs.vars.outputs.k8s_latest }} + - name: Apply CustomResourceDefinition + run: | + kubectl kustomize config/crd | kubectl apply -f - + - name: Run Tests run: make -C tests test-cel-validation From 9962e03103bf0f155f65e0e2b303b46db0c9dec9 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Mon, 28 Jul 2025 15:02:24 +0100 Subject: [PATCH 23/46] Add `--server-side` to apply command --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ad0d47db76..dcf7657084 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -335,7 +335,7 @@ jobs: - name: Apply CustomResourceDefinition run: | - kubectl kustomize config/crd | kubectl apply -f - + kubectl kustomize config/crd | kubectl apply --server-side -f - - name: Run Tests run: make -C tests test-cel-validation From feca4edd9342bb9af9a1fd194332fb273e63b1e3 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Mon, 28 Jul 2025 16:10:03 +0100 Subject: [PATCH 24/46] Add `working-directory` to test run and remove code coverage steps --- .github/workflows/ci.yml | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dcf7657084..fdd036588d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -338,16 +338,5 @@ jobs: kubectl kustomize config/crd | kubectl apply --server-side -f - - name: Run Tests - run: make -C tests test-cel-validation - - - name: Upload coverage reports to Codecov - uses: codecov/codecov-action@18283e04ce6e62d37312384ff67231eb8fd56d24 # v5.4.3 - with: - token: ${{ secrets.CODECOV_TOKEN }} - - - name: Upload Coverage Report - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 - with: - name: cover-${{ github.run_id }}.html - path: ${{ github.workspace }}/cover.html - if: always() + run: make test-cel-validation + working-directory: ./tests From d518ada3cd7c54fbfda916788eacfa3d66990a7c Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Tue, 29 Jul 2025 10:25:28 +0100 Subject: [PATCH 25/46] Add t.Parallel() --- tests/cel/clientsettingspolicy_test.go | 4 ++++ tests/go.mod | 1 + tests/go.sum | 2 ++ 3 files changed, 7 insertions(+) diff --git a/tests/cel/clientsettingspolicy_test.go b/tests/cel/clientsettingspolicy_test.go index a5fc0520b0..416e96245f 100644 --- a/tests/cel/clientsettingspolicy_test.go +++ b/tests/cel/clientsettingspolicy_test.go @@ -40,6 +40,7 @@ const ( ) func TestClientSettingsPoliciesTargetRefKind(t *testing.T) { + t.Parallel() tests := []struct { policySpec ngfAPIv1alpha1.ClientSettingsPolicySpec name string @@ -96,12 +97,14 @@ func TestClientSettingsPoliciesTargetRefKind(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + t.Parallel() validateClientSettingsPolicy(t, tt) }) } } func TestClientSettingsPoliciesTargetRefGroup(t *testing.T) { + t.Parallel() tests := []struct { policySpec ngfAPIv1alpha1.ClientSettingsPolicySpec name string @@ -140,6 +143,7 @@ func TestClientSettingsPoliciesTargetRefGroup(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + t.Parallel() validateClientSettingsPolicy(t, tt) }) } diff --git a/tests/go.mod b/tests/go.mod index ae6165ac7c..a95e32bf64 100644 --- a/tests/go.mod +++ b/tests/go.mod @@ -5,6 +5,7 @@ go 1.24.2 replace github.com/nginx/nginx-gateway-fabric/v2 => ../ require ( + github.com/nginx/nginx-gateway-fabric v1.6.2 github.com/nginx/nginx-gateway-fabric/v2 v2.0.0 github.com/onsi/ginkgo/v2 v2.23.4 github.com/onsi/gomega v1.38.0 diff --git a/tests/go.sum b/tests/go.sum index 6eaad011d3..685931ed25 100644 --- a/tests/go.sum +++ b/tests/go.sum @@ -94,6 +94,8 @@ github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f h1:KUppIJq7/+ github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U= github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f h1:y5//uYreIhSUg3J1GEMiLbxo1LJaP8RfCpH6pymGZus= github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f/go.mod h1:ZdcZmHo+o7JKHSa8/e818NopupXU1YMK5fe1lsApnBw= +github.com/nginx/nginx-gateway-fabric v1.6.2 h1:ktdShWxT/Drh/5/u8S5QMRgnnBGvVuFDV10ttRk0W+c= +github.com/nginx/nginx-gateway-fabric v1.6.2/go.mod h1:Fi2hdmoNj9nQRX9YQDju+ntMPG4Fgcw+irfl/GYWSEk= github.com/onsi/ginkgo/v2 v2.23.4 h1:ktYTpKJAVZnDT4VjxSbiBenUjmlL/5QkBEocaWXiQus= github.com/onsi/ginkgo/v2 v2.23.4/go.mod h1:Bt66ApGPBFzHyR+JO10Zbt0Gsp4uWxu5mIOTusL46e8= github.com/onsi/gomega v1.38.0 h1:c/WX+w8SLAinvuKKQFh77WEucCnPk4j2OTUr7lt7BeY= From 39a6b384a5b8d360d0e9d687e47bb69f0d6acc7e Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Tue, 29 Jul 2025 10:42:33 +0100 Subject: [PATCH 26/46] Use g.Expect for errors and ensure unique policy names --- tests/cel/clientsettingspolicy_test.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/cel/clientsettingspolicy_test.go b/tests/cel/clientsettingspolicy_test.go index 416e96245f..909b79fade 100644 --- a/tests/cel/clientsettingspolicy_test.go +++ b/tests/cel/clientsettingspolicy_test.go @@ -35,8 +35,8 @@ const ( ) const ( - PolicyName = "test-policy" - TargetRefFormat = "targetRef-name-%d" + PolicyNameFormat = "test-policy-%d" + TargetRefFormat = "targetRef-name-%d" ) func TestClientSettingsPoliciesTargetRefKind(t *testing.T) { @@ -156,6 +156,7 @@ func validateClientSettingsPolicy(t *testing.T, tt struct { }, ) { t.Helper() + g := NewWithT(t) // Get Kubernetes client from test framework // This should be set up by your test framework to connect to a real cluster @@ -163,10 +164,11 @@ func validateClientSettingsPolicy(t *testing.T, tt struct { policySpec := tt.policySpec policySpec.TargetRef.Name = gatewayv1alpha2.ObjectName(fmt.Sprintf(TargetRefFormat, time.Now().UnixNano())) + policyName := fmt.Sprintf(PolicyNameFormat, time.Now().UnixNano()) clientSettingsPolicy := &ngfAPIv1alpha1.ClientSettingsPolicy{ ObjectMeta: controllerruntime.ObjectMeta{ - Name: PolicyName, + Name: policyName, Namespace: "default", }, Spec: policySpec, @@ -182,17 +184,17 @@ func validateClientSettingsPolicy(t *testing.T, tt struct { // Check if we expected errors if len(tt.wantErrors) == 0 { if err != nil { - t.Errorf("expected no error but got: %v", err) + g.Expect(err).ToNot(HaveOccurred()) } return } // We expected errors - validation should have failed if err == nil { - t.Errorf("expected validation error but policy was accepted") + g.Expect(err).To(HaveOccurred()) return } - g := NewWithT(t) + // Check that we got the expected error messages for _, wantError := range tt.wantErrors { g.Expect(err.Error()).To(ContainSubstring(wantError), "Expected error '%s' not found in: %s", wantError, err.Error()) From 5079c2166677531bf46bc0d4ddac2877354595ad Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Tue, 29 Jul 2025 10:49:59 +0100 Subject: [PATCH 27/46] Return and assert errors from `getKubernetesClient` --- tests/cel/clientsettingspolicy_test.go | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/tests/cel/clientsettingspolicy_test.go b/tests/cel/clientsettingspolicy_test.go index 909b79fade..2a12a47117 100644 --- a/tests/cel/clientsettingspolicy_test.go +++ b/tests/cel/clientsettingspolicy_test.go @@ -160,7 +160,9 @@ func validateClientSettingsPolicy(t *testing.T, tt struct { // Get Kubernetes client from test framework // This should be set up by your test framework to connect to a real cluster - k8sClient := getKubernetesClient(t) + k8sClient, err := getKubernetesClient(t) + + g.Expect(err).ToNot(HaveOccurred()) policySpec := tt.policySpec policySpec.TargetRef.Name = gatewayv1alpha2.ObjectName(fmt.Sprintf(TargetRefFormat, time.Now().UnixNano())) @@ -174,7 +176,7 @@ func validateClientSettingsPolicy(t *testing.T, tt struct { Spec: policySpec, } - err := k8sClient.Create(context.Background(), clientSettingsPolicy) + err = k8sClient.Create(context.Background(), clientSettingsPolicy) // Clean up after test defer func() { @@ -192,7 +194,6 @@ func validateClientSettingsPolicy(t *testing.T, tt struct { // We expected errors - validation should have failed if err == nil { g.Expect(err).To(HaveOccurred()) - return } // Check that we got the expected error messages @@ -202,25 +203,22 @@ func validateClientSettingsPolicy(t *testing.T, tt struct { } // getKubernetesClient returns a client connected to a real Kubernetes cluster. -func getKubernetesClient(t *testing.T) client.Client { +func getKubernetesClient(t *testing.T) (k8sClient client.Client, err error) { t.Helper() // Use controller-runtime to get cluster connection k8sConfig, err := controllerruntime.GetConfig() if err != nil { - t.Skipf("Cannot connect to Kubernetes cluster: %v", err) + return nil, err } // Set up scheme with NGF types scheme := runtime.NewScheme() - if err := ngfAPIv1alpha1.AddToScheme(scheme); err != nil { - t.Fatalf("Failed to add NGF schema: %v", err) + if err = ngfAPIv1alpha1.AddToScheme(scheme); err != nil { + return nil, err } // Create client - k8sClient, err := client.New(k8sConfig, client.Options{Scheme: scheme}) - if err != nil { - t.Skipf("Cannot create k8s client: %v", err) - } + k8sClient, err = client.New(k8sConfig, client.Options{Scheme: scheme}) - return k8sClient + return k8sClient, err } From 248df537b127a868441c93c9416ed777260ab7f9 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Tue, 29 Jul 2025 15:26:50 +0100 Subject: [PATCH 28/46] Remove nil error checks --- tests/cel/clientsettingspolicy_test.go | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/tests/cel/clientsettingspolicy_test.go b/tests/cel/clientsettingspolicy_test.go index 2a12a47117..c0d7fdad07 100644 --- a/tests/cel/clientsettingspolicy_test.go +++ b/tests/cel/clientsettingspolicy_test.go @@ -185,16 +185,10 @@ func validateClientSettingsPolicy(t *testing.T, tt struct { // Check if we expected errors if len(tt.wantErrors) == 0 { - if err != nil { - g.Expect(err).ToNot(HaveOccurred()) - } - return + g.Expect(err).ToNot(HaveOccurred()) } - // We expected errors - validation should have failed - if err == nil { - g.Expect(err).To(HaveOccurred()) - } + g.Expect(err).To(HaveOccurred()) // Check that we got the expected error messages for _, wantError := range tt.wantErrors { From c5860e2d497d59921d789fc15a3917319a218602 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Tue, 29 Jul 2025 15:42:16 +0100 Subject: [PATCH 29/46] Create and return client on same line --- tests/cel/clientsettingspolicy_test.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/tests/cel/clientsettingspolicy_test.go b/tests/cel/clientsettingspolicy_test.go index c0d7fdad07..96592d9ba9 100644 --- a/tests/cel/clientsettingspolicy_test.go +++ b/tests/cel/clientsettingspolicy_test.go @@ -210,9 +210,6 @@ func getKubernetesClient(t *testing.T) (k8sClient client.Client, err error) { if err = ngfAPIv1alpha1.AddToScheme(scheme); err != nil { return nil, err } - - // Create client - k8sClient, err = client.New(k8sConfig, client.Options{Scheme: scheme}) - - return k8sClient, err + // Create a new client with the scheme and return it + return client.New(k8sConfig, client.Options{Scheme: scheme}) } From 83139ced8ecc940f3975e823078a4680b29b4f1d Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Tue, 29 Jul 2025 16:15:01 +0100 Subject: [PATCH 30/46] Use `rand.Prime` to attempt to keep policy names unique for each parallel test run --- tests/cel/clientsettingspolicy_test.go | 16 ++++++++++------ tests/go.mod | 1 + tests/go.sum | 2 ++ 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/tests/cel/clientsettingspolicy_test.go b/tests/cel/clientsettingspolicy_test.go index 96592d9ba9..4b322a8ba5 100644 --- a/tests/cel/clientsettingspolicy_test.go +++ b/tests/cel/clientsettingspolicy_test.go @@ -2,9 +2,9 @@ package cel import ( "context" + "crypto/rand" "fmt" "testing" - "time" . "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/runtime" @@ -164,9 +164,11 @@ func validateClientSettingsPolicy(t *testing.T, tt struct { g.Expect(err).ToNot(HaveOccurred()) + primeNum, err := rand.Prime(rand.Reader, 64) + g.Expect(err).ToNot(HaveOccurred()) policySpec := tt.policySpec - policySpec.TargetRef.Name = gatewayv1alpha2.ObjectName(fmt.Sprintf(TargetRefFormat, time.Now().UnixNano())) - policyName := fmt.Sprintf(PolicyNameFormat, time.Now().UnixNano()) + policySpec.TargetRef.Name = gatewayv1alpha2.ObjectName(fmt.Sprintf(TargetRefFormat, primeNum)) + policyName := fmt.Sprintf(PolicyNameFormat, primeNum) clientSettingsPolicy := &ngfAPIv1alpha1.ClientSettingsPolicy{ ObjectMeta: controllerruntime.ObjectMeta{ @@ -183,13 +185,15 @@ func validateClientSettingsPolicy(t *testing.T, tt struct { _ = k8sClient.Delete(context.Background(), clientSettingsPolicy) }() - // Check if we expected errors 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)) + } } - g.Expect(err).To(HaveOccurred()) - // Check that we got the expected error messages 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/go.mod b/tests/go.mod index a95e32bf64..6dbd5acc12 100644 --- a/tests/go.mod +++ b/tests/go.mod @@ -59,6 +59,7 @@ require ( github.com/spf13/pflag v1.0.7 // indirect github.com/stretchr/testify v1.10.0 // indirect github.com/x448/float16 v0.8.4 // indirect + github.com/zach-klippenstein/goregen v0.0.0-20160303162051-795b5e3961ea // indirect go.uber.org/automaxprocs v1.6.0 // indirect go.yaml.in/yaml/v2 v2.4.2 // indirect golang.org/x/mod v0.26.0 // indirect diff --git a/tests/go.sum b/tests/go.sum index 685931ed25..c7f9adaac4 100644 --- a/tests/go.sum +++ b/tests/go.sum @@ -135,6 +135,8 @@ github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM= github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= +github.com/zach-klippenstein/goregen v0.0.0-20160303162051-795b5e3961ea h1:CyhwejzVGvZ3Q2PSbQ4NRRYn+ZWv5eS1vlaEusT+bAI= +github.com/zach-klippenstein/goregen v0.0.0-20160303162051-795b5e3961ea/go.mod h1:eNr558nEUjP8acGw8FFjTeWvSgU1stO7FAO6eknhHe4= go.opentelemetry.io/auto/sdk v1.1.0 h1:cH53jehLUN6UFLY71z+NDOiNJqDdPRaXzTel0sJySYA= go.opentelemetry.io/auto/sdk v1.1.0/go.mod h1:3wSPjt5PWp2RhlCcmmOial7AvC4DQqZb7a7wCow3W8A= go.opentelemetry.io/otel v1.37.0 h1:9zhNfelUvx0KBfu/gb+ZgeAfAgtWrfHJZcAqFC228wQ= From a4e58ee9dd66f1e8ca71f6088bf9757494797a02 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Tue, 29 Jul 2025 16:35:41 +0100 Subject: [PATCH 31/46] Remove unused dependency --- tests/go.mod | 1 - tests/go.sum | 2 -- 2 files changed, 3 deletions(-) diff --git a/tests/go.mod b/tests/go.mod index 6dbd5acc12..a95e32bf64 100644 --- a/tests/go.mod +++ b/tests/go.mod @@ -59,7 +59,6 @@ require ( github.com/spf13/pflag v1.0.7 // indirect github.com/stretchr/testify v1.10.0 // indirect github.com/x448/float16 v0.8.4 // indirect - github.com/zach-klippenstein/goregen v0.0.0-20160303162051-795b5e3961ea // indirect go.uber.org/automaxprocs v1.6.0 // indirect go.yaml.in/yaml/v2 v2.4.2 // indirect golang.org/x/mod v0.26.0 // indirect diff --git a/tests/go.sum b/tests/go.sum index c7f9adaac4..685931ed25 100644 --- a/tests/go.sum +++ b/tests/go.sum @@ -135,8 +135,6 @@ github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM= github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= -github.com/zach-klippenstein/goregen v0.0.0-20160303162051-795b5e3961ea h1:CyhwejzVGvZ3Q2PSbQ4NRRYn+ZWv5eS1vlaEusT+bAI= -github.com/zach-klippenstein/goregen v0.0.0-20160303162051-795b5e3961ea/go.mod h1:eNr558nEUjP8acGw8FFjTeWvSgU1stO7FAO6eknhHe4= go.opentelemetry.io/auto/sdk v1.1.0 h1:cH53jehLUN6UFLY71z+NDOiNJqDdPRaXzTel0sJySYA= go.opentelemetry.io/auto/sdk v1.1.0/go.mod h1:3wSPjt5PWp2RhlCcmmOial7AvC4DQqZb7a7wCow3W8A= go.opentelemetry.io/otel v1.37.0 h1:9zhNfelUvx0KBfu/gb+ZgeAfAgtWrfHJZcAqFC228wQ= From 339e6b8c888327e179c0bf937fa18307f6575893 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Tue, 29 Jul 2025 17:13:31 +0100 Subject: [PATCH 32/46] Fix imports --- tests/cel/clientsettingspolicy_test.go | 2 +- tests/go.mod | 1 - tests/go.sum | 2 -- 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/cel/clientsettingspolicy_test.go b/tests/cel/clientsettingspolicy_test.go index 4b322a8ba5..d84d89d4ab 100644 --- a/tests/cel/clientsettingspolicy_test.go +++ b/tests/cel/clientsettingspolicy_test.go @@ -12,7 +12,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" - ngfAPIv1alpha1 "github.com/nginx/nginx-gateway-fabric/apis/v1alpha1" + ngfAPIv1alpha1 "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1" ) const ( diff --git a/tests/go.mod b/tests/go.mod index a95e32bf64..ae6165ac7c 100644 --- a/tests/go.mod +++ b/tests/go.mod @@ -5,7 +5,6 @@ go 1.24.2 replace github.com/nginx/nginx-gateway-fabric/v2 => ../ require ( - github.com/nginx/nginx-gateway-fabric v1.6.2 github.com/nginx/nginx-gateway-fabric/v2 v2.0.0 github.com/onsi/ginkgo/v2 v2.23.4 github.com/onsi/gomega v1.38.0 diff --git a/tests/go.sum b/tests/go.sum index 685931ed25..6eaad011d3 100644 --- a/tests/go.sum +++ b/tests/go.sum @@ -94,8 +94,6 @@ github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f h1:KUppIJq7/+ github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U= github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f h1:y5//uYreIhSUg3J1GEMiLbxo1LJaP8RfCpH6pymGZus= github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f/go.mod h1:ZdcZmHo+o7JKHSa8/e818NopupXU1YMK5fe1lsApnBw= -github.com/nginx/nginx-gateway-fabric v1.6.2 h1:ktdShWxT/Drh/5/u8S5QMRgnnBGvVuFDV10ttRk0W+c= -github.com/nginx/nginx-gateway-fabric v1.6.2/go.mod h1:Fi2hdmoNj9nQRX9YQDju+ntMPG4Fgcw+irfl/GYWSEk= github.com/onsi/ginkgo/v2 v2.23.4 h1:ktYTpKJAVZnDT4VjxSbiBenUjmlL/5QkBEocaWXiQus= github.com/onsi/ginkgo/v2 v2.23.4/go.mod h1:Bt66ApGPBFzHyR+JO10Zbt0Gsp4uWxu5mIOTusL46e8= github.com/onsi/gomega v1.38.0 h1:c/WX+w8SLAinvuKKQFh77WEucCnPk4j2OTUr7lt7BeY= From 716d051b2af12c3a94126c857f9d06317d664c93 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Wed, 30 Jul 2025 12:11:36 +0100 Subject: [PATCH 33/46] Add helper functions to gernated unique resource names --- internal/framework/helpers/helpers.go | 16 ++++++++++++++++ internal/framework/helpers/helpers_test.go | 22 ++++++++++++++++++++++ tests/cel/clientsettingspolicy_test.go | 12 +++++------- 3 files changed, 43 insertions(+), 7 deletions(-) diff --git a/internal/framework/helpers/helpers.go b/internal/framework/helpers/helpers.go index 15e51aa9ac..b61892aae7 100644 --- a/internal/framework/helpers/helpers.go +++ b/internal/framework/helpers/helpers.go @@ -3,6 +3,7 @@ package helpers import ( "bytes" + "crypto/rand" "fmt" "text/template" @@ -87,3 +88,18 @@ func MustExecuteTemplate(templ *template.Template, data interface{}) []byte { return buf.Bytes() } + +// RandomPrimeNumber generates a random prime number of 64 bits. +// It panics if it fails to generate a random prime number. +func RandomPrimeNumber() int64 { + primeNum, err := rand.Prime(rand.Reader, 64) + if err != nil { + panic(fmt.Errorf("failed to generate random prime number: %w", err)) + } + return primeNum.Int64() +} + +// UniqueResourceName generates a unique resource name by appending a random prime number to the given name. +func UniqueResourceName(name string) string { + return fmt.Sprintf("%s-%d", name, RandomPrimeNumber()) +} diff --git a/internal/framework/helpers/helpers_test.go b/internal/framework/helpers/helpers_test.go index f7365f3865..f4fd66cff9 100644 --- a/internal/framework/helpers/helpers_test.go +++ b/internal/framework/helpers/helpers_test.go @@ -109,3 +109,25 @@ func TestMustExecuteTemplatePanics(t *testing.T) { g.Expect(execute).To(Panic()) } + +func TestMustGenerateRandomPrimeNumer(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + // This test is expected to panic if it fails to generate a random prime number. + g.Expect(func() { + _ = helpers.RandomPrimeNumber() + }).ToNot(Panic()) +} + +func TestMustReturnUniqueResourceName(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + name := "test-resource" + uniqueName := helpers.UniqueResourceName(name) + + g.Expect(uniqueName).To(HavePrefix(name)) + g.Expect(uniqueName).To(HaveSuffix("-")) + g.Expect(len(uniqueName)).To(BeNumerically(">", len(name))) +} diff --git a/tests/cel/clientsettingspolicy_test.go b/tests/cel/clientsettingspolicy_test.go index d84d89d4ab..fee5196423 100644 --- a/tests/cel/clientsettingspolicy_test.go +++ b/tests/cel/clientsettingspolicy_test.go @@ -2,8 +2,6 @@ package cel import ( "context" - "crypto/rand" - "fmt" "testing" . "github.com/onsi/gomega" @@ -13,6 +11,7 @@ import ( gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" ngfAPIv1alpha1 "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1" + ngfHelpers "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/helpers" ) const ( @@ -35,8 +34,8 @@ const ( ) const ( - PolicyNameFormat = "test-policy-%d" - TargetRefFormat = "targetRef-name-%d" + PolicyName = "test-policy" + TargetRef = "targetRef-name" ) func TestClientSettingsPoliciesTargetRefKind(t *testing.T) { @@ -164,11 +163,10 @@ func validateClientSettingsPolicy(t *testing.T, tt struct { g.Expect(err).ToNot(HaveOccurred()) - primeNum, err := rand.Prime(rand.Reader, 64) g.Expect(err).ToNot(HaveOccurred()) policySpec := tt.policySpec - policySpec.TargetRef.Name = gatewayv1alpha2.ObjectName(fmt.Sprintf(TargetRefFormat, primeNum)) - policyName := fmt.Sprintf(PolicyNameFormat, primeNum) + policySpec.TargetRef.Name = gatewayv1alpha2.ObjectName(ngfHelpers.UniqueResourceName(TargetRef)) + policyName := ngfHelpers.UniqueResourceName(PolicyName) clientSettingsPolicy := &ngfAPIv1alpha1.ClientSettingsPolicy{ ObjectMeta: controllerruntime.ObjectMeta{ From d273281300bde6938546468d4895f076739972c7 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Wed, 30 Jul 2025 12:26:58 +0100 Subject: [PATCH 34/46] Move `getKubernetesClient` function to helpers --- internal/framework/helpers/helpers.go | 21 +++++++++++++++++++ internal/framework/helpers/helpers_test.go | 18 ++++++++++++++++ tests/cel/clientsettingspolicy_test.go | 24 +--------------------- 3 files changed, 40 insertions(+), 23 deletions(-) diff --git a/internal/framework/helpers/helpers.go b/internal/framework/helpers/helpers.go index b61892aae7..0c27824ab9 100644 --- a/internal/framework/helpers/helpers.go +++ b/internal/framework/helpers/helpers.go @@ -9,7 +9,11 @@ import ( "github.com/google/go-cmp/cmp" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "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" ) // Diff prints the diff between two structs. @@ -103,3 +107,20 @@ func RandomPrimeNumber() int64 { func UniqueResourceName(name string) string { return fmt.Sprintf("%s-%d", name, RandomPrimeNumber()) } + +// GetKubernetesClient returns a client connected to a real Kubernetes cluster. +func GetKubernetesClient() (k8sClient client.Client, err error) { + // Use controller-runtime to get cluster connection + k8sConfig, err := controllerruntime.GetConfig() + if err != nil { + return nil, err + } + + // Set up scheme with NGF types + scheme := runtime.NewScheme() + if err = ngfAPIv1alpha1.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}) +} diff --git a/internal/framework/helpers/helpers_test.go b/internal/framework/helpers/helpers_test.go index f4fd66cff9..3c30085908 100644 --- a/internal/framework/helpers/helpers_test.go +++ b/internal/framework/helpers/helpers_test.go @@ -1,6 +1,7 @@ package helpers_test import ( + "context" "testing" "text/template" @@ -9,6 +10,8 @@ import ( gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" gatewayv1alpha3 "sigs.k8s.io/gateway-api/apis/v1alpha3" + corev1 "k8s.io/api/core/v1" + "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/helpers" ) @@ -131,3 +134,18 @@ func TestMustReturnUniqueResourceName(t *testing.T) { g.Expect(uniqueName).To(HaveSuffix("-")) g.Expect(len(uniqueName)).To(BeNumerically(">", len(name))) } + +func TestMustCreateKubernetesClient(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + k8sClient, err := helpers.GetKubernetesClient() + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(k8sClient).ToNot(BeNil()) + + // Check that the client can be used to list namespaces + namespaces := &corev1.NamespaceList{} + err = k8sClient.List(context.Background(), namespaces) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(namespaces.Items).ToNot(BeEmpty()) +} diff --git a/tests/cel/clientsettingspolicy_test.go b/tests/cel/clientsettingspolicy_test.go index fee5196423..f3395e6fda 100644 --- a/tests/cel/clientsettingspolicy_test.go +++ b/tests/cel/clientsettingspolicy_test.go @@ -5,9 +5,7 @@ import ( "testing" . "github.com/onsi/gomega" - "k8s.io/apimachinery/pkg/runtime" 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" @@ -157,9 +155,7 @@ func validateClientSettingsPolicy(t *testing.T, tt struct { t.Helper() g := NewWithT(t) - // Get Kubernetes client from test framework - // This should be set up by your test framework to connect to a real cluster - k8sClient, err := getKubernetesClient(t) + k8sClient, err := ngfHelpers.GetKubernetesClient() g.Expect(err).ToNot(HaveOccurred()) @@ -197,21 +193,3 @@ func validateClientSettingsPolicy(t *testing.T, tt struct { g.Expect(err.Error()).To(ContainSubstring(wantError), "Expected error '%s' not found in: %s", wantError, err.Error()) } } - -// getKubernetesClient returns a client connected to a real Kubernetes cluster. -func getKubernetesClient(t *testing.T) (k8sClient client.Client, err error) { - t.Helper() - // Use controller-runtime to get cluster connection - k8sConfig, err := controllerruntime.GetConfig() - if err != nil { - return nil, err - } - - // Set up scheme with NGF types - scheme := runtime.NewScheme() - if err = ngfAPIv1alpha1.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}) -} From 4a6e84e0579e490a5e114847ab00f54de8537193 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Wed, 30 Jul 2025 12:56:08 +0100 Subject: [PATCH 35/46] Fix test failures --- internal/framework/helpers/helpers_test.go | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/internal/framework/helpers/helpers_test.go b/internal/framework/helpers/helpers_test.go index 3c30085908..073b319972 100644 --- a/internal/framework/helpers/helpers_test.go +++ b/internal/framework/helpers/helpers_test.go @@ -6,11 +6,12 @@ import ( "text/template" . "github.com/onsi/gomega" + controllerruntime "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" gatewayv1alpha3 "sigs.k8s.io/gateway-api/apis/v1alpha3" - corev1 "k8s.io/api/core/v1" + ngfAPIv1alpha1 "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1" "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/helpers" ) @@ -131,7 +132,6 @@ func TestMustReturnUniqueResourceName(t *testing.T) { uniqueName := helpers.UniqueResourceName(name) g.Expect(uniqueName).To(HavePrefix(name)) - g.Expect(uniqueName).To(HaveSuffix("-")) g.Expect(len(uniqueName)).To(BeNumerically(">", len(name))) } @@ -140,12 +140,22 @@ func TestMustCreateKubernetesClient(t *testing.T) { g := NewWithT(t) k8sClient, err := helpers.GetKubernetesClient() + g.Expect(err).ToNot(HaveOccurred()) g.Expect(k8sClient).ToNot(BeNil()) // Check that the client can be used to list namespaces - namespaces := &corev1.NamespaceList{} - err = k8sClient.List(context.Background(), namespaces) + nginxGateway := &ngfAPIv1alpha1.NginxGateway{ + ObjectMeta: controllerruntime.ObjectMeta{ + Name: helpers.UniqueResourceName("test-nginx-gateway"), + Namespace: "default", + }, + } + // Clean up after test + defer func() { + _ = k8sClient.Delete(context.Background(), nginxGateway) + }() + err = k8sClient.Create(context.Background(), nginxGateway) g.Expect(err).ToNot(HaveOccurred()) - g.Expect(namespaces.Items).ToNot(BeEmpty()) + g.Expect(nginxGateway).ToNot(BeNil()) } From 66b71056e73a45089b66cbeab33e9dd8f9d7846c Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Wed, 30 Jul 2025 15:01:50 +0100 Subject: [PATCH 36/46] Update `TestMustCreateKubernetesClient` to only assert client creation --- internal/framework/helpers/helpers_test.go | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/internal/framework/helpers/helpers_test.go b/internal/framework/helpers/helpers_test.go index 073b319972..a64e22ca68 100644 --- a/internal/framework/helpers/helpers_test.go +++ b/internal/framework/helpers/helpers_test.go @@ -1,18 +1,14 @@ package helpers_test import ( - "context" "testing" "text/template" . "github.com/onsi/gomega" - controllerruntime "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" gatewayv1alpha3 "sigs.k8s.io/gateway-api/apis/v1alpha3" - ngfAPIv1alpha1 "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1" - "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/helpers" ) @@ -143,19 +139,4 @@ func TestMustCreateKubernetesClient(t *testing.T) { g.Expect(err).ToNot(HaveOccurred()) g.Expect(k8sClient).ToNot(BeNil()) - - // Check that the client can be used to list namespaces - nginxGateway := &ngfAPIv1alpha1.NginxGateway{ - ObjectMeta: controllerruntime.ObjectMeta{ - Name: helpers.UniqueResourceName("test-nginx-gateway"), - Namespace: "default", - }, - } - // Clean up after test - defer func() { - _ = k8sClient.Delete(context.Background(), nginxGateway) - }() - err = k8sClient.Create(context.Background(), nginxGateway) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(nginxGateway).ToNot(BeNil()) } From c45403a98026b06b30925b181539d8b7186fd349 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Wed, 30 Jul 2025 15:07:47 +0100 Subject: [PATCH 37/46] Remove test for creating kubernetes cluster --- internal/framework/helpers/helpers_test.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/internal/framework/helpers/helpers_test.go b/internal/framework/helpers/helpers_test.go index a64e22ca68..a5e94c7d1b 100644 --- a/internal/framework/helpers/helpers_test.go +++ b/internal/framework/helpers/helpers_test.go @@ -113,8 +113,6 @@ func TestMustExecuteTemplatePanics(t *testing.T) { func TestMustGenerateRandomPrimeNumer(t *testing.T) { t.Parallel() g := NewWithT(t) - - // This test is expected to panic if it fails to generate a random prime number. g.Expect(func() { _ = helpers.RandomPrimeNumber() }).ToNot(Panic()) @@ -130,13 +128,3 @@ func TestMustReturnUniqueResourceName(t *testing.T) { g.Expect(uniqueName).To(HavePrefix(name)) g.Expect(len(uniqueName)).To(BeNumerically(">", len(name))) } - -func TestMustCreateKubernetesClient(t *testing.T) { - t.Parallel() - g := NewWithT(t) - - k8sClient, err := helpers.GetKubernetesClient() - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(k8sClient).ToNot(BeNil()) -} From e284740060247a84222ba21470028162227f7e61 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Wed, 30 Jul 2025 16:08:42 +0100 Subject: [PATCH 38/46] Add common finctions for cel tests --- internal/framework/helpers/helpers.go | 21 --------------------- tests/cel/clientsettingspolicy_test.go | 2 +- tests/cel/common.go | 26 ++++++++++++++++++++++++++ 3 files changed, 27 insertions(+), 22 deletions(-) create mode 100644 tests/cel/common.go diff --git a/internal/framework/helpers/helpers.go b/internal/framework/helpers/helpers.go index 0c27824ab9..b61892aae7 100644 --- a/internal/framework/helpers/helpers.go +++ b/internal/framework/helpers/helpers.go @@ -9,11 +9,7 @@ import ( "github.com/google/go-cmp/cmp" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "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" ) // Diff prints the diff between two structs. @@ -107,20 +103,3 @@ func RandomPrimeNumber() int64 { func UniqueResourceName(name string) string { return fmt.Sprintf("%s-%d", name, RandomPrimeNumber()) } - -// GetKubernetesClient returns a client connected to a real Kubernetes cluster. -func GetKubernetesClient() (k8sClient client.Client, err error) { - // Use controller-runtime to get cluster connection - k8sConfig, err := controllerruntime.GetConfig() - if err != nil { - return nil, err - } - - // Set up scheme with NGF types - scheme := runtime.NewScheme() - if err = ngfAPIv1alpha1.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}) -} diff --git a/tests/cel/clientsettingspolicy_test.go b/tests/cel/clientsettingspolicy_test.go index f3395e6fda..a581a21fdd 100644 --- a/tests/cel/clientsettingspolicy_test.go +++ b/tests/cel/clientsettingspolicy_test.go @@ -155,7 +155,7 @@ func validateClientSettingsPolicy(t *testing.T, tt struct { t.Helper() g := NewWithT(t) - k8sClient, err := ngfHelpers.GetKubernetesClient() + k8sClient, err := GetKubernetesClient() g.Expect(err).ToNot(HaveOccurred()) diff --git a/tests/cel/common.go b/tests/cel/common.go new file mode 100644 index 0000000000..e16cf737b2 --- /dev/null +++ b/tests/cel/common.go @@ -0,0 +1,26 @@ +package cel + +import ( + "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" +) + +// GetKubernetesClient returns a client connected to a real Kubernetes cluster. +func GetKubernetesClient() (k8sClient client.Client, err error) { + // Use controller-runtime to get cluster connection + k8sConfig, err := controllerruntime.GetConfig() + if err != nil { + return nil, err + } + + // Set up scheme with NGF types + scheme := runtime.NewScheme() + if err = ngfAPIv1alpha1.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}) +} From cb43b5238108cce592ff5b2663fcdd2e24608ec4 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Wed, 30 Jul 2025 16:28:04 +0100 Subject: [PATCH 39/46] Move common constatns to `common.go` and add v1alpha2 to client schema --- tests/cel/clientsettingspolicy_test.go | 26 +------------------- tests/cel/common.go | 33 +++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 26 deletions(-) diff --git a/tests/cel/clientsettingspolicy_test.go b/tests/cel/clientsettingspolicy_test.go index a581a21fdd..bc99831128 100644 --- a/tests/cel/clientsettingspolicy_test.go +++ b/tests/cel/clientsettingspolicy_test.go @@ -12,30 +12,6 @@ import ( ngfHelpers "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/helpers" ) -const ( - GatewayKind = "Gateway" - HTTPRouteKind = "HTTPRoute" - GRPCRouteKind = "GRPCRoute" - TCPRouteKind = "TCPRoute" - InvalidKind = "InvalidKind" -) - -const ( - GatewayGroup = "gateway.networking.k8s.io" - InvalidGroup = "invalid.networking.k8s.io" - DiscoveryGroup = "discovery.k8s.io/v1" -) - -const ( - ExpectedTargetRefKindError = `TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute` - ExpectedTargetRefGroupError = `TargetRef Group must be gateway.networking.k8s.io.` -) - -const ( - PolicyName = "test-policy" - TargetRef = "targetRef-name" -) - func TestClientSettingsPoliciesTargetRefKind(t *testing.T) { t.Parallel() tests := []struct { @@ -155,7 +131,7 @@ func validateClientSettingsPolicy(t *testing.T, tt struct { t.Helper() g := NewWithT(t) - k8sClient, err := GetKubernetesClient() + k8sClient, err := GetKubernetesClient(t) g.Expect(err).ToNot(HaveOccurred()) diff --git a/tests/cel/common.go b/tests/cel/common.go index e16cf737b2..c821d2b584 100644 --- a/tests/cel/common.go +++ b/tests/cel/common.go @@ -1,15 +1,43 @@ package cel import ( + "testing" + "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" +) + +const ( + GatewayKind = "Gateway" + HTTPRouteKind = "HTTPRoute" + GRPCRouteKind = "GRPCRoute" + TCPRouteKind = "TCPRoute" + InvalidKind = "InvalidKind" +) + +const ( + GatewayGroup = "gateway.networking.k8s.io" + InvalidGroup = "invalid.networking.k8s.io" + DiscoveryGroup = "discovery.k8s.io/v1" +) + +const ( + ExpectedTargetRefKindError = `TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute` + ExpectedTargetRefGroupError = `TargetRef Group must be gateway.networking.k8s.io.` +) + +const ( + PolicyName = "test-policy" + TargetRef = "targetRef-name" ) // GetKubernetesClient returns a client connected to a real Kubernetes cluster. -func GetKubernetesClient() (k8sClient client.Client, err error) { +func GetKubernetesClient(t *testing.T) (k8sClient client.Client, err error) { + t.Helper() // Use controller-runtime to get cluster connection k8sConfig, err := controllerruntime.GetConfig() if err != nil { @@ -21,6 +49,9 @@ func GetKubernetesClient() (k8sClient client.Client, err error) { 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}) } From 9217339e881dca2c7bacb4166040f949e6d8093a Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Wed, 30 Jul 2025 16:41:58 +0100 Subject: [PATCH 40/46] Move helper functions to common.go and add tests --- internal/framework/helpers/helpers.go | 16 ------------- internal/framework/helpers/helpers_test.go | 19 ---------------- tests/cel/clientsettingspolicy_test.go | 5 ++--- tests/cel/common.go | 17 ++++++++++++++ tests/cel/common_test.go | 26 ++++++++++++++++++++++ 5 files changed, 45 insertions(+), 38 deletions(-) create mode 100644 tests/cel/common_test.go diff --git a/internal/framework/helpers/helpers.go b/internal/framework/helpers/helpers.go index b61892aae7..15e51aa9ac 100644 --- a/internal/framework/helpers/helpers.go +++ b/internal/framework/helpers/helpers.go @@ -3,7 +3,6 @@ package helpers import ( "bytes" - "crypto/rand" "fmt" "text/template" @@ -88,18 +87,3 @@ func MustExecuteTemplate(templ *template.Template, data interface{}) []byte { return buf.Bytes() } - -// RandomPrimeNumber generates a random prime number of 64 bits. -// It panics if it fails to generate a random prime number. -func RandomPrimeNumber() int64 { - primeNum, err := rand.Prime(rand.Reader, 64) - if err != nil { - panic(fmt.Errorf("failed to generate random prime number: %w", err)) - } - return primeNum.Int64() -} - -// UniqueResourceName generates a unique resource name by appending a random prime number to the given name. -func UniqueResourceName(name string) string { - return fmt.Sprintf("%s-%d", name, RandomPrimeNumber()) -} diff --git a/internal/framework/helpers/helpers_test.go b/internal/framework/helpers/helpers_test.go index a5e94c7d1b..f7365f3865 100644 --- a/internal/framework/helpers/helpers_test.go +++ b/internal/framework/helpers/helpers_test.go @@ -109,22 +109,3 @@ func TestMustExecuteTemplatePanics(t *testing.T) { g.Expect(execute).To(Panic()) } - -func TestMustGenerateRandomPrimeNumer(t *testing.T) { - t.Parallel() - g := NewWithT(t) - g.Expect(func() { - _ = helpers.RandomPrimeNumber() - }).ToNot(Panic()) -} - -func TestMustReturnUniqueResourceName(t *testing.T) { - t.Parallel() - g := NewWithT(t) - - name := "test-resource" - uniqueName := helpers.UniqueResourceName(name) - - g.Expect(uniqueName).To(HavePrefix(name)) - g.Expect(len(uniqueName)).To(BeNumerically(">", len(name))) -} diff --git a/tests/cel/clientsettingspolicy_test.go b/tests/cel/clientsettingspolicy_test.go index bc99831128..34a1f70d6e 100644 --- a/tests/cel/clientsettingspolicy_test.go +++ b/tests/cel/clientsettingspolicy_test.go @@ -9,7 +9,6 @@ import ( gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" ngfAPIv1alpha1 "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1" - ngfHelpers "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/helpers" ) func TestClientSettingsPoliciesTargetRefKind(t *testing.T) { @@ -137,8 +136,8 @@ func validateClientSettingsPolicy(t *testing.T, tt struct { g.Expect(err).ToNot(HaveOccurred()) policySpec := tt.policySpec - policySpec.TargetRef.Name = gatewayv1alpha2.ObjectName(ngfHelpers.UniqueResourceName(TargetRef)) - policyName := ngfHelpers.UniqueResourceName(PolicyName) + policySpec.TargetRef.Name = gatewayv1alpha2.ObjectName(UniqueResourceName(TargetRef)) + policyName := UniqueResourceName(PolicyName) clientSettingsPolicy := &ngfAPIv1alpha1.ClientSettingsPolicy{ ObjectMeta: controllerruntime.ObjectMeta{ diff --git a/tests/cel/common.go b/tests/cel/common.go index c821d2b584..f3ce77c84a 100644 --- a/tests/cel/common.go +++ b/tests/cel/common.go @@ -1,6 +1,8 @@ package cel import ( + "crypto/rand" + "fmt" "testing" "k8s.io/apimachinery/pkg/runtime" @@ -55,3 +57,18 @@ func GetKubernetesClient(t *testing.T) (k8sClient client.Client, err error) { // Create a new client with the scheme and return it return client.New(k8sConfig, client.Options{Scheme: scheme}) } + +// RandomPrimeNumber generates a random prime number of 64 bits. +// It panics if it fails to generate a random prime number. +func RandomPrimeNumber() int64 { + primeNum, err := rand.Prime(rand.Reader, 64) + if err != nil { + panic(fmt.Errorf("failed to generate random prime number: %w", err)) + } + return primeNum.Int64() +} + +// UniqueResourceName generates a unique resource name by appending a random prime number to the given name. +func UniqueResourceName(name string) string { + return fmt.Sprintf("%s-%d", name, RandomPrimeNumber()) +} diff --git a/tests/cel/common_test.go b/tests/cel/common_test.go new file mode 100644 index 0000000000..bcacfb5dc9 --- /dev/null +++ b/tests/cel/common_test.go @@ -0,0 +1,26 @@ +package cel + +import ( + "testing" + + . "github.com/onsi/gomega" +) + +func TestMustGenerateRandomPrimeNumer(t *testing.T) { + t.Parallel() + g := NewWithT(t) + g.Expect(func() { + _ = RandomPrimeNumber() + }).ToNot(Panic()) +} + +func TestMustReturnUniqueResourceName(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + name := "test-resource" + uniqueName := UniqueResourceName(name) + + g.Expect(uniqueName).To(HavePrefix(name)) + g.Expect(len(uniqueName)).To(BeNumerically(">", len(name))) +} From f30f1174487d7ffe396a283a3dee63d3d8cfa2b8 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Wed, 30 Jul 2025 17:41:26 +0100 Subject: [PATCH 41/46] unexport consnts and functions in cel common file --- tests/Makefile | 8 ----- tests/cel/clientsettingspolicy_test.go | 48 +++++++++++++------------- tests/cel/common.go | 30 +++++++++------- 3 files changed, 41 insertions(+), 45 deletions(-) diff --git a/tests/Makefile b/tests/Makefile index e13195647c..6771828ceb 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -198,11 +198,3 @@ test-cel-validation: echo "Running test: $(CEL_TEST_TARGET) in ./cel"; \ go test -v ./cel -run "$(CEL_TEST_TARGET)"; \ fi - - -# Set up a kind cluster, install NGF CRDs, and run CEL validation tests -.PHONY: setup-and-test-cel-validation -setup-and-test-cel-validation: - kind create cluster --name $(CLUSTER_NAME) || true - kubectl kustomize ../config/crd | kubectl apply -f - - $(MAKE) test-cel-validation diff --git a/tests/cel/clientsettingspolicy_test.go b/tests/cel/clientsettingspolicy_test.go index 34a1f70d6e..c845e4aded 100644 --- a/tests/cel/clientsettingspolicy_test.go +++ b/tests/cel/clientsettingspolicy_test.go @@ -22,8 +22,8 @@ func TestClientSettingsPoliciesTargetRefKind(t *testing.T) { name: "Validate TargetRef of kind Gateway is allowed", policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ - Kind: GatewayKind, - Group: GatewayGroup, + Kind: gatewayKind, + Group: gatewayGroup, }, }, }, @@ -31,8 +31,8 @@ func TestClientSettingsPoliciesTargetRefKind(t *testing.T) { name: "Validate TargetRef of kind HTTPRoute is allowed", policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ - Kind: HTTPRouteKind, - Group: GatewayGroup, + Kind: httpRouteKind, + Group: gatewayGroup, }, }, }, @@ -40,28 +40,28 @@ func TestClientSettingsPoliciesTargetRefKind(t *testing.T) { name: "Validate TargetRef of kind GRPCRoute is allowed", policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ - Kind: GRPCRouteKind, - Group: GatewayGroup, + Kind: grpcRouteKind, + Group: gatewayGroup, }, }, }, { name: "Validate Invalid TargetRef Kind is not allowed", - wantErrors: []string{ExpectedTargetRefKindError}, + wantErrors: []string{expectedTargetRefKindError}, policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ - Kind: InvalidKind, - Group: GatewayGroup, + Kind: invalidKind, + Group: gatewayGroup, }, }, }, { name: "Validate TCPRoute TargetRef Kind is not allowed", - wantErrors: []string{ExpectedTargetRefKindError}, + wantErrors: []string{expectedTargetRefKindError}, policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ - Kind: TCPRouteKind, - Group: GatewayGroup, + Kind: tcpRouteKind, + Group: gatewayGroup, }, }, }, @@ -86,28 +86,28 @@ func TestClientSettingsPoliciesTargetRefGroup(t *testing.T) { name: "Validate gateway.networking.k8s.io TargetRef Group is allowed", policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ - Kind: GatewayKind, - Group: GatewayGroup, + Kind: gatewayKind, + Group: gatewayGroup, }, }, }, { name: "Validate invalid.networking.k8s.io TargetRef Group is not allowed", - wantErrors: []string{ExpectedTargetRefGroupError}, + wantErrors: []string{expectedTargetRefGroupError}, policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ - Kind: GatewayKind, - Group: InvalidGroup, + Kind: gatewayKind, + Group: invalidGroup, }, }, }, { name: "Validate discovery.k8s.io/v1 TargetRef Group is not allowed", - wantErrors: []string{ExpectedTargetRefGroupError}, + wantErrors: []string{expectedTargetRefGroupError}, policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ - Kind: GatewayKind, - Group: DiscoveryGroup, + Kind: gatewayKind, + Group: discoveryGroup, }, }, }, @@ -130,19 +130,19 @@ func validateClientSettingsPolicy(t *testing.T, tt struct { t.Helper() g := NewWithT(t) - k8sClient, err := GetKubernetesClient(t) + k8sClient, err := getKubernetesClient(t) g.Expect(err).ToNot(HaveOccurred()) g.Expect(err).ToNot(HaveOccurred()) policySpec := tt.policySpec - policySpec.TargetRef.Name = gatewayv1alpha2.ObjectName(UniqueResourceName(TargetRef)) - policyName := UniqueResourceName(PolicyName) + policySpec.TargetRef.Name = gatewayv1alpha2.ObjectName(UniqueResourceName(testTargetRefName)) + policyName := UniqueResourceName(testPolicyName) clientSettingsPolicy := &ngfAPIv1alpha1.ClientSettingsPolicy{ ObjectMeta: controllerruntime.ObjectMeta{ Name: policyName, - Namespace: "default", + Namespace: defaultNamespace, }, Spec: policySpec, } diff --git a/tests/cel/common.go b/tests/cel/common.go index f3ce77c84a..81778ddd2b 100644 --- a/tests/cel/common.go +++ b/tests/cel/common.go @@ -14,31 +14,35 @@ import ( ) const ( - GatewayKind = "Gateway" - HTTPRouteKind = "HTTPRoute" - GRPCRouteKind = "GRPCRoute" - TCPRouteKind = "TCPRoute" - InvalidKind = "InvalidKind" + gatewayKind = "Gateway" + httpRouteKind = "HTTPRoute" + grpcRouteKind = "GRPCRoute" + tcpRouteKind = "TCPRoute" + invalidKind = "InvalidKind" ) const ( - GatewayGroup = "gateway.networking.k8s.io" - InvalidGroup = "invalid.networking.k8s.io" - DiscoveryGroup = "discovery.k8s.io/v1" + gatewayGroup = "gateway.networking.k8s.io" + invalidGroup = "invalid.networking.k8s.io" + discoveryGroup = "discovery.k8s.io/v1" ) const ( - ExpectedTargetRefKindError = `TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute` - ExpectedTargetRefGroupError = `TargetRef Group must be gateway.networking.k8s.io.` + expectedTargetRefKindError = `TargetRef Kind must be one of: Gateway, HTTPRoute, or GRPCRoute` + expectedTargetRefGroupError = `TargetRef Group must be gateway.networking.k8s.io.` ) const ( - PolicyName = "test-policy" - TargetRef = "targetRef-name" + defaultNamespace = "default" +) + +const ( + testPolicyName = "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, err error) { t.Helper() // Use controller-runtime to get cluster connection k8sConfig, err := controllerruntime.GetConfig() From e18b9dc5867efc80c1e458e15b625d28a265151e Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Wed, 30 Jul 2025 17:51:26 +0100 Subject: [PATCH 42/46] unexport randomPrimeNumber and uniqueResourceName --- tests/cel/clientsettingspolicy_test.go | 4 ++-- tests/cel/common.go | 6 +++--- tests/cel/common_test.go | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/cel/clientsettingspolicy_test.go b/tests/cel/clientsettingspolicy_test.go index c845e4aded..105ac2dc62 100644 --- a/tests/cel/clientsettingspolicy_test.go +++ b/tests/cel/clientsettingspolicy_test.go @@ -136,8 +136,8 @@ func validateClientSettingsPolicy(t *testing.T, tt struct { g.Expect(err).ToNot(HaveOccurred()) policySpec := tt.policySpec - policySpec.TargetRef.Name = gatewayv1alpha2.ObjectName(UniqueResourceName(testTargetRefName)) - policyName := UniqueResourceName(testPolicyName) + policySpec.TargetRef.Name = gatewayv1alpha2.ObjectName(uniqueResourceName(testTargetRefName)) + policyName := uniqueResourceName(testPolicyName) clientSettingsPolicy := &ngfAPIv1alpha1.ClientSettingsPolicy{ ObjectMeta: controllerruntime.ObjectMeta{ diff --git a/tests/cel/common.go b/tests/cel/common.go index 81778ddd2b..428042adc2 100644 --- a/tests/cel/common.go +++ b/tests/cel/common.go @@ -64,7 +64,7 @@ func getKubernetesClient(t *testing.T) (k8sClient client.Client, err error) { // RandomPrimeNumber generates a random prime number of 64 bits. // It panics if it fails to generate a random prime number. -func RandomPrimeNumber() int64 { +func randomPrimeNumber() int64 { primeNum, err := rand.Prime(rand.Reader, 64) if err != nil { panic(fmt.Errorf("failed to generate random prime number: %w", err)) @@ -73,6 +73,6 @@ func RandomPrimeNumber() int64 { } // UniqueResourceName generates a unique resource name by appending a random prime number to the given name. -func UniqueResourceName(name string) string { - return fmt.Sprintf("%s-%d", name, RandomPrimeNumber()) +func uniqueResourceName(name string) string { + return fmt.Sprintf("%s-%d", name, randomPrimeNumber()) } diff --git a/tests/cel/common_test.go b/tests/cel/common_test.go index bcacfb5dc9..02f9d0c6c8 100644 --- a/tests/cel/common_test.go +++ b/tests/cel/common_test.go @@ -10,7 +10,7 @@ func TestMustGenerateRandomPrimeNumer(t *testing.T) { t.Parallel() g := NewWithT(t) g.Expect(func() { - _ = RandomPrimeNumber() + _ = randomPrimeNumber() }).ToNot(Panic()) } @@ -19,7 +19,7 @@ func TestMustReturnUniqueResourceName(t *testing.T) { g := NewWithT(t) name := "test-resource" - uniqueName := UniqueResourceName(name) + uniqueName := uniqueResourceName(name) g.Expect(uniqueName).To(HavePrefix(name)) g.Expect(len(uniqueName)).To(BeNumerically(">", len(name))) From 869a091df7bff9c828ee6daf6b1a6ba79913d044 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Wed, 30 Jul 2025 17:53:38 +0100 Subject: [PATCH 43/46] Fix casing in comments --- tests/cel/common.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/cel/common.go b/tests/cel/common.go index 428042adc2..e7b969c3a6 100644 --- a/tests/cel/common.go +++ b/tests/cel/common.go @@ -41,7 +41,7 @@ const ( testTargetRefName = "test-targetRef" ) -// GetKubernetesClient returns a client connected to a real Kubernetes cluster. +// getKubernetesClient returns a client connected to a real Kubernetes cluster. func getKubernetesClient(t *testing.T) (k8sClient client.Client, err error) { t.Helper() // Use controller-runtime to get cluster connection @@ -62,7 +62,7 @@ func getKubernetesClient(t *testing.T) (k8sClient client.Client, err error) { return client.New(k8sConfig, client.Options{Scheme: scheme}) } -// RandomPrimeNumber generates a random prime number of 64 bits. +// randomPrimeNumber generates a random prime number of 64 bits. // It panics if it fails to generate a random prime number. func randomPrimeNumber() int64 { primeNum, err := rand.Prime(rand.Reader, 64) @@ -72,7 +72,7 @@ func randomPrimeNumber() int64 { return primeNum.Int64() } -// UniqueResourceName generates a unique resource name by appending a random prime number to the given name. +// uniqueResourceName generates a unique resource name by appending a random prime number to the given name. func uniqueResourceName(name string) string { return fmt.Sprintf("%s-%d", name, randomPrimeNumber()) } From 92caf1245284ab0c574c7f899d660ee8c3d6a5f7 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Thu, 31 Jul 2025 09:50:16 +0100 Subject: [PATCH 44/46] Remove duplicate error checks --- tests/cel/clientsettingspolicy_test.go | 16 +++++----------- tests/cel/common_test.go | 2 +- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/tests/cel/clientsettingspolicy_test.go b/tests/cel/clientsettingspolicy_test.go index 105ac2dc62..c879574d5d 100644 --- a/tests/cel/clientsettingspolicy_test.go +++ b/tests/cel/clientsettingspolicy_test.go @@ -130,11 +130,6 @@ func validateClientSettingsPolicy(t *testing.T, tt struct { t.Helper() g := NewWithT(t) - k8sClient, err := getKubernetesClient(t) - - g.Expect(err).ToNot(HaveOccurred()) - - g.Expect(err).ToNot(HaveOccurred()) policySpec := tt.policySpec policySpec.TargetRef.Name = gatewayv1alpha2.ObjectName(uniqueResourceName(testTargetRefName)) policyName := uniqueResourceName(testPolicyName) @@ -147,6 +142,10 @@ func validateClientSettingsPolicy(t *testing.T, tt struct { Spec: policySpec, } + k8sClient, err := getKubernetesClient(t) + + g.Expect(err).ToNot(HaveOccurred()) + err = k8sClient.Create(context.Background(), clientSettingsPolicy) // Clean up after test @@ -159,12 +158,7 @@ func validateClientSettingsPolicy(t *testing.T, tt struct { } else { g.Expect(err).To(HaveOccurred()) for _, wantError := range tt.wantErrors { - g.Expect(err.Error()).To(ContainSubstring(wantError)) + g.Expect(err.Error()).To(ContainSubstring(wantError), "Expected error '%s' not found in: %s", wantError, err.Error()) } } - - // Check that we got the expected error messages - 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_test.go b/tests/cel/common_test.go index 02f9d0c6c8..5f025d314e 100644 --- a/tests/cel/common_test.go +++ b/tests/cel/common_test.go @@ -6,7 +6,7 @@ import ( . "github.com/onsi/gomega" ) -func TestMustGenerateRandomPrimeNumer(t *testing.T) { +func TestMustGenerateRandomPrimeNumber(t *testing.T) { t.Parallel() g := NewWithT(t) g.Expect(func() { From 81fd868e7489251e3a9ab2fcbba0bc53901742cc Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Thu, 31 Jul 2025 11:22:49 +0100 Subject: [PATCH 45/46] Initialise ginko and k8sClient at the start of each test --- tests/cel/clientsettingspolicy_test.go | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/tests/cel/clientsettingspolicy_test.go b/tests/cel/clientsettingspolicy_test.go index c879574d5d..1bbf7492c3 100644 --- a/tests/cel/clientsettingspolicy_test.go +++ b/tests/cel/clientsettingspolicy_test.go @@ -6,6 +6,7 @@ import ( . "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" @@ -13,6 +14,9 @@ import ( func TestClientSettingsPoliciesTargetRefKind(t *testing.T) { t.Parallel() + g := NewWithT(t) + k8sClient, err := getKubernetesClient(t) + g.Expect(err).ToNot(HaveOccurred()) tests := []struct { policySpec ngfAPIv1alpha1.ClientSettingsPolicySpec name string @@ -70,13 +74,16 @@ func TestClientSettingsPoliciesTargetRefKind(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - validateClientSettingsPolicy(t, tt) + validateClientSettingsPolicy(t, tt, g, k8sClient) }) } } func TestClientSettingsPoliciesTargetRefGroup(t *testing.T) { t.Parallel() + g := NewWithT(t) + k8sClient, err := getKubernetesClient(t) + g.Expect(err).ToNot(HaveOccurred()) tests := []struct { policySpec ngfAPIv1alpha1.ClientSettingsPolicySpec name string @@ -116,7 +123,7 @@ func TestClientSettingsPoliciesTargetRefGroup(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - validateClientSettingsPolicy(t, tt) + validateClientSettingsPolicy(t, tt, g, k8sClient) }) } } @@ -125,10 +132,9 @@ func validateClientSettingsPolicy(t *testing.T, tt struct { policySpec ngfAPIv1alpha1.ClientSettingsPolicySpec name string wantErrors []string -}, +}, g *WithT, k8sClient client.Client, ) { t.Helper() - g := NewWithT(t) policySpec := tt.policySpec policySpec.TargetRef.Name = gatewayv1alpha2.ObjectName(uniqueResourceName(testTargetRefName)) @@ -142,11 +148,7 @@ func validateClientSettingsPolicy(t *testing.T, tt struct { Spec: policySpec, } - k8sClient, err := getKubernetesClient(t) - - g.Expect(err).ToNot(HaveOccurred()) - - err = k8sClient.Create(context.Background(), clientSettingsPolicy) + err := k8sClient.Create(context.Background(), clientSettingsPolicy) // Clean up after test defer func() { From 37c85d2fc281690e7d46407629be94ab1720593d Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Wed, 6 Aug 2025 09:39:18 +0100 Subject: [PATCH 46/46] Add timeout to k8sClient Create --- tests/cel/clientsettingspolicy_test.go | 7 +++++-- tests/framework/timeout.go | 4 ++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/cel/clientsettingspolicy_test.go b/tests/cel/clientsettingspolicy_test.go index 1bbf7492c3..00833b39aa 100644 --- a/tests/cel/clientsettingspolicy_test.go +++ b/tests/cel/clientsettingspolicy_test.go @@ -10,6 +10,7 @@ import ( 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/tests/framework" ) func TestClientSettingsPoliciesTargetRefKind(t *testing.T) { @@ -147,8 +148,10 @@ func validateClientSettingsPolicy(t *testing.T, tt struct { }, Spec: policySpec, } - - err := k8sClient.Create(context.Background(), clientSettingsPolicy) + timeoutConfig := framework.DefaultTimeoutConfig() + ctx, cancel := context.WithTimeout(context.Background(), timeoutConfig.KubernetesClientTimeout) + err := k8sClient.Create(ctx, clientSettingsPolicy) + defer cancel() // Clean up after test defer func() { diff --git a/tests/framework/timeout.go b/tests/framework/timeout.go index 8d8557622f..87abe87189 100644 --- a/tests/framework/timeout.go +++ b/tests/framework/timeout.go @@ -35,6 +35,9 @@ type TimeoutConfig struct { // TestForTrafficTimeout represents the maximum time for NGF to test for passing or failing traffic. TestForTrafficTimeout time.Duration + + // KubernetesClientTimeout represents the maximum time for Kubernetes client operations. + KubernetesClientTimeout time.Duration } // DefaultTimeoutConfig populates a TimeoutConfig with the default values. @@ -51,5 +54,6 @@ func DefaultTimeoutConfig() TimeoutConfig { GetLeaderLeaseTimeout: 60 * time.Second, GetStatusTimeout: 60 * time.Second, TestForTrafficTimeout: 60 * time.Second, + KubernetesClientTimeout: 10 * time.Second, } }