fix(ci): validate the chart version each environment selects, not test1's - #143
Conversation
…t1's Both the render tests and the reachability check read chartVersion from test1 and used it for every environment. That inverts the coverage: a chart bump reaches staging and production before the test environments, so the manifests being changed were the ones never rendered against the chart they had asked for, and CI stayed green while production pointed at a version nobody had pulled. Each environment now renders against its own version, and the reachability check covers every distinct version rather than one. Each env.yaml is read on its own because yq separates multiple documents with ---, which would otherwise come back as a version to look up. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 45 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The chart-version read failure must be propagated before approval.
Review effort: Lite
Findings: None
What changed in this PR
This PR updates CI deployment validation to check each environment against its selected Helm chart version.
Changes:
- Validate every distinct chart version for publication.
- Render each environment with its own chart version.
- Avoid multi-file
yqdocument-separator issues.
| File | Reviewed changes |
|---|---|
scripts/test-deploy-logic.sh |
Adds per-environment chart selection and reachability checks. A moderate error-propagation issue and a grammatical nit remain. |
.github/workflows/validate.yml |
Validates all distinct environment-selected chart versions in CI. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Found by Copilot while reviewing #141 and #142.
The gap
scripts/test-deploy-logic.shand.github/workflows/validate.ymlboth readspec.platform.chartVersionfrom test1 and then use that one version for every environment:A chart bump reaches staging and production first - that is the release procedure - so the environments being changed were exactly the ones never rendered against the chart they had asked for. CI could stay green while a production manifest pointed at a version nobody had pulled. #141 and #142 are that case: they move four environments to 2.3.0 while test1 stays on 2.2.1.
The change
Each environment renders against its own
chartVersion, and the reachability check covers every distinct version rather than one.Each
env.yamlis read on its own rather than globbing them into oneyqinvocation: yq separates multiple documents with---, which comes back as a version to look up. That bug was in the first draft of this patch and is why the loop looks slightly long-winded.Verification
shellcheck -S error scripts/test-deploy-logic.shandactionlint- clean./scripts/test-deploy-logic.shon current main - ALL PASS, and the render checks still run for all eight environmentsConsequence worth knowing
Once this merges, #141 and #142 will go red until EduIDE-Helm#43 publishes chart 2.3.0. That is the check doing its job - it turns the merge order those PRs describe in prose into something CI enforces.
🤖 Generated with Claude Code