Skip to content

Add CEL validation test for targetRef in ClientSettingsPolicy #3623

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
dbb5b5c
Add CEL validation test for targetRef in ClientSettingsPolicy
shaun-nx Jul 14, 2025
48ec86b
gofumpt
shaun-nx Jul 14, 2025
c7129bd
Add tests for targetRefGroup
shaun-nx Jul 14, 2025
87a4e0e
Rename tests
shaun-nx Jul 14, 2025
29f201e
Merge branch 'main' into tests/cel-clientsettingspolicies
shaun-nx Jul 15, 2025
d921ed2
Move tests into clientsettingspolicy_test.go
shaun-nx Jul 15, 2025
f22269a
Update tests to create a ClientSettingsPolicy resource during tests
shaun-nx Jul 15, 2025
7fddffa
make lint in tests
shaun-nx Jul 15, 2025
dd66a79
Update tests to create a ClientSettingsPolicy object during validation
shaun-nx Jul 17, 2025
d664f35
Merge branch 'main' into tests/cel-clientsettingspolicies
shaun-nx Jul 17, 2025
cd9bada
Update TargetRegGroup tests
shaun-nx Jul 17, 2025
7d9989d
Fix lint errors
shaun-nx Jul 17, 2025
9274e57
Group valid and invalid test cases into single test function
shaun-nx Jul 21, 2025
506b01c
Merge branch 'main' into tests/cel-clientsettingspolicies
shaun-nx Jul 21, 2025
f99ab29
Revert dependency version
shaun-nx Jul 21, 2025
ad80b70
Move imports
shaun-nx Jul 21, 2025
e5479fb
Define constants for Kinds and Groups
shaun-nx Jul 21, 2025
1789701
Merge branch 'main' into tests/cel-clientsettingspolicies
shaun-nx Jul 22, 2025
aa4a6b2
Use controller-runtime library to get cluster information
shaun-nx Jul 22, 2025
8f74214
Add Makefile targets to run CEL test and re format test
shaun-nx Jul 22, 2025
4486bd5
Ensure TestClientSettingsPoliciesTargetRefKind and TestClientSettings…
shaun-nx Jul 22, 2025
1adf168
Merge branch 'main' into tests/cel-clientsettingspolicies
shaun-nx Jul 22, 2025
7c6dba4
Merge branch 'main' into tests/cel-clientsettingspolicies
shaun-nx Jul 23, 2025
69f485b
Merge branch 'main' into tests/cel-clientsettingspolicies
shaun-nx Jul 23, 2025
1ad24c4
Merge branch 'main' into tests/cel-clientsettingspolicies
shaun-nx Jul 25, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions tests/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Comment on lines +191 to +200
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how important this is to have a separate section for running a specific test. We don't do it for our other tests, and it's pretty straightforward for the one-off cases to just specify the test directly in the command line.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're also going to need a job in our CI pipeline to run these.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fair. I found this pretty helpful for debugging specific tests. It might be good to keep just for that purpose



# 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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have a Makefile target to create kind cluster, so shouldn't need this.

kubectl kustomize ../config/crd | kubectl apply -f -
Copy link
Contributor

@sarthyparty sarthyparty Jul 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I ran this target, I got this error: The CustomResourceDefinition "nginxproxies.gateway.nginx.org" is invalid: metadata.annotations: Too long: may not be more than 262144 bytes. I was able to fix it by adding changing the apply command to kubectl kustomize ../config/crd | kubectl apply --server-side -f -. Can you check this out?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if its just my machine

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not just your machine, it should be --server-side apply, see #3589

$(MAKE) test-cel-validation
226 changes: 226 additions & 0 deletions tests/cel/clientsettingspolicy_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,226 @@
package cel

import (
"context"
"fmt"
"strings"
"testing"
"time"

"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/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.`
)

const (
PolicyName = "test-policy"
TargetRefFormat = "targetRef-name-%d"
)

func TestClientSettingsPoliciesTargetRefKind(t *testing.T) {
tests := []struct {
policySpec ngfAPIv1alpha1.ClientSettingsPolicySpec
name string
wantErrors []string
}{
{
name: "Validate TargetRef of kind Gateway is allowed",
policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{
TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{
Kind: GatewayKind,
Group: GatewayGroup,
},
},
},
{
name: "Validate TargetRef of kind HTTPRoute is allowed",
policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{
TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{
Kind: HTTPRouteKind,
Group: GatewayGroup,
},
},
},
{
name: "Validate TargetRef of kind GRPCRoute is allowed",
policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{
TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{
Kind: GRPCRouteKind,
Group: GatewayGroup,
},
},
},
{
name: "Validate Invalid TargetRef Kind is not allowed",
wantErrors: []string{ExpectedTargetRefKindError},
policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{
TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{
Kind: InvalidKind,
Group: GatewayGroup,
},
},
},
{
name: "Validate TCPRoute TargetRef Kind is not allowed",
wantErrors: []string{ExpectedTargetRefKindError},
policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{
TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{
Kind: TCPRouteKind,
Group: GatewayGroup,
},
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
validateClientSettingsPolicy(t, tt)
})
}
}

func TestClientSettingsPoliciesTargetRefGroup(t *testing.T) {
tests := []struct {
policySpec ngfAPIv1alpha1.ClientSettingsPolicySpec
name string
wantErrors []string
}{
{
name: "Validate gateway.networking.k8s.io TargetRef Group is allowed",
policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{
TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{
Kind: GatewayKind,
Group: GatewayGroup,
},
},
},
{
name: "Validate invalid.networking.k8s.io TargetRef Group is not allowed",
wantErrors: []string{ExpectedTargetRefGroupError},
policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{
TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{
Kind: GatewayKind,
Group: InvalidGroup,
},
},
},
{
name: "Validate discovery.k8s.io/v1 TargetRef Group is not allowed",
wantErrors: []string{ExpectedTargetRefGroupError},
policySpec: ngfAPIv1alpha1.ClientSettingsPolicySpec{
TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{
Kind: GatewayKind,
Group: DiscoveryGroup,
},
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
validateClientSettingsPolicy(t, tt)
})
}
}

func validateClientSettingsPolicy(t *testing.T, tt struct {
policySpec ngfAPIv1alpha1.ClientSettingsPolicySpec
name string
wantErrors []string
},
) {
t.Helper()

// Get Kubernetes client from test framework
// 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{
Name: PolicyName,
Namespace: "default",
},
Spec: policySpec,
}

err := k8sClient.Create(context.Background(), clientSettingsPolicy)

// Clean up after test
defer func() {
_ = k8sClient.Delete(context.Background(), clientSettingsPolicy)
}()

// Check if we expected errors
if len(tt.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 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)
}
}

// 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 {
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 {
t.Skipf("Cannot create k8s client: %v", err)
}

return k8sClient
}
Loading