Feature request: Decouple proxy_intercept_errors from custom-http-errors

[simhnna]

We're using external auth and would like to serve custom error pages to our users if external auth doesn't allow the request through.
We'd like to do this while letting the protected upstream services return their own error pages.

Here's the snippet for the error handling on locations

{{/* if a location-specific error override is set, add the proxy_intercept here */}}
{{ if $location.CustomHTTPErrors }}
# Custom error pages per ingress
proxy_intercept_errors on;
{{ end }}

{{ range $errCode := $location.CustomHTTPErrors }}
error_page {{ $errCode }} = @custom_{{ $location.DefaultBackendUpstreamName }}_{{ $errCode }};{{ end }}

You can't set customHttpErrors without also intercepting all errors. Note that the external auth subrequest apparently doesn't rely on proxy_intercept_errors but uses any defined error_page if available. This is how external auth signin url works.

Is there another way to define an error page for the failure responses of external auth (401, 403, 500) without intercepting the errors of the upstream services?
Currently it looks like our only option is to overwrite the nginx template.

So our proposed feature request: By default errors should be intercepted if you specify custom http errors, but you should be able to override that using something like nginx.ingress.kubernetes.io/proxy-intercept-errors: "off"

[k8s-ci-robot]

@simhnna: This issue is currently awaiting triage.

If Ingress contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[simhnna]

I'm happy to open a pull request if this is accepted

[simhnna]

Turns out there is a pull request for this already. We're testing this internally but would have a couple of change requests #8435