fix(helm): Fix setting X-Scope-OrgID header

[jkroepke]

What this PR does / why we need it:

Which issue(s) this PR fixes:

• Fixes Docs: Remove comparisons page from docs #6938
• Fixes Live tailing was stopped due to following error: undefined #7153
• Fixes Unable to fetch labels from Loki (no org id ), please check the server logs for more details #9756
• Fixes Query error Live tailing was stopped due to following error: undefined #11915
• Fixes chore(helm): Fix setting X-Scope-OrgID header for ws #16400
• Fixes [Helm chart 6.29] Upgrading chart to 6.29.0 bricks basic auth functionality and allows reading X-Scope-OrgID not owned by user #16938
• Fixes fix: helm - nginx tenants proxy headers #16953
• Fixes Helm: Multi-tenant support is broken due to changes in Gateway configuration #18003

Special notes for your reviewer:

Checklist

• Reviewed the CONTRIBUTING.md guide (required)
• Documentation added
• Tests updated
• Title matches the required conventional commits format, see here
  • Note that Promtail is considered to be feature complete, and future development for logs collection will be in Grafana Alloy. As such, feat PRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.
• Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
• If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory. Example PR

[Jayclifford345]

Explanation for PR:

Hi,
I've recently re-examined the PRs and observed a persistent bug within the Helm chart that multiple ongoing PRs are attempting to fix, but not entirely successfully.
When multiple tenants are enabled in Loki, the common configuration involves enabling basic authentication and using the HTTP authentication username as the tenant name. In Nginx, this is typically handled by the directive: proxy_set_header X-Scope-OrgID $remote_user;
However, a regression was introduced by PR #9215 from 2023. This PR defines proxy_set_header inside a location block. A crucial point with Nginx is that proxy_set_header directives are only inherited from an upper scope if no proxy_set_header directives are defined at the current level. This inheritance behavior is causing the issue.
Currently, two PRs are attempting to address this:

• #16400 (note: auto-merge is enabled)
• #16953
  Unfortunately, neither of these PRs fully covers the configurations reported by users in the comments of PR #16953 (see comments: 2812724395, 2815967609, 2912339662).
  PR #18105 might also indirectly address this bug, albeit in a more complex manner.
  I would like to inquire if there are any theoretical counter-arguments against universally supporting WebSockets across all Nginx locations (which is typically what happens when Loki is directly exposed to an ingress controller).
  For context, I've opened PR #18414 to address this error by placing all proxy_set_header directives at the same level. Additionally, I've replaced the less-recommended if directives with a map block for better Nginx configuration practices. I believe this is the simplest and most robust approach to resolve the issue, and I would greatly appreciate it if an engineer could review it. (edited)

[trevorwhitney]

Does this mean we can close #16400 and #16953?

[jkroepke]

Yes, both PRs as linked as fixed and should be closed, if this getting merged.

[tongpu]

Am I assuming correctly that this also fixes #18003?

[jkroepke]

Yes!

[daniel-anton]

@trevorwhitney Hello, I was checking in main and 3.6.2 version of loki and cannot see the changes introduced by this PR. Were them reverted ? I'm not sure why them are not visible. Is it because latest production app version is 3.5.5 and changes were introduced to 3.6.0?

[daniel-anton]

@trevorwhitney Hello, I was checking in main and 3.6.2 version of loki and cannot see the changes introduced by this PR. Were them reverted ? I'm not sure why them are not visible. Is it because latest production app version is 3.5.5 and changes were introduced to 3.6.0?

I think, I found out, the changes were reverted by #18800