fix(helm): start cleanly on TLS/Redis defaults, move webhook and registry keys to Secrets

[Andrew Grathwohl (agrathwohl)]

Fixes FIRE-322

What was broken and what changed

The chart failed to start in several configurations, including its defaults.

• The generate-certs init container ran as UID 1000 and wrote a certificate key that the firewall image (UID 1001) could not read, so nginx crash-looped on Permission denied. initContainers.certGenerator.securityContext.runAsUser now defaults to 1001.

• Default TLS installs failed because the certificate SAN list was empty. It now always includes the Service name and its DNS forms, plus any configured domains. The cert script runs with set -e, so a failed step stops it.

• tls.existingSecret without tls.certManager could not load certificates because the keys did not match the filenames nginx reads. tls.crt and tls.key always mount as fullchain.pem and privkey.pem. The new tls.remapKeys defaults to true; setting it to false mounts secrets keyed fullchain.pem and privkey.pem without remapping. tls.certManager now only controls the ca.crt projection.

• With Redis enabled, Kubernetes service-link environment variables from a Service named redis set REDIS_PORT and caused a ValueError at startup. enableServiceLinks is now false on the pod.

• The default redis.host: redis failed to connect because nginx's resolver ignores DNS search domains. Bare hosts now render as <host>.<release namespace>.svc.<clusterDomain>. A new wait-for-redis init container holds the pod in Init until Redis accepts TCP connections.

• webhook.authHeader and pathRouting.privateRegistry.apiKey rendered in plain text in the ConfigMap. They move to Secrets, passed as environment variables WEBHOOK_AUTH_HEADER and PRIVATE_REGISTRY_KEY. New existingSecret and existingSecretKey options support existing Secrets for both.

• Default single-replica installs had a PodDisruptionBudget with minAvailable: 1 that blocked node drains. The PDB now only renders when replicaCount > 1, or autoscaling.minReplicas > 1 with autoscaling on.

Upgrade notes

• If you use a TLS secret keyed fullchain.pem/privkey.pem that worked without certManager, set tls.remapKeys: false.
• Single-replica installs no longer include a PDB.
• If you override initContainers.certGenerator.securityContext, its runAsUser must match the firewall UID 1001.

How it was validated

Tested on kind with Kubernetes 1.36.1 and firewall image 2.9.3. The chart on main reproduced the privkey.pem Permission denied, fullchain.pem No such file, and REDIS_PORT ValueError crashes.

Verified all pods reach Ready state for:

