USHIFT-6807: AI Skill: Post-Release Verification (Phase 4) - #258
Conversation
USHIFT-6806 Add --errata <advisory_id> mode to the advisory-promotion skill. Validates MicroShift RPM advisories in the Red Hat Errata Tool before QE sign-off, covering: - Advisory exists, correct type (RHEA/RHBA/RHSA), QA ownership set - All OCPBUGS in Verified/Closed state - All MicroShift RPMs present and mapped to product listings - CDN staging push completed, RHN QA testing passed - Advisory moved to REL_PREP Authenticates via Kerberos (GSSAPI). Tested against real advisory 170194 (MicroShift 4.22.7). Bump plugin version 1.4.3 → 1.5.0. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> pre-commit.check-secrets: ENABLED
Auto-applied: - errata_promotion.py:449: JSON output now exits non-zero on FAIL - SKILL.md:3: added --json to argument-hint - lib/errata.py:200: removed unused loop variables - SKILL.md:183: aligned CAT check docs with rhnqa implementation Co-Authored-By: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> pre-commit.check-secrets: ENABLED
- Only include OCPBUGS-prefixed Jira issues in bug verification - Return None (not empty list) when jira_issues key is missing from ET response, so check_bugs_verified reports WARN instead of false PASS Co-Authored-By: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> pre-commit.check-secrets: ENABLED
- Make requests/requests_gssapi imports lazy so tests run with system Python - Catch JSONDecodeError in _et_get (VPN captive portals return HTML 200) - Differentiate VPN vs auth failures in check_auth logging - Fix check_bugs_verified docstring and output: say "accepted state" not "Verified state" since Closed and Release Pending are also accepted - Remove unused _EXPECTED_STATUS constant - Document _jira_issues injection in fetch_advisory docstring Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> pre-commit.check-secrets: ENABLED
- Add blank lines before lists after bold text (MD032) - Rename duplicate headings: "Bootc Mode" → "Bootc Mode Output", "Errata Mode" → "Errata Mode Output" in Output Format section (MD024) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> pre-commit.check-secrets: ENABLED
|
Skipping CI for Draft Pull Request. |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: agullon The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
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:
WalkthroughThe MicroShift release plugin adds Errata Tool promotion validation and post-release verification. It includes CLIs, API helpers, shell wrappers, unit tests, skill documentation, and version metadata updates. ChangesMicroShift release validation
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 9 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (9 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
agullon
changed the title
![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 (1)
plugins/microshift-release/skills/post-release/SKILL.md (1)
67-84: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPut failure handling in the command step.
Step 2 executes the command, but its VPN, Kerberos, network, and non-zero-exit policy appears only after the output step. Put the relevant stop condition and error handling immediately after the command.
Based on learnings: place failure policies and edge-case rules inline with the relevant tool invocation.
🤖 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 `@plugins/microshift-release/skills/post-release/SKILL.md` around lines 67 - 84, Move the command failure policies from Step 4 into Step 2 immediately after the post_release.sh invocation: document non-zero exit handling, VPN-unavailable degradation, Kerberos authentication recovery, and network-error behavior there. Keep Step 3 focused only on displaying complete stdout, and retain the EC/RC/nightly applicability rule in the most relevant step.Sources: Path instructions, Learnings
🤖 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 `@plugins/microshift-release/scripts/post_release.py`:
- Around line 363-365: Wrap the errata_future.result() call in the same
exception-handling flow used by the dependent checks, logging any future failure
and assigning errata_info to None instead of terminating report generation.
Preserve the dependent checks so they produce their normal FAIL or WARN
outcomes, and add a test covering a malformed Hydra response from
find_errata_for_version.
- Around line 120-121: Update the Hydra query construction in the post-release
validation flow to search all errata for the exact release version instead of
limiting results to the newest 20 records; use server-side exact-version
filtering or paginate through result pages until the version is found. Preserve
the existing matching and failure behavior, and add coverage for a matching
document appearing after the first 20 records.
- Around line 142-144: Validate doc_uri in the post-release advisory URL
handling before assigning or requesting portal_url: accept only HTTPS URLs on
access.redhat.com or access.stage.redhat.com whose path matches
/errata/RH…A-YYYY:NNNNN, and reject invalid records before returning
errata_info. Replace the broad startswith("http") check while preserving the
existing fallback behavior as appropriate, and add positive production/staging
tests plus negative scheme, host, and path cases.
In `@plugins/microshift-release/skills/post-release/SKILL.md`:
- Around line 54-70: Make the post-release workflow independent of the caller’s
working directory: in plugins/microshift-release/skills/post-release/SKILL.md
lines 54-70, invoke the wrapper as "$PLUGIN_DIR/scripts/post_release.sh" in the
command block; in plugins/microshift-release/scripts/post_release.sh lines 5-8,
resolve the output location from the script or plugin directory, or run Git with
-C "${SCRIPTDIR}".
---
Nitpick comments:
In `@plugins/microshift-release/skills/post-release/SKILL.md`:
- Around line 67-84: Move the command failure policies from Step 4 into Step 2
immediately after the post_release.sh invocation: document non-zero exit
handling, VPN-unavailable degradation, Kerberos authentication recovery, and
network-error behavior there. Keep Step 3 focused only on displaying complete
stdout, and retain the EC/RC/nightly applicability rule in the most relevant
step.
🪄 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 YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 31184819-7573-46b5-bcb6-9d179dbd2ff7
📒 Files selected for processing (7)
.claude-plugin/marketplace.jsonplugins/microshift-release/.claude-plugin/plugin.jsonplugins/microshift-release/README.mdplugins/microshift-release/scripts/post_release.pyplugins/microshift-release/scripts/post_release.shplugins/microshift-release/scripts/unit_tests/test_post_release.pyplugins/microshift-release/skills/post-release/SKILL.md
| "rows": 20, | ||
| "sort": "portal_publication_date desc", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Search all matching errata records.
The Hydra query returns only the newest 20 records for a minor release. Older z-stream advisories can be outside this window, so this check reports a false failure for a published advisory.
Add server-side exact-version filtering or paginate until the exact version is found. Add a test where the matching document occurs after the first 20 records.
🤖 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 `@plugins/microshift-release/scripts/post_release.py` around lines 120 - 121,
Update the Hydra query construction in the post-release validation flow to
search all errata for the exact release version instead of limiting results to
the newest 20 records; use server-side exact-version filtering or paginate
through result pages until the version is found. Preserve the existing matching
and failure behavior, and add coverage for a matching document appearing after
the first 20 records.
| portal_url = doc_uri | ||
| if not portal_url.startswith("http"): | ||
| portal_url = f"https://access.redhat.com/errata/{advisory_name}" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- target implementation ---'
sed -n '1,260p' plugins/microshift-release/scripts/post_release.py
printf '%s\n' '--- relevant tests ---'
sed -n '1,230p' plugins/microshift-release/scripts/unit_tests/test_post_release.py
printf '%s\n' '--- URL and portal references ---'
rg -n -C 3 'doc_uri|portal_url|find_errata|errata_info|access\.redhat|requests\.(get|post)' plugins/microshift-release
printf '%s\n' '--- CONTRIBUTING guidance ---'
rg -n -C 2 'validation|negative tests|allow|credential|release skill' CONTRIBUTING.mdRepository: openshift-eng/edge-tooling
Length of output: 50382
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- control flow into the portal request ---'
sed -n '330,382p' plugins/microshift-release/scripts/post_release.py
printf '%s\n' '--- exact CONTRIBUTING requirements ---'
rg -n -C 4 'validation and parsing|positive and negative|new release skill|allow-list|strict shell|ruff' CONTRIBUTING.md
printf '%s\n' '--- focused test inventory ---'
rg -n 'find_errata_for_version|portal_url|Hydra|uri|redirect' plugins/microshift-release/scripts/unit_tests/test_post_release.py plugins/microshift-release/scripts/unit_tests
printf '%s\n' '--- read-only AST and behavior verifier ---'
python3 - <<'PY'
import ast
from pathlib import Path
path = Path("plugins/microshift-release/scripts/post_release.py")
tree = ast.parse(path.read_text())
find_fn = next(
node for node in tree.body
if isinstance(node, ast.FunctionDef) and node.name == "find_errata_for_version"
)
portal_fn = next(
node for node in tree.body
if isinstance(node, ast.FunctionDef) and node.name == "check_rpms_customer_portal"
)
print("find_errata_for_version assignments and calls:")
for node in ast.walk(find_fn):
if isinstance(node, ast.Assign):
targets = [ast.unparse(t) for t in node.targets]
if any("portal_url" in target for target in targets):
print(" assignment:", ast.unparse(node))
if isinstance(node, ast.Call) and isinstance(node.func, ast.Attribute):
if node.func.attr == "startswith":
print(" startswith call:", ast.unparse(node))
print("check_rpms_customer_portal request calls:")
for node in ast.walk(portal_fn):
if isinstance(node, ast.Call) and isinstance(node.func, ast.Attribute):
if node.func.attr == "get":
print(" request:", ast.unparse(node))
# Model the exact current fallback condition without importing repository code.
values = [
"http://127.0.0.1:8080/secret",
"https://attacker.example/collect",
"https://access.redhat.com/errata/RHBA-2026:12345",
"ftp://attacker.example/file",
"",
]
for value in values:
result = value if value.startswith("http") else (
"https://access.redhat.com/errata/RHBA-2026:12345"
)
print(f"{value!r} -> {result!r}")
PYRepository: openshift-eng/edge-tooling
Length of output: 6571
SSRF (CWE-918): Server-Side Request Forgery (SSRF)
Reachability: External
Reachability path
● Entry
plugins/microshift-release/scripts/unit_tests/test_post_release.py:69
test_el9_only
│
▼
● Sink
plugins/microshift-release/scripts/post_release.py
Validate Hydra-derived advisory URLs before the portal request.
doc["uri"] controls portal_url, and startswith("http") permits arbitrary HTTP or HTTPS destinations. The portal request also follows redirects.
Accept only HTTPS URLs for access.redhat.com or access.stage.redhat.com with an /errata/RH…A-YYYY:NNNNN path. Reject invalid records before returning errata_info. Add positive production and staging tests and negative tests for invalid schemes, hosts, and paths.
Per CONTRIBUTING.md, regex patterns and parsing rules require positive and negative tests.
🤖 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 `@plugins/microshift-release/scripts/post_release.py` around lines 142 - 144,
Validate doc_uri in the post-release advisory URL handling before assigning or
requesting portal_url: accept only HTTPS URLs on access.redhat.com or
access.stage.redhat.com whose path matches /errata/RH…A-YYYY:NNNNN, and reject
invalid records before returning errata_info. Replace the broad
startswith("http") check while preserving the existing fallback behavior as
appropriate, and add positive production/staging tests plus negative scheme,
host, and path cases.
Sources: Coding guidelines, Path instructions, Learnings
| ## Scripts Directory | ||
|
|
||
| ```bash | ||
| SCRIPTS_DIR=plugins/microshift-release/scripts | ||
| ``` | ||
|
|
||
| ## Implementation | ||
|
|
||
| ### Step 1: Parse Arguments | ||
|
|
||
| 1. Extract `version` from `$ARGUMENTS` — the first non-flag token | ||
| 2. Pass through `--verbose` and `--json` flags if present | ||
|
|
||
| ### Step 2: Run Checks | ||
|
|
||
| ```bash | ||
| bash $SCRIPTS_DIR/post_release.sh <version> [--verbose] [--json] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make the post-release entry point independent of the caller directory.
The skill uses a repository-relative SCRIPTS_DIR, and the wrapper runs git rev-parse from the caller directory. The workflow fails when the user invokes the skill outside the repository root.
plugins/microshift-release/skills/post-release/SKILL.md#L54-L70: invoke the wrapper with"$PLUGIN_DIR/scripts/post_release.sh"in the same command block.plugins/microshift-release/scripts/post_release.sh#L5-L8: derive the output location from the resolved script or plugin directory, or run Git with-C "${SCRIPTDIR}".
As per path instructions: use $PLUGIN_DIR for skill workflows.
🧰 Tools
🪛 SkillSpector (2.5.1)
[warning] 165: [EA2] Autonomous Decision Making: Skill enables autonomous high-impact decisions without human-in-the-loop verification. Critical operations (destructive commands, financial transactions, data deletion) should require explicit user confirmation.
Remediation: Add human-in-the-loop confirmation for destructive, irreversible, or high-impact operations. Never auto-execute commands that modify files, send data, or alter system state.
(Excessive Agency (EA2))
📍 Affects 2 files
plugins/microshift-release/skills/post-release/SKILL.md#L54-L70(this comment)plugins/microshift-release/scripts/post_release.sh#L5-L8
🤖 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 `@plugins/microshift-release/skills/post-release/SKILL.md` around lines 54 -
70, Make the post-release workflow independent of the caller’s working
directory: in plugins/microshift-release/skills/post-release/SKILL.md lines
54-70, invoke the wrapper as "$PLUGIN_DIR/scripts/post_release.sh" in the
command block; in plugins/microshift-release/scripts/post_release.sh lines 5-8,
resolve the output location from the script or plugin directory, or run Git with
-C "${SCRIPTDIR}".
Source: Path instructions
|
/label tide/merge-method-squash |
openshift-ci
Bot
added
the
tide/merge-method-squash
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
plugins/microshift-release/skills/post-release/SKILL.md (3)
51-58: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDefine invalid-argument stop conditions.
Specify the behavior for a missing version, multiple positional tokens, unknown flags, repeated flags, and
--verbosecombined with--json.Reject invalid input before Step 2. Define whether
--verboseand--jsonare mutually exclusive or which output mode takes precedence.As per path instructions:
SKILL.mdmust define numbered steps, explicit stop conditions, output formats, and edge cases.🤖 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 `@plugins/microshift-release/skills/post-release/SKILL.md` around lines 51 - 58, Update the Arguments section and numbered workflow in the post-release skill to define validation stop conditions for a missing version, multiple positional tokens, unknown or repeated flags, and combining --verbose with --json. State that invalid input must stop before Step 2, specify whether the output flags are mutually exclusive or establish precedence, and document the resulting output formats and edge-case behavior.Source: Path instructions
75-77: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMake the shell commands executable.
In the Bash block,
<version>is input redirection.[--verbose]and[--json]are literal arguments. Thekinit <username>@REDHAT.COM`` example has the same problem.Define
versionand the flags in Step 1, then pass them as arguments. Show an actual Kerberos principal or state that the placeholder must be replaced before execution.Per CONTRIBUTING.md: use strict quoted shell variables in Bash examples.
Proposed command form
-bash $SCRIPTS_DIR/post_release.sh <version> [--verbose] [--json] +bash "$PLUGIN_DIR/scripts/post_release.sh" "$version" "${flags[@]}"Also applies to: 89-90
🤖 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 `@plugins/microshift-release/skills/post-release/SKILL.md` around lines 75 - 77, Update the Step 1 Bash examples for post_release.sh to define version and optional flag variables before invoking the script, then pass all shell variables as quoted arguments rather than showing angle-bracket or bracket placeholders. Update the kinit example similarly by using an actual Kerberos principal or explicitly marking the username placeholder for replacement before execution, and apply strict quoting to every shell variable.Source: Path instructions
108-113: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAlign
pr_rpms_customer_portalwith the RPM availability requirement.The implementation only checks whether the advisory URL returns HTTP 200. It does not verify RPM package presence. Update the check and output, or document it as advisory-page reachability only.
🤖 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 `@plugins/microshift-release/skills/post-release/SKILL.md` around lines 108 - 113, Update the pr_rpms_customer_portal check documentation and implementation reference so it reflects the RPM availability requirement by verifying RPM package presence in addition to advisory-page access; alternatively, explicitly rename or document the check as advisory-page reachability only if package verification is out of scope. Keep the RPM check descriptions aligned with the behavior reported by each check.Source: Path instructions
🧹 Nitpick comments (1)
plugins/microshift-release/skills/post-release/SKILL.md (1)
73-90: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep failure policies beside Step 2.
The VPN, Kerberos, and network failure rules appear after Step 3. Move them immediately below the
post_release.shinvocation so the action and its stop or warning behavior remain together.Based on learnings: co-locate failure policies and edge-case rules with the relevant tool invocation.
🤖 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 `@plugins/microshift-release/skills/post-release/SKILL.md` around lines 73 - 90, Move the VPN-unavailable, Kerberos-authentication, and network-error handling bullets from Step 4 to immediately below the post_release.sh invocation in Step 2. Keep their existing WARN, authentication, and continued-check behavior unchanged, and leave the version-scope rule in the appropriate error-handling section.Source: Learnings
🤖 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.
Outside diff comments:
In `@plugins/microshift-release/skills/post-release/SKILL.md`:
- Around line 51-58: Update the Arguments section and numbered workflow in the
post-release skill to define validation stop conditions for a missing version,
multiple positional tokens, unknown or repeated flags, and combining --verbose
with --json. State that invalid input must stop before Step 2, specify whether
the output flags are mutually exclusive or establish precedence, and document
the resulting output formats and edge-case behavior.
- Around line 75-77: Update the Step 1 Bash examples for post_release.sh to
define version and optional flag variables before invoking the script, then pass
all shell variables as quoted arguments rather than showing angle-bracket or
bracket placeholders. Update the kinit example similarly by using an actual
Kerberos principal or explicitly marking the username placeholder for
replacement before execution, and apply strict quoting to every shell variable.
- Around line 108-113: Update the pr_rpms_customer_portal check documentation
and implementation reference so it reflects the RPM availability requirement by
verifying RPM package presence in addition to advisory-page access;
alternatively, explicitly rename or document the check as advisory-page
reachability only if package verification is out of scope. Keep the RPM check
descriptions aligned with the behavior reported by each check.
---
Nitpick comments:
In `@plugins/microshift-release/skills/post-release/SKILL.md`:
- Around line 73-90: Move the VPN-unavailable, Kerberos-authentication, and
network-error handling bullets from Step 4 to immediately below the
post_release.sh invocation in Step 2. Keep their existing WARN, authentication,
and continued-check behavior unchanged, and leave the version-scope rule in the
appropriate error-handling section.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: fbf165bb-a2a5-437d-ad30-002665eee42e
📒 Files selected for processing (1)
plugins/microshift-release/skills/post-release/SKILL.md
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
2 similar comments
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (10)
plugins/microshift-release/scripts/unit_tests/test_errata_promotion.py (3)
148-163: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a case for the
release pendingaccepted status.
check_bugs_verifiedinerrata_promotion.pyline 115 accepts three statuses:verified,closed, andrelease pending. These tests coverVerifiedandClosed. No test coversRelease Pending. A typo in that allow-list entry would pass the suite and then fail real advisories.💚 Proposed test
+ def test_release_pending(self): + bugs = [{"key": "OCPBUGS-1", "status": "Release Pending"}] + r = check_bugs_verified(bugs) + self.assertEqual(r["status"], "PASS") + def test_mixed_verified_closed(self):Per CONTRIBUTING.md: "Validation/parsing logic requires positive and negative tests".
🤖 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 `@plugins/microshift-release/scripts/unit_tests/test_errata_promotion.py` around lines 148 - 163, Add a positive unit test alongside test_all_closed and test_mixed_verified_closed that passes a bug with status "Release Pending" to check_bugs_verified and asserts the result status is "PASS", covering the accepted release-pending status.Source: Path instructions
205-207: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the full reason fragment instead of a bare digit.
assertIn("2", r["reason"])passes whenever any2appears in the reason string. The reason also embeds version numbers, so the assertion cannot distinguish a correct missing count from a wrong one.💚 Proposed stronger assertion
r = check_rpms_present(nvrs, _version("4.20.26")) self.assertEqual(r["status"], "FAIL") - self.assertIn("2", r["reason"]) + self.assertIn("2 MicroShift RPM(s) missing", r["reason"]) + self.assertIn("Missing: microshift-networking, microshift-selinux", + r["details"][0])🤖 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 `@plugins/microshift-release/scripts/unit_tests/test_errata_promotion.py` around lines 205 - 207, Strengthen the assertion in the test around check_rpms_present so it verifies the complete expected missing-count reason fragment rather than searching for the bare digit “2”. Keep the existing FAIL status assertion and ensure the expected text is specific enough not to match digits in embedded version numbers.
274-284: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the
push_countbranch ofcheck_cdn_staging.
check_cdn_staginginerrata_promotion.pylines 205 and 212-213 readspushcountwith apush_countfallback and returns PASS when the value is greater than zero. No test exercises that branch. It converts a WARN into a PASS, so a wrong key name silently degrades the check to a warning for every advisory.💚 Proposed tests
def test_qe_status(self): r = check_cdn_staging(_advisory(status="QE")) self.assertEqual(r["status"], "WARN") + def test_push_count_recorded(self): + r = check_cdn_staging(_advisory(status="QE", pushcount=2)) + self.assertEqual(r["status"], "PASS") + + def test_push_count_zero(self): + r = check_cdn_staging(_advisory(status="QE", pushcount=0)) + self.assertEqual(r["status"], "WARN") +Per CONTRIBUTING.md: "Validation/parsing logic requires positive and negative tests".
🤖 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 `@plugins/microshift-release/scripts/unit_tests/test_errata_promotion.py` around lines 274 - 284, Add positive and negative unit tests near test_qe_status in the test class to exercise check_cdn_staging’s push_count handling: verify a positive pushcount/push_count value returns PASS and a zero or absent value retains WARN. Use _advisory to construct the inputs and cover both accepted key forms, including the push_count fallback.Source: Path instructions
plugins/microshift-release/scripts/errata_promotion.py (3)
31-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the check identifiers from one source.
_CHECKSlists the nine check identifiers, andrun_errata_promotion_checkslines 296-306 repeats the same nine checks in the same order. The two lists must stay aligned by hand. If a check is added to one list only, the auth-failure path at line 277 and the advisory-not-found path at lines 283-285 report a different check set than the normal path, and the JSON consumers see an inconsistent result shape.Define one registry that maps each check identifier to its callable, then build
_CHECKSand the results list from it.Per CONTRIBUTING.md: "avoid redundant checks or derived state".
🤖 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 `@plugins/microshift-release/scripts/errata_promotion.py` around lines 31 - 41, Define a single check registry mapping each identifier in _CHECKS to its corresponding callable, then derive _CHECKS and the results iteration in run_errata_promotion_checks from that registry. Remove the duplicated hard-coded check sequence while preserving its current order and ensuring all early-return paths report the same registry-derived check set.Source: Path instructions
17-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider promoting the shared status helpers out of
validate_artifacts.This module imports five underscore-prefixed names from
validate_artifacts. Those names signal a private API, so the coupling is fragile. The siblingpost_release.pyin this PR needs the same helpers.Move
_pass,_fail,_warn,_skip, and the status emoji map into a shared module such aslib/status.pywith public names. Then both CLIs import a stable surface.🤖 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 `@plugins/microshift-release/scripts/errata_promotion.py` around lines 17 - 22, Move the shared status helpers and emoji map currently imported by errata promotion from validate_artifacts into a shared module such as lib/status.py, exposing public names. Update validate_artifacts, errata promotion, and sibling post_release.py to import the shared API, while preserving the existing SKIP emoji override and behavior.
168-181: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the builds traversal into
lib/errata.py.This loop re-walks
builds_datawith the same nested-shape logic asextract_microshift_nvrsinlib/errata.pylines 203-212: the same dict-or-list handling, the samebuildskey fallback, and the same"microshift" in nvr_key.lower()test. If the Errata Tool builds shape changes, both walkers must change together, and only one has direct test coverage of the arch nesting.Add a helper in
lib/errata.pythat returns the product-version to MicroShift NVR mapping. Thenextract_microshift_nvrsand this check both consume it.Per CONTRIBUTING.md: "avoid redundant checks or derived state".
🤖 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 `@plugins/microshift-release/scripts/errata_promotion.py` around lines 168 - 181, The duplicated builds traversal in the promotion check should be centralized in lib/errata.py. Add a helper that walks builds_data once and returns the product-version-to-MicroShift-NVR mapping, then update extract_microshift_nvrs and the product_versions check in the promotion flow to consume that helper instead of maintaining separate traversal and derived state.Source: Path instructions
plugins/microshift-release/scripts/lib/errata.py (3)
41-41: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winValidate the advisory identifier shape before building the URL.
advisory_idreaches this f-string from CLI argv without validation. A malformed value produces a confusing 404 instead of a clear error. An anchored check keeps the failure message actionable and matches the existing errata identifier validation convention in this plugin.♻️ Proposed validation helper
+_ADVISORY_ID_RE = re.compile(r"^(?:\d+|RH[BES]A-\d{4}:\d+)$") + + def _et_get(path, **kwargs):Then reject values that do not match in
fetch_advisoryand the otherfetch_*helpers before calling_et_get.Based on learnings: validate errata identifiers with an anchored, allowlisted pattern before using them in requests.
🤖 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 `@plugins/microshift-release/scripts/lib/errata.py` at line 41, Validate advisory_id with an anchored allowlisted errata-identifier pattern before constructing request URLs or calling _et_get. Apply the same validation in fetch_advisory and every other fetch_* helper, returning a clear actionable error for invalid values while preserving valid requests.Source: Learnings
226-229: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueGuard the leaf element type for consistency with the surrounding checks.
Every other level in this loop uses an
isinstanceguard. The leaf does not. Ifrpm_listcontains a non-string element,rpm_file.lower()raisesAttributeErrorand aborts the whole promotion check run.♻️ Proposed guard
for rpm_file in rpm_list: - if "microshift" in rpm_file.lower(): + if not isinstance(rpm_file, str): + continue + if "microshift" in rpm_file.lower(): rpms.append(rpm_file) found_rpms = True🤖 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 `@plugins/microshift-release/scripts/lib/errata.py` around lines 226 - 229, Add an isinstance string guard to the rpm_file check in the rpm_list loop before calling lower(), matching the surrounding type-checking pattern. Only append entries and set found_rpms for string values containing “microshift”; safely skip non-string elements without aborting the promotion check.
283-284: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
lib/errata.pyexposes data and helpers that no consumer reads. The promotion CLI is the only consumer in this layer, and it reads neither the per-bugsummary/is_privatefields nor four of thefetch_*helpers. The shared root cause is unused API surface inlib/errata.py; the remediation is to remove each unused item or wire it into the check that currently infers the same state.
plugins/microshift-release/scripts/lib/errata.py#L283-L284: dropsummaryandis_privatefrom the returned bug dict, or keepis_privateand use it to redactsummarywhere it is collected.plugins/microshift-release/scripts/errata_promotion.py#L288-L293: removefetch_jira_issues,fetch_external_tests,fetch_cdn_repos, andfetch_push_statusifpost_release.pydoes not call them, or callfetch_cdn_repos/fetch_push_statusfromcheck_cdn_staginginstead of inferring CDN state from the advisory status.Per CONTRIBUTING.md: "avoid redundant checks or derived state".
🤖 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 `@plugins/microshift-release/scripts/lib/errata.py` around lines 283 - 284, Remove the unused per-bug summary and is_private fields from the returned bug dictionary in plugins/microshift-release/scripts/lib/errata.py#L283-L284, unless is_private is used to redact summary during collection. In plugins/microshift-release/scripts/errata_promotion.py#L288-L293, remove fetch_jira_issues, fetch_external_tests, fetch_cdn_repos, and fetch_push_status when post_release.py does not call them; otherwise update check_cdn_staging to use fetch_cdn_repos and fetch_push_status instead of inferring CDN state from advisory status.Source: Path instructions
plugins/microshift-release/scripts/errata_promotion.sh (1)
6-8: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHandle missing Git work trees explicitly.
git rev-parse --show-toplevelexits with status 128 outside a checkout, andset -euo pipefailexposes only Git’s generic error. Either deriveOUTPUT_DIRfromSCRIPTDIRor catch this failure and report the script-specific prerequisite. Per CONTRIBUTING.md, document this prerequisite and failure mode. The sibling wrappers use the same pattern; keep their behavior consistent.🤖 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 `@plugins/microshift-release/scripts/errata_promotion.sh` around lines 6 - 8, Update the repository-root initialization in errata_promotion.sh to handle execution outside a Git work tree explicitly, either by deriving OUTPUT_DIR from SCRIPTDIR or by catching git rev-parse failure and reporting the script-specific prerequisite. Document the required Git checkout and resulting failure mode per CONTRIBUTING.md, while preserving behavior consistent with the sibling wrappers.Source: Path instructions
🤖 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 `@plugins/microshift-release/scripts/errata_promotion.sh`:
- Around line 10-20: Update the environment setup guard before the venv creation
flow to check for the executable ${ENVDIR}/bin/python3 rather than only the
${ENVDIR} directory. Ensure an incomplete existing environment reruns python3 -m
venv, while preserving the current setup and dependency-marker behavior for
valid environments.
In `@plugins/microshift-release/scripts/lib/errata.py`:
- Around line 273-282: Update the bug parsing logic around the fields selection
and status extraction to read status from the outer item when the inner
jira_issue fields do not provide it, while preserving the existing dict, string,
and unknown handling. Extend test_jira_issue_key to assert the expected
bugs[0]["status"] value for the wrapper/inner-issue response shape.
- Around line 25-28: Update the session setup around HTTPSPNEGOAuth so TLS
certificate verification remains enabled by removing _session.verify = False and
the urllib3 warning suppression. Configure _session.verify from the ET_CA_BUNDLE
environment variable, using the existing trusted CA configuration mechanism if
available, before the Kerberos session sends requests.
In `@plugins/microshift-release/scripts/post_release.py`:
- Around line 524-533: Add rejection-path tests covering EC, RC, and nightly
versions, asserting exit status 1 and the corresponding stderr messages from the
post-release validation. Include a GA or z-stream control test that mocks
run_post_release_checks and verifies it is invoked successfully.
- Around line 239-242: Update the advisory availability check around the
requests.get call to validate an advisory-specific marker in the final HTTP 200
response before calling _pass, rejecting login or generic pages; retain failure
handling for non-matching responses and add both positive and negative tests
covering a valid advisory page and an HTTP 200 non-advisory page.
In `@plugins/microshift-release/skills/advisory-promotion/SKILL.md`:
- Line 86: Update the advisory-promotion workflow documentation so both wrapper
commands in Steps 2a and 2b accept and forward the optional --json flag,
matching the flag handling described in Step 1. Preserve --json through argument
forwarding so JSON requests produce JSON output from both commands.
- Line 67: Make the --prod behavior consistent throughout the advisory-promotion
skill: either pass --prod only when the user explicitly supplies it and preserve
stage-only as the default, or remove the option and document that Bootc always
checks both catalogs. Update the usage instructions around the --prod definition
and the command requirements together so they describe the same contract.
In `@plugins/microshift-release/skills/post-release/SKILL.md`:
- Line 29: Align the RHEL 10 bootc version threshold in SKILL.md across the
entries around microshift-bootc-rhel10 and the related guidance at lines 41 and
105-106, using 4.22.2+ consistently to match the implementation and tests.
Document that versions 4.22.0 and 4.22.1 do not receive this bootc behavior.
---
Nitpick comments:
In `@plugins/microshift-release/scripts/errata_promotion.py`:
- Around line 31-41: Define a single check registry mapping each identifier in
_CHECKS to its corresponding callable, then derive _CHECKS and the results
iteration in run_errata_promotion_checks from that registry. Remove the
duplicated hard-coded check sequence while preserving its current order and
ensuring all early-return paths report the same registry-derived check set.
- Around line 17-22: Move the shared status helpers and emoji map currently
imported by errata promotion from validate_artifacts into a shared module such
as lib/status.py, exposing public names. Update validate_artifacts, errata
promotion, and sibling post_release.py to import the shared API, while
preserving the existing SKIP emoji override and behavior.
- Around line 168-181: The duplicated builds traversal in the promotion check
should be centralized in lib/errata.py. Add a helper that walks builds_data once
and returns the product-version-to-MicroShift-NVR mapping, then update
extract_microshift_nvrs and the product_versions check in the promotion flow to
consume that helper instead of maintaining separate traversal and derived state.
In `@plugins/microshift-release/scripts/errata_promotion.sh`:
- Around line 6-8: Update the repository-root initialization in
errata_promotion.sh to handle execution outside a Git work tree explicitly,
either by deriving OUTPUT_DIR from SCRIPTDIR or by catching git rev-parse
failure and reporting the script-specific prerequisite. Document the required
Git checkout and resulting failure mode per CONTRIBUTING.md, while preserving
behavior consistent with the sibling wrappers.
In `@plugins/microshift-release/scripts/lib/errata.py`:
- Line 41: Validate advisory_id with an anchored allowlisted errata-identifier
pattern before constructing request URLs or calling _et_get. Apply the same
validation in fetch_advisory and every other fetch_* helper, returning a clear
actionable error for invalid values while preserving valid requests.
- Around line 226-229: Add an isinstance string guard to the rpm_file check in
the rpm_list loop before calling lower(), matching the surrounding type-checking
pattern. Only append entries and set found_rpms for string values containing
“microshift”; safely skip non-string elements without aborting the promotion
check.
- Around line 283-284: Remove the unused per-bug summary and is_private fields
from the returned bug dictionary in
plugins/microshift-release/scripts/lib/errata.py#L283-L284, unless is_private is
used to redact summary during collection. In
plugins/microshift-release/scripts/errata_promotion.py#L288-L293, remove
fetch_jira_issues, fetch_external_tests, fetch_cdn_repos, and fetch_push_status
when post_release.py does not call them; otherwise update check_cdn_staging to
use fetch_cdn_repos and fetch_push_status instead of inferring CDN state from
advisory status.
In `@plugins/microshift-release/scripts/unit_tests/test_errata_promotion.py`:
- Around line 148-163: Add a positive unit test alongside test_all_closed and
test_mixed_verified_closed that passes a bug with status "Release Pending" to
check_bugs_verified and asserts the result status is "PASS", covering the
accepted release-pending status.
- Around line 205-207: Strengthen the assertion in the test around
check_rpms_present so it verifies the complete expected missing-count reason
fragment rather than searching for the bare digit “2”. Keep the existing FAIL
status assertion and ensure the expected text is specific enough not to match
digits in embedded version numbers.
- Around line 274-284: Add positive and negative unit tests near test_qe_status
in the test class to exercise check_cdn_staging’s push_count handling: verify a
positive pushcount/push_count value returns PASS and a zero or absent value
retains WARN. Use _advisory to construct the inputs and cover both accepted key
forms, including the push_count fallback.
🪄 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 YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: eba99182-d1e3-4d0b-aa88-627b70ade3e7
📒 Files selected for processing (12)
.claude-plugin/marketplace.jsonplugins/microshift-release/.claude-plugin/plugin.jsonplugins/microshift-release/README.mdplugins/microshift-release/scripts/errata_promotion.pyplugins/microshift-release/scripts/errata_promotion.shplugins/microshift-release/scripts/lib/errata.pyplugins/microshift-release/scripts/post_release.pyplugins/microshift-release/scripts/post_release.shplugins/microshift-release/scripts/unit_tests/test_errata_promotion.pyplugins/microshift-release/scripts/unit_tests/test_post_release.pyplugins/microshift-release/skills/advisory-promotion/SKILL.mdplugins/microshift-release/skills/post-release/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (5)
- plugins/microshift-release/.claude-plugin/plugin.json
- .claude-plugin/marketplace.json
- plugins/microshift-release/scripts/post_release.sh
- plugins/microshift-release/README.md
- plugins/microshift-release/scripts/unit_tests/test_post_release.py
| urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) | ||
| _session = requests.Session() | ||
| _session.auth = HTTPSPNEGOAuth() | ||
| _session.verify = False |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether other scripts in this repo already disable TLS verification,
# and whether an internal CA bundle path or env var convention exists.
rg -n --glob '*.py' -C3 'verify\s*=\s*False|InsecureRequestWarning|CA_BUNDLE|REQUESTS_CA_BUNDLE|ca_bundle'
rg -n --glob '*.txt' -C1 'requests|gssapi'Repository: openshift-eng/edge-tooling
Length of output: 164
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- errata.py relevant sections ---'
sed -n '1,175p' plugins/microshift-release/scripts/lib/errata.py
printf '%s\n' '--- dependency and documentation references ---'
rg -n -S -C2 'requests-gssapi|HTTPSPNEGOAuth|errata\.devel\.redhat\.com|ET_CA_BUNDLE|REQUESTS_CA_BUNDLE|verify\s*=\s*False|InsecureRequestWarning' \
plugins/microshift-release CONTRIBUTING.md requirements*.txt pyproject.toml setup.cfg 2>/dev/null || trueRepository: openshift-eng/edge-tooling
Length of output: 13817
🌐 Web query:
requests-gssapi HTTPSPNEGOAuth default mutual_authentication verify server certificate behavior
💡 Result:
In the requests-gssapi library, mutual authentication is disabled by default [1][2]. While historically it was enabled by default, this was changed because it is often unnecessary over secure TLS connections and frequently causes handshake failures due to incompatibility with certain servers that do not handle the required extra round-trip correctly [3][4]. Regarding the behavior of HTTPSPNEGOAuth and server authentication: 1. Mutual Authentication Behavior: If explicitly enabled, HTTPSPNEGOAuth requires mutual authentication from the server [3]. If a server provides a non-error response that cannot be successfully authenticated, a requests_gssapi.errors.MutualAuthenticationError is raised [3]. For error responses that cannot be authenticated, the library by default strips the response content and headers; this behavior can be suppressed by setting sanitize_mutual_error_response=False [3]. 2. Server Certificate Verification: The library does not inherently replace standard TLS certificate verification performed by the underlying requests/urllib3 stack. However, it offers optional support for GSSAPI channel bindings using the tls-server-end-point mechanism [5][6]. When channel_bindings='tls-server-end-point' is configured, the library uses the TLS server's certificate to construct channel bindings, which are then passed to GSSAPI [5]. This process requires the python-cryptography library to process the peer's certificate [6]. If the connection is closed—for instance, due to an initial authentication failure—the peer certificate may be unavailable, causing this channel binding process to fail [5][6]. In summary, the library relies on the underlying TLS layer for server certificate verification [3][4]. While it does not perform separate certificate validation for the purpose of GSSAPI authentication, it can cryptographically link the GSSAPI exchange to the TLS channel via channel bindings [5][6].
Citations:
- 1: https://github.com/pythongssapi/requests-gssapi/blob/v1.4.0/HISTORY.rst
- 2: https://github.com/pythongssapi/requests-gssapi/blob/efa1be2951676f55bdd978e673d3b4ad3774f7d2/README.rst
- 3: https://github.com/pythongssapi/requests-gssapi/blob/main/README.rst
- 4: Provide a "for dummies" guide to MutualAuthenticationError pythongssapi/requests-gssapi#15
- 5: pythongssapi/requests-gssapi@v1.3.0...v1.4.0
- 6: https://github.com/pythongssapi/requests-gssapi/
Improper Certificate Validation (CWE-295): Improper Certificate Validation
Reachability: Internal · Exploitability: Moderate
Reachability path
● Entry
plugins/microshift-release/scripts/unit_tests/test_post_release.py:409
test_full_format
│
▼
● Hop
plugins/microshift-release/scripts/errata_promotion.py:131
check_rpms_present: MicroShift RPMs are attached to the advisory.
│
▼
● Sink
plugins/microshift-release/scripts/lib/errata.py
Keep TLS certificate verification enabled for the Kerberos session.
HTTPSPNEGOAuth() does not provide server authentication by default. With _session.verify = False, the client has no server identity check before sending SPNEGO credentials or accepting Errata Tool data. Configure the trusted internal CA bundle through an environment variable such as ET_CA_BUNDLE instead. Remove the warning suppression.
🤖 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 `@plugins/microshift-release/scripts/lib/errata.py` around lines 25 - 28,
Update the session setup around HTTPSPNEGOAuth so TLS certificate verification
remains enabled by removing _session.verify = False and the urllib3 warning
suppression. Configure _session.verify from the ET_CA_BUNDLE environment
variable, using the existing trusted CA configuration mechanism if available,
before the Kerberos session sends requests.
Source: Coding guidelines
| fields = item.get("jira_issue", item) | ||
| bug = {} | ||
| bug["key"] = fields.get("key") or fields.get("id_jira") or "" | ||
| status = fields.get("status") | ||
| if isinstance(status, dict): | ||
| bug["status"] = status.get("name", "unknown") | ||
| elif isinstance(status, str): | ||
| bug["status"] = status | ||
| else: | ||
| bug["status"] = "unknown" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Read status from both the wrapper and the inner issue dict.
Line 273 selects fields as the inner jira_issue dict when that key exists. Lines 276-282 then read status only from fields. If the ET response places status on the wrapper and the issue fields under jira_issue, bug["status"] becomes "unknown". check_bugs_verified in errata_promotion.py treats "unknown" as not accepted, so the promotion check fails with no real cause.
The test at unit_tests/test_errata_promotion.py lines 420-425 already uses exactly this shape. It passes only because it asserts the key and not the status.
🐛 Proposed fix for the status fallback
fields = item.get("jira_issue", item)
bug = {}
bug["key"] = fields.get("key") or fields.get("id_jira") or ""
- status = fields.get("status")
+ status = fields.get("status") or item.get("status")
if isinstance(status, dict):Add an assertion on bugs[0]["status"] to the test_jira_issue_key test so the fallback is covered.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fields = item.get("jira_issue", item) | |
| bug = {} | |
| bug["key"] = fields.get("key") or fields.get("id_jira") or "" | |
| status = fields.get("status") | |
| if isinstance(status, dict): | |
| bug["status"] = status.get("name", "unknown") | |
| elif isinstance(status, str): | |
| bug["status"] = status | |
| else: | |
| bug["status"] = "unknown" | |
| fields = item.get("jira_issue", item) | |
| bug = {} | |
| bug["key"] = fields.get("key") or fields.get("id_jira") or "" | |
| status = fields.get("status") or item.get("status") | |
| if isinstance(status, dict): | |
| bug["status"] = status.get("name", "unknown") | |
| elif isinstance(status, str): | |
| bug["status"] = status | |
| else: | |
| bug["status"] = "unknown" |
🤖 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 `@plugins/microshift-release/scripts/lib/errata.py` around lines 273 - 282,
Update the bug parsing logic around the fields selection and status extraction
to read status from the outer item when the inner jira_issue fields do not
provide it, while preserving the existing dict, string, and unknown handling.
Extend test_jira_issue_key to assert the expected bugs[0]["status"] value for
the wrapper/inner-issue response shape.
| if version_info["type"] in ("EC", "RC"): | ||
| print(f"ERROR: Post-release checks are only for GA/z-stream releases, " | ||
| f"not {version_info['type']}: {args.version}", | ||
| file=sys.stderr) | ||
| sys.exit(1) | ||
|
|
||
| if version_info["type"] == "nightly": | ||
| print(f"ERROR: Post-release checks are not applicable to nightly builds: " | ||
| f"{args.version}", file=sys.stderr) | ||
| sys.exit(1) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add rejection-path tests before merge.
The PR objective states that EC/RC rejection verification remains pending. Add tests for EC, RC, and nightly inputs that assert exit status 1 and the expected error output. Add a GA or z-stream control case that mocks run_post_release_checks.
Per CONTRIBUTING.md: “Validation/parsing logic requires positive and negative tests.”
🤖 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 `@plugins/microshift-release/scripts/post_release.py` around lines 524 - 533,
Add rejection-path tests covering EC, RC, and nightly versions, asserting exit
status 1 and the corresponding stderr messages from the post-release validation.
Include a GA or z-stream control test that mocks run_post_release_checks and
verifies it is invoked successfully.
Source: Path instructions
There was a problem hiding this comment.
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 `@plugins/microshift-release/scripts/post_release.py`:
- Line 425: Wrap the futures assignment containing
ex.submit(check_rpms_customer_portal, advisory, errata_info, vpn_ok) across
multiple lines to comply with the project’s PEP 8 line-length requirement,
without changing its behavior.
🪄 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 YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: e9214fc0-b475-4b3f-939c-671da7667acc
📒 Files selected for processing (2)
plugins/microshift-release/scripts/post_release.pyplugins/microshift-release/scripts/unit_tests/test_post_release.py
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/microshift-release/scripts/unit_tests/test_post_release.py
USHIFT-6807: Automate post-release checks that confirm all MicroShift artifacts and documentation are publicly available after shipping. The skill runs 9 checks across 5 areas: Errata — advisory exists (Hydra) and is SHIPPED_LIVE (ET API) Bootc Images — rhel9/rhel10 images in prod catalog (amd64 + arm64) RPMs — all expected packages in advisory, CDN push complete Documentation — version string found in release notes page Lifecycle — version listed with Full Support (X.Y.0 only) The advisory is auto-discovered via the public Hydra search API. VPN-gated checks degrade to WARN. EC/RC/nightly versions are rejected. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> pre-commit.check-secrets: ENABLED
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
|
/hold Blocked: Do not merge until #256 is merged — this PR depends on lib/errata.py introduced there. |
openshift-ci
Bot
added
the
do-not-merge/hold
1 participant
Caution
Blocked: Do not merge until #256 is merged — this PR depends on
lib/errata.pyintroduced there.What
Adds
microshift-release:post-releaseskill (Phase 4) — the final phase of MicroShift release automation. Confirms all artifacts and documentation are publicly available after a GA or z-stream release has shipped.Jira: USHIFT-6807
Why
After shipping, the release manager manually checks 5+ different websites (Red Hat Catalog, Customer Portal, CDN, docs.redhat.com, lifecycle page) to confirm everything landed correctly. This automates all those checks into a single command, catching missing artifacts before customers report them.
How it works
The skill auto-discovers the advisory via the public Hydra search API — no advisory ID argument needed. It then runs 9 checks across 5 areas:
pr_errata_foundpr_errata_shippedpr_bootc_catalog_el9microshift-bootc-rhel9image in prod catalog (amd64 and arm64)pr_bootc_catalog_el10microshift-bootc-rhel10image in prod catalog (4.22.2+ only, both arches)pr_rpms_customer_portalmicroshift.specpresent in advisorypr_rpms_cdnpr_docs_publishedpr_lifecycle_listedpr_lifecycle_activeEC, RC, and nightly versions are rejected — this skill is only for GA and z-stream releases.
VPN-gated checks (
pr_errata_shipped,pr_rpms_cdn, RPM package list) degrade gracefully to WARN when VPN is unavailable. All other checks work over the public internet.Usage
Files
scripts/post_release.pyscripts/post_release.shscripts/unit_tests/test_post_release.pyscripts/lib/errata.pyfetch_push_status()for CDN push verificationskills/post-release/SKILL.mdREADME.mdTested
🤖 Generated with Claude Code