dex: render the authcodes CustomResourceDefinition from templates - #3594
dex: render the authcodes CustomResourceDefinition from templates#3594danish9039 wants to merge 1 commit into
Conversation
Helm ignores a crds directory on template and upgrade, so the schema could never be updated by a release. Rendering it from templates behind crds.enabled fixes both, with helm.sh/resource-policy: keep declared in ci/comparison.yaml so uninstall leaves the definition and the stored authorization codes behind. The synchronization script fails loudly when upstream changes the definition, and dex_helm_crd_lifecycle_test.py performs the same check in continuous integration. Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
🔵 Needs a closer look
Existing installations require an ownership-migration procedure before Helm can upgrade the previously unmanaged definition.
Pull request overview
Moves the Dex authorization-code CustomResourceDefinition into Helm templates so schema updates can be rendered and retained during uninstall.
Changes:
- Adds configurable CustomResourceDefinition rendering and retention.
- Adds lifecycle, parity, and upstream-drift checks.
- Documents the revised lifecycle behavior.
File summaries
| File | Description |
|---|---|
tests/dex_helm_crd_lifecycle_test.py |
Tests rendering, retention, disabling, and upstream parity. |
scripts/synchronize-dex-manifests.sh |
Enforces template parity during synchronization. |
common/dex/helm/values.yaml |
Adds the rendering toggle. |
common/dex/helm/templates/crds.yaml |
Templates the retained definition. |
common/dex/helm/README.md |
Documents lifecycle behavior. |
common/dex/helm/ci/comparison.yaml |
Declares the retained definition. |
.github/workflows/helm-kustomize-comparison.yml |
Runs the lifecycle tests. |
Review details
Suppressed comments (1)
common/dex/helm/templates/crds.yaml:5
- Moving this resource into
templates/makes the first upgrade from the previous chart fail. The oldcrds/installation leavesauthcodes.dex.coreos.compresent without the Helm ownership label and annotations, so a normalhelm upgraderejects the existing resource as having invalid ownership metadata before it can update the schema. The current render-only lifecycle tests cannot detect this. Add and test an explicit migration path, such as a documented ownership patch or a supportedhelm upgrade --take-ownershipprocedure.
- Files reviewed: 7/7 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Supersedes the bot-closed dex authcodes pull request #3574, which GitHub refuses to reopen; the change is rebased onto the merged comparison harness (#3577).
The problem
The chart kept
authcodes.yamlin acrds/directory. Helm installs that directory once and then ignores it:helm templaterenders no CustomResourceDefinition at all, andhelm upgradenever updates the schema. A schema change in a Dex release could therefore never reach a cluster through this chart.The change
common/dex/helm/templates/crds.yamlrenders the definition behindcrds.enabled(defaulttrue), withhelm.sh/resource-policy: keep, matching how the cert-manager and Istio charts handle definitions.common/dex/helm/ci/comparison.yamldeclares the retained definition with its reason; the harness fails if the annotation ever disappears undeclared, and fails the declaration if it stops matching.scripts/synchronize-dex-manifests.shno longer overwrites acrds/copy; it runstests/dex_helm_crd_lifecycle_test.py, which fails when the hand-written template drifts from the upstream definition.chart-behavior-testsjob.Verification
python3 tests/dex_helm_crd_lifecycle_test.py— 5 tests pass.python3 tests/run_helm_kustomize_comparison.py dex --all-scenarios— green under Helm 4.2.2; the retained declaration fires (a stale declaration fails the run).black --check,bash -n, workflow parse,git diff --checkclean.