Skip to content

Conversation

svcAPLBot
Copy link
Contributor

@svcAPLBot svcAPLBot commented Aug 13, 2025

This PR updates the dependency ingress-nginx to version 4.13.1.

It also includes a number of changes, partially as a cleanup and for compatibility:

  • Many commented configuration lines, upstream defaults, and deprecated (or otherwise non-functional) options were removed.
  • The controller scope was extended to its own namespace (using label selector), as errors were logged accordingly even before this upgrade.
  • proxy-busy-buffers-size was added to override the default. The value needs to be >= proxy-buffer-size which we have increased.
  • Log verbosity was tuned down as this was not any helpful.
  • OpenTelemetry is no longer a sidecar, but the module is included already.
  • Snippets within the Ingress resource are considered a security risk with rating Critical (https://kubernetes.github.io/ingress-nginx/user-guide/nginx-configuration/annotations-risk/) and only permitted after setting configuration options to disable checks. It has turned out that existing snippets were either ineffective due to configuration errors, or in case of OAuth2-Proxy no longer necessary.

@svcAPLBot svcAPLBot added the chart-deps Auto generated helm chart dependencies label Aug 13, 2025
svcAPLBot and others added 27 commits August 13, 2025 10:53
@merll merll marked this pull request as ready for review August 28, 2025 10:51
@@ -59,31 +42,21 @@ controller:
enable-modsecurity: {{ $app.modsecurity.enabled }}
enable-owasp-modsecurity-crs: {{ $app.modsecurity.owasp }}
hsts: true
http2-max-field-size: 64k
http2-max-header-size: 128k
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there defaults for these and if so what are they? Is it safe to remove these?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are deprecated, and have to be removed because otherwise the controller will not start. Functionality has entirely replaced with large-client-header-buffers. Due to this replacement, these settings must have been without an effect for a while. Therefore I did not update the replacement option, seeing no negative effects in the past.

Source: https://nginx.org/en/docs/http/ngx_http_v2_module.html#http2_max_field_size

Copy link
Contributor

@CasLubbers CasLubbers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on cluster created workloads with some services and all worked. Couldn't find regression ✅

Copy link
Contributor

@CasLubbers CasLubbers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was a bit to soon with my approval. When I press log-out. It redirects to:
https://auth.caslubbers-1.dev-akamai-apl.net/oauth2/start?rd=https://console.caslubbers-1.dev-akamai-apl.net%2Fapps%2Fadmin
And I see:
default backend - 404
This was on a cluster that was on main and then upgraded to this branch

edit: it keeps redirecting to that page so I cannot log back and I am basically stuck

@merll merll requested a review from ferruhcihan as a code owner September 1, 2025 14:31
@merll merll requested a review from CasLubbers September 1, 2025 15:23
@merll
Copy link
Contributor

merll commented Sep 1, 2025

I was a bit to soon with my approval. When I press log-out. It redirects to: https://auth.caslubbers-1.dev-akamai-apl.net/oauth2/start?rd=https://console.caslubbers-1.dev-akamai-apl.net%2Fapps%2Fadmin And I see: default backend - 404 This was on a cluster that was on main and then upgraded to this branch

edit: it keeps redirecting to that page so I cannot log back and I am basically stuck

Tested different approaches to fix this, and added a post-upgrade script to run Helmfile with that one ingress that does not get updated.

Copy link
Contributor

@CasLubbers CasLubbers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested upgrade on cluster again and worked without issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chart-deps Auto generated helm chart dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants