From 3872ea8d18dc38f9a9acb22ea488524fe41ec689 Mon Sep 17 00:00:00 2001 From: Michael Date: Mon, 2 Mar 2026 14:12:05 +0100 Subject: [PATCH] Fix custom error pages behavior for ingress-nginx provider --- pkg/middlewares/customerrors/custom_errors.go | 8 +- .../customerrors/custom_errors_test.go | 79 +++++++++++++++++++ .../kubernetes/ingress-nginx/kubernetes.go | 6 +- .../ingress-nginx/kubernetes_test.go | 59 ++++++++++++++ 4 files changed, 148 insertions(+), 4 deletions(-) diff --git a/pkg/middlewares/customerrors/custom_errors.go b/pkg/middlewares/customerrors/custom_errors.go index f0550edb3b..6eda509759 100644 --- a/pkg/middlewares/customerrors/custom_errors.go +++ b/pkg/middlewares/customerrors/custom_errors.go @@ -160,8 +160,12 @@ func (c *customErrors) ServeHTTP(rw http.ResponseWriter, req *http.Request) { utils.CopyHeaders(pageReq.Header, req.Header) } - c.backendHandler.ServeHTTP(newCodeModifier(rw, code), - pageReq.WithContext(req.Context())) + if len(c.forwardNginxHeaders) > 0 { + c.backendHandler.ServeHTTP(rw, pageReq.WithContext(req.Context())) + } else { + c.backendHandler.ServeHTTP(newCodeModifier(rw, code), + pageReq.WithContext(req.Context())) + } } func newRequest(baseURL string) (*http.Request, error) { diff --git a/pkg/middlewares/customerrors/custom_errors_test.go b/pkg/middlewares/customerrors/custom_errors_test.go index ec8eff1536..ed49cf13dd 100644 --- a/pkg/middlewares/customerrors/custom_errors_test.go +++ b/pkg/middlewares/customerrors/custom_errors_test.go @@ -154,6 +154,85 @@ func TestHandler(t *testing.T) { assert.Contains(t, recorder.Body.String(), "My 503 page.") }, }, + { + desc: "nginx headers: backend status code preserved", + errorPage: &dynamic.ErrorPage{ + Service: "error", + Query: "/test", + Status: []string{"500-599"}, + NginxHeaders: &http.Header{ + "X-Namespaces": {"default"}, + "X-Ingress-Name": {"my-ingress"}, + "X-Service-Name": {"my-service"}, + "X-Service-Port": {"80"}, + }, + }, + backendCode: http.StatusInternalServerError, + backendErrorHandler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Error page backend returns 200 (the default when no WriteHeader is called). + _, _ = fmt.Fprintln(w, "Custom error page.") + }), + validate: func(t *testing.T, recorder *httptest.ResponseRecorder) { + t.Helper() + // In nginx mode, the error page backend's status code (200) is preserved, + // NOT overridden to the original error code (500). + assert.Equal(t, http.StatusOK, recorder.Code, "HTTP status") + assert.Contains(t, recorder.Body.String(), "Custom error page.") + }, + }, + { + desc: "nginx headers: X-Code and nginx headers forwarded", + errorPage: &dynamic.ErrorPage{ + Service: "error", + Query: "/test", + Status: []string{"500-599"}, + NginxHeaders: &http.Header{ + "X-Namespaces": {"default"}, + "X-Ingress-Name": {"my-ingress"}, + "X-Service-Name": {"my-service"}, + "X-Service-Port": {"80"}, + }, + }, + backendCode: http.StatusBadGateway, + backendErrorHandler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Verify that nginx-specific headers are set on the request. + assert.Equal(t, "502", r.Header.Get("X-Code")) + assert.Equal(t, "default", r.Header.Get("X-Namespaces")) + assert.Equal(t, "my-ingress", r.Header.Get("X-Ingress-Name")) + assert.Equal(t, "my-service", r.Header.Get("X-Service-Name")) + assert.Equal(t, "80", r.Header.Get("X-Service-Port")) + assert.Equal(t, "/test?foo=bar&baz=buz", r.Header.Get("X-Original-Uri")) + // Return a custom status code. + w.WriteHeader(http.StatusNotFound) + _, _ = fmt.Fprintln(w, "Custom 404 page.") + }), + validate: func(t *testing.T, recorder *httptest.ResponseRecorder) { + t.Helper() + // Backend's chosen status code (404) is preserved in nginx mode. + assert.Equal(t, http.StatusNotFound, recorder.Code, "HTTP status") + assert.Contains(t, recorder.Body.String(), "Custom 404 page.") + }, + }, + { + desc: "non-nginx: code modifier enforces original error code", + errorPage: &dynamic.ErrorPage{ + Service: "error", + Query: "/test", + Status: []string{"500-599"}, + }, + backendCode: http.StatusInternalServerError, + backendErrorHandler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Error page backend returns 200 (the default). + _, _ = fmt.Fprintln(w, "Custom error page.") + }), + validate: func(t *testing.T, recorder *httptest.ResponseRecorder) { + t.Helper() + // Without nginx headers, newCodeModifier enforces the original error code (500), + // even though the error page backend returned 200. + assert.Equal(t, http.StatusInternalServerError, recorder.Code, "HTTP status") + assert.Contains(t, recorder.Body.String(), "Custom error page.") + }, + }, } for _, test := range testCases { diff --git a/pkg/provider/kubernetes/ingress-nginx/kubernetes.go b/pkg/provider/kubernetes/ingress-nginx/kubernetes.go index bf2dc51b38..b0b052a1b1 100644 --- a/pkg/provider/kubernetes/ingress-nginx/kubernetes.go +++ b/pkg/provider/kubernetes/ingress-nginx/kubernetes.go @@ -1014,8 +1014,6 @@ func (p *Provider) applyCustomHTTPErrors(namespace, ingressName, routerName stri return errors.New("targeted ingress backend has no service") } - // TODO: here we always use the default backend as a fallback, but it is not guaranteed to be created, - // so we should check if it exists before and create a dummy service if not, which is too complicated to check without pre computed model. serviceName := defaultBackendName if defaultBackend := ptr.Deref(config.DefaultBackend, ""); defaultBackend != "" { backend := netv1.IngressBackend{Service: &netv1.IngressServiceBackend{Name: defaultBackend}} @@ -1026,6 +1024,10 @@ func (p *Provider) applyCustomHTTPErrors(namespace, ingressName, routerName stri serviceName = fmt.Sprintf("default-backend-%s", routerName) conf.HTTP.Services[serviceName] = service + } else if _, ok := conf.HTTP.Services[defaultBackendName]; !ok { + // No default backend available (no annotation and no global default). + // Skip the middleware — matches nginx behavior where errors pass through. + return nil } k8sServiceName := targetedService.Service.Name diff --git a/pkg/provider/kubernetes/ingress-nginx/kubernetes_test.go b/pkg/provider/kubernetes/ingress-nginx/kubernetes_test.go index b8379dbef6..d73fa5297b 100644 --- a/pkg/provider/kubernetes/ingress-nginx/kubernetes_test.go +++ b/pkg/provider/kubernetes/ingress-nginx/kubernetes_test.go @@ -3335,6 +3335,65 @@ func TestLoadIngresses(t *testing.T) { TLS: &dynamic.TLSConfiguration{}, }, }, + { + desc: "Custom HTTP Errors without default backend", + paths: []string{ + "services.yml", + "ingressclasses.yml", + "ingresses/ingress-with-custom-http-errors.yml", + }, + expected: &dynamic.Configuration{ + TCP: &dynamic.TCPConfiguration{ + Routers: map[string]*dynamic.TCPRouter{}, + Services: map[string]*dynamic.TCPService{}, + }, + HTTP: &dynamic.HTTPConfiguration{ + Routers: map[string]*dynamic.Router{ + "default-ingress-with-custom-http-errors-rule-0-path-0": { + Rule: "Host(`whoami.localhost`) && Path(`/`)", + RuleSyntax: "default", + Service: "default-ingress-with-custom-http-errors-whoami-80", + Middlewares: []string{"default-ingress-with-custom-http-errors-rule-0-path-0-retry"}, + }, + }, + Middlewares: map[string]*dynamic.Middleware{ + "default-ingress-with-custom-http-errors-rule-0-path-0-retry": { + Retry: &dynamic.Retry{Attempts: 3}, + }, + }, + Services: map[string]*dynamic.Service{ + "default-ingress-with-custom-http-errors-whoami-80": { + LoadBalancer: &dynamic.ServersLoadBalancer{ + Servers: []dynamic.Server{ + { + URL: "http://10.10.0.1:80", + }, + { + URL: "http://10.10.0.2:80", + }, + }, + Strategy: "wrr", + PassHostHeader: ptr.To(true), + ResponseForwarding: &dynamic.ResponseForwarding{ + FlushInterval: dynamic.DefaultFlushInterval, + }, + ServersTransport: "default-ingress-with-custom-http-errors", + }, + }, + }, + ServersTransports: map[string]*dynamic.ServersTransport{ + "default-ingress-with-custom-http-errors": { + ForwardingTimeouts: &dynamic.ForwardingTimeouts{ + DialTimeout: ptypes.Duration(60 * time.Second), + ReadTimeout: ptypes.Duration(60 * time.Second), + WriteTimeout: ptypes.Duration(60 * time.Second), + }, + }, + }, + }, + TLS: &dynamic.TLSConfiguration{}, + }, + }, { desc: "Default backend annotation", paths: []string{