Repository navigation
ci(workflows): pin flux, yq and helm-unittest installs - #4645
Conversation
github-actions
Bot
added
area/uncategorized
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Important Review skippedReview was skipped as selected files did not have any reviewable changes. ⚙️ Run configurationConfiguration used: Repository: cozystack/cozystack/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: cozystack/cozystack/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (12)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughGitHub workflows replace Flux installer scripts with the Flux setup action at version 2.8.6. Selected yq and Helm unittest installations now check versions, and yq downloads are checksum-verified. Bats contracts and verification guidance are updated to reflect these workflow configurations. ChangesCI tool installation pins
Priority: ⬆️ High Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Other · Severity of issue fixed: High Merge Risk: ⚪ Minimal · up to The changes pin CI tools and strengthen installation checks. No actionable merge-blocking issue is established; merge after normal CI checks pass. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The changes reduce exposure to mutable installers and unversioned downloads without changing workflow permissions or credential configuration. Remaining uncertainty concerns the upstream setup action and recovery from interrupted tool installation. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Linked Issues checkExplanation Flux requirements in [ Full details: Out of Scope Changes checkExplanation The tool-install and contract-test changes are within scope. The PR also changes ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Every flux install fetched fluxcd.io/install.sh and ran it as root, so whatever that host served at the moment executed on runners that later hold registry credentials. Use the upstream setup action instead, pinned by commit and given an explicit version: it checks the release tarball against that release's checksums file and runs no script fetched at run time. nightly and the fork e2e lane still took yq from the floating latest release. They now use the pinned, checksum-verified install the promotion steps already have. The helm-unittest plugin embeds its own Helm and was installed at whatever version was newest, so a plugin release could change chart test results on a PR that touched no chart. Pin it, and replace a different version a persistent runner may carry. A contract test scans every workflow for these three tools, so an unpinned flux, yq or helm-unittest install copied from an older step fails the unit lane. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <[email protected]>
3e2e437 to
6df5d07
Compare
IvanHunters
left a comment
IvanHunters
left a comment
There was a problem hiding this comment.
LGTM.
CI tool-install hardening, and it holds up. I checked the three pins against upstream rather than taking them on faith:
- flux:
fluxcd/flux2/action@04acaec6161ac4fb1a82ffafa88901c03271d34fdereferences exactly from the annotatedv2.8.6tag, and the action'saction.ymlat that commit takes theversioninput, checksums the release tarball againstflux_<ver>_checksums.txt, and puts the binary on$GITHUB_PATHfor later steps. Theversion: "2.8.6"input is right. - yq: downloaded
yq_linux_amd64v4.53.3 and the sha256 matchesfa52a4e7...eded4byte for byte. - helm-unittest v1.1.2 exists; its
plugin.yamlcarriesversion: "1.1.2"(nov), so the[ "$installed" != "$HELM_UNITTEST_VERSION" ]guard is idempotent across reruns, and--version "v1.1.2"resolves to the git tag.
Coverage is complete: the merge base had 12 fluxcd.io/install.sh invocations across 8 files, all 12 are gone, and exactly 12 commit-pinned action steps replace them. No releases/latest/download left, no unpinned helm plugin install, every yq download pins v${YQ_VERSION} with the checksum. Splitting flux into its own step preserves each consuming step's if: guard and ordering. The new hack/ci-tool-installs-contract.bats passes (5/5), as do the updated promote-packages-contract.bats (7/7) and the edited stitch-multiarch_test.bats case.
On the two automated warnings: the tags.yaml yq flag is a false positive, that workflow never installs yq, it only asserts a preinstalled mikefarah build; and the promote-rc.yaml "out of scope" flag is the flux step split plus a one-word comment edit, which is intrinsic to this change.
One optional follow-up, not blocking: tags.yaml and the unit-test helm step still trust a runner-preinstalled yq/helm without a version check. That predates this PR and is out of its scope, but it is the same class of input this change is tightening elsewhere.
#4751) ## What this PR does Unit tests on main are red, and that blocks E2E on every open PR. Three contracts broke the same way. In each case a check from one PR and a line from another merged a few minutes apart on 2026-10-05, with no textual conflict. Any one of them alone keeps the job red, so they are fixed together. The first is the pin on how a suite reads HelmRelease history. #4689 added it. It accepts `$(kubectl ...` and, since #3594, the bounded `$(_cozy_guard_kubectl ...` wrapper. Nothing in front of kubectl is allowed. The monitoring suite from #4678 bounds that read with `$(timeout 30 kubectl ...`, and the bound is wanted. So the pin now accepts an optional `timeout [-k N] N` in front of kubectl. The prefix is spelled out on purpose. Any text would also let through `$(echo kubectl ...)`, which names the shared expansion without running it, and that form is now a refuse case. I checked it with mutations: the new accept cases fail against the old pattern, and a pin that takes any prefix fails a refuse case. The second is the flux install in workflows. #4645 forbids running fluxcd.io/install.sh and moved every workflow to the SHA-pinned fluxcd/flux2 action. The isp-hosted lane from #4712 still ran the script. It now uses the same action and version. The third is the negated-assertion audit. It reads the find operand in `\( ! -type d` in select-e2e_test.bats as a shell negation and fails. The line now says `-not -type d`. This is the same change as #4748, so this PR supersedes it. The audit bug itself is #4752. After the Go fix, the full log of the unit job had only these two bats failures left. Closes #4750 Closes #4755 ### Screenshots ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(ci): fix three unit-test contracts that went red on main after adjacent merges. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * History checks now recognize direct `kubectl` reads and reads through the supported wrapper, including timeout-bounded commands with an optional kill-grace setting. Commands that only include `kubectl` as an argument, use similarly named commands, or place a timeout before the wrapper are not treated as reads. * **Chores** * The hosted end-to-end workflow now uses a pinned Flux version, and suite selection handles directory exclusions consistently. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 participants
What this PR does
Workflows no longer run the flux install script as root. Every step that needed the flux CLI downloaded
https://fluxcd.io/install.shand ran it withsudo, so whatever that host served at the moment ran on runners that later get registry credentials. PR #4442 pinned the version, but the script was still fetched and executed. All 12 of those steps now usefluxcd/flux2/action, pinned to the v2.8.6 commit withversion: "2.8.6". The action downloads the release tarball, checks it against that release's checksums file, and adds the binary toPATH.Two workflows still took yq from the floating latest release, nightly and the fork e2e lane. They now use the same install the promotion steps already have: yq v4.53.3 checked against a committed SHA-256. I verified that hash against the release's
checksumsfile and against the binary itself.The helm-unittest plugin was installed at whatever version was newest. The plugin embeds its own Helm, so a plugin release could change chart test results on a PR that touched no chart. It's pinned to v1.1.2 now, the version CI already runs. A runner that carries a different version gets it replaced.
A new contract test scans every workflow for these three tools, because an unpinned install usually comes back by copying an older step. On main it fails on the install script, the missing setup action, the latest-release yq and the unpinned plugin. I ran one mutation per guard, and each one failed only the test meant to catch it. The existing contract tests that checked
FLUX_VERSIONinside the old steps now check for the action step.The e2e sandbox image still runs
install.shin its Dockerfile. That's an image build, not a workflow step, so I left it for a separate change.Closes #3619
Closes #3640
Closes #4510
Screenshots
Not applicable, no UI changes.
Downstream repositories
The diff touches only workflow tool-install steps and
hack/*.batscontract tests. Nothing in the trigger map reaches them.Release note
Summary by CodeRabbit