Skip to content

WPB-27964 use CSP header of account pages - #5534

Merged
battermann merged 8 commits into
developfrom
WPB-27964-backend-use-csp-header-of-account-pages
Sep 11, 2026
Merged

battermann merged 8 commits into
developfrom
WPB-27964-backend-use-csp-header-of-account-pages

Conversation

@battermann

@battermann battermann commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

should be merged after: https://github.com/zinfra/cailleach/pull/4956

https://wearezeta.atlassian.net/browse/WPB-27964

Checklist

  • Add a new entry in an appropriate subdirectory of changelog.d
  • Read and follow the PR guidelines

@battermann
battermann marked this pull request as ready for review September 10, 2026 13:56
@battermann
battermann requested review from a team as code owners September 10, 2026 13:56
@battermann
battermann requested review from jschaul and a lite review from Copilot September 10, 2026 13:56
@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Sep 10, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

馃煛 Changes recommended

The new ingress snippet references config.dns.accountPages unconditionally, which can render <no value> when account-pages is disabled or unset and should be guarded/validated.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Updates the nginx-ingress-services Helm chart to avoid setting the chart-generated CSP header for the account-pages host, so account-pages can supply its own CSP in multi-ingress setups.

Changes:

  • Skip ingress-level CSP injection when the request host matches config.dns.accountPages.
  • Add a changelog entry documenting the multi-ingress CSP/backend URL fix.
File summaries
File Description
charts/nginx-ingress-services/templates/ingress.yaml Adds host-based CSP-skip logic for account-pages in the nginx configuration snippet.
changelog.d/3-bug-fixes/WPB-27964 Documents the fix for account-pages CSP/backend URL behavior on multi-ingress domains.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite

馃挕 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread charts/nginx-ingress-services/templates/ingress.yaml Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

馃煛 Changes recommended

The Helm template currently contains a quoting error in the new required expression that will break chart rendering.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread charts/nginx-ingress-services/templates/ingress.yaml

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

馃煛 Changes recommended

The integration values omit config.dns.base, causing Helm rendering to fail at the required lookup.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 8/8 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread hack/helm_vars/nginx-ingress-services/values.yaml.gotmpl
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

馃煝 Approval recommended

No unresolved issues were identified.

Review details
  • Files reviewed: 8/8 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@battermann
battermann merged commit 47a8f02 into develop Sep 11, 2026
10 checks passed
@battermann
battermann deleted the WPB-27964-backend-use-csp-header-of-account-pages branch September 11, 2026 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants