feat(query): add resource selector search for config insights#2020
Merged
Conversation
Benchstat (RLS)Base: 📊 8 minor regression(s) (all within 5% threshold)
✅ 4 improvement(s)
Full benchstat output |
Benchstat (Other)Base: ✅ 2 improvement(s)
Full benchstat output |
Contributor
WalkthroughAdds config-analysis query support, maps it through resource selection, updates deleted-row handling and config-analysis model/view definitions, extends storage backfills, and adds fixtures plus selector tests. ChangesConfig analysis search support
Sequence Diagram(s)sequenceDiagram
participant SearchResources
participant FindConfigAnalysisByResourceSelector
participant FindConfigAnalysisIDsByResourceSelector
participant queryTableWithResourceSelectors
participant GetConfigAnalysisByIDs
participant database
SearchResources->>FindConfigAnalysisByResourceSelector: ConfigAnalysis selectors
FindConfigAnalysisByResourceSelector->>FindConfigAnalysisIDsByResourceSelector: resourceSelectors + limit
FindConfigAnalysisIDsByResourceSelector->>queryTableWithResourceSelectors: resolve IDs
queryTableWithResourceSelectors-->>FindConfigAnalysisIDsByResourceSelector: []uuid.UUID
FindConfigAnalysisByResourceSelector->>GetConfigAnalysisByIDs: ids
GetConfigAnalysisByIDs->>database: id IN ? query
database-->>GetConfigAnalysisByIDs: []models.ConfigAnalysis
GetConfigAnalysisByIDs-->>SearchResources: ConfigAnalysis results
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
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 |
Adds resource selector support for config_analysis (insights), matching how configs and config changes are already searchable. - New ConfigAnalysisQueryModel + registration in GetModelFromTable - FindConfigAnalysisByResourceSelector helpers - ConfigAnalysis wired into SearchResources request/response - HasDeletedAt flag on QueryModel so tables without a deleted_at column (config_analysis) skip the default soft-delete filter
adityathebe
force-pushed
the
feat/insights-resource-selector
branch
from
74acd2a to
54cd555
Compare
Gavel resultsGavel exited with code . |
ConfigAnalysisQueryModel enables HasProperties, which routes the unkeyed PEG form `properties=<value>` through the `properties_values` generated column. That column was only created for config_items and components, so the same search against config_analysis failed at runtime with 'column "properties_values" does not exist'. Add config_analysis to the generated-column migration in 002_seed.sql and exclude it from the atlas diff in apply.go, matching how config_items and components are handled. Also drop two redundant self-aliases (first_observed/last_observed) from ConfigAnalysisQueryModel.
Add a Severity *string field to SelectedResource, populated only for config insights. Severity is orthogonal to Health (healthy/unhealthy/ warning/unknown) and Status, so it gets its own field rather than overloading either. Other resource kinds leave it nil, which is omitted from the JSON response.
Contributor
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
Use config_analysis_items for resource selector searches so config insight queries can filter on parent config fields such as agent_id, deleted_at, tags, labels, and type. Extend the view with the parent config fields and cover config type filtering in resource selector tests.
Remove the conflicting config alias now that config analysis selectors expose the parent config JSON. Also stop treating analysis as a JSON selector field until it is explicitly supported. Add regression coverage for config_id filtering, unsupported analysis JSON lookup, and high/critical pod analysis selection.
Member
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/query_resource_selector_test.go`:
- Around line 795-800: Tighten the test in FindConfigAnalysisByResourceSelector
so it verifies the type filter is actually applied, not just that
dummy.LogisticsAPIPodAnalysis is returned. Update the assertion around the
"finds high or critical pod analysis" case to either scope the query with a
specific config_id or explicitly assert that other critical non-pod fixture IDs
are not present in gotIDs, using the existing
query.FindConfigAnalysisByResourceSelector and dummy fixture identifiers.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cdea0df6-404e-4e75-ad5d-b2764b54ee18
📒 Files selected for processing (5)
query/config_analysis.goquery/models.gotests/fixtures/dummy/config_analysis.gotests/query_resource_selector_test.goviews/006_config_views.sql
🚧 Files skipped from review as they are similar to previous changes (2)
- query/config_analysis.go
- query/models.go
Comment on lines
+795
to
+800
| ginkgo.It("finds high or critical pod analysis", func() { | ||
| analyses, err := query.FindConfigAnalysisByResourceSelector(DefaultContext, -1, types.ResourceSelector{Search: "severity=high,critical type=Kubernetes::Pod"}) | ||
| Expect(err).To(BeNil()) | ||
| gotIDs := lo.Map(analyses, func(a models.ConfigAnalysis, _ int) uuid.UUID { return a.ID }) | ||
| Expect(gotIDs).To(ContainElement(dummy.LogisticsAPIPodAnalysis.ID)) | ||
| }) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Tighten the pod-selector assertion.
This only proves the pod analysis is included. Since the fixtures already contain other critical analyses, a regression that ignores type=Kubernetes::Pod would still pass here. Scope this query by config_id or assert the non-pod analysis IDs are excluded.
🤖 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 `@tests/query_resource_selector_test.go` around lines 795 - 800, Tighten the
test in FindConfigAnalysisByResourceSelector so it verifies the type filter is
actually applied, not just that dummy.LogisticsAPIPodAnalysis is returned.
Update the assertion around the "finds high or critical pod analysis" case to
either scope the query with a specific config_id or explicitly assert that other
critical non-pod fixture IDs are not present in gotIDs, using the existing
query.FindConfigAnalysisByResourceSelector and dummy fixture identifiers.
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
2 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.
What
Adds resource selector search support for config insights (the
config_analysistable), matching how configs and config changes are already searchable.This means insights can now be searched through the same machinery as the other resource kinds — including the
POST /resources/searchendpoint in mission-control, which is generic overSearchResourcesRequest/SearchResourcesResponseand therefore gains a newconfig_analysiskey automatically.Changes
query/models.goConfigAnalysisQueryModel(searchable columns:config_id,scraper_id,source,analyzer,analysis_type,severity,status,summary,message,first_observed,last_observed;type/configaliases; date mappers) and registered it inGetModelFromTable.HasDeletedAtflag onQueryModel. The resource-selector engine previously always appendeddeleted_at IS NULL, which would error onconfig_analysis(no such column). The flag is settrueon all existing 9 models to preserve behavior, and leftfalseforconfig_analysis.query/resource_selector.godeleted_at IS NULLclause onqm.HasDeletedAt.ConfigAnalysistoSearchResourcesRequest/SearchResourcesResponse,GetIDs, and a new errgroup branch inSearchResources.query/config_analysis.go(new) —FindConfigAnalysisByResourceSelector,FindConfigAnalysisIDsByResourceSelector,GetConfigAnalysisByIDs, mirroring the config-changes helpers.Example
Tests
Config Analysis Resource Selectorspec block: byconfig_id,analyzer(exact + prefix),analysis_type(viatypealias),severity,status, no-match, and end-to-end viaSearchResources.HasDeletedAtchange.Summary by CodeRabbit
New Features
Bug Fixes