fix(build-push-docker-manifest): idempotent manifest + cosign reruns (RANE-4683)#1577
fix(build-push-docker-manifest): idempotent manifest + cosign reruns (RANE-4683)#1577HashWrangler wants to merge 11 commits into
Conversation
|
👋 HashWrangler, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
…(RANE-4683) - Skip imagetools create when the tag already references the expected platform digests; fail if the tag exists with different digests - Skip cosign sign when verify already succeeds (idempotent rerun) - Retry cosign verify after sign for Sigstore propagation flakes Together these changes make build-publish manifest jobs safe to rerun without digest drift or redundant signing, while still failing loudly on real conflicts.
856ffb7 to
b6b6a44
Compare
There was a problem hiding this comment.
Pull request overview
This PR hardens the build-push-docker-manifest composite action to make build-publish reruns idempotent and reduce flakiness around Cosign verification/signature propagation.
Changes:
- Adds an idempotency guard for manifest creation by comparing expected vs existing platform digests and skipping
imagetools createwhen they match. - Skips
cosign signwhen an existing valid signature is already present, and adds retry logic tocosign verifyto absorb Sigstore propagation lag. - Adds a changeset to release these hardening updates as a minor version bump.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| actions/build-push-docker-manifest/action.yml | Adds digest comparison to avoid manifest digest drift on reruns; makes cosign signing/verification rerun-safe with skip + retry behavior. |
| .changeset/rane-4683-manifest-cosign-hardening.md | Declares a minor release for the manifest + cosign hardening changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Ensure jq is installed before digest comparison - Validate docker-image-name-digests is non-empty - Only skip cosign sign when OIDC identity constraints verify - Fail fast on unexpected imagetools inspect errors (not just missing tag)
Keep MANIFEST_CREATE_SKIPPED expression on one line and only sleep between cosign verify retries, not after the final failed attempt.
- Fail fast when jq is missing and apt-get/sudo are unavailable
- Shorten step/output ids so summary env expressions stay on one line
under Prettier (avoids split ${{ }} expressions)
…empotency Treat digest lists as sets: sort -u in normalization and existing-manifest inspection, and reuse normalized digests when building imagetools create args.
- Only sleep between manifest digest inspect retries, not after the final fail - Fail fast when an existing tag has no extractable platform digests instead of falling through to imagetools create (digest drift on rerun)
Reject non-sha256 tokens in normalize_digest_csv so bad docker-image-name-digests input fails fast with a clear error instead of deferring to imagetools create.
There was a problem hiding this comment.
I wonder if its better if we don't verify the signature in this action, but rather verify it in the resuable workflow? Thoughts? @chainchad
…ty guard jq is preinstalled on GitHub-hosted ubuntu-24.04 runners; fail fast with a clear error when missing instead of apt-get/sudo install logic.
Any thoughts here? I could refactor if thats the consensus view. |
|
@erikburt Agreed — refactored per your suggestion in 36c3d8c: Composite action (
Reusable workflow (
Changeset updated for both packages ( |
…to workflow Retry manifest inspect until platform digests match expected inputs (ECR tag propagation lag). Move the cosign verify gate with 5×10s retry from the composite action into reusable-docker-build-publish; keep idempotent create, sign-skip internal verify, and sign in the action (RANE-4683).
36c3d8c to
18263d8
Compare
…ut in summary The step summary referenced a manifest-additional-tags output that inspect-docker-manifest never sets; use the manifest-additional-tags input instead so additional tags display correctly.
| - name: Ensure jq is available | ||
| shell: bash | ||
| run: | | ||
| if ! command -v jq >/dev/null 2>&1; then | ||
| echo "::error::jq is required but not installed on this runner (preinstalled on GitHub-hosted ubuntu-24.04). Install jq before running this action." | ||
| exit 1 | ||
| fi | ||
| echo "jq available: $(jq --version)" | ||
|
|
There was a problem hiding this comment.
Delete - we should assume it's available.
| verify_manifest_signature() { | ||
| cosign verify "${MANIFEST_NAME_WITH_DIGEST}" \ | ||
| --certificate-oidc-issuer "${OIDC_ISSUER}" \ | ||
| --certificate-identity-regexp "${OIDC_IDENTITY_REGEXP}" \ | ||
| --certificate-github-workflow-repository "${GITHUB_WORKFLOW_REPOSITORY}" | ||
| } | ||
|
|
||
| if [[ -n "${OIDC_IDENTITY_REGEXP}" ]] && verify_manifest_signature >/dev/null 2>&1; then | ||
| echo "✅ Manifest already signed with expected identity; skipping cosign sign (idempotent rerun)" | ||
| echo "skipped=true" | tee -a "${GITHUB_OUTPUT}" | ||
| exit 0 | ||
| fi | ||
|
|
||
| echo "skipped=false" | tee -a "${GITHUB_OUTPUT}" | ||
| cosign sign "${MANIFEST_NAME_WITH_DIGEST}" --yes |
There was a problem hiding this comment.
My suggestion was to remove signature verification from this action, and move the signature verification to it's own job in the reusable workflow. So if the signature verification fails, then it can be retried alone.
I think I would still like @chainchad's opinion here.
There was a problem hiding this comment.
Sorry, I missed the previous nudge. Replied on a separate thread to get more context.
| - name: Verify Docker manifest signature | ||
| if: ${{ inputs.docker-manifest-sign == 'true' }} | ||
| shell: bash | ||
| env: | ||
| MANIFEST_NAME_WITH_DIGEST: ${{ steps.docker-manifest.outputs.manifest-name-with-digest }} | ||
| GITHUB_WORKFLOW_REPOSITORY: ${{ inputs.github-workflow-repository }} | ||
| OIDC_ISSUER: https://token.actions.githubusercontent.com | ||
| OIDC_IDENTITY_REGEXP: "^https://github.com/smartcontractkit/.*$" | ||
| run: | | ||
| set -euo pipefail | ||
|
|
||
| MAX_RETRIES=5 | ||
| RETRY_DELAY=10 | ||
| VERIFY_OK=false | ||
|
|
||
| for i in $(seq 1 $MAX_RETRIES); do | ||
| echo "Attempt ${i}/${MAX_RETRIES}: Verifying cosign signature for ${MANIFEST_NAME_WITH_DIGEST}..." | ||
|
|
||
| if cosign verify "${MANIFEST_NAME_WITH_DIGEST}" \ | ||
| --certificate-oidc-issuer "${OIDC_ISSUER}" \ | ||
| --certificate-identity-regexp "${OIDC_IDENTITY_REGEXP}" \ | ||
| --certificate-github-workflow-repository "${GITHUB_WORKFLOW_REPOSITORY}"; then | ||
| echo "Successfully verified signature on attempt ${i}" | ||
| VERIFY_OK=true | ||
| break | ||
| fi | ||
|
|
||
| echo "Attempt ${i}/${MAX_RETRIES}: Signature not yet available..." | ||
| if [[ "${i}" -lt "${MAX_RETRIES}" ]]; then | ||
| echo "Retrying in ${RETRY_DELAY}s..." | ||
| sleep "${RETRY_DELAY}" | ||
| fi | ||
| done | ||
|
|
||
| if [[ "$VERIFY_OK" != "true" ]]; then | ||
| echo "::error::Failed to verify cosign signature for ${MANIFEST_NAME_WITH_DIGEST} after ${MAX_RETRIES} attempts" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
As stated above, my suggestion was to put this in it's own job so it can be retried separately. In this scenario, if this verification still fails for some reason and the job is retried, then the entire manifest signing is retried. I assume this is also why there's a ton of added logic to check if it has been previously signed, and skipped if so.
Summary
Single holistic hardening pass for
build-publishmanifest job flakes and non-idempotent reruns (RANE-4683).imagetools createagain → new index digest (digest drift)cosign verifyalready succeeds (internal check in action)no signatures foundright after signreusable-docker-build-publish— 5×10s retry loop (Sigstore propagation lag)Parent epic: RANE-4695 private release improvements.
Why one PR
All changes address the same operational failure mode: manifest jobs that fail intermittently and only pass on rerun. The composite action owns create/sign/idempotency; the reusable workflow owns the post-sign verify gate (per @erikburt review).
Evidence
v2.990.5-beta.0— cosign verify flake; rerun passedOut of scope (separate PR)
Test plan
build-push-docker-manifest+ patchreusable-docker-build-publishreusable-docker-build-publish@build-push-docker-manifest/v1picks up release