Add custom code handling for temporal redirect (#10651)

Co-authored-by: Ricardo Katz <[email protected]>

Author: lou-lan

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 |

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

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
 	}

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)
+			}
+		})
 	}
 }