Fix: Triage Y-stream CVEs when Z-stream clones are NOT_AFFECTED - #792
Fix: Triage Y-stream CVEs when Z-stream clones are NOT_AFFECTED#792majamassarini wants to merge 13 commits into
Conversation
PR Summary by QodoTriage Y-stream CVEs when Z-stream clones are not affected
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
Code Review by Qodo
1.
|
|
/agentic_review |
|
Code review by qodo was updated up to the latest commit 7cef65d |
|
/agentic_review |
|
Code review by qodo was updated up to the latest commit 8cd0470 |
432c327 to
094880c
Compare
|
/agentic_review |
|
Code review by qodo was updated up to the latest commit 094880c |
094880c to
b594774
Compare
|
/agentic_review |
|
Code review by qodo was updated up to the latest commit e1d1866 |
Fixes PACKIT-5281: Y-stream CVEs were incorrectly skipped or postponed when Z-stream clones were not affected. The CVE eligibility check would say "fix is handled via Z-stream CentOS path" or "waiting for Z-stream to ship" without checking if the Z-streams were actually triaged as NOT_AFFECTED. This caused maintainers to manually close Y-stream issues that should have been automatically triaged and marked as not affected. Changes: - Add _check_zstream_not_affected(): searches for Z-stream clones with ymir_triaged_not_affected label - Add _check_zstream_pending_triage(): searches for Z-stream clones without any terminal ymir_triaged* labels - Modify _check_lowmod_ystream_eligibility(): for Low/Moderate Y-stream CVEs with CS_FIRST approach detected: * First check if Z-stream was NOT_AFFECTED → return IMMEDIATELY (triage Y-stream) * Then check if Z-stream pending triage → return PENDING_DEPENDENCIES (wait) * Otherwise → return NEVER (existing behavior - skip Y-stream) - Modify _check_for_dependency_blocker(): for Important/Critical Y-stream CVEs: * Check if Z-stream was NOT_AFFECTED before postponing * If yes → return None (proceed with triage, same as if clone had shipped) Example scenarios fixed: - RHEL-214038 (rhel-9.9, Moderate): was told "CentOS Stream path", now will be triaged when Z-stream is not affected - RHEL-224798, RHEL-224847 (rhel-10.3/9.9, Important): were postponed waiting for Z-stream, now will be triaged when Z-stream is not affected Assisted-by: Claude Sonnet 4.5 <noreply@anthropic.com>
…ing check The _check_zstream_pending_triage() function was missing terminal labels for clarification-needed and exhausted-error states, causing Y-stream CVEs to wait indefinitely for Z-stream clones that are actually blocked or completed in a terminal error state. Changes: - Add exclusion for ymir_needs_attention (CLARIFICATION_NEEDED resolution) - Add exclusion for ymir_triage_errored (exhausted triage retries) - Update comment to accurately describe which labels are terminal vs non-terminal This ensures that Z-stream clones in blocked or terminal error states are not incorrectly reported as "pending triage", which would cause Y-stream issues to be postponed indefinitely. Assisted-by: Claude Sonnet 4.5 <noreply@anthropic.com>
The Z-stream status check functions were combining current and upcoming streams into one set, allowing an older current-stream clone to override the applicable upcoming clone's triage result. Changes: - Add _get_applicable_zstream_variants() helper that implements version mapper precedence: upcoming Z-stream first, falls back to current - Refactor _check_zstream_not_affected() to use the helper, eliminating duplicated logic - Refactor _check_zstream_pending_triage() to use the helper, eliminating duplicated logic - Add early returns when major version is in maintenance or no applicable Z-stream exists This ensures Y-stream CVEs check the correct/latest Z-stream clone for triage status, preventing incorrect eligibility decisions based on older Z-stream versions. Example: For a rhel-9.9 Y-stream CVE, if both rhel-9.8.z (current) and rhel-9.9.z (upcoming) Z-stream clones exist, only rhel-9.9.z results are considered, matching the version mapper behavior. Assisted-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Failures in NOT_AFFECTED and pending-triage lookups were silently treated as negative results (empty lists), causing permanent eligibility skips instead of retryable operational errors. The triage workflow uses the `error` field in CVEEligibilityResult to select the retry path. Without it, transient Jira API failures become terminal NEVER decisions that permanently skip Y-stream CVEs. Changes: - In _check_lowmod_ystream_eligibility(): * NOT_AFFECTED check failure → return NEVER with error field (retryable) * Pending triage check failure → return NEVER with error field (retryable) - In _check_for_dependency_blocker(): * NOT_AFFECTED check failure → return NEVER with error field (retryable) This matches the error handling pattern from _check_zstream_fix_approach(), ensuring that operational failures trigger retries rather than permanent skips. Example: If Jira API is temporarily unavailable when checking Z-stream NOT_AFFECTED status, the task will be retried rather than permanently marking the Y-stream as ineligible. Assisted-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Adds comprehensive unit tests for the new Z-stream status check functions introduced in PACKIT-5281. Test coverage: - _get_applicable_zstream_variants(): * Upcoming Z-stream takes precedence over current * Falls back to current when no upcoming exists * Returns None for non-existent or maintenance versions - _check_zstream_not_affected(): * Finds Z-stream clones with ymir_triaged_not_affected label * Filters by applicable Z-stream version (upcoming > current) * Ignores old current Z-stream when upcoming exists * Returns empty list when no applicable clones found - _check_zstream_pending_triage(): * Finds Z-stream clones without terminal labels * Excludes clones with terminal labels (handled by JQL) * Filters by applicable Z-stream version * Ignores old current Z-stream when upcoming exists * Returns empty list when no applicable clones found Test patterns follow existing conventions: - Uses flexmock for mocking external dependencies - Mocks SearchJiraIssuesTool.run() and load_rhel_config() - Uses RHEL_CONFIG fixture matching production structure - Tests both positive and edge cases Assisted-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Fixes test failures caused by incorrect expectations and missing mocks
for the new Z-stream status check functions.
Changes to NEW tests (test_jira_zstream_status.py):
- test_get_applicable_zstream_variants_upcoming_wins: Expect both Y and Z
forms ({"rhel-9.7", "rhel-9.7.z"}) from get_fix_version_variants
- test_get_applicable_zstream_variants_current_fallback: Use version 10
instead of 8 (version 8 is maintenance, has Z but no Y-stream)
- test_get_applicable_zstream_variants_maintenance: Test version 8 which
correctly returns None (maintenance = Z-stream but no Y-stream)
- test_check_zstream_not_affected_no_applicable_zstream: Mock SearchJira
to verify it's NOT called (function returns early)
- test_check_zstream_pending_triage_no_applicable_zstream: Mock SearchJira
to verify it's NOT called (function returns early)
Changes to EXISTING tests (test_jira.py):
- test_eligibility_ystream_clones_pending: Mock _check_zstream_not_affected
to return [] (no NOT_AFFECTED clones found)
- test_eligibility_ystream_low_moderate_cs_first: Mock both new functions
to return [] (no NOT_AFFECTED, no pending clones)
These mocks ensure existing tests continue to test their original behavior
without being affected by the new Z-stream status checks.
Assisted-by: Claude Sonnet 4.5 <noreply@anthropic.com>
…age eligibility checks Add comprehensive integration tests for the complete CVE triage eligibility flow when Z-stream clones are NOT_AFFECTED or pending triage: CS_FIRST path (Low/Moderate severity): - Z-stream NOT_AFFECTED → IMMEDIATELY (Y-stream should be triaged) - Z-stream pending triage → PENDING_DEPENDENCIES (wait for Z-stream results) - NOT_AFFECTED check fails → NEVER with error (retryable) - Pending triage check fails → NEVER with error (retryable) Dependency blocker path (Important/Critical severity): - Z-stream NOT_AFFECTED → IMMEDIATELY (proceed despite pending clones) - NOT_AFFECTED check fails → NEVER with error (retryable) These tests verify the full eligibility check flow (CheckCveTriageEligibilityTool) rather than just the helper functions, ensuring the integration works correctly. Related: PACKIT-5281 Assisted-by: Claude Sonnet 4.5 (1M context) <noreply@anthropic.com>
…fallback The test claimed to verify fallback to current_z_streams when no upcoming exists, but used major version 10 which has an upcoming Z-stream configured, so it was actually testing the upcoming path, not the fallback. Added major version 11 to test config with: - Current Y-stream (rhel-11.2) to avoid maintenance classification - Current Z-stream (rhel-11.1.z) for the fallback case - NO upcoming Z-stream entry Updated test to use version 11 and assert current Z-stream variants are returned, properly exercising the fallback code path. Related: PACKIT-5281 Assisted-by: Claude Sonnet 4.5 (1M context) <noreply@anthropic.com>
…eturn The NOT_AFFECTED and pending-triage check functions were calling Jira search even when no applicable Z-stream exists, wasting API calls. Moved the `_get_applicable_zstream_variants` check to happen BEFORE the Jira search, enabling early return when major version has no Z-stream configured. Also fixed integration test mocks to pass correct number of arguments to `_check_zstream_clones_shipped` (3 args: cve_id, component, exclude_key). Related: PACKIT-5281 Assisted-by: Claude Sonnet 4.5 (1M context) <noreply@anthropic.com>
…NDENCIES When _check_zstream_fix_approach returns PENDING (open Z-stream clone without Fixed in Build), the code was immediately returning PENDING_DEPENDENCIES without checking if the clone was actually triaged as NOT_AFFECTED. A NOT_AFFECTED clone has: - Status: Open (not closed) - Label: ymir_triaged_not_affected - Fixed in Build: Empty (no build) This makes it look "pending" to _check_zstream_fix_approach, but it's actually complete. The Y-stream should proceed to triage (IMMEDIATELY) rather than wait. Changes: - Combined PENDING and CS_FIRST handling to check NOT_AFFECTED first - After NOT_AFFECTED and pending-triage checks, differentiate final behavior: - PENDING: return PENDING_DEPENDENCIES (waiting for Fixed in Build) - CS_FIRST: return NEVER (CS-first applies, skip Y-stream) - Added regression test for PENDING + NOT_AFFECTED case Related: PACKIT-5281 Assisted-by: Claude Sonnet 4.5 (1M context) <noreply@anthropic.com>
…PENDING test The test_eligibility_ystream_low_moderate_pending test was failing because it didn't mock the new _check_zstream_not_affected and _check_zstream_pending_triage functions that are now called in the PENDING path. Without these mocks, the functions tried to call load_rhel_config multiple times, exceeding the mock's .once() expectation and causing the test to fail. Added mocks for both functions returning empty lists (no NOT_AFFECTED clones, no pending-triage clones) so the test properly exercises the PENDING path and returns PENDING_DEPENDENCIES as expected. Related: PACKIT-5281 Assisted-by: Claude Sonnet 4.5 (1M context) <noreply@anthropic.com>
Added comprehensive unit test coverage for edge cases: NOT_AFFECTED function tests: - Clone exists but is affected (no ymir_triaged_not_affected label) - Jira API search failure (connection timeout) Pending-triage function tests: - Clone exists but has terminal label (completed as affected) - Jira API search failure (server unavailable) These tests complete the unit test coverage for Z-stream NOT_AFFECTED and pending-triage behavior, covering all outcomes: NOT_AFFECTED, pending, affected, and Jira-query failure cases. Total unit tests: 18 (was 15, added 3) - _get_applicable_zstream_variants: 4 tests - _check_zstream_not_affected: 8 tests (was 6) - _check_zstream_pending_triage: 6 tests (was 5) Related: PACKIT-5281 Assisted-by: Claude Sonnet 4.5 (1M context) <noreply@anthropic.com>
834473b to
a580a44
Compare
|
/agentic_review |
|
Code review by qodo was updated up to the latest commit a580a44 |
|
/agentic_review |
|
Code review by qodo was updated up to the latest commit ea5afea |
…nation Modular trackers can share CVE ID, component, and fix version while representing different module streams. Without filtering, a NOT_AFFECTED or pending status in one module stream (e.g., postgresql:16) could incorrectly affect eligibility for another module stream (e.g., postgresql:15). Changes: - Updated _check_zstream_not_affected and _check_zstream_pending_triage to accept summary parameter - Parse module stream from current issue using parse_module_stream - Request summary field from Jira search results - Filter clones to only match when: - Both are modular with the exact same (module, stream) tuple, OR - Both are non-modular (None module stream) - Updated all 3 call sites to pass summary parameter - Updated all existing tests to pass summary parameter - Added 5 regression tests for modular tracker scenarios: - NOT_AFFECTED: modular match, modular mismatch, modular vs non-modular - Pending triage: modular match, modular mismatch Example: postgresql:15/postgis and postgresql:16/postgis both in component postgis now correctly tracked separately - NOT_AFFECTED in :16 doesn't affect :15 eligibility. Related: PACKIT-5281 Assisted-by: Claude Sonnet 4.5 (1M context) <noreply@anthropic.com>
ea5afea to
8bdab2e
Compare
Summary
Fixes PACKIT-5281: Y-stream CVEs were incorrectly skipped or postponed when Z-stream clones were NOT_AFFECTED.
Problem
When a Y-stream CVE (e.g., rhel-9.9) is checked for eligibility:
Both messages were incorrect when the CVE was actually NOT AFFECTED in the component. The Y-stream should have been triaged to confirm it's also not affected, instead of being skipped or postponed.
Impact: Maintainers had to manually close Y-stream issues that should have been automatically triaged.
Solution
Check Z-stream triage status before skipping or postponing Y-stream CVEs.
Changes
Added
_check_zstream_not_affected()ymir_triaged_not_affectedlabelAdded
_check_zstream_pending_triage()ymir_triaged*labelsModified
_check_lowmod_ystream_eligibility()IMMEDIATELY(triage Y-stream)PENDING_DEPENDENCIES(wait for results)NEVER(skip Y-stream - existing behavior)Modified
_check_for_dependency_blocker()None(proceed with triage)PENDING_DEPENDENCIES(postpone - existing behavior)Related
🤖 Generated with Claude Code