From e6cbab0351e519695767245f4b34d2837f7530e4 Mon Sep 17 00:00:00 2001 From: Venktesh Shivam Patel Date: Wed, 5 Nov 2025 16:15:27 +0000 Subject: [PATCH 1/7] initial commit for rewrite-target annotation --- internal/configs/annotations.go | 23 +++++++++++ internal/configs/ingress.go | 10 +++-- internal/configs/version1/config.go | 2 + .../configs/version1/nginx-plus.ingress.tmpl | 3 ++ internal/configs/version1/nginx.ingress.tmpl | 3 ++ internal/configs/version1/template_helper.go | 39 +++++++++++++++++++ 6 files changed, 77 insertions(+), 3 deletions(-) diff --git a/internal/configs/annotations.go b/internal/configs/annotations.go index 56f1564727..c36c0d418a 100644 --- a/internal/configs/annotations.go +++ b/internal/configs/annotations.go @@ -18,6 +18,9 @@ const BasicAuthSecretAnnotation = "nginx.org/basic-auth-secret" // #nosec G101 // PathRegexAnnotation is the annotation where the regex location (path) modifier is specified. const PathRegexAnnotation = "nginx.org/path-regex" +// RewriteTargetAnnotation is the annotation where the regex-based rewrite target is specified. +const RewriteTargetAnnotation = "nginx.org/rewrite-target" + // SSLCiphersAnnotation is the annotation where SSL ciphers are specified. const SSLCiphersAnnotation = "nginx.org/ssl-ciphers" @@ -590,6 +593,26 @@ func getRewrites(ctx context.Context, ingEx *IngressEx) map[string]string { return nil } +func getRewriteTarget(ctx context.Context, ingEx *IngressEx) (string, Warnings) { + l := nl.LoggerFromContext(ctx) + warnings := newWarnings() + + // Check for mutual exclusivity + if _, hasRewrites := ingEx.Ingress.Annotations["nginx.org/rewrites"]; hasRewrites { + if _, hasRewriteTarget := ingEx.Ingress.Annotations[RewriteTargetAnnotation]; hasRewriteTarget { + warningMsg := "nginx.org/rewrites and nginx.org/rewrite-target annotations are mutually exclusive; nginx.org/rewrites will take precedence" + nl.Errorf(l, "Ingress %s/%s: %s", ingEx.Ingress.Namespace, ingEx.Ingress.Name, warningMsg) + warnings.AddWarning(ingEx.Ingress, warningMsg) + return "", warnings + } + } + + if value, exists := ingEx.Ingress.Annotations[RewriteTargetAnnotation]; exists { + return value, warnings + } + return "", warnings +} + func getSSLServices(ingEx *IngressEx) map[string]bool { if value, exists := ingEx.Ingress.Annotations["nginx.org/ssl-services"]; exists { return ParseServiceList(value) diff --git a/internal/configs/ingress.go b/internal/configs/ingress.go index 6cf3599a3c..4afc56883e 100644 --- a/internal/configs/ingress.go +++ b/internal/configs/ingress.go @@ -103,6 +103,7 @@ func generateNginxCfg(p NginxCfgParams) (version1.IngressNginxConfig, Warnings) wsServices := getWebsocketServices(p.ingEx) spServices := getSessionPersistenceServices(p.BaseCfgParams.Context, p.ingEx) rewrites := getRewrites(p.BaseCfgParams.Context, p.ingEx) + rewriteTarget, rewriteTargetWarnings := getRewriteTarget(p.BaseCfgParams.Context, p.ingEx) sslServices := getSSLServices(p.ingEx) grpcServices := getGrpcServices(p.ingEx) @@ -129,6 +130,7 @@ func generateNginxCfg(p NginxCfgParams) (version1.IngressNginxConfig, Warnings) } allWarnings := newWarnings() + allWarnings.Add(rewriteTargetWarnings) var servers []version1.Server var limitReqZones []version1.LimitReqZone @@ -255,7 +257,7 @@ func generateNginxCfg(p NginxCfgParams) (version1.IngressNginxConfig, Warnings) ssl := isSSLEnabled(sslServices[path.Backend.Service.Name], cfgParams, p.staticParams) proxySSLName := generateProxySSLName(path.Backend.Service.Name, p.ingEx.Ingress.Namespace) loc := createLocation(pathOrDefault(path.Path), upstreams[upsName], &cfgParams, wsServices[path.Backend.Service.Name], rewrites[path.Backend.Service.Name], - ssl, grpcServices[path.Backend.Service.Name], proxySSLName, path.PathType, path.Backend.Service.Name) + ssl, grpcServices[path.Backend.Service.Name], proxySSLName, path.PathType, path.Backend.Service.Name, rewriteTarget, path.Path) if p.isMinion && cfgParams.JWTKey != "" { jwtAuth, redirectLoc, warnings := generateJWTConfig(p.ingEx.Ingress, p.ingEx.SecretRefs, &cfgParams, getNameForRedirectLocation(p.ingEx.Ingress)) @@ -320,7 +322,7 @@ func generateNginxCfg(p NginxCfgParams) (version1.IngressNginxConfig, Warnings) pathtype := networking.PathTypePrefix loc := createLocation(pathOrDefault("/"), upstreams[upsName], &cfgParams, wsServices[p.ingEx.Ingress.Spec.DefaultBackend.Service.Name], rewrites[p.ingEx.Ingress.Spec.DefaultBackend.Service.Name], - ssl, grpcServices[p.ingEx.Ingress.Spec.DefaultBackend.Service.Name], proxySSLName, &pathtype, p.ingEx.Ingress.Spec.DefaultBackend.Service.Name) + ssl, grpcServices[p.ingEx.Ingress.Spec.DefaultBackend.Service.Name], proxySSLName, &pathtype, p.ingEx.Ingress.Spec.DefaultBackend.Service.Name, rewriteTarget, "/") locations = append(locations, loc) if cfgParams.HealthCheckEnabled { @@ -487,9 +489,10 @@ func generateIngressPath(path string, pathType *networking.PathType) string { return path } -func createLocation(path string, upstream version1.Upstream, cfg *ConfigParams, websocket bool, rewrite string, ssl bool, grpc bool, proxySSLName string, pathType *networking.PathType, serviceName string) version1.Location { +func createLocation(path string, upstream version1.Upstream, cfg *ConfigParams, websocket bool, rewrite string, ssl bool, grpc bool, proxySSLName string, pathType *networking.PathType, serviceName string, rewriteTarget string, originalPath string) version1.Location { loc := version1.Location{ Path: generateIngressPath(path, pathType), + OriginalPath: originalPath, Upstream: upstream, ProxyConnectTimeout: cfg.ProxyConnectTimeout, ProxyReadTimeout: cfg.ProxyReadTimeout, @@ -498,6 +501,7 @@ func createLocation(path string, upstream version1.Upstream, cfg *ConfigParams, ClientMaxBodySize: cfg.ClientMaxBodySize, Websocket: websocket, Rewrite: rewrite, + RewriteTarget: rewriteTarget, SSL: ssl, GRPC: grpc, ProxyBuffering: cfg.ProxyBuffering, diff --git a/internal/configs/version1/config.go b/internal/configs/version1/config.go index d492697df6..7961017010 100644 --- a/internal/configs/version1/config.go +++ b/internal/configs/version1/config.go @@ -169,6 +169,7 @@ type LimitReq struct { type Location struct { LocationSnippets []string Path string + OriginalPath string Upstream Upstream ProxyConnectTimeout string ProxyReadTimeout string @@ -177,6 +178,7 @@ type Location struct { ClientMaxBodySize string Websocket bool Rewrite string + RewriteTarget string SSL bool GRPC bool ProxyBuffering bool diff --git a/internal/configs/version1/nginx-plus.ingress.tmpl b/internal/configs/version1/nginx-plus.ingress.tmpl index a92d37a890..a7b343aa75 100644 --- a/internal/configs/version1/nginx-plus.ingress.tmpl +++ b/internal/configs/version1/nginx-plus.ingress.tmpl @@ -202,6 +202,9 @@ server { set $resource_name "{{$location.MinionIngress.Name}}"; set $resource_namespace "{{$location.MinionIngress.Namespace}}"; {{- end}} + {{- if $location.RewriteTarget}} + rewrite {{ makeRewritePattern $location $.Ingress.Annotations }} {{$location.RewriteTarget}} break; + {{- end}} {{- if $location.GRPC}} {{- if not $server.GRPCOnly}} error_page 400 @grpcerror400; diff --git a/internal/configs/version1/nginx.ingress.tmpl b/internal/configs/version1/nginx.ingress.tmpl index 289860e28f..47e28c5bbb 100644 --- a/internal/configs/version1/nginx.ingress.tmpl +++ b/internal/configs/version1/nginx.ingress.tmpl @@ -124,6 +124,9 @@ server { set $resource_name "{{$location.MinionIngress.Name}}"; set $resource_namespace "{{$location.MinionIngress.Namespace}}"; {{- end}} + {{- if $location.RewriteTarget}} + rewrite {{ makeRewritePattern $location $.Ingress.Annotations }} {{$location.RewriteTarget}} break; + {{- end}} {{- if $location.GRPC}} {{- if not $server.GRPCOnly}} error_page 400 @grpcerror400; diff --git a/internal/configs/version1/template_helper.go b/internal/configs/version1/template_helper.go index 203f2b4a2e..e25c3db672 100644 --- a/internal/configs/version1/template_helper.go +++ b/internal/configs/version1/template_helper.go @@ -202,6 +202,44 @@ func makeResolver(resolverAddresses []string, resolverValid string, resolverIPV6 return builder.String() } +// makeRewritePattern takes a location and Ingress annotations and returns +// a rewrite pattern that matches the location pattern used. +// This ensures the rewrite regex matches the same requests as the location. +func makeRewritePattern(loc *Location, ingressAnnotations map[string]string) string { + var regexType string + var hasRegex bool + + // Check for path-regex annotation (same logic as makeLocationPath) + if loc.MinionIngress != nil { + ingressType, isMergeable := loc.MinionIngress.Annotations["nginx.org/mergeable-ingress-type"] + regexType, hasRegex = loc.MinionIngress.Annotations["nginx.org/path-regex"] + if !(isMergeable && ingressType == "minion" && hasRegex) { + hasRegex = false + } + } + + if !hasRegex { + regexType, hasRegex = ingressAnnotations["nginx.org/path-regex"] + } + + // If no path-regex annotation, return original path + if !hasRegex { + return loc.OriginalPath + } + + // Generate rewrite pattern based on regex type + switch regexType { + case "case_sensitive": + return fmt.Sprintf("^%s", loc.OriginalPath) + case "case_insensitive": + return fmt.Sprintf("(?i)^%s", loc.OriginalPath) + case "exact": + return loc.OriginalPath // exact matches don't need anchors in rewrite + default: + return loc.OriginalPath + } +} + var helperFunctions = template.FuncMap{ "split": split, "trim": trim, @@ -212,6 +250,7 @@ var helperFunctions = template.FuncMap{ "toUpper": strings.ToUpper, "replaceAll": strings.ReplaceAll, "makeLocationPath": makeLocationPath, + "makeRewritePattern": makeRewritePattern, "makeSecretPath": commonhelpers.MakeSecretPath, "makeOnOffFromBool": commonhelpers.MakeOnOffFromBool, "generateProxySetHeaders": generateProxySetHeaders, From b50a22979785f5f992499c9182b129bfa5c4164b Mon Sep 17 00:00:00 2001 From: Venktesh Shivam Patel Date: Fri, 7 Nov 2025 11:54:42 +0000 Subject: [PATCH 2/7] add validations and unit test --- .../rewrite-target/README.md | 112 ++++ .../rewrite-target/cafe.yaml | 65 +++ .../rewrite-target/regex-rewrite.yaml | 20 + .../rewrite-target/simple-rewrite.yaml | 19 + internal/configs/annotations.go | 4 +- internal/configs/annotations_test.go | 202 +++++++ internal/configs/ingress.go | 7 +- .../version1/__snapshots__/template_test.snap | 502 ++++++++++++++++++ internal/configs/version1/config.go | 1 - internal/configs/version1/template_helper.go | 44 +- .../configs/version1/template_helper_test.go | 149 ++++++ internal/configs/version1/template_test.go | 472 ++++++++++++++++ internal/k8s/validation.go | 31 ++ internal/k8s/validation_test.go | 136 +++++ 14 files changed, 1751 insertions(+), 13 deletions(-) create mode 100644 examples/ingress-resources/rewrite-target/README.md create mode 100644 examples/ingress-resources/rewrite-target/cafe.yaml create mode 100644 examples/ingress-resources/rewrite-target/regex-rewrite.yaml create mode 100644 examples/ingress-resources/rewrite-target/simple-rewrite.yaml diff --git a/examples/ingress-resources/rewrite-target/README.md b/examples/ingress-resources/rewrite-target/README.md new file mode 100644 index 0000000000..ed4ae28a38 --- /dev/null +++ b/examples/ingress-resources/rewrite-target/README.md @@ -0,0 +1,112 @@ +# Support for Rewrite Target + +The `nginx.org/rewrite-target` annotation enables URL path rewriting by specifying a target path that requests should be rewritten to. This annotation works with regular expression capture groups from the Ingress path to create dynamic rewrites. + +The annotation is mutually exclusive with `nginx.org/rewrites`. If both are present, `nginx.org/rewrites` takes precedence. + +## Running the Example + +## 1. Deploy the Ingress Controller + +1. Follow the [installation](https://docs.nginx.com/nginx-ingress-controller/installation/installation-with-manifests/) instructions to deploy the Ingress Controller. + +2. Save the public IP address of the Ingress Controller into a shell variable: + ```console + IC_IP=XXX.YYY.ZZZ.III + ``` + +3. Save the HTTP port of the Ingress Controller into a shell variable: + ```console + IC_HTTP_PORT= + ``` + +## 2. Deploy the Cafe Application + +Create the coffee and tea deployments and services: + +```console +kubectl create -f cafe.yaml +``` + +## 3. Configure Rewrite Examples + +### Example 1: Simple Static Rewrite + +Create an Ingress resource with basic rewrite functionality: + +```console +kubectl create -f simple-rewrite.yaml +``` + +This configures rewriting from `/coffee` to `/beverages/coffee`. + +### Example 2: Dynamic Rewrite with Regex + +Create an Ingress resource with regular expression-based rewriting: + +```console +kubectl create -f regex-rewrite.yaml +``` + +This configures dynamic rewriting using capture groups from `/menu/([^/]+)/([^/]+)` to `/beverages/$1/$2`. + +## 4. Test the Application + +### Test Simple Rewrite + +Access the coffee service through the rewritten path: + +```console +curl --resolve cafe.example.com:$IC_HTTP_PORT:$IC_IP http://cafe.example.com:$IC_HTTP_PORT/coffee --insecure +``` + +```text +Server address: 10.16.0.16:8080 +Server name: coffee-676c9f8944-n2bmb +Date: 07/Nov/2025:11:23:09 +0000 +URI: /beverages/coffee +Request ID: c224b3e06d79b66f8f33e86cef046c32 +``` + +The request to `/coffee` is rewritten to `/beverages/coffee`. + +### Test Regex Rewrite + +Access the service using the menu path with dynamic rewriting: + +```console +curl --resolve cafe.example.com:$IC_HTTP_PORT:$IC_IP http://cafe.example.com:$IC_HTTP_PORT/menu/coffee/espresso --insecure +``` + +```text +Server address: 10.16.1.29:8080 +Server name: coffee-676c9f8944-vj45p +Date: 07/Nov/2025:11:26:05 +0000 +URI: /beverages/coffee/espresso +Request ID: 88334a8b0eeaee2ffe4fdb4c7768641b +``` + +```console +curl --resolve cafe.example.com:$IC_HTTP_PORT:$IC_IP http://cafe.example.com:$IC_HTTP_PORT/menu/tea/green --insecure +``` + +```text +Server address: 10.16.0.16:8080 +Server name: coffee-676c9f8944-n2bmb +Date: 07/Nov/2025:11:26:33 +0000 +URI: /beverages/tea/green +Request ID: 2ba8f9055aecc059b32f797f1ce2aca5 +``` + +The requests to `/menu/coffee/espresso` and `/menu/tea/green` are rewritten to `/beverages/coffee/espresso` and `/beverages/tea/green` using the captured groups. + +## Validations + +1. Mutual Exclusivity: The `nginx.org/rewrite-target` annotation is mutually exclusive with `nginx.org/rewrites`. If both annotations are present, `nginx.org/rewrites` takes precedence and a warning will be generated. + +2. Security Validation: The annotation includes built-in security validation to prevent: + - Absolute URLs (`http://` or `https://`) + - Protocol-relative URLs (`//`) + - Path traversal patterns (`../` or `..\\`) + - Paths not starting with `/` + diff --git a/examples/ingress-resources/rewrite-target/cafe.yaml b/examples/ingress-resources/rewrite-target/cafe.yaml new file mode 100644 index 0000000000..07653b72cd --- /dev/null +++ b/examples/ingress-resources/rewrite-target/cafe.yaml @@ -0,0 +1,65 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: coffee +spec: + replicas: 2 + selector: + matchLabels: + app: coffee + template: + metadata: + labels: + app: coffee + spec: + containers: + - name: coffee + image: nginxdemos/nginx-hello:plain-text + ports: + - containerPort: 8080 +--- +apiVersion: v1 +kind: Service +metadata: + name: coffee-svc +spec: + ports: + - port: 80 + targetPort: 8080 + protocol: TCP + name: http + selector: + app: coffee +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: tea +spec: + replicas: 3 + selector: + matchLabels: + app: tea + template: + metadata: + labels: + app: tea + spec: + containers: + - name: tea + image: nginxdemos/nginx-hello:plain-text + ports: + - containerPort: 8080 +--- +apiVersion: v1 +kind: Service +metadata: + name: tea-svc +spec: + ports: + - port: 80 + targetPort: 8080 + protocol: TCP + name: http + selector: + app: tea \ No newline at end of file diff --git a/examples/ingress-resources/rewrite-target/regex-rewrite.yaml b/examples/ingress-resources/rewrite-target/regex-rewrite.yaml new file mode 100644 index 0000000000..488ee8abb7 --- /dev/null +++ b/examples/ingress-resources/rewrite-target/regex-rewrite.yaml @@ -0,0 +1,20 @@ +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: cafe-ingress + annotations: + nginx.org/path-regex: "case_sensitive" + nginx.org/rewrite-target: "/beverages/$1/$2" +spec: + ingressClassName: nginx + rules: + - host: cafe.example.com + http: + paths: + - path: /menu/([^/]+)/([^/]+) + pathType: ImplementationSpecific + backend: + service: + name: coffee-svc + port: + number: 80 \ No newline at end of file diff --git a/examples/ingress-resources/rewrite-target/simple-rewrite.yaml b/examples/ingress-resources/rewrite-target/simple-rewrite.yaml new file mode 100644 index 0000000000..21c7b38561 --- /dev/null +++ b/examples/ingress-resources/rewrite-target/simple-rewrite.yaml @@ -0,0 +1,19 @@ +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: cafe-ingress + annotations: + nginx.org/rewrite-target: "/beverages/coffee" +spec: + ingressClassName: nginx + rules: + - host: cafe.example.com + http: + paths: + - path: /coffee + pathType: Prefix + backend: + service: + name: coffee-svc + port: + number: 80 \ No newline at end of file diff --git a/internal/configs/annotations.go b/internal/configs/annotations.go index c36c0d418a..8e1dc373ce 100644 --- a/internal/configs/annotations.go +++ b/internal/configs/annotations.go @@ -596,7 +596,7 @@ func getRewrites(ctx context.Context, ingEx *IngressEx) map[string]string { func getRewriteTarget(ctx context.Context, ingEx *IngressEx) (string, Warnings) { l := nl.LoggerFromContext(ctx) warnings := newWarnings() - + // Check for mutual exclusivity if _, hasRewrites := ingEx.Ingress.Annotations["nginx.org/rewrites"]; hasRewrites { if _, hasRewriteTarget := ingEx.Ingress.Annotations[RewriteTargetAnnotation]; hasRewriteTarget { @@ -606,7 +606,7 @@ func getRewriteTarget(ctx context.Context, ingEx *IngressEx) (string, Warnings) return "", warnings } } - + if value, exists := ingEx.Ingress.Annotations[RewriteTargetAnnotation]; exists { return value, warnings } diff --git a/internal/configs/annotations_test.go b/internal/configs/annotations_test.go index ba49039841..de1d511402 100644 --- a/internal/configs/annotations_test.go +++ b/internal/configs/annotations_test.go @@ -553,3 +553,205 @@ func TestSSLCipherAnnotationBooleanValues(t *testing.T) { }) } } + +func TestGetRewriteTarget(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + annotations map[string]string + expectedValue string + expectedWarningMsgs []string + description string + }{ + { + name: "rewrite-target only", + annotations: map[string]string{ + "nginx.org/rewrite-target": "/api/v1/$1", + }, + expectedValue: "/api/v1/$1", + expectedWarningMsgs: nil, + description: "Should return rewrite-target value when only rewrite-target annotation is present", + }, + { + name: "rewrites only", + annotations: map[string]string{ + "nginx.org/rewrites": "serviceName=app-svc rewrite=/backend/", + }, + expectedValue: "", + expectedWarningMsgs: nil, + description: "Should return empty string when only rewrites annotation is present", + }, + { + name: "both annotations present - mutual exclusivity", + annotations: map[string]string{ + "nginx.org/rewrite-target": "/api/v1/$1", + "nginx.org/rewrites": "serviceName=app-svc rewrite=/backend/", + }, + expectedValue: "", + expectedWarningMsgs: []string{"nginx.org/rewrites and nginx.org/rewrite-target annotations are mutually exclusive; nginx.org/rewrites will take precedence"}, + description: "Should return empty string and warning when both annotations are present (rewrites takes precedence)", + }, + { + name: "no rewrite annotations", + annotations: map[string]string{}, + expectedValue: "", + expectedWarningMsgs: nil, + description: "Should return empty string when no rewrite annotations are present", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create test Ingress + ingress := &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ingress", + Namespace: "default", + Annotations: tt.annotations, + }, + } + + ingEx := &IngressEx{ + Ingress: ingress, + } + + // Call getRewriteTarget + ctx := context.Background() + value, warnings := getRewriteTarget(ctx, ingEx) + + // Verify return value + if value != tt.expectedValue { + t.Errorf("Test %q: expected value %q, got %q. %s", tt.name, tt.expectedValue, value, tt.description) + } + + // Verify warnings + if len(tt.expectedWarningMsgs) == 0 { + if len(warnings) != 0 { + t.Errorf("Test %q: expected no warnings, got %d warnings. %s", tt.name, len(warnings), tt.description) + } + } else { + // Check that warnings contain our Ingress + ingressWarnings, exists := warnings[ingress] + if !exists { + t.Errorf("Test %q: expected warnings for ingress, but found none. %s", tt.name, tt.description) + return + } + + // Check warning count + if len(ingressWarnings) != len(tt.expectedWarningMsgs) { + t.Errorf("Test %q: expected %d warnings, got %d. %s", tt.name, len(tt.expectedWarningMsgs), len(ingressWarnings), tt.description) + } + + // Check warning messages + for i, expectedMsg := range tt.expectedWarningMsgs { + if i < len(ingressWarnings) { + if ingressWarnings[i] != expectedMsg { + t.Errorf("Test %q: expected warning message %q, got %q. %s", tt.name, expectedMsg, ingressWarnings[i], tt.description) + } + } + } + } + }) + } +} + +func TestGetRewriteTargetMutualExclusivity(t *testing.T) { + t.Parallel() + + // Test that when both annotations exist, rewrites takes precedence + ingress := &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ingress", + Namespace: "default", + Annotations: map[string]string{ + "nginx.org/rewrite-target": "/should/not/be/used/$1", + "nginx.org/rewrites": "serviceName=coffee-svc rewrite=/coffee/beans/", + }, + }, + } + + ingEx := &IngressEx{ + Ingress: ingress, + } + + ctx := context.Background() + value, warnings := getRewriteTarget(ctx, ingEx) + + // Should return empty string (rewrite-target disabled) + if value != "" { + t.Errorf("Expected empty string when both annotations present, got %q", value) + } + + // Should have warning about mutual exclusivity + ingressWarnings, exists := warnings[ingress] + if !exists || len(ingressWarnings) == 0 { + t.Error("Expected warning about mutual exclusivity") + } + + expectedWarning := "nginx.org/rewrites and nginx.org/rewrite-target annotations are mutually exclusive; nginx.org/rewrites will take precedence" + if len(ingressWarnings) > 0 && ingressWarnings[0] != expectedWarning { + t.Errorf("Expected warning message %q, got %q", expectedWarning, ingressWarnings[0]) + } +} + +func TestGetRewriteTargetWithComplexValues(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + rewriteTarget string + expected string + }{ + { + name: "simple path replacement", + rewriteTarget: "/api/$1", + expected: "/api/$1", + }, + { + name: "multiple capture groups", + rewriteTarget: "/api/$1/$2/data", + expected: "/api/$1/$2/data", + }, + { + name: "static path", + rewriteTarget: "/static/path", + expected: "/static/path", + }, + { + name: "path with query parameters", + rewriteTarget: "/api/$1?version=v2", + expected: "/api/$1?version=v2", + }, + { + name: "complex pattern", + rewriteTarget: "/v1/services/$1/endpoints/$2", + expected: "/v1/services/$1/endpoints/$2", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ingress := &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ingress", + Namespace: "default", + Annotations: map[string]string{ + "nginx.org/rewrite-target": tt.rewriteTarget, + }, + }, + } + + ingEx := &IngressEx{ + Ingress: ingress, + } + + ctx := context.Background() + value, _ := getRewriteTarget(ctx, ingEx) + + if value != tt.expected { + t.Errorf("Test %q: expected %q, got %q", tt.name, tt.expected, value) + } + }) + } +} diff --git a/internal/configs/ingress.go b/internal/configs/ingress.go index 4afc56883e..1838498e36 100644 --- a/internal/configs/ingress.go +++ b/internal/configs/ingress.go @@ -257,7 +257,7 @@ func generateNginxCfg(p NginxCfgParams) (version1.IngressNginxConfig, Warnings) ssl := isSSLEnabled(sslServices[path.Backend.Service.Name], cfgParams, p.staticParams) proxySSLName := generateProxySSLName(path.Backend.Service.Name, p.ingEx.Ingress.Namespace) loc := createLocation(pathOrDefault(path.Path), upstreams[upsName], &cfgParams, wsServices[path.Backend.Service.Name], rewrites[path.Backend.Service.Name], - ssl, grpcServices[path.Backend.Service.Name], proxySSLName, path.PathType, path.Backend.Service.Name, rewriteTarget, path.Path) + ssl, grpcServices[path.Backend.Service.Name], proxySSLName, path.PathType, path.Backend.Service.Name, rewriteTarget) if p.isMinion && cfgParams.JWTKey != "" { jwtAuth, redirectLoc, warnings := generateJWTConfig(p.ingEx.Ingress, p.ingEx.SecretRefs, &cfgParams, getNameForRedirectLocation(p.ingEx.Ingress)) @@ -322,7 +322,7 @@ func generateNginxCfg(p NginxCfgParams) (version1.IngressNginxConfig, Warnings) pathtype := networking.PathTypePrefix loc := createLocation(pathOrDefault("/"), upstreams[upsName], &cfgParams, wsServices[p.ingEx.Ingress.Spec.DefaultBackend.Service.Name], rewrites[p.ingEx.Ingress.Spec.DefaultBackend.Service.Name], - ssl, grpcServices[p.ingEx.Ingress.Spec.DefaultBackend.Service.Name], proxySSLName, &pathtype, p.ingEx.Ingress.Spec.DefaultBackend.Service.Name, rewriteTarget, "/") + ssl, grpcServices[p.ingEx.Ingress.Spec.DefaultBackend.Service.Name], proxySSLName, &pathtype, p.ingEx.Ingress.Spec.DefaultBackend.Service.Name, rewriteTarget) locations = append(locations, loc) if cfgParams.HealthCheckEnabled { @@ -489,10 +489,9 @@ func generateIngressPath(path string, pathType *networking.PathType) string { return path } -func createLocation(path string, upstream version1.Upstream, cfg *ConfigParams, websocket bool, rewrite string, ssl bool, grpc bool, proxySSLName string, pathType *networking.PathType, serviceName string, rewriteTarget string, originalPath string) version1.Location { +func createLocation(path string, upstream version1.Upstream, cfg *ConfigParams, websocket bool, rewrite string, ssl bool, grpc bool, proxySSLName string, pathType *networking.PathType, serviceName string, rewriteTarget string) version1.Location { loc := version1.Location{ Path: generateIngressPath(path, pathType), - OriginalPath: originalPath, Upstream: upstream, ProxyConnectTimeout: cfg.ProxyConnectTimeout, ProxyReadTimeout: cfg.ProxyReadTimeout, diff --git a/internal/configs/version1/__snapshots__/template_test.snap b/internal/configs/version1/__snapshots__/template_test.snap index 20e332277b..2b202b1095 100644 --- a/internal/configs/version1/__snapshots__/template_test.snap +++ b/internal/configs/version1/__snapshots__/template_test.snap @@ -908,6 +908,287 @@ server { --- +[TestExecuteTemplate_ForIngressForNGINXPlusRewriteTarget/case_insensitive_regex_rewrite - 1] +# configuration for default/cafe-ingress + + + + +server { + + server_tokens "off"; + + server_name cafe.example.com; + + status_zone ; + set $resource_type "ingress"; + set $resource_name "cafe-ingress"; + set $resource_namespace "default"; + + + + + location ~* "^~* "^/(latte|espresso)"" { + set $service ""; + status_zone ""; + rewrite (?i)^/(latte|espresso) /drinks/$1 break; + proxy_http_version 1.1; + + proxy_connect_timeout ; + proxy_read_timeout ; + proxy_send_timeout ; + client_max_body_size ; + + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Host $host; + proxy_set_header X-Forwarded-Port $server_port; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_buffering off; + proxy_pass http://test; + + + } + +} + +--- + +[TestExecuteTemplate_ForIngressForNGINXPlusRewriteTarget/case_sensitive_regex_rewrite - 1] +# configuration for default/cafe-ingress + + + + +server { + + server_tokens "off"; + + server_name cafe.example.com; + + status_zone ; + set $resource_type "ingress"; + set $resource_name "cafe-ingress"; + set $resource_namespace "default"; + + + + + location ~ "^~ "^/(coffee|tea)"" { + set $service ""; + status_zone ""; + rewrite ^/(coffee|tea) /beverages/$1 break; + proxy_http_version 1.1; + + proxy_connect_timeout ; + proxy_read_timeout ; + proxy_send_timeout ; + client_max_body_size ; + + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Host $host; + proxy_set_header X-Forwarded-Port $server_port; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_buffering off; + proxy_pass http://test; + + + } + +} + +--- + +[TestExecuteTemplate_ForIngressForNGINXPlusRewriteTarget/complex_regex_pattern - 1] +# configuration for default/cafe-menu-ingress + + + + +server { + + server_tokens "off"; + + server_name cafe.example.com; + + status_zone ; + set $resource_type "ingress"; + set $resource_name "cafe-menu-ingress"; + set $resource_namespace "default"; + + + + + location ~ "^~ "^/menu/(hot|cold)/(coffee|tea)"" { + set $service ""; + status_zone ""; + rewrite ^/menu/(hot|cold)/(coffee|tea) /drinks/$1/$2 break; + proxy_http_version 1.1; + + proxy_connect_timeout ; + proxy_read_timeout ; + proxy_send_timeout ; + client_max_body_size ; + + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Host $host; + proxy_set_header X-Forwarded-Port $server_port; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_buffering off; + proxy_pass http://test; + + + } + +} + +--- + +[TestExecuteTemplate_ForIngressForNGINXPlusRewriteTarget/exact_match_rewrite - 1] +# configuration for default/cafe-ingress + + + + +server { + + server_tokens "off"; + + server_name cafe.example.com; + + status_zone ; + set $resource_type "ingress"; + set $resource_name "cafe-ingress"; + set $resource_namespace "default"; + + + + + location = "= "/cappuccino"" { + set $service ""; + status_zone ""; + rewrite /cappuccino /special/cappuccino break; + proxy_http_version 1.1; + + proxy_connect_timeout ; + proxy_read_timeout ; + proxy_send_timeout ; + client_max_body_size ; + + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Host $host; + proxy_set_header X-Forwarded-Port $server_port; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_buffering off; + proxy_pass http://test; + + + } + +} + +--- + +[TestExecuteTemplate_ForIngressForNGINXPlusRewriteTarget/no_path_regex_annotation - 1] +# configuration for default/cafe-ingress + + + + +server { + + server_tokens "off"; + + server_name cafe.example.com; + + status_zone ; + set $resource_type "ingress"; + set $resource_name "cafe-ingress"; + set $resource_namespace "default"; + + + + + location /mocha { + set $service ""; + status_zone ""; + rewrite /mocha /hot-drinks/mocha break; + proxy_http_version 1.1; + + proxy_connect_timeout ; + proxy_read_timeout ; + proxy_send_timeout ; + client_max_body_size ; + + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Host $host; + proxy_set_header X-Forwarded-Port $server_port; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_buffering off; + proxy_pass http://test; + + + } + +} + +--- + +[TestExecuteTemplate_ForIngressForNGINXPlusRewriteTarget/no_rewrite_target - 1] +# configuration for default/cafe-ingress + + + + +server { + + server_tokens "off"; + + server_name cafe.example.com; + + status_zone ; + set $resource_type "ingress"; + set $resource_name "cafe-ingress"; + set $resource_namespace "default"; + + + + + location ~ "^~ "^/americano"" { + set $service ""; + status_zone ""; + proxy_http_version 1.1; + + proxy_connect_timeout ; + proxy_read_timeout ; + proxy_send_timeout ; + client_max_body_size ; + + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Host $host; + proxy_set_header X-Forwarded-Port $server_port; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_buffering off; + proxy_pass http://test; + + + } + +} + +--- + [TestExecuteTemplate_ForIngressForNGINXPlusWithHTTP2Off - 1] # configuration for default/cafe-ingress upstream test { @@ -1787,6 +2068,227 @@ server { --- +[TestExecuteTemplate_ForIngressForNGINXRewriteTarget/case_insensitive_regex_rewrite - 1] +# configuration for default/cafe-ingress + + +server { + + server_tokens off; + + server_name cafe.example.com; + + set $resource_type "ingress"; + set $resource_name "cafe-ingress"; + set $resource_namespace "default"; + location ~* "^~* "^/(latte|espresso)"" { + set $service ""; + rewrite (?i)^/(latte|espresso) /drinks/$1 break; + proxy_http_version 1.1; + proxy_connect_timeout ; + proxy_read_timeout ; + proxy_send_timeout ; + client_max_body_size ; + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Host $host; + proxy_set_header X-Forwarded-Port $server_port; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_buffering off; + proxy_pass http://test; + + + } + +} + +--- + +[TestExecuteTemplate_ForIngressForNGINXRewriteTarget/case_sensitive_regex_rewrite - 1] +# configuration for default/cafe-ingress + + +server { + + server_tokens off; + + server_name cafe.example.com; + + set $resource_type "ingress"; + set $resource_name "cafe-ingress"; + set $resource_namespace "default"; + location ~ "^~ "^/(coffee|tea)"" { + set $service ""; + rewrite ^/(coffee|tea) /beverages/$1 break; + proxy_http_version 1.1; + proxy_connect_timeout ; + proxy_read_timeout ; + proxy_send_timeout ; + client_max_body_size ; + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Host $host; + proxy_set_header X-Forwarded-Port $server_port; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_buffering off; + proxy_pass http://test; + + + } + +} + +--- + +[TestExecuteTemplate_ForIngressForNGINXRewriteTarget/complex_regex_pattern - 1] +# configuration for default/cafe-menu-ingress + + +server { + + server_tokens off; + + server_name cafe.example.com; + + set $resource_type "ingress"; + set $resource_name "cafe-menu-ingress"; + set $resource_namespace "default"; + location ~ "^~ "^/menu/(hot|cold)/(coffee|tea)"" { + set $service ""; + rewrite ^/menu/(hot|cold)/(coffee|tea) /drinks/$1/$2 break; + proxy_http_version 1.1; + proxy_connect_timeout ; + proxy_read_timeout ; + proxy_send_timeout ; + client_max_body_size ; + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Host $host; + proxy_set_header X-Forwarded-Port $server_port; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_buffering off; + proxy_pass http://test; + + + } + +} + +--- + +[TestExecuteTemplate_ForIngressForNGINXRewriteTarget/exact_match_rewrite - 1] +# configuration for default/cafe-ingress + + +server { + + server_tokens off; + + server_name cafe.example.com; + + set $resource_type "ingress"; + set $resource_name "cafe-ingress"; + set $resource_namespace "default"; + location = "= "/cappuccino"" { + set $service ""; + rewrite /cappuccino /special/cappuccino break; + proxy_http_version 1.1; + proxy_connect_timeout ; + proxy_read_timeout ; + proxy_send_timeout ; + client_max_body_size ; + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Host $host; + proxy_set_header X-Forwarded-Port $server_port; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_buffering off; + proxy_pass http://test; + + + } + +} + +--- + +[TestExecuteTemplate_ForIngressForNGINXRewriteTarget/no_path_regex_annotation - 1] +# configuration for default/cafe-ingress + + +server { + + server_tokens off; + + server_name cafe.example.com; + + set $resource_type "ingress"; + set $resource_name "cafe-ingress"; + set $resource_namespace "default"; + location /mocha { + set $service ""; + rewrite /mocha /hot-drinks/mocha break; + proxy_http_version 1.1; + proxy_connect_timeout ; + proxy_read_timeout ; + proxy_send_timeout ; + client_max_body_size ; + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Host $host; + proxy_set_header X-Forwarded-Port $server_port; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_buffering off; + proxy_pass http://test; + + + } + +} + +--- + +[TestExecuteTemplate_ForIngressForNGINXRewriteTarget/no_rewrite_target - 1] +# configuration for default/cafe-ingress + + +server { + + server_tokens off; + + server_name cafe.example.com; + + set $resource_type "ingress"; + set $resource_name "cafe-ingress"; + set $resource_namespace "default"; + location ~ "^~ "^/americano"" { + set $service ""; + proxy_http_version 1.1; + proxy_connect_timeout ; + proxy_read_timeout ; + proxy_send_timeout ; + client_max_body_size ; + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Host $host; + proxy_set_header X-Forwarded-Port $server_port; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_buffering off; + proxy_pass http://test; + + + } + +} + +--- + [TestExecuteTemplate_ForIngressForNGINXWithHTTP2Off - 1] # configuration for default/cafe-ingress upstream test {zone test 256k; diff --git a/internal/configs/version1/config.go b/internal/configs/version1/config.go index 7961017010..b4a6b26283 100644 --- a/internal/configs/version1/config.go +++ b/internal/configs/version1/config.go @@ -169,7 +169,6 @@ type LimitReq struct { type Location struct { LocationSnippets []string Path string - OriginalPath string Upstream Upstream ProxyConnectTimeout string ProxyReadTimeout string diff --git a/internal/configs/version1/template_helper.go b/internal/configs/version1/template_helper.go index e25c3db672..a622e5bcca 100644 --- a/internal/configs/version1/template_helper.go +++ b/internal/configs/version1/template_helper.go @@ -213,7 +213,7 @@ func makeRewritePattern(loc *Location, ingressAnnotations map[string]string) str if loc.MinionIngress != nil { ingressType, isMergeable := loc.MinionIngress.Annotations["nginx.org/mergeable-ingress-type"] regexType, hasRegex = loc.MinionIngress.Annotations["nginx.org/path-regex"] - if !(isMergeable && ingressType == "minion" && hasRegex) { + if !isMergeable || ingressType != "minion" || !hasRegex { hasRegex = false } } @@ -222,22 +222,54 @@ func makeRewritePattern(loc *Location, ingressAnnotations map[string]string) str regexType, hasRegex = ingressAnnotations["nginx.org/path-regex"] } + // Extract original path from the processed Path field + originalPath := extractOriginalPath(loc.Path) + // If no path-regex annotation, return original path if !hasRegex { - return loc.OriginalPath + return originalPath } // Generate rewrite pattern based on regex type switch regexType { case "case_sensitive": - return fmt.Sprintf("^%s", loc.OriginalPath) + return fmt.Sprintf("^%s", originalPath) case "case_insensitive": - return fmt.Sprintf("(?i)^%s", loc.OriginalPath) + return fmt.Sprintf("(?i)^%s", originalPath) case "exact": - return loc.OriginalPath // exact matches don't need anchors in rewrite + return originalPath // exact matches don't need anchors in rewrite default: - return loc.OriginalPath + return originalPath + } +} + +// extractOriginalPath extracts the original path from a processed nginx location path +func extractOriginalPath(processedPath string) string { + // Handle different nginx location formats: + // ~ "^/path" -> /path + // ~* "^/path" -> /path + // = "/path" -> /path + // /path -> /path + + processedPath = strings.TrimSpace(processedPath) + + // Case-sensitive regex: ~ "^/path" + if strings.HasPrefix(processedPath, "~ \"^") && strings.HasSuffix(processedPath, "\"") { + return processedPath[4 : len(processedPath)-1] // Remove ~ "^ and " } + + // Case-insensitive regex: ~* "^/path" + if strings.HasPrefix(processedPath, "~* \"^") && strings.HasSuffix(processedPath, "\"") { + return processedPath[5 : len(processedPath)-1] // Remove ~* "^ and " + } + + // Exact match: = "/path" + if strings.HasPrefix(processedPath, "= \"") && strings.HasSuffix(processedPath, "\"") { + return processedPath[3 : len(processedPath)-1] // Remove = " and " + } + + // Plain path: /path (no quotes) + return processedPath } var helperFunctions = template.FuncMap{ diff --git a/internal/configs/version1/template_helper_test.go b/internal/configs/version1/template_helper_test.go index 69f04a0c69..a42f083353 100644 --- a/internal/configs/version1/template_helper_test.go +++ b/internal/configs/version1/template_helper_test.go @@ -926,3 +926,152 @@ func TestMakeResolver(t *testing.T) { }) } } + +func TestMakeRewritePattern_WithRegexCaseSensitiveModifier(t *testing.T) { + t.Parallel() + + want := "^/(hello|hi)" + got := makeRewritePattern( + &Location{Path: "/(hello|hi)"}, + map[string]string{"nginx.org/path-regex": "case_sensitive"}, + ) + if got != want { + t.Errorf("makeRewritePattern() = %q; want %q", got, want) + } +} + +func TestMakeRewritePattern_WithRegexCaseInsensitiveModifier(t *testing.T) { + t.Parallel() + + want := "(?i)^/(hello|hi)" + got := makeRewritePattern( + &Location{Path: "/(hello|hi)"}, + map[string]string{"nginx.org/path-regex": "case_insensitive"}, + ) + if got != want { + t.Errorf("makeRewritePattern() = %q; want %q", got, want) + } +} + +func TestMakeRewritePattern_WithRegexExactModifier(t *testing.T) { + t.Parallel() + + want := "/coffee" + got := makeRewritePattern( + &Location{Path: "/coffee"}, + map[string]string{"nginx.org/path-regex": "exact"}, + ) + if got != want { + t.Errorf("makeRewritePattern() = %q; want %q", got, want) + } +} + +func TestMakeRewritePattern_WithBogusRegexModifier(t *testing.T) { + t.Parallel() + + want := "/(hello|hi)" + got := makeRewritePattern( + &Location{Path: "/(hello|hi)"}, + map[string]string{"nginx.org/path-regex": "bogus"}, + ) + if got != want { + t.Errorf("makeRewritePattern() = %q; want %q", got, want) + } +} + +func TestMakeRewritePattern_WithoutRegexModifier(t *testing.T) { + t.Parallel() + + want := "/coffee" + got := makeRewritePattern( + &Location{Path: "/coffee"}, + map[string]string{}, + ) + if got != want { + t.Errorf("makeRewritePattern() = %q; want %q", got, want) + } +} + +func TestMakeRewritePattern_WithMergableIngress(t *testing.T) { + t.Parallel() + + // Test with minion ingress having path-regex annotation + want := "^/coffee/[A-Z0-9]{3}" + got := makeRewritePattern( + &Location{ + Path: "/coffee/[A-Z0-9]{3}", + MinionIngress: &Ingress{ + Annotations: map[string]string{ + "nginx.org/mergeable-ingress-type": "minion", + "nginx.org/path-regex": "case_sensitive", + }, + }, + }, + map[string]string{"nginx.org/path-regex": "case_insensitive"}, // Should be ignored + ) + if got != want { + t.Errorf("makeRewritePattern() = %q; want %q", got, want) + } +} + +func TestMakeRewritePattern_WithComplexPatterns(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + path string + pathRegex string + expected string + }{ + { + name: "Simple path with case sensitive regex", + path: "/api/(v1|v2)", + pathRegex: "case_sensitive", + expected: "^/api/(v1|v2)", + }, + { + name: "Complex regex pattern with case insensitive", + path: "/user/([0-9]+)/(profile|settings)", + pathRegex: "case_insensitive", + expected: "(?i)^/user/([0-9]+)/(profile|settings)", + }, + { + name: "Exact match pattern", + path: "/health", + pathRegex: "exact", + expected: "/health", + }, + { + name: "Pattern with special characters", + path: "/api/v1/([a-zA-Z0-9_-]+)/data", + pathRegex: "case_sensitive", + expected: "^/api/v1/([a-zA-Z0-9_-]+)/data", + }, + { + name: "Path with no regex annotation", + path: "/static/assets", + pathRegex: "", + expected: "/static/assets", + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + annotations := map[string]string{} + if tt.pathRegex != "" { + annotations["nginx.org/path-regex"] = tt.pathRegex + } + + got := makeRewritePattern( + &Location{Path: tt.path}, + annotations, + ) + if got != tt.expected { + t.Errorf("Test %q: makeRewritePattern() = %q; want %q", tt.name, got, tt.expected) + } + }) + } +} diff --git a/internal/configs/version1/template_test.go b/internal/configs/version1/template_test.go index 4a4ed277f0..f1774171a5 100644 --- a/internal/configs/version1/template_test.go +++ b/internal/configs/version1/template_test.go @@ -3686,6 +3686,478 @@ func TestExecuteTemplate_ForIngressForNGINXWithSSLCiphersDisabled(t *testing.T) snaps.MatchSnapshot(t, buf.String()) } +func TestExecuteTemplate_ForIngressForNGINXRewriteTarget(t *testing.T) { + t.Parallel() + + tmpl := newNGINXIngressTmpl(t) + + tests := []struct { + name string + ingressCfg IngressNginxConfig + description string + wantDirectives []string + unwantDirectives []string + }{ + { + name: "case_sensitive_regex_rewrite", + ingressCfg: IngressNginxConfig{ + Servers: []Server{ + { + Name: "cafe.example.com", + ServerTokens: "off", + Locations: []Location{ + { + Path: "~ \"^/(coffee|tea)\"", + RewriteTarget: "/beverages/$1", + Upstream: testUpstream, + }, + }, + }, + }, + Ingress: Ingress{ + Name: "cafe-ingress", + Namespace: "default", + Annotations: map[string]string{ + "nginx.org/path-regex": "case_sensitive", + }, + }, + }, + description: "Should generate rewrite directive with case-sensitive anchored regex pattern", + wantDirectives: []string{ + "rewrite ^/(coffee|tea) /beverages/$1 break;", + }, + unwantDirectives: []string{ + "rewrite (?i)^/(coffee|tea)", + }, + }, + { + name: "case_insensitive_regex_rewrite", + ingressCfg: IngressNginxConfig{ + Servers: []Server{ + { + Name: "cafe.example.com", + ServerTokens: "off", + Locations: []Location{ + { + Path: "~* \"^/(latte|espresso)\"", + RewriteTarget: "/drinks/$1", + Upstream: testUpstream, + }, + }, + }, + }, + Ingress: Ingress{ + Name: "cafe-ingress", + Namespace: "default", + Annotations: map[string]string{ + "nginx.org/path-regex": "case_insensitive", + }, + }, + }, + description: "Should generate rewrite directive with case-insensitive anchored regex pattern", + wantDirectives: []string{ + "rewrite (?i)^/(latte|espresso) /drinks/$1 break;", + }, + unwantDirectives: []string{ + "rewrite ^/(latte|espresso)", + }, + }, + { + name: "exact_match_rewrite", + ingressCfg: IngressNginxConfig{ + Servers: []Server{ + { + Name: "cafe.example.com", + ServerTokens: "off", + Locations: []Location{ + { + Path: "= \"/cappuccino\"", + RewriteTarget: "/special/cappuccino", + Upstream: testUpstream, + }, + }, + }, + }, + Ingress: Ingress{ + Name: "cafe-ingress", + Namespace: "default", + Annotations: map[string]string{ + "nginx.org/path-regex": "exact", + }, + }, + }, + description: "Should generate rewrite directive with exact path pattern (no anchors)", + wantDirectives: []string{ + "rewrite /cappuccino /special/cappuccino break;", + }, + unwantDirectives: []string{ + "rewrite ^/cappuccino", + "rewrite (?i)/cappuccino", + }, + }, + { + name: "no_path_regex_annotation", + ingressCfg: IngressNginxConfig{ + Servers: []Server{ + { + Name: "cafe.example.com", + ServerTokens: "off", + Locations: []Location{ + { + Path: "/mocha", + RewriteTarget: "/hot-drinks/mocha", + Upstream: testUpstream, + }, + }, + }, + }, + Ingress: Ingress{ + Name: "cafe-ingress", + Namespace: "default", + }, + }, + description: "Should generate rewrite directive with original path when no path-regex annotation", + wantDirectives: []string{ + "rewrite /mocha /hot-drinks/mocha break;", + }, + unwantDirectives: []string{ + "rewrite ^/mocha", + "rewrite (?i)/mocha", + }, + }, + { + name: "no_rewrite_target", + ingressCfg: IngressNginxConfig{ + Servers: []Server{ + { + Name: "cafe.example.com", + ServerTokens: "off", + Locations: []Location{ + { + Path: "~ \"^/americano\"", + Upstream: testUpstream, + // RewriteTarget is empty - should not generate rewrite directive + }, + }, + }, + }, + Ingress: Ingress{ + Name: "cafe-ingress", + Namespace: "default", + Annotations: map[string]string{ + "nginx.org/path-regex": "case_sensitive", + }, + }, + }, + description: "Should not generate rewrite directive when RewriteTarget is empty", + wantDirectives: []string{}, + unwantDirectives: []string{ + "rewrite", + }, + }, + { + name: "complex_regex_pattern", + ingressCfg: IngressNginxConfig{ + Servers: []Server{ + { + Name: "cafe.example.com", + ServerTokens: "off", + Locations: []Location{ + { + Path: "~ \"^/menu/(hot|cold)/(coffee|tea)\"", + RewriteTarget: "/drinks/$1/$2", + Upstream: testUpstream, + }, + }, + }, + }, + Ingress: Ingress{ + Name: "cafe-menu-ingress", + Namespace: "default", + Annotations: map[string]string{ + "nginx.org/path-regex": "case_sensitive", + }, + }, + }, + description: "Should handle complex regex patterns with multiple capture groups", + wantDirectives: []string{ + "rewrite ^/menu/(hot|cold)/(coffee|tea) /drinks/$1/$2 break;", + }, + unwantDirectives: []string{ + "rewrite (?i)^/menu/", + }, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + buf := &bytes.Buffer{} + err := tmpl.Execute(buf, tt.ingressCfg) + if err != nil { + t.Fatalf("Failed to execute template: %v", err) + } + + ingConf := buf.String() + + // Check for expected directives + for _, want := range tt.wantDirectives { + if !strings.Contains(ingConf, want) { + t.Errorf("want %q in generated config", want) + } + } + + // Check for unwanted directives + for _, unwant := range tt.unwantDirectives { + if strings.Contains(ingConf, unwant) { + t.Errorf("unwant %q in generated config", unwant) + } + } + + // Use snapshot testing for consistent comparison + snaps.MatchSnapshot(t, buf.String()) + }) + } +} + +func TestExecuteTemplate_ForIngressForNGINXPlusRewriteTarget(t *testing.T) { + t.Parallel() + + tmpl := newNGINXPlusIngressTmpl(t) + + tests := []struct { + name string + ingressCfg IngressNginxConfig + description string + wantDirectives []string + unwantDirectives []string + }{ + { + name: "case_sensitive_regex_rewrite", + ingressCfg: IngressNginxConfig{ + Servers: []Server{ + { + Name: "cafe.example.com", + ServerTokens: "off", + Locations: []Location{ + { + Path: "~ \"^/(coffee|tea)\"", + RewriteTarget: "/beverages/$1", + Upstream: testUpstream, + }, + }, + }, + }, + Ingress: Ingress{ + Name: "cafe-ingress", + Namespace: "default", + Annotations: map[string]string{ + "nginx.org/path-regex": "case_sensitive", + }, + }, + }, + description: "Should generate rewrite directive with case-sensitive anchored regex pattern", + wantDirectives: []string{ + "rewrite ^/(coffee|tea) /beverages/$1 break;", + }, + unwantDirectives: []string{ + "rewrite (?i)^/(coffee|tea)", + }, + }, + { + name: "case_insensitive_regex_rewrite", + ingressCfg: IngressNginxConfig{ + Servers: []Server{ + { + Name: "cafe.example.com", + ServerTokens: "off", + Locations: []Location{ + { + Path: "~* \"^/(latte|espresso)\"", + RewriteTarget: "/drinks/$1", + Upstream: testUpstream, + }, + }, + }, + }, + Ingress: Ingress{ + Name: "cafe-ingress", + Namespace: "default", + Annotations: map[string]string{ + "nginx.org/path-regex": "case_insensitive", + }, + }, + }, + description: "Should generate rewrite directive with case-insensitive anchored regex pattern", + wantDirectives: []string{ + "rewrite (?i)^/(latte|espresso) /drinks/$1 break;", + }, + unwantDirectives: []string{ + "rewrite ^/(latte|espresso)", + }, + }, + { + name: "exact_match_rewrite", + ingressCfg: IngressNginxConfig{ + Servers: []Server{ + { + Name: "cafe.example.com", + ServerTokens: "off", + Locations: []Location{ + { + Path: "= \"/cappuccino\"", + RewriteTarget: "/special/cappuccino", + Upstream: testUpstream, + }, + }, + }, + }, + Ingress: Ingress{ + Name: "cafe-ingress", + Namespace: "default", + Annotations: map[string]string{ + "nginx.org/path-regex": "exact", + }, + }, + }, + description: "Should generate rewrite directive with exact path pattern (no anchors)", + wantDirectives: []string{ + "rewrite /cappuccino /special/cappuccino break;", + }, + unwantDirectives: []string{ + "rewrite ^/cappuccino", + "rewrite (?i)/cappuccino", + }, + }, + { + name: "no_path_regex_annotation", + ingressCfg: IngressNginxConfig{ + Servers: []Server{ + { + Name: "cafe.example.com", + ServerTokens: "off", + Locations: []Location{ + { + Path: "/mocha", + RewriteTarget: "/hot-drinks/mocha", + Upstream: testUpstream, + }, + }, + }, + }, + Ingress: Ingress{ + Name: "cafe-ingress", + Namespace: "default", + }, + }, + description: "Should generate rewrite directive with original path when no path-regex annotation", + wantDirectives: []string{ + "rewrite /mocha /hot-drinks/mocha break;", + }, + unwantDirectives: []string{ + "rewrite ^/mocha", + "rewrite (?i)/mocha", + }, + }, + { + name: "no_rewrite_target", + ingressCfg: IngressNginxConfig{ + Servers: []Server{ + { + Name: "cafe.example.com", + ServerTokens: "off", + Locations: []Location{ + { + Path: "~ \"^/americano\"", + Upstream: testUpstream, + // RewriteTarget is empty - should not generate rewrite directive + }, + }, + }, + }, + Ingress: Ingress{ + Name: "cafe-ingress", + Namespace: "default", + Annotations: map[string]string{ + "nginx.org/path-regex": "case_sensitive", + }, + }, + }, + description: "Should not generate rewrite directive when RewriteTarget is empty", + wantDirectives: []string{}, + unwantDirectives: []string{ + "rewrite", + }, + }, + { + name: "complex_regex_pattern", + ingressCfg: IngressNginxConfig{ + Servers: []Server{ + { + Name: "cafe.example.com", + ServerTokens: "off", + Locations: []Location{ + { + Path: "~ \"^/menu/(hot|cold)/(coffee|tea)\"", + RewriteTarget: "/drinks/$1/$2", + Upstream: testUpstream, + }, + }, + }, + }, + Ingress: Ingress{ + Name: "cafe-menu-ingress", + Namespace: "default", + Annotations: map[string]string{ + "nginx.org/path-regex": "case_sensitive", + }, + }, + }, + description: "Should handle complex regex patterns with multiple capture groups", + wantDirectives: []string{ + "rewrite ^/menu/(hot|cold)/(coffee|tea) /drinks/$1/$2 break;", + }, + unwantDirectives: []string{ + "rewrite (?i)^/menu/", + }, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + buf := &bytes.Buffer{} + err := tmpl.Execute(buf, tt.ingressCfg) + if err != nil { + t.Fatalf("Failed to execute template: %v", err) + } + + ingConf := buf.String() + + // Check for expected directives + for _, want := range tt.wantDirectives { + if !strings.Contains(ingConf, want) { + t.Errorf("want %q in generated config", want) + } + } + + // Check for unwanted directives + for _, unwant := range tt.unwantDirectives { + if strings.Contains(ingConf, unwant) { + t.Errorf("unwant %q in generated config", unwant) + } + } + + // Use snapshot testing for consistent comparison + snaps.MatchSnapshot(t, buf.String()) + }) + } +} + func TestExecuteTemplate_ForIngressForNGINXPlusWithSSLCiphersDisabled(t *testing.T) { t.Parallel() diff --git a/internal/k8s/validation.go b/internal/k8s/validation.go index 5b5d6169a1..a6669932a7 100644 --- a/internal/k8s/validation.go +++ b/internal/k8s/validation.go @@ -69,6 +69,7 @@ const ( sslServicesAnnotation = "nginx.org/ssl-services" grpcServicesAnnotation = "nginx.org/grpc-services" rewritesAnnotation = "nginx.org/rewrites" + rewriteTargetAnnotation = "nginx.org/rewrite-target" stickyCookieServicesAnnotation = "nginx.com/sticky-cookie-services" pathRegexAnnotation = "nginx.org/path-regex" useClusterIPAnnotation = "nginx.org/use-cluster-ip" @@ -334,6 +335,10 @@ var ( validateRequiredAnnotation, validateRewriteListAnnotation, }, + rewriteTargetAnnotation: { + validateRequiredAnnotation, + validateRewriteTargetAnnotation, + }, stickyCookieServicesAnnotation: { validatePlusOnlyAnnotation, validateRequiredAnnotation, @@ -771,6 +776,32 @@ func validateRewriteListAnnotation(context *annotationValidationContext) field.E return nil } +func validateRewriteTargetAnnotation(context *annotationValidationContext) field.ErrorList { + target := context.value + + // Prevent absolute URLs (http://, https://) + if strings.HasPrefix(target, "http://") || strings.HasPrefix(target, "https://") { + return field.ErrorList{field.Invalid(context.fieldPath, target, "absolute URLs not allowed in rewrite target")} + } + + // Prevent protocol-relative URLs (//) + if strings.HasPrefix(target, "//") { + return field.ErrorList{field.Invalid(context.fieldPath, target, "protocol-relative URLs not allowed in rewrite target")} + } + + // Prevent path traversal patterns + if strings.Contains(target, "../") || strings.Contains(target, "..\\") { + return field.ErrorList{field.Invalid(context.fieldPath, target, "path traversal patterns not allowed in rewrite target")} + } + + // Must start with / (relative path) + if !strings.HasPrefix(target, "/") { + return field.ErrorList{field.Invalid(context.fieldPath, target, "rewrite target must start with /")} + } + + return nil +} + func validateAppProtectSecurityLogAnnotation(context *annotationValidationContext) field.ErrorList { allErrs := field.ErrorList{} logConf := strings.Split(context.value, ",") diff --git a/internal/k8s/validation_test.go b/internal/k8s/validation_test.go index 6e170180b8..2e13d34435 100644 --- a/internal/k8s/validation_test.go +++ b/internal/k8s/validation_test.go @@ -3244,6 +3244,142 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { expectedErrors: nil, msg: "valid nginx.org/use-cluster-ip annotation", }, + + // nginx.org/rewrite-target annotation tests + { + annotations: map[string]string{ + "nginx.org/rewrite-target": "/api/v1/$1", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/rewrite-target annotation", + }, + { + annotations: map[string]string{ + "nginx.org/rewrite-target": "/newpath", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/rewrite-target annotation, simple path", + }, + { + annotations: map[string]string{ + "nginx.org/rewrite-target": "/api/$1/$2/data", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/rewrite-target annotation, multiple capture groups", + }, + { + annotations: map[string]string{ + "nginx.org/rewrite-target": "", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.org/rewrite-target: Required value`, + }, + msg: "invalid nginx.org/rewrite-target annotation, empty value", + }, + { + annotations: map[string]string{ + "nginx.org/rewrite-target": "http://example.com/path", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.org/rewrite-target: Invalid value: "http://example.com/path": absolute URLs not allowed in rewrite target`, + }, + msg: "invalid nginx.org/rewrite-target annotation, absolute HTTP URL", + }, + { + annotations: map[string]string{ + "nginx.org/rewrite-target": "https://example.com/path", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.org/rewrite-target: Invalid value: "https://example.com/path": absolute URLs not allowed in rewrite target`, + }, + msg: "invalid nginx.org/rewrite-target annotation, absolute HTTPS URL", + }, + { + annotations: map[string]string{ + "nginx.org/rewrite-target": "//example.com/path", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.org/rewrite-target: Invalid value: "//example.com/path": protocol-relative URLs not allowed in rewrite target`, + }, + msg: "invalid nginx.org/rewrite-target annotation, protocol-relative URL", + }, + { + annotations: map[string]string{ + "nginx.org/rewrite-target": "/api/../admin/users", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.org/rewrite-target: Invalid value: "/api/../admin/users": path traversal patterns not allowed in rewrite target`, + }, + msg: "invalid nginx.org/rewrite-target annotation, path traversal with ../", + }, + { + annotations: map[string]string{ + "nginx.org/rewrite-target": "/api/..\\admin/users", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.org/rewrite-target: Invalid value: "/api/..\\admin/users": path traversal patterns not allowed in rewrite target`, + }, + msg: "invalid nginx.org/rewrite-target annotation, path traversal with ..\\ (Windows style)", + }, + { + annotations: map[string]string{ + "nginx.org/rewrite-target": "api/users", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.org/rewrite-target: Invalid value: "api/users": rewrite target must start with /`, + }, + msg: "invalid nginx.org/rewrite-target annotation, does not start with slash", + }, } for _, test := range tests { From 9d05e0d126ffdc78f57ef5d342f512086867fcea Mon Sep 17 00:00:00 2001 From: Venktesh Shivam Patel Date: Fri, 7 Nov 2025 12:45:16 +0000 Subject: [PATCH 3/7] add python tests --- tests/data/rewrite-target/rewrite-regex.yaml | 20 ++++ tests/data/rewrite-target/rewrite-static.yaml | 19 +++ tests/suite/test_rewrite_target.py | 108 ++++++++++++++++++ 3 files changed, 147 insertions(+) create mode 100644 tests/data/rewrite-target/rewrite-regex.yaml create mode 100644 tests/data/rewrite-target/rewrite-static.yaml create mode 100644 tests/suite/test_rewrite_target.py diff --git a/tests/data/rewrite-target/rewrite-regex.yaml b/tests/data/rewrite-target/rewrite-regex.yaml new file mode 100644 index 0000000000..90a2de8b2f --- /dev/null +++ b/tests/data/rewrite-target/rewrite-regex.yaml @@ -0,0 +1,20 @@ +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: rewrite-regex-ingress + annotations: + nginx.org/rewrite-target: "/api/$1/$2" + nginx.org/path-regex: "case_sensitive" +spec: + ingressClassName: nginx + rules: + - host: rewrite.example.com + http: + paths: + - path: /v1/([^/]+)/([^/]+) + pathType: ImplementationSpecific + backend: + service: + name: backend1-svc + port: + number: 80 \ No newline at end of file diff --git a/tests/data/rewrite-target/rewrite-static.yaml b/tests/data/rewrite-target/rewrite-static.yaml new file mode 100644 index 0000000000..dd9b5b25ad --- /dev/null +++ b/tests/data/rewrite-target/rewrite-static.yaml @@ -0,0 +1,19 @@ +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: rewrite-static-ingress + annotations: + nginx.org/rewrite-target: "/backend" +spec: + ingressClassName: nginx + rules: + - host: rewrite.example.com + http: + paths: + - path: /app + pathType: Prefix + backend: + service: + name: backend1-svc + port: + number: 80 \ No newline at end of file diff --git a/tests/suite/test_rewrite_target.py b/tests/suite/test_rewrite_target.py new file mode 100644 index 0000000000..8b0f672110 --- /dev/null +++ b/tests/suite/test_rewrite_target.py @@ -0,0 +1,108 @@ +import pytest +import requests +from settings import TEST_DATA +from suite.fixtures.fixtures import PublicEndpoint +from suite.utils.resources_utils import ( + create_example_app, + create_items_from_yaml, + delete_common_app, + delete_items_from_yaml, + ensure_connection_to_public_endpoint, + wait_before_test, + wait_until_all_pods_are_ready, +) +from suite.utils.yaml_utils import get_first_ingress_host_from_yaml, get_name_from_yaml + + +class RewriteTargetSetup: + """Encapsulate Rewrite Target example details.""" + + def __init__( + self, + public_endpoint: PublicEndpoint, + ingress_src_file, + ingress_name, + ingress_host, + namespace, + request_url, + ): + self.public_endpoint = public_endpoint + self.ingress_name = ingress_name + self.namespace = namespace + self.ingress_host = ingress_host + self.ingress_src_file = ingress_src_file + self.request_url = request_url + + +@pytest.fixture(scope="function") +def rewrite_target_setup( + request, + kube_apis, + ingress_controller_prerequisites, + ingress_controller_endpoint, + ingress_controller, + test_namespace, +) -> RewriteTargetSetup: + print("------------------------- Deploy Ingress with rewrite-target annotations -----------------------------------") + src = f"{TEST_DATA}/rewrite-target/{request.param}.yaml" + create_items_from_yaml(kube_apis, src, test_namespace) + ingress_name = get_name_from_yaml(src) + ingress_host = get_first_ingress_host_from_yaml(src) + request_url = f"http://{ingress_controller_endpoint.public_ip}:{ingress_controller_endpoint.port}/backend1" + + create_example_app(kube_apis, "simple", test_namespace) + wait_until_all_pods_are_ready(kube_apis.v1, test_namespace) + + ensure_connection_to_public_endpoint( + ingress_controller_endpoint.public_ip, ingress_controller_endpoint.port, ingress_controller_endpoint.port_ssl + ) + + def fin(): + if request.config.getoption("--skip-fixture-teardown") == "no": + print("Clean up:") + delete_common_app(kube_apis, "simple", test_namespace) + delete_items_from_yaml(kube_apis, src, test_namespace) + + request.addfinalizer(fin) + + return RewriteTargetSetup( + ingress_controller_endpoint, + src, + ingress_name, + ingress_host, + test_namespace, + request_url, + ) + + +@pytest.mark.annotations +class TestRewriteTarget: + @pytest.mark.parametrize("rewrite_target_setup", ["rewrite-static"], indirect=True) + def test_static_rewrite_target(self, rewrite_target_setup): + """ + Test static rewrite target functionality. + Request to /app should be rewritten to /backend. + """ + request_url = f"http://{rewrite_target_setup.public_endpoint.public_ip}:{rewrite_target_setup.public_endpoint.port}/app" + resp = requests.get( + request_url, + headers={"host": rewrite_target_setup.ingress_host}, + ) + + assert resp.status_code == 200 + assert "URI: /backend" in resp.text + + @pytest.mark.parametrize("rewrite_target_setup", ["rewrite-regex"], indirect=True) + def test_regex_rewrite_target(self, rewrite_target_setup): + """ + Test regex rewrite target functionality with capture groups. + Request to /v1/users/123 should be rewritten to /api/users/123. + """ + request_url = f"http://{rewrite_target_setup.public_endpoint.public_ip}:{rewrite_target_setup.public_endpoint.port}/v1/users/123" + resp = requests.get( + request_url, + headers={"host": rewrite_target_setup.ingress_host}, + ) + + assert resp.status_code == 200 + assert "URI: /api/users/123" in resp.text \ No newline at end of file From f3912ddb3b6740ae85746bf4bb649bba646099e6 Mon Sep 17 00:00:00 2001 From: Venktesh Shivam Patel Date: Fri, 7 Nov 2025 13:02:44 +0000 Subject: [PATCH 4/7] remove unused import from tests Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> Signed-off-by: Venktesh Shivam Patel --- tests/suite/test_rewrite_target.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/suite/test_rewrite_target.py b/tests/suite/test_rewrite_target.py index 8b0f672110..75b6cd1267 100644 --- a/tests/suite/test_rewrite_target.py +++ b/tests/suite/test_rewrite_target.py @@ -8,7 +8,6 @@ delete_common_app, delete_items_from_yaml, ensure_connection_to_public_endpoint, - wait_before_test, wait_until_all_pods_are_ready, ) from suite.utils.yaml_utils import get_first_ingress_host_from_yaml, get_name_from_yaml From aa0f809a2e25b89d8afd41e3fdc191c245dd618d Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 7 Nov 2025 13:05:30 +0000 Subject: [PATCH 5/7] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- .../ingress-resources/rewrite-target/README.md | 1 - .../ingress-resources/rewrite-target/cafe.yaml | 2 +- .../rewrite-target/regex-rewrite.yaml | 2 +- .../rewrite-target/simple-rewrite.yaml | 2 +- tests/data/rewrite-target/rewrite-regex.yaml | 2 +- tests/data/rewrite-target/rewrite-static.yaml | 2 +- tests/suite/test_rewrite_target.py | 14 +++++++++----- 7 files changed, 14 insertions(+), 11 deletions(-) diff --git a/examples/ingress-resources/rewrite-target/README.md b/examples/ingress-resources/rewrite-target/README.md index ed4ae28a38..f73262f2c4 100644 --- a/examples/ingress-resources/rewrite-target/README.md +++ b/examples/ingress-resources/rewrite-target/README.md @@ -109,4 +109,3 @@ The requests to `/menu/coffee/espresso` and `/menu/tea/green` are rewritten to ` - Protocol-relative URLs (`//`) - Path traversal patterns (`../` or `..\\`) - Paths not starting with `/` - diff --git a/examples/ingress-resources/rewrite-target/cafe.yaml b/examples/ingress-resources/rewrite-target/cafe.yaml index 07653b72cd..4127392147 100644 --- a/examples/ingress-resources/rewrite-target/cafe.yaml +++ b/examples/ingress-resources/rewrite-target/cafe.yaml @@ -62,4 +62,4 @@ spec: protocol: TCP name: http selector: - app: tea \ No newline at end of file + app: tea diff --git a/examples/ingress-resources/rewrite-target/regex-rewrite.yaml b/examples/ingress-resources/rewrite-target/regex-rewrite.yaml index 488ee8abb7..63acd465bc 100644 --- a/examples/ingress-resources/rewrite-target/regex-rewrite.yaml +++ b/examples/ingress-resources/rewrite-target/regex-rewrite.yaml @@ -17,4 +17,4 @@ spec: service: name: coffee-svc port: - number: 80 \ No newline at end of file + number: 80 diff --git a/examples/ingress-resources/rewrite-target/simple-rewrite.yaml b/examples/ingress-resources/rewrite-target/simple-rewrite.yaml index 21c7b38561..bf222cbb6c 100644 --- a/examples/ingress-resources/rewrite-target/simple-rewrite.yaml +++ b/examples/ingress-resources/rewrite-target/simple-rewrite.yaml @@ -16,4 +16,4 @@ spec: service: name: coffee-svc port: - number: 80 \ No newline at end of file + number: 80 diff --git a/tests/data/rewrite-target/rewrite-regex.yaml b/tests/data/rewrite-target/rewrite-regex.yaml index 90a2de8b2f..ac5ab4338c 100644 --- a/tests/data/rewrite-target/rewrite-regex.yaml +++ b/tests/data/rewrite-target/rewrite-regex.yaml @@ -17,4 +17,4 @@ spec: service: name: backend1-svc port: - number: 80 \ No newline at end of file + number: 80 diff --git a/tests/data/rewrite-target/rewrite-static.yaml b/tests/data/rewrite-target/rewrite-static.yaml index dd9b5b25ad..4c93c08840 100644 --- a/tests/data/rewrite-target/rewrite-static.yaml +++ b/tests/data/rewrite-target/rewrite-static.yaml @@ -16,4 +16,4 @@ spec: service: name: backend1-svc port: - number: 80 \ No newline at end of file + number: 80 diff --git a/tests/suite/test_rewrite_target.py b/tests/suite/test_rewrite_target.py index 75b6cd1267..20d467bf04 100644 --- a/tests/suite/test_rewrite_target.py +++ b/tests/suite/test_rewrite_target.py @@ -42,7 +42,9 @@ def rewrite_target_setup( ingress_controller, test_namespace, ) -> RewriteTargetSetup: - print("------------------------- Deploy Ingress with rewrite-target annotations -----------------------------------") + print( + "------------------------- Deploy Ingress with rewrite-target annotations -----------------------------------" + ) src = f"{TEST_DATA}/rewrite-target/{request.param}.yaml" create_items_from_yaml(kube_apis, src, test_namespace) ingress_name = get_name_from_yaml(src) @@ -82,12 +84,14 @@ def test_static_rewrite_target(self, rewrite_target_setup): Test static rewrite target functionality. Request to /app should be rewritten to /backend. """ - request_url = f"http://{rewrite_target_setup.public_endpoint.public_ip}:{rewrite_target_setup.public_endpoint.port}/app" + request_url = ( + f"http://{rewrite_target_setup.public_endpoint.public_ip}:{rewrite_target_setup.public_endpoint.port}/app" + ) resp = requests.get( request_url, headers={"host": rewrite_target_setup.ingress_host}, ) - + assert resp.status_code == 200 assert "URI: /backend" in resp.text @@ -102,6 +106,6 @@ def test_regex_rewrite_target(self, rewrite_target_setup): request_url, headers={"host": rewrite_target_setup.ingress_host}, ) - + assert resp.status_code == 200 - assert "URI: /api/users/123" in resp.text \ No newline at end of file + assert "URI: /api/users/123" in resp.text From ebc3e1965fa57057b4824d0db195af8ed39f6dfe Mon Sep 17 00:00:00 2001 From: Venktesh Shivam Patel Date: Fri, 7 Nov 2025 15:31:11 +0000 Subject: [PATCH 6/7] fix test data by removing preprocessed inputs --- .../version1/__snapshots__/template_test.snap | 20 +++++++++---------- internal/configs/version1/template_test.go | 20 +++++++++---------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/internal/configs/version1/__snapshots__/template_test.snap b/internal/configs/version1/__snapshots__/template_test.snap index fec22bc723..61a3dbfaef 100644 --- a/internal/configs/version1/__snapshots__/template_test.snap +++ b/internal/configs/version1/__snapshots__/template_test.snap @@ -928,7 +928,7 @@ server { - location ~* "^~* "^/(latte|espresso)"" { + location ~* "^/(latte|espresso)" { set $service ""; status_zone ""; rewrite (?i)^/(latte|espresso) /drinks/$1 break; @@ -975,7 +975,7 @@ server { - location ~ "^~ "^/(coffee|tea)"" { + location ~ "^/(coffee|tea)" { set $service ""; status_zone ""; rewrite ^/(coffee|tea) /beverages/$1 break; @@ -1022,7 +1022,7 @@ server { - location ~ "^~ "^/menu/(hot|cold)/(coffee|tea)"" { + location ~ "^/menu/(hot|cold)/(coffee|tea)" { set $service ""; status_zone ""; rewrite ^/menu/(hot|cold)/(coffee|tea) /drinks/$1/$2 break; @@ -1069,7 +1069,7 @@ server { - location = "= "/cappuccino"" { + location = "/cappuccino" { set $service ""; status_zone ""; rewrite /cappuccino /special/cappuccino break; @@ -1163,7 +1163,7 @@ server { - location ~ "^~ "^/americano"" { + location ~ "^/americano" { set $service ""; status_zone ""; proxy_http_version 1.1; @@ -2081,7 +2081,7 @@ server { set $resource_type "ingress"; set $resource_name "cafe-ingress"; set $resource_namespace "default"; - location ~* "^~* "^/(latte|espresso)"" { + location ~* "^/(latte|espresso)" { set $service ""; rewrite (?i)^/(latte|espresso) /drinks/$1 break; proxy_http_version 1.1; @@ -2118,7 +2118,7 @@ server { set $resource_type "ingress"; set $resource_name "cafe-ingress"; set $resource_namespace "default"; - location ~ "^~ "^/(coffee|tea)"" { + location ~ "^/(coffee|tea)" { set $service ""; rewrite ^/(coffee|tea) /beverages/$1 break; proxy_http_version 1.1; @@ -2155,7 +2155,7 @@ server { set $resource_type "ingress"; set $resource_name "cafe-menu-ingress"; set $resource_namespace "default"; - location ~ "^~ "^/menu/(hot|cold)/(coffee|tea)"" { + location ~ "^/menu/(hot|cold)/(coffee|tea)" { set $service ""; rewrite ^/menu/(hot|cold)/(coffee|tea) /drinks/$1/$2 break; proxy_http_version 1.1; @@ -2192,7 +2192,7 @@ server { set $resource_type "ingress"; set $resource_name "cafe-ingress"; set $resource_namespace "default"; - location = "= "/cappuccino"" { + location = "/cappuccino" { set $service ""; rewrite /cappuccino /special/cappuccino break; proxy_http_version 1.1; @@ -2266,7 +2266,7 @@ server { set $resource_type "ingress"; set $resource_name "cafe-ingress"; set $resource_namespace "default"; - location ~ "^~ "^/americano"" { + location ~ "^/americano" { set $service ""; proxy_http_version 1.1; proxy_connect_timeout ; diff --git a/internal/configs/version1/template_test.go b/internal/configs/version1/template_test.go index 1908aefc49..0f0d6b7714 100644 --- a/internal/configs/version1/template_test.go +++ b/internal/configs/version1/template_test.go @@ -3791,7 +3791,7 @@ func TestExecuteTemplate_ForIngressForNGINXRewriteTarget(t *testing.T) { ServerTokens: "off", Locations: []Location{ { - Path: "~ \"^/(coffee|tea)\"", + Path: "/(coffee|tea)", RewriteTarget: "/beverages/$1", Upstream: testUpstream, }, @@ -3823,7 +3823,7 @@ func TestExecuteTemplate_ForIngressForNGINXRewriteTarget(t *testing.T) { ServerTokens: "off", Locations: []Location{ { - Path: "~* \"^/(latte|espresso)\"", + Path: "/(latte|espresso)", RewriteTarget: "/drinks/$1", Upstream: testUpstream, }, @@ -3855,7 +3855,7 @@ func TestExecuteTemplate_ForIngressForNGINXRewriteTarget(t *testing.T) { ServerTokens: "off", Locations: []Location{ { - Path: "= \"/cappuccino\"", + Path: "/cappuccino", RewriteTarget: "/special/cappuccino", Upstream: testUpstream, }, @@ -3918,7 +3918,7 @@ func TestExecuteTemplate_ForIngressForNGINXRewriteTarget(t *testing.T) { ServerTokens: "off", Locations: []Location{ { - Path: "~ \"^/americano\"", + Path: "/americano", Upstream: testUpstream, // RewriteTarget is empty - should not generate rewrite directive }, @@ -3948,7 +3948,7 @@ func TestExecuteTemplate_ForIngressForNGINXRewriteTarget(t *testing.T) { ServerTokens: "off", Locations: []Location{ { - Path: "~ \"^/menu/(hot|cold)/(coffee|tea)\"", + Path: "/menu/(hot|cold)/(coffee|tea)", RewriteTarget: "/drinks/$1/$2", Upstream: testUpstream, }, @@ -4027,7 +4027,7 @@ func TestExecuteTemplate_ForIngressForNGINXPlusRewriteTarget(t *testing.T) { ServerTokens: "off", Locations: []Location{ { - Path: "~ \"^/(coffee|tea)\"", + Path: "/(coffee|tea)", RewriteTarget: "/beverages/$1", Upstream: testUpstream, }, @@ -4059,7 +4059,7 @@ func TestExecuteTemplate_ForIngressForNGINXPlusRewriteTarget(t *testing.T) { ServerTokens: "off", Locations: []Location{ { - Path: "~* \"^/(latte|espresso)\"", + Path: "/(latte|espresso)", RewriteTarget: "/drinks/$1", Upstream: testUpstream, }, @@ -4091,7 +4091,7 @@ func TestExecuteTemplate_ForIngressForNGINXPlusRewriteTarget(t *testing.T) { ServerTokens: "off", Locations: []Location{ { - Path: "= \"/cappuccino\"", + Path: "/cappuccino", RewriteTarget: "/special/cappuccino", Upstream: testUpstream, }, @@ -4154,7 +4154,7 @@ func TestExecuteTemplate_ForIngressForNGINXPlusRewriteTarget(t *testing.T) { ServerTokens: "off", Locations: []Location{ { - Path: "~ \"^/americano\"", + Path: "/americano", Upstream: testUpstream, // RewriteTarget is empty - should not generate rewrite directive }, @@ -4184,7 +4184,7 @@ func TestExecuteTemplate_ForIngressForNGINXPlusRewriteTarget(t *testing.T) { ServerTokens: "off", Locations: []Location{ { - Path: "~ \"^/menu/(hot|cold)/(coffee|tea)\"", + Path: "/menu/(hot|cold)/(coffee|tea)", RewriteTarget: "/drinks/$1/$2", Upstream: testUpstream, }, From effd464780eeb618b00cfed8971849a0b5431028 Mon Sep 17 00:00:00 2001 From: Venktesh Shivam Patel Date: Fri, 7 Nov 2025 16:06:39 +0000 Subject: [PATCH 7/7] add wait in tests --- tests/suite/test_rewrite_target.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/suite/test_rewrite_target.py b/tests/suite/test_rewrite_target.py index 20d467bf04..16965105c8 100644 --- a/tests/suite/test_rewrite_target.py +++ b/tests/suite/test_rewrite_target.py @@ -8,6 +8,7 @@ delete_common_app, delete_items_from_yaml, ensure_connection_to_public_endpoint, + wait_before_test, wait_until_all_pods_are_ready, ) from suite.utils.yaml_utils import get_first_ingress_host_from_yaml, get_name_from_yaml @@ -87,6 +88,8 @@ def test_static_rewrite_target(self, rewrite_target_setup): request_url = ( f"http://{rewrite_target_setup.public_endpoint.public_ip}:{rewrite_target_setup.public_endpoint.port}/app" ) + + wait_before_test() resp = requests.get( request_url, headers={"host": rewrite_target_setup.ingress_host}, @@ -102,6 +105,8 @@ def test_regex_rewrite_target(self, rewrite_target_setup): Request to /v1/users/123 should be rewritten to /api/users/123. """ request_url = f"http://{rewrite_target_setup.public_endpoint.public_ip}:{rewrite_target_setup.public_endpoint.port}/v1/users/123" + + wait_before_test() resp = requests.get( request_url, headers={"host": rewrite_target_setup.ingress_host},