fix: follow-up enhancements for AlmaLinux available_fix#5600
fix: follow-up enhancements for AlmaLinux available_fix#5600VanitasCodes wants to merge 1 commit intoossf:mainfrom
Conversation
fe6af89 to
24b78e6
Compare
There was a problem hiding this comment.
Pull request overview
This PR improves the AlmaLinux available_fix reporting by collapsing duplicate fixed-package entries that differ only by architecture, and adds an online integration test to validate the AlmaLinux errata API integration over time.
Changes:
- Deduplicate AlmaLinux advisory packages by
(name, version, release)during errata processing to avoid repeated output lines per architecture. - Add an external (online) AlmaLinux available-fix integration test using a known CVE.
- Fix minor repo hygiene items (newline at EOF for spelling expectations; replace legacy logger formatting).
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
test/test_available_fix.py |
Adds a long/online AlmaLinux integration test and corresponding mock CVE input data. |
cve_bin_tool/available_fix/alma_cve_tracker.py |
Deduplicates fixed package entries across architectures and modernizes a warning log format. |
.github/actions/spelling/expect.txt |
Ensures the file ends with a newline (spelling action expectation file). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| expected_output = [ | ||
| "xz: CVE-2022-1271 - Status: Fixed - Fixed package: xz v5.2.5-8.el9_0 (Advisory: ALSA-2022:4940; AlmaLinux 9)", | ||
| ] | ||
|
|
||
| assert expected_output == [rec.message for rec in caplog.records] | ||
|
|
There was a problem hiding this comment.
This online integration test asserts an exact fixed package version/release/advisory string. Since AlmaLinux errata can be updated over time (e.g., a newer advisory or release for the same CVE), the logic could remain correct but this test would fail. Consider asserting more stable invariants instead (e.g., that at least one log line reports Status: Fixed for CVE-2022-1271 on AlmaLinux 9, and optionally that the advisory prefix matches ALSA-2022:4940), rather than pinning the full package NEVRA fragment.
| expected_output = [ | |
| "xz: CVE-2022-1271 - Status: Fixed - Fixed package: xz v5.2.5-8.el9_0 (Advisory: ALSA-2022:4940; AlmaLinux 9)", | |
| ] | |
| assert expected_output == [rec.message for rec in caplog.records] | |
| messages = [rec.message for rec in caplog.records] | |
| # Assert that at least one log line reports this CVE as fixed on AlmaLinux 9 | |
| assert any( | |
| ("xz:" in msg) | |
| and ("CVE-2022-1271" in msg) | |
| and ("Status: Fixed" in msg) | |
| and ("AlmaLinux 9" in msg) | |
| for msg in messages | |
| ) | |
| # Optionally, assert that the advisory identifier has the expected prefix, | |
| # without pinning the entire NEVRA or full advisory string. | |
| alma_msgs = [msg for msg in messages if "CVE-2022-1271" in msg] | |
| assert alma_msgs | |
| for msg in alma_msgs: | |
| if "Advisory:" in msg: | |
| advisory_part = msg.split("Advisory:", 1)[1].split(";", 1)[0].strip() | |
| assert advisory_part.startswith("ALSA-2022:4940") | |
| break | |
| else: | |
| pytest.fail("No advisory information found for CVE-2022-1271 in AlmaLinux output") |
There was a problem hiding this comment.
Following the established pattern used by test_long_redhat_available_fix_output and test_long_debian_available_fix_output which also use exact string matching.
| fixes = AvailableFixReport(self.MOCK_XZ_CVE_DATA, "alma-9", False) | ||
| fixes.check_available_fix() |
There was a problem hiding this comment.
The new deduplication logic in Alma errata processing (collapsing identical name/version/release across architectures) isn't exercised by the mocked AlmaLinux unit test data (MOCK_ALMA_API only has a single package entry). To prevent regressions, add/adjust an offline test case where the mocked advisory contains duplicate package entries (as would happen with multiple architectures) and assert that only one line is emitted.
There was a problem hiding this comment.
Right, the current mock data doesn't exercise the deduplication logic. @alex-ter, would you like me to update MOCK_ALMA_API to include multiple architecture entries so the offline test covers deduplication?
|
@alex-ter @ffontaine The online test is failing in CI due to |
You'll need to update the respective |
074c79f to
e7a4660
Compare
Signed-off-by: Vishwajeet Singh <[email protected]>
e7a4660 to
8fa80e5
Compare
|
@alex-ter Ready for review. |
Fixes #5597
This PR addresses the follow-up items identified after merging the AlmaLinux available_fix feature.
The main change is deduplicating packages across architectures in the errata processing. Previously, when AlmaLinux published fixes for multiple architectures (x86_64, aarch64, s390x, etc.), each architecture variant appeared as a separate line in the output. Now packages differing only by architecture are collapsed into a single entry, making the output cleaner and easier to read.
An online integration test has been added to verify the AlmaLinux errata API integration works correctly. The test uses a known CVE (CVE-2022-1271 in xz) that has been fixed in AlmaLinux 9, ensuring the logic doesn't get out of sync with the actual data provided by the server over time.
Two minor fixes are also included: adding the missing newline at the end of
spelling/expect.txtto resolve the spelling check warning, and converting a legacy%sstring format to an f-string for consistency with the rest of the codebase.All unit tests pass, and manual end-to-end testing with
curl-7.76.1-29.el9_4.1.x86_64.rpmconfirms the deduplication works as expected.