From e8cca518bc16e0ecd11bde36c57677a8bafa4a01 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Tue, 15 Jul 2025 11:54:33 -0700 Subject: [PATCH 01/13] Add support for percentage based request mirroring --- .../controller/nginx/config/http/config.go | 25 +- internal/controller/nginx/config/servers.go | 140 ++++- .../nginx/config/servers_template.go | 6 + .../controller/nginx/config/servers_test.go | 294 +++++++-- .../controller/nginx/config/split_clients.go | 77 ++- .../nginx/config/split_clients_template.go | 4 +- .../nginx/config/split_clients_test.go | 561 ++++++++++++++++-- .../state/dataplane/configuration.go | 10 +- .../state/dataplane/configuration_test.go | 19 +- .../controller/state/dataplane/convert.go | 21 +- .../state/dataplane/convert_test.go | 68 ++- internal/controller/state/dataplane/types.go | 4 +- .../controller/state/graph/common_filter.go | 34 ++ .../state/graph/common_filter_test.go | 160 ++++- internal/controller/state/graph/grpcroute.go | 11 +- .../controller/state/graph/grpcroute_test.go | 4 +- internal/controller/state/graph/httproute.go | 11 +- .../controller/state/graph/httproute_test.go | 4 +- internal/controller/state/mirror/mirror.go | 29 +- .../controller/state/mirror/mirror_test.go | 53 +- tests/Makefile | 2 +- 21 files changed, 1324 insertions(+), 213 deletions(-) diff --git a/internal/controller/nginx/config/http/config.go b/internal/controller/nginx/config/http/config.go index f1ae092fc0..2ae51a6bd6 100644 --- a/internal/controller/nginx/config/http/config.go +++ b/internal/controller/nginx/config/http/config.go @@ -33,18 +33,19 @@ const ( // Location holds all configuration for an HTTP location. type Location struct { - Path string - ProxyPass string - HTTPMatchKey string - Type LocationType - ProxySetHeaders []Header - ProxySSLVerify *ProxySSLVerify - Return *Return - ResponseHeaders ResponseHeaders - Rewrites []string - MirrorPaths []string - Includes []shared.Include - GRPC bool + Path string + ProxyPass string + HTTPMatchKey string + MirrorSplitClientsVariableName string + Type LocationType + ProxySetHeaders []Header + ProxySSLVerify *ProxySSLVerify + Return *Return + ResponseHeaders ResponseHeaders + Rewrites []string + MirrorPaths []string + Includes []shared.Include + GRPC bool } // Header defines an HTTP header to be passed to the proxied server. diff --git a/internal/controller/nginx/config/servers.go b/internal/controller/nginx/config/servers.go index 629237f8c9..06d17c1847 100644 --- a/internal/controller/nginx/config/servers.go +++ b/internal/controller/nginx/config/servers.go @@ -224,6 +224,32 @@ type rewriteConfig struct { MainRewrite string } +// extractMirrorTargetsWithPercentages extracts mirror targets and their percentages from path rules. +func extractMirrorTargetsWithPercentages(pathRules []dataplane.PathRule) map[string]float64 { + mirrorTargets := make(map[string]float64) + + for _, rule := range pathRules { + for _, matchRule := range rule.MatchRules { + for _, mirrorFilter := range matchRule.Filters.RequestMirrors { + if mirrorFilter.Target != nil { + if mirrorFilter.Percent == nil { + mirrorTargets[*mirrorFilter.Target] = 100.0 + continue + } + + percentage := *mirrorFilter.Percent + + if _, exists := mirrorTargets[*mirrorFilter.Target]; !exists || percentage > mirrorTargets[*mirrorFilter.Target] { + mirrorTargets[*mirrorFilter.Target] = percentage // set a higher percentage if it exists + } + } + } + } + } + + return mirrorTargets +} + type httpMatchPairs map[string][]routeMatch func createLocations( @@ -239,6 +265,8 @@ func createLocations( var rootPathExists bool var grpcServer bool + mirrorPathToPercentage := extractMirrorTargetsWithPercentages(server.PathRules) + for pathRuleIdx, rule := range server.PathRules { matches := make([]routeMatch, 0, len(rule.MatchRules)) @@ -250,6 +278,11 @@ func createLocations( grpcServer = true } + mirrorPercentage, exists := mirrorPathToPercentage[rule.Path] + if !exists { + mirrorPercentage = -1 + } + extLocations := initializeExternalLocations(rule, pathsAndTypes) for i := range extLocations { extLocations[i].Includes = createIncludesFromPolicyGenerateResult( @@ -267,6 +300,7 @@ func createLocations( rule.Path, rule.GRPC, keepAliveCheck, + mirrorPercentage, ) } @@ -290,6 +324,7 @@ func createLocations( rule.Path, rule.GRPC, keepAliveCheck, + mirrorPercentage, ) internalLocations = append(internalLocations, intLocation) @@ -427,12 +462,28 @@ func updateLocation( path string, grpc bool, keepAliveCheck keepAliveChecker, + mirrorPercentage float64, ) http.Location { if filters.InvalidFilter != nil { location.Return = &http.Return{Code: http.StatusInternalServerError} return location } + location = updateLocationMirrorRoute(location, path, grpc) + location.Includes = append(location.Includes, createIncludesFromLocationSnippetsFilters(filters.SnippetsFilters)...) + + if filters.RequestRedirect != nil { + return updateLocationRedirectFilter(location, filters.RequestRedirect, listenerPort, path) + } + + location = updateLocationRewriteFilter(location, filters.RequestURLRewrite, path) + location = updateLocationMirrorFilters(location, filters.RequestMirrors, path, mirrorPercentage) + location = updateLocationProxySettings(location, matchRule, grpc, keepAliveCheck) + + return location +} + +func updateLocationMirrorRoute(location http.Location, path string, grpc bool) http.Location { if strings.HasPrefix(path, http.InternalMirrorRoutePathPrefix) { location.Type = http.InternalLocationType if grpc { @@ -440,18 +491,30 @@ func updateLocation( } } - location.Includes = append(location.Includes, createIncludesFromLocationSnippetsFilters(filters.SnippetsFilters)...) + return location +} - if filters.RequestRedirect != nil { - ret, rewrite := createReturnAndRewriteConfigForRedirectFilter(filters.RequestRedirect, listenerPort, path) - if rewrite.MainRewrite != "" { - location.Rewrites = append(location.Rewrites, rewrite.MainRewrite) - } - location.Return = ret - return location +func updateLocationRedirectFilter( + location http.Location, + redirectFilter *dataplane.HTTPRequestRedirectFilter, + listenerPort int32, + path string, +) http.Location { + ret, rewrite := createReturnAndRewriteConfigForRedirectFilter(redirectFilter, listenerPort, path) + if rewrite.MainRewrite != "" { + location.Rewrites = append(location.Rewrites, rewrite.MainRewrite) } + location.Return = ret - rewrites := createRewritesValForRewriteFilter(filters.RequestURLRewrite, path) + return location +} + +func updateLocationRewriteFilter( + location http.Location, + rewriteFilter *dataplane.HTTPURLRewriteFilter, + path string, +) http.Location { + rewrites := createRewritesValForRewriteFilter(rewriteFilter, path) if rewrites != nil { if location.Type == http.InternalLocationType && rewrites.InternalRewrite != "" { location.Rewrites = append(location.Rewrites, rewrites.InternalRewrite) @@ -461,12 +524,42 @@ func updateLocation( } } - for _, filter := range filters.RequestMirrors { + return location +} + +func updateLocationMirrorFilters( + location http.Location, + mirrorFilters []*dataplane.HTTPRequestMirrorFilter, + path string, + mirrorPercentage float64, +) http.Location { + for _, filter := range mirrorFilters { if filter.Target != nil { location.MirrorPaths = append(location.MirrorPaths, *filter.Target) } } + if location.MirrorPaths != nil { + location.MirrorPaths = deduplicateStrings(location.MirrorPaths) + } + + // if the mirrorPercentage is 100.0 or negative (we set it to negative when there is no mirror filter because 0.0 + // is valid), the split clients variable is not generated, and we want to let all the traffic get mirrored. + if mirrorPercentage != 100.0 && mirrorPercentage >= 0.0 { + location.MirrorSplitClientsVariableName = convertSplitClientVariableName( + fmt.Sprintf("%s_%.2f", path, mirrorPercentage), + ) + } + + return location +} + +func updateLocationProxySettings( + location http.Location, + matchRule dataplane.MatchRule, + grpc bool, + keepAliveCheck keepAliveChecker, +) http.Location { extraHeaders := make([]http.Header, 0, 3) if grpc { extraHeaders = append(extraHeaders, grpcAuthorityHeader) @@ -504,11 +597,21 @@ func updateLocations( path string, grpc bool, keepAliveCheck keepAliveChecker, + mirrorPercentage float64, ) []http.Location { updatedLocations := make([]http.Location, len(buildLocations)) for i, loc := range buildLocations { - updatedLocations[i] = updateLocation(filters, loc, matchRule, listenerPort, path, grpc, keepAliveCheck) + updatedLocations[i] = updateLocation( + filters, + loc, + matchRule, + listenerPort, + path, + grpc, + keepAliveCheck, + mirrorPercentage, + ) } return updatedLocations @@ -962,3 +1065,18 @@ func getConnectionHeader(keepAliveCheck keepAliveChecker, backends []dataplane.B return httpConnectionHeader } + +// deduplicateStrings removes duplicate strings from a slice while preserving order. +func deduplicateStrings(content []string) []string { + seen := make(map[string]bool) + result := make([]string, 0, len(content)) + + for _, str := range content { + if !seen[str] { + seen[str] = true + result = append(result, str) + } + } + + return result +} diff --git a/internal/controller/nginx/config/servers_template.go b/internal/controller/nginx/config/servers_template.go index 6676426b86..b49884a94b 100644 --- a/internal/controller/nginx/config/servers_template.go +++ b/internal/controller/nginx/config/servers_template.go @@ -93,6 +93,12 @@ server { internal; {{ end }} + {{ if ne $l.MirrorSplitClientsVariableName "" -}} + if (${{ $l.MirrorSplitClientsVariableName }} = "") { + return 204; + } + {{- end }} + {{- range $i := $l.Includes }} include {{ $i.Name }}; {{- end -}} diff --git a/internal/controller/nginx/config/servers_test.go b/internal/controller/nginx/config/servers_test.go index b29550eb8a..b1313338ca 100644 --- a/internal/controller/nginx/config/servers_test.go +++ b/internal/controller/nginx/config/servers_test.go @@ -62,6 +62,14 @@ func TestExecuteServers(t *testing.T) { }, }, }, + RequestMirrors: []*dataplane.HTTPRequestMirrorFilter{ + { + Name: helpers.GetPointer("mirror-filter"), + Namespace: helpers.GetPointer("test-ns"), + Target: helpers.GetPointer(http.InternalMirrorRoutePathPrefix + "-my-backend-test/route1-0"), + Percent: helpers.GetPointer(float64(50)), + }, + }, }, Match: dataplane.Match{}, BackendGroup: dataplane.BackendGroup{ @@ -78,6 +86,26 @@ func TestExecuteServers(t *testing.T) { }, }, }, + { + Path: http.InternalMirrorRoutePathPrefix + "-my-backend-test/route1-0", + PathType: dataplane.PathTypeExact, + MatchRules: []dataplane.MatchRule{ + { + Match: dataplane.Match{}, + BackendGroup: dataplane.BackendGroup{ + Source: types.NamespacedName{Namespace: "test", Name: "route1"}, + RuleIdx: 0, + Backends: []dataplane.Backend{ + { + UpstreamName: "test_foo_443", + Valid: true, + Weight: 1, + }, + }, + }, + }, + }, + }, }, }, }, @@ -133,18 +161,21 @@ func TestExecuteServers(t *testing.T) { } expSubStrings := map[string]int{ - "listen 8080 default_server;": 1, - "listen 8080;": 2, - "listen 8443 ssl;": 2, - "listen 8443 ssl default_server;": 1, - "server_name example.com;": 2, - "server_name cafe.example.com;": 2, - "ssl_certificate /etc/nginx/secrets/test-keypair.pem;": 2, - "ssl_certificate_key /etc/nginx/secrets/test-keypair.pem;": 2, - "proxy_ssl_server_name on;": 1, - "status_zone": 0, - "include /etc/nginx/includes/location-snippet.conf": 1, - "include /etc/nginx/includes/server-snippet.conf": 1, + "listen 8080 default_server;": 1, + "listen 8080;": 2, + "listen 8443 ssl;": 2, + "listen 8443 ssl default_server;": 1, + "server_name example.com;": 2, + "server_name cafe.example.com;": 2, + "ssl_certificate /etc/nginx/secrets/test-keypair.pem;": 2, + "ssl_certificate_key /etc/nginx/secrets/test-keypair.pem;": 2, + "proxy_ssl_server_name on;": 1, + "status_zone": 0, + "include /etc/nginx/includes/location-snippet.conf": 1, + "include /etc/nginx/includes/server-snippet.conf": 1, + "mirror /_ngf-internal-mirror-my-backend-test/route1-0;": 1, + "if ($__ngf_internal_mirror_my_backend_test_route1_0_50_00 = \"\")": 1, + "return 204": 1, } type assertion func(g *WithT, data string) @@ -874,7 +905,37 @@ func TestCreateServers(t *testing.T) { { Name: helpers.GetPointer("mirror-filter"), Namespace: helpers.GetPointer("test-ns"), - Target: helpers.GetPointer(http.InternalMirrorRoutePathPrefix + "-my-backend"), + Target: helpers.GetPointer(http.InternalMirrorRoutePathPrefix + "-my-backend-test/route1-0"), + }, + }, + }, + BackendGroup: fooGroup, + }, + }, + }, + { + Path: http.InternalMirrorRoutePathPrefix + "-my-backend-test/route1-0", + PathType: dataplane.PathTypeExact, + MatchRules: []dataplane.MatchRule{ + { + Match: dataplane.Match{}, + BackendGroup: fooGroup, + }, + }, + }, + { + Path: "/mirror-filter-percentage-defined", + PathType: dataplane.PathTypeExact, + MatchRules: []dataplane.MatchRule{ + { + Match: dataplane.Match{}, + Filters: dataplane.HTTPFilters{ + RequestMirrors: []*dataplane.HTTPRequestMirrorFilter{ + { + Name: helpers.GetPointer("mirror-filter-percentage-defined"), + Namespace: helpers.GetPointer("test-ns"), + Target: helpers.GetPointer(http.InternalMirrorRoutePathPrefix + "-my-backend-test/route1-1"), + Percent: helpers.GetPointer(float64(50)), }, }, }, @@ -883,7 +944,109 @@ func TestCreateServers(t *testing.T) { }, }, { - Path: http.InternalMirrorRoutePathPrefix + "-my-backend", + Path: http.InternalMirrorRoutePathPrefix + "-my-backend-test/route1-1", + PathType: dataplane.PathTypeExact, + MatchRules: []dataplane.MatchRule{ + { + Match: dataplane.Match{}, + BackendGroup: fooGroup, + }, + }, + }, + { + Path: "/mirror-filter-100-percent", + PathType: dataplane.PathTypeExact, + MatchRules: []dataplane.MatchRule{ + { + Match: dataplane.Match{}, + Filters: dataplane.HTTPFilters{ + RequestMirrors: []*dataplane.HTTPRequestMirrorFilter{ + { + Name: helpers.GetPointer("mirror-filter-100-percent"), + Namespace: helpers.GetPointer("test-ns"), + Target: helpers.GetPointer(http.InternalMirrorRoutePathPrefix + "-my-backend-test/route1-2"), + Percent: helpers.GetPointer(float64(100)), + }, + }, + }, + BackendGroup: fooGroup, + }, + }, + }, + { + Path: http.InternalMirrorRoutePathPrefix + "-my-backend-test/route1-2", + PathType: dataplane.PathTypeExact, + MatchRules: []dataplane.MatchRule{ + { + Match: dataplane.Match{}, + BackendGroup: fooGroup, + }, + }, + }, + { + Path: "/mirror-filter-0-percent", + PathType: dataplane.PathTypeExact, + MatchRules: []dataplane.MatchRule{ + { + Match: dataplane.Match{}, + Filters: dataplane.HTTPFilters{ + RequestMirrors: []*dataplane.HTTPRequestMirrorFilter{ + { + Name: helpers.GetPointer("mirror-filter-0-percent"), + Namespace: helpers.GetPointer("test-ns"), + Target: helpers.GetPointer(http.InternalMirrorRoutePathPrefix + "-my-backend-test/route1-3"), + Percent: helpers.GetPointer(float64(0)), + }, + }, + }, + BackendGroup: fooGroup, + }, + }, + }, + { + Path: http.InternalMirrorRoutePathPrefix + "-my-backend-test/route1-3", + PathType: dataplane.PathTypeExact, + MatchRules: []dataplane.MatchRule{ + { + Match: dataplane.Match{}, + BackendGroup: fooGroup, + }, + }, + }, + { + Path: "/mirror-filter-duplicate-targets", + PathType: dataplane.PathTypeExact, + MatchRules: []dataplane.MatchRule{ + { + Match: dataplane.Match{}, + Filters: dataplane.HTTPFilters{ + RequestMirrors: []*dataplane.HTTPRequestMirrorFilter{ + { + Name: helpers.GetPointer("mirror-filter-duplicate-targets-0-percent"), + Namespace: helpers.GetPointer("test-ns"), + Target: helpers.GetPointer(http.InternalMirrorRoutePathPrefix + "-my-backend-test/route1-4"), + Percent: helpers.GetPointer(float64(0)), + }, + { + Name: helpers.GetPointer("mirror-filter-duplicate-targets-25-percent"), + Namespace: helpers.GetPointer("test-ns"), + Target: helpers.GetPointer(http.InternalMirrorRoutePathPrefix + "-my-backend-test/route1-4"), + Percent: helpers.GetPointer(float64(25)), + }, + { + Name: helpers.GetPointer("mirror-filter-duplicate-targets-50-percent"), + Namespace: helpers.GetPointer("test-ns"), + Target: helpers.GetPointer(http.InternalMirrorRoutePathPrefix + "-my-backend-test/route1-4"), + Percent: helpers.GetPointer(float64(50)), + }, + }, + }, + BackendGroup: fooGroup, + }, + }, + }, + { + Path: http.InternalMirrorRoutePathPrefix + "-my-backend-test/route1-4", PathType: dataplane.PathTypeExact, MatchRules: []dataplane.MatchRule{ { @@ -903,7 +1066,7 @@ func TestCreateServers(t *testing.T) { { Name: helpers.GetPointer("grpc-mirror-filter"), Namespace: helpers.GetPointer("test-ns"), - Target: helpers.GetPointer(http.InternalMirrorRoutePathPrefix + "-my-grpc-backend"), + Target: helpers.GetPointer(http.InternalMirrorRoutePathPrefix + "-my-grpc-backend-test/route1-0"), }, }, }, @@ -913,7 +1076,7 @@ func TestCreateServers(t *testing.T) { GRPC: true, }, { - Path: http.InternalMirrorRoutePathPrefix + "-my-grpc-backend", + Path: http.InternalMirrorRoutePathPrefix + "-my-grpc-backend-test/route1-0", PathType: dataplane.PathTypeExact, MatchRules: []dataplane.MatchRule{ { @@ -1154,25 +1317,25 @@ func TestCreateServers(t *testing.T) { RedirectPath: "/_ngf-internal-rule8-route0", }, }, - "1_14": { + "1_22": { { Headers: []string{"filter:Exact:this"}, - RedirectPath: "/_ngf-internal-rule14-route0", + RedirectPath: "/_ngf-internal-rule22-route0", }, }, - "1_16": { + "1_24": { { Method: "GET", - RedirectPath: "/_ngf-internal-rule16-route0", + RedirectPath: "/_ngf-internal-rule24-route0", Headers: nil, QueryParams: nil, Any: false, }, }, - "1_21": { + "1_29": { { Method: "GET", - RedirectPath: "/_ngf-internal-rule21-route0", + RedirectPath: "/_ngf-internal-rule29-route0", }, }, } @@ -1407,7 +1570,7 @@ func TestCreateServers(t *testing.T) { Path: "/mirror/", ProxyPass: "http://test_foo_80$request_uri", ProxySetHeaders: httpBaseHeaders, - MirrorPaths: []string{"/_ngf-internal-mirror-my-backend"}, + MirrorPaths: []string{"/_ngf-internal-mirror-my-backend-test/route1-0"}, Type: http.ExternalLocationType, Includes: externalIncludes, }, @@ -1415,28 +1578,91 @@ func TestCreateServers(t *testing.T) { Path: "= /mirror", ProxyPass: "http://test_foo_80$request_uri", ProxySetHeaders: httpBaseHeaders, - MirrorPaths: []string{"/_ngf-internal-mirror-my-backend"}, + MirrorPaths: []string{"/_ngf-internal-mirror-my-backend-test/route1-0"}, + Type: http.ExternalLocationType, + Includes: externalIncludes, + }, + { + Path: "= /_ngf-internal-mirror-my-backend-test/route1-0", + ProxyPass: "http://test_foo_80$request_uri", + ProxySetHeaders: httpBaseHeaders, + Type: http.InternalLocationType, + Includes: externalIncludes, + }, + { + Path: "= /mirror-filter-percentage-defined", + ProxyPass: "http://test_foo_80$request_uri", + ProxySetHeaders: httpBaseHeaders, + MirrorPaths: []string{"/_ngf-internal-mirror-my-backend-test/route1-1"}, + Type: http.ExternalLocationType, + Includes: externalIncludes, + }, + { + Path: "= /_ngf-internal-mirror-my-backend-test/route1-1", + ProxyPass: "http://test_foo_80$request_uri", + ProxySetHeaders: httpBaseHeaders, + MirrorSplitClientsVariableName: "__ngf_internal_mirror_my_backend_test_route1_1_50_00", + Type: http.InternalLocationType, + Includes: externalIncludes, + }, + { + Path: "= /mirror-filter-100-percent", + ProxyPass: "http://test_foo_80$request_uri", + ProxySetHeaders: httpBaseHeaders, + MirrorPaths: []string{"/_ngf-internal-mirror-my-backend-test/route1-2"}, Type: http.ExternalLocationType, Includes: externalIncludes, }, { - Path: "= /_ngf-internal-mirror-my-backend", + Path: "= /_ngf-internal-mirror-my-backend-test/route1-2", ProxyPass: "http://test_foo_80$request_uri", ProxySetHeaders: httpBaseHeaders, Type: http.InternalLocationType, Includes: externalIncludes, }, + { + Path: "= /mirror-filter-0-percent", + ProxyPass: "http://test_foo_80$request_uri", + ProxySetHeaders: httpBaseHeaders, + MirrorPaths: []string{"/_ngf-internal-mirror-my-backend-test/route1-3"}, + Type: http.ExternalLocationType, + Includes: externalIncludes, + }, + { + Path: "= /_ngf-internal-mirror-my-backend-test/route1-3", + ProxyPass: "http://test_foo_80$request_uri", + ProxySetHeaders: httpBaseHeaders, + MirrorSplitClientsVariableName: "__ngf_internal_mirror_my_backend_test_route1_3_0_00", + Type: http.InternalLocationType, + Includes: externalIncludes, + }, + { + Path: "= /mirror-filter-duplicate-targets", + ProxyPass: "http://test_foo_80$request_uri", + ProxySetHeaders: httpBaseHeaders, + MirrorPaths: []string{"/_ngf-internal-mirror-my-backend-test/route1-4"}, + Type: http.ExternalLocationType, + Includes: externalIncludes, + }, + { + Path: "= /_ngf-internal-mirror-my-backend-test/route1-4", + ProxyPass: "http://test_foo_80$request_uri", + ProxySetHeaders: httpBaseHeaders, + MirrorSplitClientsVariableName: "__ngf_internal_mirror_my_backend_test_route1_4_50_00", + Type: http.InternalLocationType, + Includes: externalIncludes, + }, { Path: "= /grpc/mirror", GRPC: true, ProxyPass: "grpc://test_foo_80", ProxySetHeaders: grpcBaseHeaders, - MirrorPaths: []string{"/_ngf-internal-mirror-my-grpc-backend"}, + MirrorPaths: []string{"/_ngf-internal-mirror-my-grpc-backend-test/route1-0"}, Type: http.ExternalLocationType, Includes: externalIncludes, }, { - Path: "= /_ngf-internal-mirror-my-grpc-backend", + Path: "= /_ngf-internal-mirror-my-grpc-backend-test/route1-0", GRPC: true, ProxyPass: "grpc://test_foo_80", Rewrites: []string{"^ $request_uri break"}, @@ -1462,18 +1688,18 @@ func TestCreateServers(t *testing.T) { }, { Path: "/invalid-filter-with-headers/", - HTTPMatchKey: ssl + "1_14", + HTTPMatchKey: ssl + "1_22", Type: http.RedirectLocationType, Includes: externalIncludes, }, { Path: "= /invalid-filter-with-headers", - HTTPMatchKey: ssl + "1_14", + HTTPMatchKey: ssl + "1_22", Type: http.RedirectLocationType, Includes: externalIncludes, }, { - Path: "/_ngf-internal-rule14-route0", + Path: "/_ngf-internal-rule22-route0", Return: &http.Return{ Code: http.StatusInternalServerError, }, @@ -1489,12 +1715,12 @@ func TestCreateServers(t *testing.T) { }, { Path: "= /test", - HTTPMatchKey: ssl + "1_16", + HTTPMatchKey: ssl + "1_24", Type: http.RedirectLocationType, Includes: externalIncludes, }, { - Path: "/_ngf-internal-rule16-route0", + Path: "/_ngf-internal-rule24-route0", ProxyPass: "http://test_foo_80$request_uri", ProxySetHeaders: httpBaseHeaders, Type: http.InternalLocationType, @@ -1573,12 +1799,12 @@ func TestCreateServers(t *testing.T) { }, { Path: "= /include-header-match", - HTTPMatchKey: ssl + "1_21", + HTTPMatchKey: ssl + "1_29", Type: http.RedirectLocationType, Includes: externalIncludes, }, { - Path: "/_ngf-internal-rule21-route0", + Path: "/_ngf-internal-rule29-route0", ProxyPass: "http://test_foo_80$request_uri", ProxySetHeaders: httpBaseHeaders, Type: http.InternalLocationType, diff --git a/internal/controller/nginx/config/split_clients.go b/internal/controller/nginx/config/split_clients.go index 502859d029..4019eb3c72 100644 --- a/internal/controller/nginx/config/split_clients.go +++ b/internal/controller/nginx/config/split_clients.go @@ -3,6 +3,7 @@ package config import ( "fmt" "math" + "strings" gotemplate "text/template" "github.com/nginx/nginx-gateway-fabric/internal/controller/nginx/config/http" @@ -13,7 +14,7 @@ import ( var splitClientsTemplate = gotemplate.Must(gotemplate.New("split_clients").Parse(splitClientsTemplateText)) func executeSplitClients(conf dataplane.Configuration) []executeResult { - splitClients := createSplitClients(conf.BackendGroups) + splitClients := collectAllSplitClients(conf) result := executeResult{ dest: httpConfigFile, @@ -23,7 +24,75 @@ func executeSplitClients(conf dataplane.Configuration) []executeResult { return []executeResult{result} } -func createSplitClients(backendGroups []dataplane.BackendGroup) []http.SplitClient { +func collectAllSplitClients(conf dataplane.Configuration) []http.SplitClient { + var splitClients []http.SplitClient + + splitClients = append(splitClients, createBackendGroupSplitClients(conf.BackendGroups)...) + splitClients = append(splitClients, createRequestMirrorSplitClients(conf.HTTPServers)...) + splitClients = append(splitClients, createRequestMirrorSplitClients(conf.SSLServers)...) + splitClients = removeDuplicateSplitClients(splitClients) + + return splitClients +} + +func createRequestMirrorSplitClients(servers []dataplane.VirtualServer) []http.SplitClient { + var splitClients []http.SplitClient + + for _, server := range servers { + mirrorPathToPercentage := extractMirrorTargetsWithPercentages(server.PathRules) + + for _, pathRule := range server.PathRules { + mirrorPercentage, exists := mirrorPathToPercentage[pathRule.Path] + + if mirrorPercentage != 100 && exists { + splitClient := http.SplitClient{ + // this has to be something unique and able to be accessed from the server side + VariableName: convertSplitClientVariableName(fmt.Sprintf("%s_%.2f", pathRule.Path, mirrorPercentage)), + Distributions: []http.SplitClientDistribution{ + { + Percent: fmt.Sprintf("%.2f", mirrorPercentage), + Value: pathRule.Path, + }, + { + Percent: "*", + Value: "\"\"", + }, + }, + } + + splitClients = append(splitClients, splitClient) + } + } + } + + return splitClients +} + +// convertSplitClientVariableName converts a name to a safe variable name for split clients. This includes +// replacing hypens, slashes, and dots with underscores. +func convertSplitClientVariableName(name string) string { + safeName := convertStringToSafeVariableName(name) + safeName = strings.ReplaceAll(safeName, "/", "_") + safeName = strings.ReplaceAll(safeName, ".", "_") + + return safeName +} + +func removeDuplicateSplitClients(splitClients []http.SplitClient) []http.SplitClient { + seen := make(map[string]bool) + result := make([]http.SplitClient, 0, len(splitClients)) + + for _, client := range splitClients { + if !seen[client.VariableName] { + seen[client.VariableName] = true + result = append(result, client) + } + } + + return result +} + +func createBackendGroupSplitClients(backendGroups []dataplane.BackendGroup) []http.SplitClient { numSplits := 0 for _, group := range backendGroups { if backendGroupNeedsSplit(group) { @@ -38,7 +107,7 @@ func createSplitClients(backendGroups []dataplane.BackendGroup) []http.SplitClie splitClients := make([]http.SplitClient, 0, numSplits) for _, group := range backendGroups { - distributions := createSplitClientDistributions(group) + distributions := createBackendGroupSplitClientDistributions(group) if distributions == nil { continue } @@ -52,7 +121,7 @@ func createSplitClients(backendGroups []dataplane.BackendGroup) []http.SplitClie return splitClients } -func createSplitClientDistributions(group dataplane.BackendGroup) []http.SplitClientDistribution { +func createBackendGroupSplitClientDistributions(group dataplane.BackendGroup) []http.SplitClientDistribution { if !backendGroupNeedsSplit(group) { return nil } diff --git a/internal/controller/nginx/config/split_clients_template.go b/internal/controller/nginx/config/split_clients_template.go index 6d2a0e208f..d3192e0040 100644 --- a/internal/controller/nginx/config/split_clients_template.go +++ b/internal/controller/nginx/config/split_clients_template.go @@ -6,7 +6,9 @@ split_clients $request_id ${{ $sc.VariableName }} { {{- range $d := $sc.Distributions }} {{- if eq $d.Percent "0.00" }} # {{ $d.Percent }}% {{ $d.Value }}; - {{- else }} + {{- else if eq $d.Percent "*" }} + * {{ $d.Value }}; + {{- else }} {{ $d.Percent }}% {{ $d.Value }}; {{- end }} {{- end }} diff --git a/internal/controller/nginx/config/split_clients_test.go b/internal/controller/nginx/config/split_clients_test.go index b5ff62a349..639b047ed2 100644 --- a/internal/controller/nginx/config/split_clients_test.go +++ b/internal/controller/nginx/config/split_clients_test.go @@ -1,6 +1,7 @@ package config import ( + "strings" "testing" . "github.com/onsi/gomega" @@ -8,105 +9,355 @@ import ( "github.com/nginx/nginx-gateway-fabric/internal/controller/nginx/config/http" "github.com/nginx/nginx-gateway-fabric/internal/controller/state/dataplane" + "github.com/nginx/nginx-gateway-fabric/internal/framework/helpers" ) func TestExecuteSplitClients(t *testing.T) { t.Parallel() - bg1 := dataplane.BackendGroup{ - Source: types.NamespacedName{Namespace: "test", Name: "hr"}, - RuleIdx: 0, - Backends: []dataplane.Backend{ - {UpstreamName: "test1", Valid: true, Weight: 1}, - {UpstreamName: "test2", Valid: true, Weight: 1}, - }, - } - - bg2 := dataplane.BackendGroup{ - Source: types.NamespacedName{Namespace: "test", Name: "no-split"}, - RuleIdx: 1, - Backends: []dataplane.Backend{ - {UpstreamName: "no-split", Valid: true, Weight: 1}, - }, - } - - bg3 := dataplane.BackendGroup{ - Source: types.NamespacedName{Namespace: "test", Name: "hr"}, - RuleIdx: 1, - Backends: []dataplane.Backend{ - {UpstreamName: "test3", Valid: true, Weight: 1}, - {UpstreamName: "test4", Valid: true, Weight: 1}, - }, - } tests := []struct { + expStrings map[string]int msg string - backendGroups []dataplane.BackendGroup - expStrings []string notExpStrings []string + configuration dataplane.Configuration }{ { msg: "non-zero weights", - backendGroups: []dataplane.BackendGroup{ - bg1, - bg2, - bg3, - }, - expStrings: []string{ - "split_clients $request_id $group_test__hr_rule0", - "split_clients $request_id $group_test__hr_rule1", - "50.00% test1;", - "50.00% test2;", - "50.00% test3;", - "50.00% test4;", + configuration: dataplane.Configuration{ + BackendGroups: []dataplane.BackendGroup{ + { + Source: types.NamespacedName{Namespace: "test", Name: "hr"}, + RuleIdx: 0, + Backends: []dataplane.Backend{ + {UpstreamName: "test1", Valid: true, Weight: 1}, + {UpstreamName: "test2", Valid: true, Weight: 1}, + }, + }, + { + Source: types.NamespacedName{Namespace: "test", Name: "no-split"}, + RuleIdx: 1, + Backends: []dataplane.Backend{ + {UpstreamName: "no-split", Valid: true, Weight: 1}, + }, + }, + { + Source: types.NamespacedName{Namespace: "test", Name: "hr"}, + RuleIdx: 1, + Backends: []dataplane.Backend{ + {UpstreamName: "test3", Valid: true, Weight: 1}, + {UpstreamName: "test4", Valid: true, Weight: 1}, + }, + }, + }, + }, + expStrings: map[string]int{ + "split_clients $request_id $group_test__hr_rule0": 1, + "split_clients $request_id $group_test__hr_rule1": 1, + "50.00% test1;": 1, + "50.00% test2;": 1, + "50.00% test3;": 1, + "50.00% test4;": 1, }, notExpStrings: []string{"no-split", "#"}, }, { msg: "zero weight", - backendGroups: []dataplane.BackendGroup{ - { - Source: types.NamespacedName{Namespace: "test", Name: "zero-percent"}, - RuleIdx: 0, - Backends: []dataplane.Backend{ - {UpstreamName: "non-zero", Valid: true, Weight: 1}, - {UpstreamName: "zero", Valid: true, Weight: 0}, + configuration: dataplane.Configuration{ + BackendGroups: []dataplane.BackendGroup{ + { + Source: types.NamespacedName{Namespace: "test", Name: "zero-percent"}, + RuleIdx: 0, + Backends: []dataplane.Backend{ + {UpstreamName: "non-zero", Valid: true, Weight: 1}, + {UpstreamName: "zero", Valid: true, Weight: 0}, + }, }, }, }, - expStrings: []string{ - "split_clients $request_id $group_test__zero_percent_rule0", - "100.00% non-zero;", - "# 0.00% zero;", + expStrings: map[string]int{ + "split_clients $request_id $group_test__zero_percent_rule0": 1, + "100.00% non-zero;": 1, + "# 0.00% zero;": 1, }, notExpStrings: nil, }, { msg: "no split clients", - backendGroups: []dataplane.BackendGroup{ - { - Source: types.NamespacedName{Namespace: "test", Name: "single-backend-route"}, - RuleIdx: 0, - Backends: []dataplane.Backend{ - {UpstreamName: "single-backend", Valid: true, Weight: 1}, + configuration: dataplane.Configuration{ + BackendGroups: []dataplane.BackendGroup{ + { + Source: types.NamespacedName{Namespace: "test", Name: "single-backend-route"}, + RuleIdx: 0, + Backends: []dataplane.Backend{ + {UpstreamName: "single-backend", Valid: true, Weight: 1}, + }, }, }, }, - expStrings: []string{}, + expStrings: map[string]int{}, notExpStrings: []string{"split_clients"}, }, + { + msg: "HTTPServer mirror split clients", + configuration: dataplane.Configuration{ + HTTPServers: []dataplane.VirtualServer{ + { + PathRules: []dataplane.PathRule{ + { + Path: "/mirror", + MatchRules: []dataplane.MatchRule{ + { + Filters: dataplane.HTTPFilters{ + RequestMirrors: []*dataplane.HTTPRequestMirrorFilter{ + { + Target: helpers.GetPointer(http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route1-0"), + Percent: helpers.GetPointer(float64(25)), + }, + { + Target: helpers.GetPointer(http.InternalMirrorRoutePathPrefix + "-my-tea-backend-test/route1-0"), + Percent: helpers.GetPointer(float64(50)), + }, + }, + }, + }, + { + Filters: dataplane.HTTPFilters{ + RequestMirrors: []*dataplane.HTTPRequestMirrorFilter{ + { + Target: helpers.GetPointer(http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route1-1"), + Percent: helpers.GetPointer(float64(25)), + }, + }, + }, + }, + }, + }, + { + Path: http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route1-0", + }, + { + Path: http.InternalMirrorRoutePathPrefix + "-my-tea-backend-test/route1-0", + }, + { + Path: http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route1-1", + }, + { + Path: "/mirror-edge-case-percentages", + MatchRules: []dataplane.MatchRule{ + { + Filters: dataplane.HTTPFilters{ + RequestMirrors: []*dataplane.HTTPRequestMirrorFilter{ + { + Target: helpers.GetPointer(http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route2-0"), + Percent: helpers.GetPointer(float64(0)), + }, + { + Target: helpers.GetPointer(http.InternalMirrorRoutePathPrefix + "-my-tea-backend-test/route2-0"), + Percent: helpers.GetPointer(float64(99.999)), + }, + }, + }, + }, + { + Filters: dataplane.HTTPFilters{ + RequestMirrors: []*dataplane.HTTPRequestMirrorFilter{ + { + Target: helpers.GetPointer(http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route2-1"), + Percent: helpers.GetPointer(float64(0.001)), + }, + { + Target: helpers.GetPointer(http.InternalMirrorRoutePathPrefix + "-my-tea-backend-test/route2-1"), + Percent: helpers.GetPointer(float64(100)), + }, + }, + }, + }, + }, + }, + { + Path: http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route2-0", + }, + { + Path: http.InternalMirrorRoutePathPrefix + "-my-tea-backend-test/route2-0", + }, + { + Path: http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route2-1", + }, + }, + }, + }, + }, + expStrings: map[string]int{ + "split_clients $request_id $__ngf_internal_mirror_my_coffee_backend_test_route1_0_25_00": 1, + "25.00% /_ngf-internal-mirror-my-coffee-backend-test/route1-0": 1, + + "split_clients $request_id $__ngf_internal_mirror_my_tea_backend_test_route1_0_50_00": 1, + "50.00% /_ngf-internal-mirror-my-tea-backend-test/route1-0": 1, + + "split_clients $request_id $__ngf_internal_mirror_my_coffee_backend_test_route1_1_25_00": 1, + "25.00% /_ngf-internal-mirror-my-coffee-backend-test/route1-1": 1, + + "split_clients $request_id $__ngf_internal_mirror_my_coffee_backend_test_route2_0_0_00": 1, + "0.00% /_ngf-internal-mirror-my-coffee-backend-test/route2-0": 1, + + "split_clients $request_id $__ngf_internal_mirror_my_tea_backend_test_route2_0_100_00": 1, + "100.00% /_ngf-internal-mirror-my-tea-backend-test/route2-0": 1, + + "split_clients $request_id $__ngf_internal_mirror_my_coffee_backend_test_route2_1_0_00": 1, + "0.00% /_ngf-internal-mirror-my-coffee-backend-test/route2-1": 1, + "* \"\"": 6, + }, + notExpStrings: []string{ + "split_clients $request_id $__ngf_internal_mirror_my_tea_backend_test_route2_1_100_00", + }, + }, + { + msg: "Duplicate split clients are not created", + configuration: dataplane.Configuration{ + HTTPServers: []dataplane.VirtualServer{ + { + PathRules: []dataplane.PathRule{ + { + Path: "/mirror", + MatchRules: []dataplane.MatchRule{ + { + Filters: dataplane.HTTPFilters{ + RequestMirrors: []*dataplane.HTTPRequestMirrorFilter{ + { + Target: helpers.GetPointer(http.InternalMirrorRoutePathPrefix + "-my-same-backend-test/route1-0"), + Percent: helpers.GetPointer(float64(25)), + }, + { + Target: helpers.GetPointer(http.InternalMirrorRoutePathPrefix + "-my-same-backend-test/route1-0"), + Percent: helpers.GetPointer(float64(50)), + }, + { + Target: helpers.GetPointer(http.InternalMirrorRoutePathPrefix + "-my-same-backend-test/route1-0"), + Percent: helpers.GetPointer(float64(50)), + }, + }, + }, + }, + }, + }, + { + Path: http.InternalMirrorRoutePathPrefix + "-my-same-backend-test/route1-0", + }, + }, + }, + }, + }, + expStrings: map[string]int{ + "split_clients $request_id $__ngf_internal_mirror_my_same_backend_test_route1_0_50_00": 1, + "50.00% /_ngf-internal-mirror-my-same-backend-test/route1-0": 1, + "* \"\"": 1, + }, + notExpStrings: []string{ + "split_clients $request_id $__ngf_internal_mirror_my_coffee_backend_test_route1_0_25_00", + "25.00% /_ngf-internal-mirror-my-same-backend-test/route1-0", + }, + }, + { + msg: "BackendGroup and Server split clients", + configuration: dataplane.Configuration{ + BackendGroups: []dataplane.BackendGroup{ + { + Source: types.NamespacedName{Namespace: "test", Name: "hr"}, + RuleIdx: 0, + Backends: []dataplane.Backend{ + {UpstreamName: "test1", Valid: true, Weight: 1}, + {UpstreamName: "test2", Valid: true, Weight: 1}, + }, + }, + { + Source: types.NamespacedName{Namespace: "test", Name: "hr"}, + RuleIdx: 1, + Backends: []dataplane.Backend{ + {UpstreamName: "test3", Valid: true, Weight: 1}, + {UpstreamName: "test4", Valid: true, Weight: 1}, + }, + }, + }, + HTTPServers: []dataplane.VirtualServer{ + { + PathRules: []dataplane.PathRule{ + { + Path: "/mirror", + MatchRules: []dataplane.MatchRule{ + { + Filters: dataplane.HTTPFilters{ + RequestMirrors: []*dataplane.HTTPRequestMirrorFilter{ + { + Target: helpers.GetPointer(http.InternalMirrorRoutePathPrefix + "-my-backend-test/route1-0"), + Percent: helpers.GetPointer(float64(25)), + }, + }, + }, + }, + }, + }, + { + Path: http.InternalMirrorRoutePathPrefix + "-my-backend-test/route1-0", + }, + }, + }, + }, + SSLServers: []dataplane.VirtualServer{ + { + PathRules: []dataplane.PathRule{ + { + Path: "/mirror-ssl", + MatchRules: []dataplane.MatchRule{ + { + Filters: dataplane.HTTPFilters{ + RequestMirrors: []*dataplane.HTTPRequestMirrorFilter{ + { + Target: helpers.GetPointer(http.InternalMirrorRoutePathPrefix + "-my-ssl-backend-test/route1-0"), + Percent: helpers.GetPointer(float64(50)), + }, + }, + }, + }, + }, + }, + { + Path: http.InternalMirrorRoutePathPrefix + "-my-ssl-backend-test/route1-0", + }, + }, + }, + }, + }, + expStrings: map[string]int{ + "split_clients $request_id $group_test__hr_rule0": 1, + "split_clients $request_id $group_test__hr_rule1": 1, + "50.00% test1;": 1, + "50.00% test2;": 1, + "50.00% test3;": 1, + "50.00% test4;": 1, + "split_clients $request_id $__ngf_internal_mirror_my_backend_test_route1_0_25_00": 1, + "25.00% /_ngf-internal-mirror-my-backend-test/route1-0": 1, + "split_clients $request_id $__ngf_internal_mirror_my_ssl_backend_test_route1_0_50_00": 1, + "50.00% /_ngf-internal-mirror-my-ssl-backend-test/route1-0": 1, + "* \"\"": 2, + }, + notExpStrings: nil, + }, } for _, test := range tests { t.Run(test.msg, func(t *testing.T) { t.Parallel() g := NewWithT(t) - splitResults := executeSplitClients(dataplane.Configuration{BackendGroups: test.backendGroups}) + + splitResults := executeSplitClients(test.configuration) + g.Expect(splitResults).To(HaveLen(1)) - sc := string(splitResults[0].data) g.Expect(splitResults[0].dest).To(Equal(httpConfigFile)) - for _, expSubString := range test.expStrings { - g.Expect(sc).To(ContainSubstring(expSubString)) + sc := string(splitResults[0].data) + + for expSubStr, expCount := range test.expStrings { + g.Expect(strings.Count(sc, expSubStr)).To(Equal(expCount)) } for _, notExpString := range test.notExpStrings { @@ -116,7 +367,187 @@ func TestExecuteSplitClients(t *testing.T) { } } -func TestCreateSplitClients(t *testing.T) { +func TestCreateRequestMirrorSplitClients(t *testing.T) { + t.Parallel() + + tests := []struct { + msg string + servers []dataplane.VirtualServer + expSplitClients []http.SplitClient + }{ + { + msg: "normal case", + servers: []dataplane.VirtualServer{ + { + PathRules: []dataplane.PathRule{ + { + Path: "/mirror", + MatchRules: []dataplane.MatchRule{ + { + Filters: dataplane.HTTPFilters{ + RequestMirrors: []*dataplane.HTTPRequestMirrorFilter{ + { + Target: helpers.GetPointer(http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route1-0"), + Percent: helpers.GetPointer(float64(25)), + }, + { + Target: helpers.GetPointer(http.InternalMirrorRoutePathPrefix + "-my-tea-backend-test/route1-0"), + Percent: helpers.GetPointer(float64(50)), + }, + }, + }, + }, + { + Filters: dataplane.HTTPFilters{ + RequestMirrors: []*dataplane.HTTPRequestMirrorFilter{ + { + Target: helpers.GetPointer(http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route1-1"), + Percent: helpers.GetPointer(float64(25)), + }, + }, + }, + }, + }, + }, + { + Path: http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route1-0", + }, + { + Path: http.InternalMirrorRoutePathPrefix + "-my-tea-backend-test/route1-0", + }, + { + Path: http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route1-1", + }, + }, + }, + { + PathRules: []dataplane.PathRule{ + { + Path: "/mirror-different-server", + MatchRules: []dataplane.MatchRule{ + { + Filters: dataplane.HTTPFilters{ + RequestMirrors: []*dataplane.HTTPRequestMirrorFilter{ + { + Target: helpers.GetPointer(http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route1-0"), + Percent: helpers.GetPointer(float64(30)), + }, + }, + }, + }, + }, + }, + { + Path: http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route1-0", + }, + }, + }, + }, + expSplitClients: []http.SplitClient{ + { + VariableName: "__ngf_internal_mirror_my_coffee_backend_test_route1_0_25_00", + Distributions: []http.SplitClientDistribution{ + { + Percent: "25.00", + Value: "/_ngf-internal-mirror-my-coffee-backend-test/route1-0", + }, + { + Percent: "*", + Value: "\"\"", + }, + }, + }, + { + VariableName: "__ngf_internal_mirror_my_tea_backend_test_route1_0_50_00", + Distributions: []http.SplitClientDistribution{ + { + Percent: "50.00", + Value: "/_ngf-internal-mirror-my-tea-backend-test/route1-0", + }, + { + Percent: "*", + Value: "\"\"", + }, + }, + }, + { + VariableName: "__ngf_internal_mirror_my_coffee_backend_test_route1_1_25_00", + Distributions: []http.SplitClientDistribution{ + { + Percent: "25.00", + Value: "/_ngf-internal-mirror-my-coffee-backend-test/route1-1", + }, + { + Percent: "*", + Value: "\"\"", + }, + }, + }, + { + VariableName: "__ngf_internal_mirror_my_coffee_backend_test_route1_0_30_00", + Distributions: []http.SplitClientDistribution{ + { + Percent: "30.00", + Value: "/_ngf-internal-mirror-my-coffee-backend-test/route1-0", + }, + { + Percent: "*", + Value: "\"\"", + }, + }, + }, + }, + }, + { + msg: "no split clients are needed", + servers: []dataplane.VirtualServer{ + { + PathRules: []dataplane.PathRule{ + { + Path: "/mirror", + MatchRules: []dataplane.MatchRule{ + { + Filters: dataplane.HTTPFilters{ + RequestMirrors: []*dataplane.HTTPRequestMirrorFilter{ + { + Target: helpers.GetPointer(http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route1-0"), + Percent: helpers.GetPointer(float64(100)), + }, + { + Target: helpers.GetPointer(http.InternalMirrorRoutePathPrefix + "-my-tea-backend-test/route1-0"), + }, + { + Target: helpers.GetPointer("path-does-not-exist"), + }, + }, + }, + }, + }, + }, + { + Path: http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route1-0", + }, + { + Path: http.InternalMirrorRoutePathPrefix + "-my-tea-backend-test/route1-0", + }, + }, + }, + }, + expSplitClients: nil, + }, + } + + for _, test := range tests { + t.Run(test.msg, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + result := createRequestMirrorSplitClients(test.servers) + g.Expect(result).To(Equal(test.expSplitClients)) + }) + } +} + +func TestBackendGroupCreateSplitClients(t *testing.T) { t.Parallel() hrNoSplit := types.NamespacedName{Namespace: "test", Name: "hr-no-split"} hrOneSplit := types.NamespacedName{Namespace: "test", Name: "hr-one-split"} @@ -246,13 +677,13 @@ func TestCreateSplitClients(t *testing.T) { t.Run(test.msg, func(t *testing.T) { t.Parallel() g := NewWithT(t) - result := createSplitClients(test.backendGroups) + result := createBackendGroupSplitClients(test.backendGroups) g.Expect(result).To(Equal(test.expSplitClients)) }) } } -func TestCreateSplitClientDistributions(t *testing.T) { +func TestCreateBackendGroupSplitClientDistributions(t *testing.T) { t.Parallel() tests := []struct { msg string @@ -395,7 +826,7 @@ func TestCreateSplitClientDistributions(t *testing.T) { t.Run(test.msg, func(t *testing.T) { t.Parallel() g := NewWithT(t) - result := createSplitClientDistributions(dataplane.BackendGroup{Backends: test.backends}) + result := createBackendGroupSplitClientDistributions(dataplane.BackendGroup{Backends: test.backends}) g.Expect(result).To(Equal(test.expDistributions)) }) } diff --git a/internal/controller/state/dataplane/configuration.go b/internal/controller/state/dataplane/configuration.go index 2874248a8d..ecdba8d3e8 100644 --- a/internal/controller/state/dataplane/configuration.go +++ b/internal/controller/state/dataplane/configuration.go @@ -504,6 +504,8 @@ func (hpr *hostPathRules) upsertRoute( var objectSrc *metav1.ObjectMeta + routeNsName := client.ObjectKeyFromObject(route.Source) + if GRPC { objectSrc = &helpers.MustCastObject[*v1.GRPCRoute](route.Source).ObjectMeta } else { @@ -541,7 +543,7 @@ func (hpr *hostPathRules) upsertRoute( var filters HTTPFilters if rule.Filters.Valid { - filters = createHTTPFilters(rule.Filters.Filters, idx) + filters = createHTTPFilters(rule.Filters.Filters, idx, routeNsName) } else { filters = HTTPFilters{ InvalidFilter: &InvalidHTTPFilter{}, @@ -565,8 +567,6 @@ func (hpr *hostPathRules) upsertRoute( hostRule.PathType = convertPathType(*m.Path.Type) } - routeNsName := client.ObjectKeyFromObject(route.Source) - hostRule.GRPC = GRPC hostRule.Policies = append(hostRule.Policies, pols...) @@ -805,7 +805,7 @@ func getPath(path *v1.HTTPPathMatch) string { return *path.Value } -func createHTTPFilters(filters []graph.Filter, ruleIdx int) HTTPFilters { +func createHTTPFilters(filters []graph.Filter, ruleIdx int, routeNsName types.NamespacedName) HTTPFilters { var result HTTPFilters for _, f := range filters { @@ -823,7 +823,7 @@ func createHTTPFilters(filters []graph.Filter, ruleIdx int) HTTPFilters { case graph.FilterRequestMirror: result.RequestMirrors = append( result.RequestMirrors, - convertHTTPRequestMirrorFilter(f.RequestMirror, ruleIdx), + convertHTTPRequestMirrorFilter(f.RequestMirror, ruleIdx, routeNsName), ) case graph.FilterRequestHeaderModifier: if result.RequestHeaderModifiers == nil { diff --git a/internal/controller/state/dataplane/configuration_test.go b/internal/controller/state/dataplane/configuration_test.go index 2e3e105aae..07b7874937 100644 --- a/internal/controller/state/dataplane/configuration_test.go +++ b/internal/controller/state/dataplane/configuration_test.go @@ -2177,8 +2177,9 @@ func TestBuildConfiguration(t *testing.T) { Filters: HTTPFilters{ RequestMirrors: []*HTTPRequestMirrorFilter{ { - Name: helpers.GetPointer("mirror-backend"), - Target: helpers.GetPointer("/_ngf-internal-mirror-mirror-backend-0"), + Name: helpers.GetPointer("mirror-backend"), + Target: helpers.GetPointer("/_ngf-internal-mirror-mirror-backend-test/hr-with-mirror-0"), + Percent: helpers.GetPointer(float64(100)), }, }, }, @@ -2750,6 +2751,7 @@ func TestCreateFilters(t *testing.T) { Kind: helpers.GetPointer(v1.Kind("Service")), Name: v1.ObjectName("mirror-backend2"), }, + Percent: helpers.GetPointer(int32(50)), }, } requestHeaderModifiers1 := graph.Filter{ @@ -2807,12 +2809,14 @@ func TestCreateFilters(t *testing.T) { } expectedMirror1 := HTTPRequestMirrorFilter{ - Name: helpers.GetPointer("mirror-backend"), - Target: helpers.GetPointer("/_ngf-internal-mirror-mirror-backend-0"), + Name: helpers.GetPointer("mirror-backend"), + Target: helpers.GetPointer("/_ngf-internal-mirror-mirror-backend-test/route1-0"), + Percent: helpers.GetPointer(float64(100)), } expectedMirror2 := HTTPRequestMirrorFilter{ - Name: helpers.GetPointer("mirror-backend2"), - Target: helpers.GetPointer("/_ngf-internal-mirror-mirror-backend2-0"), + Name: helpers.GetPointer("mirror-backend2"), + Target: helpers.GetPointer("/_ngf-internal-mirror-mirror-backend2-test/route1-0"), + Percent: helpers.GetPointer(float64(50)), } expectedHeaderModifier1 := HTTPHeaderFilter{ @@ -2975,7 +2979,8 @@ func TestCreateFilters(t *testing.T) { t.Run(test.msg, func(t *testing.T) { t.Parallel() g := NewWithT(t) - result := createHTTPFilters(test.filters, 0) + routeNsName := types.NamespacedName{Namespace: "test", Name: "route1"} + result := createHTTPFilters(test.filters, 0, routeNsName) g.Expect(helpers.Diff(test.expected, result)).To(BeEmpty()) }) diff --git a/internal/controller/state/dataplane/convert.go b/internal/controller/state/dataplane/convert.go index 0ab57bf0e9..666b85cabb 100644 --- a/internal/controller/state/dataplane/convert.go +++ b/internal/controller/state/dataplane/convert.go @@ -3,6 +3,7 @@ package dataplane import ( "fmt" + "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" v1 "sigs.k8s.io/gateway-api/apis/v1" @@ -62,7 +63,11 @@ func convertHTTPURLRewriteFilter(filter *v1.HTTPURLRewriteFilter) *HTTPURLRewrit } } -func convertHTTPRequestMirrorFilter(filter *v1.HTTPRequestMirrorFilter, ruleIdx int) *HTTPRequestMirrorFilter { +func convertHTTPRequestMirrorFilter( + filter *v1.HTTPRequestMirrorFilter, + ruleIdx int, + routeNsName types.NamespacedName, +) *HTTPRequestMirrorFilter { if filter.BackendRef.Name == "" { return &HTTPRequestMirrorFilter{} } @@ -76,7 +81,19 @@ func convertHTTPRequestMirrorFilter(filter *v1.HTTPRequestMirrorFilter, ruleIdx result.Namespace = namespace } - result.Target = mirror.BackendPath(ruleIdx, namespace, *result.Name) + result.Target = mirror.BackendPath(ruleIdx, namespace, *result.Name, routeNsName) + switch { + case filter.Percent != nil: + result.Percent = helpers.GetPointer(float64(*filter.Percent)) + case filter.Fraction != nil: + denominator := int32(100) + if filter.Fraction.Denominator != nil { + denominator = *filter.Fraction.Denominator + } + result.Percent = helpers.GetPointer(float64(filter.Fraction.Numerator*100) / float64(denominator)) + default: + result.Percent = helpers.GetPointer(float64(100)) + } return result } diff --git a/internal/controller/state/dataplane/convert_test.go b/internal/controller/state/dataplane/convert_test.go index 77b4c689ee..d515d3e6fb 100644 --- a/internal/controller/state/dataplane/convert_test.go +++ b/internal/controller/state/dataplane/convert_test.go @@ -4,6 +4,7 @@ import ( "testing" . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/types" v1 "sigs.k8s.io/gateway-api/apis/v1" "github.com/nginx/nginx-gateway-fabric/internal/framework/helpers" @@ -337,9 +338,10 @@ func TestConvertHTTPMirrorFilter(t *testing.T) { expected: &HTTPRequestMirrorFilter{ Name: helpers.GetPointer("backend"), Namespace: nil, - Target: helpers.GetPointer("/_ngf-internal-mirror-backend-0"), + Target: helpers.GetPointer("/_ngf-internal-mirror-backend-test/route1-0"), + Percent: helpers.GetPointer(float64(100)), }, - name: "missing namespace", + name: "missing backendRef namespace", }, { filter: &v1.HTTPRequestMirrorFilter{ @@ -347,13 +349,67 @@ func TestConvertHTTPMirrorFilter(t *testing.T) { Name: "backend", Namespace: helpers.GetPointer[v1.Namespace]("namespace"), }, + Fraction: &v1.Fraction{ + Numerator: 25, + }, }, expected: &HTTPRequestMirrorFilter{ Name: helpers.GetPointer("backend"), Namespace: helpers.GetPointer("namespace"), - Target: helpers.GetPointer("/_ngf-internal-mirror-namespace/backend-0"), + Target: helpers.GetPointer("/_ngf-internal-mirror-namespace/backend-test/route1-0"), + Percent: helpers.GetPointer(float64(25)), }, - name: "full", + name: "fraction denominator not specified", + }, + { + filter: &v1.HTTPRequestMirrorFilter{ + BackendRef: v1.BackendObjectReference{ + Name: "backend", + Namespace: helpers.GetPointer[v1.Namespace]("namespace"), + }, + Percent: helpers.GetPointer(int32(50)), + }, + expected: &HTTPRequestMirrorFilter{ + Name: helpers.GetPointer("backend"), + Namespace: helpers.GetPointer("namespace"), + Target: helpers.GetPointer("/_ngf-internal-mirror-namespace/backend-test/route1-0"), + Percent: helpers.GetPointer(float64(50)), + }, + name: "full with filter percent", + }, + { + filter: &v1.HTTPRequestMirrorFilter{ + BackendRef: v1.BackendObjectReference{ + Name: "backend", + Namespace: helpers.GetPointer[v1.Namespace]("namespace"), + }, + Fraction: &v1.Fraction{ + Numerator: 1, + Denominator: helpers.GetPointer(int32(2)), + }, + }, + expected: &HTTPRequestMirrorFilter{ + Name: helpers.GetPointer("backend"), + Namespace: helpers.GetPointer("namespace"), + Target: helpers.GetPointer("/_ngf-internal-mirror-namespace/backend-test/route1-0"), + Percent: helpers.GetPointer(float64(50)), + }, + name: "full with filter fraction", + }, + { + filter: &v1.HTTPRequestMirrorFilter{ + BackendRef: v1.BackendObjectReference{ + Name: "backend", + Namespace: helpers.GetPointer[v1.Namespace]("namespace"), + }, + }, + expected: &HTTPRequestMirrorFilter{ + Name: helpers.GetPointer("backend"), + Namespace: helpers.GetPointer("namespace"), + Target: helpers.GetPointer("/_ngf-internal-mirror-namespace/backend-test/route1-0"), + Percent: helpers.GetPointer(float64(100)), + }, + name: "full with no filter percent or fraction specified", }, } @@ -361,7 +417,9 @@ func TestConvertHTTPMirrorFilter(t *testing.T) { t.Run(test.name, func(t *testing.T) { g := NewWithT(t) - result := convertHTTPRequestMirrorFilter(test.filter, 0) + routeNsName := types.NamespacedName{Namespace: "test", Name: "route1"} + + result := convertHTTPRequestMirrorFilter(test.filter, 0, routeNsName) g.Expect(result).To(Equal(test.expected)) }) } diff --git a/internal/controller/state/dataplane/types.go b/internal/controller/state/dataplane/types.go index 95589e6ee1..9c26eb164d 100644 --- a/internal/controller/state/dataplane/types.go +++ b/internal/controller/state/dataplane/types.go @@ -215,8 +215,10 @@ type HTTPRequestMirrorFilter struct { Name *string // Namespace is the namespace of the service. Namespace *string - // Target is the target of the mirror (path with hostname and service name). + // Target is the target of the mirror (path with hostname, service name and route NamespacedName). Target *string + // Percent is the percentage of requests to mirror. + Percent *float64 } // PathModifierType is the type of the PathModifier in a redirect or rewrite rule. diff --git a/internal/controller/state/graph/common_filter.go b/internal/controller/state/graph/common_filter.go index 41266d42c2..e1fe43a372 100644 --- a/internal/controller/state/graph/common_filter.go +++ b/internal/controller/state/graph/common_filter.go @@ -230,6 +230,40 @@ func validateFilterMirror(mirror *v1.HTTPRequestMirrorFilter, filterPath *field. return field.ErrorList{field.Required(mirrorPath, "cannot be nil")} } + if mirror.Percent != nil && mirror.Fraction != nil { + return field.ErrorList{field.Invalid(mirrorPath, mirror, "cannot set both percent and fraction")} + } + + if mirror.Fraction != nil && mirror.Fraction.Denominator != nil { + if *mirror.Fraction.Denominator <= 0 { + return field.ErrorList{ + field.Invalid( + mirrorPath.Child("fraction", "denominator"), + mirror.Fraction.Denominator, + "must be greater than 0", + ), + } + } + if mirror.Fraction.Numerator < 0 { + return field.ErrorList{ + field.Invalid( + mirrorPath.Child("fraction", "numerator"), + mirror.Fraction.Numerator, + "must be greater than or equal to 0", + ), + } + } + if mirror.Fraction.Numerator > *mirror.Fraction.Denominator { + return field.ErrorList{ + field.Invalid( + mirrorPath.Child("fraction", "numerator"), + mirror.Fraction.Numerator, + "must be less than or equal to denominator", + ), + } + } + } + return nil } diff --git a/internal/controller/state/graph/common_filter_test.go b/internal/controller/state/graph/common_filter_test.go index 0e31a4cbb1..33c28e029a 100644 --- a/internal/controller/state/graph/common_filter_test.go +++ b/internal/controller/state/graph/common_filter_test.go @@ -10,6 +10,7 @@ import ( ngfAPI "github.com/nginx/nginx-gateway-fabric/apis/v1alpha1" "github.com/nginx/nginx-gateway-fabric/internal/controller/state/validation/validationfakes" + "github.com/nginx/nginx-gateway-fabric/internal/framework/helpers" "github.com/nginx/nginx-gateway-fabric/internal/framework/kinds" ) @@ -39,24 +40,6 @@ func TestValidateFilter(t *testing.T) { expectErrCount: 0, name: "valid HTTP rewrite filter", }, - { - filter: Filter{ - RouteType: RouteTypeHTTP, - FilterType: FilterRequestMirror, - RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{}, - }, - expectErrCount: 0, - name: "valid HTTP mirror filter", - }, - { - filter: Filter{ - RouteType: RouteTypeHTTP, - FilterType: FilterRequestMirror, - RequestMirror: nil, - }, - expectErrCount: 1, - name: "invalid HTTP mirror filter", - }, { filter: Filter{ RouteType: RouteTypeHTTP, @@ -96,15 +79,6 @@ func TestValidateFilter(t *testing.T) { expectErrCount: 1, name: "unsupported HTTP filter type", }, - { - filter: Filter{ - RouteType: RouteTypeGRPC, - FilterType: FilterRequestMirror, - RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{}, - }, - expectErrCount: 0, - name: "valid GRPC mirror filter", - }, { filter: Filter{ RouteType: RouteTypeGRPC, @@ -159,6 +133,138 @@ func TestValidateFilter(t *testing.T) { } } +func TestValidateFilterMirror(t *testing.T) { + t.Parallel() + + tests := []struct { + filter Filter + name string + expectErrCount int + }{ + { + filter: Filter{ + RouteType: RouteTypeHTTP, + FilterType: FilterRequestMirror, + RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{}, + }, + expectErrCount: 0, + name: "valid HTTP mirror filter", + }, + { + filter: Filter{ + RouteType: RouteTypeHTTP, + FilterType: FilterRequestMirror, + RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{ + Percent: helpers.GetPointer(int32(50)), + }, + }, + expectErrCount: 0, + name: "valid HTTP mirror filter with percentage set", + }, + { + filter: Filter{ + RouteType: RouteTypeHTTP, + FilterType: FilterRequestMirror, + RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{ + Fraction: &gatewayv1.Fraction{ + Numerator: 1, + Denominator: helpers.GetPointer(int32(2)), + }, + }, + }, + expectErrCount: 0, + name: "valid HTTP mirror filter with fraction set", + }, + { + filter: Filter{ + RouteType: RouteTypeHTTP, + FilterType: FilterRequestMirror, + RequestMirror: nil, + }, + expectErrCount: 1, + name: "invalid nil HTTP mirror filter", + }, + { + filter: Filter{ + RouteType: RouteTypeHTTP, + FilterType: FilterRequestMirror, + RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{ + Percent: helpers.GetPointer(int32(5)), + Fraction: &gatewayv1.Fraction{ + Numerator: 1, + Denominator: helpers.GetPointer(int32(3)), + }, + }, + }, + expectErrCount: 1, + name: "invalid HTTP mirror filter both percent and fraction set", + }, + { + filter: Filter{ + RouteType: RouteTypeHTTP, + FilterType: FilterRequestMirror, + RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{ + Fraction: &gatewayv1.Fraction{ + Numerator: 1, + Denominator: helpers.GetPointer(int32(0)), + }, + }, + }, + expectErrCount: 1, + name: "invalid HTTP mirror filter, fraction denominator value must be greater than 0", + }, + { + filter: Filter{ + RouteType: RouteTypeHTTP, + FilterType: FilterRequestMirror, + RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{ + Fraction: &gatewayv1.Fraction{ + Numerator: -1, + Denominator: helpers.GetPointer(int32(2)), + }, + }, + }, + expectErrCount: 1, + name: "invalid HTTP mirror filter, fraction numerator value must be greater than or equal to 0", + }, + { + filter: Filter{ + RouteType: RouteTypeHTTP, + FilterType: FilterRequestMirror, + RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{ + Fraction: &gatewayv1.Fraction{ + Numerator: 5, + Denominator: helpers.GetPointer(int32(2)), + }, + }, + }, + expectErrCount: 1, + name: "invalid HTTP mirror filter, fraction numerator value must be less than denominator", + }, + { + filter: Filter{ + RouteType: RouteTypeGRPC, + FilterType: FilterRequestMirror, + RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{}, + }, + expectErrCount: 0, + name: "valid GRPC mirror filter", + }, + } + + filterPath := field.NewPath("test") + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + g := NewWithT(t) + allErrs := validateFilter(&validationfakes.FakeHTTPFieldsValidator{}, test.filter, filterPath) + g.Expect(allErrs).To(HaveLen(test.expectErrCount)) + }) + } +} + func TestValidateFilterResponseHeaderModifier(t *testing.T) { t.Parallel() diff --git a/internal/controller/state/graph/grpcroute.go b/internal/controller/state/graph/grpcroute.go index 51976006b4..c8ae71f38e 100644 --- a/internal/controller/state/graph/grpcroute.go +++ b/internal/controller/state/graph/grpcroute.go @@ -6,6 +6,7 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/controller-runtime/pkg/client" v1 "sigs.k8s.io/gateway-api/apis/v1" "github.com/nginx/nginx-gateway-fabric/internal/controller/nginx/config/http" @@ -92,7 +93,12 @@ func buildGRPCMirrorRoutes( Spec: v1.GRPCRouteSpec{ CommonRouteSpec: route.Spec.CommonRouteSpec, Hostnames: route.Spec.Hostnames, - Rules: buildGRPCMirrorRouteRule(idx, route.Spec.Rules[idx].Filters, filter), + Rules: buildGRPCMirrorRouteRule( + idx, + route.Spec.Rules[idx].Filters, + filter, + client.ObjectKeyFromObject(l7route.Source), + ), }, } @@ -115,6 +121,7 @@ func buildGRPCMirrorRouteRule( ruleIdx int, filters []v1.GRPCRouteFilter, filter Filter, + routeNsName types.NamespacedName, ) []v1.GRPCRouteRule { return []v1.GRPCRouteRule{ { @@ -122,7 +129,7 @@ func buildGRPCMirrorRouteRule( { Method: &v1.GRPCMethodMatch{ Type: helpers.GetPointer(v1.GRPCMethodMatchExact), - Service: mirror.PathWithBackendRef(ruleIdx, filter.RequestMirror.BackendRef), + Service: mirror.PathWithBackendRef(ruleIdx, filter.RequestMirror.BackendRef, routeNsName), }, }, }, diff --git a/internal/controller/state/graph/grpcroute_test.go b/internal/controller/state/graph/grpcroute_test.go index 5c9da4c5c6..698c3faa9a 100644 --- a/internal/controller/state/graph/grpcroute_test.go +++ b/internal/controller/state/graph/grpcroute_test.go @@ -1164,7 +1164,7 @@ func TestBuildGRPCRouteWithMirrorRoutes(t *testing.T) { { Method: &v1.GRPCMethodMatch{ Type: helpers.GetPointer(v1.GRPCMethodMatchExact), - Service: helpers.GetPointer("/_ngf-internal-mirror-mirror-backend-0"), + Service: helpers.GetPointer("/_ngf-internal-mirror-mirror-backend-test/gr-0"), }, }, }, @@ -1210,7 +1210,7 @@ func TestBuildGRPCRouteWithMirrorRoutes(t *testing.T) { { Path: &v1.HTTPPathMatch{ Type: helpers.GetPointer(v1.PathMatchExact), - Value: helpers.GetPointer("/_ngf-internal-mirror-mirror-backend-0"), + Value: helpers.GetPointer("/_ngf-internal-mirror-mirror-backend-test/gr-0"), }, Headers: []v1.HTTPHeaderMatch{}, }, diff --git a/internal/controller/state/graph/httproute.go b/internal/controller/state/graph/httproute.go index bacf34b363..dc96830c20 100644 --- a/internal/controller/state/graph/httproute.go +++ b/internal/controller/state/graph/httproute.go @@ -6,6 +6,7 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/controller-runtime/pkg/client" v1 "sigs.k8s.io/gateway-api/apis/v1" "github.com/nginx/nginx-gateway-fabric/internal/controller/nginx/config/http" @@ -98,7 +99,12 @@ func buildHTTPMirrorRoutes( Spec: v1.HTTPRouteSpec{ CommonRouteSpec: route.Spec.CommonRouteSpec, Hostnames: route.Spec.Hostnames, - Rules: buildHTTPMirrorRouteRule(idx, route.Spec.Rules[idx].Filters, filter), + Rules: buildHTTPMirrorRouteRule( + idx, + route.Spec.Rules[idx].Filters, + filter, + client.ObjectKeyFromObject(l7route.Source), + ), }, } @@ -121,6 +127,7 @@ func buildHTTPMirrorRouteRule( ruleIdx int, filters []v1.HTTPRouteFilter, filter Filter, + routeNsName types.NamespacedName, ) []v1.HTTPRouteRule { return []v1.HTTPRouteRule{ { @@ -128,7 +135,7 @@ func buildHTTPMirrorRouteRule( { Path: &v1.HTTPPathMatch{ Type: helpers.GetPointer(v1.PathMatchExact), - Value: mirror.PathWithBackendRef(ruleIdx, filter.RequestMirror.BackendRef), + Value: mirror.PathWithBackendRef(ruleIdx, filter.RequestMirror.BackendRef, routeNsName), }, }, }, diff --git a/internal/controller/state/graph/httproute_test.go b/internal/controller/state/graph/httproute_test.go index f4d1347a6a..d069980026 100644 --- a/internal/controller/state/graph/httproute_test.go +++ b/internal/controller/state/graph/httproute_test.go @@ -1020,7 +1020,7 @@ func TestBuildHTTPRouteWithMirrorRoutes(t *testing.T) { { Path: &gatewayv1.HTTPPathMatch{ Type: helpers.GetPointer(gatewayv1.PathMatchExact), - Value: helpers.GetPointer("/_ngf-internal-mirror-mirror-backend-0"), + Value: helpers.GetPointer("/_ngf-internal-mirror-mirror-backend-test/hr-0"), }, }, }, @@ -1066,7 +1066,7 @@ func TestBuildHTTPRouteWithMirrorRoutes(t *testing.T) { { Path: &gatewayv1.HTTPPathMatch{ Type: helpers.GetPointer(gatewayv1.PathMatchExact), - Value: helpers.GetPointer("/_ngf-internal-mirror-mirror-backend-0"), + Value: helpers.GetPointer("/_ngf-internal-mirror-mirror-backend-test/hr-0"), }, }, }, diff --git a/internal/controller/state/mirror/mirror.go b/internal/controller/state/mirror/mirror.go index 9e66a44a72..9067908570 100644 --- a/internal/controller/state/mirror/mirror.go +++ b/internal/controller/state/mirror/mirror.go @@ -4,6 +4,7 @@ import ( "fmt" "strings" + "k8s.io/apimachinery/pkg/types" v1 "sigs.k8s.io/gateway-api/apis/v1" "github.com/nginx/nginx-gateway-fabric/internal/controller/nginx/config/http" @@ -17,23 +18,37 @@ func RouteName(routeName, service, namespace string, idx int) string { return fmt.Sprintf("%s-%s-%s/%s-%d", prefix, routeName, namespace, service, idx) } -// BackendPath builds the path for the internal mirror location, using the BackendRef. -func PathWithBackendRef(idx int, backendRef v1.BackendObjectReference) *string { +// PathWithBackendRef builds the path for the internal mirror location, using the BackendRef. +func PathWithBackendRef(idx int, backendRef v1.BackendObjectReference, routeNsName types.NamespacedName) *string { svcName := string(backendRef.Name) if backendRef.Namespace == nil { - return BackendPath(idx, nil, svcName) + return BackendPath(idx, nil, svcName, routeNsName) } - return BackendPath(idx, (*string)(backendRef.Namespace), svcName) + return BackendPath(idx, (*string)(backendRef.Namespace), svcName, routeNsName) } // BackendPath builds the path for the internal mirror location. -func BackendPath(idx int, namespace *string, service string) *string { +func BackendPath(idx int, namespace *string, service string, routeNsName types.NamespacedName) *string { var mirrorPath string if namespace != nil { - mirrorPath = fmt.Sprintf("%s-%s/%s-%d", http.InternalMirrorRoutePathPrefix, *namespace, service, idx) + mirrorPath = fmt.Sprintf( + "%s-%s/%s-%s/%s-%d", + http.InternalMirrorRoutePathPrefix, + *namespace, + service, + routeNsName.Namespace, + routeNsName.Name, + idx, + ) } else { - mirrorPath = fmt.Sprintf("%s-%s-%d", http.InternalMirrorRoutePathPrefix, service, idx) + mirrorPath = fmt.Sprintf("%s-%s-%s/%s-%d", + http.InternalMirrorRoutePathPrefix, + service, + routeNsName.Namespace, + routeNsName.Name, + idx, + ) } return &mirrorPath diff --git a/internal/controller/state/mirror/mirror_test.go b/internal/controller/state/mirror/mirror_test.go index 06c63e7adb..69e9e65f1f 100644 --- a/internal/controller/state/mirror/mirror_test.go +++ b/internal/controller/state/mirror/mirror_test.go @@ -4,6 +4,7 @@ import ( "testing" . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/types" v1 "sigs.k8s.io/gateway-api/apis/v1" "github.com/nginx/nginx-gateway-fabric/internal/framework/helpers" @@ -21,10 +22,11 @@ func TestPathWithBackendRef(t *testing.T) { t.Parallel() tests := []struct { - backendRef v1.BackendObjectReference - expected *string - name string - idx int + backendRef v1.BackendObjectReference + expected *string + name string + routeNsName types.NamespacedName + idx int }{ { name: "with namespace", @@ -33,7 +35,8 @@ func TestPathWithBackendRef(t *testing.T) { Name: "service1", Namespace: helpers.GetPointer[v1.Namespace]("namespace1"), }, - expected: helpers.GetPointer("/_ngf-internal-mirror-namespace1/service1-1"), + routeNsName: types.NamespacedName{Namespace: "routeNs", Name: "routeName1"}, + expected: helpers.GetPointer("/_ngf-internal-mirror-namespace1/service1-routeNs/routeName1-1"), }, { name: "without namespace", @@ -41,7 +44,8 @@ func TestPathWithBackendRef(t *testing.T) { backendRef: v1.BackendObjectReference{ Name: "service2", }, - expected: helpers.GetPointer("/_ngf-internal-mirror-service2-2"), + routeNsName: types.NamespacedName{Namespace: "routeNs", Name: "routeName1"}, + expected: helpers.GetPointer("/_ngf-internal-mirror-service2-routeNs/routeName1-2"), }, } @@ -50,7 +54,7 @@ func TestPathWithBackendRef(t *testing.T) { t.Parallel() g := NewWithT(t) - result := PathWithBackendRef(tt.idx, tt.backendRef) + result := PathWithBackendRef(tt.idx, tt.backendRef, tt.routeNsName) g.Expect(result).To(Equal(tt.expected)) }) } @@ -60,25 +64,28 @@ func TestBackendPath(t *testing.T) { t.Parallel() tests := []struct { - namespace *string - expected *string - name string - service string - idx int + namespace *string + expected *string + name string + service string + routeNsName types.NamespacedName + idx int }{ { - name: "With namespace", - idx: 1, - namespace: helpers.GetPointer("namespace1"), - service: "service1", - expected: helpers.GetPointer("/_ngf-internal-mirror-namespace1/service1-1"), + name: "With namespace", + idx: 1, + namespace: helpers.GetPointer("namespace1"), + service: "service1", + routeNsName: types.NamespacedName{Namespace: "routeNs", Name: "routeName1"}, + expected: helpers.GetPointer("/_ngf-internal-mirror-namespace1/service1-routeNs/routeName1-1"), }, { - name: "Without namespace", - idx: 2, - namespace: nil, - service: "service2", - expected: helpers.GetPointer("/_ngf-internal-mirror-service2-2"), + name: "Without namespace", + idx: 2, + namespace: nil, + service: "service2", + routeNsName: types.NamespacedName{Namespace: "routeNs", Name: "routeName1"}, + expected: helpers.GetPointer("/_ngf-internal-mirror-service2-routeNs/routeName1-2"), }, } @@ -87,7 +94,7 @@ func TestBackendPath(t *testing.T) { t.Parallel() g := NewWithT(t) - result := BackendPath(tt.idx, tt.namespace, tt.service) + result := BackendPath(tt.idx, tt.namespace, tt.service, tt.routeNsName) g.Expect(result).To(Equal(tt.expected)) }) } diff --git a/tests/Makefile b/tests/Makefile index 52a2503f93..4f0e86b247 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -12,7 +12,7 @@ GW_SERVICE_TYPE = NodePort## Service type to use for the gateway NGF_VERSION ?= edge## NGF version to be tested PULL_POLICY = Never## Pull policy for the images NGINX_CONF_DIR = internal/controller/nginx/conf -SUPPORTED_EXTENDED_FEATURES = HTTPRouteQueryParamMatching,HTTPRouteMethodMatching,HTTPRoutePortRedirect,HTTPRouteSchemeRedirect,HTTPRouteHostRewrite,HTTPRoutePathRewrite,GatewayPort8080,GatewayAddressEmpty,HTTPRouteResponseHeaderModification,HTTPRoutePathRedirect,GatewayHTTPListenerIsolation,GatewayInfrastructurePropagation,HTTPRouteRequestMirror,HTTPRouteRequestMultipleMirrors,HTTPRouteBackendProtocolWebSocket +SUPPORTED_EXTENDED_FEATURES = HTTPRouteQueryParamMatching,HTTPRouteMethodMatching,HTTPRoutePortRedirect,HTTPRouteSchemeRedirect,HTTPRouteHostRewrite,HTTPRoutePathRewrite,GatewayPort8080,GatewayAddressEmpty,HTTPRouteResponseHeaderModification,HTTPRoutePathRedirect,GatewayHTTPListenerIsolation,GatewayInfrastructurePropagation,HTTPRouteRequestMirror,HTTPRouteRequestMultipleMirrors,HTTPRouteBackendProtocolWebSocket,HTTPRouteRequestPercentageMirror 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. From 92bdabfbe486c6bf0a1615cbdbc2ce582cda7698 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Tue, 15 Jul 2025 11:58:03 -0700 Subject: [PATCH 02/13] Add some comments --- internal/controller/nginx/config/servers.go | 2 ++ internal/controller/state/dataplane/types.go | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/controller/nginx/config/servers.go b/internal/controller/nginx/config/servers.go index 06d17c1847..4b6851cf93 100644 --- a/internal/controller/nginx/config/servers.go +++ b/internal/controller/nginx/config/servers.go @@ -280,6 +280,8 @@ func createLocations( mirrorPercentage, exists := mirrorPathToPercentage[rule.Path] if !exists { + // need a way to differentiate between no mirror filter and a mirror filter with 0 percent set, and + // I don't want to pass an extra boolean around. mirrorPercentage = -1 } diff --git a/internal/controller/state/dataplane/types.go b/internal/controller/state/dataplane/types.go index 9c26eb164d..0cfe48beb0 100644 --- a/internal/controller/state/dataplane/types.go +++ b/internal/controller/state/dataplane/types.go @@ -215,7 +215,7 @@ type HTTPRequestMirrorFilter struct { Name *string // Namespace is the namespace of the service. Namespace *string - // Target is the target of the mirror (path with hostname, service name and route NamespacedName). + // Target is the target of the mirror (path with hostname, service name, and route NamespacedName). Target *string // Percent is the percentage of requests to mirror. Percent *float64 From 010c98115082921a5c11ea5c4951da1ae9e8f492 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Tue, 15 Jul 2025 13:14:37 -0700 Subject: [PATCH 03/13] Add nolint --- internal/framework/types/doc.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/framework/types/doc.go b/internal/framework/types/doc.go index f479c9ba99..e5ceb61094 100644 --- a/internal/framework/types/doc.go +++ b/internal/framework/types/doc.go @@ -1,4 +1,4 @@ /* Package types contains types that are shared by the provisioner and static modes. */ -package types +package types //nolint:revive, nolintlint // ignoring “meaningless package name” and the unused-nolint warning From ee41b2a972ae14da1a7170d012244e0612d672a6 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Tue, 15 Jul 2025 15:58:31 -0700 Subject: [PATCH 04/13] Add some feedback --- internal/controller/nginx/config/servers.go | 34 ++++++++----------- .../controller/nginx/config/split_clients.go | 16 ++++----- .../nginx/config/split_clients_template.go | 6 ++-- .../controller/state/dataplane/convert.go | 4 +++ .../state/dataplane/convert_test.go | 19 +++++++++++ tests/Makefile | 2 +- 6 files changed, 50 insertions(+), 31 deletions(-) diff --git a/internal/controller/nginx/config/servers.go b/internal/controller/nginx/config/servers.go index 4b6851cf93..c8c1d8a9bb 100644 --- a/internal/controller/nginx/config/servers.go +++ b/internal/controller/nginx/config/servers.go @@ -225,21 +225,22 @@ type rewriteConfig struct { } // extractMirrorTargetsWithPercentages extracts mirror targets and their percentages from path rules. -func extractMirrorTargetsWithPercentages(pathRules []dataplane.PathRule) map[string]float64 { - mirrorTargets := make(map[string]float64) +func extractMirrorTargetsWithPercentages(pathRules []dataplane.PathRule) map[string]*float64 { + mirrorTargets := make(map[string]*float64) for _, rule := range pathRules { for _, matchRule := range rule.MatchRules { for _, mirrorFilter := range matchRule.Filters.RequestMirrors { if mirrorFilter.Target != nil { if mirrorFilter.Percent == nil { - mirrorTargets[*mirrorFilter.Target] = 100.0 + mirrorTargets[*mirrorFilter.Target] = helpers.GetPointer(100.0) continue } - percentage := *mirrorFilter.Percent + percentage := mirrorFilter.Percent - if _, exists := mirrorTargets[*mirrorFilter.Target]; !exists || percentage > mirrorTargets[*mirrorFilter.Target] { + if _, exists := mirrorTargets[*mirrorFilter.Target]; !exists || + *percentage > *mirrorTargets[*mirrorFilter.Target] { mirrorTargets[*mirrorFilter.Target] = percentage // set a higher percentage if it exists } } @@ -278,12 +279,7 @@ func createLocations( grpcServer = true } - mirrorPercentage, exists := mirrorPathToPercentage[rule.Path] - if !exists { - // need a way to differentiate between no mirror filter and a mirror filter with 0 percent set, and - // I don't want to pass an extra boolean around. - mirrorPercentage = -1 - } + mirrorPercentage := mirrorPathToPercentage[rule.Path] extLocations := initializeExternalLocations(rule, pathsAndTypes) for i := range extLocations { @@ -464,7 +460,7 @@ func updateLocation( path string, grpc bool, keepAliveCheck keepAliveChecker, - mirrorPercentage float64, + mirrorPercentage *float64, ) http.Location { if filters.InvalidFilter != nil { location.Return = &http.Return{Code: http.StatusInternalServerError} @@ -533,7 +529,7 @@ func updateLocationMirrorFilters( location http.Location, mirrorFilters []*dataplane.HTTPRequestMirrorFilter, path string, - mirrorPercentage float64, + mirrorPercentage *float64, ) http.Location { for _, filter := range mirrorFilters { if filter.Target != nil { @@ -547,9 +543,9 @@ func updateLocationMirrorFilters( // if the mirrorPercentage is 100.0 or negative (we set it to negative when there is no mirror filter because 0.0 // is valid), the split clients variable is not generated, and we want to let all the traffic get mirrored. - if mirrorPercentage != 100.0 && mirrorPercentage >= 0.0 { + if mirrorPercentage != nil && *mirrorPercentage != 100.0 { location.MirrorSplitClientsVariableName = convertSplitClientVariableName( - fmt.Sprintf("%s_%.2f", path, mirrorPercentage), + fmt.Sprintf("%s_%.2f", path, *mirrorPercentage), ) } @@ -599,7 +595,7 @@ func updateLocations( path string, grpc bool, keepAliveCheck keepAliveChecker, - mirrorPercentage float64, + mirrorPercentage *float64, ) []http.Location { updatedLocations := make([]http.Location, len(buildLocations)) @@ -1070,12 +1066,12 @@ func getConnectionHeader(keepAliveCheck keepAliveChecker, backends []dataplane.B // deduplicateStrings removes duplicate strings from a slice while preserving order. func deduplicateStrings(content []string) []string { - seen := make(map[string]bool) + seen := make(map[string]struct{}) result := make([]string, 0, len(content)) for _, str := range content { - if !seen[str] { - seen[str] = true + if _, exists := seen[str]; !exists { + seen[str] = struct{}{} result = append(result, str) } } diff --git a/internal/controller/nginx/config/split_clients.go b/internal/controller/nginx/config/split_clients.go index 4019eb3c72..e78a74ce4f 100644 --- a/internal/controller/nginx/config/split_clients.go +++ b/internal/controller/nginx/config/split_clients.go @@ -42,15 +42,15 @@ func createRequestMirrorSplitClients(servers []dataplane.VirtualServer) []http.S mirrorPathToPercentage := extractMirrorTargetsWithPercentages(server.PathRules) for _, pathRule := range server.PathRules { - mirrorPercentage, exists := mirrorPathToPercentage[pathRule.Path] + mirrorPercentage := mirrorPathToPercentage[pathRule.Path] - if mirrorPercentage != 100 && exists { + if mirrorPercentage != nil && *mirrorPercentage != 100 { splitClient := http.SplitClient{ - // this has to be something unique and able to be accessed from the server side - VariableName: convertSplitClientVariableName(fmt.Sprintf("%s_%.2f", pathRule.Path, mirrorPercentage)), + // this has to be something unique and able to be accessed from the server block + VariableName: convertSplitClientVariableName(fmt.Sprintf("%s_%.2f", pathRule.Path, *mirrorPercentage)), Distributions: []http.SplitClientDistribution{ { - Percent: fmt.Sprintf("%.2f", mirrorPercentage), + Percent: fmt.Sprintf("%.2f", *mirrorPercentage), Value: pathRule.Path, }, { @@ -79,12 +79,12 @@ func convertSplitClientVariableName(name string) string { } func removeDuplicateSplitClients(splitClients []http.SplitClient) []http.SplitClient { - seen := make(map[string]bool) + seen := make(map[string]struct{}) result := make([]http.SplitClient, 0, len(splitClients)) for _, client := range splitClients { - if !seen[client.VariableName] { - seen[client.VariableName] = true + if _, exists := seen[client.VariableName]; !exists { + seen[client.VariableName] = struct{}{} result = append(result, client) } } diff --git a/internal/controller/nginx/config/split_clients_template.go b/internal/controller/nginx/config/split_clients_template.go index d3192e0040..d0b933012b 100644 --- a/internal/controller/nginx/config/split_clients_template.go +++ b/internal/controller/nginx/config/split_clients_template.go @@ -6,9 +6,9 @@ split_clients $request_id ${{ $sc.VariableName }} { {{- range $d := $sc.Distributions }} {{- if eq $d.Percent "0.00" }} # {{ $d.Percent }}% {{ $d.Value }}; - {{- else if eq $d.Percent "*" }} - * {{ $d.Value }}; - {{- else }} + {{- else if eq $d.Percent "*" }} + * {{ $d.Value }}; + {{- else }} {{ $d.Percent }}% {{ $d.Value }}; {{- end }} {{- end }} diff --git a/internal/controller/state/dataplane/convert.go b/internal/controller/state/dataplane/convert.go index 666b85cabb..bff41f1b6c 100644 --- a/internal/controller/state/dataplane/convert.go +++ b/internal/controller/state/dataplane/convert.go @@ -95,6 +95,10 @@ func convertHTTPRequestMirrorFilter( result.Percent = helpers.GetPointer(float64(100)) } + if *result.Percent > 100.0 { + result.Percent = helpers.GetPointer(100.0) + } + return result } diff --git a/internal/controller/state/dataplane/convert_test.go b/internal/controller/state/dataplane/convert_test.go index d515d3e6fb..3b5ebcfa9b 100644 --- a/internal/controller/state/dataplane/convert_test.go +++ b/internal/controller/state/dataplane/convert_test.go @@ -361,6 +361,25 @@ func TestConvertHTTPMirrorFilter(t *testing.T) { }, name: "fraction denominator not specified", }, + { + filter: &v1.HTTPRequestMirrorFilter{ + BackendRef: v1.BackendObjectReference{ + Name: "backend", + Namespace: helpers.GetPointer[v1.Namespace]("namespace"), + }, + Fraction: &v1.Fraction{ + Numerator: 300, + Denominator: helpers.GetPointer(int32(1)), + }, + }, + expected: &HTTPRequestMirrorFilter{ + Name: helpers.GetPointer("backend"), + Namespace: helpers.GetPointer("namespace"), + Target: helpers.GetPointer("/_ngf-internal-mirror-namespace/backend-test/route1-0"), + Percent: helpers.GetPointer(float64(100)), + }, + name: "fraction result over 100", + }, { filter: &v1.HTTPRequestMirrorFilter{ BackendRef: v1.BackendObjectReference{ diff --git a/tests/Makefile b/tests/Makefile index 4f0e86b247..1e986c4984 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -12,7 +12,7 @@ GW_SERVICE_TYPE = NodePort## Service type to use for the gateway NGF_VERSION ?= edge## NGF version to be tested PULL_POLICY = Never## Pull policy for the images NGINX_CONF_DIR = internal/controller/nginx/conf -SUPPORTED_EXTENDED_FEATURES = HTTPRouteQueryParamMatching,HTTPRouteMethodMatching,HTTPRoutePortRedirect,HTTPRouteSchemeRedirect,HTTPRouteHostRewrite,HTTPRoutePathRewrite,GatewayPort8080,GatewayAddressEmpty,HTTPRouteResponseHeaderModification,HTTPRoutePathRedirect,GatewayHTTPListenerIsolation,GatewayInfrastructurePropagation,HTTPRouteRequestMirror,HTTPRouteRequestMultipleMirrors,HTTPRouteBackendProtocolWebSocket,HTTPRouteRequestPercentageMirror +SUPPORTED_EXTENDED_FEATURES = HTTPRouteQueryParamMatching,HTTPRouteMethodMatching,HTTPRoutePortRedirect,HTTPRouteSchemeRedirect,HTTPRouteHostRewrite,HTTPRoutePathRewrite,GatewayPort8080,GatewayAddressEmpty,HTTPRouteResponseHeaderModification,HTTPRoutePathRedirect,GatewayHTTPListenerIsolation,GatewayInfrastructurePropagation,HTTPRouteRequestMirror,HTTPRouteRequestMultipleMirrors,HTTPRouteRequestPercentageMirror,HTTPRouteBackendProtocolWebSocket 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. From 00bcccc597da15a5bed460a9b38b37360c64a386 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Tue, 15 Jul 2025 17:20:26 -0700 Subject: [PATCH 05/13] Remove filter validation already covered by CEL validation --- .../controller/state/graph/common_filter.go | 34 ---- .../state/graph/common_filter_test.go | 160 +++--------------- 2 files changed, 27 insertions(+), 167 deletions(-) diff --git a/internal/controller/state/graph/common_filter.go b/internal/controller/state/graph/common_filter.go index e1fe43a372..41266d42c2 100644 --- a/internal/controller/state/graph/common_filter.go +++ b/internal/controller/state/graph/common_filter.go @@ -230,40 +230,6 @@ func validateFilterMirror(mirror *v1.HTTPRequestMirrorFilter, filterPath *field. return field.ErrorList{field.Required(mirrorPath, "cannot be nil")} } - if mirror.Percent != nil && mirror.Fraction != nil { - return field.ErrorList{field.Invalid(mirrorPath, mirror, "cannot set both percent and fraction")} - } - - if mirror.Fraction != nil && mirror.Fraction.Denominator != nil { - if *mirror.Fraction.Denominator <= 0 { - return field.ErrorList{ - field.Invalid( - mirrorPath.Child("fraction", "denominator"), - mirror.Fraction.Denominator, - "must be greater than 0", - ), - } - } - if mirror.Fraction.Numerator < 0 { - return field.ErrorList{ - field.Invalid( - mirrorPath.Child("fraction", "numerator"), - mirror.Fraction.Numerator, - "must be greater than or equal to 0", - ), - } - } - if mirror.Fraction.Numerator > *mirror.Fraction.Denominator { - return field.ErrorList{ - field.Invalid( - mirrorPath.Child("fraction", "numerator"), - mirror.Fraction.Numerator, - "must be less than or equal to denominator", - ), - } - } - } - return nil } diff --git a/internal/controller/state/graph/common_filter_test.go b/internal/controller/state/graph/common_filter_test.go index 33c28e029a..32451858d6 100644 --- a/internal/controller/state/graph/common_filter_test.go +++ b/internal/controller/state/graph/common_filter_test.go @@ -10,7 +10,6 @@ import ( ngfAPI "github.com/nginx/nginx-gateway-fabric/apis/v1alpha1" "github.com/nginx/nginx-gateway-fabric/internal/controller/state/validation/validationfakes" - "github.com/nginx/nginx-gateway-fabric/internal/framework/helpers" "github.com/nginx/nginx-gateway-fabric/internal/framework/kinds" ) @@ -49,6 +48,24 @@ func TestValidateFilter(t *testing.T) { expectErrCount: 0, name: "valid HTTP request header modifiers filter", }, + { + filter: Filter{ + RouteType: RouteTypeHTTP, + FilterType: FilterRequestMirror, + RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{}, + }, + expectErrCount: 0, + name: "valid HTTP mirror filter", + }, + { + filter: Filter{ + RouteType: RouteTypeHTTP, + FilterType: FilterRequestMirror, + RequestMirror: nil, + }, + expectErrCount: 1, + name: "invalid HTTP mirror filter", + }, { filter: Filter{ RouteType: RouteTypeHTTP, @@ -79,6 +96,15 @@ func TestValidateFilter(t *testing.T) { expectErrCount: 1, name: "unsupported HTTP filter type", }, + { + filter: Filter{ + RouteType: RouteTypeGRPC, + FilterType: FilterRequestMirror, + RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{}, + }, + expectErrCount: 0, + name: "valid GRPC mirror filter", + }, { filter: Filter{ RouteType: RouteTypeGRPC, @@ -133,138 +159,6 @@ func TestValidateFilter(t *testing.T) { } } -func TestValidateFilterMirror(t *testing.T) { - t.Parallel() - - tests := []struct { - filter Filter - name string - expectErrCount int - }{ - { - filter: Filter{ - RouteType: RouteTypeHTTP, - FilterType: FilterRequestMirror, - RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{}, - }, - expectErrCount: 0, - name: "valid HTTP mirror filter", - }, - { - filter: Filter{ - RouteType: RouteTypeHTTP, - FilterType: FilterRequestMirror, - RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{ - Percent: helpers.GetPointer(int32(50)), - }, - }, - expectErrCount: 0, - name: "valid HTTP mirror filter with percentage set", - }, - { - filter: Filter{ - RouteType: RouteTypeHTTP, - FilterType: FilterRequestMirror, - RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{ - Fraction: &gatewayv1.Fraction{ - Numerator: 1, - Denominator: helpers.GetPointer(int32(2)), - }, - }, - }, - expectErrCount: 0, - name: "valid HTTP mirror filter with fraction set", - }, - { - filter: Filter{ - RouteType: RouteTypeHTTP, - FilterType: FilterRequestMirror, - RequestMirror: nil, - }, - expectErrCount: 1, - name: "invalid nil HTTP mirror filter", - }, - { - filter: Filter{ - RouteType: RouteTypeHTTP, - FilterType: FilterRequestMirror, - RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{ - Percent: helpers.GetPointer(int32(5)), - Fraction: &gatewayv1.Fraction{ - Numerator: 1, - Denominator: helpers.GetPointer(int32(3)), - }, - }, - }, - expectErrCount: 1, - name: "invalid HTTP mirror filter both percent and fraction set", - }, - { - filter: Filter{ - RouteType: RouteTypeHTTP, - FilterType: FilterRequestMirror, - RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{ - Fraction: &gatewayv1.Fraction{ - Numerator: 1, - Denominator: helpers.GetPointer(int32(0)), - }, - }, - }, - expectErrCount: 1, - name: "invalid HTTP mirror filter, fraction denominator value must be greater than 0", - }, - { - filter: Filter{ - RouteType: RouteTypeHTTP, - FilterType: FilterRequestMirror, - RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{ - Fraction: &gatewayv1.Fraction{ - Numerator: -1, - Denominator: helpers.GetPointer(int32(2)), - }, - }, - }, - expectErrCount: 1, - name: "invalid HTTP mirror filter, fraction numerator value must be greater than or equal to 0", - }, - { - filter: Filter{ - RouteType: RouteTypeHTTP, - FilterType: FilterRequestMirror, - RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{ - Fraction: &gatewayv1.Fraction{ - Numerator: 5, - Denominator: helpers.GetPointer(int32(2)), - }, - }, - }, - expectErrCount: 1, - name: "invalid HTTP mirror filter, fraction numerator value must be less than denominator", - }, - { - filter: Filter{ - RouteType: RouteTypeGRPC, - FilterType: FilterRequestMirror, - RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{}, - }, - expectErrCount: 0, - name: "valid GRPC mirror filter", - }, - } - - filterPath := field.NewPath("test") - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - t.Parallel() - - g := NewWithT(t) - allErrs := validateFilter(&validationfakes.FakeHTTPFieldsValidator{}, test.filter, filterPath) - g.Expect(allErrs).To(HaveLen(test.expectErrCount)) - }) - } -} - func TestValidateFilterResponseHeaderModifier(t *testing.T) { t.Parallel() From 72bedec80070c7523d9737e4251b9bd53b2b05f7 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Tue, 15 Jul 2025 17:28:40 -0700 Subject: [PATCH 06/13] Add more feedback changes --- internal/controller/nginx/config/split_clients.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/internal/controller/nginx/config/split_clients.go b/internal/controller/nginx/config/split_clients.go index e78a74ce4f..983ddc71d5 100644 --- a/internal/controller/nginx/config/split_clients.go +++ b/internal/controller/nginx/config/split_clients.go @@ -41,17 +41,15 @@ func createRequestMirrorSplitClients(servers []dataplane.VirtualServer) []http.S for _, server := range servers { mirrorPathToPercentage := extractMirrorTargetsWithPercentages(server.PathRules) - for _, pathRule := range server.PathRules { - mirrorPercentage := mirrorPathToPercentage[pathRule.Path] - - if mirrorPercentage != nil && *mirrorPercentage != 100 { + for path, percentage := range mirrorPathToPercentage { + if percentage != nil && *percentage != 100 { splitClient := http.SplitClient{ // this has to be something unique and able to be accessed from the server block - VariableName: convertSplitClientVariableName(fmt.Sprintf("%s_%.2f", pathRule.Path, *mirrorPercentage)), + VariableName: convertSplitClientVariableName(fmt.Sprintf("%s_%.2f", path, *percentage)), Distributions: []http.SplitClientDistribution{ { - Percent: fmt.Sprintf("%.2f", *mirrorPercentage), - Value: pathRule.Path, + Percent: fmt.Sprintf("%.2f", *percentage), + Value: path, }, { Percent: "*", From ce3e5e783e0ef5c6ca7c606d8c514303f045e178 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Wed, 16 Jul 2025 12:00:56 -0700 Subject: [PATCH 07/13] Add back validation --- .../controller/state/graph/common_filter.go | 34 ++++ .../state/graph/common_filter_test.go | 160 +++++++++++++++--- 2 files changed, 167 insertions(+), 27 deletions(-) diff --git a/internal/controller/state/graph/common_filter.go b/internal/controller/state/graph/common_filter.go index 41266d42c2..e1fe43a372 100644 --- a/internal/controller/state/graph/common_filter.go +++ b/internal/controller/state/graph/common_filter.go @@ -230,6 +230,40 @@ func validateFilterMirror(mirror *v1.HTTPRequestMirrorFilter, filterPath *field. return field.ErrorList{field.Required(mirrorPath, "cannot be nil")} } + if mirror.Percent != nil && mirror.Fraction != nil { + return field.ErrorList{field.Invalid(mirrorPath, mirror, "cannot set both percent and fraction")} + } + + if mirror.Fraction != nil && mirror.Fraction.Denominator != nil { + if *mirror.Fraction.Denominator <= 0 { + return field.ErrorList{ + field.Invalid( + mirrorPath.Child("fraction", "denominator"), + mirror.Fraction.Denominator, + "must be greater than 0", + ), + } + } + if mirror.Fraction.Numerator < 0 { + return field.ErrorList{ + field.Invalid( + mirrorPath.Child("fraction", "numerator"), + mirror.Fraction.Numerator, + "must be greater than or equal to 0", + ), + } + } + if mirror.Fraction.Numerator > *mirror.Fraction.Denominator { + return field.ErrorList{ + field.Invalid( + mirrorPath.Child("fraction", "numerator"), + mirror.Fraction.Numerator, + "must be less than or equal to denominator", + ), + } + } + } + return nil } diff --git a/internal/controller/state/graph/common_filter_test.go b/internal/controller/state/graph/common_filter_test.go index 32451858d6..33c28e029a 100644 --- a/internal/controller/state/graph/common_filter_test.go +++ b/internal/controller/state/graph/common_filter_test.go @@ -10,6 +10,7 @@ import ( ngfAPI "github.com/nginx/nginx-gateway-fabric/apis/v1alpha1" "github.com/nginx/nginx-gateway-fabric/internal/controller/state/validation/validationfakes" + "github.com/nginx/nginx-gateway-fabric/internal/framework/helpers" "github.com/nginx/nginx-gateway-fabric/internal/framework/kinds" ) @@ -48,24 +49,6 @@ func TestValidateFilter(t *testing.T) { expectErrCount: 0, name: "valid HTTP request header modifiers filter", }, - { - filter: Filter{ - RouteType: RouteTypeHTTP, - FilterType: FilterRequestMirror, - RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{}, - }, - expectErrCount: 0, - name: "valid HTTP mirror filter", - }, - { - filter: Filter{ - RouteType: RouteTypeHTTP, - FilterType: FilterRequestMirror, - RequestMirror: nil, - }, - expectErrCount: 1, - name: "invalid HTTP mirror filter", - }, { filter: Filter{ RouteType: RouteTypeHTTP, @@ -96,15 +79,6 @@ func TestValidateFilter(t *testing.T) { expectErrCount: 1, name: "unsupported HTTP filter type", }, - { - filter: Filter{ - RouteType: RouteTypeGRPC, - FilterType: FilterRequestMirror, - RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{}, - }, - expectErrCount: 0, - name: "valid GRPC mirror filter", - }, { filter: Filter{ RouteType: RouteTypeGRPC, @@ -159,6 +133,138 @@ func TestValidateFilter(t *testing.T) { } } +func TestValidateFilterMirror(t *testing.T) { + t.Parallel() + + tests := []struct { + filter Filter + name string + expectErrCount int + }{ + { + filter: Filter{ + RouteType: RouteTypeHTTP, + FilterType: FilterRequestMirror, + RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{}, + }, + expectErrCount: 0, + name: "valid HTTP mirror filter", + }, + { + filter: Filter{ + RouteType: RouteTypeHTTP, + FilterType: FilterRequestMirror, + RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{ + Percent: helpers.GetPointer(int32(50)), + }, + }, + expectErrCount: 0, + name: "valid HTTP mirror filter with percentage set", + }, + { + filter: Filter{ + RouteType: RouteTypeHTTP, + FilterType: FilterRequestMirror, + RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{ + Fraction: &gatewayv1.Fraction{ + Numerator: 1, + Denominator: helpers.GetPointer(int32(2)), + }, + }, + }, + expectErrCount: 0, + name: "valid HTTP mirror filter with fraction set", + }, + { + filter: Filter{ + RouteType: RouteTypeHTTP, + FilterType: FilterRequestMirror, + RequestMirror: nil, + }, + expectErrCount: 1, + name: "invalid nil HTTP mirror filter", + }, + { + filter: Filter{ + RouteType: RouteTypeHTTP, + FilterType: FilterRequestMirror, + RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{ + Percent: helpers.GetPointer(int32(5)), + Fraction: &gatewayv1.Fraction{ + Numerator: 1, + Denominator: helpers.GetPointer(int32(3)), + }, + }, + }, + expectErrCount: 1, + name: "invalid HTTP mirror filter both percent and fraction set", + }, + { + filter: Filter{ + RouteType: RouteTypeHTTP, + FilterType: FilterRequestMirror, + RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{ + Fraction: &gatewayv1.Fraction{ + Numerator: 1, + Denominator: helpers.GetPointer(int32(0)), + }, + }, + }, + expectErrCount: 1, + name: "invalid HTTP mirror filter, fraction denominator value must be greater than 0", + }, + { + filter: Filter{ + RouteType: RouteTypeHTTP, + FilterType: FilterRequestMirror, + RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{ + Fraction: &gatewayv1.Fraction{ + Numerator: -1, + Denominator: helpers.GetPointer(int32(2)), + }, + }, + }, + expectErrCount: 1, + name: "invalid HTTP mirror filter, fraction numerator value must be greater than or equal to 0", + }, + { + filter: Filter{ + RouteType: RouteTypeHTTP, + FilterType: FilterRequestMirror, + RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{ + Fraction: &gatewayv1.Fraction{ + Numerator: 5, + Denominator: helpers.GetPointer(int32(2)), + }, + }, + }, + expectErrCount: 1, + name: "invalid HTTP mirror filter, fraction numerator value must be less than denominator", + }, + { + filter: Filter{ + RouteType: RouteTypeGRPC, + FilterType: FilterRequestMirror, + RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{}, + }, + expectErrCount: 0, + name: "valid GRPC mirror filter", + }, + } + + filterPath := field.NewPath("test") + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + g := NewWithT(t) + allErrs := validateFilter(&validationfakes.FakeHTTPFieldsValidator{}, test.filter, filterPath) + g.Expect(allErrs).To(HaveLen(test.expectErrCount)) + }) + } +} + func TestValidateFilterResponseHeaderModifier(t *testing.T) { t.Parallel() From b68d549348d90546a908fb13cc3f85866e6fc95b Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Wed, 16 Jul 2025 12:51:27 -0700 Subject: [PATCH 08/13] Refactor mirror percentage onto pathrules --- internal/controller/nginx/config/servers.go | 8 ++-- .../controller/nginx/config/servers_test.go | 9 ++++- .../controller/nginx/config/split_clients.go | 34 ++++++++++++---- .../nginx/config/split_clients_test.go | 39 ++++++++++++------- .../state/dataplane/configuration.go | 39 +++++++++++++++---- internal/controller/state/dataplane/types.go | 3 ++ 6 files changed, 98 insertions(+), 34 deletions(-) diff --git a/internal/controller/nginx/config/servers.go b/internal/controller/nginx/config/servers.go index c8c1d8a9bb..1e3a997ac8 100644 --- a/internal/controller/nginx/config/servers.go +++ b/internal/controller/nginx/config/servers.go @@ -266,7 +266,7 @@ func createLocations( var rootPathExists bool var grpcServer bool - mirrorPathToPercentage := extractMirrorTargetsWithPercentages(server.PathRules) + //mirrorPathToPercentage := extractMirrorTargetsWithPercentages(server.PathRules) for pathRuleIdx, rule := range server.PathRules { matches := make([]routeMatch, 0, len(rule.MatchRules)) @@ -279,7 +279,7 @@ func createLocations( grpcServer = true } - mirrorPercentage := mirrorPathToPercentage[rule.Path] + //mirrorPercentage := mirrorPathToPercentage[rule.Path] extLocations := initializeExternalLocations(rule, pathsAndTypes) for i := range extLocations { @@ -298,7 +298,7 @@ func createLocations( rule.Path, rule.GRPC, keepAliveCheck, - mirrorPercentage, + rule.MirrorPercent, ) } @@ -322,7 +322,7 @@ func createLocations( rule.Path, rule.GRPC, keepAliveCheck, - mirrorPercentage, + rule.MirrorPercent, ) internalLocations = append(internalLocations, intLocation) diff --git a/internal/controller/nginx/config/servers_test.go b/internal/controller/nginx/config/servers_test.go index b1313338ca..145e3cb287 100644 --- a/internal/controller/nginx/config/servers_test.go +++ b/internal/controller/nginx/config/servers_test.go @@ -105,6 +105,7 @@ func TestExecuteServers(t *testing.T) { }, }, }, + MirrorPercent: helpers.GetPointer(float64(50)), }, }, }, @@ -952,6 +953,7 @@ func TestCreateServers(t *testing.T) { BackendGroup: fooGroup, }, }, + MirrorPercent: helpers.GetPointer(float64(50)), }, { Path: "/mirror-filter-100-percent", @@ -982,6 +984,7 @@ func TestCreateServers(t *testing.T) { BackendGroup: fooGroup, }, }, + MirrorPercent: helpers.GetPointer(float64(100)), }, { Path: "/mirror-filter-0-percent", @@ -1012,6 +1015,7 @@ func TestCreateServers(t *testing.T) { BackendGroup: fooGroup, }, }, + MirrorPercent: helpers.GetPointer(float64(0)), }, { Path: "/mirror-filter-duplicate-targets", @@ -1054,6 +1058,7 @@ func TestCreateServers(t *testing.T) { BackendGroup: fooGroup, }, }, + MirrorPercent: helpers.GetPointer(float64(50)), }, { Path: "/grpc/mirror", @@ -2646,8 +2651,8 @@ func TestCreateReturnValForRedirectFilter(t *testing.T) { modifiedHTTPRequestRedirectFilter := func( mod func( - filter *dataplane.HTTPRequestRedirectFilter, - ) *dataplane.HTTPRequestRedirectFilter, + filter *dataplane.HTTPRequestRedirectFilter, + ) *dataplane.HTTPRequestRedirectFilter, ) *dataplane.HTTPRequestRedirectFilter { return mod(createBasicHTTPRequestRedirectFilter()) } diff --git a/internal/controller/nginx/config/split_clients.go b/internal/controller/nginx/config/split_clients.go index 983ddc71d5..bc7b51a478 100644 --- a/internal/controller/nginx/config/split_clients.go +++ b/internal/controller/nginx/config/split_clients.go @@ -39,17 +39,15 @@ func createRequestMirrorSplitClients(servers []dataplane.VirtualServer) []http.S var splitClients []http.SplitClient for _, server := range servers { - mirrorPathToPercentage := extractMirrorTargetsWithPercentages(server.PathRules) - - for path, percentage := range mirrorPathToPercentage { - if percentage != nil && *percentage != 100 { + for _, pathRule := range server.PathRules { + if pathRule.MirrorPercent != nil && *pathRule.MirrorPercent != 100 { splitClient := http.SplitClient{ // this has to be something unique and able to be accessed from the server block - VariableName: convertSplitClientVariableName(fmt.Sprintf("%s_%.2f", path, *percentage)), + VariableName: convertSplitClientVariableName(fmt.Sprintf("%s_%.2f", pathRule.Path, *pathRule.MirrorPercent)), Distributions: []http.SplitClientDistribution{ { - Percent: fmt.Sprintf("%.2f", *percentage), - Value: path, + Percent: fmt.Sprintf("%.2f", *pathRule.MirrorPercent), + Value: pathRule.Path, }, { Percent: "*", @@ -61,6 +59,28 @@ func createRequestMirrorSplitClients(servers []dataplane.VirtualServer) []http.S splitClients = append(splitClients, splitClient) } } + //mirrorPathToPercentage := extractMirrorTargetsWithPercentages(server.PathRules) + // + //for path, percentage := range mirrorPathToPercentage { + // if percentage != nil && *percentage != 100 { + // splitClient := http.SplitClient{ + // // this has to be something unique and able to be accessed from the server block + // VariableName: convertSplitClientVariableName(fmt.Sprintf("%s_%.2f", path, *percentage)), + // Distributions: []http.SplitClientDistribution{ + // { + // Percent: fmt.Sprintf("%.2f", *percentage), + // Value: path, + // }, + // { + // Percent: "*", + // Value: "\"\"", + // }, + // }, + // } + // + // splitClients = append(splitClients, splitClient) + // } + //} } return splitClients diff --git a/internal/controller/nginx/config/split_clients_test.go b/internal/controller/nginx/config/split_clients_test.go index 639b047ed2..552fbae866 100644 --- a/internal/controller/nginx/config/split_clients_test.go +++ b/internal/controller/nginx/config/split_clients_test.go @@ -133,13 +133,16 @@ func TestExecuteSplitClients(t *testing.T) { }, }, { - Path: http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route1-0", + Path: http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route1-0", + MirrorPercent: helpers.GetPointer(float64(25)), }, { - Path: http.InternalMirrorRoutePathPrefix + "-my-tea-backend-test/route1-0", + Path: http.InternalMirrorRoutePathPrefix + "-my-tea-backend-test/route1-0", + MirrorPercent: helpers.GetPointer(float64(50)), }, { - Path: http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route1-1", + Path: http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route1-1", + MirrorPercent: helpers.GetPointer(float64(25)), }, { Path: "/mirror-edge-case-percentages", @@ -175,13 +178,16 @@ func TestExecuteSplitClients(t *testing.T) { }, }, { - Path: http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route2-0", + Path: http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route2-0", + MirrorPercent: helpers.GetPointer(float64(0)), }, { - Path: http.InternalMirrorRoutePathPrefix + "-my-tea-backend-test/route2-0", + Path: http.InternalMirrorRoutePathPrefix + "-my-tea-backend-test/route2-0", + MirrorPercent: helpers.GetPointer(float64(99.999)), }, { - Path: http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route2-1", + Path: http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route2-1", + MirrorPercent: helpers.GetPointer(float64(0.001)), }, }, }, @@ -241,7 +247,8 @@ func TestExecuteSplitClients(t *testing.T) { }, }, { - Path: http.InternalMirrorRoutePathPrefix + "-my-same-backend-test/route1-0", + Path: http.InternalMirrorRoutePathPrefix + "-my-same-backend-test/route1-0", + MirrorPercent: helpers.GetPointer(float64(50)), }, }, }, @@ -297,7 +304,8 @@ func TestExecuteSplitClients(t *testing.T) { }, }, { - Path: http.InternalMirrorRoutePathPrefix + "-my-backend-test/route1-0", + Path: http.InternalMirrorRoutePathPrefix + "-my-backend-test/route1-0", + MirrorPercent: helpers.GetPointer(float64(25)), }, }, }, @@ -321,7 +329,8 @@ func TestExecuteSplitClients(t *testing.T) { }, }, { - Path: http.InternalMirrorRoutePathPrefix + "-my-ssl-backend-test/route1-0", + Path: http.InternalMirrorRoutePathPrefix + "-my-ssl-backend-test/route1-0", + MirrorPercent: helpers.GetPointer(float64(50)), }, }, }, @@ -410,13 +419,16 @@ func TestCreateRequestMirrorSplitClients(t *testing.T) { }, }, { - Path: http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route1-0", + Path: http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route1-0", + MirrorPercent: helpers.GetPointer(float64(25)), }, { - Path: http.InternalMirrorRoutePathPrefix + "-my-tea-backend-test/route1-0", + Path: http.InternalMirrorRoutePathPrefix + "-my-tea-backend-test/route1-0", + MirrorPercent: helpers.GetPointer(float64(50)), }, { - Path: http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route1-1", + Path: http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route1-1", + MirrorPercent: helpers.GetPointer(float64(25)), }, }, }, @@ -438,7 +450,8 @@ func TestCreateRequestMirrorSplitClients(t *testing.T) { }, }, { - Path: http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route1-0", + Path: http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route1-0", + MirrorPercent: helpers.GetPointer(float64(30)), }, }, }, diff --git a/internal/controller/state/dataplane/configuration.go b/internal/controller/state/dataplane/configuration.go index ecdba8d3e8..1b7bc0e673 100644 --- a/internal/controller/state/dataplane/configuration.go +++ b/internal/controller/state/dataplane/configuration.go @@ -462,18 +462,20 @@ type pathAndType struct { } type hostPathRules struct { - rulesPerHost map[string]map[pathAndType]PathRule - listenersForHost map[string]*graph.Listener - httpsListeners []*graph.Listener - port int32 - listenersExist bool + rulesPerHost map[string]map[pathAndType]PathRule + listenersForHost map[string]*graph.Listener + httpsListeners []*graph.Listener + port int32 + listenersExist bool + mirrorTargetToPercentage map[string]*float64 } func newHostPathRules() *hostPathRules { return &hostPathRules{ - rulesPerHost: make(map[string]map[pathAndType]PathRule), - listenersForHost: make(map[string]*graph.Listener), - httpsListeners: make([]*graph.Listener, 0), + rulesPerHost: make(map[string]map[pathAndType]PathRule), + listenersForHost: make(map[string]*graph.Listener), + httpsListeners: make([]*graph.Listener, 0), + mirrorTargetToPercentage: make(map[string]*float64), } } @@ -553,6 +555,25 @@ func (hpr *hostPathRules) upsertRoute( pols := buildPolicies(gateway, route.Policies) for _, h := range hostnames { + + if filters.RequestMirrors != nil { + for _, m := range filters.RequestMirrors { + if m.Target != nil { + if m.Percent == nil { + hpr.mirrorTargetToPercentage[*m.Target] = helpers.GetPointer(100.0) + continue + } + + percentage := m.Percent + + if _, exists := hpr.mirrorTargetToPercentage[*m.Target]; !exists || + *percentage > *hpr.mirrorTargetToPercentage[*m.Target] { + hpr.mirrorTargetToPercentage[*m.Target] = percentage // set a higher percentage if it exists + } + } + } + } + for _, m := range rule.Matches { path := getPath(m.Path) @@ -607,6 +628,8 @@ func (hpr *hostPathRules) buildServers() []VirtualServer { for _, r := range rules { sortMatchRules(r.MatchRules) + r.MirrorPercent = hpr.mirrorTargetToPercentage[r.Path] + s.PathRules = append(s.PathRules, r) } diff --git a/internal/controller/state/dataplane/types.go b/internal/controller/state/dataplane/types.go index 0cfe48beb0..12620c3dff 100644 --- a/internal/controller/state/dataplane/types.go +++ b/internal/controller/state/dataplane/types.go @@ -135,6 +135,9 @@ type PathRule struct { Policies []policies.Policy // GRPC indicates if this is a gRPC rule GRPC bool + // MirrorPercent is the percentage of requests to mirror for this PathRule. If this PathRule is not an internal + // mirrored rule, this field is nil. + MirrorPercent *float64 } // InvalidHTTPFilter is a special filter for handling the case when configured filters are invalid. From 14df0a5e62a508a5292d1a431596044378fec363 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Wed, 16 Jul 2025 14:44:37 -0700 Subject: [PATCH 09/13] Revert "Refactor mirror percentage onto pathrules" This reverts commit 0b6afd87347fabc982e20cbe86ee063572e1f793. --- internal/controller/nginx/config/servers.go | 8 ++-- .../controller/nginx/config/servers_test.go | 9 +---- .../controller/nginx/config/split_clients.go | 34 ++++------------ .../nginx/config/split_clients_test.go | 39 +++++++------------ .../state/dataplane/configuration.go | 39 ++++--------------- internal/controller/state/dataplane/types.go | 3 -- 6 files changed, 34 insertions(+), 98 deletions(-) diff --git a/internal/controller/nginx/config/servers.go b/internal/controller/nginx/config/servers.go index 1e3a997ac8..c8c1d8a9bb 100644 --- a/internal/controller/nginx/config/servers.go +++ b/internal/controller/nginx/config/servers.go @@ -266,7 +266,7 @@ func createLocations( var rootPathExists bool var grpcServer bool - //mirrorPathToPercentage := extractMirrorTargetsWithPercentages(server.PathRules) + mirrorPathToPercentage := extractMirrorTargetsWithPercentages(server.PathRules) for pathRuleIdx, rule := range server.PathRules { matches := make([]routeMatch, 0, len(rule.MatchRules)) @@ -279,7 +279,7 @@ func createLocations( grpcServer = true } - //mirrorPercentage := mirrorPathToPercentage[rule.Path] + mirrorPercentage := mirrorPathToPercentage[rule.Path] extLocations := initializeExternalLocations(rule, pathsAndTypes) for i := range extLocations { @@ -298,7 +298,7 @@ func createLocations( rule.Path, rule.GRPC, keepAliveCheck, - rule.MirrorPercent, + mirrorPercentage, ) } @@ -322,7 +322,7 @@ func createLocations( rule.Path, rule.GRPC, keepAliveCheck, - rule.MirrorPercent, + mirrorPercentage, ) internalLocations = append(internalLocations, intLocation) diff --git a/internal/controller/nginx/config/servers_test.go b/internal/controller/nginx/config/servers_test.go index 145e3cb287..b1313338ca 100644 --- a/internal/controller/nginx/config/servers_test.go +++ b/internal/controller/nginx/config/servers_test.go @@ -105,7 +105,6 @@ func TestExecuteServers(t *testing.T) { }, }, }, - MirrorPercent: helpers.GetPointer(float64(50)), }, }, }, @@ -953,7 +952,6 @@ func TestCreateServers(t *testing.T) { BackendGroup: fooGroup, }, }, - MirrorPercent: helpers.GetPointer(float64(50)), }, { Path: "/mirror-filter-100-percent", @@ -984,7 +982,6 @@ func TestCreateServers(t *testing.T) { BackendGroup: fooGroup, }, }, - MirrorPercent: helpers.GetPointer(float64(100)), }, { Path: "/mirror-filter-0-percent", @@ -1015,7 +1012,6 @@ func TestCreateServers(t *testing.T) { BackendGroup: fooGroup, }, }, - MirrorPercent: helpers.GetPointer(float64(0)), }, { Path: "/mirror-filter-duplicate-targets", @@ -1058,7 +1054,6 @@ func TestCreateServers(t *testing.T) { BackendGroup: fooGroup, }, }, - MirrorPercent: helpers.GetPointer(float64(50)), }, { Path: "/grpc/mirror", @@ -2651,8 +2646,8 @@ func TestCreateReturnValForRedirectFilter(t *testing.T) { modifiedHTTPRequestRedirectFilter := func( mod func( - filter *dataplane.HTTPRequestRedirectFilter, - ) *dataplane.HTTPRequestRedirectFilter, + filter *dataplane.HTTPRequestRedirectFilter, + ) *dataplane.HTTPRequestRedirectFilter, ) *dataplane.HTTPRequestRedirectFilter { return mod(createBasicHTTPRequestRedirectFilter()) } diff --git a/internal/controller/nginx/config/split_clients.go b/internal/controller/nginx/config/split_clients.go index bc7b51a478..983ddc71d5 100644 --- a/internal/controller/nginx/config/split_clients.go +++ b/internal/controller/nginx/config/split_clients.go @@ -39,15 +39,17 @@ func createRequestMirrorSplitClients(servers []dataplane.VirtualServer) []http.S var splitClients []http.SplitClient for _, server := range servers { - for _, pathRule := range server.PathRules { - if pathRule.MirrorPercent != nil && *pathRule.MirrorPercent != 100 { + mirrorPathToPercentage := extractMirrorTargetsWithPercentages(server.PathRules) + + for path, percentage := range mirrorPathToPercentage { + if percentage != nil && *percentage != 100 { splitClient := http.SplitClient{ // this has to be something unique and able to be accessed from the server block - VariableName: convertSplitClientVariableName(fmt.Sprintf("%s_%.2f", pathRule.Path, *pathRule.MirrorPercent)), + VariableName: convertSplitClientVariableName(fmt.Sprintf("%s_%.2f", path, *percentage)), Distributions: []http.SplitClientDistribution{ { - Percent: fmt.Sprintf("%.2f", *pathRule.MirrorPercent), - Value: pathRule.Path, + Percent: fmt.Sprintf("%.2f", *percentage), + Value: path, }, { Percent: "*", @@ -59,28 +61,6 @@ func createRequestMirrorSplitClients(servers []dataplane.VirtualServer) []http.S splitClients = append(splitClients, splitClient) } } - //mirrorPathToPercentage := extractMirrorTargetsWithPercentages(server.PathRules) - // - //for path, percentage := range mirrorPathToPercentage { - // if percentage != nil && *percentage != 100 { - // splitClient := http.SplitClient{ - // // this has to be something unique and able to be accessed from the server block - // VariableName: convertSplitClientVariableName(fmt.Sprintf("%s_%.2f", path, *percentage)), - // Distributions: []http.SplitClientDistribution{ - // { - // Percent: fmt.Sprintf("%.2f", *percentage), - // Value: path, - // }, - // { - // Percent: "*", - // Value: "\"\"", - // }, - // }, - // } - // - // splitClients = append(splitClients, splitClient) - // } - //} } return splitClients diff --git a/internal/controller/nginx/config/split_clients_test.go b/internal/controller/nginx/config/split_clients_test.go index 552fbae866..639b047ed2 100644 --- a/internal/controller/nginx/config/split_clients_test.go +++ b/internal/controller/nginx/config/split_clients_test.go @@ -133,16 +133,13 @@ func TestExecuteSplitClients(t *testing.T) { }, }, { - Path: http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route1-0", - MirrorPercent: helpers.GetPointer(float64(25)), + Path: http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route1-0", }, { - Path: http.InternalMirrorRoutePathPrefix + "-my-tea-backend-test/route1-0", - MirrorPercent: helpers.GetPointer(float64(50)), + Path: http.InternalMirrorRoutePathPrefix + "-my-tea-backend-test/route1-0", }, { - Path: http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route1-1", - MirrorPercent: helpers.GetPointer(float64(25)), + Path: http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route1-1", }, { Path: "/mirror-edge-case-percentages", @@ -178,16 +175,13 @@ func TestExecuteSplitClients(t *testing.T) { }, }, { - Path: http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route2-0", - MirrorPercent: helpers.GetPointer(float64(0)), + Path: http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route2-0", }, { - Path: http.InternalMirrorRoutePathPrefix + "-my-tea-backend-test/route2-0", - MirrorPercent: helpers.GetPointer(float64(99.999)), + Path: http.InternalMirrorRoutePathPrefix + "-my-tea-backend-test/route2-0", }, { - Path: http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route2-1", - MirrorPercent: helpers.GetPointer(float64(0.001)), + Path: http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route2-1", }, }, }, @@ -247,8 +241,7 @@ func TestExecuteSplitClients(t *testing.T) { }, }, { - Path: http.InternalMirrorRoutePathPrefix + "-my-same-backend-test/route1-0", - MirrorPercent: helpers.GetPointer(float64(50)), + Path: http.InternalMirrorRoutePathPrefix + "-my-same-backend-test/route1-0", }, }, }, @@ -304,8 +297,7 @@ func TestExecuteSplitClients(t *testing.T) { }, }, { - Path: http.InternalMirrorRoutePathPrefix + "-my-backend-test/route1-0", - MirrorPercent: helpers.GetPointer(float64(25)), + Path: http.InternalMirrorRoutePathPrefix + "-my-backend-test/route1-0", }, }, }, @@ -329,8 +321,7 @@ func TestExecuteSplitClients(t *testing.T) { }, }, { - Path: http.InternalMirrorRoutePathPrefix + "-my-ssl-backend-test/route1-0", - MirrorPercent: helpers.GetPointer(float64(50)), + Path: http.InternalMirrorRoutePathPrefix + "-my-ssl-backend-test/route1-0", }, }, }, @@ -419,16 +410,13 @@ func TestCreateRequestMirrorSplitClients(t *testing.T) { }, }, { - Path: http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route1-0", - MirrorPercent: helpers.GetPointer(float64(25)), + Path: http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route1-0", }, { - Path: http.InternalMirrorRoutePathPrefix + "-my-tea-backend-test/route1-0", - MirrorPercent: helpers.GetPointer(float64(50)), + Path: http.InternalMirrorRoutePathPrefix + "-my-tea-backend-test/route1-0", }, { - Path: http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route1-1", - MirrorPercent: helpers.GetPointer(float64(25)), + Path: http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route1-1", }, }, }, @@ -450,8 +438,7 @@ func TestCreateRequestMirrorSplitClients(t *testing.T) { }, }, { - Path: http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route1-0", - MirrorPercent: helpers.GetPointer(float64(30)), + Path: http.InternalMirrorRoutePathPrefix + "-my-coffee-backend-test/route1-0", }, }, }, diff --git a/internal/controller/state/dataplane/configuration.go b/internal/controller/state/dataplane/configuration.go index 1b7bc0e673..ecdba8d3e8 100644 --- a/internal/controller/state/dataplane/configuration.go +++ b/internal/controller/state/dataplane/configuration.go @@ -462,20 +462,18 @@ type pathAndType struct { } type hostPathRules struct { - rulesPerHost map[string]map[pathAndType]PathRule - listenersForHost map[string]*graph.Listener - httpsListeners []*graph.Listener - port int32 - listenersExist bool - mirrorTargetToPercentage map[string]*float64 + rulesPerHost map[string]map[pathAndType]PathRule + listenersForHost map[string]*graph.Listener + httpsListeners []*graph.Listener + port int32 + listenersExist bool } func newHostPathRules() *hostPathRules { return &hostPathRules{ - rulesPerHost: make(map[string]map[pathAndType]PathRule), - listenersForHost: make(map[string]*graph.Listener), - httpsListeners: make([]*graph.Listener, 0), - mirrorTargetToPercentage: make(map[string]*float64), + rulesPerHost: make(map[string]map[pathAndType]PathRule), + listenersForHost: make(map[string]*graph.Listener), + httpsListeners: make([]*graph.Listener, 0), } } @@ -555,25 +553,6 @@ func (hpr *hostPathRules) upsertRoute( pols := buildPolicies(gateway, route.Policies) for _, h := range hostnames { - - if filters.RequestMirrors != nil { - for _, m := range filters.RequestMirrors { - if m.Target != nil { - if m.Percent == nil { - hpr.mirrorTargetToPercentage[*m.Target] = helpers.GetPointer(100.0) - continue - } - - percentage := m.Percent - - if _, exists := hpr.mirrorTargetToPercentage[*m.Target]; !exists || - *percentage > *hpr.mirrorTargetToPercentage[*m.Target] { - hpr.mirrorTargetToPercentage[*m.Target] = percentage // set a higher percentage if it exists - } - } - } - } - for _, m := range rule.Matches { path := getPath(m.Path) @@ -628,8 +607,6 @@ func (hpr *hostPathRules) buildServers() []VirtualServer { for _, r := range rules { sortMatchRules(r.MatchRules) - r.MirrorPercent = hpr.mirrorTargetToPercentage[r.Path] - s.PathRules = append(s.PathRules, r) } diff --git a/internal/controller/state/dataplane/types.go b/internal/controller/state/dataplane/types.go index 12620c3dff..0cfe48beb0 100644 --- a/internal/controller/state/dataplane/types.go +++ b/internal/controller/state/dataplane/types.go @@ -135,9 +135,6 @@ type PathRule struct { Policies []policies.Policy // GRPC indicates if this is a gRPC rule GRPC bool - // MirrorPercent is the percentage of requests to mirror for this PathRule. If this PathRule is not an internal - // mirrored rule, this field is nil. - MirrorPercent *float64 } // InvalidHTTPFilter is a special filter for handling the case when configured filters are invalid. From 444a4865d8c2dc3b5ac63615846125bd38710374 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Thu, 17 Jul 2025 09:39:56 -0700 Subject: [PATCH 10/13] Change comment --- internal/controller/nginx/config/servers.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/controller/nginx/config/servers.go b/internal/controller/nginx/config/servers.go index c8c1d8a9bb..eaf1a78379 100644 --- a/internal/controller/nginx/config/servers.go +++ b/internal/controller/nginx/config/servers.go @@ -541,8 +541,8 @@ func updateLocationMirrorFilters( location.MirrorPaths = deduplicateStrings(location.MirrorPaths) } - // if the mirrorPercentage is 100.0 or negative (we set it to negative when there is no mirror filter because 0.0 - // is valid), the split clients variable is not generated, and we want to let all the traffic get mirrored. + // if mirrorPercentage is nil (no mirror filter configured) or 100.0, the split clients variable is not generated, + // and we let all traffic get mirrored. if mirrorPercentage != nil && *mirrorPercentage != 100.0 { location.MirrorSplitClientsVariableName = convertSplitClientVariableName( fmt.Sprintf("%s_%.2f", path, *mirrorPercentage), From 0eabb190eb5a4560b010bd23a78026c4d5acb83f Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Thu, 17 Jul 2025 09:41:00 -0700 Subject: [PATCH 11/13] Adjust test names --- internal/controller/state/mirror/mirror_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/controller/state/mirror/mirror_test.go b/internal/controller/state/mirror/mirror_test.go index 69e9e65f1f..3a4ba5bc36 100644 --- a/internal/controller/state/mirror/mirror_test.go +++ b/internal/controller/state/mirror/mirror_test.go @@ -29,7 +29,7 @@ func TestPathWithBackendRef(t *testing.T) { idx int }{ { - name: "with namespace", + name: "with backendRef namespace", idx: 1, backendRef: v1.BackendObjectReference{ Name: "service1", @@ -39,7 +39,7 @@ func TestPathWithBackendRef(t *testing.T) { expected: helpers.GetPointer("/_ngf-internal-mirror-namespace1/service1-routeNs/routeName1-1"), }, { - name: "without namespace", + name: "without backendRef namespace", idx: 2, backendRef: v1.BackendObjectReference{ Name: "service2", @@ -72,7 +72,7 @@ func TestBackendPath(t *testing.T) { idx int }{ { - name: "With namespace", + name: "With backendRef namespace", idx: 1, namespace: helpers.GetPointer("namespace1"), service: "service1", @@ -80,7 +80,7 @@ func TestBackendPath(t *testing.T) { expected: helpers.GetPointer("/_ngf-internal-mirror-namespace1/service1-routeNs/routeName1-1"), }, { - name: "Without namespace", + name: "Without backendRef namespace", idx: 2, namespace: nil, service: "service2", From 61b8ceeb623508696466711af812cfd3c1bda7fb Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Thu, 17 Jul 2025 09:59:15 -0700 Subject: [PATCH 12/13] Adjust split clients test assertion --- internal/controller/nginx/config/split_clients_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/controller/nginx/config/split_clients_test.go b/internal/controller/nginx/config/split_clients_test.go index 639b047ed2..63bbbc2896 100644 --- a/internal/controller/nginx/config/split_clients_test.go +++ b/internal/controller/nginx/config/split_clients_test.go @@ -542,7 +542,7 @@ func TestCreateRequestMirrorSplitClients(t *testing.T) { t.Parallel() g := NewWithT(t) result := createRequestMirrorSplitClients(test.servers) - g.Expect(result).To(Equal(test.expSplitClients)) + g.Expect(result).To(ContainElements(test.expSplitClients)) }) } } From 27e033ecb2246fe455fc206c42561d1c88cc921b Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Thu, 17 Jul 2025 14:01:22 -0700 Subject: [PATCH 13/13] Refactor argument length --- internal/controller/nginx/config/servers.go | 34 +++++++++------------ 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/internal/controller/nginx/config/servers.go b/internal/controller/nginx/config/servers.go index eaf1a78379..ec37ed764e 100644 --- a/internal/controller/nginx/config/servers.go +++ b/internal/controller/nginx/config/servers.go @@ -291,12 +291,10 @@ func createLocations( if !needsInternalLocations(rule) { for _, r := range rule.MatchRules { extLocations = updateLocations( - r.Filters, - extLocations, r, + rule, + extLocations, server.Port, - rule.Path, - rule.GRPC, keepAliveCheck, mirrorPercentage, ) @@ -315,12 +313,10 @@ func createLocations( ) intLocation = updateLocation( - r.Filters, - intLocation, r, + rule, + intLocation, server.Port, - rule.Path, - rule.GRPC, keepAliveCheck, mirrorPercentage, ) @@ -453,15 +449,17 @@ func initializeInternalLocation( // updateLocation updates a location with any relevant configurations, like proxy_pass, filters, tls settings, etc. func updateLocation( - filters dataplane.HTTPFilters, - location http.Location, matchRule dataplane.MatchRule, + pathRule dataplane.PathRule, + location http.Location, listenerPort int32, - path string, - grpc bool, keepAliveCheck keepAliveChecker, mirrorPercentage *float64, ) http.Location { + filters := matchRule.Filters + path := pathRule.Path + grpc := pathRule.GRPC + if filters.InvalidFilter != nil { location.Return = &http.Return{Code: http.StatusInternalServerError} return location @@ -588,12 +586,10 @@ func updateLocationProxySettings( // updateLocations updates the existing locations with any relevant configurations, like proxy_pass, // filters, tls settings, etc. func updateLocations( - filters dataplane.HTTPFilters, - buildLocations []http.Location, matchRule dataplane.MatchRule, + pathRule dataplane.PathRule, + buildLocations []http.Location, listenerPort int32, - path string, - grpc bool, keepAliveCheck keepAliveChecker, mirrorPercentage *float64, ) []http.Location { @@ -601,12 +597,10 @@ func updateLocations( for i, loc := range buildLocations { updatedLocations[i] = updateLocation( - filters, - loc, matchRule, + pathRule, + loc, listenerPort, - path, - grpc, keepAliveCheck, mirrorPercentage, )