Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe Helm chart now forwards global HTTPS proxy settings and combined no-proxy values to node-agent and kubevuln SBOM scanner sidecars. Snapshot tests verify both rendered configurations. ChangesSBOM scanner proxy support
Estimated code review effort: 2 (Simple) | ~10 minutes Mergeability Score: ⚪ Minimal · up to This localized change forwards the configured HTTPS proxy settings to the sbom-scanner sidecar and adds coverage for the new behavior; no actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant GlobalProxyConfiguration
participant HelmTemplates
participant SBOMScannerSidecars
GlobalProxyConfiguration->>HelmTemplates: provide httpsProxy and noProxy values
HelmTemplates->>SBOMScannerSidecars: render HTTPS_PROXY and combined no_proxy
SBOMScannerSidecars-->>HelmTemplates: produce configured pod specifications
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
The sbom-scanner sidecar pulls image data over HTTPS itself but never received global.httpsProxy/no_proxy like the main container does. In clusters that route egress through a proxy (especially with global.networkPolicy.createEgressRules enabled, which only opens egress to the proxy IP/port) the sidecar had no route to the registry. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>
5e5906f to
e99f60e
Compare
|
On GKE Autopilot the node-agent would be rejected by GKE Warden until Google approves a matching allowlist, so customers could not install this chart version. To unblock this PR:
Drift detail |
|
cc @slashben we'll need a new autopilot submission |
matthyx
left a comment
There was a problem hiding this comment.
Code review: this looks correct and well-tested.
node-agent/_node-agent.tpl:no_proxy_envar_listis now threaded through tonode-agent.sbomScannerContainerthe same way it already is fornode-agent.container, and the call site update (nodeAgent.podSpec→node-agent.sbomScannerContainer) passes it correctly.kubevuln/deployment.yaml:$no_proxy_envar_listis defined at file scope (top of the template) so it's in scope for the new sbom-scanner block; the addedHTTPS_PROXY/no_proxyenv entries are an exact mirror of the existing block for the mainkubevulncontainer.- New snapshot tests assert the right container index for both the node-agent daemonset and the kubevuln deployment, and match the existing test style in this file.
helm unittestcoverage (60 tests / 996 snapshots + 2 new) is consistent with the PR description.
Holding off on approval for one reason: the allowlist-drift check is failing — the new sbom-scanner env vars (HTTPS_PROXY, no_proxy) aren't in the approved GKE Autopilot allowlist (armo-kubescape-node-agent-1.40-v2), so on GKE Autopilot this node-agent spec would be rejected by GKE Warden until Google approves an updated allowlist. I see this is already being tracked (cc @slashben re: new autopilot submission) — once the Gerrit allowlist update lands and a maintainer adds allowlist-drift-ack, this is good to merge from my side.
Overview
Follow-up to #898. The
sbom-scannersidecar (added tonode-agentandkubevulnfor OOM isolation) pulls image data over HTTPS itself but never receivedglobal.httpsProxy/no_proxythe way the main container does. In clusters that route egress through a proxy — especially withglobal.networkPolicy.createEgressRulesenabled, which only opens egress to DNS, the API server, and the proxy IP/port — the sidecar had no route to the registry at all.Adds the same
HTTPS_PROXY/no_proxyenv vars to thesbom-scannercontainer in:charts/kubescape-operator/templates/kubevuln/deployment.yamlcharts/kubescape-operator/templates/node-agent/_node-agent.tplHow to Test
Added two new test cases in
tests/snapshot_test.yamlasserting the sidecar picks upHTTPS_PROXY/no_proxywhenglobal.httpsProxyis set. Full suite (60 tests / 996 snapshots) passes unchanged plus the 2 new tests.Checklist before requesting a review
AI-skills: none
Summary by CodeRabbit
New Features
no_proxyenvironment variable.Tests