USHIFT-6806: AI Skill: Advisory Promotion — Errata Tool (Phase 3) - #256
USHIFT-6806: AI Skill: Advisory Promotion — Errata Tool (Phase 3)#256agullon wants to merge 5 commits into
Conversation
|
[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 |
|
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:
WalkthroughChangesErrata promotion validation
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning)
✅ Passed checks (9 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/microshift-release/skills/advisory-promotion/SKILL.md (1)
70-74: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse
$PLUGIN_DIRfor portable script paths.
SCRIPTS_DIR=plugins/microshift-release/scriptsdepends on the current working directory. The skill can fail when the agent starts outside the repository root.Set
SCRIPTS_DIR="$PLUGIN_DIR/scripts"and quote the path in each command.As per path instructions, SKILL.md must use portable paths such as
$PLUGIN_DIR.Also applies to: 90-100
🤖 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/advisory-promotion/SKILL.md` around lines 70 - 74, Update the SCRIPTS_DIR definition in SKILL.md to use the portable "$PLUGIN_DIR/scripts" path, then replace the affected script command paths in the referenced sections with the quoted SCRIPTS_DIR variable so they work from any current working directory.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.py`:
- Around line 162-177: Update the product-version validation loop to collect the
MicroShift NVR keys listed in each entry, then compare that set against nvrs
rather than passing when any listing exists. Return FAIL with the missing NVRs
when the difference is non-empty; otherwise retain the existing PASS result with
the sorted product versions. Add a test covering a partial product-version
listing.
- Around line 226-245: Update the external-test evaluation logic in the relevant
check function to require records for both CAT and rpmdiff and accept only their
explicit successful result states; treat missing, pending, unknown, skipped,
unrelated, or failed results as incomplete and return the appropriate WARN or
FAIL outcome instead of PASS. Remove the status-based PASS path for empty tests,
and update test_no_tests_rel_prep to assert the new behavior.
- Around line 411-418: Validate the positional advisory argument in the
argument-parsing flow before it is used to construct an API URL, accepting only
anchored numeric IDs or supported advisory-name formats and rejecting path
separators, query delimiters, control characters, and arbitrary text. Add
positive and negative tests covering each accepted format and representative
invalid inputs.
- Around line 183-204: Update check_cdn_staging to call the existing
errata.fetch_cdn_repos API and base PASS/FAIL on the returned CDN repository
state rather than advisory status or push_count. Require all expected staging
repositories to report a successful push, preserving the existing
unavailable-advisory and text-only skip behavior; apply the same validation to
the related logic in the later indicated section.
- Around line 294-306: Preserve the None result from fetch_jira_issues through
the orchestration flow by avoiding extract_bug_keys when jira_data is None,
while retaining normal bug-key extraction for successful Jira responses. Ensure
check_bugs_verified receives None and returns its WARN outcome rather than
treating the advisory as having no bugs, and add an orchestration test covering
a None Jira response.
- Around line 436-453: Update the output flow around the json_output branch so
JSON formatting is selected and printed without returning early. Move the common
any(r["status"] == "FAIL" for r in results) exit check after all output
branches, preserving verbose and short text formatting while ensuring failed
JSON checks exit with status 1.
In `@plugins/microshift-release/scripts/lib/errata.py`:
- Line 154: Update the loop over builds_data to iterate directly over its values
instead of unpacking the unused product_version key, while preserving the
existing build_entries processing and ensuring Ruff B007 passes.
- Line 15: Enable TLS certificate verification in the session configuration by
removing the _session.verify = False behavior and the corresponding
urllib3.disable_warnings call. Configure verification to use the trusted system
store or the Red Hat CA bundle, preserving secure HTTPS connections without
suppressing certificate warnings.
In `@plugins/microshift-release/skills/advisory-promotion/SKILL.md`:
- Line 3: Update the advisory-promotion command contract to support the --json
option everywhere: add [--json] to the argument-hint and include --json
pass-through in both execution templates, while preserving the existing version,
--prod, --verbose, and --errata options.
---
Outside diff comments:
In `@plugins/microshift-release/skills/advisory-promotion/SKILL.md`:
- Around line 70-74: Update the SCRIPTS_DIR definition in SKILL.md to use the
portable "$PLUGIN_DIR/scripts" path, then replace the affected script command
paths in the referenced sections with the quoted SCRIPTS_DIR variable so they
work from any current working directory.
🪄 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 YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 2b3cdf59-0d8b-47b4-b692-1941c49c49d0
📒 Files selected for processing (5)
plugins/microshift-release/scripts/errata_promotion.pyplugins/microshift-release/scripts/errata_promotion.shplugins/microshift-release/scripts/lib/errata.pyplugins/microshift-release/scripts/unit_tests/test_errata_promotion.pyplugins/microshift-release/skills/advisory-promotion/SKILL.md
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 `@plugins/microshift-release/scripts/lib/errata.py`:
- Around line 75-108: Validate response shapes before mapping access: in
plugins/microshift-release/scripts/lib/errata.py:75-108, update _unwrap_advisory
to return None unless data, errata, and the selected advisory are dictionaries;
in plugins/microshift-release/scripts/lib/errata.py:166-212, return an empty
result for non-dictionary builds_data and treat non-dictionary variant_arch as
absent; in plugins/microshift-release/scripts/lib/errata.py:250-265, skip items
whose resolved jira_issue is not a dictionary. Add positive and negative tests
covering list, null, and scalar response shapes for each affected parsing path.
- Around line 255-266: The conditional check before bugs.append(bug) currently
accepts any non-empty key, but should only include Jira issues with keys
starting with the OCPBUGS prefix. Update the condition to verify that bug["key"]
starts with "OCPBUGS-" in addition to the existing truthiness check, ensuring
non-OCPBUGS linked issues are filtered out before being added to the bugs list.
🪄 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 YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: f02b09db-8da5-4a61-8290-734b32d22c1b
📒 Files selected for processing (3)
plugins/microshift-release/scripts/errata_promotion.pyplugins/microshift-release/scripts/lib/errata.pyplugins/microshift-release/scripts/unit_tests/test_errata_promotion.py
🚧 Files skipped from review as they are similar to previous changes (2)
- plugins/microshift-release/scripts/unit_tests/test_errata_promotion.py
- plugins/microshift-release/scripts/errata_promotion.py
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
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/lib/errata.py`:
- Around line 200-203: Update the nested loops in the variant_arch iteration to
use mapping values instead of binding the unused variant and arch keys, while
preserving the existing arches type check and rpm_list processing. Ensure the
Python passes Ruff B007.
In `@plugins/microshift-release/skills/advisory-promotion/SKILL.md`:
- Around line 171-183: Update the Errata Mode Checks documentation for
et_cat_tests and its associated example to reflect check_cat_tests behavior:
describe validation of the rhnqa, qa_complete, and advisory status fields rather
than counting CAT or rpmdiff records. Replace the unsupported output example
with an actual --errata result such as “RHN QA testing passed (rhnqa=1)”,
ensuring descriptions and expected output match the implementation.
- Around line 81-86: Update the argument-routing checks in advisory promotion to
detect when --prod appears alongside --errata and stop with a clear
unsupported-combination error before entering Errata mode. Preserve the existing
missing-advisory-ID validation and Bootc-mode behavior when --errata is absent.
- Around line 90-100: Update the command examples for advisory_promotion.sh and
errata_promotion.sh to quote the complete $SCRIPTS_DIR-based script paths, while
preserving the existing arguments and options.
🪄 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: 2e6e3163-88e2-44eb-806f-1a22a0749afe
📒 Files selected for processing (7)
.claude-plugin/marketplace.jsonplugins/microshift-release/.claude-plugin/plugin.jsonplugins/microshift-release/scripts/errata_promotion.pyplugins/microshift-release/scripts/errata_promotion.shplugins/microshift-release/scripts/lib/errata.pyplugins/microshift-release/scripts/unit_tests/test_errata_promotion.pyplugins/microshift-release/skills/advisory-promotion/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (4)
- .claude-plugin/marketplace.json
- plugins/microshift-release/.claude-plugin/plugin.json
- plugins/microshift-release/scripts/unit_tests/test_errata_promotion.py
- plugins/microshift-release/scripts/errata_promotion.sh
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
|
Addressed CodeRabbit findings: Applied in 7e29f09:
Deferred:
Dropped (summary pre-merge checks):
|
- 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
|
/label tide/merge-method-squash |
What
Adds Errata Tool RPM advisory validation to the
advisory-promotionskill (Phase 3). The skill now checks both Konflux bootc advisories (existing) and Errata Tool RPM advisories (new) when invoked.Jira: USHIFT-6806
Why
QE sign-off on RPM advisories requires manually verifying advisory state in the Errata Tool — correct type, QA ownership, bugs verified, RPMs present, CDN push done, etc. This automates those checks into a single command, reducing the release manager's manual work.
How it works
The skill calls the Errata Tool REST API (Kerberos/GSSAPI auth) and runs 9 checks:
et_advisory_existset_advisory_typeet_qa_owneret_bugs_verifiedet_rpms_presentmicroshift.specfound in ET buildset_rpms_product_listedet_cdn_staginget_cat_testsrhnqafield)et_status_rel_prepThe expected RPM list comes from the spec file on
release-{minor}. The found RPMs come from the ET builds response (variant_archdata). The advisory ID is provided via--errataor auto-discovered fromocp-build-data/releases.yml.Usage
Files
lib/errata.pyrequests_gssapierrata_promotion.pyerrata_promotion.shtest_errata_promotion.pySKILL.md--erratamode,--prodalways on, verbatim output ruleTested
🤖 Generated with Claude Code