feat(release): immutable tags & rc→stable promotion (#2677) - #3017
Conversation
|
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:
📝 WalkthroughWalkthroughThe PR replaces force-updated release handling with write-once tag publication, adds rc promotion and nightly artifact workflows, introduces nightly GHCR retention, and propagates runtime version data through installer, dashboard, and version code. ChangesRelease Pipeline Overhaul
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
github-actions
Bot
added
area/release
dosubot
Bot
added
the
area/ci
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a robust, immutable release promotion mechanism for Cozystack. By decoupling the version reporting from the build-time binary and enabling runtime overrides via environment variables, the system can now promote release candidates to stable releases through atomic retagging of immutable image digests. This ensures that the exact bits validated during the release candidate phase are those deployed in the stable release, significantly improving reliability and auditability of the release process. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![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 mechanism to promote release-candidate images to stable by retagging them instead of rebuilding. It adds a promotion script 'hack/promote-retag.sh' that copies images by digest using 'skopeo', updates the 'Makefile' to pass the platform version as deploy-time data, and updates the operator deployment template and Go code to support overriding the version via the 'COZYSTACK_VERSION' environment variable. Feedback on these changes highlights several issues: the 'ref_repo' function in the promotion script incorrectly strips registry ports when no tag is present; piping to the 'while' loop runs it in a subshell, which can prevent 'exit 1' from terminating the parent script; in the 'Makefile', an empty 'COZYSTACK_VERSION' evaluates to 'v' and incorrectly overrides the default version; and the script should explicitly verify that the mikefarah version of 'yq' is installed.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| } | ||
|
|
||
| # Split a "<repo>[:<tag>]@sha256:<digest>" ref into repo and digest. | ||
| ref_repo() { r="${1%@*}"; printf '%s' "${r%:*}"; } # strip @digest, then :tag |
There was a problem hiding this comment.
The current implementation of ref_repo uses ${r%:*} to strip the tag. However, if the repository reference contains a port (e.g., localhost:5000/cozystack/operator) but no tag (which is the case for shape 2 and shape 3 outputs from collect_refs), ${r%:*} will strip the port and the rest of the path, returning just localhost. This will break any deployment using a registry with a port.
We can fix this robustly by only stripping the tag from the last component of the path.
| ref_repo() { r="${1%@*}"; printf '%s' "${r%:*}"; } # strip @digest, then :tag | |
| ref_repo() { | |
| r="${1%@*}" | |
| img="${r##*/}" | |
| if [ "$r" = "$img" ]; then | |
| printf '%s' "${img%:*}" | |
| else | |
| printf '%s/%s' "${r%/*}" "${img%:*}" | |
| fi | |
| } |
There was a problem hiding this comment.
Good catch — fixed in 76c0369. ref_repo now strips the :tag from the last path component only, so a registry host that carries a :port (e.g. localhost:5000/cozystack/operator) on a no-tag ref is preserved instead of being truncated to the host. Verified against all three collect_refs shapes plus port/tag combinations; the top-of-file no-port caveat is removed since the split is now port-safe.
| echo "$refs" | while IFS= read -r ref; do | ||
| [ -n "$ref" ] || continue | ||
| repo="${ref%@*}" | ||
| digest="${ref##*@}" | ||
| echo "▸ ${repo} ${digest}" | ||
| copy "$ref" "${repo}:${STABLE}" | ||
| copy "$ref" "${repo}:latest" | ||
| # Verify the stable tag now resolves to the exact rc digest (skip in dry-run). | ||
| if [ "$DRY_RUN" -eq 0 ]; then | ||
| got="$(skopeo inspect --format '{{.Digest}}' "docker://${repo}:${STABLE}" 2>/dev/null || echo '')" | ||
| if [ "$got" != "$digest" ]; then | ||
| echo "::error::${repo}:${STABLE} resolved to '${got}', expected '${digest}'" >&2 | ||
| exit 1 | ||
| fi | ||
| fi | ||
| done |
There was a problem hiding this comment.
Piping echo "$refs" to the while loop causes the loop to run in a subshell. If exit 1 is triggered inside the loop (e.g., when verification fails), it exits the subshell, but depending on the shell implementation and environment, it might not reliably terminate the parent script. Using a here-document avoids the subshell entirely and ensures consistent error propagation.
| echo "$refs" | while IFS= read -r ref; do | |
| [ -n "$ref" ] || continue | |
| repo="${ref%@*}" | |
| digest="${ref##*@}" | |
| echo "▸ ${repo} ${digest}" | |
| copy "$ref" "${repo}:${STABLE}" | |
| copy "$ref" "${repo}:latest" | |
| # Verify the stable tag now resolves to the exact rc digest (skip in dry-run). | |
| if [ "$DRY_RUN" -eq 0 ]; then | |
| got="$(skopeo inspect --format '{{.Digest}}' "docker://${repo}:${STABLE}" 2>/dev/null || echo '')" | |
| if [ "$got" != "$digest" ]; then | |
| echo "::error::${repo}:${STABLE} resolved to '${got}', expected '${digest}'" >&2 | |
| exit 1 | |
| fi | |
| fi | |
| done | |
| while IFS= read -r ref; do | |
| [ -n "$ref" ] || continue | |
| repo="${ref%@*}" | |
| digest="${ref##*}" | |
| echo "▸ ${repo} ${digest}" | |
| copy "$ref" "${repo}:${STABLE}" | |
| copy "$ref" "${repo}:latest" | |
| # Verify the stable tag now resolves to the exact rc digest (skip in dry-run). | |
| if [ "$DRY_RUN" -eq 0 ]; then | |
| got="$(skopeo inspect --format '{{.Digest}}' "docker://${repo}:${STABLE}" 2>/dev/null || echo '')" | |
| if [ "$got" != "$digest" ]; then | |
| echo "::error::${repo}:${STABLE} resolved to '${got}', expected '${digest}'" >&2 | |
| exit 1 | |
| fi | |
| fi | |
| done <<EOF | |
| $refs | |
| EOF |
There was a problem hiding this comment.
Leaving as-is here: the script runs under set -eu, and echo "$refs" | while … is a standalone pipeline (not part of an if/while/&& condition), so when the subshell hits exit 1 the pipeline returns non-zero and set -e terminates the parent script. I confirmed this by running the exact pattern under set -e — the echo after the loop does not execute. The loop body doesn't need to persist any variables back to the parent shell, so the subshell is harmless and a here-doc rewrite would add no behavioural change.
| # Talos variant (default) | ||
| helm template installer packages/core/installer -n cozy-system \ | ||
| --set bareNamespace=true \ | ||
| --set cozystackOperator.platformVersion=v$(COZYSTACK_VERSION) \ |
There was a problem hiding this comment.
If COZYSTACK_VERSION is empty or unset, v$(COZYSTACK_VERSION) evaluates to v. This sets platformVersion to v, which is invalid and overrides the default baked-in version. We should guard against empty COZYSTACK_VERSION using GNU Make's $(if ...) function.
--set cozystackOperator.platformVersion=$(if $(COZYSTACK_VERSION),v$(COZYSTACK_VERSION),) \
There was a problem hiding this comment.
Fixed in 69448ec. Wrapped the --set in $(if $(COZYSTACK_VERSION),v$(COZYSTACK_VERSION),) so an empty COZYSTACK_VERSION passes an empty value instead of a bare v. With an empty value the installer template's {{- if .Values.cozystackOperator.platformVersion }} is false, so no COZYSTACK_VERSION env is rendered and the operator keeps its baked-in version. Applied to all three variant renders (lines 56/64/73).
| helm template installer packages/core/installer -n cozy-system \ | ||
| --set bareNamespace=true \ | ||
| --set cozystackOperator.variant=generic \ | ||
| --set cozystackOperator.platformVersion=v$(COZYSTACK_VERSION) \ |
There was a problem hiding this comment.
If COZYSTACK_VERSION is empty or unset, v$(COZYSTACK_VERSION) evaluates to v. This sets platformVersion to v, which is invalid and overrides the default baked-in version. We should guard against empty COZYSTACK_VERSION using GNU Make's $(if ...) function.
--set cozystackOperator.platformVersion=$(if $(COZYSTACK_VERSION),v$(COZYSTACK_VERSION),) \
There was a problem hiding this comment.
Fixed in 69448ec — same $(if ...) guard applied to this generic-variant render; see the reply on line 56 for the rationale.
| helm template installer packages/core/installer -n cozy-system \ | ||
| --set bareNamespace=true \ | ||
| --set cozystackOperator.variant=hosted \ | ||
| --set cozystackOperator.platformVersion=v$(COZYSTACK_VERSION) \ |
There was a problem hiding this comment.
If COZYSTACK_VERSION is empty or unset, v$(COZYSTACK_VERSION) evaluates to v. This sets platformVersion to v, which is invalid and overrides the default baked-in version. We should guard against empty COZYSTACK_VERSION using GNU Make's $(if ...) function.
--set cozystackOperator.platformVersion=$(if $(COZYSTACK_VERSION),v$(COZYSTACK_VERSION),) \
There was a problem hiding this comment.
Fixed in 69448ec — same $(if ...) guard applied to this hosted-variant render; see the reply on line 56 for the rationale.
| DRY_RUN=0 | ||
| [ "${2:-}" = "--dry-run" ] && DRY_RUN=1 | ||
|
|
||
| command -v yq >/dev/null || { echo "yq (mikefarah) is required" >&2; exit 1; } |
There was a problem hiding this comment.
command -v yq only checks if any binary named yq is installed. If the user has python-yq or another variant installed, the script will fail with syntax errors because it uses mikefarah-specific syntax. It is safer to verify that the installed yq is indeed the mikefarah version, similar to the check in the Makefile.
| command -v yq >/dev/null || { echo "yq (mikefarah) is required" >&2; exit 1; } | |
| yq_version=$(yq --version 2>&1 || true) | |
| case "$yq_version" in | |
| *mikefarah*) ;; | |
| *) echo "yq (mikefarah) is required" >&2; exit 1 ;; | |
| esac |
There was a problem hiding this comment.
Fixed in 2f92149. Added a yq --version | grep -q mikefarah check after the command -v yq probe so python-yq and other variants are rejected — the collect_refs queries use mikefarah-specific syntax. This mirrors the existing build-deps check in the Makefile.
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
NOT LGTM — the core promotion script aborts against the real package tree, the nightly job over-selects branches, and several release artifacts/docs drift from the new flow.
Business context: #2677 wants every git tag write-once and stable vX.Y.Z created only by promoting an already-built, e2e-tested rc — retagging the rc's images by digest so stable is bit-for-bit the rc, never a rebuild. This PR removes the five force-retag sites, adds promote-rc.yaml + hack/promote-retag.sh, decouples the operator's reported version from the image (COZYSTACK_VERSION env), and adds dark-by-default nightly + retention workflows. It is stacked on #2983 (refactor/build-matrix-2937) and must merge after it.
The version-decoupling half is solid: the pkg/version env override plus the Makefile platformVersion threading render correctly in all three operator variants with and without the version (I rendered all six combinations). The blockers are in the promotion script, the nightly job, and asset/doc consistency.
Blockers
B1 — hack/promote-retag.sh collects third-party and malformed image refs; promotion aborts before completing, or tries to push into registries it does not own.
File: hack/promote-retag.sh:37-47 (collect_refs), :53-61 (ref_repo), :87-102.
Evidence: the shape-1 query selects every string scalar containing @sha256:, and shape-2 every {repository,digest} map — not only releasable cozystack images. Against the real tree this yields, among others:
packages/system/postgres-operator/values.yaml:17-19→docker.io/clastix/kubectl@sha256:…(third-party); the script runsskopeo copy … docker://docker.io/clastix/kubectl:vX.Y.Zand:latest→ push to a registry the CI app cannot write.packages/system/ingress-nginx/values.yaml:19→ghcr.io/kvaps/…@sha256:…(foreign namespace) → same push failure.packages/system/kamaji/values.yaml:39→ the CLI-arg string--migrate-image=ghcr.io/…@sha256:…;ref_reporeturns the malformed repo--migrate-image=ghcr.io/…/kamaji, soskopeo copy docker://--migrate-image=…is an invalid reference.packages/system/kubeovn/values.yaml:68,packages/system/keycloak-operator/values.yaml:5,packages/system/kilo/values.yaml:4→ baretag:scalars likev1.15.10@sha256:…/latest@sha256:…;ref_repoyields a bare invalid repo (v1.15.10,latest).
Underset -euthe first failingskopeoaborts the whole script, so stable promotion — the central feature of this PR — cannot complete. The local validation in the PR description used a synthetic single-image tree, so it never exercised the real mixed-registry values.
Impact: promotion is broken in practice; no stable can be cut by this flow.
Fix: restrict collection to cozystack-owned, fully-qualified<registry>/<repo>@<digest>refs (anchor the registry host and namespace; excludetag:-only scalars and arg strings), or drive promotion from an explicit produced-images manifest instead of scraping allvalues.yaml. Add a test over the real package set.
B2 — nightly-rc.yaml target discovery matches release-X.Y.Z[-suffix] staging branches, not just release-X.Y maintenance branches.
File: .github/workflows/nightly-rc.yaml:57-59.
Evidence: the case glob origin/release-[0-9]*.[0-9]* matches origin/release-1.4.0, origin/release-1.4.0-rc.2, and origin/release-1.5.0-nightly.20260624 — all staging branches that tags.yaml:223-241 ("Create release branch", release-${GITHUB_REF#refs/tags/v}) creates and pushes for every rc/nightly/stable tag and never deletes (retention.yaml prunes nightly tags+releases, not branches). The step comment states the intent is "main + every release-X.Y maintenance branch"; the code selects more. For each spurious target the job cuts a v…-nightly.<date> tag at that branch's HEAD and pushes it, which then triggers further builds.
Impact: once NIGHTLY_RC_ENABLED is set, the nightly job emits bogus tags and spurious CI runs. Gated dark today, so not release-breaking yet, but it is a wrong-artifact bug in new code.
Fix: anchor the match to exactly two numeric components, e.g. select only refs matching ^origin/release-[0-9]+\.[0-9]+$.
B3 — the stable restamp leaves openapi.json with an rc-derived version.
File: .github/workflows/promote-rc.yaml:169-173, Makefile:87-89.
Evidence: the restamp runs make manifests assets-cozypkg openapi-json COZYSTACK_VERSION="${STABLE_VERSION}". manifests (operator env), assets-cozypkg (-ldflags … Version=v$(COZYSTACK_VERSION)) and check-readiness all honor COZYSTACK_VERSION, but openapi-json sets VERSION=$(shell git describe --tags --always …). At promote time the checkout is the rc staging branch and the stable tag does not exist yet (it is created only at PR merge), so git describe resolves to vX.Y.Z-rc.N-… and the stable release's openapi.json info.version reports the rc version. That asset is then uploaded via make upload_assets.
Impact: a published stable asset ships rc version metadata, defeating the restamp step's stated purpose.
Fix: make openapi-json honor COZYSTACK_VERSION (fall back to git describe only when it is unset), mirroring the other version-stamped targets.
B4 — docs/release.md still documents the removed force-retag flow as the current, load-bearing mechanism.
File: docs/release.md:83, 177, 239-241, 531-546 (not touched by this PR).
Evidence: the PR removes the force sites and adds the promote flow, but docs/release.md still says the finalize step "Moves the tag … by force-pushing a tag" (:83, :177), "Force-move the tag … Intentional and load-bearing for the digest-bake flow" (:239), maintenance branch "force-updated if newer" (:240), and keeps the entire "Force-retagging" section (:531-546) framing #2677 as an "open RFC" with only "Stage 1" landed. None of that holds after this PR.
Impact: the canonical release runbook now actively misdescribes how releases are cut; a maintainer following it would push a stable tag by hand (which the new flow and intended tag-protection are designed to reject) and would not learn about promote-rc.yaml.
Fix: update docs/release.md (and docs/agents/releasing.md where it references the flow) to the write-once + promote model implemented here.
Non-blocking follow-ups
- Nightly tags
vX.Y.Z-nightly.<date>DO match thev*.*.*filter intags.yaml/release-e2e.yaml(the*segments absorb the-nightly.<date>suffix), so a nightly triggers the full stable-prep machinery — including a write-onceapi/apps/v1alpha1/vX.Y.Z-nightly.<date>Go-module tag (tags.yaml:153-172) and an auto-opened release PR per nightly. That contradicts #2677 goal 3 (rc/nightly must not get Go-module tags) and would permanently pollute the module proxy — the exact failure mode #2677 exists to prevent. Scope the api-submodule tag and release-PR steps to stable tags only. promote-rc.yaml:147-148rewrites the rc→stable substring withsed -i "s/${RC_VERSION}/${STABLE_VERSION}/g"; the.characters inRC_VERSIONare unescaped regex metacharacters. Harmless for the current literal but imprecise — escape or anchor.packages/core/installerships no helm-unittest covering the newCOZYSTACK_VERSIONenv rendering (3 variants × version set/unset). Add coverage so the env-block restructure cannot silently regress.promote-rc.yaml:149-153("Prepare stable branch") pushes the stable branch without-f; a re-run after a partial failure where the remote branch already diverged would fail the push. Minor idempotency gap (the concurrency group mitigates concurrent runs).
This PR is stacked on #2983 (refactor/build-matrix-2937) and should merge after it.
github-actions
Bot
added
size/XXL
|
Aleksei Sviridkin (@lexfrei) addressed across Blockers
Non-blocking
One I did not fold in: nightly tags still hard-fail |
Re-sync the release-promotion PR (#3017) onto the #2983 base, which now contains main. The conflicts were release-pipeline files, resolved keeping #3017's write-once + promotion logic and grafting main's hardening: - tags.yaml: keep #3017's is_stable gate on the api-submodule tag and the auto-opened release PR; take main's SHA-pinned actions. - auto-release.yaml: kept deleted (#3017 removes the nightly auto-bump; main's hardening of a soon-deleted file is moot). - installer/values.yaml: keep #3017's platformVersion field + comment; take main's v1.5.0 operator image. actionlint clean; installer + promote-retag unit tests pass. Signed-off-by: Myasnikov Daniil <[email protected]>
e074864 to
9826400
Compare
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
NOT LGTM — all four prior blockers are resolved (with evidence and a real regression test), but the PR ships a broken nightly build path that should be fixed before merge.
The prior blockers are genuinely fixed, and I want to credit that:
- B1 (promote-retag.sh scraped third-party/malformed refs): fixed.
hack/promote-retag.shfilters every ref through an allowlist (case "$_repo" in "${REGISTRY}/"*), defaultghcr.io/cozystack/cozystack), dedups on<repo>@<digest>, and aborts loudly on an empty result. Pinned by a newhack/promote-retag_test.batsthat runs the selector--dry-runover the real tree and asserts no non-cozystack ref leaks, plus aREGISTRY-override case — and it runs undermake unit-testsin CI. - B2 (nightly glob over-selected staging branches): fixed via
grep -qE '^release-[0-9]+\.[0-9]+$'. - B3 (openapi.json restamped with the rc version): fixed —
make openapi-jsonhonorsCOZYSTACK_VERSION, andpromote-rc.yamlpassesSTABLE_VERSION. - B4 (docs/release.md documented the removed force-retag flow): fixed.
Blocker
B1: the nightly build path is broken
nightly-rc.yaml cuts a v<maj>.<min>.0-nightly.<date> tag and its own header claims the tag "triggers the normal rc build/e2e path (tags.yaml + release-e2e.yaml)". But release-e2e.yaml no longer exists, and tags.yaml is the only tag-push-triggered workflow — and its Parse-tag regex ^v(\d+\.\d+\.\d+)(-(?:alpha|beta|rc)\.\d+)?$ rejects -nightly.<date>, so the run hits core.setFailed. The net effect: an enabled nightly fails tags.yaml at parse and never builds or runs e2e.
It's gated off by default (NIGHTLY_RC_ENABLED), so nothing runs today — but enabling nightlies is a documented next step, and shipping a path that is dead-on-arrival the moment it's turned on is the same class of nightly-targeting defect that was a blocker in the previous round. Fix: extend the Parse-tag regex to accept -nightly.<date> (the is_stable gating already keeps the Go-module tag and the release-PR stable-only), or correct the stale release-e2e.yaml reference and define the real nightly build/e2e path.
Minor (non-blocking): the installer env helm-unittest covers talos and hosted but not the generic variant; the COZYSTACK_VERSION block is variant-independent, so the decoupling is covered, but a generic case would close the gap.
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
LGTM — the nightly blocker from my last review is resolved, the four earlier blockers haven't regressed, and the new mirror path is well-built and unit-tested.
Nightly path (the remaining blocker): resolved by redesign rather than a patch. nightly-rc.yaml is gone; nightlies no longer cut a git tag at all. nightly.yaml mints a non-tag version 0.0.0-nightly.<date>, mirrors the latest main build's images by digest (no rebuild), re-publishes the packages artifact + installer chart + nocloud disk to GHCR, and runs the full e2e against that closure. Because no git tag is pushed, tags.yaml's Parse-tag regex is never reached for a nightly — the core.setFailed path I reported is gone. is_stable gating is unchanged, so the Go-module tag and release PR stay stable-only. docs/release.md is updated accurately. Nice touches: the resolve step validates the source revision is a bare 40-hex sha before using it as a checkout ref, and the mirror reuses the proven allowlist/exclude selector with a digest round-trip verify after each copy.
One thing to fix before enabling nightlies (non-blocking for this PR): build-disk publishes the new cozystack-nocloud GHCR package via oras push without --annotation "org.opencontainers.image.source=https://github.com/cozystack/cozystack". No existing workflow publishes that package, so the first run creates it unlinked and the immediately-following oras tag can hit permission_denied: write_package. Adding the source annotation to the push avoids it.
Two minor notes: (1) in nightly-mirror.sh, ref collection globs */*/values.yaml (depth-2) while the host rewrite uses find -name values.yaml (any depth) — harmless for the current layout, but a deeper ref would be rewritten without being mirrored; (2) resolve reads the sha from cozystack-packages:main but the pull re-fetches :main by tag — pinning the pull to the resolved digest closes a small TOCTOU window.
(Merge-readiness, not code: the branch is stacked on its feature base and currently conflicting; it'll need that resolved before merge.)
The base branch was changed.
6308617 to
9f82616
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (1)
packages/core/installer/tests/cozystack_version_test.yaml (1)
16-74: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a generic-variant regression case.
The shared
env:refactor also changed thegenericbranch, but this suite only exercisestalosandhosted. A regression in thegenericKUBERNETES_SERVICE_*rendering would currently slip through.Example test to add
+ - it: generic + platformVersion sets COZYSTACK_VERSION and the API endpoint env + set: + cozystackOperator.variant: generic + cozystackOperator.platformVersion: v1.4.0 + cozystack.apiServerHost: api.example.test + cozystack.apiServerPort: "6443" + asserts: + - equal: + path: spec.template.spec.containers[0].env + value: + - name: COZYSTACK_VERSION + value: v1.4.0 + - name: KUBERNETES_SERVICE_HOST + value: api.example.test + - name: KUBERNETES_SERVICE_PORT + value: "6443" + documentSelector: + path: kind + value: Deployment🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/installer/tests/cozystack_version_test.yaml` around lines 16 - 74, The current test suite in cozystack_version_test.yaml only covers talos and hosted, so add a regression case for the generic variant to validate the shared env rendering. Extend the existing installer test cases around the cozystackOperator.variant and cozystackOperator.platformVersion matrix with a generic branch assertion that checks the Deployment container env output, especially the KUBERNETES_SERVICE_* values and whether COZYSTACK_VERSION is present or omitted as expected.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/nightly.yaml:
- Around line 79-95: The nightly version in the Resolve source commit and
version step is still mutable because it only uses the date, so same-day reruns
reuse the same tag. Update the version generation logic to include a run-unique
component from the workflow context in addition to the date, and keep the
existing sha validation and output wiring intact so each nightly build gets a
distinct, write-once version.
- Around line 79-105: The nightly workflow resolves a commit SHA from
cozystack-packages:main, but Pull baked package tree still fetches the mutable
:main tag, which can drift between steps. Update the workflow to pin the
artifact pull to the same digest or immutable reference resolved by the Resolve
source commit and version step, and pass that pinned reference into the flux
pull artifact command so Checkout source commit and the baked tree always come
from the same build.
- Around line 97-101: The source checkout step is leaving persisted credentials
in the worktree, which can carry the token into copied sandboxes. Update each
actions/checkout use for the source repo, including the Checkout source commit
step, to set persist-credentials: false so .git/config does not retain
credentials before the checkout is copied to /tmp/$SANDBOX_NAME.
In @.github/workflows/promote-rc.yaml:
- Around line 101-107: The Checkout rc staging branch step is persisting the
GitHub token in the local git config even though git remote set-url later
supplies the app token explicitly. Update the actions/checkout@v4 configuration
in this workflow to disable persisted credentials by setting persist-credentials
to false, keeping the existing ref, fetch-depth, fetch-tags, and token behavior
otherwise unchanged.
- Around line 45-49: The `actions/create-github-app-token` step in the
promote-rc workflow is using an unpinned version and grants broader owner-wide
access than needed. Update that step to use the same SHA-pinned action reference
used elsewhere in the repo, and add a `repositories` restriction set to `${{
github.event.repository.name }}` so the token is scoped only to the current
repository.
- Around line 91-93: The promotion workflow only checks for an existing release,
so a pre-existing stable git tag can slip through and fail later during the
merge step. Update the logic around the stableTag check in the promote-rc
workflow to also verify whether refs/tags/vX.Y.Z already exists before
proceeding, and fail early if either the GitHub release or the git tag is
present.
In @.github/workflows/pull-requests-release.yaml:
- Around line 123-134: Only treat the GitHub non-fast-forward rejection as a
recoverable warning in the updateRef try/catch. In the fast-forward block that
uses github.rest.git.updateRef and logs via console.log, inspect ffErr and
rethrow or fail the workflow for auth, API, or malformed-ref errors, while
keeping the existing warning path only for the descendant-check/non-fast-forward
case so other update failures do not get swallowed.
In @.github/workflows/retention.yaml:
- Around line 82-90: The nightly retention logic in the package pruning step is
counting and slicing per-page JSON instead of the full result set because `gh
api --paginate --jq` emits one array per page. Update the `ids` pipeline in the
retention workflow to combine all pages first with `jq -s` or `--slurp`, then
compute `total` and `drop_ids` from the merged array so `KEEP`, `total`, and
`drop_ids` work correctly across multi-page package listings.
In @.github/workflows/tags.yaml:
- Around line 58-60: The release existence check in the tags workflow is using a
single skip output for two different meanings, which causes stable promotion
runs to skip downstream jobs like docs updates. Update the logic around the
release lookup in the tags workflow so it emits separate outputs from the
existing prepare-release step, such as release_exists for the draft-release
guard and skip_build for rebuild suppression, then change downstream job
conditions to depend on the correct output instead of
prepare-release.outputs.skip.
In `@docs/release.md`:
- Line 83: The release guide uses conflicting stable-tag workflows, so update
the release steps around the stable tag flow in the release doc to describe one
consistent model only. Align the numbered release process and the rc promotion
section so that vX.Y.Z is created only once via the merge/rc promotion path, and
remove any instructions that tell maintainers to push stable tags directly
before CI or otherwise bypass the promotion flow. Make sure the wording around
the release steps and the stable-tag policy references the same behavior across
the relevant sections.
In `@hack/promote-retag.sh`:
- Around line 105-111: The promotion loop in promote-retag.sh currently
overwrites ${repo}:${STABLE} unconditionally via copy, which can retag an
already-published stable image to different bytes. Update the loop around the
ref processing to inspect the current destination tag first, compare its digest
with the source digest, and skip the copy when they match or fail fast when they
differ; keep the existing mutable behavior for ${repo}:latest.
In `@pkg/version/version_test.go`:
- Around line 36-39: The version test setup only sets COZYSTACK_VERSION when
tt.set is true, so the unset case can still pick up an existing ambient value
and miss the fallback path. Update the t.Run block in version_test.go to
explicitly clear or unset COZYSTACK_VERSION for the tt.set false branch before
calling the version lookup, keeping the behavior isolated within the test table
case. Use the existing tt.set, tt.env, and t.Setenv usage in TestVersion to
place the fix.
---
Nitpick comments:
In `@packages/core/installer/tests/cozystack_version_test.yaml`:
- Around line 16-74: The current test suite in cozystack_version_test.yaml only
covers talos and hosted, so add a regression case for the generic variant to
validate the shared env rendering. Extend the existing installer test cases
around the cozystackOperator.variant and cozystackOperator.platformVersion
matrix with a generic branch assertion that checks the Deployment container env
output, especially the KUBERNETES_SERVICE_* values and whether COZYSTACK_VERSION
is present or omitted as expected.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f4c27c7c-217b-4167-adad-4d7795441b86
📒 Files selected for processing (21)
.github/workflows/auto-release.yaml.github/workflows/nightly.yaml.github/workflows/promote-rc.yaml.github/workflows/pull-requests-release.yaml.github/workflows/retention.yaml.github/workflows/tags.yamlMakefiledocs/release.mdhack/nightly-mirror.shhack/nightly-mirror_test.batshack/promote-retag.shhack/promote-retag_test.batspackages/core/installer/Makefilepackages/core/installer/templates/cozystack-operator.yamlpackages/core/installer/tests/cozystack_version_test.yamlpackages/core/installer/values.yamlpackages/system/dashboard/images/console/apps/console/src/App.tsxpackages/system/dashboard/images/console/apps/console/src/lib/config.tspackages/system/dashboard/templates/configmap.yamlpkg/version/version.gopkg/version/version_test.go
💤 Files with no reviewable changes (1)
- .github/workflows/auto-release.yaml
|
Aleksei Sviridkin (@lexfrei) The broken nightly build path no longer exists — the nightly flow was redesigned after your review. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/promote-rc.yaml (1)
1-31: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winSerialize promotions by stable version, not
rc_tag.The workflow-level concurrency described here still lets
vX.Y.Z-rc.1andvX.Y.Z-rc.2run at the same time, even though both mutate the same stable refs (vX.Y.Z,release-X.Y.Z, and the draft stable release). That leaves a race where one promotion can overwrite the other before the merge-time write-once guard ever runs. A stable-version key, or even a single global promotion group, would avoid that.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/promote-rc.yaml around lines 1 - 31, The workflow concurrency key in Promote RC is too specific to rc_tag and allows multiple rc promotions for the same stable version to run in parallel. Update the concurrency group in the workflow so it serializes by stable version (derived from the rc_tag, or use a single global promotion group) rather than by the full rc tag, ensuring only one promotion can mutate the same stable refs at a time.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/release.md`:
- Around line 174-181: Rename the section heading in docs/release.md so it
accurately reflects all the workflows it describes, not just tag pushes. Update
the title near the list of tags.yaml, promote-rc.yaml,
pull-requests-release.yaml, and update-releasenotes.yaml to a broader
trigger-based heading, and keep the wording aligned with those workflow names so
maintainers can find the right entry point quickly.
---
Outside diff comments:
In @.github/workflows/promote-rc.yaml:
- Around line 1-31: The workflow concurrency key in Promote RC is too specific
to rc_tag and allows multiple rc promotions for the same stable version to run
in parallel. Update the concurrency group in the workflow so it serializes by
stable version (derived from the rc_tag, or use a single global promotion group)
rather than by the full rc tag, ensuring only one promotion can mutate the same
stable refs at a time.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 326ada2d-d24a-45be-888d-25ceb289366f
📒 Files selected for processing (8)
.github/workflows/nightly.yaml.github/workflows/promote-rc.yaml.github/workflows/pull-requests-release.yaml.github/workflows/retention.yaml.github/workflows/tags.yamldocs/release.mdhack/promote-retag.shpkg/version/version_test.go
🚧 Files skipped from review as they are similar to previous changes (5)
- pkg/version/version_test.go
- .github/workflows/pull-requests-release.yaml
- hack/promote-retag.sh
- .github/workflows/retention.yaml
- .github/workflows/tags.yaml
The console config ConfigMap stripped the leading v (trimPrefix "v") while the operator's COZYSTACK_VERSION and the vX.Y.Z release tags keep it, so the dashboard displayed 1.4.0 where the operator reported v1.4.0. Keep the tag verbatim, and build the baked VITE_APP_VERSION fallback as v$(COZYSTACK_ VERSION) too so the deploy-time and build-time version strings stay consistent with each other and with the operator. Assisted-By: Claude <[email protected]> Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
|
Aleksei Sviridkin (@lexfrei) thanks — both blockers were real; verified and fixed (5 commits on the rebased branch). B1 (wrong registry) — B2 (stable chart never published) — Follow-ups:
|
…o finalize Promotion mutated the registry at dispatch time — retagging ~37 images to vX.Y.Z (+:latest), publishing the stable cozy-installer chart — before the promote PR's e2e ran and before any merge. An abandoned promotion then left stable-named bytes for a version that never shipped, and the (correct) image write-once check wedged any later re-promotion to the same version. Move every irreversible registry side effect into pull-requests-release.yaml finalize, which runs only after the promote PR merged (= after its e2e passed), making promotion atomic. promote-rc.yaml now only stages the release-X.Y.Z branch, drafts the stable release, and opens the PR; an abandoned promotion leaves nothing but an overwritable draft + branch. Folded into the same restructure: - Cut the api/apps/v1alpha1/vX.Y.Z Go-module tag in finalize, at the stable tag's commit. It was gated in tags.yaml on `release_exists==false && is_stable`, unsatisfiable under promotion (the draft always pre-exists), so promoted stables never got their submodule tag — a regression for Go consumers. Finalize is where the stable tag is born, so it cuts the matching submodule tag write-once (stable only). - Gate :latest by max-semver. promote-retag.sh now moves :latest only when MOVE_LATEST=1 (default off); finalize passes it the same make_latest decision the release gets, so a patch on an older line moves neither the release's `latest` nor the images' :latest. Added MOVE_LATEST bats coverage. - Stamp platformVersion into the published stable chart's default values (uncommitted, at package time) so `helm --install --version X.Y.Z` reports the stable version in telemetry instead of the rc version baked into the operator image. Not committed to the branch — that would leak platformVersion=vX.Y.Z onto main and mis-stamp the next rc. - promote-rc: serialize all promotions (static concurrency group, not per-rc); label the PR full-e2e so the full suite runs; scope the rc->stable sed to the depth-2 values.yaml glob plus the kubernetes .tag files (the build's own write set); tolerate a leftover draft so re-promotion is not wedged. - finalize: assert the PR author is cozystack-ci[bot], and run on an ephemeral oracle-vm runner (needs skopeo/helm). Co-Authored-By: Claude Fable 5 <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
…eject hand-pushed stables The tag workflow assumed the old hand-pushed-stable flow in several places that break (or misbehave) under rc-promotion: - Drop PUBLISH_FLOATING from the Build step. This job now only ever builds a prerelease (a stable tag skips via its pre-existing draft, or is rejected — below), so moving :latest here would track the newest rc, and an rc on an old line would drag :latest backwards. :latest belongs to promotion (finalize). - Publish rc releases. tags.yaml created the rc release as a draft and nothing ever published it, so the community could not see the rc page or download its nocloud-amd64.raw.xz — defeating the rc's stated purpose. Publish it (prerelease:true, make_latest:false) once assets are uploaded; rc tags are write-once, nothing to protect with draft status. - Reject hand-pushed stable tags. A stable vX.Y.Z with no pre-existing draft is a hand-push down the legacy full-rebuild path (full rebuild, moved :latest, rogue release PR). Fail fast with a message pointing at promote-rc.yaml. This replaces two now-unreachable steps that shared the same gate: the submodule Go-module tag (moved to finalize in the previous commit) and "Create pull request if not exists" (promote-rc owns PR creation). - Gate generate-changelog and update-website-docs on stable (ref_name has no '-'), so each rc no longer opens a changelog-vX.Y.Z-rc.N PR that never merges and an "update managed apps for vX.Y.Z-rc.N" website PR. - Paginate the release lookups (check_release, draft-reuse): a promote-created draft can sit past the default first 30 and be missed. Co-Authored-By: Claude Fable 5 <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
… regex - nightly.yaml: pass the registry tokens to `skopeo login` over stdin (--password-stdin) instead of `-p` on argv, matching promote-rc/finalize and keeping the token out of the process list. - dashboard configmap: require digits-then-dot (^v?[0-9]+\.) when deriving the displayed platform version from the console image tag. The old ^v?[0-9] let a tagless, port-carrying ref (ghcr.io:443/.../console, whose last ":"-segment is "443/.../console") pass as a version. Verified: v1.5.0 / 1.5.0 still surface; the port ref and :latest no longer do. Co-Authored-By: Claude Fable 5 <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
NOT LGTM — the transactional-promotion rework adds a "re-promotion never wedges" invariant that the stable-branch push does not actually honor.
Business context: implements #2677 — a stable vX.Y.Z becomes a digest-retagged rc (immutable tags, no rebuild); the operator's reported version is decoupled from the image via COZYSTACK_VERSION.
Blockers
B1: release-X.Y.Z staging-branch push wedges re-promotion
File: .github/workflows/promote-rc.yaml:200
Issue: The stable staging branch is built with git checkout -B + git commit -s (a fresh, timestamp-dependent SHA each run) and pushed with a plain, non-force git push origin HEAD:refs/heads/${STABLE_BRANCH}. On a second dispatch — a retry after a later step fails (Stage rc assets / Restamp / Create draft / Open PR), or promoting a newer rc to the same version — the new commit is a sibling of the already-pushed one (same parent = rc tip, different SHA), so the push is rejected non-fast-forward and promotion stops until the remote branch is deleted by hand.
Evidence: The validate step (lines 105–115) deliberately tolerates a leftover draft so as not to "wedge re-promotion of a newer rc to the same version," and the workflow header (lines 15–17) states an abandoned promotion "leaves nothing but an overwritable … branch: nothing to wedge a later re-promotion." The plain push contradicts both. The sibling step in tags.yaml Create release branch (lines 266–277) already implements the correct compare-before-force for its release-X.Y.Z-rc.N staging branch.
Impact: Fails safe — this runs at dispatch, before any registry side effect, so no stable-named bytes are produced — but a second promotion attempt errors at the push and needs manual branch deletion, breaking the re-promotion path this PR was designed to support.
Fix: Reuse the compare-before-force pattern from tags.yaml Create release branch (skip when unchanged, push -f + log when the branch genuinely moves); the header already treats this as a mutable/overwritable staging branch.
Non-blocking follow-ups
- PR description drift: the body names
nightly-rc.yamlandNIGHTLY_RC_ENABLED, but the shipped workflow isnightly.yamlgated byNIGHTLY_ENABLED(nightly.yaml:43) — the "Out of scope" checklist points an admin at the wrong variable. packages/system/dashboard/Makefile:34bakesAPP_VERSION=v(bare) whenCOZYSTACK_VERSIONis empty for local builds, unlike the$(if $(COZYSTACK_VERSION),v…,)guard the root Makefile uses. Dev-only cosmetics (the config.json path overrides it at deploy).
| git add -A | ||
| git commit -s -m "Prepare release v${STABLE_VERSION} (promoted from ${RC_TAG})" \ | ||
| || echo "No tag-string changes to commit (digests already stable)" | ||
| git push origin "HEAD:refs/heads/${STABLE_BRANCH}" |
There was a problem hiding this comment.
B1 (blocker): this push is non-force, but git checkout -B + git commit -s above produce a fresh SHA each run, so a second dispatch (retry after a later step fails, or promoting a newer rc to the same version) creates a sibling commit and this push is rejected non-fast-forward — the promotion then wedges until the remote branch is deleted by hand.
That contradicts the validate step (105–115), which tolerates a leftover draft precisely so a newer rc can be re-promoted, and the header's "overwritable … branch: nothing to wedge a later re-promotion." Fails safe (before any registry write), but breaks the re-promotion path by design.
Reuse the compare-before-force pattern already in tags.yaml's Create release branch step (skip when unchanged, push -f + log when it genuinely moves).
There was a problem hiding this comment.
Fixed in 8aebc34. The stable-branch push now mirrors tags.yaml's Create release branch step: after the commit it compares the local SHA against the remote release-X.Y.Z branch and force-pushes (with a ::notice::) only when they differ, skipping when they already match. A re-dispatch's sibling commit now overwrites the staging branch instead of being rejected non-fast-forward, so re-promotion no longer wedges — consistent with the validate step's leftover-draft tolerance and the header's overwritable-branch invariant.
Address review feedback from lexfrei on .github/workflows/promote-rc.yaml:200. The "Prepare stable branch" step built the release-X.Y.Z branch with `git checkout -B` + `git commit -s` (a fresh, timestamp-dependent SHA every run) and pushed it with a plain non-force push. On a re-dispatch — retrying after a later step failed, or promoting a newer rc to the same version — the new commit is a sibling of the already-pushed one (same parent, different SHA), so the push is rejected non-fast-forward and promotion wedges until the branch is deleted by hand. That contradicts both the validate step (which tolerates a leftover draft precisely so a newer rc can be re-promoted) and the workflow header's "overwritable branch, nothing to wedge a later re-promotion". Reuse the compare-before-force pattern from tags.yaml's Create release branch step: skip when the remote already matches, force-push + log when the branch genuinely moves. It is a mutable staging branch, so overwriting it is correct. Co-Authored-By: Claude Fable 5 <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
…ION build-arg Address non-blocking review feedback from lexfrei on packages/system/dashboard/Makefile:34. When COZYSTACK_VERSION is empty (a local build with no matching git tag) the build-arg baked a bare `APP_VERSION=v` into the console bundle. Match the guard the root Makefile already uses for platformVersion so an empty version yields an empty APP_VERSION instead of "v". Dev-only cosmetics — the config.json path overrides the displayed version at deploy time — but it removes the mismatch. Co-Authored-By: Claude Fable 5 <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
|
Aleksei Sviridkin (@lexfrei) Thanks for the catch — all three addressed:
|
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
LGTM — the retag-by-digest core, the transactional promote→finalize split, and the version-decoupling half are all sound; every prior blocker is resolved and the one new model-flagged P1 does not reproduce.
Business context update: promotion is now transactional — promote-rc.yaml performs zero registry mutation at dispatch (it only pushes the release-X.Y.Z staging branch, drafts the release, and opens the PR); every irreversible side effect (retag rc→stable by digest, move :latest, publish the stable cozy-installer chart, cut the write-once vX.Y.Z + api/apps/v1alpha1/vX.Y.Z tags) runs in pull-requests-release.yaml finalize, post-merge. (previously: retag/chart/tag ran at promote-dispatch time.)
Verified resolved since the last round:
- Staging-branch push wedge →
promote-rc.yamlnow compare-before-force (skip-if-unchanged,-f+log on a genuine move), matchingtags.yaml's Create-release-branch step. - Registry mismatch (retag hit OCIR, images live on GHCR) → dissolved by the rework: finalize sets
REGISTRY: ghcr.io/cozystack/cozystackand logs intoghcr.io; the script default matches. - Stable
cozy-installernever published → finalize's "Publish stable cozy-installer chart" packages from the merged stable tree andhelm pushes:vX.Y.Z(+:latest, gated by the samemake_latestdecision the release uses).
One reviewer flagged a P1: "gh release create --draft in the promote job auto-creates the vX.Y.Z git tag early, so finalize's write-once tag step then sees a tag at a different SHA and fails." Disproven empirically: a --draft release created against a non-existent tag yields an untagged-… release and no refs/tags/… ref (GET git/refs/tags/<tag> → 404); the tag is materialized only when the draft is published. Finalize creates the write-once tag at the merge commit before publishing the draft, so the tag correctly pins the tested merge commit. No early tag, no wedge.
Non-blocking follow-ups
-
hack/promote-retag.shsilently omits 8 cozystack-namespace images that vendor their digest in therepository:+tag: <upstream-ver>@sha256:…shape (kamaji, kilo, linstor-csi, linstor-gui, metallb-controller/-speaker, piraeus-server, redis-operator).collect_refsshape-1 picks up thetagscalar, butref_reporeducesv1.5.0@sha256:…tov1.5.0, which the${REGISTRY}/*allowlist then drops (shape-2 needs a separatedigest:key these values don't have). Functionally safe today — they carry upstream-version tags, not the cozystack release version, so there is no rc→stable tag to promote and deploys resolve by digest — but the exclusion is an artifact of ref parsing, not an intended filter, and the script comment/test rationale frameskilo(aghcr.io/cozystack/cozystack/*image) as a "bare upstream tag" alongside the genuinely-foreign kube-ovn/keycloak. Normalizing those values to therepository+digestshape later would silently start creatingkamaji:vX.Y.Ztags. Worth a test pinning the exact intended retag set, plus a comment correction. -
The dashboard chart has a helm-unittest suite (
packages/system/dashboard/tests/), but the newconfigmap.yamlversion-extraction branch — including the deliberateregexMatch "^v?[0-9]+\\."guard against a port-carrying tagless ref — ships without a unittest. Consistent with how the rest of this PR pins every new piece of logic; a 3-case test (normal tag setsversion: vX.Y.Z; port-in-ref skips;vprefix preserved) closes it. -
The piped
whileinpromote-retag.shwas flagged as swallowingexit 1— investigated and dismissed: thewhileis the terminal pipeline stage, so its non-zero exit is the pipeline's status andset -euaborts the script; the write-once and verify failures do abort correctly.
The zizmor "Audit workflows" gate (added in #3223) is red on main: five actions/* references in the release workflows from #3017 are pinned to a floating tag instead of a commit SHA (blanket unpinned-uses policy, High severity): - nightly.yaml (x3) actions/checkout@v4 - promote-rc.yaml actions/checkout@v4 - retention.yaml actions/create-github-app-token@v1 Pin each to the SHA the rest of the repo already standardizes on: - actions/checkout -> 34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - actions/create-github-app-token -> d72941d797fd3113feb6b93fd0dec494b13a2547 # v1 `zizmor --offline .github/workflows/` now reports no findings. Co-Authored-By: Claude Opus 4.8 <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
## What this PR does Adds `cut-prerelease.yaml` — a `workflow_dispatch` that becomes the **sole entry point for creating a pre-release tag** (`vX.Y.Z-alpha.N` / `-beta.N` / `-rc.N`), replacing the manual `git push origin HEAD:refs/tags/<tag>`. ## Why #2677 makes stable tags immutable, but tag *creation* was still only convention-enforced for **pre-releases**. Repo rulesets **can** already restrict *stable* `vX.Y.Z` creation to CI — a "Restrict creations" ruleset targeting `v*` with an exclude pattern for `v*-*` covers stable tags without any new workflow. What that approach **cannot** cover is pre-releases: `-rc` / `-alpha` / `-beta` tags were hand-pushed, so they can't be locked to CI while they still have a human entry point. This workflow closes that gap by making CI the sole creator of pre-release tags too. With it in place, an admin can extend "Restrict creations" (CI app as sole bypass) to **all** of `v*` — pre-release and stable alike — instead of only the stable subset. _(Correction: an earlier version of this description claimed a ruleset couldn't express "stable-only" creation. It can, via include + exclude patterns — thanks @IvanHunters. The workflow's real value is extending creation-control to pre-release tags, plus the audit trail, single entry point, guaranteed `base_ref`, and app-identity push that makes `tags.yaml` fire.)_ ## How it works - Generates the CozyStack CI app token (same app as `promote-rc.yaml`) and pushes the tag **as the app** — so `tags.yaml` fires (a tag pushed with the default `GITHUB_TOKEN` would not) and the push event's `base_ref` is populated (`tags.yaml`'s *Get base branch* step requires it). - Validates `^v\d+\.\d+\.\d+-(alpha|beta|rc)\.\d+$` and **rejects stable `vX.Y.Z`** — stable tags are cut write-once by the promote flow at a PR merge commit, never here. - Gates on the dispatcher having **write access** to the repo (`workflow_dispatch` already requires it; the explicit check is auditable defense-in-depth that fails closed). - Enforces dispatch from `main` (new minor's `vX.Y.0-*` only) or the matching `release-X.Y` branch; write-once (refuses an existing tag) with a stale-tip guard so a branch that advanced since dispatch fails cleanly instead of burning the tag name. - Pushes `HEAD:refs/tags/<tag>` from the branch tip — the exact form `docs/release.md` prescribes. Runs on `ubuntu-latest` (no build runner needed). Also updates `tags.yaml`'s two human-facing error messages and `docs/release.md` (the RC and patch flows, the workflow list, and a new repo-side tag-ruleset enforcement note) to point at the workflow instead of a manual push. Completes SHA-pinning on the touched release workflows (`nightly.yaml`, `promote-rc.yaml`, `retention.yaml`) — the zizmor "Audit workflows" gate was red on `main` for those. ## Rollout ordering The two `v*` tag rulesets have a hard order (now documented in `docs/release.md`): 1. **Immutability** (*Restrict updates* + *deletions*, no bypass) — safe to enable at any time; nothing in the flow ever moves or deletes a `v*` tag. 2. **Creation control** (*Restrict creations*, CI app sole bypass) — enable **only after this PR ships**, since this workflow is what makes CI the sole creator of pre-release tags. Enabling it earlier would lock maintainers out of cutting rc/alpha/beta. ## Validation - `actionlint` — clean. - `act -l` — job graph resolves (`cut` / `workflow_dispatch`). - `zizmor --offline` — no findings across the whole workflows dir (SHA-pinned actions, least-privilege `contents: read`). - Run blocks `bash -n` syntax-checked. - **Untested locally:** the workflow runtime (app-token auth, the real tag push, `base_ref` population, the `getCollaboratorPermissionLevel` gate) needs a live run with the CI secrets — the same "orchestration is unproven until it runs" caveat #3017 carried. ## Review follow-up Addressed the non-blocking findings from @IvanHunters' review (commit `fix(ci): address review findings`): 1. **[MINOR]** `main` dispatch now requires patch `Z == 0` (a new minor's pre-release), so a patch-line rc can't be cut from `main` content. 2. **[MINOR]** Added a stale-tip guard: if the dispatch branch advanced between dispatch and push, fail with a re-run hint instead of tagging a stale commit / burning the write-once name on an empty-`base_ref` failure. 3. **[MINOR]** Corrected the PR-body rationale above (rulesets can express stable-only creation via include + exclude). 4. **[NIT]** `ls-remote` now distinguishes "tag absent" (exit 2) from a transport/auth failure. 5. **[NIT]** Shallow checkout (`fetch-depth: 1`, no `fetch-tags`); dropped the redundant local tag pre-check. 6. **[NIT]** Pure-input validation now runs before the token mint + checkout (fail-fast). Plus a **write-access gate** on the dispatcher, requested separately. Part of #2677. ## Release note ```release-note NONE ``` 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a controlled workflow to create pre-release tags (alpha/beta/rc) with write-once behavior. * Added stronger enforcement so pre-releases are cut from the correct release line. * **Bug Fixes** * Improved guidance when stable tags are rejected or when base branch info is missing. * Added safeguards to prevent cutting tags from an out-of-date branch tip. * **Documentation** * Updated release process docs, including new tag protection rulesets for `v*` tags. * **Chores** * Pinned multiple CI workflow actions to fixed versions for reliability. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
3 participants
What this PR does
Implements the immutable-tag + rc-promotion flow from #2677. A stable release becomes a renamed release-candidate: the bytes shipped as
vX.Y.Zare bit-for-bit the bytes built and e2e-tested asvX.Y.Z-rc.N. No tag is ever force-moved, and stable is never rebuilt — it is promoted by retagging the rc's existing images.Layered onto the build matrix from #2937/#2983 (this PR is stacked on
refactor/build-matrix-2937and must merge after it).The five force-retag sites, removed
tags.yamlapi/apps/v1alpha1 taggit tag -f/push -ftags.yamlrelease-X.Y.Z branchgit branch -f/push -fpull-requests-release.yamlstable taggit tag -f/push -fpull-requests-release.yamlmaintenance branchupdateRef force:trueauto-release.yamlpatch tagsVersion decoupling (the enabler)
The operator baked its version into the image at build time, so an rc image self-reported the rc string — blocking retag-promotion. It now reads
COZYSTACK_VERSIONfrom the environment (threaded viacozystackOperator.platformVersion→ Deployment env, stamped bymake manifests), falling back to the build-time value when unset. The same image bits can report any release name. The only runtime reader is the telemetry metriccozy_cluster_info{cozystack_version=...}.Promotion flow
promote-rc.yaml(workflow_dispatch,rc_tag=vX.Y.Z-rc.N):hack/promote-retag.shreads the rc's digest-pinned image refs frompackages/*/*/values.yamlandskopeo copys each — by digest — to:vX.Y.Zand:latest, verifying withskopeo inspect.-rc.Nsubstring in vendored tags to the stable version (the@sha256wins regardless), restamp the version-stamped assets (operator manifests, cozypkg, openapi; the heavy Talos assets are copied verbatim from the rc draft), open arelease-X.Y.ZPR.pull-requests-release.yamlfinalize to cut the write-once stable tag at the merge commit and publish the release. Squash is disallowed (the tag needs a real merge commit).nightly.yamlcuts write-once*-nightly.<date>tags (gated byNIGHTLY_ENABLED);retention.yamlkeeps the newest 14 per line (dry-run by default).Validation
Local end-to-end — the core claim is proven, not asserted. The whole design rests on "copy-by-digest to a new tag preserves the digest, so stable == the e2e-tested rc, bit-for-bit." This was exercised against a real registry (
ttl.sh) with realskopeo::v1.4.0-rc.2tag, built a temp tree exercising all threevalues.yamldigest shapes (singleimage:string, splitrepository/tag/digestmap, and theplatformSourceRefOCI artifact), then ran the actualhack/promote-retag.sh v1.4.0.:v1.4.0-rc.2,:v1.4.0, and:latestall resolve to the identical digest (sha256:fd8d9aa6…), across all 17 platform manifests. The script's ownskopeo inspectpost-check passed, and an independent re-inspection confirmed it.Local validation caught (and this PR fixes) two bugs that only surface against a real registry — never in static lint:
skopeo copy --multi-arch all --all— mutually exclusive flags, fatal error; every retag would have failed on CI. Now--multi-arch all.repo@digest.Other local checks:
go test ./pkg/version+go build/vet;helm templaterendersCOZYSTACK_VERSIONin all 3 operator variants (omitted when unset);make manifestsstamps the version into the install assets;shellcheckclean onpromote-retag.sh;actionlintclean andact -lresolves the job graph on all workflows; the rc-tag parser and nightly version-math unit-tested (accept/reject + minor-vs-patch bumps).Still CI-only (cannot be exercised offline): the full workflow runtime — github-script API calls, OCIR auth, the rc-release / staging-branch preconditions, the nightly
base_refpush mechanics, and self-hosted runners. The logic inside the steps is unit-tested; the orchestration is not. Treat it as unproven until the workflows run.Out of scope / required follow-up (needs repo admin — not doable from a PR)
v*andapi/apps/v1alpha1/*create-only by the CI app, no delete/update; "limit branches/tags updated in a single push" = 1.NIGHTLY_ENABLED(andRETENTION_APPLYwhen ready) repo variables.*-nightly.*image tags (no OCIR delete parity yet — tracked TODO inretention.yaml).Release note
Summary by CodeRabbit
COZYSTACK_VERSION.