Several keyless verification task improvements#3187
Merged
simonbaird merged 1 commit intoconforma:mainfrom Mar 24, 2026
Merged
Conversation
Contributor
Review Summary by QodoAdd regexp support for keyless verification parameters
WalkthroughsDescription• Add support for regexp versions of keyless verification parameters • Implement preference logic for non-regexp params over regexp variants • Force --ignore-rekor=false for keyless verification workflows • Add comprehensive test scenarios for regexp-based certificate matching Diagramflowchart LR
A["Keyless Verification Parameters"] --> B["CERTIFICATE_IDENTITY<br/>CERTIFICATE_OIDC_ISSUER"]
A --> C["CERTIFICATE_IDENTITY_REGEXP<br/>CERTIFICATE_OIDC_ISSUER_REGEXP"]
B --> D["Preferred if present"]
C --> E["Fallback option"]
D --> F["Build command args"]
E --> F
F --> G["Force ignore-rekor=false<br/>for keyless flow"]
H["Traditional Signing"] --> I["Use PUBLIC_KEY<br/>and IGNORE_REKOR param"]
File Changes1. docs/modules/ROOT/pages/verify-conforma-konflux-ta.adoc
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds regexp-based keyless verification parameters and wiring across docs, tasks, and tests; validation logic now treats keyless mode as active when any certificate exact or regexp param is present, prefers exact params over regexp, and forces Rekor checks for keyless flows. Changes
Sequence Diagram(s)sequenceDiagram
participant Task as Task Runner
participant Validate as Validate Script
participant Cosign as Cosign
participant Rekor as Rekor
Task->>Validate: invoke with params (PUBLIC_KEY, IGNORE_REKOR, CERTIFICATE_IDENTITY, CERTIFICATE_OIDC_ISSUER, CERTIFICATE_IDENTITY_REGEXP, CERTIFICATE_OIDC_ISSUER_REGEXP)
Validate->>Validate: if any CERTIFICATE_* present -> keyless branch
alt Keyless branch
Validate->>Validate: pick identity := CERTIFICATE_IDENTITY or CERTIFICATE_IDENTITY_REGEXP
Validate->>Validate: pick issuer := CERTIFICATE_OIDC_ISSUER or CERTIFICATE_OIDC_ISSUER_REGEXP
Validate->>Validate: set --ignore-rekor=false
Validate->>Cosign: run verification with certificate args (identity, issuer, --ignore-rekor=false)
Cosign->>Rekor: record/check entry
Cosign-->>Validate: result
else Traditional branch
Validate->>Validate: use --public-key and --ignore-rekor="${IGNORE_REKOR}"
Validate->>Cosign: run verification with public-key and ignore-rekor flag
alt IGNORE_REKOR == false
Cosign->>Rekor: record/check entry
else
Note right of Cosign: Rekor checks skipped
end
Cosign-->>Validate: result
end
Validate-->>Task: final task result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Code Review by Qodo
1. PUBLIC_KEY precedence misdocumented
|
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@features/task_validate_image.feature`:
- Around line 382-412: Add two assertions to the keyless regexp scenarios: (1)
add a case proving exact params override regexp params by including both an
exact match parameter (e.g., CERTIFICATE_IDENTITY or CERTIFICATE_OIDC_ISSUER)
alongside CERTIFICATE_IDENTITY_REGEXP/CERTIFICATE_OIDC_ISSUER_REGEXP and assert
the task uses the exact values (update the "Keyless signing verification cosign
v2 style with regexp params" scenario and its counterpart around the other
block), and (2) add a scenario or variant that sets IGNORE_REKOR=true and runs
in keyless mode (IMAGES with keyless_v2) and assert Rekor is still used (e.g.,
via REKOR_HOST present in task results/logs) to ensure keyless forces Rekor;
update the step expectations (task success, "report-json" logs, and task results
snapshots) to include these verifications.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1bb57d69-5b2a-497a-98b7-c65b428d8658
⛔ Files ignored due to path filters (1)
features/__snapshots__/task_validate_image.snapis excluded by!**/*.snap
📒 Files selected for processing (5)
docs/modules/ROOT/pages/verify-conforma-konflux-ta.adocdocs/modules/ROOT/pages/verify-enterprise-contract.adocfeatures/task_validate_image.featuretasks/verify-conforma-konflux-ta/0.1/verify-conforma-konflux-ta.yamltasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml
Codecov Report✅ All modified and coverable lines are covered by tests.
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
simonbaird
force-pushed
the
improve-keyless-support
branch
from
7302a30 to
b7d26cf
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tasks/verify-conforma-konflux-ta/0.1/verify-conforma-konflux-ta.yaml (1)
86-98: Typo in parameter descriptions: "precendence" → "precedence".Lines 90 and 97 both have "precendence" instead of "precedence".
📝 Proposed fix
- name: CERTIFICATE_IDENTITY_REGEXP type: string description: >- Similar to CERTIFICATE_IDENTITY but the value is a regexp that will be matched. - Note that CERTIFICATE_IDENTITY takes precendence over this if both are present. + Note that CERTIFICATE_IDENTITY takes precedence over this if both are present. default: "" - name: CERTIFICATE_OIDC_ISSUER_REGEXP type: string description: >- Similar to CERTIFICATE_OIDC_ISSUER but a regexp that will be matched. Note that - CERTIFICATE_OIDC_ISSUER takes precendence over this if both are present. + CERTIFICATE_OIDC_ISSUER takes precedence over this if both are present. default: ""🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tasks/verify-conforma-konflux-ta/0.1/verify-conforma-konflux-ta.yaml` around lines 86 - 98, The two parameter descriptions for CERTIFICATE_IDENTITY_REGEXP and CERTIFICATE_OIDC_ISSUER_REGEXP contain a typo ("precendence"); update both descriptions to use the correct spelling "precedence" so the text reads "Note that CERTIFICATE_IDENTITY takes precedence over this..." and "Note that CERTIFICATE_OIDC_ISSUER takes precedence over this..." respectively; locate the descriptions by the parameter names CERTIFICATE_IDENTITY_REGEXP and CERTIFICATE_OIDC_ISSUER_REGEXP and replace the misspelled word.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tasks/verify-conforma-konflux-ta/0.1/verify-conforma-konflux-ta.yaml`:
- Around line 86-98: The two parameter descriptions for
CERTIFICATE_IDENTITY_REGEXP and CERTIFICATE_OIDC_ISSUER_REGEXP contain a typo
("precendence"); update both descriptions to use the correct spelling
"precedence" so the text reads "Note that CERTIFICATE_IDENTITY takes precedence
over this..." and "Note that CERTIFICATE_OIDC_ISSUER takes precedence over
this..." respectively; locate the descriptions by the parameter names
CERTIFICATE_IDENTITY_REGEXP and CERTIFICATE_OIDC_ISSUER_REGEXP and replace the
misspelled word.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0390ef42-15a1-40b5-9b88-bd0a5b27bf62
⛔ Files ignored due to path filters (1)
features/__snapshots__/task_validate_image.snapis excluded by!**/*.snap
📒 Files selected for processing (5)
docs/modules/ROOT/pages/verify-conforma-konflux-ta.adocdocs/modules/ROOT/pages/verify-enterprise-contract.adocfeatures/task_validate_image.featuretasks/verify-conforma-konflux-ta/0.1/verify-conforma-konflux-ta.yamltasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml
✅ Files skipped from review due to trivial changes (3)
- docs/modules/ROOT/pages/verify-conforma-konflux-ta.adoc
- docs/modules/ROOT/pages/verify-enterprise-contract.adoc
- tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- features/task_validate_image.feature
Contributor
robnester-rh
left a comment
robnester-rh
left a comment
There was a problem hiding this comment.
One small nit, and I'd like to see you address the IGNORE_REKOR documentation and PUBLIC_KEY descriptions identified by the ai reviewers.
simonbaird
force-pushed
the
improve-keyless-support
branch
from
b7d26cf to
2917135
Compare
Member
Author
|
I guess I'll fix the fresh conflict. |
Member
Author
|
Fixed the misleading param docs and the typo as suggested. |
Here's what's included: - Support for the -regexp versions of the keyless verification params. Prefer the non-regexp param if (for some reason) both are present. - Make it so we never use --ignore-rekor when doing keyless verification even if IGNORE_REKOR is true. This is because you need a transparency log entry from Rekor to do keyless verification. - Some minor bash env var handling logic tweaks related to handling of unlikely edge cases. Note that we're still trying not to add a layer of bash logic for param sanitizing as per the comment there. This could be broken up into multiple commits, and originally it was, but I've been working on a previous version of PR too long and I don't think it's worth the effort right now. Ref: https://redhat.atlassian.net/browse/EC-1652
simonbaird
force-pushed
the
improve-keyless-support
branch
from
2917135 to
1b3ecae
Compare
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
3 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.
Here's what's included:
This could be broken up into multiple commits, and originally it was, but I've been working on a previous version of PR too long and I don't think it's worth the effort right now.
Ref: https://redhat.atlassian.net/browse/EC-1652
Note: These changes were originally in #3171 also.