ART-21572: Add doozer verify-signatures command for image signed check - #3162
ART-21572: Add doozer verify-signatures command for image signed check#3162tomasdavidorg wants to merge 4 commits into
Conversation
|
@tomasdavidorg: This pull request references ART-21572 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the sub-task to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
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:
WalkthroughAdds a ChangesRelease signature verification
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant VerifyReleaseSignaturesPipeline
participant get_image_info
participant SigstoreSignatory
participant SignatureMirror
CLI->>VerifyReleaseSignaturesPipeline: run release verification
VerifyReleaseSignaturesPipeline->>get_image_info: retrieve image digest
get_image_info-->>VerifyReleaseSignaturesPipeline: return digest
VerifyReleaseSignaturesPipeline->>SigstoreSignatory: check enabled mirror
SigstoreSignatory->>SignatureMirror: probe signature files
SignatureMirror-->>SigstoreSignatory: return HTTP status
SigstoreSignatory-->>VerifyReleaseSignaturesPipeline: return mirror result
VerifyReleaseSignaturesPipeline-->>CLI: render PASS or FAIL result
🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
pyartcd/tests/pipelines/test_verify_release_signatures.py (1)
104-169: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the "no digest" and "no mirrors checked" branches.
run()(per the upstream snippet) has anif not digest:branch that appends an error when digest lookup returns a falsy value without raising, and a path where neithercheck_dev_mirrornorcheck_prod_mirroris set (leaving an arch result with no mirror flags). Neither is exercised here; only the exception-raising digest failure (test_digest_failure) and dual-mirror-checked (test_all_pass,test_mixed_results) paths are tested.🤖 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 `@pyartcd/tests/pipelines/test_verify_release_signatures.py` around lines 104 - 169, Add tests in TestPipeline covering a falsy digest returned by get_digest_for_pullspec, asserting run() fails and records the expected error, and covering both check_dev_mirror and check_prod_mirror disabled, asserting the architecture result has no mirror flags while preserving the resulting pass/failure behavior defined by run().
🤖 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 `@pyartcd/pyartcd/pipelines/verify_release_signatures.py`:
- Around line 105-117: Update the mirror-check logic in run() so each awaited
SigstoreSignatory.check_signature_on_mirror call is independently wrapped in
error handling. On timeout or client failure, append the exception to
result.errors and continue processing the remaining mirror checks and
architectures, preserving normal success and missing-signature logging.
In `@pyartcd/pyartcd/signatory.py`:
- Around line 667-674: Update check_signature_on_mirror so a 404 response
returns False only when no signature has been found, while non-404 non-200
statuses—including 429 and 5xx responses—raise an appropriate HTTP error.
Preserve the existing success behavior for 200 responses and the sig iteration
flow.
---
Nitpick comments:
In `@pyartcd/tests/pipelines/test_verify_release_signatures.py`:
- Around line 104-169: Add tests in TestPipeline covering a falsy digest
returned by get_digest_for_pullspec, asserting run() fails and records the
expected error, and covering both check_dev_mirror and check_prod_mirror
disabled, asserting the architecture result has no mirror flags while preserving
the resulting pass/failure behavior defined by run().
🪄 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: Repository: openshift-eng/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: cc9b17e5-dd5f-4963-b1ce-701ec2e53f66
📒 Files selected for processing (5)
pyartcd/pyartcd/__main__.pypyartcd/pyartcd/pipelines/__init__.pypyartcd/pyartcd/pipelines/verify_release_signatures.pypyartcd/pyartcd/signatory.pypyartcd/tests/pipelines/test_verify_release_signatures.py
There was a problem hiding this comment.
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 `@pyartcd/pyartcd/signatory.py`:
- Around line 667-681: Update the signature lookup flow around the aiohttp
request to probe numbered signature files sequentially rather than hard-coding
only signature-1. Return true when any numbered signature exists, return false
only after all supported candidates are absent, and preserve the existing
unexpected-status error handling.
In `@pyartcd/tests/pipelines/test_verify_release_signatures.py`:
- Around line 118-119: Update the test around
SigstoreSignatory.check_signature_on_mirror to assert
aiohttp.ClientResponseError instead of the broad Exception type, while
preserving the existing call and asynchronous assertion behavior.
🪄 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: Repository: openshift-eng/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 6d410767-c7d0-493e-a1de-709db28ee4a4
📒 Files selected for processing (3)
pyartcd/pyartcd/pipelines/verify_release_signatures.pypyartcd/pyartcd/signatory.pypyartcd/tests/pipelines/test_verify_release_signatures.py
a6c3ef5 to
235bb51
Compare
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
…ify-signatures Resolve release name from runtime.assembly and default arches from group config, consistent with verify-image-grades pattern. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
Both dev and prod mirror signature checks are now on by default, matching what a release verification should cover. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
|
@tomasdavidorg: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
Summary by CodeRabbit
verify-release-signaturesCLI pipeline to verify release image signatures across selected architectures, with optional dev/prod sigstore mirror checks.