feat: add workload filter support (RHINENG-27093)#2417
Open
adarshdubey-star wants to merge 3 commits into
Open
feat: add workload filter support (RHINENG-27093)#2417adarshdubey-star wants to merge 3 commits into
adarshdubey-star wants to merge 3 commits into
Conversation
Reviewer's GuideAdds workload filter support across the Vulnerability API by introducing new workload columns, ingesting them from inventory events, and unifying all workload-related filtering into a single OR-based workload filter used consistently by systems, CVEs, dashboard, vulnerabilities, and dashbar endpoints. Sequence diagram for unified workload filtering in Vulnerability APIsequenceDiagram
actor Client
participant SystemHandler
participant Filters as apply_filters
participant WorkloadFilter as _filter_system_by_workloads
participant DB as SystemPlatform
Client->>SystemHandler: GET /systems?sap_system&ansible&crowdstrike
SystemHandler->>Filters: apply_filters(query, args, [SYSTEM_WORKLOADS])
Filters->>WorkloadFilter: _filter_system_by_workloads(query, args)
WorkloadFilter->>WorkloadFilter: build exprs from WORKLOAD_PARAMS
WorkloadFilter->>DB: query.where(reduce(or_, exprs))
DB-->>SystemHandler: matching systems
SystemHandler-->>Client: filtered systems response
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}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The new OR-based
_filter_system_by_workloadschanges semantics for combinations oftrueandfalseworkload flags (e.g.ansible=false&mssql=falsenow matches~ansible OR ~mssqlinstead of~ansible AND ~mssql); consider confirming this matches intended API behavior or restricting OR logic totrueselections only. - The repeated
fn.COALESCE(field, False)patterns inWORKLOAD_PARAMScould be centralized into a small helper to reduce duplication and make future workload additions less error-prone.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new OR-based `_filter_system_by_workloads` changes semantics for combinations of `true` and `false` workload flags (e.g. `ansible=false&mssql=false` now matches `~ansible OR ~mssql` instead of `~ansible AND ~mssql`); consider confirming this matches intended API behavior or restricting OR logic to `true` selections only.
- The repeated `fn.COALESCE(field, False)` patterns in `WORKLOAD_PARAMS` could be centralized into a small helper to reduce duplication and make future workload additions less error-prone.
## Individual Comments
### Comment 1
<location path="manager/filters.py" line_range="450-456" />
<code_context>
- query = query.where(expr)
+ exprs = []
+ for param, expr_fns in WORKLOAD_PARAMS.items():
+ if param in args and args[param] is not None:
+ if args[param]:
+ exprs.append(expr_fns["positive"]())
+ else:
+ exprs.append(expr_fns["negative"]())
+ if exprs:
+ query = query.where(reduce(or_, exprs))
return query
</code_context>
<issue_to_address>
**issue (bug_risk):** Using OR semantics for negative workload flags (`False` values) leads to broader-than-expected result sets compared to the previous implementation.
Previously, negative workload flags were combined with AND semantics: e.g. `ansible=False` and `mssql=False` yielded `~ansible AND ~mssql` (systems that were neither ansible- nor mssql-managed). With the new `reduce(or_, exprs)` approach, these become `~ansible OR ~mssql`, which also matches systems that are only ansible-managed or only mssql-managed. This changes the exclusion behavior and may surprise callers. If OR is desired for positive flags, consider handling negative flags differently (e.g., combine them with AND or restrict to a single negative flag) to avoid overly broad matches.
</issue_to_address>
### Comment 2
<location path="tests/manager_tests/test_system_handler.py" line_range="627-625" />
<code_context>
+ expected_ids = set(ANSIBLE_SYSTEMS) | set(MSSQL_SYSTEMS)
+ assert returned_ids == expected_ids
+
+ def test_system_workload_or_logic_new_types(self):
+ """Test OR logic across new workload types"""
+ response = self.vfetch("systems?crowdstrike=true&ibm_db2=true").check_response()
+ returned_ids = {s.attributes.inventory_id for s in response.body.data}
+ expected_ids = set(CROWDSTRIKE_SYSTEMS) | set(IBM_DB2_SYSTEMS)
+ assert returned_ids == expected_ids
+
def test_system_cves_known_exploit(self):
</code_context>
<issue_to_address>
**suggestion (testing):** Add tests for mixed true/false workload flags to validate correct OR behavior when some flags are false
Current tests only cover cases where all workload flags are `true`. Please add scenarios with mixed `true`/`false` flags (e.g. `systems?crowdstrike=true&ibm_db2=false`, `systems?ansible=false&mssql=true`) to verify that only active (`true`) filters participate in the OR and that `false` flags are enforced as exclusion conditions rather than incorrectly included in the OR expression.
Suggested implementation:
```python
for system in response.body.data:
assert system.attributes.inventory_id not in RHEL_AI_SYSTEMS
def test_system_workload_or_logic(self):
"""Test that multiple workload filters use OR logic"""
response = self.vfetch("systems?ansible=true&mssql=true").check_response()
returned_ids = {s.attributes.inventory_id for s in response.body.data}
expected_ids = set(ANSIBLE_SYSTEMS) | set(MSSQL_SYSTEMS)
assert returned_ids == expected_ids
def test_system_workload_or_logic_new_types(self):
"""Test OR logic across new workload types"""
response = self.vfetch("systems?crowdstrike=true&ibm_db2=true").check_response()
returned_ids = {s.attributes.inventory_id for s in response.body.data}
expected_ids = set(CROWDSTRIKE_SYSTEMS) | set(IBM_DB2_SYSTEMS)
assert returned_ids == expected_ids
def test_system_workload_or_logic_mixed_flags(self):
"""Mixed true/false flags: only true workloads participate in OR, false workloads are excluded"""
# ansible=false&mssql=true => only MSSQL systems returned, no Ansible systems
response = self.vfetch("systems?ansible=false&mssql=true").check_response()
returned_ids = {s.attributes.inventory_id for s in response.body.data}
expected_ids = set(MSSQL_SYSTEMS)
excluded_ids = set(ANSIBLE_SYSTEMS)
assert returned_ids == expected_ids
assert returned_ids.isdisjoint(excluded_ids)
def test_system_workload_or_logic_mixed_flags_new_types(self):
"""Mixed true/false flags across new workload types"""
# crowdstrike=true&ibm_db2=false => only CrowdStrike systems returned, no IBM DB2 systems
response = self.vfetch("systems?crowdstrike=true&ibm_db2=false").check_response()
returned_ids = {s.attributes.inventory_id for s in response.body.data}
expected_ids = set(CROWDSTRIKE_SYSTEMS)
excluded_ids = set(IBM_DB2_SYSTEMS)
assert returned_ids == expected_ids
assert returned_ids.isdisjoint(excluded_ids)
def test_system_cves_known_exploit(self):
def count_known_exploits(data):
cnt = 0
```
These tests assume that `ANSIBLE_SYSTEMS`, `MSSQL_SYSTEMS`, `CROWDSTRIKE_SYSTEMS`, and `IBM_DB2_SYSTEMS` are already defined in this test module and that the API correctly interprets `workload=false` as an exclusion filter. If the underlying handler supports additional workload flags, you may want to mirror these patterns to cover those as well. Also ensure that the test class name and any setup fixtures (e.g. `vfetch`, system populations) are consistent with the rest of the file so the new tests run in the same context as the existing workload tests.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
adarshdubey-star
force-pushed
the
workload_filter
branch
from
July 14, 2026 12:19
b601047 to
61deeef
Compare
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
1 participant
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.
JIra: https://redhat.atlassian.net/browse/RHINENG-27093
Summary
What changed
Why
Summary by Sourcery
Add support for filtering vulnerability-related system data by multiple workload types using unified OR-based logic across handlers and APIs.
New Features:
Enhancements:
Build:
Tests: