rename helm chart agent-platform to agent#29
Conversation
There was a problem hiding this comment.
Based on my review, here's my analysis:
Review Summary
PR #29: rename helm chart agent-platform to agent
Scope: 2 files changed (CHANGELOG.md, helm/Chart.yaml)
Findings
The PR is incomplete. It renamed the chart name: in helm/Chart.yaml from agent-platform to agent, but the internal template references and Kubernetes object labels still use the old name agent-platform.
Files with stale references (NOT updated by this PR):
| File | Issue |
|---|---|
helm/values.yaml:1 |
Comment still says # agent-platform chart values. |
helm/templates/_helpers.tpl:2 |
Defines agent-platform.labels (and app.kubernetes.io/part-of: agent-platform) |
helm/templates/_helpers.tpl:10 |
Defines agent-platform.namespace |
helm/templates/_helpers.tpl:15 |
Defines agent-platform.executor.image |
helm/templates/executor-deployment.yaml:6,9,50 |
References agent-platform.namespace, agent-platform.labels, agent-platform.executor.image |
helm/templates/executor-rbac.yaml |
References agent-platform.namespace, agent-platform.labels |
helm/templates/executor-service.yaml:5,7 |
References agent-platform.namespace, agent-platform.labels |
helm/templates/executor-secret.yaml:5,7 |
References agent-platform.namespace, agent-platform.labels |
The app.kubernetes.io/part-of label will show agent-platform in all deployed resources even though the chart is named agent.
{
"verdict": "request-changes",
"summary": "PR renamed the chart name in Chart.yaml but did not update internal template references, values.yaml comment, or the app.kubernetes.io/part-of label — all still reference the old name 'agent-platform'. This creates an inconsistency between the chart name and deployed resource labels.",
"comments": [
{
"file": "helm/values.yaml",
"line": 1,
"severity": "major",
"message": "Comment still says '# agent-platform chart values.' — should be updated to '# agent chart values.' to match the renamed chart."
},
{
"file": "helm/templates/_helpers.tpl",
"line": 4,
"severity": "major",
"message": "app.kubernetes.io/part-of label still says 'agent-platform' — should be updated to 'agent' to match the chart name. This label appears on all deployed Kubernetes objects."
},
{
"file": "helm/templates/_helpers.tpl",
"line": 2,
"severity": "major",
"message": "Template name 'agent-platform.labels' should be renamed to 'agent.labels' to match the chart name convention."
},
{
"file": "helm/templates/_helpers.tpl",
"line": 10,
"severity": "major",
"message": "Template name 'agent-platform.namespace' should be renamed to 'agent.namespace' to match the chart name convention."
},
{
"file": "helm/templates/_helpers.tpl",
"line": 15,
"severity": "major",
"message": "Template name 'agent-platform.executor.image' should be renamed to 'agent.executor.image' to match the chart name convention."
}
],
"concerns_addressed": [
"correctness: PARTIALLY ADDRESSED — Chart.yaml name updated but internal template references and labels not updated; additional files need changes to fully complete the rename"
]
}superseded by new automated review
There was a problem hiding this comment.
Now I have all the files. Let me analyze the changes systematically.
Review findings:
Must Fix (Critical)
None — no security, correctness, or critical bugs found.
Should Fix (Important)
- No Helm chart tests — The
helm/directory has nohelm unittestor smoke test. A basichelm template .sanity check would catch template errors before CI.
Nice to Have (Optional)
None.
Correctness Verification ✓
app.kubernetes.io/part-of label (helm/templates/_helpers.tpl:4):
- Now correctly set to
agent(wasagent-platform) - All resources (
Deployment,ServiceAccount,ClusterRole,ClusterRoleBinding,Role,RoleBinding,Service,Secret) include{{- include "agent.labels" . | nindent 4 }}— ✓ consistent
Stale reference check:
- No
agent-platformstrings remain in any helm file — ✓ clean - Template defines renamed:
agent-platform.labels→agent.labels,agent-platform.namespace→agent.namespace,agent-platform.executor.image→agent.executor.image— ✓ all updated values.yamlcomment (# agent chart values) updated — ✓
CHANGELOG — ## Unreleased section correctly documents the new helm/ chart as agent with accurate description — ✓
Concerns addressed
concerns_addressed:correctness: app.kubernetes.io/part-of label updated to 'agent' in all resources — verified in _helpers.tpl:4 and all 6 resource templates include agent.labels;correctness: all agent-platform.* template defines renamed to agent.* — no stale references remain;tests: no helm unittest added — flagged as Should Fix
{
"verdict": "approve",
"summary": "Helm chart rename from agent-platform to agent is complete and correct. All template defines renamed, app.kubernetes.io/part-of label consistently updated across all 7 resources, no stale references remain, CHANGELOG accurately documents the change. One noted gap: no helm template unit tests.",
"comments": [
{
"file": "helm/",
"line": 0,
"severity": "major",
"message": "Should Fix: No Helm chart tests — add a basic helm unittest suite or at minimum a `helm template .` smoke test to catch template errors in CI before they reach production."
}
],
"concerns_addressed": [
"correctness: app.kubernetes.io/part-of label updated to 'agent' in _helpers.tpl:4 — verified all 7 resource templates include agent.labels with nindent 4",
"correctness: all agent-platform.* template defines renamed to agent.* — no stale references remain in any helm file",
"tests: no helm unittest added — raised as Should Fix comment above"
]
}
Rename the chart from
agent-platformtoagentfor consistency: chart name = repo name (bborbe/agent), matching the quant install dirquant/agent/+ releaseagent-dev, and pairing cleanly with the futuremaintainerchart. Published asoci://registry-1.docker.io/bborbe/agent. The chart entry is still Unreleased, so no released version shipped under the old name.