fix(installer): skip the packages artifact push and stamp with PUSH=0 by lexfrei · Pull Request #4663 · cozystack/cozystack

Aleksei Sviridkin (lexfrei) · 2026-10-01T19:24:24Z

What this PR does

A local build with PUSH=0 no longer publishes the packages artifact. Before this change, make image-packages in packages/core/installer ran flux push artifact no matter what PUSH was set to, and then wrote the pushed digest into values.yaml. The chart target in the same Makefile already respects PUSH.

With PUSH set to anything other than 1, the target now skips both the push and the digest stamp. It prints one line saying so and leaves the committed platformSourceUrl and platformSourceRef as they are. It still writes packages/core/platform/.build-revision, which is gitignored.

I chose to skip the stamp instead of computing a local digest, because every reader of that value needs a digest that exists in the registry. The chart passes platformSourceRef to the operator as --platform-source-ref, and Flux then pulls cozystack-packages@<digest> from the registry. The release scripts verify-promoted-packages.sh, promote-packages-artifact.sh, stitch-multiarch.sh and nightly-mirror.sh also treat it as a registry digest. A digest of an artifact that was never pushed points at nothing. On main an unpushed run would not even get that far, since the recipe dies at test -n "$DIGEST".

The gate is a make-level ifeq ($(PUSH),1), so it reads whatever PUSH ends up as after hack/common-envs.mk. On main that means PUSH=0 on the command line. I also applied the common-envs.mk change from #4654 locally, which takes PUSH from the environment and makes LOAD=1 default it to 0. With that applied, a dry run shows no push and no stamp for PUSH=0 in the environment, for LOAD=1, and for PUSH=0 on the command line, while LOAD=1 PUSH=1 and an empty PUSH still push.

CI is not affected, because no workflow sets PUSH or LOAD and PUSH stays at 1. I dry-ran the three CI invocations of this Makefile before and after the change, on main and again with #4654 applied:

tags.yaml, release stitch step: make -C packages/core/installer image-packages chart with REGISTRY, a release tag as IMAGE_TAG, and PUSHED_TAGS_LOG
pull-requests.yaml, installer step of a same-repo PR: make -C packages/core/installer image with a pr-N-sha tag, PLATFORM=linux/amd64, BUILDER, and an empty OCI_EXPORT_DIR
build-main.yaml and build-release.yaml: make build, which runs the installer image target with defaults

$ diff --recursive before/ after/     # tags.txt pr.txt build.txt
$ echo $?
0

There is one side effect. OCI_EXPORT_DIR forces PUSH := 0, so running image-packages with it set no longer pushes, where before it would have tried an anonymous push. No CI path does that. The fork PR build already runs only pre-checks image-operator for the installer, and the make -n ... image OCI_EXPORT_DIR=/oci scan in e2e-fork.yaml greps only the .oci.tar names, which this change does not touch.

The new test dry-runs the target in both modes. With PUSH=0 there must be no flux push artifact and no platformSourceRef stamp; with PUSH at its default, both must be present. The first test fails on main. I mutated the guard three ways, and each time only the matching test went red:

condition forced true: the PUSH=0 test fails
compared against 2: the default-path test fails
endif moved up so only the push is gated: the PUSH=0 test fails on the stamp check

One limitation is left as a follow-up. The test's env -u PUSH -u LOAD only isolates it once #4654 lands, because on main a file assignment wins over the environment anyway. It does not cover PUSH=0 coming in through MAKEFLAGS, so make bats-unit-tests PUSH=0 would fail the default-path test here and the ordering check in hack/stitch-multiarch_test.bats. Nothing runs the suite that way today. If that changes, passing an explicit PUSH=1 on the default-path make lines, or unsetting MAKEFLAGS the way hack/bats-unit-keep-going.bats does, would fix it.

Screenshots

Not a UI change.

Downstream repositories

