Repository navigation
ci(workflows): pin flux version across CI toolchain steps - #4442
Conversation
github-actions
Bot
added
size/M
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 (1)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughCI workflows now check whether the Flux client reports version 2.8.6. If it does not, the workflows install that version. The e2e sandbox Dockerfile also sets and passes a default Flux version of 2.8.6 to the installer. ChangesFlux version pinning
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The CI workflows and sandbox image use the intended Flux version, and no concrete merge-blocking regression is evident. Architecture SummaryArchitecture risk: 🔵 Low · up to The change affects 1 system. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
IvanHunters NOT LGTM, but only because of the text. The workflow change is correct: every flux install in .github/workflows now goes through the version gate with 2.8.6, and the gate works. What blocks is a missing sign-off on the first commit and two sentences in the PR body, which becomes the merge commit message.
I checked the gate against a fake flux with different outputs. flux: v2.8.6 skips the install. A newer or older version, v2.8.60, a version without the v, a CRLF line ending, and a missing binary all reinstall. Extra lines or stderr noise still match, since grep -x works per line. The real 2.8.6 binary prints exactly flux: v2.8.6. The installer takes FLUX_VERSION without the v and builds releases/tags/v${FLUX_VERSION} from it, so passing it through sudo env is the right form. On this head the changed steps ran and installed 2.8.6, both in Build packages/core/platform and in Finalize. actionlint finds nothing new in the six files.
Commit 783adc7 has no Signed-off-by: line. Its body ends with Assisted-by: LLM, and the DCO check on this head is in action_required. The sign-off on the second commit covers only that commit, and docs/agents/contributing.md treats a missing sign-off as a blocker. git rebase --signoff origin/main and a force-push fix it.
The PR body claims more than the pin does, in two places. First, "no CI step resolves latest anymore" is not true yet. packages/core/testing/images/e2e-sandbox/Dockerfile:22 still runs curl -sSL "https://fluxcd.io/install.sh" | bash with no version, and the Build packages/core/testing job builds that image. On this head the layer came from cache, but on a cache miss it resolves latest like the old steps did.
Second, the body and the release note read as if the pin removes the transient 4xx. It does not. The job on #4429 failed at Downloading hash https://github.com/fluxcd/flux2/releases/download/v2.9.5/flux_2.9.5_checksums.txt with curl exit 22. v2.9.5 was published on 2026-08-31, weeks before that run, so it was not a release with half-uploaded assets. With the pin, the build job on this head fetched https://github.com/fluxcd/flux2/releases/download/v2.8.6/flux_2.8.6_checksums.txt, which is the same host and the same kind of URL. A transient 4xx there fails the pinned install in the same way. The pin makes the version fixed and reproducible, and that is the benefit to describe. Please either pin the sandbox Dockerfile too (an ARG FLUX_VERSION next to the other tool versions) or say "no workflow step", and drop the claim that the flakiness is gone from both the body and the release note.
Two non-blocking notes:
2.8.6is now a literal in elevenFLUX_VERSION:lines across eight workflow files, andhack/promote-packages-contract.batspins only the three promotion steps. This matters because the build steps runflux push artifact, andhack/verify-promoted-packages.shcompares the result against anexcludeOCIlist tied to the flux version "pinned as FLUX_VERSION (2.8.6) in all three workflow steps". Push and verify agree now, but the next bump can touch only the three pinned steps and nothing will fail. Extending the contract test to everyFLUX_VERSION:line would close that, and that comment could stop counting three.- The e2e-fork comment this PR edits still points to "packages/system/fluxcd, 2.8.x". That directory does not exist. The shipped distro is
internal/fluxinstall/manifests/fluxcd.yamlatv2.8.0. This is older than the PR, but the comment is already being edited. A 2.8.6 CLI with 2.8.0 controllers is fine here, since CI uses the CLI only to push and pull OCI artifacts.
5e69cc0 to
ac51128
Compare
|
Thanks for the careful review — all three blocking points are addressed and the branch is force-pushed. DCO / sign-off: both commits are rebuilt so every commit now carries Unpinned flux in the sandbox image: Overclaim in the PR text: you're right that pinning does not remove the transient 4xx — the checksums are fetched from the same host regardless of the version. I dropped every "removes flakiness / 4xx" claim from the PR body, the release note, and the commit messages, and restated the real benefit: the flux CLI version is now fixed and reproducible across all of CI, with no surprise version bump, and no step (workflows or the e2e-sandbox image) resolves floating latest anymore. That last statement now holds precisely because the Dockerfile is pinned too. Comment path (non-blocking): fixed the e2e-fork comment to point at the distro actually shipped, Follow-up (accepted, separate PR): extending |
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
IvanHunters LGTM, thanks.
Kirill Ilin (sircthulhu)
left a comment
Kirill Ilin (sircthulhu)
left a comment
There was a problem hiding this comment.
Pinning the flux CLI is the right call, and the version guard plus the no-v value line up with how install.sh resolves tags. A few things block merge: the pin isn't protected by a contract test, the post-install result isn't verified, and the e2e-sandbox RUN can still pipe an error page into bash. Details inline.
| run: | | ||
| command -v flux >/dev/null \ | ||
| || curl -fsSL https://fluxcd.io/install.sh | sudo bash | ||
| if ! flux version --client 2>/dev/null | grep -qx "flux: v${FLUX_VERSION}"; then |
There was a problem hiding this comment.
This pin is now duplicated across the workflows and the e2e-sandbox Dockerfile, and nothing keeps the copies in sync or stops the old command -v flux || curl … | sudo bash idiom from coming back via copy-paste — hack/promote-packages-contract.bats already calls out that exact failure mode for the promotion steps. Please extend it (or add a sibling contract) so that CI fails when:
- any
fluxcd.io/install.shcall in.github/workflows/*.yamlor the e2e-sandbox Dockerfile has noFLUX_VERSIONnext to it; FLUX_VERSIONvalues differ between files.
Alternatively, a composite action (e.g. .github/actions/setup-flux) would collapse the repeated block into one place with a single version constant.
| if ! flux version --client 2>/dev/null | grep -qx "flux: v${FLUX_VERSION}"; then | ||
| install_script="$(mktemp)" | ||
| curl -fsSL https://fluxcd.io/install.sh -o "$install_script" | ||
| sudo env "FLUX_VERSION=$FLUX_VERSION" bash "$install_script" |
There was a problem hiding this comment.
The installed version is never checked after install.sh runs. install.sh writes to /usr/local/bin; if another flux earlier in PATH shadows it, the step succeeds and later steps silently use the unpinned binary. Suggest re-running the check after install (here and in the other copies):
flux version --client | grep -Fqx "flux: v${FLUX_VERSION}" \
|| { echo "::error::flux ${FLUX_VERSION} not effective on PATH"; exit 1; }Nit: grep -Fqx instead of grep -qx, so the dots in the version aren't treated as regex wildcards.
| @@ -63,9 +63,15 @@ jobs: | |||
| # Ephemeral runners lack the flux CLI the installer's image-packages step | |||
| # shells out to; install if absent (idempotent). | |||
There was a problem hiding this comment.
The comment still says "install if absent (idempotent)", but the step now also reinstalls when the version differs. The same wording is in build-release.yaml:110, tags.yaml:144 and pull-requests.yaml:356.
| RUN curl -sSL "https://github.com/mikefarah/yq/releases/download/v4.53.3/yq_${TARGETOS}_${TARGETARCH}" -o /usr/local/bin/yq \ | ||
| && chmod +x /usr/local/bin/yq | ||
| RUN curl -sSL "https://fluxcd.io/install.sh" | bash | ||
| RUN curl -sSL "https://fluxcd.io/install.sh" | FLUX_VERSION="${FLUX_VERSION}" bash |
There was a problem hiding this comment.
curl -sSL without -f, under the default /bin/sh -c without pipefail: on a 4xx/5xx the error page is piped into bash and curl's exit code is lost — the same transient-installer failure class this PR is addressing. Please use curl -fsSL together with SHELL ["/bin/bash", "-o", "pipefail", "-c"], or download to a file first as the workflows do.
| sudo env "FLUX_VERSION=$FLUX_VERSION" bash "$install_script" | ||
| rm -f "$install_script" | ||
| fi | ||
| command -v yq >/dev/null || { sudo curl -sSL -o /usr/local/bin/yq https://github.com/mikefarah/yq/releases/latest/download/yq_linux_amd64 && sudo chmod +x /usr/local/bin/yq; } |
There was a problem hiding this comment.
The same step still installs yq from releases/latest and helm via get-helm-3 without a version, and mc in the e2e-sandbox Dockerfile is unpinned too. The PR description says no step resolves a floating latest any more — please either pin these here or narrow the description and release note to the flux CLI.
| # the OCIR token is loaded, where a pinned installer matters most; | ||
| # keep it in step with the flux distro cozystack ships | ||
| # (packages/system/fluxcd, 2.8.x). | ||
| # (internal/fluxinstall/manifests/fluxcd.yaml, v2.8.0). |
There was a problem hiding this comment.
Non-blocking: "keep it in step with the flux distro" isn't enforced anywhere — the CLI is at 2.8.6 while the shipped distro is v2.8.0. If the two must stay in the same minor, the contract test from the comment on build-main.yaml could assert it.
|
Thanks, this is a sharp review. Pushed fixes for the concrete correctness points and scoped the rest. Literal version match: fixed. All the guards this PR adds now use Sandbox installer piped into bash: fixed. The e2e-sandbox flux step now does Stale comments: fixed. The "install if absent (idempotent)" comments in Overclaim in the description: narrowed. The body and release note now scope the "no floating latest" statement to the flux CLI and explicitly say yq, helm and mc are not touched here (yq's floating-latest is tracked in #3640). On the two I did not change in this PR: Contract test for the pin: agreed, and I'd like to do it right rather than minimally. A sibling contract that fails when any Post-install verification: I judged this non-blocking rather than skipping it on purpose. One note on the red check: |
e1d80c9 to
626a107
Compare
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
IvanHunters NOT LGTM, only because the branch conflicts with main. The two new commits are fine.
The conflict is a single line in the e2e-sandbox Dockerfile. Main added ARG MC_VERSION=... exactly where this branch adds ARG FLUX_VERSION=2.8.6, so keep both ARG lines. The rest merges cleanly, including your flux step next to the new mc block. The rebase also brings in the mc fix, and that is what makes Build packages/core/testing red on this head. The log shows curl: (22) ... 410 from dl.min.io, and the flux layer right before it installed 2.8.6 without problems.
The new commits do what they say. The guards match the version literally with grep -Fqx, and the sandbox image downloads the installer to a file with curl -f, so an HTTP error fails the layer. No flux install in the workflows, the sandbox image or hack/ resolves latest, and actionlint shows no new warnings on the six workflows.
One non-blocking note. The three promotion guards still use grep -qx, so the repo now has two spellings of the same check. That fits the contract-test follow-up you mentioned.
The "Set up build toolchain" steps installed flux with a bare `curl https://fluxcd.io/install.sh | sudo bash`, letting the installer resolve its floating "latest". latest had advanced to v2.9.5, and a matrix job on PR #4429 failed when that release's checksums transiently 4xx'd during install. Pin FLUX_VERSION to 2.8.6 and guard on the installed client version, the same shape the "Set up promotion toolchain" steps already use, so the flux CLI is fixed and reproducible across CI instead of following latest. This does not remove the checksum host's transient errors; it removes the uncontrolled version bump. Related to the yq floating-latest pin in #3640. Assisted-by: LLM Signed-off-by: IvanHunters <[email protected]>
The nightly toolchain steps still fetched flux through the installer's floating "latest". Pin them to FLUX_VERSION 2.8.6 with the same version-gated guard so nightly's flux push artifact step uses a fixed, reproducible CLI rather than whatever latest resolves to. Also switch e2e-fork's already-pinned step from a presence check to the same version gate, so a runner with a pre-cached older flux is corrected rather than left as is, and fix that step's comment to point at the flux distro cozystack actually ships (internal/fluxinstall/manifests/fluxcd.yaml, v2.8.0). Assisted-by: LLM Signed-off-by: IvanHunters <[email protected]>
The e2e-sandbox image fetched flux through `install.sh` with no version, so the build resolved the installer's floating "latest". Add an ARG FLUX_VERSION, matching the neighbouring COZYHR_VERSION and CRUST_GATHER_VERSION pins, and pass it to install.sh so the sandbox's flux CLI is fixed and reproducible like the rest of the image's tools. Assisted-by: LLM Signed-off-by: IvanHunters <[email protected]>
The guard ran `grep -qx "flux: v${FLUX_VERSION}"`, where the dots in the
version are regex metacharacters, so a client printing `flux: v2X8X6`
would satisfy it. Use `grep -Fqx` for a fixed-string match in the build
toolchain guards this change added.
Also correct the step comments that still read "install if absent
(idempotent)": the step now installs whenever the pinned version is not
the one present, not only when flux is missing.
Assisted-by: LLM
Signed-off-by: IvanHunters <[email protected]>
…ox image The flux install piped `curl -sSL` into bash, so on a 4xx/5xx the error page was piped into the shell and curl's exit code was lost. Download the installer to a file with `curl -fsSL` first, then run it, so an HTTP error fails the build step instead of feeding bash a bad script. Assisted-by: LLM Signed-off-by: IvanHunters <[email protected]>
626a107 to
ba46b9f
Compare
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
IvanHunters LGTM. I rebased the branch onto main myself. The one conflict was in the e2e-sandbox Dockerfile, where main added ARG MC_VERSION next to your ARG FLUX_VERSION; I kept both.
## 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.sh` and ran it with `sudo`, 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 use `fluxcd/flux2/action`, pinned to the v2.8.6 commit with `version: "2.8.6"`. The action downloads the release tarball, checks it against that release's checksums file, and adds the binary to `PATH`. 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 `checksums` file 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_VERSION` inside the old steps now check for the action step. The e2e sandbox image still runs `install.sh` in 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 - [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: The diff touches only workflow tool-install steps and `hack/*.bats` contract tests. Nothing in the trigger map reaches them. ### Release note ```release-note ci(workflows): install flux through the commit-pinned upstream setup action instead of running fluxcd.io/install.sh as root, pin yq with checksum verification in the nightly and fork e2e workflows, and pin the helm-unittest plugin version. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Reliability** * CI workflows now use a consistent, pinned Flux version. The yq and Helm plugin setup also verifies or enforces specific versions. * **Tests** * Added checks to ensure workflow tool versions are pinned and consistent, and that downloaded yq releases are checksum-verified. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
3 participants
What this PR does
The flux CLI was installed across the build, PR, tag, nightly and e2e-fork workflows and in the e2e-sandbox image through the vendor
install.shwith no version pin, so each install resolved the installer's floatinglatest.latesthad advanced to v2.9.5, and a matrix job on #4429 failed when that release's checksums transiently 4xx'd during install. Pinning does not stop the checksum host from returning a transient error, so it is not a fix for that flake; what it fixes is the uncontrolled version. This setsFLUX_VERSIONto 2.8.6 and guards each flux install on the installed client version (workflows, with a fixed-string match) or passes it toinstall.sh(the e2e-sandbox image), matching the already-pinned "Set up promotion toolchain" steps, so the flux CLI is fixed and reproducible across CI with no flux install resolving floatinglatest. This is scoped to the flux CLI only: other tools some of these steps install (yq, helm, mc) are not touched here, and the yq floating-latest is tracked separately in #3640.Screenshots
Not applicable — CI and test-image change, no UI impact.
Downstream repositories
Release note
Summary by CodeRabbit