ROX-33816: query active deployments in VM 1.0#20428
ROX-33816: query active deployments in VM 1.0#20428stehessel wants to merge 1 commit intoROX-33816/soft-deletes-ui-vm-2.0from
Conversation
|
Skipping CI for Draft Pull Request. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
🚀 Build Images ReadyImages are ready for commit 214b13b. To use with deploy scripts: export MAIN_IMAGE_TAG=4.11.x-930-g214b13bb15 |
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughThis PR introduces feature-flag-driven deployment soft deletion support across the Vulnerability Management (VulnMgmt) system. The ChangesDeployment Soft Deletion Feature Flag Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
ui/apps/platform/src/Containers/VulnMgmt/Entity/TableWidgetFixableCves.jsx (1)
68-94:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
$deploymentQuerydeclared unconditionally but unused forNODE_CVEandCLUSTER_CVE— runtime GraphQL validation failure.
$deploymentQuery: Stringis always emitted in the operation signature (line 76), but onlyIMAGE_CVE_LIST_FRAGMENTreferencesdeploymentCount(query: $deploymentQuery). WhenvulnTypeisCLUSTER_CVEorNODE_CVE, neither the fragment nor the operation body uses$deploymentQuery, making it an unused variable. All variables defined by an operation must be used in that operation or a fragment transitively included by that operation — unused variables cause a validation error. The existingunusedVarSinkinCLUSTER_CVE_DETAIL_FRAGMENTconfirms the server enforces this rule, so these two widget cases will fail at runtime.Apply the same conditional-inclusion pattern already used for
$query(lines 71–75):🐛 Proposed fix — guard `$deploymentQuery` like `$query`
- $deploymentQuery: String + ${ + vulnType !== entityTypes.CLUSTER_CVE && vulnType !== entityTypes.NODE_CVE + ? '$deploymentQuery: String' + : '' + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/apps/platform/src/Containers/VulnMgmt/Entity/TableWidgetFixableCves.jsx` around lines 68 - 94, The GraphQL operation string in fixableCvesQuery declares $deploymentQuery unconditionally which causes unused-variable validation failures for vulnType === NODE_CVE or CLUSTER_CVE; update the operation signature to include $deploymentQuery only when it is needed (mirror the existing conditional used for $query): emit '$deploymentQuery: String' only when vulnType is not NODE_CVE and not CLUSTER_CVE (so IMAGE_CVE cases that reference deploymentCount can use it), leaving the rest of the query and the referenced queryFragment/IMAGE_CVE_LIST_FRAGMENT unchanged.ui/apps/platform/src/Containers/VulnMgmt/Entity/Component/VulnMgmtEntityImageComponent.jsx (1)
33-52:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate
deploymentCountin the overview query to use$deploymentQueryfor consistency with the sibling Image entity.The overview query defines parameters
($id: ID!, $query: String, $scopeQuery: String)but does not include$deploymentQuery, yet the code constructs and passesdeploymentQuery: withActiveDeploymentQuery('', isDeploymentSoftDeletionEnabled)in queryOptions (line 80). The sibling fileVulnMgmtEntityImage.jsxcorrectly includes$deploymentQueryin its overview query parameters and uses it fordeploymentCount(query: $deploymentQuery)on line 42. To apply active-deployment filtering consistently whenROX_DEPLOYMENT_SOFT_DELETIONis enabled, the overview query should match this pattern.♻️ Suggested change
const overviewQuery = gql` - query getImageComponent($id: ID!, $query: String, $scopeQuery: String) { + query getImageComponent($id: ID!, $query: String, $deploymentQuery: String, $scopeQuery: String) { result: imageComponent(id: $id) { id name version fixedIn location(query: $scopeQuery) priority - deploymentCount(query: $query) + deploymentCount(query: $deploymentQuery) imageVulnerabilityCount(query: $query, scopeQuery: $scopeQuery) imageCount(query: $query)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/apps/platform/src/Containers/VulnMgmt/Entity/Component/VulnMgmtEntityImageComponent.jsx` around lines 33 - 52, The overviewQuery for imageComponent is missing the $deploymentQuery variable and still calls deploymentCount(query: $query); update the GraphQL operation signature to include ($deploymentQuery: String) and change the deploymentCount field to use deploymentCount(query: $deploymentQuery) so the call that passes deploymentQuery: withActiveDeploymentQuery('', isDeploymentSoftDeletionEnabled) is honored; adjust any variable usage in the same component (overviewQuery) accordingly to match the sibling VulnMgmtEntityImage.jsx pattern.
🧹 Nitpick comments (2)
ui/apps/platform/src/Containers/VulnMgmt/List/Components/VulnMgmtListComponents.jsx (1)
200-214: ⚡ Quick winMake
$deploymentQueryusage explicit in this operation.Right now this query depends on
...componentFieldsbeing the only place that consumes$deploymentQuery. That makes the operation brittle: a fragment edit can turn this into a runtime GraphQL validation failure without any change in this file. The other list queries in this PR avoid that by sinking the variable locally.Suggested change
const query = gql` query getComponents($query: String, $deploymentQuery: String, $pagination: Pagination) { results: components(query: $query, pagination: $pagination) { ...componentFields + unusedVarSink(query: $deploymentQuery) } count: componentCount(query: $query) } ${VULN_COMPONENT_LIST_FRAGMENT} `;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/apps/platform/src/Containers/VulnMgmt/List/Components/VulnMgmtListComponents.jsx` around lines 200 - 214, The GraphQL operation getComponents declares $deploymentQuery but only uses it indirectly inside the VULN_COMPONENT_LIST_FRAGMENT, which is brittle; make $deploymentQuery usage explicit by adding it to the components field arguments (the components(...) call inside the getComponents query) so the operation directly consumes $deploymentQuery, and ensure the queryOptions.variables still passes deploymentQuery via withActiveDeploymentQuery; update the getComponents query (and its components(...) argument list) and keep references to VULN_COMPONENT_LIST_FRAGMENT, queryOptions.variables, withActiveDeploymentQuery, and the query constant named query.ui/apps/platform/src/Containers/VulnMgmt/Entity/VulnMgmtPolicyQueryUtil.js (1)
11-25: ⚡ Quick winAdd focused coverage for the deployment filter helper.
This function is now the switch point for whether VM deployment lists exclude soft-deleted rows. A small flag-on/flag-off test here would catch regressions before they skew results across every page wired up in this PR.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/apps/platform/src/Containers/VulnMgmt/Entity/VulnMgmtPolicyQueryUtil.js` around lines 11 - 25, Add a focused unit test for tryUpdateQueryWithVulMgmtPolicyClause that verifies the deployment soft-deletion filter behavior: call tryUpdateQueryWithVulMgmtPolicyClause with entityType === entityTypes.DEPLOYMENT and isDeploymentSoftDeletionEnabled true and assert the returned query was passed through withActiveDeploymentQuery (i.e., includes the active-deployment constraint), then call it again with isDeploymentSoftDeletionEnabled false and assert the returned query equals the raw query (no active filter). Also add a complementary case for a non-DEPLOYMENT entityType to confirm the function returns the normal queryService.objectToWhereClause result (and for POLICY ensure the Category: 'Vulnerability Management' clause is present). Use the existing helper names tryUpdateQueryWithVulMgmtPolicyClause, withActiveDeploymentQuery, entityTypes, and queryService in the test to locate and exercise the logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ui/apps/platform/src/Containers/VulnMgmt/widgets/TopRiskiestEntities.jsx`:
- Around line 347-351: The withActiveDeploymentQuery deployment-state filter is
being applied to all queries but this widget never queries DEPLOYMENT; remove
the deployment-filtering call by replacing the wrapped expression so that query
is set directly from queryService.objectToWhereClause(queryObject) (i.e., stop
calling withActiveDeploymentQuery), and also remove the now-unused
isDeploymentSoftDeletionEnabled variable/import if it becomes unused; target
symbols: withActiveDeploymentQuery, isDeploymentSoftDeletionEnabled, query, and
queryService.objectToWhereClause.
---
Outside diff comments:
In
`@ui/apps/platform/src/Containers/VulnMgmt/Entity/Component/VulnMgmtEntityImageComponent.jsx`:
- Around line 33-52: The overviewQuery for imageComponent is missing the
$deploymentQuery variable and still calls deploymentCount(query: $query); update
the GraphQL operation signature to include ($deploymentQuery: String) and change
the deploymentCount field to use deploymentCount(query: $deploymentQuery) so the
call that passes deploymentQuery: withActiveDeploymentQuery('',
isDeploymentSoftDeletionEnabled) is honored; adjust any variable usage in the
same component (overviewQuery) accordingly to match the sibling
VulnMgmtEntityImage.jsx pattern.
In `@ui/apps/platform/src/Containers/VulnMgmt/Entity/TableWidgetFixableCves.jsx`:
- Around line 68-94: The GraphQL operation string in fixableCvesQuery declares
$deploymentQuery unconditionally which causes unused-variable validation
failures for vulnType === NODE_CVE or CLUSTER_CVE; update the operation
signature to include $deploymentQuery only when it is needed (mirror the
existing conditional used for $query): emit '$deploymentQuery: String' only when
vulnType is not NODE_CVE and not CLUSTER_CVE (so IMAGE_CVE cases that reference
deploymentCount can use it), leaving the rest of the query and the referenced
queryFragment/IMAGE_CVE_LIST_FRAGMENT unchanged.
---
Nitpick comments:
In `@ui/apps/platform/src/Containers/VulnMgmt/Entity/VulnMgmtPolicyQueryUtil.js`:
- Around line 11-25: Add a focused unit test for
tryUpdateQueryWithVulMgmtPolicyClause that verifies the deployment soft-deletion
filter behavior: call tryUpdateQueryWithVulMgmtPolicyClause with entityType ===
entityTypes.DEPLOYMENT and isDeploymentSoftDeletionEnabled true and assert the
returned query was passed through withActiveDeploymentQuery (i.e., includes the
active-deployment constraint), then call it again with
isDeploymentSoftDeletionEnabled false and assert the returned query equals the
raw query (no active filter). Also add a complementary case for a non-DEPLOYMENT
entityType to confirm the function returns the normal
queryService.objectToWhereClause result (and for POLICY ensure the Category:
'Vulnerability Management' clause is present). Use the existing helper names
tryUpdateQueryWithVulMgmtPolicyClause, withActiveDeploymentQuery, entityTypes,
and queryService in the test to locate and exercise the logic.
In
`@ui/apps/platform/src/Containers/VulnMgmt/List/Components/VulnMgmtListComponents.jsx`:
- Around line 200-214: The GraphQL operation getComponents declares
$deploymentQuery but only uses it indirectly inside the
VULN_COMPONENT_LIST_FRAGMENT, which is brittle; make $deploymentQuery usage
explicit by adding it to the components field arguments (the components(...)
call inside the getComponents query) so the operation directly consumes
$deploymentQuery, and ensure the queryOptions.variables still passes
deploymentQuery via withActiveDeploymentQuery; update the getComponents query
(and its components(...) argument list) and keep references to
VULN_COMPONENT_LIST_FRAGMENT, queryOptions.variables, withActiveDeploymentQuery,
and the query constant named query.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: eb477577-e0f6-4c84-8e81-32595515c42c
📒 Files selected for processing (20)
ui/apps/platform/src/Containers/VulnMgmt/Entity/Cluster/VulnMgmtEntityCluster.jsxui/apps/platform/src/Containers/VulnMgmt/Entity/Component/VulnMgmtEntityImageComponent.jsxui/apps/platform/src/Containers/VulnMgmt/Entity/Component/VulnMgmtEntityNodeComponent.jsxui/apps/platform/src/Containers/VulnMgmt/Entity/Cve/VulnMgmtEntityCve.jsxui/apps/platform/src/Containers/VulnMgmt/Entity/Deployment/VulnMgmtEntityDeployment.jsxui/apps/platform/src/Containers/VulnMgmt/Entity/Image/VulnMgmtEntityImage.jsxui/apps/platform/src/Containers/VulnMgmt/Entity/Namespace/VulnMgmtEntityNamespace.jsxui/apps/platform/src/Containers/VulnMgmt/Entity/Node/VulnMgmtEntityNode.jsxui/apps/platform/src/Containers/VulnMgmt/Entity/TableWidgetFixableCves.jsxui/apps/platform/src/Containers/VulnMgmt/Entity/VulnMgmtPolicyQueryUtil.jsui/apps/platform/src/Containers/VulnMgmt/List/Clusters/VulnMgmtListClusters.jsxui/apps/platform/src/Containers/VulnMgmt/List/Components/VulnMgmtListComponents.jsxui/apps/platform/src/Containers/VulnMgmt/List/Cves/VulnMgmtListCves.jsxui/apps/platform/src/Containers/VulnMgmt/List/Deployments/VulnMgmtListDeployments.jsxui/apps/platform/src/Containers/VulnMgmt/List/ImageComponents/VulnMgmtListImageComponents.jsxui/apps/platform/src/Containers/VulnMgmt/List/Images/VulnMgmtListImages.jsxui/apps/platform/src/Containers/VulnMgmt/List/Namespaces/VulnMgmtListNamespaces.jsxui/apps/platform/src/Containers/VulnMgmt/VulnMgmt.fragments.jsui/apps/platform/src/Containers/VulnMgmt/widgets/TopRiskiestEntities.jsxui/apps/platform/src/Containers/VulnMgmt/widgets/TopRiskyEntitiesByVulnerabilities.jsx
| const isDeploymentSoftDeletionEnabled = isFeatureFlagEnabled('ROX_DEPLOYMENT_SOFT_DELETION'); | ||
| const query = withActiveDeploymentQuery( | ||
| queryService.objectToWhereClause(queryObject), | ||
| isDeploymentSoftDeletionEnabled | ||
| ); // Get final gql query string, filtering out deleted deployments when enabled. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect withActiveDeploymentQuery to understand what it appends
# when enabled, so we can determine whether it is safe to apply to non-deployment queries.
rg -n "withActiveDeploymentQuery" --type js -A 10 -B 2
fd "deploymentUtils" --type f --exec cat {}Repository: stackrox/stackrox
Length of output: 50373
🏁 Script executed:
fd "deploymentUtils" --type fRepository: stackrox/stackrox
Length of output: 177
🏁 Script executed:
cat -n ui/apps/platform/src/utils/deploymentUtils.tsRepository: stackrox/stackrox
Length of output: 1538
🏁 Script executed:
sed -n '1,100p' ui/apps/platform/src/Containers/VulnMgmt/widgets/TopRiskiestEntities.jsx | head -50Repository: stackrox/stackrox
Length of output: 1979
🏁 Script executed:
sed -n '320,370p' ui/apps/platform/src/Containers/VulnMgmt/widgets/TopRiskiestEntities.jsxRepository: stackrox/stackrox
Length of output: 1669
🏁 Script executed:
sed -n '1,330p' ui/apps/platform/src/Containers/VulnMgmt/widgets/TopRiskiestEntities.jsx | grep -A 50 "getQueryBySelectedEntityVulns\|getEntitiesByContext"Repository: stackrox/stackrox
Length of output: 2166
🏁 Script executed:
sed -n '260,280p' ui/apps/platform/src/Containers/VulnMgmt/widgets/TopRiskyEntitiesByVulnerabilities.jsxRepository: stackrox/stackrox
Length of output: 786
Gate the deployment soft-deletion filter to deployment queries only.
The withActiveDeploymentQuery call applies a deployment-specific state filter (Deployment State:DEPLOYMENT_STATE_ACTIVE) to all queries from this widget. However, selectedEntity is constrained to IMAGE, NODE, IMAGE_COMPONENT, or NODE_COMPONENT — never DEPLOYMENT. This filter should not be applied to images(), nodes(), or component queries as it will either corrupt the query syntax or cause backend errors.
Since this widget never queries deployments, either:
- Remove the
withActiveDeploymentQuerycall entirely, or - Gate it to match
TopRiskyEntitiesByVulnerabilities.jsx(line 270):isDeploymentSoftDeletionEnabled && selectedEntityType === entityTypes.DEPLOYMENT(though this condition would always be false here and effectively disables the filter).
The correct approach is to remove the deployment filter application entirely from this widget.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ui/apps/platform/src/Containers/VulnMgmt/widgets/TopRiskiestEntities.jsx`
around lines 347 - 351, The withActiveDeploymentQuery deployment-state filter is
being applied to all queries but this widget never queries DEPLOYMENT; remove
the deployment-filtering call by replacing the wrapped expression so that query
is set directly from queryService.objectToWhereClause(queryObject) (i.e., stop
calling withActiveDeploymentQuery), and also remove the now-unused
isDeploymentSoftDeletionEnabled variable/import if it becomes unused; target
symbols: withActiveDeploymentQuery, isDeploymentSoftDeletionEnabled, query, and
queryService.objectToWhereClause.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## ROX-33816/soft-deletes-ui-vm-2.0 #20428 +/- ##
=================================================================
Coverage 49.63% 49.63%
=================================================================
Files 2802 2802
Lines 213115 213115
=================================================================
+ Hits 105769 105774 +5
+ Misses 99595 99592 -3
+ Partials 7751 7749 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1 participant
Description
change me!
User-facing documentation
Testing and quality
Automated testing
How I validated my change
change me!