The website's development.md tells developers to run make image-packages with REGISTRY set to their own registry. That is the default PUSH=1 path, and it works as before. Neither the website nor ccp mentions PUSH=0 for this target.

Closes #4656

Release note

fix(installer): `make image-packages` no longer pushes the cozystack-packages artifact or stamps its digest into values.yaml when PUSH=0.

Summary by CodeRabbit

New Features
- You can run image-packages with PUSH=0 to skip publishing the artifact and keep the committed platform source reference unchanged.
- When pushing is enabled, the command publishes the artifact and updates the platform source URL and digest-based reference.

image-packages ran `flux push artifact` regardless of PUSH, so a local PUSH=0 build still published cozystack-packages and then stamped its digest into values.yaml. The stamped platformSourceRef is the digest the operator hands Flux to pull from the registry, so a build that pushes nothing has no digest worth writing: it now skips both steps and keeps the committed ref. With PUSH at its default of 1 the recipe is unchanged. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <[email protected]>

coderabbitai · 2026-10-01T19:24:58Z

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: cozystack/cozystack/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 963f787e-3af8-4ff8-8902-3ebd39462ed1

📥 Commits

Reviewing files that changed from the base of the PR and between bc400df and 7c5d9d5.

📒 Files selected for processing (2)

hack/installer-image-packages-push_test.bats
packages/core/installer/Makefile

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.

📝 Walkthrough

Walkthrough

The installer image-packages target now gates artifact publishing and values.yaml updates on PUSH=1. New dry-run tests check the PUSH=0 and unset PUSH cases.

Changes

Installer package publishing

Layer / File(s)	Summary
Push gate and dry-run tests `packages/core/installer/Makefile`, `hack/installer-image-packages-push_test.bats`	The Makefile runs the artifact push and updates `values.yaml` when `PUSH=1`. Otherwise, it reports that the package was not pushed and retains the committed `platformSourceRef`. Dry-run tests check the disabled and unset `PUSH` cases.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to 7c5d9

Package publishing remains controlled by PUSH, and the non-push path retains the committed source reference. No concrete merge-blocking behavior change is indicated.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to 7c5d9

The change narrows package publication without adding privileges or weakening deployment-source controls. Non-publishing builds retain their existing source reference, and the established fork-build recovery path publishes packages separately.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

inferred — The changed effects are publication to the configured cozystack-packages repository and updates to local installer source values, which may subsequently configure consuming clusters. The PR introduces no additional repository authority, credential source, or cluster-side privilege; it suppresses these publication effects when the effective setting disables them.

Trust Boundaries and Controls

observed — Fork builds select OCI export mode without registry credentials. The new gate avoids their package-push attempt. Publication remains in the existing privileged workflow, which validates image archive names against a trusted allowlist, publishes the packages tree as data, and captures its digest. The Make gate does not replace registry authentication or those workflow controls.

Resilience and Maintainability Implications

inferred — The enabled path retains inherited failure and concurrency limitations: publication runs through tee, jobs share a log path, and URL/ref stamping uses separate writes. Missing digest output blocks stamping, but interruption between writes or concurrent log use can still produce inconsistent metadata. No transaction, rollback, or isolation guarantee is added or removed; these are pre-existing conditions, not active PR concerns.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: `image-packages` skips the packages artifact push and digest stamp when `PUSH=0`.
Linked Issues check	✅ Passed	Issue [`#4656`] requires `image-packages` to respect `PUSH`, avoid `flux push` when `PUSH=0`, handle the digest step, and add a dry-run test. The Makefile gates the artifact push and `values.yaml` diges…
Out of Scope Changes check	✅ Passed	The changes are limited to the linked issue. The Makefile change implements the `PUSH` guard and preserves the committed reference when no artifact is pushed. The added Bats tests verify the required …
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…

✨ Finishing Touches

📝 Generate docstrings

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

IvanHunters

LGTM, approve.

