ROX-33654: Add entity scope and query field support to report service#19861
Open
ROX-33654: Add entity scope and query field support to report service#19861
Conversation
- Add bidirectional conversion between API and storage for entity scope resource scope type in report configurations and snapshots - Add query field passthrough in vulnerability report filters - Add validation for entity scope rules (unset entity/field, duplicate entity+field pairs, cluster annotation unsupported, label key=value format) - Add query field validation using search.ParseQuery - Store ResourceScope in report snapshot Partially generated with AI assistance.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #19861 +/- ##
==========================================
+ Coverage 49.60% 49.61% +0.01%
==========================================
Files 2763 2763
Lines 208339 208428 +89
==========================================
+ Hits 103346 103417 +71
- Misses 97327 97339 +12
- Partials 7666 7672 +6
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:
|
Contributor
🚀 Build Images ReadyImages are ready for commit 9af5987. To use with deploy scripts: export MAIN_IMAGE_TAG=4.11.x-566-g9af5987db7 |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe changes extend the central/reports service's vulnerability report handling to support entity-scoped resource configurations alongside collection-scoped ones. This includes bidirectional conversion between API v2 and storage representations, validation logic for entity scope rules, and Query field parsing for vulnerability report filters. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 (1)
central/reports/validation/validate.go (1)
351-356:⚠️ Potential issue | 🔴 CriticalGuard
collectionbefore populatingCollectionSnapshot.Line 353 dereferences
collectionunconditionally, but lines 284-295 now leavecollection == nilfor the new entity-scope path. The first valid entity-scoped report request will panic here before the snapshot is created, so the feature is effectively unusable.🐛 Proposed fix
snapshot := &storage.ReportSnapshot{ ReportConfigurationId: config.GetId(), Name: config.GetName(), Description: config.GetDescription(), Type: storage.ReportSnapshot_VULNERABILITY, - Collection: &storage.CollectionSnapshot{ - Id: config.GetResourceScope().GetCollectionId(), - Name: collection.GetName(), - }, ResourceScope: config.GetResourceScope(), Schedule: config.GetSchedule(), ReportStatus: &storage.ReportStatus{ RunState: storage.ReportStatus_WAITING, ReportRequestType: requestType, ReportNotificationMethod: notificationMethod, }, } + if collection != nil { + snapshot.Collection = &storage.CollectionSnapshot{ + Id: config.GetResourceScope().GetCollectionId(), + Name: collection.GetName(), + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@central/reports/validation/validate.go` around lines 351 - 356, The code unconditionally dereferences collection when building the CollectionSnapshot even though collection can be nil for entity-scoped requests; modify the snapshot construction in validate.go so you only populate Collection: &storage.CollectionSnapshot{...} when collection != nil (i.e., guard the access to collection.GetName()/GetId()), otherwise leave Collection nil (or omit it) to avoid a panic; update the block that sets Collection: to conditionally assign based on collection and keep ResourceScope and Schedule assignments unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@central/reports/service/v2/convert.go`:
- Around line 483-486: The current conversion calls
convertProtoResourceScopeToV2 on snapshot.GetResourceScope(), which performs
live datastore collection lookups and can fail or rewrite historical names;
change the conversion logic to first build ResourceScope.CollectionScope from
snapshot.GetCollection() (use snapshot.GetCollection() to populate
CollectionScope fields) and only call convertProtoResourceScopeToV2 as a
fallback when snapshot.GetCollection() is empty/missing; implement this in a
new/updated helper (e.g., convertProtoSnapshotResourceScopeToV2) that accepts
the snapshot, prefers snapshot.GetCollection() for CollectionScope, and
delegates to convertProtoResourceScopeToV2(snapshot.GetResourceScope()) only
when snapshot collection metadata is absent.
---
Outside diff comments:
In `@central/reports/validation/validate.go`:
- Around line 351-356: The code unconditionally dereferences collection when
building the CollectionSnapshot even though collection can be nil for
entity-scoped requests; modify the snapshot construction in validate.go so you
only populate Collection: &storage.CollectionSnapshot{...} when collection !=
nil (i.e., guard the access to collection.GetName()/GetId()), otherwise leave
Collection nil (or omit it) to avoid a panic; update the block that sets
Collection: to conditionally assign based on collection and keep ResourceScope
and Schedule assignments unchanged.
🪄 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: Pro
Run ID: e3908763-42bc-4423-9b33-71e46f51d3e9
📒 Files selected for processing (4)
central/reports/service/v2/convert.gocentral/reports/service/v2/convert_test.gocentral/reports/validation/validate.gocentral/reports/validation/validate_test.go
Comment on lines
+483
to
+486
| resourceScope, err := s.convertProtoResourceScopeToV2(snapshot.GetResourceScope()) | ||
| if err != nil { | ||
| return nil, err | ||
| } |
There was a problem hiding this comment.
Use snapshot metadata instead of live collection lookups here.
Lines 483-486 now route snapshot ResourceScope through convertProtoResourceScopeToV2, which resolves collection-scoped names from the current datastore. That means one deleted collection will make the whole snapshot conversion fail, and a renamed collection will also rewrite the historical name, even though snapshot.GetCollection() already stores the snapshot-time metadata. Please build ResourceScope.CollectionScope from the snapshot copy first and only fall back to a live lookup when that snapshot metadata is absent.
💡 Suggested direction
- resourceScope, err := s.convertProtoResourceScopeToV2(snapshot.GetResourceScope())
- if err != nil {
- return nil, err
- }
+ resourceScope, err := s.convertProtoSnapshotResourceScopeToV2(
+ snapshot.GetResourceScope(),
+ snapshot.GetCollection(),
+ )
+ if err != nil {
+ return nil, err
+ }convertProtoSnapshotResourceScopeToV2 should prefer snapshot.GetCollection() for collection-scoped snapshots so historical report reads stay stable after collection rename/deletion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@central/reports/service/v2/convert.go` around lines 483 - 486, The current
conversion calls convertProtoResourceScopeToV2 on snapshot.GetResourceScope(),
which performs live datastore collection lookups and can fail or rewrite
historical names; change the conversion logic to first build
ResourceScope.CollectionScope from snapshot.GetCollection() (use
snapshot.GetCollection() to populate CollectionScope fields) and only call
convertProtoResourceScopeToV2 as a fallback when snapshot.GetCollection() is
empty/missing; implement this in a new/updated helper (e.g.,
convertProtoSnapshotResourceScopeToV2) that accepts the snapshot, prefers
snapshot.GetCollection() for CollectionScope, and delegates to
convertProtoResourceScopeToV2(snapshot.GetResourceScope()) only when snapshot
collection metadata is absent.
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.
Description
Adds support for two new fields in the vulnerability report service (v2
API):
collection-based scoping that lets callers filter by
cluster/namespace/deployment entity rules directly, without needing a
pre-created collection.
string appended to CVE filter criteria.
Changes are confined to the service and validation layers:
convertV2ResourceScopeToProto / convertProtoResourceScopeToV2
entity+field pairs, cluster annotation unsupported, label key=value format,
query parseable via search.ParseQuery)
User-facing documentation
Testing and quality
Automated testing
How I validated my change
TestValidateReportFiltersQuery)
(TestConvertEntityScope)