Update IOP tests to support multi RHEL versions#21482
Open
ColeHiggins2 wants to merge 4 commits into
Open
Conversation
ColeHiggins2
added
CherryPick
Contributor
Reviewer's GuideGeneralize IoP and vulnerability UI/CLI tests to support RHEL 8, 9, and 10 by parameterizing vulnerable package data, loosening RHEL version markers, and updating content sync and remediation logic to be version-aware. Sequence diagram for version-aware IoP vulnerability CLI testsequenceDiagram
participant TestRunner
participant IopCliTest
participant Constants as RobotteloConstants
participant RhelHost
participant InsightsService
TestRunner->>IopCliTest: run_test(rhel_version)
IopCliTest->>Constants: get VULNERABLE_PACKAGES[rhel_version]
Constants-->>IopCliTest: rpm, cves, erratum
IopCliTest->>RhelHost: ensure_content_synced(rhel_version)
RhelHost-->>IopCliTest: content_available
IopCliTest->>RhelHost: install_vulnerable_rpm(rpm)
RhelHost-->>IopCliTest: installation_status
IopCliTest->>InsightsService: upload_insights_archive(rhel_version)
InsightsService-->>IopCliTest: analysis_result(cves_detected)
IopCliTest->>IopCliTest: assert cves_detected contains cves
IopCliTest->>InsightsService: trigger_remediation(rhel_version)
InsightsService-->>RhelHost: apply_playbook
RhelHost-->>InsightsService: remediation_status
InsightsService-->>IopCliTest: remediation_complete
IopCliTest->>RhelHost: verify_package_state(rpm)
RhelHost-->>IopCliTest: package_state
IopCliTest->>TestRunner: report_result
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Accessing
constants.VULNERABLE_PACKAGES[rhel_ver]assumesos_version.majoris an int; if it is a string (e.g.'8'/'9'), this will raise aKeyError, so consider normalizing the type (e.g.int(os_version.major)) or using consistent string keys in the mapping. - For RHEL 8 and 9 in
VULNERABLE_PACKAGES,erratumis set to an empty string but later used in theapply_erratassearch expression, which will generate an invalid/empty search likeerrata_id ==; it would be safer to either provide real errata IDs or guard the remediation step when no erratum is defined.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Accessing `constants.VULNERABLE_PACKAGES[rhel_ver]` assumes `os_version.major` is an int; if it is a string (e.g. `'8'`/`'9'`), this will raise a `KeyError`, so consider normalizing the type (e.g. `int(os_version.major)`) or using consistent string keys in the mapping.
- For RHEL 8 and 9 in `VULNERABLE_PACKAGES`, `erratum` is set to an empty string but later used in the `apply_erratas` search expression, which will generate an invalid/empty search like `errata_id == `; it would be safer to either provide real errata IDs or guard the remediation step when no erratum is defined.
## Individual Comments
### Comment 1
<location path="tests/foreman/ui/test_rhcloud_insights_vulnerability.py" line_range="256" />
<code_context>
- # Use the first 2 CVEs from the mariadb package for deterministic bulk edit testing
- cve_ids = constants.RHEL10_VULNERABLE_MARIADB_CVES[:2]
+ cve_ids = vuln_data['cves'][:2]
# Bulk edit business risk
</code_context>
<issue_to_address>
**suggestion (testing):** Bulk-edit test may only exercise a single CVE on RHEL versions with fewer CVEs
On RHEL 10, `vuln_data['cves'][:2]` correctly yields two CVEs, but on RHEL 8/9 the list currently has only one element, so this behaves like a single-CVE edit despite the test docstring describing a multi-CVE bulk edit.
To keep the test semantics accurate and ensure we truly exercise bulk behavior on all supported RHEL versions, please either (1) ensure `VULNERABLE_PACKAGES` for RHEL 8/9 includes at least two CVEs, (2) skip/xfail when fewer than two CVEs are available, or (3) parametrize so that bulk assertions only run when `len(cves) >= 2`. Otherwise, the test may overstate our coverage of bulk operations on RHEL 8/9.
Suggested implementation:
```python
vuln_data = constants.VULNERABLE_PACKAGES[rhel_ver]
# Ensure we actually exercise bulk behavior; skip when fewer than two CVEs are available
cves = vuln_data['cves']
if len(cves) < 2:
pytest.skip("Bulk edit requires at least two CVEs in VULNERABLE_PACKAGES for this RHEL version")
with satellite.ui_session() as session:
session.organization.select(org_name=rhcloud_manifest_org.name)
cve_ids = cves[:2]
# Bulk edit business risk
```
This change assumes `pytest` is already imported in `tests/foreman/ui/test_rhcloud_insights_vulnerability.py`.
If it is not, add `import pytest` near the top of the file alongside the other imports.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| # Use the first 2 CVEs from the mariadb package for deterministic bulk edit testing | ||
| cve_ids = constants.RHEL10_VULNERABLE_MARIADB_CVES[:2] | ||
| cve_ids = vuln_data['cves'][:2] |
Contributor
There was a problem hiding this comment.
suggestion (testing): Bulk-edit test may only exercise a single CVE on RHEL versions with fewer CVEs
On RHEL 10, vuln_data['cves'][:2] correctly yields two CVEs, but on RHEL 8/9 the list currently has only one element, so this behaves like a single-CVE edit despite the test docstring describing a multi-CVE bulk edit.
To keep the test semantics accurate and ensure we truly exercise bulk behavior on all supported RHEL versions, please either (1) ensure VULNERABLE_PACKAGES for RHEL 8/9 includes at least two CVEs, (2) skip/xfail when fewer than two CVEs are available, or (3) parametrize so that bulk assertions only run when len(cves) >= 2. Otherwise, the test may overstate our coverage of bulk operations on RHEL 8/9.
Suggested implementation:
vuln_data = constants.VULNERABLE_PACKAGES[rhel_ver]
# Ensure we actually exercise bulk behavior; skip when fewer than two CVEs are available
cves = vuln_data['cves']
if len(cves) < 2:
pytest.skip("Bulk edit requires at least two CVEs in VULNERABLE_PACKAGES for this RHEL version")
with satellite.ui_session() as session:
session.organization.select(org_name=rhcloud_manifest_org.name)
cve_ids = cves[:2]
# Bulk edit business riskThis change assumes pytest is already imported in tests/foreman/ui/test_rhcloud_insights_vulnerability.py.
If it is not, add import pytest near the top of the file alongside the other imports.
Member
Author
|
trigger: test-robottelo |
Collaborator
|
PRT Result |
Satellite-QE
added
the
PRT-Failed
Contributor
LadislavVasina1
left a comment
LadislavVasina1
left a comment
There was a problem hiding this comment.
Changes looks valid to me, I just don't understand why it failing in PRT
Member
Author
|
trigger: test-robottelo |
Collaborator
|
PRT Result |
ColeHiggins2
force-pushed
the
update-iop-to-support-multi-rhel
branch
from
49ad307 to
3858720
Compare
Member
Author
|
trigger: test-robottelo |
Collaborator
|
PRT Result |
Member
Author
|
trigger: test-robottelo |
nofaralfasi
reviewed
May 20, 2026
| @pytest.mark.rhel_ver_match('10') | ||
| @pytest.mark.parametrize('module_target_sat_insights', [False], ids=['local'], indirect=True) | ||
| @pytest.mark.rhel_ver_match(r'^(?!7).*') | ||
| @pytest.mark.parametrize('module_target_sat_insights', [True], ids=['local'], indirect=True) |
Contributor
There was a problem hiding this comment.
This test is in the IoP suite and the docstring says “Set up Satellite with iop enabled,” but [True] here selects hosted Insights behavior from the fixture.
Was this intentional? If this test is meant to validate IoP flow, this should be [False]. Otherwise the test name/docstring should be updated to reflect hosted coverage.
ColeHiggins2
force-pushed
the
update-iop-to-support-multi-rhel
branch
from
3858720 to
01d2ca7
Compare
Member
Author
|
trigger: test-robottelo |
Collaborator
|
PRT Result |
Member
Author
|
trigger: test-robottelo |
Member
Author
|
Member
Author
|
trigger: test-robottelo |
Collaborator
|
PRT Result |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SAT-44051
Update IOP tests to support RHEL 8 9 and 10.
Added in Vulnerability packages for RHEL 8 and 9.
Updated tests.
Summary by Sourcery
Generalize RH Cloud IoP vulnerability and recommendations tests to support multiple RHEL major versions instead of only RHEL 10.
New Features:
Enhancements: