Repository navigation
fix(build): retry cosign signing when on_existing_tag skips the build - #603
Merged
Merged
Conversation
When on_existing_tag: skip skips build/push because the tag already exists, the cosign steps were also skipped since they depended on steps.build-push.outputs.digest and should_build != 'false'. A transient cosign/Rekor failure on an already-pushed tag then had no retry path short of a full rebuild or a brand new release. Add a 'Resolve digest for existing tag' step that resolves the digest from the existing registry tag via docker buildx imagetools inspect when the build was skipped, and gate the cosign steps on enable_cosign_sign alone, feeding steps.build-push.outputs.digest || steps.existing-digest.outputs.digest. Closes #559
lerian-studio
added
size/XS
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
Summary by CodeRabbit
WalkthroughThe build workflow resolves existing registry digests when ChangesCosign retry flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8).github/workflows/build.ymlTraceback (most recent call last): Comment |
Contributor
🔍 PR Validation Summary✅ PR Mergeable — no blocking failures
|
Contributor
🔍 Lint Analysis
|
Contributor
🛡️ CodeQL Analysis ResultsLanguages analyzed: Found 1 issue(s): 1 Medium
🔍 View full scan logs | 🛡️ Security tab |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 1
🤖 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/build.yml:
- Around line 476-492: Update the existing-digest resolution step and its
corresponding later block to handle Docker Hub and GHCR independently when both
registries are enabled. Check each registry’s tag separately, emit a reference
only when that tag exists, and use that registry’s own digest; alternatively,
make preflight fail when only one registry tag exists so signing never uses a
nonexistent ref or the other registry’s digest. Preserve single-registry
behavior and the existing signing flow.
🪄 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.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 424da934-7b8d-46f0-aba3-6a26d44298b4
📒 Files selected for processing (2)
.github/workflows/build.ymldocs/build-workflow.md
Address CodeRabbit finding on PR #603: when both DockerHub and GHCR are enabled, the existing-tag digest resolution always picked DockerHub (or GHCR alone) and reused that single digest for both registries' refs. If a tag existed in only one registry, this could construct a nonexistent ref for the other registry using the wrong digest. Resolve each enabled registry's digest independently and only emit a ref for a registry when its own digest was actually found.
lerian-studio
added
size/S
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 2
🤖 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/build.yml:
- Around line 562-565: Update the failure-report environment setup for the
cosign signing step to preserve both Docker Hub and GHCR digests when signing an
existing tag. Replace the singular DIGEST fallback chain with registry-qualified
digest/ref pairs, ensuring the report can correlate each digest with its
corresponding registry reference without discarding either value.
- Around line 487-510: Update the digest lookup logic in the cosign-sign step
for the Docker Hub and GHCR branches to distinguish inspection or jq failures
from a genuinely missing digest. Track an error state separately for each
enabled registry, propagate failures from docker buildx imagetools inspect or
jq, and abort before signing if any enabled registry cannot be resolved, while
preserving the existing per-registry digest outputs.
🪄 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.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d7f94278-35d8-410c-a718-b49cb7601af0
📒 Files selected for processing (1)
.github/workflows/build.yml
…t digests Address CodeRabbit findings on PR #603: - Resolve digest for existing tag: distinguish a genuine 'tag not found' from an inspect/lookup failure (transient network error, auth issue) by emitting a ::warning:: with the actual error in the latter case, instead of silently treating both as 'no digest'. Kept fail-open (skip that registry, don't abort the job) to match this file's existing convention for the same tradeoff in the pre-flight tag-existence check just above. - Report cosign signing status: stop collapsing to a single DIGEST value that discarded one registry's digest when both were resolved; REFS already lists each registry's fully-qualified ref with its own digest, so the redundant/lossy single-digest line was removed instead of duplicating that same registry-qualified data.
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 2
🤖 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/build.yml:
- Around line 497-502: Update both registry branches in the imagetools
inspection logic to keep each ::warning:: message static and avoid interpolating
raw OUTPUT. Print the captured diagnostics separately using safe escaping or a
non-workflow-command diagnostic path, while preserving the existing no-digest
and inspection-failure handling.
- Around line 568-584: The unsigned-image summary in the failure step must not
claim that every ref is unsigned, since earlier refs may already have been
signed before the loop failed. Update the messages in the failure-summary run
block and related wording to state only that signing did not complete for every
requested ref, without asserting per-ref status; use the existing
steps.cosign-sign failure flow and REFS output.
🪄 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.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d7e48f0b-2165-4422-8f47-04d7b7ba7c41
📒 Files selected for processing (1)
.github/workflows/build.yml
…rding Address CodeRabbit findings on PR #603: - Resolve digest for existing tag: percent-encode the captured 'docker buildx imagetools inspect' output (%, CR, LF) before embedding it in a ::warning:: line, so it can't corrupt or spoof a workflow command annotation. - Report cosign signing status: cosign-sign signs refs one by one, so a failure partway through the list doesn't mean every ref is unsigned. Reworded the summary to reflect that signing didn't complete for every ref instead of asserting all refs are unsigned.
1 of 14 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
When
on_existing_tag: skipskips build/push because the tag already exists in a registry, the cosign signing steps were also skipped — they depended onsteps.build-push.outputs.digest(only set when the build actually ran) and were gated onsteps.preflight.outputs.should_build != 'false'.Net effect: once a tag is successfully pushed but fails to sign (e.g. a transient network flake in
sigstore/cosign-installer), there was no way to retry just the signing step. The only recovery paths were a full rebuild (blocked by the immutable-tag pre-flight guard) or cutting an entirely new release just to get a signed image.Real-world case:
LerianStudio/br-ccsrun https://github.com/LerianStudio/br-ccs/actions/runs/28989167685/job/86024981607 — image pushed successfully, cosign signing failed transiently, and re-running hit the tag-immutability guard before signing could even be attempted again.Fix
Resolve digest for existing tagstep in.github/workflows/build.yml, gated oninputs.enable_cosign_sign && steps.preflight.outputs.should_build == 'false'. Resolves the digest of the already-pushed tag viadocker buildx imagetools inspect <ref> --format '{{json .Manifest}}' | jq -r '.digest'.Build cosign image references,Sign container images with cosign, andReport cosign signing statusare now gated oninputs.enable_cosign_signalone (dropped theshould_build != 'false'condition), and usesteps.build-push.outputs.digest || steps.existing-digest.outputs.digestfor the digest.docs/build-workflow.md: documented thaton_existing_tag: skipnow also retries cosign signing.This turns
skipinto a genuine idempotent re-run path for signing, not just for build/push — a maintainer can flipon_existing_tag: skipfor one re-run to recover from a transient signing failure instead of cutting a new release.Type of Change
fix: Bug fix in a workflow (incorrect behavior, broken step, wrong condition)Breaking Changes
None. When
should_buildistrue(the normal build path), behavior is unchanged — digest still comes fromsteps.build-push.outputs.digest. The change only affects theskippath, which previously skipped signing entirely.Testing
@this-branchor the beta tagRelated Issues
Closes #559