• Default install
• tls.existingSecret (served the secret's certificate)
• Secret keyed fullchain.pem/privkey.pem with tls.remapKeys: false
• Redis enabled alongside a Service named redis (3 client connections in redis-cli client list)
• Webhook and private registry configured (values resolved in /app/config.env from Secrets, not the ConfigMap)

Set redis.host to an invalid address and confirmed the pod remained in Init state, logging Waiting for nope.<namespace>.svc.cluster.local/6379.

Passed helm lint, kubeconform against Kubernetes 1.30.0 for the default install, examples, and 4 code paths, and helm unittest with PR #10's 2 test files, 5 of 5 passing.

What is left

• cloudformation/firewall-eks.yaml ChartVersion default is still 0.11.7.
• Basic auth password and private registry username/password remain in the ConfigMap, as moving them requires a firewall image change.

---

Note

Medium Risk
Changes default TLS mounting, Redis connectivity, PDB presence, and secret wiring—upgrades may need tls.remapKeys: false or lose single-replica PDBs, but fixes prior crash-loop defaults.

Overview
Helm chart 0.12.0 fixes several default-install crash paths and tightens how TLS, Redis, and secrets are handled.

Startup and TLS: The cert generator init container runs as UID 1001 so nginx can read privkey.pem, always adds Service DNS SANs (plus clusterDomain), and uses set -e. Existing TLS secrets mount via tls.remapKeys (tls.crt/tls.key → fullchain.pem/privkey.pem by default); tls.certManager now only controls optional ca.crt projection.

Redis: Bare redis.host names are FQDN-qualified for nginx’s resolver, a wait-for-redis init container blocks until TCP is up, and enableServiceLinks: false avoids REDIS_PORT service-link env collisions.

Secrets: webhook.authHeader and pathRouting.privateRegistry.apiKey are removed from the ConfigMap; chart-created or existingSecret values are injected as WEBHOOK_AUTH_HEADER and PRIVATE_REGISTRY_KEY.

Ops: PDB renders only when more than one replica is always running; README documents new values (clusterDomain, Redis host behavior, TLS/PDB notes).

^{Reviewed by Cursor Bugbot for commit 07df1e8. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

^{Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issue.}

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 07df1e8. Configure here.}

[cursor]

Secret updates skip pod rollout

Medium Severity

Moving webhook.authHeader and pathRouting.privateRegistry.apiKey out of the ConfigMap means a Helm upgrade that only changes those values no longer touches checksum/config. Pods keep the old WEBHOOK_AUTH_HEADER and PRIVATE_REGISTRY_KEY until they are recreated, so rotated credentials do not take effect.

Additional Locations (2)

• helm/templates/deployment.yaml#L16-L17
• helm/templates/secret.yaml#L47-L73

 

^{Reviewed by Cursor Bugbot for commit 07df1e8. Configure here.}

[Julian Gruber (juliangruber)]

Is there a way to add CI for this, in order to catch future regressions?

[Andre Coetzee (Andre153)]

This loop has no bound and no opt-out other than redis.enabled. The image treats Redis as optional at runtime: lua/client/cache.lua falls back to the local shared dict when Redis is unreachable, metadata_cache.lua falls back to disk, and /health returns 200 without a Redis check (identical at 2.8.0, 2.9.3 and 2.9.10).

With Redis down, any pod re-created in that window (node drain, eviction, preemption, HPA scale-up) sits at Init:0/3 printing Waiting for redis.<ns>.svc.cluster.local/6379 for the whole outage, so the firewall serves nothing where 0.11.x served from local cache. Rollouts under the default RollingUpdate stall rather than fail. Reproduced the loop in the 2.9.3 image against an unresolvable host: it never exits.

[Andre Coetzee (Andre153)]

The dot/colon test is the only thing deciding whether a value is rewritten, and it is wrong in both directions.

A bare name is always qualified: redis.host: localhost (Redis as an extraContainers sidecar) renders localhost.<ns>.svc.cluster.local in the ConfigMap and in the init gate. Even 127.0.0.1 cannot satisfy the gate, because sidecars render under containers: and start after init containers.

A dotted name is passed through: redis.host: redis-master.redis (a Service in another namespace, the usual way to write it) passes the gate, because bash /dev/tcp resolves with the pod's search domains (verified in the 2.9.3 image with --dns-search), but the generated nginx resolver carries only nameserver lines (utils/resolver.py), so lua-resty-redis gets NXDOMAIN, every request logs Failed to connect to Redis, and the pod is Ready with no cache. That is the Axis defect the README (lines 660-662) now says the gate turns into a stuck rollout.

[Andre Coetzee (Andre153)]

remapKeys: true now applies to every non-self-signed install, which breaks the two existingSecret setups that worked on 0.11.x without certManager:

• a secret keyed fullchain.pem/privkey.pem: the mount now lists tls.crt/tls.key items that do not exist, kubelet refuses the volume with references non-existent secret key, and the rollout sticks in ContainerCreating;
• a standard TLS secret with the values.yaml-offered ssl.cert=/etc/nginx/ssl/tls.crt / ssl.key=/etc/nginx/ssl/tls.key overrides: the ConfigMap still points at tls.crt, the mount now only has the .pem names, and nginx cannot load the certificate (the 2.9.3 config tool honours ssl.cert).

Rendered both on main and on this branch to confirm. The only notice is the upgrade note in the PR body; the README presents remapKeys: false as an option rather than an upgrade step, and the PR is a fix(helm), so nothing in the published chart tells an upgrading 0.11.x user.

[Andre Coetzee (Andre153)]

version goes to 0.12.0 while cloudformation/firewall-eks.yaml ChartVersion and cloudformation/README.md stay at 0.11.7. The release bot's updater requires those to equal the chart version (re.subn count exactly 1, else SystemExit) and exits 1 against this tree. After merge, the next firewall release opens no deployments bump PR and skips the Slack notice, which re-creates the lag this ticket and FIRE-197 are about.

This branch is also based on 0.11.7/2.9.3 and conflicts with main on this line and the comment above it (0.11.9/2.9.10 once #52 lands). After a rebase, 0.12.0 at appVersion 2.9.3 passes every check in #52 and publishes a higher chart carrying an older image than the current 0.11.x.

[Andre Coetzee (Andre153)]

Requesting changes. The credential moves, checksum/secret, the cert-generator fix and the PDB gating render and behave as described. I ran the chart's rendered shapes against the real 2.9.3 and 2.9.10 images: the basic-auth gate returns 401 without credentials and passes with the Secret value, and the discovery daemon picks up the Secret-held password through the config tool's env override.

Four problems, each inline:

• wait-for-redis turns a Redis outage into a firewall outage. The image runs without Redis; the gate never exits.
• The redis.host heuristic is wrong both ways: bare names are always rewritten (sidecar localhost breaks), and dotted partial names pass the gate but nginx cannot resolve them, so the Ready-without-Redis defect survives for <svc>.<ns> values.
• remapKeys: true by default breaks the two TLS setups that worked on 0.11.x; only the PR body says so.
• CloudFormation ChartVersion is left at 0.11.7, which stops the release bot after merge. The branch also conflicts with main and needs a rebase above 0.11.9.