ART-21879: Add elliott verify-kernel-tag command - #3250
Conversation
Checks RHCOS builds in advisories for kernel packages with early-kernel-stop-ship Brew tag. Downloads metadata.json from brewroot to find kernel NVRs, then checks Koji tags. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
openshift-ci-robot
added
the
jira/valid-reference
|
@tomasdavidorg: This pull request references ART-21879 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 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift-eng/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughAdds the ChangesKernel tag verification
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ElliottCLI
participant AsyncErrataAPI
participant Brewroot
participant Koji
User->>ElliottCLI: run verify-kernel-tag
ElliottCLI->>AsyncErrataAPI: retrieve advisory builds
ElliottCLI->>Brewroot: retrieve RHCOS metadata
Brewroot-->>ElliottCLI: return kernel RPM NVRs
ElliottCLI->>Koji: check stop-ship tags
Koji-->>ElliottCLI: return tag status
ElliottCLI-->>User: render text or JSON result
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (8 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
elliott/elliottlib/cli/verify_kernel_tag_cli.py (2)
99-104: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider retrying transient brewroot failures.
A single transient network error or HTTP 5xx marks the advisory as errored. The command then exits with code 1 and reports a false stop-ship failure. Add a bounded retry with backoff for connection errors and 5xx responses.
🤖 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 `@elliott/elliottlib/cli/verify_kernel_tag_cli.py` around lines 99 - 104, Update get_kernel_rpms_from_rhcos to retry requests that fail with connection errors or HTTP 5xx responses, using a bounded number of attempts and backoff between retries. Preserve immediate propagation for other HTTP errors, and only call raise_for_status after a successful or non-retriable response.
83-85: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMalformed NVRs produce a silently wrong URL.
If
rhcos_nvrdoes not match the pattern,re.subreturns the input unchanged. The function then builds a URL without version and release segments. The failure surfaces later as an opaque HTTP 404. Also, an NVR that contains/or..alters the request path.Match explicitly and reject input that does not conform.
♻️ Proposed refactor
def nvr_to_brewroot_metadata_url(rhcos_nvr: str) -> str: - path = re.sub(r"-([\d.]+)-(\d+)$", r"/\1/\2", rhcos_nvr) - return f"{BREW_DOWNLOAD_URL}/packages/{path}/metadata.json" + match = re.fullmatch(r"([A-Za-z0-9_.+-]+?)-([\d.]+)-(\d+)", rhcos_nvr) + if not match: + raise ValueError(f"Cannot parse RHCOS NVR: {rhcos_nvr}") + name, version, release = match.groups() + return f"{BREW_DOWNLOAD_URL}/packages/{name}/{version}/{release}/metadata.json"🤖 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 `@elliott/elliottlib/cli/verify_kernel_tag_cli.py` around lines 83 - 85, Update nvr_to_brewroot_metadata_url to explicitly validate that rhcos_nvr matches the expected name-version-release pattern before constructing the URL. Reject malformed values instead of allowing re.sub to return the unchanged input, and reject values containing path traversal or slash characters such as "/" or "..". Preserve the existing URL format for valid NVRs.
🤖 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 `@elliott/elliottlib/cli/verify_kernel_tag_cli.py`:
- Around line 68-76: Define the intended multiple-tag contract in
get_kernel_packages_and_tag: either return a package-to-tag mapping or raise
when rpm_deliveries contain conflicting stop_ship_tag values, rather than
silently retaining the last tag. Update
elliott/elliottlib/cli/verify_kernel_tag_cli.py:68-76 accordingly, and update
elliott/tests/test_verify_kernel_tag_cli.py:162-168 to assert the tag1/tag2
behavior and use the tag variable.
- Around line 151-154: Update the no-kernel-RPM branch in the advisory
verification flow to fail closed instead of setting result.skipped and returning
a passing outcome. Treat an RHCOS build with no matching kernel packages as an
error, or at minimum record a visible warning consumed by the rendered output,
while preserving normal handling when kernel RPMs are found.
In `@elliott/tests/test_verify_kernel_tag_cli.py`:
- Around line 162-168: Update test_multiple_entries to assert the returned tag
from get_kernel_packages_and_tag, after confirming the intended tag-collapse
behavior for entries with tag1 and tag2 in get_kernel_packages_and_tag. Use that
expected value in an assertion so the unpacked tag is validated rather than
unused.
- Around line 252-258: Update test_http_error to assert
requests.exceptions.HTTPError rather than the broad Exception type, and verify
the raised exception message contains “404 Not Found” using the context-manager
result. Keep the existing mocked raise_for_status behavior and
get_kernel_rpms_from_rhcos invocation unchanged.
---
Nitpick comments:
In `@elliott/elliottlib/cli/verify_kernel_tag_cli.py`:
- Around line 99-104: Update get_kernel_rpms_from_rhcos to retry requests that
fail with connection errors or HTTP 5xx responses, using a bounded number of
attempts and backoff between retries. Preserve immediate propagation for other
HTTP errors, and only call raise_for_status after a successful or non-retriable
response.
- Around line 83-85: Update nvr_to_brewroot_metadata_url to explicitly validate
that rhcos_nvr matches the expected name-version-release pattern before
constructing the URL. Reject malformed values instead of allowing re.sub to
return the unchanged input, and reject values containing path traversal or slash
characters such as "/" or "..". Preserve the existing URL format for valid NVRs.
🪄 Autofix
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: 6b37cac3-163a-4a7d-9013-c040fa1d52ba
📒 Files selected for processing (3)
elliott/elliottlib/cli/__main__.pyelliott/elliottlib/cli/verify_kernel_tag_cli.pyelliott/tests/test_verify_kernel_tag_cli.py
- Validate conflicting stop_ship_tag values across rpm_deliveries entries - Fail closed (error) instead of open (skip) when no kernel RPMs found - Use specific HTTPError type in test assertions - Split test_multiple_entries into same-tag and different-tag cases Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
Reject malformed NVRs with ValueError instead of silently producing a wrong URL that would fail later with an opaque 404. 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. |
2 participants
Checks RHCOS builds in advisories for kernel packages with early-kernel-stop-ship Brew tag. Downloads metadata.json from brewroot to find kernel NVRs, then checks Koji tags.
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com
rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
Summary by CodeRabbit
New Features
verify-kernel-tagcommand to validate kernel builds against stop-ship tags.Tests