From 58f9f130f163c65c08f543d70eaf4744bf6f043b Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Mon, 25 Aug 2025 15:25:53 +0100 Subject: [PATCH 01/10] Add test for debugging --- .../controller/nginx/config/servers_test.go | 117 ++++++++++++++++++ 1 file changed, 117 insertions(+) diff --git a/internal/controller/nginx/config/servers_test.go b/internal/controller/nginx/config/servers_test.go index 6b604d7bec..7916627322 100644 --- a/internal/controller/nginx/config/servers_test.go +++ b/internal/controller/nginx/config/servers_test.go @@ -8,8 +8,13 @@ import ( . "github.com/onsi/gomega" "github.com/onsi/gomega/format" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" + + ngfAPIv1alpha1 "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/http" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies/policiesfakes" @@ -2126,6 +2131,118 @@ func TestCreateServersConflicts(t *testing.T) { } } +func TestInternalLocationForHttpRouteAdvancedRules(t *testing.T) { + + pathRules := []dataplane.PathRule{ + { + Path: "/rest", + PathType: dataplane.PathTypePrefix, + MatchRules: []dataplane.MatchRule{ + { + Match: dataplane.Match{ + Headers: []dataplane.HTTPHeaderMatch{ + { + Name: "Referer", + Type: dataplane.MatchTypeRegularExpression, + Value: "(?i)(mydomain|myotherdomain).+\\.example\\.(cloud|com)", + }, + }, + }, + }, + { + BackendGroup: dataplane.BackendGroup{ + Backends: []dataplane.Backend{ + { + UpstreamName: "coffee", + }, + }, + }, + }, + }, + }, + { + Path: "/rest", + PathType: dataplane.PathTypePrefix, + MatchRules: []dataplane.MatchRule{ + { + BackendGroup: dataplane.BackendGroup{ + Backends: []dataplane.Backend{ + { + UpstreamName: "coffee-api", + }, + }, + }, + }, + }, + }, + { + MatchRules: []dataplane.MatchRule{ + { + BackendGroup: dataplane.BackendGroup{ + Backends: []dataplane.Backend{ + { + UpstreamName: "coffee", + }, + }, + }, + }, + }, + }, + } + + policy := &ngfAPIv1alpha1.ClientSettingsPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "http-example-com", + Namespace: "default", + }, + Spec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ + TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ + Kind: "Gateway", + Group: "gateway.networking.k8s.io", + }, + Body: &ngfAPIv1alpha1.ClientBody{ + MaxSize: helpers.GetPointer[ngfAPIv1alpha1.Size]("250m"), + }, + }, + } + + httpServers := []dataplane.VirtualServer{ + { + IsDefault: true, + Port: 8080, + }, + { + Hostname: "http.example.com", + PathRules: pathRules, + Port: 8080, + Policies: []policies.Policy{policy}, + }, + } + + fakeGenerator := &policiesfakes.FakeGenerator{} + fakeGenerator.GenerateForLocationReturns(policies.GenerateResultFiles{ + { + Name: "ext-policy.conf", + Content: []byte("client_max_body_size 600m"), + }, + }) + + fakeGenerator.GenerateForInternalLocationReturns(policies.GenerateResultFiles{ + { + Name: "ext-policy.conf", + Content: []byte("client_max_body_size 600m"), + }, + }) + locations, matches, _ := createLocations(&httpServers[1], "1", fakeGenerator, alwaysFalseKeepAliveChecker) + + fmt.Println("---------------------------") + fmt.Printf("%+v\n", locations) + fmt.Println("---------------------------") + fmt.Println("---------------------------") + fmt.Printf("%+v\n", matches) + fmt.Println("---------------------------") +} + func TestCreateServers_Includes(t *testing.T) { t.Parallel() From 6abbd1ae1f7304fd91f2115b9f0a57c3783b61a5 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Tue, 26 Aug 2025 11:54:31 +0100 Subject: [PATCH 02/10] Add tests for valid and invalid path rules --- .../controller/nginx/config/servers_test.go | 162 ++++++++++++++---- 1 file changed, 133 insertions(+), 29 deletions(-) diff --git a/internal/controller/nginx/config/servers_test.go b/internal/controller/nginx/config/servers_test.go index 7916627322..b6905b5294 100644 --- a/internal/controller/nginx/config/servers_test.go +++ b/internal/controller/nginx/config/servers_test.go @@ -17,6 +17,7 @@ import ( "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/http" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies/clientsettings" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies/policiesfakes" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/shared" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/dataplane" @@ -2131,7 +2132,23 @@ func TestCreateServersConflicts(t *testing.T) { } } -func TestInternalLocationForHttpRouteAdvancedRules(t *testing.T) { +func TestInvalidHttpRoute(t *testing.T) { + + policy := &ngfAPIv1alpha1.ClientSettingsPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "http-example-com", + Namespace: "default", + }, + Spec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ + TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ + Kind: "Gateway", + Group: "gateway.networking.k8s.io", + }, + Body: &ngfAPIv1alpha1.ClientBody{ + MaxSize: helpers.GetPointer[ngfAPIv1alpha1.Size]("600m"), + }, + }, + } pathRules := []dataplane.PathRule{ { @@ -2159,6 +2176,9 @@ func TestInternalLocationForHttpRouteAdvancedRules(t *testing.T) { }, }, }, + Policies: []policies.Policy{ + policy, + }, }, { Path: "/rest", @@ -2174,6 +2194,9 @@ func TestInternalLocationForHttpRouteAdvancedRules(t *testing.T) { }, }, }, + Policies: []policies.Policy{ + policy, + }, }, { MatchRules: []dataplane.MatchRule{ @@ -2190,6 +2213,35 @@ func TestInternalLocationForHttpRouteAdvancedRules(t *testing.T) { }, } + policyGenerator := policies.NewCompositeGenerator( + clientsettings.NewGenerator(), + ) + + conf := dataplane.Configuration{ + HTTPServers: []dataplane.VirtualServer{ + { + IsDefault: true, + Port: 8080, + }, + { + Hostname: "http.example.com", + PathRules: pathRules, + Port: 8080, + Policies: []policies.Policy{}, + }, + }, + } + gen := GeneratorImpl{} + results := gen.executeServers(conf, policyGenerator, alwaysFalseKeepAliveChecker) + + // Print resulting NGINX Config + for _, r := range results { + fmt.Print(string(r.data)) + } +} + +func TestValidHttpRoute(t *testing.T) { + policy := &ngfAPIv1alpha1.ClientSettingsPolicy{ ObjectMeta: metav1.ObjectMeta{ Name: "http-example-com", @@ -2201,46 +2253,98 @@ func TestInternalLocationForHttpRouteAdvancedRules(t *testing.T) { Group: "gateway.networking.k8s.io", }, Body: &ngfAPIv1alpha1.ClientBody{ - MaxSize: helpers.GetPointer[ngfAPIv1alpha1.Size]("250m"), + MaxSize: helpers.GetPointer[ngfAPIv1alpha1.Size]("600m"), }, }, } - httpServers := []dataplane.VirtualServer{ + pathRules := []dataplane.PathRule{ { - IsDefault: true, - Port: 8080, + Path: "/rest1", + PathType: dataplane.PathTypePrefix, + MatchRules: []dataplane.MatchRule{ + { + Match: dataplane.Match{ + Headers: []dataplane.HTTPHeaderMatch{ + { + Name: "Referer", + Type: dataplane.MatchTypeRegularExpression, + Value: "(?i)(mydomain|myotherdomain).+\\.example\\.(cloud|com)", + }, + }, + }, + }, + { + BackendGroup: dataplane.BackendGroup{ + Backends: []dataplane.Backend{ + { + UpstreamName: "coffee", + }, + }, + }, + }, + }, + Policies: []policies.Policy{ + policy, + }, }, { - Hostname: "http.example.com", - PathRules: pathRules, - Port: 8080, - Policies: []policies.Policy{policy}, + Path: "/rest2", + PathType: dataplane.PathTypePrefix, + MatchRules: []dataplane.MatchRule{ + { + BackendGroup: dataplane.BackendGroup{ + Backends: []dataplane.Backend{ + { + UpstreamName: "coffee-api", + }, + }, + }, + }, + }, + Policies: []policies.Policy{ + policy, + }, }, - } - - fakeGenerator := &policiesfakes.FakeGenerator{} - fakeGenerator.GenerateForLocationReturns(policies.GenerateResultFiles{ { - Name: "ext-policy.conf", - Content: []byte("client_max_body_size 600m"), + MatchRules: []dataplane.MatchRule{ + { + BackendGroup: dataplane.BackendGroup{ + Backends: []dataplane.Backend{ + { + UpstreamName: "coffee", + }, + }, + }, + }, + }, }, - }) + } - fakeGenerator.GenerateForInternalLocationReturns(policies.GenerateResultFiles{ - { - Name: "ext-policy.conf", - Content: []byte("client_max_body_size 600m"), + policyGenerator := policies.NewCompositeGenerator( + clientsettings.NewGenerator(), + ) + + conf := dataplane.Configuration{ + HTTPServers: []dataplane.VirtualServer{ + { + IsDefault: true, + Port: 8080, + }, + { + Hostname: "http.example.com", + PathRules: pathRules, + Port: 8080, + Policies: []policies.Policy{}, + }, }, - }) - locations, matches, _ := createLocations(&httpServers[1], "1", fakeGenerator, alwaysFalseKeepAliveChecker) - - fmt.Println("---------------------------") - fmt.Printf("%+v\n", locations) - fmt.Println("---------------------------") - fmt.Println("---------------------------") - fmt.Printf("%+v\n", matches) - fmt.Println("---------------------------") + } + gen := GeneratorImpl{} + results := gen.executeServers(conf, policyGenerator, alwaysFalseKeepAliveChecker) + + for _, r := range results { + fmt.Print(string(r.data)) + } } func TestCreateServers_Includes(t *testing.T) { From c0f20053feb13c9a0272c81b4ea5173194fe1372 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Tue, 26 Aug 2025 13:04:22 +0100 Subject: [PATCH 03/10] Update tests name. Add test for the invalid HTTPRoute with no policy attached --- .../controller/nginx/config/servers_test.go | 89 ++++++++++++++++++- 1 file changed, 87 insertions(+), 2 deletions(-) diff --git a/internal/controller/nginx/config/servers_test.go b/internal/controller/nginx/config/servers_test.go index b6905b5294..0d11b05478 100644 --- a/internal/controller/nginx/config/servers_test.go +++ b/internal/controller/nginx/config/servers_test.go @@ -2132,7 +2132,7 @@ func TestCreateServersConflicts(t *testing.T) { } } -func TestInvalidHttpRoute(t *testing.T) { +func TestHttpRouteSameRoutesAdvancedRoutingWithPolicy(t *testing.T) { policy := &ngfAPIv1alpha1.ClientSettingsPolicy{ ObjectMeta: metav1.ObjectMeta{ @@ -2240,7 +2240,92 @@ func TestInvalidHttpRoute(t *testing.T) { } } -func TestValidHttpRoute(t *testing.T) { +func TestHttpRouteSameRoutesAdvancedRoutingWithoutPolicy(t *testing.T) { + + pathRules := []dataplane.PathRule{ + { + Path: "/rest", + PathType: dataplane.PathTypePrefix, + MatchRules: []dataplane.MatchRule{ + { + Match: dataplane.Match{ + Headers: []dataplane.HTTPHeaderMatch{ + { + Name: "Referer", + Type: dataplane.MatchTypeRegularExpression, + Value: "(?i)(mydomain|myotherdomain).+\\.example\\.(cloud|com)", + }, + }, + }, + }, + { + BackendGroup: dataplane.BackendGroup{ + Backends: []dataplane.Backend{ + { + UpstreamName: "coffee", + }, + }, + }, + }, + }, + }, + { + Path: "/rest", + PathType: dataplane.PathTypePrefix, + MatchRules: []dataplane.MatchRule{ + { + BackendGroup: dataplane.BackendGroup{ + Backends: []dataplane.Backend{ + { + UpstreamName: "coffee-api", + }, + }, + }, + }, + }, + }, + { + MatchRules: []dataplane.MatchRule{ + { + BackendGroup: dataplane.BackendGroup{ + Backends: []dataplane.Backend{ + { + UpstreamName: "coffee", + }, + }, + }, + }, + }, + }, + } + + conf := dataplane.Configuration{ + HTTPServers: []dataplane.VirtualServer{ + { + IsDefault: true, + Port: 8080, + }, + { + Hostname: "http.example.com", + PathRules: pathRules, + Port: 8080, + }, + }, + } + gen := GeneratorImpl{} + + policyGenerator := policies.NewCompositeGenerator( + clientsettings.NewGenerator(), + ) + + results := gen.executeServers(conf, policyGenerator, alwaysFalseKeepAliveChecker) + + for _, r := range results { + fmt.Print(string(r.data)) + } +} + +func TestValidHttpRouteDifferentRoutes(t *testing.T) { policy := &ngfAPIv1alpha1.ClientSettingsPolicy{ ObjectMeta: metav1.ObjectMeta{ From 3e096a7852fad586faf815b69dd067f15ba85af5 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Tue, 26 Aug 2025 13:08:24 +0100 Subject: [PATCH 04/10] Add comments to tests. Remove tests with /rest1 and /rest2 endpoints --- .../controller/nginx/config/servers_test.go | 112 +----------------- 1 file changed, 5 insertions(+), 107 deletions(-) diff --git a/internal/controller/nginx/config/servers_test.go b/internal/controller/nginx/config/servers_test.go index 0d11b05478..b14109eeb9 100644 --- a/internal/controller/nginx/config/servers_test.go +++ b/internal/controller/nginx/config/servers_test.go @@ -2132,6 +2132,8 @@ func TestCreateServersConflicts(t *testing.T) { } } +// TestHttpRouteSameRoutesAdvancedRoutingWithPolicy uses pathRules that are the same as the HTTPRoute in issue 3763 +// This test will generate the invalid NGINX configuration that would cause the duplicate directive error. func TestHttpRouteSameRoutesAdvancedRoutingWithPolicy(t *testing.T) { policy := &ngfAPIv1alpha1.ClientSettingsPolicy{ @@ -2240,6 +2242,9 @@ func TestHttpRouteSameRoutesAdvancedRoutingWithPolicy(t *testing.T) { } } +// TestHttpRouteSameRoutesAdvancedRoutingWithoutPolicy uses pathRules that are the same as the HTTPRoute in issue 3763 +// This test is the same as TestHttpRouteSameRoutesAdvancedRoutingWithPolicy with the exception that none of the pathRules have policies +// This test will generate the expected NGINX config for a normal advancted routing scenario. func TestHttpRouteSameRoutesAdvancedRoutingWithoutPolicy(t *testing.T) { pathRules := []dataplane.PathRule{ @@ -2325,113 +2330,6 @@ func TestHttpRouteSameRoutesAdvancedRoutingWithoutPolicy(t *testing.T) { } } -func TestValidHttpRouteDifferentRoutes(t *testing.T) { - - policy := &ngfAPIv1alpha1.ClientSettingsPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Name: "http-example-com", - Namespace: "default", - }, - Spec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ - TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ - Kind: "Gateway", - Group: "gateway.networking.k8s.io", - }, - Body: &ngfAPIv1alpha1.ClientBody{ - MaxSize: helpers.GetPointer[ngfAPIv1alpha1.Size]("600m"), - }, - }, - } - - pathRules := []dataplane.PathRule{ - { - Path: "/rest1", - PathType: dataplane.PathTypePrefix, - MatchRules: []dataplane.MatchRule{ - { - Match: dataplane.Match{ - Headers: []dataplane.HTTPHeaderMatch{ - { - Name: "Referer", - Type: dataplane.MatchTypeRegularExpression, - Value: "(?i)(mydomain|myotherdomain).+\\.example\\.(cloud|com)", - }, - }, - }, - }, - { - BackendGroup: dataplane.BackendGroup{ - Backends: []dataplane.Backend{ - { - UpstreamName: "coffee", - }, - }, - }, - }, - }, - Policies: []policies.Policy{ - policy, - }, - }, - { - Path: "/rest2", - PathType: dataplane.PathTypePrefix, - MatchRules: []dataplane.MatchRule{ - { - BackendGroup: dataplane.BackendGroup{ - Backends: []dataplane.Backend{ - { - UpstreamName: "coffee-api", - }, - }, - }, - }, - }, - Policies: []policies.Policy{ - policy, - }, - }, - { - MatchRules: []dataplane.MatchRule{ - { - BackendGroup: dataplane.BackendGroup{ - Backends: []dataplane.Backend{ - { - UpstreamName: "coffee", - }, - }, - }, - }, - }, - }, - } - - policyGenerator := policies.NewCompositeGenerator( - clientsettings.NewGenerator(), - ) - - conf := dataplane.Configuration{ - HTTPServers: []dataplane.VirtualServer{ - { - IsDefault: true, - Port: 8080, - }, - { - Hostname: "http.example.com", - PathRules: pathRules, - Port: 8080, - Policies: []policies.Policy{}, - }, - }, - } - gen := GeneratorImpl{} - results := gen.executeServers(conf, policyGenerator, alwaysFalseKeepAliveChecker) - - for _, r := range results { - fmt.Print(string(r.data)) - } -} - func TestCreateServers_Includes(t *testing.T) { t.Parallel() From 13977c7a19a6570aef19b43b2c678da13091e405 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Tue, 26 Aug 2025 15:43:14 +0100 Subject: [PATCH 05/10] Remove debug tests --- .../controller/nginx/config/servers_test.go | 204 ------------------ 1 file changed, 204 deletions(-) diff --git a/internal/controller/nginx/config/servers_test.go b/internal/controller/nginx/config/servers_test.go index b14109eeb9..6b604d7bec 100644 --- a/internal/controller/nginx/config/servers_test.go +++ b/internal/controller/nginx/config/servers_test.go @@ -8,16 +8,10 @@ import ( . "github.com/onsi/gomega" "github.com/onsi/gomega/format" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" - - ngfAPIv1alpha1 "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1" - "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/http" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies" - "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies/clientsettings" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies/policiesfakes" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/shared" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/dataplane" @@ -2132,204 +2126,6 @@ func TestCreateServersConflicts(t *testing.T) { } } -// TestHttpRouteSameRoutesAdvancedRoutingWithPolicy uses pathRules that are the same as the HTTPRoute in issue 3763 -// This test will generate the invalid NGINX configuration that would cause the duplicate directive error. -func TestHttpRouteSameRoutesAdvancedRoutingWithPolicy(t *testing.T) { - - policy := &ngfAPIv1alpha1.ClientSettingsPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Name: "http-example-com", - Namespace: "default", - }, - Spec: ngfAPIv1alpha1.ClientSettingsPolicySpec{ - TargetRef: gatewayv1alpha2.LocalPolicyTargetReference{ - Kind: "Gateway", - Group: "gateway.networking.k8s.io", - }, - Body: &ngfAPIv1alpha1.ClientBody{ - MaxSize: helpers.GetPointer[ngfAPIv1alpha1.Size]("600m"), - }, - }, - } - - pathRules := []dataplane.PathRule{ - { - Path: "/rest", - PathType: dataplane.PathTypePrefix, - MatchRules: []dataplane.MatchRule{ - { - Match: dataplane.Match{ - Headers: []dataplane.HTTPHeaderMatch{ - { - Name: "Referer", - Type: dataplane.MatchTypeRegularExpression, - Value: "(?i)(mydomain|myotherdomain).+\\.example\\.(cloud|com)", - }, - }, - }, - }, - { - BackendGroup: dataplane.BackendGroup{ - Backends: []dataplane.Backend{ - { - UpstreamName: "coffee", - }, - }, - }, - }, - }, - Policies: []policies.Policy{ - policy, - }, - }, - { - Path: "/rest", - PathType: dataplane.PathTypePrefix, - MatchRules: []dataplane.MatchRule{ - { - BackendGroup: dataplane.BackendGroup{ - Backends: []dataplane.Backend{ - { - UpstreamName: "coffee-api", - }, - }, - }, - }, - }, - Policies: []policies.Policy{ - policy, - }, - }, - { - MatchRules: []dataplane.MatchRule{ - { - BackendGroup: dataplane.BackendGroup{ - Backends: []dataplane.Backend{ - { - UpstreamName: "coffee", - }, - }, - }, - }, - }, - }, - } - - policyGenerator := policies.NewCompositeGenerator( - clientsettings.NewGenerator(), - ) - - conf := dataplane.Configuration{ - HTTPServers: []dataplane.VirtualServer{ - { - IsDefault: true, - Port: 8080, - }, - { - Hostname: "http.example.com", - PathRules: pathRules, - Port: 8080, - Policies: []policies.Policy{}, - }, - }, - } - gen := GeneratorImpl{} - results := gen.executeServers(conf, policyGenerator, alwaysFalseKeepAliveChecker) - - // Print resulting NGINX Config - for _, r := range results { - fmt.Print(string(r.data)) - } -} - -// TestHttpRouteSameRoutesAdvancedRoutingWithoutPolicy uses pathRules that are the same as the HTTPRoute in issue 3763 -// This test is the same as TestHttpRouteSameRoutesAdvancedRoutingWithPolicy with the exception that none of the pathRules have policies -// This test will generate the expected NGINX config for a normal advancted routing scenario. -func TestHttpRouteSameRoutesAdvancedRoutingWithoutPolicy(t *testing.T) { - - pathRules := []dataplane.PathRule{ - { - Path: "/rest", - PathType: dataplane.PathTypePrefix, - MatchRules: []dataplane.MatchRule{ - { - Match: dataplane.Match{ - Headers: []dataplane.HTTPHeaderMatch{ - { - Name: "Referer", - Type: dataplane.MatchTypeRegularExpression, - Value: "(?i)(mydomain|myotherdomain).+\\.example\\.(cloud|com)", - }, - }, - }, - }, - { - BackendGroup: dataplane.BackendGroup{ - Backends: []dataplane.Backend{ - { - UpstreamName: "coffee", - }, - }, - }, - }, - }, - }, - { - Path: "/rest", - PathType: dataplane.PathTypePrefix, - MatchRules: []dataplane.MatchRule{ - { - BackendGroup: dataplane.BackendGroup{ - Backends: []dataplane.Backend{ - { - UpstreamName: "coffee-api", - }, - }, - }, - }, - }, - }, - { - MatchRules: []dataplane.MatchRule{ - { - BackendGroup: dataplane.BackendGroup{ - Backends: []dataplane.Backend{ - { - UpstreamName: "coffee", - }, - }, - }, - }, - }, - }, - } - - conf := dataplane.Configuration{ - HTTPServers: []dataplane.VirtualServer{ - { - IsDefault: true, - Port: 8080, - }, - { - Hostname: "http.example.com", - PathRules: pathRules, - Port: 8080, - }, - }, - } - gen := GeneratorImpl{} - - policyGenerator := policies.NewCompositeGenerator( - clientsettings.NewGenerator(), - ) - - results := gen.executeServers(conf, policyGenerator, alwaysFalseKeepAliveChecker) - - for _, r := range results { - fmt.Print(string(r.data)) - } -} - func TestCreateServers_Includes(t *testing.T) { t.Parallel() From 003ed75ac186c2af9e4952c725008b5418f2cde1 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Tue, 26 Aug 2025 15:47:49 +0100 Subject: [PATCH 06/10] Prevent duplicate `includes` in advanced route matching --- internal/controller/state/dataplane/configuration.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/controller/state/dataplane/configuration.go b/internal/controller/state/dataplane/configuration.go index 94f5f0819a..bb19b8217f 100644 --- a/internal/controller/state/dataplane/configuration.go +++ b/internal/controller/state/dataplane/configuration.go @@ -586,10 +586,10 @@ func (hpr *hostPathRules) upsertRoute( if !exist { hostRule.Path = path hostRule.PathType = convertPathType(*m.Path.Type) + hostRule.Policies = append(hostRule.Policies, pols...) } hostRule.GRPC = GRPC - hostRule.Policies = append(hostRule.Policies, pols...) hostRule.MatchRules = append(hostRule.MatchRules, MatchRule{ Source: objectSrc, From eb3918818ddae30cf7a53e28dcb6d0fcce2e9ebd Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Wed, 27 Aug 2025 15:15:28 +0100 Subject: [PATCH 07/10] Add test "Simple Gateway and HTTPRoute with advanced routing and policies attached" --- .../state/dataplane/configuration_test.go | 94 +++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/internal/controller/state/dataplane/configuration_test.go b/internal/controller/state/dataplane/configuration_test.go index 75fefc982a..17b7b52e80 100644 --- a/internal/controller/state/dataplane/configuration_test.go +++ b/internal/controller/state/dataplane/configuration_test.go @@ -680,6 +680,22 @@ func TestBuildConfiguration(t *testing.T) { }, ) + hrAdvancedRouteWithPolicy, groupsHRAdvanced, routeHRAdvanced := createTestResources( + "hr-advanced-route-with-policy", + "policy.com", + "listener-80-1", + pathAndType{path: "/rest", pathType: prefix}, + pathAndType{path: "/rest", pathType: prefix}, + ) + + routeHRAdvanced.Spec.Rules[0].Matches[0].Headers = []v1.HTTPHeaderMatch{ + { + Name: "Referrer", + Type: helpers.GetPointer(v1.HeaderMatchRegularExpression), + Value: "(?i)(mydomain|myotherdomain).+\\.example\\.(cloud|com)", + }, + } + l7RouteWithPolicy.Policies = []*graph.Policy{hrPolicy1, invalidPolicy} httpsHRWithPolicy, expHTTPSHRWithPolicyGroups, l7HTTPSRouteWithPolicy := createTestResources( @@ -2346,6 +2362,78 @@ func TestBuildConfiguration(t *testing.T) { }), msg: "Simple Gateway and HTTPRoute with policies attached", }, + { + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + gw := g.Gateways[gatewayNsName] + gw.Listeners = append(gw.Listeners, []*graph.Listener{ + { + Name: "listener-80-1", + GatewayName: gatewayNsName, + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(hrAdvancedRouteWithPolicy): routeHRAdvanced, + }, + }, + }...) + gw.Policies = []*graph.Policy{gwPolicy1, gwPolicy2} + g.Routes = map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(hrAdvancedRouteWithPolicy): routeHRAdvanced, + } + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.HTTPServers = []VirtualServer{ + { + IsDefault: true, + Port: 80, + Policies: []policies.Policy{gwPolicy1.Source, gwPolicy2.Source}, + }, + { + Hostname: "policy.com", + PathRules: []PathRule{ + { + Path: "/rest", + PathType: PathTypePrefix, + MatchRules: []MatchRule{ + { + BackendGroup: groupsHRAdvanced[0], + Source: &hrAdvancedRouteWithPolicy.ObjectMeta, + Match: Match{ + Headers: []HTTPHeaderMatch{ + { + Name: "Referrer", + Value: "(?i)(mydomain|myotherdomain).+\\.example\\.(cloud|com)", + Type: "RegularExpression", + }, + }, + }, + }, + }, + Policies: []policies.Policy{hrPolicy2.Source}, + }, + { + Path: "/rest", + PathType: PathTypePrefix, + MatchRules: []MatchRule{ + { + BackendGroup: groupsHRAdvanced[1], + Source: &hrAdvancedRouteWithPolicy.ObjectMeta, + }, + }, + // Policies: []policies.Policy{hrPolicy2.Source}, + }, + }, + Port: 80, + Policies: []policies.Policy{gwPolicy1.Source, gwPolicy2.Source}, + }, + } + conf.Upstreams = []Upstream{fooUpstream} + conf.BackendGroups = []BackendGroup{groupsHRAdvanced[0], groupsHRAdvanced[1]} + return conf + }), + msg: "Simple Gateway and HTTPRoute with advanced routing and policies attached", + }, { graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { gw := g.Gateways[gatewayNsName] @@ -2547,6 +2635,12 @@ func TestBuildConfiguration(t *testing.T) { false, ) + if test.msg == "Simple Gateway and HTTPRoute with advanced routing and policies attached" { + fmt.Printf("Results for %s: %+v\n", test.msg, result.HTTPServers) + fmt.Println("-----") + fmt.Printf("Expected for %s: %+v\n", test.msg, test.expConf.HTTPServers) + } + g.Expect(result.BackendGroups).To(ConsistOf(test.expConf.BackendGroups)) g.Expect(result.Upstreams).To(ConsistOf(test.expConf.Upstreams)) g.Expect(result.HTTPServers).To(ConsistOf(test.expConf.HTTPServers)) From b22f959d6ea84864b5c24b3f5760955593002ca1 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Wed, 27 Aug 2025 17:07:31 +0100 Subject: [PATCH 08/10] Add two seperate resource creations for "/rest" --- .../state/dataplane/configuration_test.go | 32 ++++++++++++------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/internal/controller/state/dataplane/configuration_test.go b/internal/controller/state/dataplane/configuration_test.go index 17b7b52e80..c5505120ad 100644 --- a/internal/controller/state/dataplane/configuration_test.go +++ b/internal/controller/state/dataplane/configuration_test.go @@ -680,15 +680,14 @@ func TestBuildConfiguration(t *testing.T) { }, ) - hrAdvancedRouteWithPolicy, groupsHRAdvanced, routeHRAdvanced := createTestResources( - "hr-advanced-route-with-policy", + hrAdvancedRouteWithPolicyAndHeaderMatch, groupsHRAdvancedWithHeaderMatch, routeHRAdvancedWithHeaderMatch := createTestResources( + "hr-advanced-route-with-policy-header-match", "policy.com", "listener-80-1", pathAndType{path: "/rest", pathType: prefix}, - pathAndType{path: "/rest", pathType: prefix}, ) - routeHRAdvanced.Spec.Rules[0].Matches[0].Headers = []v1.HTTPHeaderMatch{ + routeHRAdvancedWithHeaderMatch.Spec.Rules[0].Matches[0].Headers = []v1.HTTPHeaderMatch{ { Name: "Referrer", Type: helpers.GetPointer(v1.HeaderMatchRegularExpression), @@ -696,6 +695,13 @@ func TestBuildConfiguration(t *testing.T) { }, } + hrAdvancedRouteWithPolicyNoHeaderMatch, groupsHRAdvancedNoHeaderMatch, routeHRAdvancedNoHeaderMatch := createTestResources( + "hr-advanced-route-with-policy-no-header-match", + "policy.com", + "listener-80-1", + pathAndType{path: "/rest", pathType: prefix}, + ) + l7RouteWithPolicy.Policies = []*graph.Policy{hrPolicy1, invalidPolicy} httpsHRWithPolicy, expHTTPSHRWithPolicyGroups, l7HTTPSRouteWithPolicy := createTestResources( @@ -2372,13 +2378,15 @@ func TestBuildConfiguration(t *testing.T) { Source: listener80, Valid: true, Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(hrAdvancedRouteWithPolicy): routeHRAdvanced, + graph.CreateRouteKey(hrAdvancedRouteWithPolicyAndHeaderMatch): routeHRAdvancedWithHeaderMatch, + graph.CreateRouteKey(hrAdvancedRouteWithPolicyNoHeaderMatch): routeHRAdvancedNoHeaderMatch, }, }, }...) gw.Policies = []*graph.Policy{gwPolicy1, gwPolicy2} g.Routes = map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(hrAdvancedRouteWithPolicy): routeHRAdvanced, + graph.CreateRouteKey(hrAdvancedRouteWithPolicyAndHeaderMatch): routeHRAdvancedWithHeaderMatch, + graph.CreateRouteKey(hrAdvancedRouteWithPolicyNoHeaderMatch): routeHRAdvancedNoHeaderMatch, } return g }), @@ -2397,8 +2405,8 @@ func TestBuildConfiguration(t *testing.T) { PathType: PathTypePrefix, MatchRules: []MatchRule{ { - BackendGroup: groupsHRAdvanced[0], - Source: &hrAdvancedRouteWithPolicy.ObjectMeta, + BackendGroup: groupsHRAdvancedWithHeaderMatch[0], + Source: &hrAdvancedRouteWithPolicyAndHeaderMatch.ObjectMeta, Match: Match{ Headers: []HTTPHeaderMatch{ { @@ -2417,11 +2425,11 @@ func TestBuildConfiguration(t *testing.T) { PathType: PathTypePrefix, MatchRules: []MatchRule{ { - BackendGroup: groupsHRAdvanced[1], - Source: &hrAdvancedRouteWithPolicy.ObjectMeta, + BackendGroup: groupsHRAdvancedNoHeaderMatch[0], + Source: &hrAdvancedRouteWithPolicyNoHeaderMatch.ObjectMeta, }, }, - // Policies: []policies.Policy{hrPolicy2.Source}, + Policies: []policies.Policy{hrPolicy2.Source}, }, }, Port: 80, @@ -2429,7 +2437,7 @@ func TestBuildConfiguration(t *testing.T) { }, } conf.Upstreams = []Upstream{fooUpstream} - conf.BackendGroups = []BackendGroup{groupsHRAdvanced[0], groupsHRAdvanced[1]} + conf.BackendGroups = []BackendGroup{groupsHRAdvancedWithHeaderMatch[0], groupsHRAdvancedNoHeaderMatch[0]} return conf }), msg: "Simple Gateway and HTTPRoute with advanced routing and policies attached", From 4642eb928af63ac721132175660bedeeaea834f0 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Thu, 28 Aug 2025 15:27:12 +0100 Subject: [PATCH 09/10] Update test case to ensure it passes only if the same policy appears once in a PathRule --- .../state/dataplane/configuration_test.go | 54 ++++++++----------- 1 file changed, 23 insertions(+), 31 deletions(-) diff --git a/internal/controller/state/dataplane/configuration_test.go b/internal/controller/state/dataplane/configuration_test.go index 539c558e93..b9d2216f0d 100644 --- a/internal/controller/state/dataplane/configuration_test.go +++ b/internal/controller/state/dataplane/configuration_test.go @@ -687,20 +687,24 @@ func TestBuildConfiguration(t *testing.T) { pathAndType{path: "/rest", pathType: prefix}, ) - routeHRAdvancedWithHeaderMatch.Spec.Rules[0].Matches[0].Headers = []v1.HTTPHeaderMatch{ + pathMatch := helpers.GetPointer(v1.HTTPPathMatch{Value: helpers.GetPointer("/rest"), Type: helpers.GetPointer(v1.PathMatchPathPrefix)}) + + routeHRAdvancedWithHeaderMatch.Spec.Rules[0].Matches = []v1.HTTPRouteMatch{ { - Name: "Referrer", - Type: helpers.GetPointer(v1.HeaderMatchRegularExpression), - Value: "(?i)(mydomain|myotherdomain).+\\.example\\.(cloud|com)", + Path: pathMatch, + Headers: []v1.HTTPHeaderMatch{ + { + Name: "Referrer", + Type: helpers.GetPointer(v1.HeaderMatchRegularExpression), + Value: "(?i)(mydomain|myotherdomain).+\\.example\\.(cloud|com)", + }, + }, + }, + { + Path: pathMatch, }, } - - hrAdvancedRouteWithPolicyNoHeaderMatch, groupsHRAdvancedNoHeaderMatch, routeHRAdvancedNoHeaderMatch := createTestResources( - "hr-advanced-route-with-policy-no-header-match", - "policy.com", - "listener-80-1", - pathAndType{path: "/rest", pathType: prefix}, - ) + routeHRAdvancedWithHeaderMatch.Spec.Hostnames = []v1.Hostname{"policy.com"} l7RouteWithPolicy.Policies = []*graph.Policy{hrPolicy1, invalidPolicy} @@ -2379,18 +2383,19 @@ func TestBuildConfiguration(t *testing.T) { Valid: true, Routes: map[graph.RouteKey]*graph.L7Route{ graph.CreateRouteKey(hrAdvancedRouteWithPolicyAndHeaderMatch): routeHRAdvancedWithHeaderMatch, - graph.CreateRouteKey(hrAdvancedRouteWithPolicyNoHeaderMatch): routeHRAdvancedNoHeaderMatch, }, }, }...) gw.Policies = []*graph.Policy{gwPolicy1, gwPolicy2} + routeHRAdvancedWithHeaderMatch.Policies = []*graph.Policy{hrPolicy1} g.Routes = map[graph.RouteKey]*graph.L7Route{ graph.CreateRouteKey(hrAdvancedRouteWithPolicyAndHeaderMatch): routeHRAdvancedWithHeaderMatch, - graph.CreateRouteKey(hrAdvancedRouteWithPolicyNoHeaderMatch): routeHRAdvancedNoHeaderMatch, } return g }), expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.SSLServers = []VirtualServer{} + conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} conf.HTTPServers = []VirtualServer{ { IsDefault: true, @@ -2417,19 +2422,12 @@ func TestBuildConfiguration(t *testing.T) { }, }, }, - }, - Policies: []policies.Policy{hrPolicy2.Source}, - }, - { - Path: "/rest", - PathType: PathTypePrefix, - MatchRules: []MatchRule{ { - BackendGroup: groupsHRAdvancedNoHeaderMatch[0], - Source: &hrAdvancedRouteWithPolicyNoHeaderMatch.ObjectMeta, + BackendGroup: groupsHRAdvancedWithHeaderMatch[0], + Source: &hrAdvancedRouteWithPolicyAndHeaderMatch.ObjectMeta, }, }, - Policies: []policies.Policy{hrPolicy2.Source}, + Policies: []policies.Policy{hrPolicy1.Source}, }, }, Port: 80, @@ -2437,10 +2435,10 @@ func TestBuildConfiguration(t *testing.T) { }, } conf.Upstreams = []Upstream{fooUpstream} - conf.BackendGroups = []BackendGroup{groupsHRAdvancedWithHeaderMatch[0], groupsHRAdvancedNoHeaderMatch[0]} + conf.BackendGroups = []BackendGroup{groupsHRAdvancedWithHeaderMatch[0]} return conf }), - msg: "Simple Gateway and HTTPRoute with advanced routing and policies attached", + msg: "Gateway and HTTPRoute with policies attached with advanced routing", }, { graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { @@ -2643,12 +2641,6 @@ func TestBuildConfiguration(t *testing.T) { false, ) - if test.msg == "Simple Gateway and HTTPRoute with advanced routing and policies attached" { - fmt.Printf("Results for %s: %+v\n", test.msg, result.HTTPServers) - fmt.Println("-----") - fmt.Printf("Expected for %s: %+v\n", test.msg, test.expConf.HTTPServers) - } - g.Expect(result.BackendGroups).To(ConsistOf(test.expConf.BackendGroups)) g.Expect(result.Upstreams).To(ConsistOf(test.expConf.Upstreams)) g.Expect(result.HTTPServers).To(ConsistOf(test.expConf.HTTPServers)) From 9839fd4af1294c891971d8d555f01d4ae3d01799 Mon Sep 17 00:00:00 2001 From: shaun-nx Date: Thu, 28 Aug 2025 15:34:30 +0100 Subject: [PATCH 10/10] Fix linter errors --- .../controller/state/dataplane/configuration_test.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/internal/controller/state/dataplane/configuration_test.go b/internal/controller/state/dataplane/configuration_test.go index b9d2216f0d..b329b9d46a 100644 --- a/internal/controller/state/dataplane/configuration_test.go +++ b/internal/controller/state/dataplane/configuration_test.go @@ -680,14 +680,19 @@ func TestBuildConfiguration(t *testing.T) { }, ) - hrAdvancedRouteWithPolicyAndHeaderMatch, groupsHRAdvancedWithHeaderMatch, routeHRAdvancedWithHeaderMatch := createTestResources( + hrAdvancedRouteWithPolicyAndHeaderMatch, + groupsHRAdvancedWithHeaderMatch, + routeHRAdvancedWithHeaderMatch := createTestResources( "hr-advanced-route-with-policy-header-match", "policy.com", "listener-80-1", pathAndType{path: "/rest", pathType: prefix}, ) - pathMatch := helpers.GetPointer(v1.HTTPPathMatch{Value: helpers.GetPointer("/rest"), Type: helpers.GetPointer(v1.PathMatchPathPrefix)}) + pathMatch := helpers.GetPointer(v1.HTTPPathMatch{ + Value: helpers.GetPointer("/rest"), + Type: helpers.GetPointer(v1.PathMatchPathPrefix), + }) routeHRAdvancedWithHeaderMatch.Spec.Rules[0].Matches = []v1.HTTPRouteMatch{ {