fix(helm): preserve STS serviceName + networkPolicy.egress back-compat#4569
Conversation
Greptile flagged two real upgrade-breaking changes vs the prior chart:
1. statefulset-postgresql spec.serviceName flipped from <name>-postgresql
to <name>-postgresql-headless. spec.serviceName is immutable, so any
existing install would hit 'Forbidden: updates to statefulset spec ...'
on helm upgrade. Revert to the original name (the headless Service in
services.yaml is added alongside, not as a swap).
2. networkPolicy.egress changed from a list to a map ({extraRules, exceptCidrs}),
silently dropping any custom egress list set by existing users. Restore
the original list semantics for networkPolicy.egress and move cloud-metadata
blocking to a sibling top-level field networkPolicy.egressExceptCidrs.
Adds NOTES.txt upgrade-notes entry covering both + the ESO v1→v1beta1 default
flip (functionally a no-op, but worth surfacing).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryMedium Risk Overview Restores backward compatibility for network policy customization by keeping Adds Reviewed by Cursor Bugbot for commit 32f8ca4. Configure here. |
Greptile SummaryThis PR fixes two upgrade-path regressions introduced by #4565: it reverts
Confidence Score: 5/5Safe to merge — all changes are targeted rollbacks of immutable-field renames and value-schema restructuring that would have broken existing installs. The StatefulSet spec.serviceName revert is a straightforward correctness fix for an immutable Kubernetes field; no data or state is affected. The networkPolicy.egress shape change restores backward compatibility and is covered by the updated unit tests (51/51 pass). The only open item is a documentation gap in NOTES.txt for users who briefly ran the intermediate #4565 chart with customized egress.exceptCidrs — a narrow window with minimal real-world exposure given that #4565 targeted staging. helm/sim/templates/NOTES.txt — the upgrade note could more explicitly guide users who customised networkPolicy.egress.exceptCidrs or networkPolicy.egress.extraRules on the short-lived intermediate chart. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
UP[helm upgrade] --> STS_CHECK{spec.serviceName\nimmutable check}
STS_CHECK -- "was -headless\n(broke upgrade)" --> FAIL[❌ Forbidden error]
STS_CHECK -- "reverted to -postgresql\n(this PR)" --> OK[✅ Upgrade succeeds]
subgraph networkPolicy.egress shape
OLD_MAP["#4565: egress:\n exceptCidrs: [...]\n extraRules: [...]"] -- "silent drop\nof user rules" --> BROKEN[❌ user egress lost]
NEW_FLAT["This PR: egress: []\negressExceptCidrs: [...]"] --> COMPAT[✅ back-compat\nwith pre-1.0 shape]
end
Reviews (2): Last reviewed commit: "docs(helm-skill): trim narrative bloat i..." | Re-trigger Greptile |
…bility issue) Audit caught that the main fix in d5c2e8e missed statefulset-copilot-postgres.yaml, which had the identical immutable-field rename from -copilot-postgresql to -copilot-postgresql-headless. Same upgrade-break vector for anyone running copilot.enabled=true on a prior chart version. Mirrors the fix and comment from the main postgresql STS.
- add startupProbe defaults for both postgresql + copilot-postgresql STSs to shield liveness from slow first-boot (pgvector init, WAL replay) - render a dedicated NetworkPolicy for the otel-collector when telemetry.enabled=true (OTLP ingress from app/realtime/copilot, DNS + HTTPS egress for forwarding to external observability backends) - document why copilot + copilot-postgresql intentionally do NOT ship dedicated NetworkPolicies (Redis URL is unknowable at render time) - regression test pins the otel-collector NP at documentIndex 3
The prior test claimed coverage of both app and realtime NPs but only asserted documentIndex 0. Split into two tests so a regression that drops custom egress from realtime would fail loudly.
Cut the historical 'Layer 2 was added in chart 1.0.0' note and the generic 'single source of truth' framing. Kept the two actionable points: ESO requires mapping Layer 1 keys; app.env overrides envDefaults.
|
@greptile |
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 32f8ca4. Configure here.
Summary
spec.serviceNameto<name>-postgresql— the field is immutable, so the rename to-headlesswould breakhelm upgradeon every existing install. Headless Service is still rendered alongside.networkPolicy.egressas a list of custom egress rules (back-compat). Cloud-metadata blocking moves to a sibling fieldnetworkPolicy.egressExceptCidrs. This prevents the silent-drop of user-supplied egress rules when upgrading.NOTES.txtcovering the above plus the ESOapiVersiondefault flip (v1 → v1beta1) from the previous PR.Type of Change
Testing
helm lint . --strictclean;helm unittest .51/51 pass.Checklist