Skip to content

fix: treat Done-Errata CVEs as already shipped in precheck#196

Open
agullon wants to merge 2 commits into
openshift-eng:mainfrom
agullon:fix/done-errata-cve-skip
Open

fix: treat Done-Errata CVEs as already shipped in precheck#196
agullon wants to merge 2 commits into
openshift-eng:mainfrom
agullon:fix/done-errata-cve-skip

Conversation

@agullon

@agullon agullon commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Bug: interpret_cves() in precheck_xyz.py treated Done-Errata resolution the same as Done, adding it to must_release_cves. This caused the precheck to recommend "ASK ART TO CREATE ARTIFACTS" for CVEs whose fix had already been shipped in a prior errata.
  • Fix: Move Done-Errata to the skip branch alongside Not a Bug, since both mean no further MicroShift release action is needed for that CVE.
  • Tests: Updated test_cve_must_releasetest_cve_done_errata_skipped to assert the corrected "none" impact.

Context

The Jira tickets checked by this code are MicroShift-specific (the advisory publication report returns per-CVE Jira tickets scoped to MicroShift — CVEs without a MicroShift ticket are already skipped with continue). This means Done-Errata on these tickets indicates MicroShift itself already shipped the fix, not just that some other OCP component did. If these were broader OCP CVE tickets, Done-Errata could mean a different component shipped while MicroShift still needs to — but that's not the case here, confirming this is a bug.

Test plan

  • Unit tests pass (python3 -m unittest unit_tests.test_logic -v)
  • Verify with a real advisory report containing Done-Errata CVEs to confirm they no longer trigger must_release

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes

  • Updated CVE resolution handling so mandatory release is triggered only when the Jira resolution is “Done”.
  • “Done-Errata” and “Not a Bug” are now treated as no-action, rather than requiring a mandatory release.
  • Standardized the release reason text for “Done”-driven impacts to a single fixed message.

Tests

  • Updated unit test coverage to verify “Done-Errata” no longer produces mandatory release impact.

Done-Errata resolution means the fix was already included in a
shipped errata, so it should be skipped like "Not a Bug" rather
than triggering a must_release recommendation.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

pre-commit.check-secrets: ENABLED
@openshift-ci

openshift-ci Bot commented Jun 18, 2026

Copy link
Copy Markdown

[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

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 18, 2026
@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 8cfcb46e-273e-4fde-a478-d3f5f146c5be

📥 Commits

Reviewing files that changed from the base of the PR and between 2fa4dba and 7beb977.

📒 Files selected for processing (1)
  • plugins/microshift-release/scripts/precheck_xyz.py

Walkthrough

interpret_cves in precheck_xyz.py is updated so that only Jira resolution "Done" triggers must_release impact (with reason "Fix completed"), while "Done-Errata" and "Not a Bug" now produce no-action. The corresponding unit test is renamed and its expected impact changed from "must_release" to "none".

Changes

CVE Resolution Logic Update

Layer / File(s) Summary
Resolution branching and unit test
plugins/microshift-release/scripts/precheck_xyz.py, plugins/microshift-release/scripts/unit_tests/test_logic.py
Resolution comment block designates "Done" as must-release and "Done-Errata"/"Not a Bug" as no-action. The branching logic removes the combined "Done-Errata"/"Done" must-release path, adds a sole "Done" branch with fixed reason "Fix completed", and routes "Done-Errata" to the no-action path. Unit test test_cve_must_release is replaced by test_cve_done_errata_skipped, asserting result["impact"] == "none" for a "Done-Errata" fixture.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • openshift-eng/edge-tooling#48: compute_recommendation uses must_release impact produced by interpret_cves, making this reclassification directly relevant to that recommendation precedence logic.
  • openshift-eng/edge-tooling#149: Also modifies interpret_cves in the same file to change how "Done"/"Done-Errata"/"Not a Bug" map to impact, and adjusts TestInterpretCves unit tests in the same test file.
🚥 Pre-merge checks | ✅ 9 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Ai-Attribution ⚠️ Warning PR mentions AI tool usage ("Generated with Claude Code"), but commit uses improper "Co-Authored-By" trailer for AI instead of required "Assisted-by" or "Generated-by" trailer. Replace commit's "Co-Authored-By: Claude Opus 4.6..." trailer with "Assisted-by: Claude Opus 4.6..." or "Generated-by: Claude Opus 4.6..." trailer.
✅ Passed checks (9 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: treating Done-Errata CVEs as already shipped (no further release action needed) in the precheck function, which directly addresses the bug fix described in the PR objectives.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
No-Weak-Crypto ✅ Passed No weak cryptography (MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB), custom crypto implementations, or non-constant-time secret comparisons detected. PR addresses CVE resolution logic, not cryptography.
Container-Privileges ✅ Passed PR contains only Python source code (precheck_xyz.py and test_logic.py); no container or K8s manifests present to evaluate for privilege escalation issues.
No-Sensitive-Data-In-Logs ✅ Passed The PR modifies CVE resolution handling in interpret_cves() and its tests. Data logged includes only CVE IDs, Jira ticket IDs, and descriptive reason strings - no passwords, tokens, API keys, PII,...
No-Hardcoded-Secrets ✅ Passed No hardcoded secrets, API keys, tokens, passwords, or credentials found in the PR changes. All string values are test data, documentation, or legitimate status identifiers.
No-Injection-Vectors ✅ Passed No injection vectors detected. Code uses safe subprocess patterns (list-based args), no eval/exec/pickle/yaml.load, no os.system, no SQL concatenation, and f-strings use only trusted internal JSON...

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

Distinguish Done-Errata (already shipped) from Not a Bug (doesn't
affect MicroShift) in the docstring. Update the Done reason to make
the required action explicit.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

pre-commit.check-secrets: ENABLED
@agullon

agullon commented Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

/label tide/merge-method-squash

@openshift-ci openshift-ci Bot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Jun 18, 2026
@openshift-ci openshift-ci Bot added the ready-for-human-review Indicates a PR has been reviewed by automated tools and is ready for human review label Jun 18, 2026
@agullon

agullon commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

/retest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. ready-for-human-review Indicates a PR has been reviewed by automated tools and is ready for human review tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant