ci: enforce version sync invariant#34
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds repository-wide version drift detection: new Bash scripts enforce synced chart/k3s/collection versions, the lint CI job runs the checks and their self-test before setup/lint steps, and CHANGELOG.rst documents these CI additions. ChangesVersion invariant checks
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new CI script, hack/check-versions.sh, which ensures version strings remain synchronized across multiple configuration files, including galaxy.yml, role defaults, and example requirements. The script is integrated into the Lint job to prevent version drift. Feedback was provided to improve the clarity of error reporting by aligning a label with the actual variable name used in the code.
|
|
||
| report "cozy-installer" \ | ||
| "galaxy.yml:version" "$cozy_galaxy" \ | ||
| "roles/cozystack/defaults/main.yml:chart_version" "$cozy_role" \ |
There was a problem hiding this comment.
The label used in the report for the role defaults file is slightly inconsistent with the actual key name. In roles/cozystack/defaults/main.yml, the variable is named cozystack_chart_version. Updating the label to match the key exactly will make it easier to locate the variable if a drift is reported.
| "roles/cozystack/defaults/main.yml:chart_version" "$cozy_role" \ | |
| "roles/cozystack/defaults/main.yml:cozystack_chart_version" "$cozy_role" \ |
There was a problem hiding this comment.
Applied in a5c43f4. The cozy-installer group now labels the role default as roles/cozystack/defaults/main.yml:cozystack_chart_version (full variable name) so a DRIFT report points directly at the variable to grep for in the file.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hack/check-versions.sh`:
- Around line 70-101: The yq/get_collection_version command substitutions (e.g.,
cozy_galaxy, k3s_ci, orch_tests) can silently produce empty strings on failure
which bypasses report; enable bash's inherit_errexit (shopt -s inherit_errexit)
before these assignments or add explicit non-empty validation after each
assignment (or inside report()) to set err=1 and print a clear error when a
value is empty; update the block that assigns
cozy_galaxy/cozy_role/cozy_rhel/... and k3s_ci/k3s_rhel/.../orch_tests to either
enable inherit_errexit at top or to check each variable (and call err=1/report
error) before calling report().
In `@README.md`:
- Line 313: Update the Unreleased section of CHANGELOG.rst to include the chart
version bump: add a bullet "Bump cozystack_chart_version to 1.3.0" so the
changelog reflects the change made to roles/cozystack/defaults/main.yml
(cozystack_chart_version) and matches the README.md/galaxy.yml updates; place
the bullet alongside the other Unreleased entries for new features and CI
improvements.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 46c50414-cdb6-47cc-8958-94692b504a50
📒 Files selected for processing (5)
.github/workflows/test.ymlCHANGELOG.rstREADME.mdhack/check-versions.shhack/test-check-versions.sh
| | --- | --- | --- | | ||
| | `cozystack_chart_ref` | `oci://ghcr.io/cozystack/cozystack/cozy-installer` | Helm chart OCI reference | | ||
| | `cozystack_chart_version` | `1.2.2` | Helm chart version | | ||
| | `cozystack_chart_version` | `1.3.0` | Helm chart version | |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP '^\s*(version|cozystack_chart_version)\s*[:=]' galaxy.yml roles/cozystack/defaults/main.yml
rg -nP 'cozystack\.installer' -A3 examples/*/requirements.ymlRepository: cozystack/ansible-cozystack
Length of output: 971
🏁 Script executed:
cat -n CHANGELOG.rst | head -100Repository: cozystack/ansible-cozystack
Length of output: 6034
Add CHANGELOG.rst entry documenting cozystack_chart_version bump to 1.3.0.
The default chart version in roles/cozystack/defaults/main.yml was bumped from 1.2.2 to 1.3.0, matching the updates in galaxy.yml and README.md. However, the Unreleased section of CHANGELOG.rst documents new features and CI improvements but omits this chart version bump. Previous chart version bumps (v1.2.2, v1.2.1, v1.1.3, v1.1.2, v1.0.2) each got their own changelog entry. Add a bullet to the Unreleased section: Bump cozystack_chart_version to 1.3.0.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` at line 313, Update the Unreleased section of CHANGELOG.rst to
include the chart version bump: add a bullet "Bump cozystack_chart_version to
1.3.0" so the changelog reflects the change made to
roles/cozystack/defaults/main.yml (cozystack_chart_version) and matches the
README.md/galaxy.yml updates; place the bullet alongside the other Unreleased
entries for new features and CI improvements.
There was a problem hiding this comment.
Out of scope for this PR. The 1.3.0 chart bump landed in main via a separate renovate PR that already created the v1.3.0 tag, so this PR does not bump the chart version — it only adds the CI invariant and fixes a stale README default. The missing CHANGELOG entry is a pre-existing gap in the release flow (the release workflow tags but doesn't rename Unreleased); the correct fix is to create a real v1.3.0 section alongside a change to the release workflow, which is a separate concern.
There was a problem hiding this comment.
@lexfrei, understood — thanks for the context. The README update here is purely correcting a stale default that lagged behind the Renovate-driven 1.3.0 bump, not introducing a new version change, so a CHANGELOG entry in this PR would indeed be misattributed. Withdrawing this comment.
✏️ Learnings added
Learnt from: lexfrei
Repo: cozystack/ansible-cozystack PR: 34
File: README.md:313-313
Timestamp: 2026-04-23T10:42:54.380Z
Learning: In cozystack/ansible-cozystack, README.md default values for `cozystack_chart_version` can lag behind the actual version in `roles/cozystack/defaults/main.yml` and `galaxy.yml` after Renovate PRs. A PR that only corrects a stale README default is NOT bumping the chart version and does NOT require a CHANGELOG entry. The missing CHANGELOG entry for a released version (e.g. v1.3.0) is a pre-existing gap in the release flow: the release workflow tags the repo but does not rename the `Unreleased` CHANGELOG section to the new version. The correct fix is to create a proper versioned section alongside a release workflow change, handled separately.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: CR
Repo: cozystack/ansible-cozystack PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-15T11:53:04.566Z
Learning: Applies to roles/cozystack/defaults/main.yml : Collection version is inherited from `cozystack/cozystack` and tracks the upstream Cozystack chart version in `roles/cozystack/defaults/main.yml:cozystack_chart_version`. Do NOT bump collection version just because a PR adds features or fixes bugs.
Learnt from: CR
Repo: cozystack/ansible-cozystack PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-15T11:53:04.566Z
Learning: Applies to CHANGELOG.rst : `CHANGELOG.rst` must use per-version sections in reverse chronological order. For changes on `main` without a version bump, use an `Unreleased` section at the top. Rename it to the new version when upstream bumps. Never attribute new changes to an already-tagged version.
|
Cannot merge — branch has conflicts with |
Add hack/check-versions.sh and wire it into the Lint job. The script fails the build if any of three tracked dependency groups drifts across its authoritative files: - cozy-installer chart version: galaxy.yml, defaults/main.yml, and the three examples/*/requirements.yml (compared without the leading "v") - k3s binary version: ci-inventory.yml and all examples/*/inventory.yml - k3s.orchestration collection version: tests/requirements.yml and all examples/*/requirements.yml Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
- Fix stale cozystack_chart_version default in README (1.2.2 -> 1.3.0) - Move CHANGELOG Unreleased section to the top (reverse-chronological) - check-versions.sh: guard missing yq, normalise strip_v across all cozy-installer values, return status from report() instead of mutating a global so calls from subshells stay correct - Add hack/test-check-versions.sh self-test covering the positive path and one negative per invariant; wire into the Lint job Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
- Add a middle-file perturbation (examples/suse/inventory.yml) to test-check-versions.sh so the self-test does not implicitly rely on report() always comparing every value against pairs[1] - Document why the k3s group does not use strip_v (every inventory already uses the v-prefixed form and adding one without it should fail on purpose) Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
Address review feedback from gemini-code-assist on hack/check-versions.sh: use the full `cozystack_chart_version` name (matching the actual key in roles/cozystack/defaults/main.yml) instead of the abbreviated `chart_version` so a DRIFT report points directly at the variable to inspect. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
Address review feedback from coderabbitai on hack/check-versions.sh: without `inherit_errexit`, a failing yq in $(...) would silently become an empty string and pass every equality check, so drift could go undetected if a tracked key were renamed. - Enable `shopt -s inherit_errexit` so command-substitution failures propagate into outer assignments under `set -e`. - Add a defence-in-depth empty-value guard inside report() that fails loudly when the reference value is empty. - Extend test-check-versions.sh with a case that deletes `galaxy.yml:version` and asserts the script exits nonzero instead of reporting OK on an empty string. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
Aleksei Sviridkin (lexfrei)
force-pushed
the
ci/version-sync-invariant
branch
from
3065bea to
50961b9
Compare
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
3 participants
Summary
Add a version-sync invariant to the Lint job:
hack/check-versions.shfails the build if any tracked dependency's version drifts across its authoritative files.Three groups are checked:
galaxy.yml,roles/cozystack/defaults/main.yml, and the threeexamples/*/requirements.yml(compared without the leadingv).tests/ci-inventory.ymland all threeexamples/*/inventory.yml.tests/requirements.ymland all threeexamples/*/requirements.yml.Renovate bumps already update all files together, so the check is transparent for automated flows. A manual version change in a single file now fails CI with a diff showing which file lags.
Changes
hack/check-versions.shusingyq(preinstalled on GitHub-hosted ubuntu runners), wired as the first step of the existingLintjob in.github/workflows/test.yml.hack/test-check-versions.shself-test: asserts the clean tree exits 0, then perturbs one file per invariant group (including a middle file fork3sto guard against future refactor bugs) and asserts exit 1 with the correctDRIFT in <group>label on stderr. Runs alongside in the sameLintjob.cozystack_chart_versiondefault in the optional variables table (1.2.2→1.3.0).Unreleasedsection to the top of the file to match the reverse-chronological convention in the project CLAUDE.md.Test plan
shellcheckclean on both scripts.ansible-lintpasses (CI)ansible-test sanitypasses (CI)Summary by CodeRabbit