diff --git a/e2e/ingress_nginx_redirect_test.go b/e2e/ingress_nginx_redirect_test.go new file mode 100644 index 000000000..d58f7be59 --- /dev/null +++ b/e2e/ingress_nginx_redirect_test.go @@ -0,0 +1,324 @@ +/* +Copyright 2026 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package e2e + +import ( + "fmt" + "net/http" + "regexp" + "testing" + + "github.com/kubernetes-sigs/ingress2gateway/pkg/i2gw/providers/ingressnginx" + "github.com/kubernetes-sigs/ingress2gateway/pkg/i2gw/providers/istio" + "github.com/stretchr/testify/require" + networkingv1 "k8s.io/api/networking/v1" +) + +func TestIngressNGINXRedirect(t *testing.T) { + t.Parallel() + t.Run("to Istio", func(t *testing.T) { + t.Parallel() + t.Run("permanent redirect", func(t *testing.T) { + suffix, err := randString() + require.NoError(t, err) + redirectURL := fmt.Sprintf("https://new-site-%s.example.com/new-path/", suffix) + + runTestCase(t, &testCase{ + gatewayImplementation: istio.ProviderName, + providers: []string{ingressnginx.Name}, + providerFlags: map[string]map[string]string{ + ingressnginx.Name: { + ingressnginx.NginxIngressClassFlag: ingressnginx.NginxIngressClass, + }, + }, + ingresses: []*networkingv1.Ingress{ + basicIngress(). + withName("permanent-redirect"). + withIngressClass(ingressnginx.NginxIngressClass). + withAnnotation("nginx.ingress.kubernetes.io/permanent-redirect", redirectURL). + build(), + }, + verifiers: map[string][]verifier{ + "permanent-redirect": { + &httpRequestVerifier{ + path: "/", + allowedCodes: []int{ + http.StatusMovedPermanently, // 301 + }, + headerMatches: []headerMatch{ + { + name: "Location", + patterns: []*maybeNegativePattern{ + {pattern: regexp.MustCompile("^" + regexp.QuoteMeta(redirectURL) + "$")}, + }, + }, + }, + }, + }, + }, + }) + }) + + t.Run("temporal redirect", func(t *testing.T) { + suffix, err := randString() + require.NoError(t, err) + redirectURL := fmt.Sprintf("https://temp-site-%s.example.com/temp-path/", suffix) + + runTestCase(t, &testCase{ + gatewayImplementation: istio.ProviderName, + providers: []string{ingressnginx.Name}, + providerFlags: map[string]map[string]string{ + ingressnginx.Name: { + ingressnginx.NginxIngressClassFlag: ingressnginx.NginxIngressClass, + }, + }, + ingresses: []*networkingv1.Ingress{ + basicIngress(). + withName("temporal-redirect"). + withIngressClass(ingressnginx.NginxIngressClass). + withAnnotation("nginx.ingress.kubernetes.io/temporal-redirect", redirectURL). + build(), + }, + verifiers: map[string][]verifier{ + "temporal-redirect": { + &httpRequestVerifier{ + path: "/", + allowedCodes: []int{ + http.StatusFound, // 302 + }, + headerMatches: []headerMatch{ + { + name: "Location", + patterns: []*maybeNegativePattern{ + {pattern: regexp.MustCompile("^" + regexp.QuoteMeta(redirectURL) + "$")}, + }, + }, + }, + }, + }, + }, + }) + }) + + t.Run("permanent redirect with supported custom code", func(t *testing.T) { + suffix, err := randString() + require.NoError(t, err) + redirectURL := fmt.Sprintf("https://custom-code-%s.example.com/path/", suffix) + + runTestCase(t, &testCase{ + gatewayImplementation: istio.ProviderName, + providers: []string{ingressnginx.Name}, + providerFlags: map[string]map[string]string{ + ingressnginx.Name: { + ingressnginx.NginxIngressClassFlag: ingressnginx.NginxIngressClass, + }, + }, + ingresses: []*networkingv1.Ingress{ + basicIngress(). + withName("permanent-redirect-301"). + withIngressClass(ingressnginx.NginxIngressClass). + withAnnotation("nginx.ingress.kubernetes.io/permanent-redirect", redirectURL). + withAnnotation("nginx.ingress.kubernetes.io/permanent-redirect-code", "301"). + build(), + }, + verifiers: map[string][]verifier{ + "permanent-redirect-301": { + &httpRequestVerifier{ + path: "/", + allowedCodes: []int{ + http.StatusMovedPermanently, // 301 + }, + headerMatches: []headerMatch{ + { + name: "Location", + patterns: []*maybeNegativePattern{ + {pattern: regexp.MustCompile("^" + regexp.QuoteMeta(redirectURL) + "$")}, + }, + }, + }, + }, + }, + }, + }) + }) + + t.Run("temporal redirect with supported custom code", func(t *testing.T) { + suffix, err := randString() + require.NoError(t, err) + redirectURL := fmt.Sprintf("https://custom-temp-%s.example.com/path/", suffix) + + runTestCase(t, &testCase{ + gatewayImplementation: istio.ProviderName, + providers: []string{ingressnginx.Name}, + providerFlags: map[string]map[string]string{ + ingressnginx.Name: { + ingressnginx.NginxIngressClassFlag: ingressnginx.NginxIngressClass, + }, + }, + ingresses: []*networkingv1.Ingress{ + basicIngress(). + withName("temporal-redirect-302"). + withIngressClass(ingressnginx.NginxIngressClass). + withAnnotation("nginx.ingress.kubernetes.io/temporal-redirect", redirectURL). + withAnnotation("nginx.ingress.kubernetes.io/temporal-redirect-code", "302"). + build(), + }, + verifiers: map[string][]verifier{ + "temporal-redirect-302": { + &httpRequestVerifier{ + path: "/", + allowedCodes: []int{ + http.StatusFound, // 302 + }, + headerMatches: []headerMatch{ + { + name: "Location", + patterns: []*maybeNegativePattern{ + {pattern: regexp.MustCompile("^" + regexp.QuoteMeta(redirectURL) + "$")}, + }, + }, + }, + }, + }, + }, + }) + }) + + t.Run("redirect with scheme and hostname only", func(t *testing.T) { + redirectURL := "https://another-domain.example.com/" + + runTestCase(t, &testCase{ + gatewayImplementation: istio.ProviderName, + providers: []string{ingressnginx.Name}, + providerFlags: map[string]map[string]string{ + ingressnginx.Name: { + ingressnginx.NginxIngressClassFlag: ingressnginx.NginxIngressClass, + }, + }, + ingresses: []*networkingv1.Ingress{ + basicIngress(). + withName("redirect-hostname-only"). + withIngressClass(ingressnginx.NginxIngressClass). + withAnnotation("nginx.ingress.kubernetes.io/permanent-redirect", redirectURL). + build(), + }, + verifiers: map[string][]verifier{ + "redirect-hostname-only": { + &httpRequestVerifier{ + path: "/some/path", + allowedCodes: []int{ + http.StatusMovedPermanently, // 301 + }, + headerMatches: []headerMatch{ + { + name: "Location", + patterns: []*maybeNegativePattern{ + {pattern: regexp.MustCompile("^" + regexp.QuoteMeta(redirectURL) + "$")}, + }, + }, + }, + }, + }, + }, + }) + }) + + t.Run("redirect with port", func(t *testing.T) { + suffix, err := randString() + require.NoError(t, err) + redirectURL := fmt.Sprintf("https://custom-port-%s.example.com:8443/secure/", suffix) + + runTestCase(t, &testCase{ + gatewayImplementation: istio.ProviderName, + providers: []string{ingressnginx.Name}, + providerFlags: map[string]map[string]string{ + ingressnginx.Name: { + ingressnginx.NginxIngressClassFlag: ingressnginx.NginxIngressClass, + }, + }, + ingresses: []*networkingv1.Ingress{ + basicIngress(). + withName("redirect-with-port"). + withIngressClass(ingressnginx.NginxIngressClass). + withAnnotation("nginx.ingress.kubernetes.io/temporal-redirect", redirectURL). + build(), + }, + verifiers: map[string][]verifier{ + "redirect-with-port": { + &httpRequestVerifier{ + path: "/", + allowedCodes: []int{ + http.StatusFound, // 302 + }, + headerMatches: []headerMatch{ + { + name: "Location", + patterns: []*maybeNegativePattern{ + {pattern: regexp.MustCompile("^" + regexp.QuoteMeta(redirectURL) + "$")}, + }, + }, + }, + }, + }, + }, + }) + }) + + t.Run("both redirect annotations - temporal takes priority", func(t *testing.T) { + suffix, err := randString() + require.NoError(t, err) + permanentURL := fmt.Sprintf("https://permanent-%s.example.com/path/", suffix) + temporalURL := fmt.Sprintf("https://temporal-%s.example.com/path/", suffix) + + runTestCase(t, &testCase{ + gatewayImplementation: istio.ProviderName, + providers: []string{ingressnginx.Name}, + providerFlags: map[string]map[string]string{ + ingressnginx.Name: { + ingressnginx.NginxIngressClassFlag: ingressnginx.NginxIngressClass, + }, + }, + ingresses: []*networkingv1.Ingress{ + basicIngress(). + withName("both-redirects"). + withIngressClass(ingressnginx.NginxIngressClass). + withAnnotation("nginx.ingress.kubernetes.io/permanent-redirect", permanentURL). + withAnnotation("nginx.ingress.kubernetes.io/temporal-redirect", temporalURL). + build(), + }, + verifiers: map[string][]verifier{ + "both-redirects": { + &httpRequestVerifier{ + path: "/", + allowedCodes: []int{ + http.StatusFound, // 302 - temporal takes priority + }, + headerMatches: []headerMatch{ + { + name: "Location", + patterns: []*maybeNegativePattern{ + {pattern: regexp.MustCompile("^" + regexp.QuoteMeta(temporalURL) + "$")}, + }, + }, + }, + }, + }, + }, + }) + }) + }) +} diff --git a/pkg/i2gw/providers/ingressnginx/annotations.go b/pkg/i2gw/providers/ingressnginx/annotations.go index 3355699f5..164ce31ed 100644 --- a/pkg/i2gw/providers/ingressnginx/annotations.go +++ b/pkg/i2gw/providers/ingressnginx/annotations.go @@ -29,6 +29,15 @@ const ( // Rewrite annotations RewriteTargetAnnotation = "nginx.ingress.kubernetes.io/rewrite-target" + // Redirect annotations + PermanentRedirectAnnotation = "nginx.ingress.kubernetes.io/permanent-redirect" + PermanentRedirectCodeAnnotation = "nginx.ingress.kubernetes.io/permanent-redirect-code" + TemporalRedirectAnnotation = "nginx.ingress.kubernetes.io/temporal-redirect" + TemporalRedirectCodeAnnotation = "nginx.ingress.kubernetes.io/temporal-redirect-code" + FromToWWWRedirectAnnotation = "nginx.ingress.kubernetes.io/from-to-www-redirect" + ProxyRedirectFromAnnotation = "nginx.ingress.kubernetes.io/proxy-redirect-from" + ProxyRedirectToAnnotation = "nginx.ingress.kubernetes.io/proxy-redirect-to" + // Header annotations XForwardedPrefixAnnotation = "nginx.ingress.kubernetes.io/x-forwarded-prefix" UpstreamVhostAnnotation = "nginx.ingress.kubernetes.io/upstream-vhost" diff --git a/pkg/i2gw/providers/ingressnginx/converter.go b/pkg/i2gw/providers/ingressnginx/converter.go index 252eedeb4..3f44da527 100644 --- a/pkg/i2gw/providers/ingressnginx/converter.go +++ b/pkg/i2gw/providers/ingressnginx/converter.go @@ -34,6 +34,7 @@ func newResourcesToIRConverter() *resourcesToIRConverter { return &resourcesToIRConverter{ featureParsers: []i2gw.FeatureParser{ canaryFeature, + redirectFeature, headerModifierFeature, regexFeature, }, diff --git a/pkg/i2gw/providers/ingressnginx/redirect.go b/pkg/i2gw/providers/ingressnginx/redirect.go index adfc7d44d..187084c98 100644 --- a/pkg/i2gw/providers/ingressnginx/redirect.go +++ b/pkg/i2gw/providers/ingressnginx/redirect.go @@ -18,10 +18,13 @@ package ingressnginx import ( "fmt" + "net/url" "strconv" emitterir "github.com/kubernetes-sigs/ingress2gateway/pkg/i2gw/emitter_intermediate" + "github.com/kubernetes-sigs/ingress2gateway/pkg/i2gw/notifications" providerir "github.com/kubernetes-sigs/ingress2gateway/pkg/i2gw/provider_intermediate" + networkingv1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/validation/field" @@ -30,6 +33,191 @@ import ( gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" ) +// redirectFeature converts permanent and temporal redirect annotations to Gateway API RequestRedirect filters. +// This matches ingress-nginx's execution order: temporal redirect is checked first, then permanent. +// If the temporal-redirect annotation key is present (even with an empty value), the function +// short-circuits and permanent redirect annotations are never evaluated. +// +// Gateway API only supports status codes 301, 302, 303, 307, 308. +// Intersecting with ingress-nginx's valid ranges: +// - temporal-redirect defaults to 302, supported custom codes: 301, 302, 303, 307 +// - permanent-redirect defaults to 301, supported custom codes: 301, 302, 303, 307, 308 +func redirectFeature(ingresses []networkingv1.Ingress, _ map[types.NamespacedName]map[string]int32, ir *providerir.ProviderIR) field.ErrorList { + var errs field.ErrorList + + // Iterate over all HTTPRoutes in the IR + for key, httpRouteContext := range ir.HTTPRoutes { + // Iterate over each rule in the HTTPRoute + for ruleIndex := range httpRouteContext.HTTPRoute.Spec.Rules { + // Check if this rule has backend sources + if ruleIndex >= len(httpRouteContext.RuleBackendSources) { + continue + } + + // Get the non canary ingress for this rule + ingress := getNonCanaryIngress(httpRouteContext.RuleBackendSources[ruleIndex]) + + if ingress == nil { + continue + } + + // Warn about unsupported proxy-redirect annotations + if ingress.Annotations[ProxyRedirectFromAnnotation] != "" { + notify(notifications.WarningNotification, fmt.Sprintf("ingress %s/%s uses unsupported annotation %s", + ingress.Namespace, ingress.Name, ProxyRedirectFromAnnotation), ingress) + } + if ingress.Annotations[ProxyRedirectToAnnotation] != "" { + notify(notifications.WarningNotification, fmt.Sprintf("ingress %s/%s uses unsupported annotation %s", + ingress.Namespace, ingress.Name, ProxyRedirectToAnnotation), ingress) + } + + temporalRedirectURL, hasTemporal := ingress.Annotations[TemporalRedirectAnnotation] + permanentRedirectURL, hasPermanent := ingress.Annotations[PermanentRedirectAnnotation] + + // Skip if neither annotation is present + if !hasPermanent && !hasTemporal { + continue + } + + // Determine redirect URL and status code. + // Matching ingress-nginx execution order: temporal is checked first. + // If the temporal-redirect annotation key is present (even with an empty value), + // the function short-circuits — permanent redirect is never evaluated. + var redirectURL string + var statusCode int + var annotationUsed string + + if hasTemporal { + redirectURL = temporalRedirectURL + statusCode = 302 + annotationUsed = TemporalRedirectAnnotation + + // Warn if both annotations are present (permanent is ignored) + if hasPermanent { + notify(notifications.WarningNotification, fmt.Sprintf("ingress %s/%s has both %s and %s annotations; temporal-redirect takes priority, permanent-redirect is ignored", + ingress.Namespace, ingress.Name, PermanentRedirectAnnotation, TemporalRedirectAnnotation), ingress) + } + + // Check custom status code annotation. + if codeStr := ingress.Annotations[TemporalRedirectCodeAnnotation]; codeStr != "" { + code, err := strconv.Atoi(codeStr) + if err != nil || !isValidTemporalRedirectCode(code) { + notify(notifications.WarningNotification, fmt.Sprintf("ingress %s/%s uses unsupported status code %q in %s annotation (Gateway API supports: 301, 302, 303, 307 for temporal redirects), using default 302", + ingress.Namespace, ingress.Name, codeStr, TemporalRedirectCodeAnnotation), ingress) + } else { + statusCode = code + } + } + } else { + // Only reached if temporal-redirect annotation is completely absent + redirectURL = permanentRedirectURL + statusCode = 301 + annotationUsed = PermanentRedirectAnnotation + + // Check custom status code annotation. + if codeStr := ingress.Annotations[PermanentRedirectCodeAnnotation]; codeStr != "" { + code, err := strconv.Atoi(codeStr) + if err != nil || !isValidPermanentRedirectCode(code) { + notify(notifications.WarningNotification, fmt.Sprintf("ingress %s/%s uses unsupported status code %q in %s annotation (Gateway API supports: 301, 302, 303, 307, 308 for permanent redirects), using default 301", + ingress.Namespace, ingress.Name, codeStr, PermanentRedirectCodeAnnotation), ingress) + } else { + statusCode = code + } + } + } + + // Validate that the redirect URL is not empty + if redirectURL == "" { + errs = append(errs, field.Invalid( + field.NewPath("ingress", ingress.Namespace, ingress.Name, "metadata", "annotations", annotationUsed), + redirectURL, + "redirect URL cannot be empty", + )) + continue + } + + // Parse the redirect URL + parsedURL, err := url.Parse(redirectURL) + if err != nil { + errs = append(errs, field.Invalid( + field.NewPath("ingress", ingress.Namespace, ingress.Name, "metadata", "annotations", annotationUsed), + redirectURL, + fmt.Sprintf("invalid redirect URL: %v", err), + )) + continue + } + + // Create the redirect filter + redirectFilterConfig := &gatewayv1.HTTPRequestRedirectFilter{ + StatusCode: ptr.To(statusCode), + } + + // Set scheme if present + if parsedURL.Scheme != "" { + redirectFilterConfig.Scheme = ptr.To(parsedURL.Scheme) + } + + // Set hostname if present + if parsedURL.Hostname() != "" { + hostname := gatewayv1.PreciseHostname(parsedURL.Hostname()) + redirectFilterConfig.Hostname = &hostname + } + + // Set port if present + if parsedURL.Port() != "" { + port, err := strconv.Atoi(parsedURL.Port()) + if err == nil { + portNumber := gatewayv1.PortNumber(port) + redirectFilterConfig.Port = &portNumber + } else { + errs = append(errs, field.Invalid( + field.NewPath("ingress", ingress.Namespace, ingress.Name, "metadata", "annotations", annotationUsed), + redirectURL, + fmt.Sprintf("invalid port in redirect URL: %v", err), + )) + continue + } + } + + // Set path - default to root path if not specified in redirect URL + // This matches ingress-nginx behavior where redirects override the request path + path := parsedURL.Path + if path == "" { + path = "/" + } + pathType := gatewayv1.FullPathHTTPPathModifier + redirectFilterConfig.Path = &gatewayv1.HTTPPathModifier{ + Type: pathType, + ReplaceFullPath: ptr.To(path), + } + + redirectFilter := gatewayv1.HTTPRouteFilter{ + Type: gatewayv1.HTTPRouteFilterRequestRedirect, + RequestRedirect: redirectFilterConfig, + } + + // Add redirect filter to the current rule + httpRouteContext.HTTPRoute.Spec.Rules[ruleIndex].Filters = append( + httpRouteContext.HTTPRoute.Spec.Rules[ruleIndex].Filters, + redirectFilter, + ) + + // Clear backend refs as redirects don't route to backends + httpRouteContext.HTTPRoute.Spec.Rules[ruleIndex].BackendRefs = nil + + notify(notifications.InfoNotification, + fmt.Sprintf("parsed %q annotation of ingress %s/%s with redirect to %q (status code: %d). ", + annotationUsed, ingress.Namespace, ingress.Name, redirectURL, statusCode), + &httpRouteContext.HTTPRoute) + } + + // Save the updated context back to the IR + ir.HTTPRoutes[key] = httpRouteContext + } + + return errs +} + // Ingress NGINX has some quirky behaviors around SSL redirect. // The formula we follow is that if an ingress has certs configured, and it does not have the // "nginx.ingress.kubernetes.io/ssl-redirect" annotation set to "false" (or "0", etc), then we @@ -112,3 +300,27 @@ func addDefaultSSLRedirect(pir *providerir.ProviderIR, eir *emitterir.EmitterIR) } return nil } + +// isValidTemporalRedirectCode returns true if the code is in the intersection of +// ingress-nginx temporal-redirect codes (300-307) and Gateway API codes (301,302,303,307,308). +// Result: 301, 302, 303, 307 +func isValidTemporalRedirectCode(code int) bool { + switch code { + case 301, 302, 303, 307: + return true + default: + return false + } +} + +// isValidPermanentRedirectCode returns true if the code is in the intersection of +// ingress-nginx permanent-redirect codes (300-308) and Gateway API codes (301,302,303,307,308). +// Result: 301, 302, 303, 307, 308 +func isValidPermanentRedirectCode(code int) bool { + switch code { + case 301, 302, 303, 307, 308: + return true + default: + return false + } +} diff --git a/pkg/i2gw/providers/ingressnginx/redirect_test.go b/pkg/i2gw/providers/ingressnginx/redirect_test.go index d5300c8f9..458f193bd 100644 --- a/pkg/i2gw/providers/ingressnginx/redirect_test.go +++ b/pkg/i2gw/providers/ingressnginx/redirect_test.go @@ -19,183 +19,516 @@ package ingressnginx import ( "testing" - emitterir "github.com/kubernetes-sigs/ingress2gateway/pkg/i2gw/emitter_intermediate" providerir "github.com/kubernetes-sigs/ingress2gateway/pkg/i2gw/provider_intermediate" + "github.com/kubernetes-sigs/ingress2gateway/pkg/i2gw/providers/common" networkingv1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/ptr" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" ) -func TestAddDefaultSSLRedirect_enabled(t *testing.T) { - key := types.NamespacedName{Namespace: "default", Name: "route"} - parentRefs := []gatewayv1.ParentReference{{Name: gatewayv1.ObjectName("gw")}} - - ing := networkingv1.Ingress{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: key.Namespace, - Name: "ing", - Annotations: map[string]string{ - // no SSLRedirectAnnotation -> default enabled +func Test_redirectFeature(t *testing.T) { + tests := []struct { + name string + ingress networkingv1.Ingress + initialHTTPRoute *gatewayv1.HTTPRoute + expectedHTTPRoute *gatewayv1.HTTPRoute + expectError bool + }{ + { + name: "permanent-redirect annotation", + ingress: networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ingress", + Namespace: "default", + Annotations: map[string]string{ + PermanentRedirectAnnotation: "https://example.com/new-path", + }, + }, + Spec: networkingv1.IngressSpec{ + Rules: []networkingv1.IngressRule{ + { + Host: "foo.com", + IngressRuleValue: networkingv1.IngressRuleValue{ + HTTP: &networkingv1.HTTPIngressRuleValue{ + Paths: []networkingv1.HTTPIngressPath{ + { + Path: "/", + Backend: networkingv1.IngressBackend{ + Service: &networkingv1.IngressServiceBackend{ + Name: "foo", + Port: networkingv1.ServiceBackendPort{ + Number: 3000, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + initialHTTPRoute: &gatewayv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ingress-foo-com", + Namespace: "default", + }, + Spec: gatewayv1.HTTPRouteSpec{ + Hostnames: []gatewayv1.Hostname{"foo.com"}, + Rules: []gatewayv1.HTTPRouteRule{ + { + BackendRefs: []gatewayv1.HTTPBackendRef{ + {BackendRef: gatewayv1.BackendRef{BackendObjectReference: gatewayv1.BackendObjectReference{Name: "foo", Port: ptr.To(gatewayv1.PortNumber(3000))}}}, + }, + }, + }, + }, }, + expectedHTTPRoute: &gatewayv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ingress-foo-com", + Namespace: "default", + }, + Spec: gatewayv1.HTTPRouteSpec{ + Hostnames: []gatewayv1.Hostname{"foo.com"}, + Rules: []gatewayv1.HTTPRouteRule{ + { + Filters: []gatewayv1.HTTPRouteFilter{ + { + Type: gatewayv1.HTTPRouteFilterRequestRedirect, + RequestRedirect: &gatewayv1.HTTPRequestRedirectFilter{ + Scheme: ptr.To("https"), + Hostname: ptr.To(gatewayv1.PreciseHostname("example.com")), + Path: &gatewayv1.HTTPPathModifier{Type: gatewayv1.FullPathHTTPPathModifier, ReplaceFullPath: ptr.To("/new-path")}, + StatusCode: ptr.To(301), + }, + }, + }, + BackendRefs: nil, + }, + }, + }, + }, + expectError: false, }, - Spec: networkingv1.IngressSpec{ - TLS: []networkingv1.IngressTLS{{SecretName: "secret"}}, + { + name: "temporal-redirect annotation", + ingress: networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ingress", + Namespace: "default", + Annotations: map[string]string{ + TemporalRedirectAnnotation: "https://example.com/temporary", + }, + }, + Spec: networkingv1.IngressSpec{ + Rules: []networkingv1.IngressRule{ + { + Host: "bar.com", + IngressRuleValue: networkingv1.IngressRuleValue{ + HTTP: &networkingv1.HTTPIngressRuleValue{ + Paths: []networkingv1.HTTPIngressPath{ + { + Path: "/", + Backend: networkingv1.IngressBackend{ + Service: &networkingv1.IngressServiceBackend{ + Name: "bar", + Port: networkingv1.ServiceBackendPort{ + Number: 8080, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + initialHTTPRoute: &gatewayv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ingress-bar-com", + Namespace: "default", + }, + Spec: gatewayv1.HTTPRouteSpec{ + Hostnames: []gatewayv1.Hostname{"bar.com"}, + Rules: []gatewayv1.HTTPRouteRule{ + { + BackendRefs: []gatewayv1.HTTPBackendRef{ + {BackendRef: gatewayv1.BackendRef{BackendObjectReference: gatewayv1.BackendObjectReference{Name: "bar", Port: ptr.To(gatewayv1.PortNumber(8080))}}}, + }, + }, + }, + }, + }, + expectedHTTPRoute: &gatewayv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ingress-bar-com", + Namespace: "default", + }, + Spec: gatewayv1.HTTPRouteSpec{ + Hostnames: []gatewayv1.Hostname{"bar.com"}, + Rules: []gatewayv1.HTTPRouteRule{ + { + Filters: []gatewayv1.HTTPRouteFilter{ + { + Type: gatewayv1.HTTPRouteFilterRequestRedirect, + RequestRedirect: &gatewayv1.HTTPRequestRedirectFilter{ + Scheme: ptr.To("https"), + Hostname: ptr.To(gatewayv1.PreciseHostname("example.com")), + Path: &gatewayv1.HTTPPathModifier{Type: gatewayv1.FullPathHTTPPathModifier, ReplaceFullPath: ptr.To("/temporary")}, + StatusCode: ptr.To(302), + }, + }, + }, + BackendRefs: nil, + }, + }, + }, + }, + expectError: false, }, - } - - route := gatewayv1.HTTPRoute{ - ObjectMeta: metav1.ObjectMeta{Namespace: key.Namespace, Name: key.Name}, - Spec: gatewayv1.HTTPRouteSpec{ - CommonRouteSpec: gatewayv1.CommonRouteSpec{ - ParentRefs: append([]gatewayv1.ParentReference(nil), parentRefs...), + { + name: "both annotations present should choose temporal", + ingress: networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ingress", + Namespace: "default", + Annotations: map[string]string{ + PermanentRedirectAnnotation: "https://example.com/permanent", + TemporalRedirectAnnotation: "https://example.com/temporal", + }, + }, + Spec: networkingv1.IngressSpec{ + Rules: []networkingv1.IngressRule{ + { + Host: "conflict.com", + IngressRuleValue: networkingv1.IngressRuleValue{ + HTTP: &networkingv1.HTTPIngressRuleValue{ + Paths: []networkingv1.HTTPIngressPath{ + { + Path: "/", + Backend: networkingv1.IngressBackend{ + Service: &networkingv1.IngressServiceBackend{ + Name: "conflict", + Port: networkingv1.ServiceBackendPort{ + Number: 8080, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, }, - Hostnames: []gatewayv1.Hostname{"example.com"}, + initialHTTPRoute: &gatewayv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ingress-conflict-com", + Namespace: "default", + }, + Spec: gatewayv1.HTTPRouteSpec{ + Hostnames: []gatewayv1.Hostname{"conflict.com"}, + Rules: []gatewayv1.HTTPRouteRule{ + { + BackendRefs: []gatewayv1.HTTPBackendRef{ + {BackendRef: gatewayv1.BackendRef{BackendObjectReference: gatewayv1.BackendObjectReference{Name: "conflict", Port: ptr.To(gatewayv1.PortNumber(8080))}}}, + }, + }, + }, + }, + }, + expectedHTTPRoute: &gatewayv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ingress-conflict-com", + Namespace: "default", + }, + Spec: gatewayv1.HTTPRouteSpec{ + Hostnames: []gatewayv1.Hostname{"conflict.com"}, + Rules: []gatewayv1.HTTPRouteRule{ + { + Filters: []gatewayv1.HTTPRouteFilter{ + { + Type: gatewayv1.HTTPRouteFilterRequestRedirect, + RequestRedirect: &gatewayv1.HTTPRequestRedirectFilter{ + Scheme: ptr.To("https"), + Hostname: ptr.To(gatewayv1.PreciseHostname("example.com")), + Path: &gatewayv1.HTTPPathModifier{Type: gatewayv1.FullPathHTTPPathModifier, ReplaceFullPath: ptr.To("/temporal")}, + StatusCode: ptr.To(302), + }, + }, + }, + BackendRefs: nil, + }, + }, + }, + }, + expectError: false, + }, + { + name: "no redirect annotations", + ingress: networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ingress", + Namespace: "default", + }, + Spec: networkingv1.IngressSpec{ + Rules: []networkingv1.IngressRule{ + { + Host: "normal.com", + IngressRuleValue: networkingv1.IngressRuleValue{ + HTTP: &networkingv1.HTTPIngressRuleValue{ + Paths: []networkingv1.HTTPIngressPath{ + { + Path: "/", + Backend: networkingv1.IngressBackend{ + Service: &networkingv1.IngressServiceBackend{ + Name: "normal", + Port: networkingv1.ServiceBackendPort{ + Number: 80, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + initialHTTPRoute: &gatewayv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ingress-normal-com", + Namespace: "default", + }, + Spec: gatewayv1.HTTPRouteSpec{ + Hostnames: []gatewayv1.Hostname{"normal.com"}, + Rules: []gatewayv1.HTTPRouteRule{ + { + BackendRefs: []gatewayv1.HTTPBackendRef{ + {BackendRef: gatewayv1.BackendRef{BackendObjectReference: gatewayv1.BackendObjectReference{Name: "normal", Port: ptr.To(gatewayv1.PortNumber(80))}}}, + }, + }, + }, + }, + }, + expectedHTTPRoute: &gatewayv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ingress-normal-com", + Namespace: "default", + }, + Spec: gatewayv1.HTTPRouteSpec{ + Hostnames: []gatewayv1.Hostname{"normal.com"}, + Rules: []gatewayv1.HTTPRouteRule{ + { + BackendRefs: []gatewayv1.HTTPBackendRef{ + {BackendRef: gatewayv1.BackendRef{BackendObjectReference: gatewayv1.BackendObjectReference{Name: "normal", Port: ptr.To(gatewayv1.PortNumber(80))}}}, + }, + }, + }, + }, + }, + expectError: false, }, } - pIR := providerir.ProviderIR{HTTPRoutes: map[types.NamespacedName]providerir.HTTPRouteContext{}} - pIR.HTTPRoutes[key] = providerir.HTTPRouteContext{ - HTTPRoute: route, - RuleBackendSources: [][]providerir.BackendSource{{ - {Ingress: &ing}, - }}, - } - - eIR := emitterir.EmitterIR{HTTPRoutes: map[types.NamespacedName]emitterir.HTTPRouteContext{}} - eIR.HTTPRoutes[key] = emitterir.HTTPRouteContext{HTTPRoute: route} - - addDefaultSSLRedirect(&pIR, &eIR) - - redirectKey := types.NamespacedName{Namespace: key.Namespace, Name: key.Name + "-ssl-redirect"} - redirectCtx, ok := eIR.HTTPRoutes[redirectKey] - if !ok { - t.Fatalf("expected redirect route %v to be added", redirectKey) - } - - if len(redirectCtx.Spec.ParentRefs) != 1 || redirectCtx.Spec.ParentRefs[0].Port == nil || *redirectCtx.Spec.ParentRefs[0].Port != 80 { - t.Fatalf("expected redirect route parentRef port 80, got %#v", redirectCtx.Spec.ParentRefs) - } - - origCtx := eIR.HTTPRoutes[key] - if len(origCtx.Spec.ParentRefs) != 1 || origCtx.Spec.ParentRefs[0].Port == nil || *origCtx.Spec.ParentRefs[0].Port != 443 { - t.Fatalf("expected original route parentRef port 443, got %#v", origCtx.Spec.ParentRefs) - } - - if len(redirectCtx.Spec.Rules) != 1 || len(redirectCtx.Spec.Rules[0].Filters) != 1 { - t.Fatalf("expected redirect route to have 1 rule with 1 filter, got %#v", redirectCtx.Spec.Rules) - } - - f := redirectCtx.Spec.Rules[0].Filters[0] - if f.Type != gatewayv1.HTTPRouteFilterRequestRedirect || f.RequestRedirect == nil { - t.Fatalf("expected RequestRedirect filter, got %#v", f) - } - if f.RequestRedirect.Scheme == nil || *f.RequestRedirect.Scheme != "https" { - t.Fatalf("expected scheme https, got %#v", f.RequestRedirect.Scheme) - } - if f.RequestRedirect.StatusCode == nil || *f.RequestRedirect.StatusCode != 308 { - t.Fatalf("expected status code 308, got %#v", f.RequestRedirect.StatusCode) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Set up the IR with the initial HTTPRoute + ir := providerir.ProviderIR{ + HTTPRoutes: map[types.NamespacedName]providerir.HTTPRouteContext{}, + } + + if tt.initialHTTPRoute != nil { + routeKey := types.NamespacedName{ + Namespace: tt.initialHTTPRoute.Namespace, + Name: tt.initialHTTPRoute.Name, + } + // Initialize RuleBackendSources to match the number of rules + ruleBackendSources := make([][]providerir.BackendSource, len(tt.initialHTTPRoute.Spec.Rules)) + for i := range ruleBackendSources { + ruleBackendSources[i] = []providerir.BackendSource{ + { + Ingress: &tt.ingress, + }, + } + } + ir.HTTPRoutes[routeKey] = providerir.HTTPRouteContext{ + HTTPRoute: *tt.initialHTTPRoute, + RuleBackendSources: ruleBackendSources, + } + } + + // Call the feature parser + errs := redirectFeature([]networkingv1.Ingress{tt.ingress}, nil, &ir) + + // Check error expectations + if tt.expectError && len(errs) == 0 { + t.Errorf("Expected error but got none") + } + if !tt.expectError && len(errs) > 0 { + t.Errorf("Unexpected errors: %v", errs) + } + + // If we don't expect errors, verify the HTTPRoute was modified correctly + if !tt.expectError && tt.expectedHTTPRoute != nil { + routeKey := types.NamespacedName{ + Namespace: tt.expectedHTTPRoute.Namespace, + Name: tt.expectedHTTPRoute.Name, + } + httpRouteContext, exists := ir.HTTPRoutes[routeKey] + if !exists { + t.Errorf("Expected HTTPRoute %s to exist", routeKey) + return + } + + actualRoute := httpRouteContext.HTTPRoute + + // Verify number of rules + if len(actualRoute.Spec.Rules) != len(tt.expectedHTTPRoute.Spec.Rules) { + t.Errorf("Expected %d rules, got %d", len(tt.expectedHTTPRoute.Spec.Rules), len(actualRoute.Spec.Rules)) + return + } + + // Verify redirect filter in first rule if expected + if len(tt.expectedHTTPRoute.Spec.Rules) > 0 && len(tt.expectedHTTPRoute.Spec.Rules[0].Filters) > 0 { + if len(actualRoute.Spec.Rules[0].Filters) == 0 { + t.Errorf("Expected redirect filter in first rule") + return + } + + actualFilter := actualRoute.Spec.Rules[0].Filters[0] + expectedFilter := tt.expectedHTTPRoute.Spec.Rules[0].Filters[0] + + if actualFilter.Type != expectedFilter.Type { + t.Errorf("Expected filter type %v, got %v", expectedFilter.Type, actualFilter.Type) + } + + if actualFilter.RequestRedirect == nil { + t.Errorf("Expected RequestRedirect to be set") + return + } + + expected := expectedFilter.RequestRedirect + actual := actualFilter.RequestRedirect + + if expected.StatusCode != nil { + if actual.StatusCode == nil { + t.Errorf("Expected status code to be set") + } else if *actual.StatusCode != *expected.StatusCode { + t.Errorf("Expected status code %d, got %d", *expected.StatusCode, *actual.StatusCode) + } + } + + if expected.Scheme != nil { + if actual.Scheme == nil { + t.Errorf("Expected scheme to be set") + } else if *actual.Scheme != *expected.Scheme { + t.Errorf("Expected scheme %s, got %s", *expected.Scheme, *actual.Scheme) + } + } + + if expected.Hostname != nil { + if actual.Hostname == nil { + t.Errorf("Expected hostname to be set") + } else if *actual.Hostname != *expected.Hostname { + t.Errorf("Expected hostname %s, got %s", *expected.Hostname, *actual.Hostname) + } + } + + if expected.Path != nil { + if actual.Path == nil { + t.Errorf("Expected path to be set") + } else { + if actual.Path.Type != expected.Path.Type { + t.Errorf("Expected path type %v, got %v", expected.Path.Type, actual.Path.Type) + } + if expected.Path.ReplaceFullPath != nil { + if actual.Path.ReplaceFullPath == nil { + t.Errorf("Expected ReplaceFullPath to be set") + } else if *actual.Path.ReplaceFullPath != *expected.Path.ReplaceFullPath { + t.Errorf("Expected ReplaceFullPath %s, got %s", *expected.Path.ReplaceFullPath, *actual.Path.ReplaceFullPath) + } + } + } + } + + // Verify BackendRefs are cleared when redirect is present + if len(actualRoute.Spec.Rules[0].BackendRefs) != 0 { + t.Errorf("Expected BackendRefs to be cleared for redirect rule, got %d refs", len(actualRoute.Spec.Rules[0].BackendRefs)) + } + } + } + }) } } -func TestAddDefaultSSLRedirect_disabledByAnnotation(t *testing.T) { - key := types.NamespacedName{Namespace: "default", Name: "route"} - - ing := networkingv1.Ingress{ +func Test_redirectFeature_emptyURL(t *testing.T) { + ingress := networkingv1.Ingress{ ObjectMeta: metav1.ObjectMeta{ - Namespace: key.Namespace, - Name: "ing", + Name: "test-ingress", + Namespace: "default", Annotations: map[string]string{ - SSLRedirectAnnotation: "false", + PermanentRedirectAnnotation: "", }, }, Spec: networkingv1.IngressSpec{ - TLS: []networkingv1.IngressTLS{{SecretName: "secret"}}, - }, - } - - route := gatewayv1.HTTPRoute{ - ObjectMeta: metav1.ObjectMeta{Namespace: key.Namespace, Name: key.Name}, - Spec: gatewayv1.HTTPRouteSpec{ - CommonRouteSpec: gatewayv1.CommonRouteSpec{ - ParentRefs: []gatewayv1.ParentReference{{Name: gatewayv1.ObjectName("gw")}}, + Rules: []networkingv1.IngressRule{ + { + Host: "empty.com", + IngressRuleValue: networkingv1.IngressRuleValue{ + HTTP: &networkingv1.HTTPIngressRuleValue{ + Paths: []networkingv1.HTTPIngressPath{ + { + Path: "/", + Backend: networkingv1.IngressBackend{ + Service: &networkingv1.IngressServiceBackend{ + Name: "empty", + Port: networkingv1.ServiceBackendPort{ + Number: 80, + }, + }, + }, + }, + }, + }, + }, + }, }, - Hostnames: []gatewayv1.Hostname{"example.com"}, - }, - } - - pIR := providerir.ProviderIR{HTTPRoutes: map[types.NamespacedName]providerir.HTTPRouteContext{}} - pIR.HTTPRoutes[key] = providerir.HTTPRouteContext{ - HTTPRoute: route, - RuleBackendSources: [][]providerir.BackendSource{{ - {Ingress: &ing}, - }}, - } - - eIR := emitterir.EmitterIR{HTTPRoutes: map[types.NamespacedName]emitterir.HTTPRouteContext{}} - eIR.HTTPRoutes[key] = emitterir.HTTPRouteContext{HTTPRoute: route} - - addDefaultSSLRedirect(&pIR, &eIR) - - redirectKey := types.NamespacedName{Namespace: key.Namespace, Name: key.Name + "-ssl-redirect"} - if _, ok := eIR.HTTPRoutes[redirectKey]; ok { - t.Fatalf("did not expect redirect route %v to be added", redirectKey) - } - - origCtx := eIR.HTTPRoutes[key] - if len(origCtx.Spec.ParentRefs) != 1 { - t.Fatalf("expected 1 parentRef, got %#v", origCtx.Spec.ParentRefs) - } - if origCtx.Spec.ParentRefs[0].Port != nil { - t.Fatalf("expected original route parentRef port to remain nil, got %#v", origCtx.Spec.ParentRefs[0].Port) - } -} - -func TestAddDefaultSSLRedirect_noTLS(t *testing.T) { - key := types.NamespacedName{Namespace: "default", Name: "route"} - - ing := networkingv1.Ingress{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: key.Namespace, - Name: "ing", - Annotations: map[string]string{}, }, - Spec: networkingv1.IngressSpec{}, } - route := gatewayv1.HTTPRoute{ - ObjectMeta: metav1.ObjectMeta{Namespace: key.Namespace, Name: key.Name}, - Spec: gatewayv1.HTTPRouteSpec{ - CommonRouteSpec: gatewayv1.CommonRouteSpec{ - ParentRefs: []gatewayv1.ParentReference{{Name: gatewayv1.ObjectName("gw")}}, + ir := providerir.ProviderIR{ + HTTPRoutes: map[types.NamespacedName]providerir.HTTPRouteContext{ + {Namespace: "default", Name: common.RouteName("test-ingress", "empty.com")}: { + HTTPRoute: gatewayv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: common.RouteName("test-ingress", "empty.com"), + Namespace: "default", + }, + Spec: gatewayv1.HTTPRouteSpec{ + Rules: []gatewayv1.HTTPRouteRule{{}}, + }, + }, + RuleBackendSources: [][]providerir.BackendSource{ + { + {Ingress: &ingress}, + }, + }, }, - Hostnames: []gatewayv1.Hostname{"example.com"}, }, } - pIR := providerir.ProviderIR{HTTPRoutes: map[types.NamespacedName]providerir.HTTPRouteContext{}} - pIR.HTTPRoutes[key] = providerir.HTTPRouteContext{ - HTTPRoute: route, - RuleBackendSources: [][]providerir.BackendSource{{ - {Ingress: &ing}, - }}, - } - - eIR := emitterir.EmitterIR{HTTPRoutes: map[types.NamespacedName]emitterir.HTTPRouteContext{}} - eIR.HTTPRoutes[key] = emitterir.HTTPRouteContext{HTTPRoute: route} - - addDefaultSSLRedirect(&pIR, &eIR) + errs := redirectFeature([]networkingv1.Ingress{ingress}, nil, &ir) - redirectKey := types.NamespacedName{Namespace: key.Namespace, Name: key.Name + "-ssl-redirect"} - if _, ok := eIR.HTTPRoutes[redirectKey]; ok { - t.Fatalf("did not expect redirect route %v to be added", redirectKey) - } - - origCtx := eIR.HTTPRoutes[key] - if len(origCtx.Spec.ParentRefs) != 1 { - t.Fatalf("expected 1 parentRef, got %#v", origCtx.Spec.ParentRefs) - } - if origCtx.Spec.ParentRefs[0].Port != nil { - t.Fatalf("expected original route parentRef port to remain nil, got %#v", origCtx.Spec.ParentRefs[0].Port) + if len(errs) == 0 { + t.Errorf("Expected error for empty redirect URL") } }