feat: add SBOM scanner sidecar container to node-agent pod#802
Merged
Conversation
📝 WalkthroughWalkthroughAdds an optional SBOM scanner sidecar to the node-agent Helm chart. Enablement is computed from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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
🧹 Nitpick comments (1)
charts/kubescape-operator/templates/node-agent/_node-agent.tpl (1)
64-69: UseincludeSbomScanneras the single toggle across env/mounts/volumes too.Right now, Line 517 controls sidecar container inclusion, but Lines 64-69, 137-140, and 369-373 are still controlled only by values/capability checks. Threading
includeSbomScannerinto those helpers will keep behavior consistent and avoid partial SBOM wiring without the sidecar.♻️ Suggested refactor
{{- define "node-agent.env" -}} @@ -{{- if and (eq .Values.capabilities.nodeSbomGeneration "enable") .Values.nodeAgent.sbomScanner.enabled (not .autoscalerMode) }} +{{- if and .includeSbomScanner .components.sbomScanner.enabled (not .autoscalerMode) }} - name: SBOM_SCANNER_SOCKET value: "/sbom-comm/scanner.sock" - name: SCANNER_MEMORY_LIMIT value: "{{ .Values.nodeAgent.sbomScanner.resources.limits.memory }}" {{- end }} {{- define "node-agent.volumeMounts" -}} @@ -{{- if and (eq .Values.capabilities.nodeSbomGeneration "enable") .Values.nodeAgent.sbomScanner.enabled }} +{{- if and .includeSbomScanner .components.sbomScanner.enabled }} - name: sbom-comm mountPath: /sbom-comm {{- end }} {{- define "node-agent.volumes" -}} @@ -{{- if and (eq .Values.capabilities.nodeSbomGeneration "enable") .Values.nodeAgent.sbomScanner.enabled }} +{{- if and .includeSbomScanner .components.sbomScanner.enabled }} {{- if .Values.nodeAgent.sbomScanner.volumes }} {{ toYaml .Values.nodeAgent.sbomScanner.volumes | trim }} {{- end }} {{- end }} {{- define "node-agent.container" -}} @@ - {{- include "node-agent.env" (dict "Values" .Values "components" .components "no_proxy_envar_list" .no_proxy_envar_list "autoscalerMode" .autoscalerMode "testingMode" .testingMode) | nindent 4 }} + {{- include "node-agent.env" (dict "Values" .Values "components" .components "no_proxy_envar_list" .no_proxy_envar_list "autoscalerMode" .autoscalerMode "testingMode" .testingMode "includeSbomScanner" .includeSbomScanner) | nindent 4 }} @@ - {{- include "node-agent.volumeMounts" (dict "Values" .Values "components" .components) | nindent 4 }} + {{- include "node-agent.volumeMounts" (dict "Values" .Values "components" .components "includeSbomScanner" .includeSbomScanner) | nindent 4 }} {{- define "node-agent.podSpec" -}} @@ -volumes: -{{ include "node-agent.volumes" (dict "Values" .Values "components" .components) | trim | nindent 0 }} +volumes: +{{ include "node-agent.volumes" (dict "Values" .Values "components" .components "includeSbomScanner" .includeSbomScanner) | trim | nindent 0 }} @@ -{{ include "node-agent.container" (dict "Values" .Values "components" .components "no_proxy_envar_list" .no_proxy_envar_list "autoscalerMode" .autoscalerMode "testingMode" .testingMode "resources" .resources) | trim | nindent 0 }} +{{ include "node-agent.container" (dict "Values" .Values "components" .components "no_proxy_envar_list" .no_proxy_envar_list "autoscalerMode" .autoscalerMode "testingMode" .testingMode "resources" .resources "includeSbomScanner" .includeSbomScanner) | trim | nindent 0 }}Also applies to: 137-140, 369-373, 517-519, 566-566
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/kubescape-operator/templates/node-agent/_node-agent.tpl` around lines 64 - 69, The env vars and mounts for the SBOM scanner (e.g., SBOM_SCANNER_SOCKET and SCANNER_MEMORY_LIMIT) are gated by capability/value checks but not by the includeSbomScanner toggle, which can lead to orphaned wiring when the sidecar is omitted; update the template helpers and conditionals that render these blocks (the helpers/sections rendering SBOM_SCANNER_SOCKET, SCANNER_MEMORY_LIMIT, the mounts and volumes at the other mentioned locations) to require includeSbomScanner to be true in addition to the existing checks (i.e., replace or augment conditions like and (eq .Values.capabilities.nodeSbomGeneration "enable") .Values.nodeAgent.sbomScanner.enabled with and (includeSbomScanner) (eq .Values.capabilities.nodeSbomGeneration "enable") .Values.nodeAgent.sbomScanner.enabled) so env/mount/volume fragments are only emitted when the sidecar is actually included.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@charts/kubescape-operator/templates/node-agent/daemonset.yaml`:
- Line 36: The helm-unittest snapshots need updating because the DaemonSet
template node-agent/daemonset.yaml changed (the spec input now includes
"includeSbomScanner" true); run the helm-unittest tests for the
charts/kubescape-operator chart (including multi-daemonset variants), regenerate
the failing snapshot files, review the new rendered outputs for
node-agent/daemonset.yaml to confirm the includeSbomScanner change is expected,
and commit the updated snapshot files alongside this PR.
---
Nitpick comments:
In `@charts/kubescape-operator/templates/node-agent/_node-agent.tpl`:
- Around line 64-69: The env vars and mounts for the SBOM scanner (e.g.,
SBOM_SCANNER_SOCKET and SCANNER_MEMORY_LIMIT) are gated by capability/value
checks but not by the includeSbomScanner toggle, which can lead to orphaned
wiring when the sidecar is omitted; update the template helpers and conditionals
that render these blocks (the helpers/sections rendering SBOM_SCANNER_SOCKET,
SCANNER_MEMORY_LIMIT, the mounts and volumes at the other mentioned locations)
to require includeSbomScanner to be true in addition to the existing checks
(i.e., replace or augment conditions like and (eq
.Values.capabilities.nodeSbomGeneration "enable")
.Values.nodeAgent.sbomScanner.enabled with and (includeSbomScanner) (eq
.Values.capabilities.nodeSbomGeneration "enable")
.Values.nodeAgent.sbomScanner.enabled) so env/mount/volume fragments are only
emitted when the sidecar is actually included.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0c401786-dfa9-4b1a-80f7-ac0fc86a2aa2
📒 Files selected for processing (6)
charts/kubescape-operator/templates/_common.tplcharts/kubescape-operator/templates/node-agent/_node-agent.tplcharts/kubescape-operator/templates/node-agent/daemonset.yamlcharts/kubescape-operator/templates/node-agent/daemonsets.yamlcharts/kubescape-operator/templates/node-agent/template-configmap.yamlcharts/kubescape-operator/values.yaml
Add Helm chart support for the SBOM scanner sidecar that runs Syft in a separate container with its own memory cgroup. Configuration is nested under nodeAgent.sbomScanner in values.yaml. - Add nodeAgent.sbomScanner section in values.yaml (enabled, image, resources, volumes) - Add sbomScannerContainer template in _node-agent.tpl with GOMEMLIMIT downward API - Wire sidecar into podSpec containers, volumes, and volumeMounts - Add SBOM_SCANNER_SOCKET and SCANNER_MEMORY_LIMIT env vars to main container - Add sbomScanner component flag in _common.tpl - Exclude sidecar from autoscaler template (includeSbomScanner: false) Made-with: Cursor Signed-off-by: Ben <ben@armosec.io> # Conflicts: # charts/kubescape-operator/values.yaml
Signed-off-by: Ben <ben@armosec.io> Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>
Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>
matthyx
force-pushed
the
feature/sbom-scanner-sidecar
branch
from
8ab94e4 to
4331bc9
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
charts/kubescape-operator/templates/node-agent/_node-agent.tpl (1)
259-295: Health probes are consistent with chart's sidecar design pattern, though adding them would be a best practice.The missing liveness/readiness probes match the pattern used for other sidecars in this chart (e.g., ClamAV container at lines 240-257 also lacks probes). However, since the main node-agent container has comprehensive HTTP probes (lines 195-211), adding basic probes to the sbomScanner would improve observability:
- Optional: Add readiness/liveness probes if the scanner exposes a health endpoint or socket availability check (e.g., probe the
/sbom-comm/scanner.socksocket path).Regarding capabilities: The
drop: ["ALL"]restriction is appropriate. The sbomScanner doesn't directly access the host filesystem—it communicates via a shared volume (/sbom-comm) and receives the host path only as an environment variable (HOST_ROOT). Direct hostPath mounting is handled by the main container.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/kubescape-operator/templates/node-agent/_node-agent.tpl` around lines 259 - 295, Add optional liveness/readiness probes to the node-agent.sbomScannerContainer template: introduce a conditional flag .Values.nodeAgent.sbomScanner.probes.enabled and, when set, render livenessProbe and readinessProbe under the sbom scanner container (inside the define "node-agent.sbomScannerContainer"). If the scanner exposes a Unix socket use an exec probe like ["sh","-c","test -S /sbom-comm/scanner.sock"] (or use httpGet to the health endpoint if one exists) and keep probe settings (initialDelaySeconds, periodSeconds, failureThreshold) configurable via .Values.nodeAgent.sbomScanner.probes.*; render them with toYaml / nindent consistent with the existing container fields so the probes appear alongside env, resources, and volumeMounts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@charts/kubescape-operator/templates/node-agent/_node-agent.tpl`:
- Around line 259-295: Add optional liveness/readiness probes to the
node-agent.sbomScannerContainer template: introduce a conditional flag
.Values.nodeAgent.sbomScanner.probes.enabled and, when set, render livenessProbe
and readinessProbe under the sbom scanner container (inside the define
"node-agent.sbomScannerContainer"). If the scanner exposes a Unix socket use an
exec probe like ["sh","-c","test -S /sbom-comm/scanner.sock"] (or use httpGet to
the health endpoint if one exists) and keep probe settings (initialDelaySeconds,
periodSeconds, failureThreshold) configurable via
.Values.nodeAgent.sbomScanner.probes.*; render them with toYaml / nindent
consistent with the existing container fields so the probes appear alongside
env, resources, and volumeMounts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 60a0a9bf-2423-4111-a5c8-c35d055c0b18
⛔ Files ignored due to path filters (1)
charts/kubescape-operator/tests/__snapshot__/snapshot_test.yaml.snapis excluded by!**/*.snap
📒 Files selected for processing (6)
charts/kubescape-operator/templates/_common.tplcharts/kubescape-operator/templates/node-agent/_node-agent.tplcharts/kubescape-operator/templates/node-agent/daemonset.yamlcharts/kubescape-operator/templates/node-agent/daemonsets.yamlcharts/kubescape-operator/templates/node-agent/template-configmap.yamlcharts/kubescape-operator/values.yaml
✅ Files skipped from review due to trivial changes (1)
- charts/kubescape-operator/templates/node-agent/template-configmap.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
- charts/kubescape-operator/templates/node-agent/daemonset.yaml
- charts/kubescape-operator/templates/_common.tpl
- charts/kubescape-operator/templates/node-agent/daemonsets.yaml
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.
Summary
nodeAgent.sbomScannerin values.yaml, keeping node-agent pod config co-locatedincludeSbomScanner: false), matching the ClamAV patternChanges
values.yamlnodeAgent.sbomScannerconfig (enabled, image, command, resources, volumeMounts, volumes)_node-agent.tplsbomScannerContainertemplate, wire into podSpec containers, add sbom-comm volume/mount, add env vars_common.tplsbomScanner.enabledcomponent flagdaemonset.yamlincludeSbomScanner: truedaemonsets.yamlincludeSbomScanner: truetemplate-configmap.yamlincludeSbomScanner: false(autoscaler)Companion PR
Test plan
helm lintpasseshelm templaterenders sidecar container with correct image, command, resources, env vars, and volumescapabilities.nodeSbomGeneration=enableandnodeAgent.sbomScanner.enabled=trueMade with Cursor
Summary by CodeRabbit