From d6b127ba91fd50229f537bdb0294072a46d64d22 Mon Sep 17 00:00:00 2001 From: Michael Date: Thu, 4 Dec 2025 13:44:05 +0100 Subject: [PATCH] Fix SSL redirect middleware to match NGINX behavior --- .../kubernetes-crd-definition-v1.yml | 5 ++- .../traefik.io_middlewares.yaml | 5 ++- integration/fixtures/k8s/01-traefik-crd.yml | 5 ++- pkg/config/dynamic/middlewares.go | 7 ++- pkg/middlewares/redirect/redirect.go | 44 +++++++++++-------- pkg/middlewares/redirect/redirect_regex.go | 2 +- pkg/middlewares/redirect/redirect_scheme.go | 2 +- .../redirect/redirect_scheme_test.go | 21 +++++++++ .../kubernetes/ingress-nginx/kubernetes.go | 4 +- .../ingress-nginx/kubernetes_test.go | 8 ++-- 10 files changed, 70 insertions(+), 33 deletions(-) diff --git a/docs/content/reference/dynamic-configuration/kubernetes-crd-definition-v1.yml b/docs/content/reference/dynamic-configuration/kubernetes-crd-definition-v1.yml index 1f2ef1382..ef7c35ea0 100644 --- a/docs/content/reference/dynamic-configuration/kubernetes-crd-definition-v1.yml +++ b/docs/content/reference/dynamic-configuration/kubernetes-crd-definition-v1.yml @@ -2041,8 +2041,9 @@ spec: More info: https://doc.traefik.io/traefik/v3.6/middlewares/http/redirectscheme/ properties: permanent: - description: Permanent defines whether the redirection is permanent - (308). + description: |- + Permanent defines whether the redirection is permanent. + For HTTP GET requests a 301 is returned, otherwise a 308 is returned. type: boolean port: description: Port defines the port of the new URL. diff --git a/docs/content/reference/dynamic-configuration/traefik.io_middlewares.yaml b/docs/content/reference/dynamic-configuration/traefik.io_middlewares.yaml index 68b9f14f1..7db8a0aed 100644 --- a/docs/content/reference/dynamic-configuration/traefik.io_middlewares.yaml +++ b/docs/content/reference/dynamic-configuration/traefik.io_middlewares.yaml @@ -1211,8 +1211,9 @@ spec: More info: https://doc.traefik.io/traefik/v3.6/middlewares/http/redirectscheme/ properties: permanent: - description: Permanent defines whether the redirection is permanent - (308). + description: |- + Permanent defines whether the redirection is permanent. + For HTTP GET requests a 301 is returned, otherwise a 308 is returned. type: boolean port: description: Port defines the port of the new URL. diff --git a/integration/fixtures/k8s/01-traefik-crd.yml b/integration/fixtures/k8s/01-traefik-crd.yml index 1f2ef1382..ef7c35ea0 100644 --- a/integration/fixtures/k8s/01-traefik-crd.yml +++ b/integration/fixtures/k8s/01-traefik-crd.yml @@ -2041,8 +2041,9 @@ spec: More info: https://doc.traefik.io/traefik/v3.6/middlewares/http/redirectscheme/ properties: permanent: - description: Permanent defines whether the redirection is permanent - (308). + description: |- + Permanent defines whether the redirection is permanent. + For HTTP GET requests a 301 is returned, otherwise a 308 is returned. type: boolean port: description: Port defines the port of the new URL. diff --git a/pkg/config/dynamic/middlewares.go b/pkg/config/dynamic/middlewares.go index 466b62c52..fa10c302b 100644 --- a/pkg/config/dynamic/middlewares.go +++ b/pkg/config/dynamic/middlewares.go @@ -655,8 +655,13 @@ type RedirectScheme struct { Scheme string `json:"scheme,omitempty" toml:"scheme,omitempty" yaml:"scheme,omitempty" export:"true"` // Port defines the port of the new URL. Port string `json:"port,omitempty" toml:"port,omitempty" yaml:"port,omitempty" export:"true"` - // Permanent defines whether the redirection is permanent (308). + // Permanent defines whether the redirection is permanent. + // For HTTP GET requests a 301 is returned, otherwise a 308 is returned. Permanent bool `json:"permanent,omitempty" toml:"permanent,omitempty" yaml:"permanent,omitempty" export:"true"` + // ForcePermanentRedirect is an internal field (not exposed in configuration). + // When set to true, this forces the use of permanent redirects 308, regardless of the request method. + // Used by the provider ingress-ngin. + ForcePermanentRedirect bool `json:"-" toml:"-" yaml:"-" label:"-"` } // +k8s:deepcopy-gen=true diff --git a/pkg/middlewares/redirect/redirect.go b/pkg/middlewares/redirect/redirect.go index d8499d1aa..503df4419 100644 --- a/pkg/middlewares/redirect/redirect.go +++ b/pkg/middlewares/redirect/redirect.go @@ -18,30 +18,32 @@ const typeName = "Redirect" var uriRegexp = regexp.MustCompile(`^(https?):\/\/(\[[\w:.]+\]|[\w\._-]+)?(:\d+)?(.*)$`) type redirect struct { - next http.Handler - regex *regexp.Regexp - replacement string - permanent bool - errHandler utils.ErrorHandler - name string - rawURL func(*http.Request) string + next http.Handler + regex *regexp.Regexp + replacement string + permanent bool + forcePermanentRedirect bool + errHandler utils.ErrorHandler + name string + rawURL func(*http.Request) string } // New creates a Redirect middleware. -func newRedirect(next http.Handler, regex, replacement string, permanent bool, rawURL func(*http.Request) string, name string) (http.Handler, error) { +func newRedirect(next http.Handler, regex, replacement string, permanent bool, forcePermanentRedirect bool, rawURL func(*http.Request) string, name string) (http.Handler, error) { re, err := regexp.Compile(regex) if err != nil { return nil, err } return &redirect{ - regex: re, - replacement: replacement, - permanent: permanent, - errHandler: utils.DefaultHandler, - next: next, - name: name, - rawURL: rawURL, + regex: re, + replacement: replacement, + permanent: permanent, + forcePermanentRedirect: forcePermanentRedirect, + errHandler: utils.DefaultHandler, + next: next, + name: name, + rawURL: rawURL, }, nil } @@ -69,7 +71,7 @@ func (r *redirect) ServeHTTP(rw http.ResponseWriter, req *http.Request) { } if newURL != oldURL { - handler := &moveHandler{location: parsedURL, permanent: r.permanent} + handler := &moveHandler{location: parsedURL, permanent: r.permanent, forcePermanentRedirect: r.forcePermanentRedirect} handler.ServeHTTP(rw, req) return } @@ -82,8 +84,9 @@ func (r *redirect) ServeHTTP(rw http.ResponseWriter, req *http.Request) { } type moveHandler struct { - location *url.URL - permanent bool + location *url.URL + permanent bool + forcePermanentRedirect bool } func (m *moveHandler) ServeHTTP(rw http.ResponseWriter, req *http.Request) { @@ -100,6 +103,11 @@ func (m *moveHandler) ServeHTTP(rw http.ResponseWriter, req *http.Request) { status = http.StatusPermanentRedirect } } + + if m.forcePermanentRedirect { + status = http.StatusPermanentRedirect + } + rw.WriteHeader(status) _, err := rw.Write([]byte(http.StatusText(status))) if err != nil { diff --git a/pkg/middlewares/redirect/redirect_regex.go b/pkg/middlewares/redirect/redirect_regex.go index f68493205..5846d5c0d 100644 --- a/pkg/middlewares/redirect/redirect_regex.go +++ b/pkg/middlewares/redirect/redirect_regex.go @@ -17,7 +17,7 @@ func NewRedirectRegex(ctx context.Context, next http.Handler, conf dynamic.Redir logger.Debug().Msg("Creating middleware") logger.Debug().Msgf("Setting up redirection from %s to %s", conf.Regex, conf.Replacement) - return newRedirect(next, conf.Regex, conf.Replacement, conf.Permanent, rawURL, name) + return newRedirect(next, conf.Regex, conf.Replacement, conf.Permanent, false, rawURL, name) } func rawURL(req *http.Request) string { diff --git a/pkg/middlewares/redirect/redirect_scheme.go b/pkg/middlewares/redirect/redirect_scheme.go index b0d8146c7..b1b1e3c3c 100644 --- a/pkg/middlewares/redirect/redirect_scheme.go +++ b/pkg/middlewares/redirect/redirect_scheme.go @@ -40,7 +40,7 @@ func NewRedirectScheme(ctx context.Context, next http.Handler, conf dynamic.Redi rs := &redirectScheme{name: name} - handler, err := newRedirect(next, uriPattern, conf.Scheme+"://${2}"+port+"${4}", conf.Permanent, rs.clientRequestURL, name) + handler, err := newRedirect(next, uriPattern, conf.Scheme+"://${2}"+port+"${4}", conf.Permanent, conf.ForcePermanentRedirect, rs.clientRequestURL, name) if err != nil { return nil, err } diff --git a/pkg/middlewares/redirect/redirect_scheme_test.go b/pkg/middlewares/redirect/redirect_scheme_test.go index aa90bddbc..1e45f5bc7 100644 --- a/pkg/middlewares/redirect/redirect_scheme_test.go +++ b/pkg/middlewares/redirect/redirect_scheme_test.go @@ -165,6 +165,27 @@ func TestRedirectSchemeHandler(t *testing.T) { expectedURL: "https://foo:8443", expectedStatus: http.StatusMovedPermanently, }, + { + desc: "HTTP to HTTPS with explicit 308 status code", + config: dynamic.RedirectScheme{ + Scheme: "https", + ForcePermanentRedirect: true, + }, + url: "http://foo", + expectedURL: "https://foo", + expectedStatus: http.StatusPermanentRedirect, + }, + { + desc: "HTTP to HTTPS with explicit 308 status code for GET request", + method: http.MethodGet, + config: dynamic.RedirectScheme{ + Scheme: "https", + ForcePermanentRedirect: true, + }, + url: "http://foo", + expectedURL: "https://foo", + expectedStatus: http.StatusPermanentRedirect, + }, { desc: "to HTTP 80", config: dynamic.RedirectScheme{ diff --git a/pkg/provider/kubernetes/ingress-nginx/kubernetes.go b/pkg/provider/kubernetes/ingress-nginx/kubernetes.go index c15b4d594..7946c9b5f 100644 --- a/pkg/provider/kubernetes/ingress-nginx/kubernetes.go +++ b/pkg/provider/kubernetes/ingress-nginx/kubernetes.go @@ -968,8 +968,8 @@ func applySSLRedirectConfiguration(routerName string, ingressConfig ingressConfi redirectMiddlewareName := routerName + "-redirect-scheme" conf.HTTP.Middlewares[redirectMiddlewareName] = &dynamic.Middleware{ RedirectScheme: &dynamic.RedirectScheme{ - Scheme: "https", - Permanent: true, + Scheme: "https", + ForcePermanentRedirect: true, }, } redirectRouter.Middlewares = append(redirectRouter.Middlewares, redirectMiddlewareName) diff --git a/pkg/provider/kubernetes/ingress-nginx/kubernetes_test.go b/pkg/provider/kubernetes/ingress-nginx/kubernetes_test.go index 8faba81fb..df325c8a2 100644 --- a/pkg/provider/kubernetes/ingress-nginx/kubernetes_test.go +++ b/pkg/provider/kubernetes/ingress-nginx/kubernetes_test.go @@ -207,14 +207,14 @@ func TestLoadIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{ "default-ingress-with-ssl-redirect-rule-0-path-0-redirect-scheme": { RedirectScheme: &dynamic.RedirectScheme{ - Scheme: "https", - Permanent: true, + Scheme: "https", + ForcePermanentRedirect: true, }, }, "default-ingress-with-force-ssl-redirect-rule-0-path-0-redirect-scheme": { RedirectScheme: &dynamic.RedirectScheme{ - Scheme: "https", - Permanent: true, + Scheme: "https", + ForcePermanentRedirect: true, }, }, },