From 24450ea509b20937f1e6b7c6010d892cba80f9ac Mon Sep 17 00:00:00 2001 From: lou-lan Date: Sat, 24 Aug 2024 04:10:20 +0800 Subject: [PATCH] Add custom code handling for temporal redirect (#10651) Co-authored-by: Ricardo Katz --- .../nginx-configuration/annotations-risk.md | 1 + .../nginx-configuration/annotations.md | 5 ++ .../ingress/annotations/redirect/redirect.go | 23 ++++++++- .../annotations/redirect/redirect_test.go | 47 +++++++++++++++++-- 4 files changed, 70 insertions(+), 6 deletions(-) diff --git a/docs/user-guide/nginx-configuration/annotations-risk.md b/docs/user-guide/nginx-configuration/annotations-risk.md index b538601742..890608b4b2 100755 --- a/docs/user-guide/nginx-configuration/annotations-risk.md +++ b/docs/user-guide/nginx-configuration/annotations-risk.md @@ -108,6 +108,7 @@ | Redirect | permanent-redirect | Medium | location | | Redirect | permanent-redirect-code | Low | location | | Redirect | temporal-redirect | Medium | location | +| Redirect | temporal-redirect-code | Low | location | | Rewrite | app-root | Medium | location | | Rewrite | force-ssl-redirect | Medium | location | | Rewrite | preserve-trailing-slash | Medium | location | diff --git a/docs/user-guide/nginx-configuration/annotations.md b/docs/user-guide/nginx-configuration/annotations.md index 34c5f18d5e..7de05d32c1 100755 --- a/docs/user-guide/nginx-configuration/annotations.md +++ b/docs/user-guide/nginx-configuration/annotations.md @@ -71,6 +71,7 @@ You can add these Kubernetes annotations to specific Ingress objects to customiz |[nginx.ingress.kubernetes.io/permanent-redirect](#permanent-redirect)|string| |[nginx.ingress.kubernetes.io/permanent-redirect-code](#permanent-redirect-code)|number| |[nginx.ingress.kubernetes.io/temporal-redirect](#temporal-redirect)|string| +|[nginx.ingress.kubernetes.io/temporal-redirect-code](#temporal-redirect-code)|number| |[nginx.ingress.kubernetes.io/preserve-trailing-slash](#server-side-https-enforcement-through-redirect)|"true" or "false"| |[nginx.ingress.kubernetes.io/proxy-body-size](#custom-max-body-size)|string| |[nginx.ingress.kubernetes.io/proxy-cookie-domain](#proxy-cookie-domain)|string| @@ -610,6 +611,10 @@ This annotation allows you to modify the status code used for permanent redirect ### Temporal Redirect This annotation allows you to return a temporal redirect (Return Code 302) instead of sending data to the upstream. For example `nginx.ingress.kubernetes.io/temporal-redirect: https://www.google.com` would redirect everything to Google with a Return Code of 302 (Moved Temporarily) +### Temporal Redirect Code + +This annotation allows you to modify the status code used for temporal redirects. For example `nginx.ingress.kubernetes.io/temporal-redirect-code: '307'` would return your temporal-redirect with a 307. + ### SSL Passthrough The annotation `nginx.ingress.kubernetes.io/ssl-passthrough` instructs the controller to send TLS connections directly diff --git a/internal/ingress/annotations/redirect/redirect.go b/internal/ingress/annotations/redirect/redirect.go index e774b2fe89..0716e1ce15 100644 --- a/internal/ingress/annotations/redirect/redirect.go +++ b/internal/ingress/annotations/redirect/redirect.go @@ -28,7 +28,10 @@ import ( "k8s.io/ingress-nginx/internal/ingress/resolver" ) -const defaultPermanentRedirectCode = http.StatusMovedPermanently +const ( + defaultPermanentRedirectCode = http.StatusMovedPermanently + defaultTemporalRedirectCode = http.StatusFound +) // Config returns the redirect configuration for an Ingress rule type Config struct { @@ -40,6 +43,7 @@ type Config struct { const ( fromToWWWRedirAnnotation = "from-to-www-redirect" temporalRedirectAnnotation = "temporal-redirect" + temporalRedirectAnnotationCode = "temporal-redirect-code" permanentRedirectAnnotation = "permanent-redirect" permanentRedirectAnnotationCode = "permanent-redirect-code" ) @@ -60,6 +64,12 @@ var redirectAnnotations = parser.Annotation{ Documentation: `This annotation allows you to return a temporal redirect (Return Code 302) instead of sending data to the upstream. For example setting this annotation to https://www.google.com would redirect everything to Google with a Return Code of 302 (Moved Temporarily).`, }, + temporalRedirectAnnotationCode: { + Validator: parser.ValidateInt, + Scope: parser.AnnotationScopeLocation, + Risk: parser.AnnotationRiskLow, // Low, as it allows just a set of options + Documentation: `This annotation allows you to modify the status code used for temporal redirects.`, + }, permanentRedirectAnnotation: { Validator: parser.ValidateRegex(parser.URLIsValidRegex, false), Scope: parser.AnnotationScopeLocation, @@ -105,13 +115,22 @@ func (r redirect) Parse(ing *networking.Ingress) (interface{}, error) { } if tr != "" { + trc, err := parser.GetIntAnnotation(temporalRedirectAnnotationCode, ing, r.annotationConfig.Annotations) + if err != nil && !errors.IsMissingAnnotations(err) { + return nil, err + } + + if trc < http.StatusMultipleChoices || trc > http.StatusTemporaryRedirect { + trc = defaultTemporalRedirectCode + } + if err := isValidURL(tr); err != nil { return nil, err } return &Config{ URL: tr, - Code: http.StatusFound, + Code: trc, FromToWWW: r3w, }, nil } diff --git a/internal/ingress/annotations/redirect/redirect_test.go b/internal/ingress/annotations/redirect/redirect_test.go index bd2f98211f..b5c34879e2 100644 --- a/internal/ingress/annotations/redirect/redirect_test.go +++ b/internal/ingress/annotations/redirect/redirect_test.go @@ -103,7 +103,7 @@ func TestPermanentRedirectWithCustomCode(t *testing.T) { } } -func TestTemporalRedirect(t *testing.T) { +func TestTemporalRedirectWithDefaultCode(t *testing.T) { rp := NewParser(resolver.Mock{}) if rp == nil { t.Fatalf("Expected a parser.IngressAnnotation but returned nil") @@ -128,10 +128,49 @@ func TestTemporalRedirect(t *testing.T) { t.Errorf("Expected %v as redirect but returned %s", defRedirectURL, redirect.URL) } if redirect.Code != http.StatusFound { - t.Errorf("Expected %v as redirect to have a code %d but had %d", defRedirectURL, defaultPermanentRedirectCode, redirect.Code) + t.Errorf("Expected %v as redirect to have a code %d but had %d", defRedirectURL, http.StatusFound, redirect.Code) + } +} + +func TestTemporalRedirectWithCustomCode(t *testing.T) { + rp := NewParser(resolver.Mock{}) + if rp == nil { + t.Fatalf("Expected a parser.IngressAnnotation but returned nil") + } + + testCases := map[string]struct { + input int + expectOutput int + }{ + "valid code": {http.StatusTemporaryRedirect, http.StatusTemporaryRedirect}, + "invalid code": {http.StatusTeapot, http.StatusFound}, } - if redirect.FromToWWW != true { - t.Errorf("Expected %v as redirect to have from-to-www as %v but got %v", defRedirectURL, true, redirect.FromToWWW) + + for n, tc := range testCases { + t.Run(n, func(t *testing.T) { + ing := new(networking.Ingress) + + data := make(map[string]string, 2) + data[parser.GetAnnotationWithPrefix(fromToWWWRedirAnnotation)] = "true" + data[parser.GetAnnotationWithPrefix(temporalRedirectAnnotation)] = defRedirectURL + data[parser.GetAnnotationWithPrefix(temporalRedirectAnnotationCode)] = strconv.Itoa(tc.input) + ing.SetAnnotations(data) + + i, err := rp.Parse(ing) + if err != nil { + t.Errorf("Unexpected error with ingress: %v", err) + } + redirect, ok := i.(*Config) + if !ok { + t.Errorf("Expected a Redirect type") + } + if redirect.URL != defRedirectURL { + t.Errorf("Expected %v as redirect but returned %s", defRedirectURL, redirect.URL) + } + if redirect.Code != tc.expectOutput { + t.Errorf("Expected %v as redirect to have a code %d but had %d", defRedirectURL, tc.expectOutput, redirect.Code) + } + }) } }