ROX-33207: Fix ImageV2 SearchListImages query#19084
ROX-33207: Fix ImageV2 SearchListImages query#19084ajheflin wants to merge 2 commits intomasterstackrox/stackrox:masterfrom ajheflin/fix-exclude-images-requeststackrox/stackrox:ajheflin/fix-exclude-images-requestCopy head branch name to clipboard
Conversation
|
Skipping CI for Draft Pull Request. |
|
Images are ready for the commit at 1321ad1. To use with deploy scripts, first |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #19084 +/- ##
==========================================
+ Coverage 49.49% 49.52% +0.02%
==========================================
Files 2668 2671 +3
Lines 201435 201582 +147
==========================================
+ Hits 99706 99839 +133
- Misses 94281 94288 +7
- Partials 7448 7455 +7
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:
|
6d6eb41 to
d3da3d5
Compare
There was a problem hiding this comment.
You probably don't need an aggregate and group by here. image_v2.proto already has a regular field for Component Count. The search framework is not using that and trying to join with components table because Component Count is also a derived field in search options . Just rename the field in image_v2.proto to Image Component Count and add a regular search option for that and the query should use the cached field just like it does for CVECount and FixableCVECount.
stackrox/proto/storage/image_v2.proto
Line 25 in adc404b
stackrox/pkg/search/options.go
Line 358 in adc404b
1f5289c to
1321ad1
Compare
|
@ajheflin: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
dashrews78
left a comment
There was a problem hiding this comment.
LGTM, assuming you ran the datastore test with the image feature flag on.
Description
Fixes the ImageV2 SearchListImages query to not use a derived field and instead count a the component IDs using an aggregate function, and then adding the appropriate fields to the group by to make Postgres happy.
User-facing documentation
Testing and quality
Automated testing
How I validated my change
Ran
TestSearchListImagesfor ImageV2 datastore withROX_FLATTEN_IMAGE_DATAon and verified that the test passes.