Add override for proxy_intercept_errors when using Custom HTTP Errors

[chriss-de]

This PR adds a new flag to the global ConfigMap configurations and Ingress Annotations to disable the proxy_intercept_errors setting in nginx.conf when using CustomHTTPErrors.

What this PR does / why we need it:

The motivation behind this change is to enable the use case outlined in #8376. We would like to be able to override the default nginx error pages (e.g. 503 service unavailable and 404 not found) with a custom error page without intercepting genuine error responses from proxied services.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• CVE Report (Scanner found CVE and adding report)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation only

Which issue/s this PR fixes

fixes #8376
solves #9026
solves #8435
solves #9211

How Has This Been Tested?

Added unit tests to cover this change. Tested in our datacenter and confirmed fix by applying annotations for proxy-intercept-errors and custom-http-errors.

Checklist:

• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I've read the CONTRIBUTION guide
• I have added unit and/or e2e tests to cover my changes.
• All new and existing tests passed.
• Added Release Notes.

Does my pull request need a release note?

Added annotation to control if proxy-intercept-errors are enabled when custom-http-errors are enabled.
Added configmap option to control if proxy-intercept-errors are enabled when custom-http-errors are enabled in configmap.

[k8s-ci-robot]

@chriss-de: 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.

[k8s-ci-robot]

Hi @chriss-de. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

[chriss-de]

My apologies for creating a new PR on this matter.
In my previous PR #9411 I made a fatal mistake by merging instead of rebase to main.

[chriss-de]

/retest

[k8s-ci-robot]

@chriss-de: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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.

[chriss-de]

I have reversed the logic so its more clear what you are doing.
Now you have to disable proxy-intercept-errors.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: chriss-de (cc0872c, ae2d6d4, 0a5708b, e87fd22, ef17381, 0c86dac, 23dac1d, 621f892, de01686, 892a3b6, d706f1e, 4ab1ab2, 4ecc6c4, bb1175e, 2d134cd, 1ea61f1, b78728f, ed67cbd, 59c49c3, 8f066f0, c252a1e, 35deadb, a0d988f, 04f52f2, 4c28b9e, fc4d1f0, cd5452c, f05d795, c18544a, 37ea3d6, 4c36f94, 05f6001, ad923aa, 4cfda9e, 452436f, 06c1bba, d1e34d2, 4763069, 0fd5b19, 874e89c)

[chriss-de]

@tao12345666333 ok?

[tao12345666333]

thanks, let me take a look

[RuStyC0der]

Hi @chriss-de , many thanks for fixing that linting issue and also thanks @tao12345666333 for taking resposibility to rwiew this code. Merging it wil help us so much)

[tao12345666333]

Sorry for the delay, left some comments.

/ok-to-test

[tao12345666333]

Could you please resolve the comments and conflicts? Thanks

[sboardwell]

Hi @tao12345666333, is there anything stopping this being merged now?

I don't want to be unappreciative as I'm sure you've got a lot on your plate but, from what I see, @chriss-de is doing as much as possible to move this forward and replied to your comments "25 mins" after you posted them.

It just seems like this issue is looked at once every 1-2 weeks regardless of activity on the PR. Would it help if we were to ping you when we see movement? I've held back up until now because I didn't want to start spamming 😅 but I could keep an eye on it and give you a gentle reminder if it helps.

Again, no animosity here as I'm sure you are really busy. Just trying to help move this along.

[tao12345666333]

Thank you for your reminder.

Before this PR was rebased, I was just waiting for the rebase.

[tao12345666333]

I will do a final review today.

Thank you all.

[tao12345666333]

Thanks

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chriss-de, tao12345666333

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [tao12345666333]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[sboardwell]

thank you @tao12345666333 - Last question, do you or @strongjz know when the next release will be coming out? Looking forward to this feature 💪

[strongjz]

/cherry-pick release-1.9

[k8s-infra-cherrypick-robot]

@strongjz: #9497 failed to apply on top of branch "release-1.9":

Applying: added proxy-intercept-errors config option
Using index info to reconstruct a base tree...
M	docs/user-guide/nginx-configuration/annotations.md
M	docs/user-guide/nginx-configuration/configmap.md
M	internal/ingress/annotations/annotations.go
M	internal/ingress/controller/config/config.go
M	internal/ingress/defaults/main.go
M	pkg/apis/ingress/types.go
M	pkg/apis/ingress/types_equals.go
M	rootfs/etc/nginx/template/nginx.tmpl
Falling back to patching base and 3-way merge...
Auto-merging rootfs/etc/nginx/template/nginx.tmpl
Auto-merging pkg/apis/ingress/types_equals.go
Auto-merging pkg/apis/ingress/types.go
Auto-merging internal/ingress/defaults/main.go
Auto-merging internal/ingress/controller/config/config.go
Auto-merging internal/ingress/annotations/annotations.go
CONFLICT (content): Merge conflict in internal/ingress/annotations/annotations.go
Auto-merging docs/user-guide/nginx-configuration/configmap.md
CONFLICT (content): Merge conflict in docs/user-guide/nginx-configuration/configmap.md
Auto-merging docs/user-guide/nginx-configuration/annotations.md
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 added proxy-intercept-errors config option
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release-1.9

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.

[chriss-de]

Is that sth I have to fix?

[sboardwell]

@strongjz not sure if this has slipped under the radar. Does anything need to be done on this PR?

[chriss-de]

@sboardwell I checked - its in main/master - maybe not release 1.9 but its merged

[tao12345666333]

Let me do the cherry-picking manually

[Richard87]

Hi @tao12345666333 We just updated to 1.9.5, was it supposed to be included here?

[Abbas-b-b]

@Richard87 I believe it's included but I think it's not working as expected. Was anyone able to apply this on v1.9.5?

[emelyanovvictor]

@Abbas-b-b I also couldn’t get this functionality on either 1.9.5 or 1.9.6.

[Abbas-b-b]

@emelyanovvictor It has been removed in 1.9.6 and 4.9.1. #10832 #10919