SIGN IN SIGN UP

feat(sbom): route host SBOM scans through the sbom-scanner sidecar (#979)

* feat(sbom): route host SBOM scans through the sbom-scanner sidecar

The host root-filesystem scan is the heaviest thing node-agent does, and the
sbom-scanner sidecar exists precisely to isolate this class of work: it has a
far more generous budget (1 CPU / 4Gi vs. node-agent's 394m / 682Mi) and
already mounts /host read-only. It went unused for host scanning only because
its gRPC protocol was shaped entirely around container images.

Capping in-process parallelism (the previous change) made the in-process scan
survivable; this makes the sidecar the default path, with that capped
in-process scan as the permanent fallback.

Protocol. A dedicated ScanHostFilesystem RPC rather than a oneof on
CreateSBOM: the two share no request fields, and a separate RPC lets the
admission redesign land without touching the container path's handler beyond
extracting a shared scan-and-serialize helper. The request is minimized to
{source_name, enable_embedded_sboms, timeout_seconds}; each omission is a
decision. No root_path: the sidecar resolves and validates its OWN HOST_ROOT,
failing with FailedPrecondition if it does not look like a node root, rather
than silently returning a structurally valid SBOM of the wrong filesystem.
No exclude_globs: compiled into both processes. No max_sbom_size: the size
gate stays client-side so the state machine is path-independent. No
tool_version in the response: both binaries resolve the same Syft version from
the same go.mod, so Spec.Metadata.Tool.Version is stamped from node-agent's own
version on both paths.

Admission. The sidecar's sync.Mutex becomes a semaphore.Weighted(1) shared by
both RPCs, acquired with a separate 45s admission context distinct from the
scan's own deadline. Acquiring on the scan deadline would be identical to the
mutex and bound nothing; TryAcquire alone would regress the container path's
shipped "blocks, eventually succeeds" behaviour into spurious failures.
Exhausting the window returns ResourceExhausted/ErrScannerBusy, which is never
a scan failure on either caller -- nothing was dispatched.

Busy is retried under one bounded policy (5s doubling to 60s, +/-20% jitter,
5 attempts, cancelled on shutdown). On the container path via a timer that
re-Submits to the worker pool -- not a sleep inside its single worker, which
would head-of-line-block every other container -- and not via pendingScans,
whose drain condition a merely-busy sidecar already satisfies. On the host path
it falls back to the in-process scan for that one cycle, since waiting for the
next tick would be a coverage gap of up to 24h; that is safe precisely because
a busy rejection is pre-dispatch and wasted no work.

A post-dispatch sidecar failure is the opposite case: the scan WAS attempted,
so it is not retried in-cycle, and it is counted on a new host-specific counter
that is separate from and never feeds crashLoopRetries. That separation is
load-bearing -- crashLoopRetries is the only mechanism that can pin an SBOM
into the TooLarge one-way door, and sidecar connectivity failures say nothing
about a document's size.

An oversized host document would otherwise fail as an opaque transport error
and re-walk the host root every interval forever. The sidecar now measures the
serialized document before sending and returns a distinguishable
ErrHostDocumentTooLargeToTransfer carrying the size, which the host path maps
into the same hadContent/TooLarge/Incomplete branch the client-side size gate
uses. That gate is otherwise unchanged; the two paths converge at the
Spec.Syft assignment, not at the scan call.

Also: the sidecar applies the same downward-API parallelism cap to both its
RPCs from its own CPU_LIMIT_MILLIS (chart change in kubescape/helm-charts);
hostSbomOffloadEnabled (default true) disables host offload independently of
SBOM_SCANNER_SOCKET; ReportSBOMScan/ObserveSBOMScanDuration gained a
in_process/sidecar path label; and the doc comments asserting the host "always
scans in-process, never via the sidecar" are corrected, including the
hostTooLargeReleased rationale, which now rests on what actually gates the host
size check rather than on the host having no sidecar.

host_sbom_test.go is deliberately untouched: its size/TooLarge/Incomplete/
hadContent/tool-version tests pass unmodified.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>

* fix: two real regressions found in PR2 sidecar-offload code review

Independent code review of the PR 2 implementation (host SBOM scanning via
the sbom-scanner sidecar) found two HIGH-severity gaps against the plan's
own explicit requirements, plus several MEDIUM findings from the same pass.

HIGH: container image scans were collaterally failed for the full duration
of a host scan. The shared admission semaphore (Weighted(1)) is correct and
intentional -- a host scan and a container scan cannot usefully run
concurrently in a 1 CPU sidecar -- but the container path's busy-retry
ceiling (~2 minutes) was based on the assumption that sidecar busy periods
are short. PR 1's own live-cluster measurement showed a real host scan
takes 15-20 minutes, so every container scan queued behind one would
exhaust its ceiling, fall through to handleGenericFailure/reportFailure,
and eventually be pinned Incomplete -- a genuine Requirement 5 regression
("no regression to the container path") that the host scan itself
introduced. Fixed by raising the ceiling (busyRetryMaxAttempts 5 -> 22,
~19 minutes) to comfortably outlast either scan type's own timeout, with
the rationale corrected in busy_retry.go's doc comment.

HIGH: a pre-dispatch FailedPrecondition/InvalidArgument rejection from
ScanHostFilesystem (a misconfigured HOST_ROOT, or a bad source_name) fell
through to the same generic-failure path as a post-dispatch crash, taking
3 rescan ticks (3 DAYS at the 24h default) to pin the host SBOM Incomplete.
This is exactly the case the plan's own reasoning says is free to fall back
on (no scan work was attempted), yet it got the strictly worse handling.
Fixed with a new ErrScannerHostScanRejected error and hostScanRejected
outcome, routed to the same one-cycle in-process fallback as a busy
rejection, logged at WARN since it is an operator-fixable configuration
defect rather than transient contention.

Also fixed from the same review pass:
- A busy-retry aborted by shutdown was incorrectly counted as a sidecar
  failure (touching hostSidecarFailures and issuing a storage write into a
  manager being torn down). New hostScanAborted outcome routes shutdown to
  a plain return, touching nothing.
- resolveHostRoot used sync.Once, permanently memoizing a single transient
  stat failure (an unsettled mount, a slow CSI-backed /host) for the
  sidecar's entire process lifetime. Now only a SUCCESSFUL resolution is
  memoized; a failure re-validates on every call.
- scanParallelism's NumCPU() fallback was silent. Now logs at WARN,
  mirroring PR 1's own established pattern for the identical failure mode
  (an unnoticed fallback to uncapped parallelism in a CPU-limited
  container) -- relevant here because node-agent's and the sidecar's own
  CPU_LIMIT_MILLIS chart changes ship as two independent PRs.
- busyRetries wasn't cleared on a genuine (non-busy) failure, letting a
  stale count from an earlier busy episode silently halve an image's
  contention tolerance on its next unrelated busy episode. Now cleared in
  both handleGenericFailure and handleScannerCrash (nil-safe, since
  host-only test managers, including the intentionally-frozen
  host_sbom_test.go, don't construct a busyRetries LRU).
- The client's request context for ScanHostFilesystem only covered the
  scan's own timeout, not the server's admission wait -- a scan admitted
  late could be killed client-side as a false DeadlineExceeded before the
  server's own deadline. Now sized to timeout+sbomscanner.AdmissionWindow
  (exported for this purpose). The container path's existing 1-minute
  client/server timeout margin already covers this and needed no change.
- Added Test_ContainerBusy_RetryTimerStopsOnShutdown (the container path's
  timer-cleanup test was missing; the host path already had one) and
  extended the existing host shutdown test to assert no failure counter is
  touched.

Verified: go build/vet/gofmt clean, go test -race -count=1 across all
touched packages clean, go test -count=1 ./... shows only the two known
pre-existing sandbox failures (tracers, validator -- MEMLOCK/eBPF privilege
limitations). The mechanical check (host_sbom_test.go's PR-1-era tests
pass byte-for-byte unmodified, confirming Requirement 3's "identical state
machine regardless of path" was not violated) still holds after these
fixes: zero diff to that file.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>

* fix: default sidecar SBOM scanning to serial without CPU metadata

Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>

* fix: address host offload review findings

Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>

* fix: handle legacy sidecars and align busy retry metrics

Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>

---------

Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
M
Matthias Bertschy committed
43e0f1397c8da4a65e4cf098c15495f204716524
Parent: 91f7672
Committed by GitHub <noreply@github.com> on 9/22/2026, 1:09:03 PM