ci: build release images once on main, retag at release (plan PR 1)#190
Conversation
|
Warning Review limit reached
Next review available in: 53 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR restructures CI/release image handling so the coverage-instrumented project image is built and delivered as an artifact only (no registry push), while release-grade multi-arch digests are built and pushed on main; release time retags/signs these existing digests instead of rebuilding. Scanning is split between PR and release-grade flows. Documentation and a new design document describe the change. ChangesCI/Release Workflow Changes
Estimated code review effort: 3 (Moderate) | ~35 minutes Documentation Updates
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant PullRequest
participant CIBuild
participant image-scan
participant MainPush
participant build-release
participant image-scan-release
participant release-please
participant publish-manifest
participant Registry
PullRequest->>CIBuild: build instrumented image (GOCOVER=1)
CIBuild->>image-scan: load artifact, scan (CRITICAL fixable gate)
MainPush->>build-release: build release digests (amd64/arm64)
build-release->>image-scan-release: scan digest per arch
image-scan-release->>image-scan-release: SARIF upload + CRITICAL gate
release-please->>publish-manifest: release_created
build-release->>publish-manifest: digests-* artifacts
publish-manifest->>publish-manifest: verify digests exist
publish-manifest->>Registry: retag to semver + latest, sign, attest
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
a806afb to
c96c4b5
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: 1
🧹 Nitpick comments (2)
.github/workflows/release.yml (1)
93-103: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winHarden the digest count check to expect exactly the known arch count, not just >0.
The check only guards against zero digests. If a future change causes only one of
build-release-amd64/build-release-arm64to be skipped (not failed — e.g. a divergentifgate),cicould still succeed with 1 digest, this check would pass, andpublish-manifestwould silently produce/tag a single-arch "multi-arch" manifest. Today both jobs share the sameifcondition so the risk is low, but validating against the expected architecture count is cheap insurance for a release-integrity path.🛡️ Proposed tightening
- name: Verify release digests are present # Fail loudly if build-release-* produced nothing (e.g. a bad gate # skipped them): a retag with no digests must never silently no-op. working-directory: /tmp/digests run: | count="$(find . -type f | wc -l)" - if [ "${count}" -eq 0 ]; then - echo "No release-grade digests found — build-release-* did not publish" >&2 + expected=2 # amd64 + arm64 + if [ "${count}" -ne "${expected}" ]; then + echo "Expected ${expected} release-grade digest(s), found ${count} — build-release-* did not publish all architectures" >&2 exit 1 fi echo "Found ${count} release digest(s) to retag"🤖 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/release.yml around lines 93 - 103, The digest presence guard in the release workflow only checks for non-zero files, which can let a partial release slip through. Tighten the verification in the “Verify release digests are present” step so it compares the count from /tmp/digests against the expected architecture total, and fail unless it matches exactly; keep the existing find/count logic but update the conditional and success message in the release job before publish-manifest runs..github/workflows/ci.yml (1)
652-658: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winUse the pinned login action instead of shelling out with templated values.
This removes the zizmor template-expansion finding on Line 658 and keeps registry auth consistent with the existing composite action.
Proposed workflow adjustment
- - name: Log in to registry - # Trivy resolves the image by digest and pulls it itself, analyzing the - # filesystem — this works cross-arch (amd64 runner, arm64 digest) since - # a single-arch manifest digest carries no platform-selection step. The - # login just covers authenticated pulls; the package is public anyway. - run: | - echo "${{ secrets.GITHUB_TOKEN }}" | docker login ${{ env.REGISTRY }} -u ${{ github.actor }} --password-stdin + - name: Log in to registry + uses: docker/login-action@c99871dec2022cc055c062a10cc1a1310835ceb4 # v4.3.0 + with: + registry: ${{ env.REGISTRY }} + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }}🤖 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/ci.yml around lines 652 - 658, The registry authentication step in the workflow is shelling out with templated values, which triggers the template-expansion finding. Replace the direct docker login command in the “Log in to registry” step with the existing pinned login action used elsewhere in the workflow so auth stays consistent and no shell interpolation is needed. Keep the surrounding Trivy-related step context intact, but switch the implementation to the action-based login path rather than echoing secrets into docker login.Source: Linters/SAST tools
🤖 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/design/release-image-reuse-plan.md`:
- Around line 3-10: The PR 1 status text in the release-image-reuse-plan
document is overstating the signing behavior and conflicts with the current
release.yml flow. Update the status block to describe only what this PR actually
ships, and move any mention of per-arch release-digest signing into the
follow-up section so it matches the publish-manifest behavior and keeps signing
unchanged in this PR.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 652-658: The registry authentication step in the workflow is
shelling out with templated values, which triggers the template-expansion
finding. Replace the direct docker login command in the “Log in to registry”
step with the existing pinned login action used elsewhere in the workflow so
auth stays consistent and no shell interpolation is needed. Keep the surrounding
Trivy-related step context intact, but switch the implementation to the
action-based login path rather than echoing secrets into docker login.
In @.github/workflows/release.yml:
- Around line 93-103: The digest presence guard in the release workflow only
checks for non-zero files, which can let a partial release slip through. Tighten
the verification in the “Verify release digests are present” step so it compares
the count from /tmp/digests against the expected architecture total, and fail
unless it matches exactly; keep the existing find/count logic but update the
conditional and success message in the release job before publish-manifest runs.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 20cd45e0-3ddb-4040-ac32-19ab53f28b26
📒 Files selected for processing (5)
.github/RELEASES.md.github/workflows/ci.yml.github/workflows/release.ymldocs/ci-overview.mddocs/design/release-image-reuse-plan.md
| Status: **PR 1 (§8 core) implemented** — `build-release-amd64`/`-arm64` + | ||
| `image-scan-release` in `ci.yml`, `publish` rebuild job deleted and | ||
| `publish-manifest` retags the CI digests (gated on `release_created`), | ||
| instrumented image is artifact-only everywhere. PRs 2–5 (quickstart→release | ||
| digest, per-commit signing, guarded e2e skip, release-PR dispatch) remain | ||
| follow-ups. Revised after review: scan both | ||
| arches, tighter `build-release` gate, signing targets the per-arch release | ||
| digests (never `ci-<sha>`), phased rollout (§8). |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep the signing status aligned with the actual workflow.
This says PR 1 already "targets the per-arch release digests" for signing, but the current release.yml flow still signs the multi-arch manifest digest in publish-manifest; per-arch digest signing is described later as a follow-up. Please move that language into the follow-up section and keep the PR 1 status block scoped to the behavior that actually ships here. Per the PR objectives and release.yml, signing stays unchanged in this PR.
🤖 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 `@docs/design/release-image-reuse-plan.md` around lines 3 - 10, The PR 1 status
text in the release-image-reuse-plan document is overstating the signing
behavior and conflicts with the current release.yml flow. Update the status
block to describe only what this PR actually ships, and move any mention of
per-arch release-digest signing into the follow-up section so it matches the
publish-manifest behavior and keeps signing unchanged in this PR.
Proposal to build release-grade images once on main (amd64+arm64) and retag them at release instead of rebuilding, closing the scan/ship gap and unlocking per-commit signing. Nothing implemented yet; sequencing and risks in the doc. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Implements PR 1 of docs/design/release-image-reuse-plan.md: the release
tail no longer rebuilds images from scratch. ci.yml builds the
release-grade multi-arch digests on every push to main, and release.yml
retags those exact bytes at release time.
ci.yml:
- build (instrumented, GOCOVER=1) is now artifact-only on every run: never
pushed, delivered to e2e via docker save/load + k3d import, so the
fork-safe path becomes the only path. Own cache scope so it stops
invalidating the release build. Project-image artifact retention 1 -> 7d.
- new build-release-amd64 / build-release-arm64 jobs (gated to push+main)
build clean, semver-stamped images (VERSION from the release-please
manifest) and push them by digest, reusing the build-linux/{amd64,arm64}
cache scopes so they stay warm from every main push.
- new image-scan-release matrix job Trivy-scans BOTH shipped digests before
release; the existing image-scan is now PR-only. Skip semantics stay
transitive (no always()).
- e2e uses IMAGE_DELIVERY_MODE=load everywhere; the project image is loaded
from the artifact on main too.
release.yml:
- delete the publish rebuild jobs (both arches).
- publish-manifest gains its own release_created gate and sources the
per-arch digests from the ci run's artifacts, retagging them to
semver + latest, then signs + attests as before; fails loudly if the
digest artifacts are missing.
Signing identity is unchanged (release.yml@refs/heads/main). Docs updated:
ci-overview.md, RELEASES.md, and the plan status.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
c96c4b5 to
cda93a4
Compare
Bring the plan doc to the current handover state: PR 1 merged via #190 (2b3a949), PRs 2-5 remaining with current-tree anchors, and the §4 cross-arch claim corrected to match the TRIVY_PLATFORM fix in this PR — push-by-digest builds are OCI indexes, so Trivy must be told which child to scan. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(ci): pin Trivy platform when scanning release digests
build-release-* pushes by digest and buildx wraps the single-platform image
in an OCI index (it attaches a provenance attestation manifest). Trivy
resolves the digest remotely but, for an index ref, selects the child
matching the runner's platform — so on the amd64 runner the arm64 leg fails
100% with "no child with platform linux/amd64 in index".
That reds image-scan-release, which fails the reusable ci workflow, which
release-please needs — so it blocks every release (fails safe: no tag, no
half-published release, but nothing ships).
Pin TRIVY_PLATFORM=linux/${matrix.arch} on both Trivy steps so each leg
selects the correct child from the index, and correct the stale comment that
claimed a push-by-digest ref carries no platform-selection step.
Can only be fully validated by a main run (GitHub Actions behavior); actionlint clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs: sync release-image-reuse plan to merged state + #191 blocker
Bring the plan doc to the current handover state: PR 1 merged via #190
(2b3a949), PRs 2-5 remaining with current-tree anchors, and the §4 cross-arch
claim corrected to match the TRIVY_PLATFORM fix in this PR — push-by-digest
builds are OCI indexes, so Trivy must be told which child to scan.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Implements PR 1 (the §8 "core") of
docs/design/release-image-reuse-plan.md: build the release-grade images once onmainand retag them at release, instead of rebuilding from scratch in the release tail.What changes
ci.ymlbuildis now artifact-only on every run. It keepsGOCOVER=1and itsci-<sha>local tag name, but is never pushed — delivered to e2e viadocker save/load+ k3d import (the fork-safe path becomes the only path). Given its own cache scope (build-instrumented-linux/amd64) so it stops invalidating the release build; project-image artifact retention bumped1 → 7days.build-release-amd64/build-release-arm64jobs (gatedpush+refs/heads/main): clean, no-coverage, semver-stamped (VERSIONfrom.release-please-manifest.json), pushed by digest, reusing thebuild-linux/{amd64,arm64}cache scopes so they stay warm from every main push. Two named jobs (not a matrix) so each per-arch digest is an individual output.image-scan-releasematrix job Trivy-scans both shipped digests (amd64 + arm64) before release and uploads SARIF; the existingimage-scanis now PR-only. Skip semantics stay transitive vianeeds:— noalways()anywhere.IMAGE_DELIVERY_MODE=loadeverywhere — the project image is loaded from the artifact onmaintoo (same path PRs already exercise; chart ispullPolicy: IfNotPresent).release.ymlpublishrebuild jobs (both arches).publish-manifestgains its ownrelease_createdgate (it no longer inherits it transitively) and now sources the per-arch digests from thecirun's artifacts, retagging them to semver +latest, then signs + attests as before. Added a guard that fails loudly if the digest artifacts are missing.Docs
docs/ci-overview.md(trust zones, fork-PR diagram, release-artifacts, Trivy hygiene),.github/RELEASES.md, and the plan status line.Notably unchanged
release.yml@refs/heads/main— sign/attest still run inrelease.yml, socosign verifyworks verbatim.GOCOVER=1) on main is preserved.build-release-*andimage-scan-releaseskip on PRs.Deferred (sequenced as later PRs by §8)
PR 2 (quickstart lane → release digest), PR 3 (per-commit signing), PR 4 (guarded
full-lane e2e skip), PR 5 (release-PR auto-dispatch). PR 3+ depend on this landing and cutting one real release first.Validation
task lint✓ (golangci-lint, actionlint, hadolint, helm)task test✓ (unit + coverage ratchet, no regression)task test-e2e✓ (53 passed / 0 failed)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
latesttags applied at release time.Bug Fixes
Documentation