feat(promote-release): promote the caller's own release and baseline on the latest pointer - #212
feat(promote-release): promote the caller's own release and baseline on the latest pointer#212sydorovdmytro wants to merge 1 commit into
Conversation
…on the Latest pointer
Two changes needed for a caller that publishes stable cuts with the GitHub
"None" label (release.prerelease: auto + make_latest: false), where nothing
ever flips the release afterwards:
- New promote-self input: also promotes the CALLER repo's release (unset
pre-release, set Latest), gated by the same backport check as :latest. Off
by default, and only an exact "true" enables it, so a caller whose releases
already go out promoted is unaffected.
- The unscoped backport gate now compares against the release flagged
isLatest, i.e. the last one actually promoted, instead of the newest
isPrerelease == false. Under prerelease: auto an un-promoted stable cut is
already non-prerelease, so the old baseline would refuse to advance :latest
behind any newer un-promoted cut and strand it. Re-promoting the release
that is already Latest now resolves as promotable too, so a partially
failed promotion can simply be re-run. The line-scoped :{major}.{minor}
gate keeps its shape comparison: GitHub has no per-line Latest pointer.
Also document that this belongs on workflow_dispatch rather than
release: types: [released] - with prerelease: auto that event fires at build
time, and it resolves its workflow file from the release's own tag ref, so it
never fires for tags on maintenance branches that predate the file.
feat(release-notification): add a promote reminder and link both releases
- needs_promotion + promote_workflow render a GitHub label field naming the
promote workflow, since the release build is the only thing that tells the
Release Captain a step is still outstanding. is_prerelease was declared but
unused; it now feeds the same field. Fallback is the neutral "Release", so
callers passing neither flag get no guessed label.
- paired_repo links the release and changelog for BOTH repos. One vCluster cut
publishes two releases, and the single link pointed at the private pro repo,
the less useful of the two.
All three are opt-in with defaults that reproduce the previous output exactly,
so vCluster Platform banners are unchanged.
Refs DEVOPS-1270
| published with the "None" label, since nothing else ever promotes them. The | ||
| release is edited to `--prerelease=false --latest`, with `--latest` withheld on a | ||
| backport promotion (same gate as `:latest`). A missing release warns and skips. | ||
| Only an exact `"true"` enables it. |
There was a problem hiding this comment.
blocking — promote-self makes gh release edit --repo ${GITHUB_REPOSITORY} a required mutation, but the documented token scope never mentions the caller's own repo. All three places that describe it still say only oss-repo/homebrew-tap-repo:
- this README's input table (
github-token: "Token with GHCRwrite:packages, andcontents:writeon oss-repo and homebrew-tap-repo if set.") action.yml'sgithub-tokendescription (the auto-doc source for that row)- the
Required env: GH_TOKENblock at the top ofsrc/action.sh
Before this PR, GITHUB_REPOSITORY was only ever read (gh release list); this is the first write against it, so the pre-PR token contract genuinely did not need that scope.
The consequence is the failure mode this PR exists to remove. A caller who scoped their PAT to exactly what's documented gets a 403 on every run, it is swallowed by the warn-and-continue wrapper (::warning::gh release edit failed for …), the job stays green, and the release is never set Latest — moving tags and the formula then drift exactly as described in DEVOPS-1270, just from a different cause. Because nothing else ever flips a "None"-label release, there is no second chance to catch it.
Suggest extending all three descriptions to require contents:write on GITHUB_REPOSITORY when promote-self is enabled, and adding a line here — e.g. appending to this section:
| Only an exact `"true"` enables it. | |
| Only an exact `"true"` enables it. | |
| Requires `github-token` to also carry `contents:write` on the **caller's own** | |
| repo (`GITHUB_REPOSITORY`), not just on `oss-repo`/`homebrew-tap-repo`. Without | |
| it `gh release edit` returns 403, which is caught as a warning — the run stays | |
| green and the release is silently left un-promoted. |
| # --prerelease=false a no-op for an auto-classified stable cut; it is what | ||
| # promotes a legacy tag still built under prerelease: true. | ||
| if [[ "${PROMOTE_SELF}" == "true" ]]; then | ||
| if gh release view "${VERSION}" --repo "${GITHUB_REPOSITORY}" >/dev/null 2>&1; then |
There was a problem hiding this comment.
consider — This probe discards stderr, so a transient API/network/rate-limit failure is indistinguishable from "the release genuinely doesn't exist": both fall into the same ::warning::no ${VERSION} release found on …; skipping its promotion branch, and the script still exits 0.
Since promote-self exists precisely because nothing else ever flips the caller's own release, a blip here leaves the flagship release un-promoted with a green job and a warning that actively misreports the cause. is_latest_stable a few lines up already handles this correctly — it captures the output and surfaces the real error text.
Suggest the same shape here: capture raw=$(gh release view … 2>&1), treat only a recognizable not-found message as skip-worthy, and let anything else surface louder (or at least include the actual gh output) so the operator can tell a missing release from a failed lookup.
This becomes blocking if a transient lookup failure during a real promotion window leaves the release un-promoted while CI reports success, since the misleading "no release found" warning would send whoever investigates down the wrong path.
| if [[ -z "${line}" ]]; then | ||
| # Unscoped: the promoted pointer. At most one release carries isLatest, so | ||
| # this yields 0 or 1 tags; no shape filter is applied, because whatever a | ||
| # human promoted IS the baseline even if its shape is unusual. | ||
| max=$(jq -r '[.[] | select(.isLatest) | .tagName][]' <<<"${raw}") | ||
| else | ||
| # Anchor the line filter on the literal, dot-escaped {major}.{minor} so a | ||
| # "9.9" line never also matches "9x9" or a "99" prefix. | ||
| filter="^v${line//./\\.}\.[0-9]+$" | ||
| max=$(jq -r '[.[] | select(.isPrerelease == false) | .tagName][]' <<<"${raw}" \ | ||
| | grep -E "${filter}" \ | ||
| | sort -V | tail -1) | ||
| fi |
There was a problem hiding this comment.
consider — is_latest_stable now answers two genuinely different questions depending on an unnamed positional argument. Unscoped it compares against the isLatest pointer (any tag shape, prerelease flag ignored, equality passes); line-scoped it compares against the newest stable-shaped non-prerelease tag in the line. The two results aren't comparable, and the name no longer describes either — nothing here is "latest stable" anymore.
The call sites show the cost: is_latest_stable "${OSS_REPO}" "" "soft" selects the pointer baseline via an empty placeholder, and the failure-policy argument is only ever meaningful for the unscoped form. A fourth call site added later will pick a baseline by accident.
Consider splitting into two named predicates over a shared list fetch — e.g. is_at_or_after_latest_pointer <repo> <on_fail> and is_newest_in_line <repo> <line>. Each then has one job and no mode flag, and the on_fail policy lives only where it applies. The block comment you've written above already explains the two baselines clearly; this would let the code say it structurally rather than in prose.
This becomes blocking if a future call site picks the wrong baseline through the positional interface and moves :latest or the repo's Latest pointer backwards.
| done | ||
| done | ||
|
|
||
| # --- Caller repo's own release ------------------------------------------- |
There was a problem hiding this comment.
consider — This "Caller repo's own release" block duplicates the "Paired public release" block immediately below it (starts around src/action.sh:321). Both do: gh release view probe → build a --prerelease=false[/--latest] args array → echo a promoting notice → gh release edit → on failure emit a ::warning:: carrying a manual gh release edit repair command → on a missing release emit a ::warning:: and skip.
The only real difference is how the --latest gate is obtained: ADVANCE_LATEST_MAJOR here, versus a fresh is_latest_stable … "soft" call there. A small helper taking the already-decided flag — promote_gh_release <repo> <advance_bool> — would collapse both ~20-line sequences into one, so the warning text and the repair-command wording can't drift between the two call sites as either is edited later.
This becomes blocking if the two copies diverge in a way that makes one path's failure message point at the wrong repo or omit the repair command.
| text: | ||
| type: mrkdwn | ||
| text: "*Changes:*\n<https://github.com/${{ inputs.target_repo }}/compare/${{ inputs.previous_tag }}...${{ inputs.version }}|View Full Changelog>" | ||
| text: "*Changes:*\n${{ inputs.paired_repo != '' && format('<https://github.com/{0}/compare/{1}...{2}|{0}> | <https://github.com/{3}/compare/{1}...{2}|{3}>', inputs.target_repo, inputs.previous_tag, inputs.version, inputs.paired_repo) || format('<https://github.com/{0}/compare/{1}...{2}|View Full Changelog>', inputs.target_repo, inputs.previous_tag, inputs.version) }}" |
There was a problem hiding this comment.
consider — This line, together with the GitHub label: field (130) and the release link (145), puts the whole banner-state decision inline in the payload string: a three-way conditional plus two ~300-character expressions whose format() placeholders are reused out of order across two URLs and their link texts. Verifying that {0} is simultaneously the compare host, the changelog link target and the link label is hard on a first read, and the two link expressions repeat the same paired_repo != '' ? two-link : one-link shape twice. Nothing covers any of it — this action's bats suite only exercises detect-branch.sh.
This file already establishes the pattern for exactly this: the Resolve status text step (around 146-163) computes the failure banner's emoji/label into $GITHUB_OUTPUT and leaves the payload a flat interpolation, referenced twice below.
Suggest a small Resolve links / Resolve label bash step before the post step, emitting label_text, changelog_text and release_text outputs. The payload stays readable, the two link formats can't silently diverge, and the branching becomes bats-testable like the rest of the shell in this repo.
This becomes blocking if a placeholder-ordering mistake in one of these expressions ships a banner whose link text names one repo while the href points at the other — which would be invisible in review and only noticed by a reader who clicked.
| # that pass neither flag (e.g. vCluster Platform) have their | ||
| # own labelling rules, and claiming Latest for them would be a | ||
| # guess. | ||
| text: "*GitHub label:*\n${{ (inputs.needs_promotion == 'true' && format('`None` - not Latest yet. <https://github.com/{0}/actions/workflows/{1}|Promote it> to move Latest, the moving image tags and the Homebrew formula.', inputs.target_repo, inputs.promote_workflow)) || (inputs.is_prerelease == 'true' && 'Pre-release') || 'Release' }}" |
There was a problem hiding this comment.
consider — Whenever needs_promotion == 'true', this text unconditionally promises that running promote_workflow will "move Latest, the moving image tags and the Homebrew formula". But promote-release's own backport gate deliberately does less than that in two reachable cases: for an older-line patch it skips :latest/:{major} and skips the Homebrew tap entirely, and for a release that is already Latest the flip is a no-op.
The banner can't know which case applies — needs_promotion is a static, caller-supplied flag with no live check against the release's actual isLatest/isPrerelease state. So it overpromises for exactly the cases where the promote step will do less than advertised, and the real outcome only appears as ::notice:: lines buried in the promote-release job log rather than in the notification that is meant to be the operational signal for the Release Captain.
Consider softening the claim, e.g. "Promote it to unset pre-release (and move Latest, the moving image tags and the Homebrew formula if this is the newest release)".
This becomes blocking if a Release Captain reads the banner on a backport cut, assumes the moving tags and formula were advanced, and stops checking — which is the same class of undetected staleness DEVOPS-1270 documents.
| flips a release from pre-release to a full release (verified live for | ||
| DEVOPS-1083); a bot-authored release publish never triggers it, so there is | ||
| no risk of the build itself re-entering this action. | ||
| Wire this from `on: workflow_dispatch` on the repo that owns the moving tags, |
There was a problem hiding this comment.
consider — Rewriting this paragraph leaves the top-level README.md "### Promote Release" section (around lines 682-734) as the stale copy of the old contract. Three lanes landed on this independently. That section still:
- describes
Wire from on: release: types: [released], and shows it in its usage snippet - passes
version: ${{ github.event.release.tag_name }}rather than aninputs.version - describes the retag mechanism as
docker buildx imagetools create, superseded bycrane tagback in feat(promote-release): use crane tag so per-arch moving tags stay signed #197 - omits
promote-selffrom its inputs list entirely
That section is the copy-paste source for callers, so someone wiring from it gets precisely the trigger this file now warns against — a released event that self-promotes an unvetted "None"-label cut. The root README restating each action's summary is a known drift point here (the crane item above is the same section having already gone stale once).
Suggest updating it in this change, or replacing the restated summary with a link to this README so it can't drift again.
| run ! grep -qF -- 'EDIT example-org/example-caller-repo v9.9.9 --prerelease=false --latest' "$GH_MOCK_CALLS" | ||
| } | ||
|
|
||
| @test "promote-self with no matching release on the caller repo -> warns, run still succeeds" { |
There was a problem hiding this comment.
consider — The six new promote-self cases cover off-by-default, happy path, backport withholding --latest, missing release, exact-"true" gating and dry-run — but not the edit-failure branch. if ! run gh release edit … "${self_args[@]}" and its ::warning::gh release edit failed for … repair message are untested, while the equivalent oss-repo path does have a GH_MOCK_FAIL=1 case (around line 589).
That branch is also the one the blocking token-scope comment turns on: a 403 from an under-scoped PAT lands exactly here, and it's the path that decides whether the operator gets an actionable message or a silent green run. A case setting INPUT_PROMOTE_SELF=true plus GH_MOCK_FAIL=1 and asserting both status -eq 0 and the warning text (including the manual repair command) would pin it down, mirroring the existing oss-repo test.
This becomes blocking if that warning's wording or the ${self_args[*]} repair command regresses unnoticed, since the warning is the only signal a failed self-promotion produces.
| required: false | ||
| default: 'false' | ||
| needs_promotion: | ||
| description: | |
There was a problem hiding this comment.
nit — needs_promotion here (and paired_repo below) carry multi-paragraph rationale in description:. Docs in this repo are generated by tj-actions/auto-doc, which pads every cell in a column to the width of the widest cell — so these two blow the whole DESCRIPTION column out, and the regenerated release-notification/README.md table in this PR is ~2.5x wider than before with every other row padded to match. (The generated table is legitimately in sync; the source descriptions are what to shorten, not the table.)
The convention here is a one-to-two-line description: with the rationale in README prose — which is exactly how promote-release handles the sibling input in this same PR, via a ### promote-self section. Worth noting that release-notification/README.md currently gains no prose for either new input, so trimming the descriptions and adding a short section for needs_promotion/paired_repo would fix the table and document them properly in one go.
2 participants
Summary
Prerequisite for switching vCluster to the GitHub None label for stable cuts (
release.prerelease: auto+make_latest: false). Three opt-in additions, all defaulting to the current behaviour.promote-releasepromote-self: also promotes the caller's own release (unset pre-release, set Latest), gated by the same backport check as:latest. Under the None label nothing else ever flips it. Off by default; only an exact"true"enables it.isLatest— the last one actually promoted — instead of the newestisPrerelease == false. Withprerelease: autoan un-promoted stable cut is already non-prerelease, so the old baseline would refuse to advance:latestbehind any newer un-promoted cut and strand it. Re-promoting the current Latest now resolves as promotable, so a partially failed promotion can be re-run. The line-scoped:{major}.{minor}gate keeps its shape comparison, since GitHub has no per-line Latest pointer.workflow_dispatch, notrelease: types: [released]: withautothat event fires at build time, and it resolves its workflow file from the release's own tag ref, so it never fires for tags on maintenance branches that predate the file.release-notification/notify-release.yamlneeds_promotion+promote_workflowrender aGitHub labelfield naming the promote workflow. The release build is the only thing that tells the Release Captain a step is outstanding.is_prereleasewas declared but unused and now feeds the same field; the fallback is the neutralRelease, never a guessedLatest.paired_repolinks the release and changelog for both repos. One vCluster cut publishes two releases, and the single link pointed at the private pro repo — the less useful of the two.Test plan
bats .github/actions/promote-release/test/action.bats— 54/54 pass, including 9 new cases: newer-un-promoted-cut does not block:latest, idempotent re-run of the current Latest, and 6promote-selfcases (off by default, happy path, backport withholds--latest, missing release warns, exact-"true"only, dry-run).isLatest, since the baseline changed.make lint(actionlint + zizmor): no findings.make check-docs: clean.shellcheck src/action.sh: clean.prerelease: autosets pre-release fromctx.Semver.Prerelease, and thatmake_latestis applied on publish — the behaviour this action's new baseline assumes.Rollout
Merge this and repoint both
promote-release/v1andrelease-notification/v2before thevcluster-proPRs merge. A stalerelease-notification/v2fails loudly on the unknown reusable-workflow input; a stalepromote-release/v1fails silently, because a composite action drops undeclared inputs, sopromote-selfwould be ignored and the release would never be set Latest.References DEVOPS-1270