Skip to content

feat(cache): support server-side user-managed profile merge configurations#831

Draft
yugal07 wants to merge 1 commit into
kubescape:mainfrom
yugal07:830
Draft

feat(cache): support server-side user-managed profile merge configurations#831
yugal07 wants to merge 1 commit into
kubescape:mainfrom
yugal07:830

Conversation

@yugal07

@yugal07 yugal07 commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Overview

Adds a ServerSideUserManagedMerge configuration flag (profileProjection.serverSideUserManagedMergeEnabled, default false) that lets node-agent skip its client-side user-managed (ug-) ApplicationProfile/NetworkNeighborhood fetch + projection and rely on the storage backend serving the already-merged ContainerProfile on GET (kubescape/storage#319).

Current behavior: node-agent always fetches ug-<workload> AP/NN by their well-known name and projects them on top of the consolidated ContainerProfile, in both tryPopulateEntry and the reconciler refresh path. This duplicates work that storage#319 now performs server-side, resulting in two extra GETs per container populate/refresh cycle.

New behavior: when the flag is enabled, node-agent performs no ug- GETs and no client-side ug- projection. The user-managed overlay is received transparently via the merged ContainerProfile returned by GetContainerProfile.

The flag is disabled by default and should only be enabled once a storage version containing kubescape/storage#319 is confirmed to be deployed.

Additional Information

  • The gating is intentionally minimal and only short-circuits the two ug- fetch sites.
  • Leaving userManagedAP / userManagedNN as nil allows the downstream projection pass, resource-version bookkeeping, overlay metrics, synthetic-CP fallback, and reconciler RV fast-skip logic to become natural no-ops without introducing flag checks throughout the projection pipeline.
  • The label-driven user-defined overlay (pass 2, keyed off UserDefinedProfileMetadataKey) remains unchanged. This is a separate client-side feature and is not covered by storage#319.
  • This follows the feature-flag approach suggested in the issue discussion rather than immediately deleting the client-side ug- path. Existing behavior remains the default, and cleanup can be deferred until the flag is eventually flipped.
  • Assumes storage#319 performs merging on GET requests only (not List/Watch). Since node-agent currently reads ContainerProfiles via GET, the merged view is always observed. A code comment near the CP GET documents this dependency for future maintainers.
  • A companion change in kubescape/helm-charts exposes profileProjection.serverSideUserManagedMergeEnabled through the node-agent ConfigMap (separate PR, default false).

How to Test

go test ./pkg/objectcache/containerprofilecache/... ./pkg/config/... -count=1

Covered test cases

  • TestServerSideMerge_SkipsUgFetch

    • Flag ON.
    • No ug- AP/NN GETs are performed.
    • No client-side merge occurs.
    • User-managed exec data is observed only through the simulated server-merged ContainerProfile.
  • TestServerSideMerge_RefreshSkipsUgRefetch

    • Reconciler refresh path also skips ug- re-fetches when the flag is enabled.
  • TestServerSideMerge_LabelOverlayStillApplies

    • Label-driven overlays continue to be projected when the flag is enabled.
  • TestClientSideMerge_DefaultFetchesUg

    • Default behavior (flag OFF) continues to fetch and merge ug- profiles.

Related Issues / PRs

Checklist before requesting a review

  • My code follows the style guidelines of this project
  • I have commented on my code, particularly in hard-to-understand areas
  • I have performed a self-review of my code
  • If it is a core feature, I have added thorough tests
  • New and existing unit tests pass locally with my changes

Summary by CodeRabbit

  • New Features

    • Introduced optional server-side user-managed merge configuration flag, default disabled. When enabled, shifts user-managed overlay processing to the server-side, reducing client-side operations and improving efficiency.
  • Tests

    • Added comprehensive test coverage validating both server-side and client-side merge behavior under different configuration states.

…tion

Signed-off-by: yugal07 <yashsadhwani544@gmail.com>
@coderabbitai

coderabbitai Bot commented Jun 4, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ef8d9bf6-8b32-4a4b-9196-91d0f796e8da

📥 Commits

Reviewing files that changed from the base of the PR and between 2038a1b and 0cc8a92.

📒 Files selected for processing (6)
  • pkg/config/config.go
  • pkg/objectcache/containerprofilecache/containerprofilecache.go
  • pkg/objectcache/containerprofilecache/containerprofilecache_test.go
  • pkg/objectcache/containerprofilecache/reconciler.go
  • pkg/objectcache/containerprofilecache/reconciler_test.go
  • tests/chart/values.yaml

📝 Walkthrough

Walkthrough

This PR adds a feature flag to conditionally skip redundant client-side user-managed (ug-) profile fetching and merging when the storage backend (kubescape/storage#319) serves pre-merged ContainerProfiles. The implementation gates existing overlay fetches in the cache population and refresh paths, includes test instrumentation to validate both behaviors, and configures the flag as disabled by default in Helm values.

Changes

Server-Side User-Managed Merge

Layer / File(s) Summary
Configuration schema
pkg/config/config.go
Adds ServerSideUserManagedMerge bool field to ProfileProjectionConfig with documentation explaining server-side merge behavior, default semantics, and storage version assumptions.
Cache population with conditional overlay fetch
pkg/objectcache/containerprofilecache/containerprofilecache.go
tryPopulateEntry now skips per-container ug-<workloadName> ApplicationProfile and NetworkNeighborhood fetches when the flag is enabled, assuming the consolidated ContainerProfile already includes the server-merged overlay.
Cache refresh with conditional overlay re-fetch
pkg/objectcache/containerprofilecache/reconciler.go
refreshOneEntry now conditionally skips re-fetching ug- overlays during refresh when the flag is enabled, treating the earlier consolidated CP fetch as already containing the merged overlay.
Test instrumentation for ug- call tracking*
pkg/objectcache/containerprofilecache/containerprofilecache_test.go
fakeProfileClient is extended with ugAPCalls and ugNNCalls counters to track ug- prefix fetches; test cache constructors are refactored to support configuration-driven instantiation via newTestCacheWithConfig and serverSideMergeConfig.
Test cases for merge behavior validation
pkg/objectcache/containerprofilecache/containerprofilecache_test.go, pkg/objectcache/containerprofilecache/reconciler_test.go
Adds comprehensive tests: TestServerSideMerge_SkipsUgFetch and TestServerSideMerge_LabelOverlayStillApplies verify server-side merge skips ug- fetches while preserving server-merged execs and label-driven overlays; TestClientSideMerge_DefaultFetchesUg asserts default behavior still performs client-side ug- fetch/merge; TestServerSideMerge_RefreshSkipsUgRefetch validates refresh-time skip behavior under the flag.
Helm chart configuration defaults
tests/chart/values.yaml
Adds serverSideUserManagedMergeEnabled: false setting under nodeAgent configuration to maintain current client-side merge behavior by default until storage-side deployment is confirmed.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

  • kubescape/storage#315: Complements the storage-side server-side merge of user-managed ug- profiles so node-agent can conditionally skip redundant client-side overlays based on deployment readiness.

Possibly related PRs

  • kubescape/node-agent#788: Both PRs modify the ContainerProfileCache user-managed overlay path in pkg/objectcache/containerprofilecache, with this PR adding flag-controlled skip/refetch behavior that builds on the CP-cache design introduced in #788.

Poem

🐰 A merge moves server-ward today,
Two fetches shrink to one, hip-hooray!
The cache still works when flags aren't set,
And tests prove both paths, no regret.
Hop along, storage sync—we're ready yet!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Linked Issues check ❓ Inconclusive The PR partially addresses issue #830 by adding a feature flag to skip client-side ug- merges when server-side merge is enabled, but does not complete cleanup of dead code like UserManagedAPRV fields or user-managed branches in related functions. Clarify whether this PR is intended as a phased approach (flag addition first, cleanup later) or if dead code cleanup should be included; verify all issue requirements are met or document deferred work.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely describes the main change: adding support for server-side user-managed profile merge configuration.
Out of Scope Changes check ✅ Passed All changes are directly scoped to implementing the server-side user-managed merge feature flag: configuration field, conditional skip logic, tests, and Helm values.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@yugal07

yugal07 commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

I will make the relevant changes once storage 319 is deployed. Let me know when

@matthyx matthyx moved this to WIP in KS PRs tracking Jun 5, 2026
@yugal07 yugal07 marked this pull request as draft June 5, 2026 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: WIP

Development

Successfully merging this pull request may close these issues.

Remove client-side ug- profile merge once storage server-side merge (storage#319) is deployed

2 participants