image-packages now gates flux push artifact and the values.yaml digest stamp behind ifeq ($(PUSH),1), and the else branch keeps the committed platformSourceRef. Right call: the stamped ref is a registry digest the operator hands to Flux, so stamping one for an artifact that was never pushed would point at nothing.

What I checked:

Consumer trace. Every reader of platformSourceRef treats it as a registry digest: the operator via --platform-source-ref, and the release scripts verify-promoted-packages.sh, promote-packages-artifact.sh, stitch-multiarch.sh, nightly-mirror.sh. PUSH=0 leaves the committed ref untouched, so they keep resolving the last released artifact. Nothing downstream breaks.
CI and release are untouched. No workflow sets PUSH or LOAD, so PUSH stays 1. The tags.yaml release stitch (image-packages chart) and the same-repo PR build (make image with an empty OCI_EXPORT_DIR) push as before. The fork PR build runs only pre-checks image-operator and never reaches image-packages, and the e2e-fork.yaml allowlist (make -n image OCI_EXPORT_DIR=/oci) greps only the .oci.tar names from image-operator, which the push/echo line never matched.
Tests pass on the PR head. I mutated the guard to always-true and the PUSH=0 test went red, so it isn't inert. The if grep -q ...; then ...; false; fi idiom is what sidesteps cozytest.sh suppressing !-negated assertions. I also merged current main into the head and reran the installer tests, still green, and the installer Makefile is untouched on main.

Three non-blocking notes:

[NIT] The one behaviour change is OCI_EXPORT_DIR forcing PUSH := 0: image-packages now skips the push where it used to attempt an anonymous one. No CI path reaches image-packages with OCI_EXPORT_DIR set, and skipping beats a push that can only fail without credentials, so this is informational, not a regression.
[NIT] On main, before #4654, the gate honours only a command-line PUSH=0, because hack/common-envs.mk still assigns PUSH := 1 and that beats the environment. The test uses the command-line form, so it is correct here; the environment-level isolation only starts working once #4654 lands. Worth a line saying so.
[NIT] As the description already notes, PUSH=0 through MAKEFLAGS (make bats-unit-tests PUSH=0) would fail the default-path test and the stitch ordering check. Nothing runs the suite that way today, so it is fine as a documented follow-up.

Aleksei Sviridkin (lexfrei) requested review from IvanHunters, Kingdon Barrett (kingdonb), Andrei Kvapil (kvaps), Timofei Larkin (lllamnyp), myasnikovdaniil and Kirill Ilin (sircthulhu) as code owners October 1, 2026 19:24

github-actions Bot added size/M This PR changes 30-99 lines, ignoring generated files area/platform Issues or PRs related to platform infrastructure (bundle, flux, talos, installer) kind/bug Categorizes issue or PR as related to a bug labels Oct 1, 2026

Aleksei Sviridkin (lexfrei) added this to the v1.7.0 milestone Oct 3, 2026

IvanHunters approved these changes Oct 5, 2026

View reviewed changes

Aleksei Sviridkin (lexfrei) merged commit 9c025fa into main Oct 5, 2026
20 checks passed

Aleksei Sviridkin (lexfrei) deleted the fix/image-packages-honour-push branch October 5, 2026 11:14

Navigation Menu

Repository navigation

fix(installer): skip the packages artifact push and stamp with PUSH=0 - #4663

fix(installer): skip the packages artifact push and stamp with PUSH=0#4663
Aleksei Sviridkin (lexfrei) merged 1 commit into
mainfrom
fix/image-packages-honour-push

Aleksei Sviridkin (lexfrei) commented Oct 1, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 1, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

IvanHunters left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Aleksei Sviridkin (lexfrei) commented Oct 1, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does

Screenshots

Downstream repositories

Release note

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Security Architecture Review

Uh oh!

IvanHunters left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Aleksei Sviridkin (lexfrei) commented Oct 1, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 1, 2026 •

edited

Loading