Fix custom error pages behavior for ingress-nginx provider

This commit is contained in:
Michael 2026-03-02 14:12:05 +01:00 committed by GitHub
parent f5efe1e69b
commit 3872ea8d18
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 148 additions and 4 deletions

View File

@ -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) {

View File

@ -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 {

View File

@ -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

View File

@ -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{