[jschumacher-wire]

Part of https://wearezeta.atlassian.net/browse/WPB-29921

[claude]

Review

This is a small change and it looks correct. Setting ingress.enabled: false stops templates/ingress.yaml from rendering, so the next helm upgrade deletes the nginx Ingress. Nothing else in the chart depends on the Ingress values. The pointer comment to the cailleach config is useful.

Things to check before merging

1. Cutover order and downtime. When this deploys, the nginx Ingress is removed right away. GitHub webhooks and Wire callbacks both go to https://github-app.apps-prod.wire.com (the value of GHAPP_API_HOST). Before this merges, please confirm that:

   • the Envoy Gateway HTTPRoute for github-app.apps-prod.wire.com is already applied and has an accepted parent Gateway,
   • DNS for the hostname already points at the Envoy Gateway load balancer and not the nginx one,
   • the Gateway listener has a valid certificate for that host.

   If any of these is missing, webhook deliveries will fail until it is fixed. GitHub does not retry failed deliveries automatically, so those events would have to be redelivered by hand. A quick check before merging: curl -v --resolve github-app.apps-prod.wire.com:443:<envoy-lb-ip> https://github-app.apps-prod.wire.com/health.

2. Leftover TLS secret. cert-manager does not delete the github-app-tls Secret when the Ingress or Certificate goes away. If the Gateway has its own certificate, you can delete this Secret after the cutover. If the Gateway reuses it, it has to stay, and it would be worth noting that in the comment so nobody removes it later.

3. GHAPP_API_HOST stays the same. That is correct. Keeping the public hostname means the GitHub App webhook URL does not need to change. I'm mentioning it only to confirm it was intentional.

Minor

• ingress.enabled already defaults to false in values.yaml, so the override is technically redundant. I'd still keep it, because it states the prod intent clearly and pairs well with the comment.
• Unrelated to this PR: templates/NOTES.txt says kubectl logs -f deployment/..., but the chart deploys a StatefulSet. That could be fixed in a separate PR.

Tests / security

• The chart has no tests and this change doesn't need any. Running helm template -f values-prod.yaml should show no Ingress resource.
• No security concerns, as long as TLS terminates at the Gateway as it did with nginx and cert-manager.

LGTM once the Gateway route, DNS and certificate are confirmed live 👍