Conversation
Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>
Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>
📝 WalkthroughWalkthroughThe pull request optimizes container scanning by adding early-skip conditions to prevent redundant processing. When a container profile has already been scanned, remaining containers from the same workload are skipped. Additionally, timestamp-based profiles are now skipped if a specific metadata key is present in annotations. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 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)
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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@utils/containerprofile.go`:
- Around line 29-31: The SkipContainerProfile function currently returns early
for any profile with helpersv1.ReportSeriesIdMetadataKey which globally changes
behavior; revert that TS-specific check out of SkipContainerProfile and restore
generic validation there, then add a new relevancy-specific helper or extend the
relevancy scan call path (e.g., in watcher/containerprofilewatcher.go or the
relevancy scorer) to perform the ReportSeriesIdMetadataKey check and skip TS
profiles only during relevancy scans; update callers of SkipContainerProfile to
call the new relevancy helper where appropriate so only the relevancy flow
excludes ReportSeriesIdMetadataKey profiles.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c95eda5e-508e-47e4-bcf7-231ef97283c7
📒 Files selected for processing (2)
mainhandler/handlerequests.goutils/containerprofile.go
| if _, ok := annotations[helpersv1.ReportSeriesIdMetadataKey]; ok { | ||
| return true, nil // skip TS profiles | ||
| } |
There was a problem hiding this comment.
Scope this TS skip to relevancy flow only (Line 29).
This early return changes behavior for every caller of utils.SkipContainerProfile, including watcher/containerprofilewatcher.go (snippet 1), not just relevancy scans. Profiles carrying helpersv1.ReportSeriesIdMetadataKey will now be globally skipped.
Please keep generic validation in SkipContainerProfile and move TS-specific skipping into a relevancy-specific helper/call path.
Proposed refactor to limit behavior change to relevancy scan path
func SkipContainerProfile(annotations map[string]string) (bool, error) {
@@
- if _, ok := annotations[helpersv1.ReportSeriesIdMetadataKey]; ok {
- return true, nil // skip TS profiles
- }
if status, ok := annotations[helpersv1.StatusMetadataKey]; ok && !slices.Contains(ann, status) {
return true, fmt.Errorf("invalid status")
}
@@
return false, nil // do not skip
}
+
+func SkipContainerProfileForRelevancyScan(annotations map[string]string) (bool, error) {
+ if _, ok := annotations[helpersv1.ReportSeriesIdMetadataKey]; ok {
+ return true, nil // skip TS profiles only for relevancy scans
+ }
+ return SkipContainerProfile(annotations)
+}
@@
- if skip, err := SkipContainerProfile(profile.Annotations); skip {
+ if skip, err := SkipContainerProfileForRelevancyScan(profile.Annotations); skip {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if _, ok := annotations[helpersv1.ReportSeriesIdMetadataKey]; ok { | |
| return true, nil // skip TS profiles | |
| } | |
| // SkipContainerProfile performs generic validation on container profile annotations | |
| if status, ok := annotations[helpersv1.StatusMetadataKey]; ok && !slices.Contains(ann, status) { | |
| return true, fmt.Errorf("invalid status") | |
| } | |
| return false, nil // do not skip | |
| } | |
| // SkipContainerProfileForRelevancyScan skips profiles for the relevancy scan path only | |
| func SkipContainerProfileForRelevancyScan(annotations map[string]string) (bool, error) { | |
| if _, ok := annotations[helpersv1.ReportSeriesIdMetadataKey]; ok { | |
| return true, nil // skip TS profiles only for relevancy scans | |
| } | |
| return SkipContainerProfile(annotations) | |
| } | |
| // In the relevancy scan caller: | |
| if skip, err := SkipContainerProfileForRelevancyScan(profile.Annotations); skip { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@utils/containerprofile.go` around lines 29 - 31, The SkipContainerProfile
function currently returns early for any profile with
helpersv1.ReportSeriesIdMetadataKey which globally changes behavior; revert that
TS-specific check out of SkipContainerProfile and restore generic validation
there, then add a new relevancy-specific helper or extend the relevancy scan
call path (e.g., in watcher/containerprofilewatcher.go or the relevancy scorer)
to perform the ReportSeriesIdMetadataKey check and skip TS profiles only during
relevancy scans; update callers of SkipContainerProfile to call the new
relevancy helper where appropriate so only the relevancy flow excludes
ReportSeriesIdMetadataKey profiles.
There was a problem hiding this comment.
it's on purpose, we should not react on TS profiles
There was a problem hiding this comment.
@matthyx, understood — skipping TS profiles (those carrying helpersv1.ReportSeriesIdMetadataKey) globally across all callers of SkipContainerProfile is intentional, not just for the relevancy scan path. Thanks for the clarification!
✏️ Learnings added
Learnt from: matthyx
Repo: kubescape/operator PR: 362
File: utils/containerprofile.go:29-31
Timestamp: 2026-03-26T15:05:03.285Z
Learning: In `kubescape/operator`, TS (time-series) container profiles — identified by the presence of `helpersv1.ReportSeriesIdMetadataKey` in their annotations — should be globally skipped across all callers of `utils.SkipContainerProfile` in `utils/containerprofile.go`. This is intentional design: no code path should react to TS profiles.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
|
Summary:
|
1 participant
Summary by CodeRabbit