feat(helm): add configMap mount fields to Config CRD schema (chart 0.6.3) - #65
Conversation
There was a problem hiding this comment.
I have enough information for a full review. No worktree needed — we're already at HEAD with a clean diff.
PR Review: feat(helm): add configMap mount fields to Config CRD schema (chart 0.6.3)
Must Fix (Critical)
None.
Should Fix (Important)
None.
Nice to Have (Optional)
None.
Manual Review
helm/crds/config-crd.yaml — New fields follow the same schema style as existing sibling fields (secretName, volumeMountPath): plain {type: string} with no constraints, and the configMapItems nested object mirrors the structure of other items definitions in the CRD. Placement is alphabetically correct alongside configMap* fields. Schema is well-formed YAML.
helm/Chart.yaml — version: 0.6.2 → 0.6.3 is a correct PATCH bump for a backwards-compatible feature addition. appVersion unchanged, which is correct (the chart ships a CRD; the app version it wraps is independent).
CHANGELOG.md — The ## Unreleased section is correctly added with a conventional feat prefix, describes the purpose clearly, and references the chart version bump. Format matches the project's existing style.
Concerns Addressed
| Concern | Disposition | Detail |
|---|---|---|
| correctness: new CRD fields must match executor runtime schema | not-an-issue | Fields follow the same plain-string pattern as existing CRD string fields (secretName, volumeMountPath). No contradictory constraints are present. The executor schema (configSpecProperties in k8s_connector.go) lives in the agent-task-executor repo, not this repo; this CRD ships the schema a fresh helm install bootstraps so the executor reconciles against an already-accepting CRD. |
| tests: automated CRD schema validation on CI | not-an-issue | No CRD/Helm lint target exists in the Makefile, and none was added — consistent with the existing repo practice. The CRD is validated structurally by the Kubernetes API server on apply; the YAML parses correctly. |
precommit
Skipped (selector mode) — CI covers lint+test.
{
"verdict": "approve",
"summary": "Clean helm-only change: three new optional ConfigMap-mount fields added to the Config CRD schema, chart version bumped 0.6.2→0.6.3, CHANGELOG entry added. Schema structure is consistent with existing CRD patterns. No issues found.",
"comments": [],
"concerns_addressed": [
{
"concern": "correctness: New fields configMapName, configMapMountPath, configMapItems must match the executor's runtime schema exactly",
"disposition": "not-an-issue",
"detail": "Fields follow the same plain-string pattern as existing CRD string fields (secretName, volumeMountPath). No contradictory constraints. The executor schema lives in agent-task-executor; this CRD bootstraps schema acceptance before reconciliation."
},
{
"concern": "tests: YAML parsing verified manually; confirm automated CRD schema validation on CI",
"disposition": "not-an-issue",
"detail": "No CRD/Helm lint target in Makefile — consistent with existing repo practice. YAML parses correctly; the Kubernetes API server validates structurally on apply."
}
]
}
What
Adds
configMapName/configMapMountPath/configMapItemsto the Config CRD schema shipped in the chart (helm/crds/config-crd.yaml), bumping chart 0.6.2→0.6.3.Why
The executor (
agent-task-executorv0.13.0) already reconciles the authoritative CRD schema at runtime, so live clusters have the fields. But the chart'scrds/copy is what bootstraps a freshhelm install— without this, a first install applies a CRD that prunes the ConfigMap mount fields before the executor reconciles. Mirrors the executor'sconfigSpecProperties()(k8s_connector.go).Verification
config-crd.yamlandChart.yamlagent-task-executorv0.13.0 (Jobeasy-agent-verify-11111111-20260908060043)Notes
Chart.yamlversion (matches the latest published chart).nuke/agentCHART_VERSIONpin bump are tracked as follow-up (the executor reconciles the schema at runtime, so nothing blocks on the chart publish).