Repository navigation
Conversation
….7%/-12.4% memory) Signed-off-by: entlein <einentlein@gmail.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
📝 WalkthroughWalkthroughThis PR introduces a ChangesCollapse Configuration and Dynamic Path Analysis
🎯 4 (Complex) | ⏱️ ~60 minutes Sequence DiagramssequenceDiagram
participant Admin as Administrator
participant ApiServer as API Server
participant Strategy as CollapseConfigurationStrategy
participant Store as etcd Store
Admin->>ApiServer: POST CollapseConfiguration
ApiServer->>Strategy: Validate
Strategy->>Strategy: Check thresholds >= 0
Strategy->>Strategy: Validate prefixes (non-empty, /*, no duplicates)
Strategy-->>ApiServer: Field errors or OK
ApiServer->>Store: Store or Reject
Store-->>ApiServer: Acknowledged
ApiServer-->>Admin: Created or Error
sequenceDiagram
participant Processor as ApplicationProfileProcessor
participant Settings as CollapseSettings
participant Analyzer as PathAnalyzer
participant Consolidate as consolidateOpens
Processor->>Settings: GetCollapseSettings()
Settings-->>Processor: Thresholds + CollapseConfigs
Processor->>Analyzer: NewPathAnalyzerWithConfigs(threshold, configs)
Processor->>Analyzer: AnalyzePath for opens
Analyzer->>Analyzer: processSegments with per-prefix thresholds
Analyzer-->>Processor: Dynamic paths + literal paths
Processor->>Consolidate: consolidateOpens(results, sbomSet)
Consolidate->>Consolidate: Check each literal against patterns
Consolidate-->>Processor: Filtered + merged results
Processor-->>Processor: PreSave completes deflation
Suggested Labels
Suggested Reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
|
@CodeRabbit review this one together with kubescape/node-agent#810 |
🧩 Analysis chain
🏁 Script executed: Length of output: 9559
Key Areas Introduced
I'll now run a thorough review across all changed files. Given the size and scope, I'll pay close attention to:
✅ Actions performedFull review triggered. |
🧩 Analysis chainLet me fetch both PRs so I can review them together with full context. 🏁 Script executed: Length of output: 20562
Cross-PR Context:
|
| Aspect | Detail |
|---|---|
| New CRD | CollapseConfiguration — full lifecycle (types, registry, protobuf, openapi, informers, listers, clientset) |
| Wildcard matching | DNS (leading *., mid ..., trailing *), IP/CIDR, exec-arg comparison |
| Signing/tamper | Cosign-based sign+verify for ApplicationProfile / NetworkNeighborhood in node-agent |
| Memory impact | -11.7% avg / -12.4% peak (from node-agent benchmarks) |
| Test coverage | Extensive unit + component tests across both repos |
Triggering the full review now with this cross-PR context in mind.
✅ Actions performed
Full review triggered.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/registry/file/dynamicpathdetector/tests/analyze_endpoints_test.go (1)
169-178:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a length assertion before indexed comparisons.
On Line 174, the test iterates over
resultand indexestt.expected[i]without first asserting equal lengths. Ifresultis shorter thanexpected, missing endpoints won’t be detected.Proposed fix
t.Run(tt.name, func(t *testing.T) { analyzer := dynamicpathdetector.NewPathAnalyzerWithConfigs(dynamicpathdetector.EndpointDynamicThreshold, nil) result := dynamicpathdetector.AnalyzeEndpoints(&tt.input, analyzer) + assert.Len(t, result, len(tt.expected)) ja := jsonassert.New(t) for i := range result { assert.Equal(t, tt.expected[i].Endpoint, result[i].Endpoint) assert.Equal(t, tt.expected[i].Methods, result[i].Methods) ja.Assert(string(result[i].Headers), string(tt.expected[i].Headers)) } })🤖 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 `@pkg/registry/file/dynamicpathdetector/tests/analyze_endpoints_test.go` around lines 169 - 178, The test iterates result and indexes tt.expected without checking lengths; before the for-loop that ranges over result in the test (the block that calls dynamicpathdetector.AnalyzeEndpoints and assigns result), add an assertion that len(result) == len(tt.expected) to ensure mismatched lengths fail the test; reference the test variable names result and tt.expected and the call to dynamicpathdetector.AnalyzeEndpoints (and the analyzer created by NewPathAnalyzerWithConfigs) so you place the length check immediately after result is computed and before the indexed comparisons.
🤖 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 `@go.mod`:
- Line 9: The diff downgraded critical modules—github.com/anchore/syft,
github.com/anchore/stereoscope, and
github.com/opencontainers/runtime-spec—reintroducing known CVE/behavior
regressions; revert these changes by restoring or pinning safe versions (e.g.,
github.com/anchore/syft at >= v1.42.3 to include CVE-2026-33481 fixes,
github.com/anchore/stereoscope to a version that patches the tar path traversal
vulnerability, and github.com/opencontainers/runtime-spec to v1.3.0+ to retain
spec features), then run go mod tidy and run the test suite/CI to ensure
compatibility; add a short PR note explaining why these exact versions are
required (security fixes and spec compatibility) so the downgrade rationale is
documented.
In `@pkg/apis/softwarecomposition/v1beta1/generated.proto`:
- Around line 134-177: The generated proto is out of sync with the Go struct:
add the missing ipAddresses field (field number 9) to the NetworkNeighbor
message in the protobuf definition so it matches the Go struct's IPAddresses
[]string protobuf tag (bytes,9,rep,name=ipAddresses), then re-run the protobuf
generation to update generated types (or regenerate the file that produced
CollapseConfiguration additions) so marshal/unmarshal uses the same schema;
ensure the NetworkNeighbor message contains "repeated string ipAddresses = 9;"
(or equivalent) and that the regenerated Go/JSON tags align with the existing
NetworkNeighbor.IPAddresses field.
In `@pkg/registry/file/applicationprofile_processor.go`:
- Line 92: The call to a.collapseSettings() inside ApplicationProfileProcessor
can panic when a is zero-valued; before using it in
deflateApplicationProfileContainer, obtain a local settings variable (e.g., cs
:= a.collapseSettings()) and if cs == nil set cs =
dynamicpathdetector.DefaultCollapseSettings, then pass cs to
deflateApplicationProfileContainer instead of calling a.collapseSettings()
inline; update the call site in the container loop (where
deflateApplicationProfileContainer is invoked) to use this guarded local
variable.
In `@pkg/registry/file/dynamicpathdetector/analyzer.go`:
- Around line 397-433: The recursive wildcard backtracking in compareSegments
can explode exponentially; add memoization keyed by the pair of positions in the
two slices to cache boolean results and short-circuit repeated subcalls.
Implement either a small wrapper (e.g., compareSegmentsMemo or change
compareSegments signature to accept indices di, ri plus a memo map) where the
memo key is a pair (di, ri) or a compact int64/struct, check memo at entry,
compute as now, store the true/false result in memo before returning, and use
that memo for the loop that tries offsets when dynamic[0] == WildcardIdentifier;
keep existing semantics for WildcardIdentifier and DynamicIdentifier unchanged.
In `@pkg/registry/file/dynamicpathdetector/tests/compare_exec_args_test.go`:
- Around line 284-313: The timing assertion in
TestCompareExecArgs_ReDoSResistance is too brittle; update the test to avoid CI
flakes by (1) skipping the timing check when testing.Short() is true and (2)
raising the wall‑clock cap from 100ms to a more generous value (e.g. 500ms) for
the remaining runs. Modify the TestCompareExecArgs_ReDoSResistance function to
wrap the elapsed > threshold check with a conditional that first returns if
testing.Short() and otherwise uses the larger threshold when asserting
performance of dynamicpathdetector.CompareExecArgs.
---
Outside diff comments:
In `@pkg/registry/file/dynamicpathdetector/tests/analyze_endpoints_test.go`:
- Around line 169-178: The test iterates result and indexes tt.expected without
checking lengths; before the for-loop that ranges over result in the test (the
block that calls dynamicpathdetector.AnalyzeEndpoints and assigns result), add
an assertion that len(result) == len(tt.expected) to ensure mismatched lengths
fail the test; reference the test variable names result and tt.expected and the
call to dynamicpathdetector.AnalyzeEndpoints (and the analyzer created by
NewPathAnalyzerWithConfigs) so you place the length check immediately after
result is computed and before the indexed comparisons.
🪄 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: e12e4fd2-1b66-4ed9-b497-e7bf4ca9c574
⛔ Files ignored due to path filters (18)
go.sumis excluded by!**/*.sumpkg/apis/softwarecomposition/v1beta1/generated.pb.gois excluded by!**/*.pb.gopkg/apis/softwarecomposition/v1beta1/generated.protomessage.pb.gois excluded by!**/*.pb.gopkg/generated/applyconfiguration/softwarecomposition/v1beta1/collapseconfigentry.gois excluded by!**/generated/**pkg/generated/applyconfiguration/softwarecomposition/v1beta1/collapseconfiguration.gois excluded by!**/generated/**pkg/generated/applyconfiguration/softwarecomposition/v1beta1/collapseconfigurationspec.gois excluded by!**/generated/**pkg/generated/applyconfiguration/utils.gois excluded by!**/generated/**pkg/generated/clientset/versioned/typed/softwarecomposition/v1beta1/collapseconfiguration.gois excluded by!**/generated/**pkg/generated/clientset/versioned/typed/softwarecomposition/v1beta1/fake/fake_collapseconfiguration.gois excluded by!**/generated/**pkg/generated/clientset/versioned/typed/softwarecomposition/v1beta1/fake/fake_softwarecomposition_client.gois excluded by!**/generated/**pkg/generated/clientset/versioned/typed/softwarecomposition/v1beta1/generated_expansion.gois excluded by!**/generated/**pkg/generated/clientset/versioned/typed/softwarecomposition/v1beta1/softwarecomposition_client.gois excluded by!**/generated/**pkg/generated/informers/externalversions/generic.gois excluded by!**/generated/**pkg/generated/informers/externalversions/softwarecomposition/v1beta1/collapseconfiguration.gois excluded by!**/generated/**pkg/generated/informers/externalversions/softwarecomposition/v1beta1/interface.gois excluded by!**/generated/**pkg/generated/listers/softwarecomposition/v1beta1/collapseconfiguration.gois excluded by!**/generated/**pkg/generated/listers/softwarecomposition/v1beta1/expansion_generated.gois excluded by!**/generated/**pkg/generated/openapi/zz_generated.openapi.gois excluded by!**/generated/**
📒 Files selected for processing (52)
.gitignoreartifacts/collapseconfiguration-default-sample.yamlgo.modpkg/apis/softwarecomposition/collapse_types.gopkg/apis/softwarecomposition/network_types.gopkg/apis/softwarecomposition/register.gopkg/apis/softwarecomposition/v1beta1/collapse_types.gopkg/apis/softwarecomposition/v1beta1/generated.protopkg/apis/softwarecomposition/v1beta1/network_types.gopkg/apis/softwarecomposition/v1beta1/network_types_protobuf_test.gopkg/apis/softwarecomposition/v1beta1/register.gopkg/apis/softwarecomposition/v1beta1/zz_generated.conversion.gopkg/apis/softwarecomposition/v1beta1/zz_generated.deepcopy.gopkg/apis/softwarecomposition/v1beta1/zz_generated.model_name.gopkg/apis/softwarecomposition/zz_generated.deepcopy.gopkg/apiserver/apiserver.gopkg/registry/file/applicationprofile_processor.gopkg/registry/file/applicationprofile_processor_collapse_provider_test.gopkg/registry/file/applicationprofile_processor_test.gopkg/registry/file/cleanup.gopkg/registry/file/cleanup_test.gopkg/registry/file/containerprofile_processor.gopkg/registry/file/containerprofile_processor_collapse_provider_test.gopkg/registry/file/dynamicpathdetector/analyze_endpoints.gopkg/registry/file/dynamicpathdetector/analyze_endpoints_internal_test.gopkg/registry/file/dynamicpathdetector/analyze_opens.gopkg/registry/file/dynamicpathdetector/analyzer.gopkg/registry/file/dynamicpathdetector/collapse_config_from_crd.gopkg/registry/file/dynamicpathdetector/compare_exec_args.gopkg/registry/file/dynamicpathdetector/tests/analyze_endpoints_test.gopkg/registry/file/dynamicpathdetector/tests/analyze_opens_test.gopkg/registry/file/dynamicpathdetector/tests/benchmark_test.gopkg/registry/file/dynamicpathdetector/tests/collapse_config_crd_test.gopkg/registry/file/dynamicpathdetector/tests/compare_exec_args_test.gopkg/registry/file/dynamicpathdetector/tests/consolidate_opens_test.gopkg/registry/file/dynamicpathdetector/tests/coverage_test.gopkg/registry/file/dynamicpathdetector/tests/profile_test.gopkg/registry/file/dynamicpathdetector/types.gopkg/registry/file/networkmatch/README.mdpkg/registry/file/networkmatch/bench_test.gopkg/registry/file/networkmatch/doc.gopkg/registry/file/networkmatch/match_dns.gopkg/registry/file/networkmatch/match_dns_test.gopkg/registry/file/networkmatch/match_ip.gopkg/registry/file/networkmatch/match_ip_test.gopkg/registry/file/networkmatch/validate.gopkg/registry/file/networkmatch/validate_test.gopkg/registry/softwarecomposition/collapseconfiguration/etcd.gopkg/registry/softwarecomposition/collapseconfiguration/strategy.gopkg/registry/softwarecomposition/collapseconfiguration/strategy_test.gopkg/registry/softwarecomposition/networkneighborhood/strategy.gopkg/registry/softwarecomposition/networkneighborhood/strategy_test.go
Signed-off-by: entlein <einentlein@gmail.com>
🧩 Analysis chain🏁 Script executed: Length of output: 5716
✅ Confirmed Atomic
|
🧩 Analysis chain
🏁 Script executed: Unrecoverable error: Failed to clone repository 🏁 Script executed: Unrecoverable error: Failed to clone repository 🏁 Script executed: Unrecoverable error: Stream setup permanently failed: 14 UNAVAILABLE: Connection dropped
✅ Confirmed Atomic / Well-Scoped
|
|
the syft downgrade (etc) was now addressed in storage , we should be perfectly aligned with upstream, please recheck |
Signed-off-by: entlein <einentlein@gmail.com>
|
superseded by the other PRs |
Umbrella PR consolidating the SBOB-wildcards storage surface. Tier-by-tier breakdown per CodeRabbit's atomic-split advisory on
kubescape/storage#323:kubescape/storage#323(path-wildcards core: anchored trailing-*, per-endpoint port key, CollapseConfig types)kubescape/storage#322(CompareExecArgs),#325(CollapseConfig CRD + processors)kubescape/storage#324(networkmatch package: DNS leading-*+ trailing-*+ mid-⋯+ CIDR + IPAddresses +*sentinel)This umbrella carries the union of the above. Once the per-tier PRs land in order, the umbrella shrinks to empty.
Performance evidence (node-agent side)
The bench is run on the node-agent build that links against this storage version. Validated on workflow run 25959674209:
Companion node-agent umbrella
kubescape/node-agent#810. Storage and node-agent run on different syft chains by design — storage onanchore/syft v1.42.4(matches storage upstream main), node-agent onkubescape/syft v1.32.0-ks.2via replace (matches node-agent upstream main).Notes
v1.42.4for CVE-2026-33481 (matches the storage upstream main bump in commitcecb767f).CompareDynamicperf: thesplitPathallocation pattern called out in matthyx's earlier review of PR CollapsConfigs and Wildcard Matching for Opens (and Paths in Endpoints) while reproducing default storage behavior if not used #316 line 358 is unchanged here. We've added DP-memoisation incompareSegments(pkg/registry/file/dynamicpathdetector/analyzer.go) to bound the worst-case for multi-*patterns, butsplitPathitself still allocates two slices per call. Happy to rebase onto the zero-alloc rewrite when it lands.pkg/registry/file/networkmatch/corrected tobillofbehavior.com.