ROX-33608: hook VM pipelines to V2 datastores by dashrews78 · Pull Request #19441 · stackrox/stackrox

Base automatically changed from dashrews/vm-v2-scan-datastore-33385 to master

March 17, 2026 10:23

sourcery-ai[bot]

When the feature flag is OFF, both virtualmachines and virtualmachineindex
pipelines write to the v1 datastore (unchanged behavior). When ON, they
write exclusively to the v2 datastore using normalized scan parts.

Adds internaltostorage.VirtualMachineV2() conversion for sensor VM events,
and v1tov2storage.ScanPartsFromV1Scan() to split embedded v1 scans into
normalized v2 records (scan, components, CVEs) with CVSS/NVD extraction.

The v2 datastore singleton returns nil when the flag is off, so the
pipelines branch on whether the v2 store is non-nil rather than checking
the flag directly.

Partially generated by AI.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

@dashrews78

Replace assert.Equal with protoassert.SlicesEqual when comparing
proto message slices to satisfy the golangci-lint proto comparison check.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…add v2 scan path logging

Guard against nil scan after enrichment in the VM index pipeline to
prevent panics when parts.Scan is nil. Add defense-in-depth nil check
in the store layer. Move GuestOSKey/UnknownGuestOS to pkg/virtualmachine
so central and sensor share the same constants instead of hardcoding
"guestOS". Add info-level logs to the v2 scan pipeline and store to
aid debugging scan upsert flow.

Partially generated by AI.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The VM v2 scan upsert path panics with a nil pointer dereference
somewhere inside fullScanReplace, but safe.Run loses the original
stack trace. Adding numbered log statements before each operation
so the last visible step identifies the culprit.

Partially generated by AI.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The nil pointer dereference occurs during CopyFrom of VM components
(step 7). Adding a recover() with debug.Stack() to capture the full
stack trace instead of crashing, so the exact line can be identified.

Partially generated by AI.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The VTProtobuf generated MarshalToSizedBufferVT for the
VirtualMachineComponentV2_TopCvss oneof wrapper has no nil receiver
check. When topCvssFromComponent returned a typed nil pointer
(*VirtualMachineComponentV2_TopCvss)(nil), the oneof interface was
non-nil (typed nil), so the marshal type assertion succeeded and
called MarshalToSizedBufferVT on a nil receiver, panicking at
m.TopCvss.

Fix: match the image v2 pattern (split.go:94-96) — only set the
oneof field when the value is non-zero, leaving the interface as
an untyped nil otherwise. Remove the now-unused topCvssFromComponent
helper. Also remove temporary debug logging and panic recovery
instrumentation.

Also adds EnsureVMExists to the store for lock-free FK satisfaction.

Partially generated by AI.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use EnsureVirtualMachineExists in the VM index pipeline so the minimal VM row is inserted only when missing, preserving existing rich VM fields and keeping FK requirements for scan upserts. Add datastore/store interface support, mock updates, and adapt pipeline tests to the new ensure path.

User request: "Propose some changes so that the minimal VM in the scan pipeline is only executed if the VM does not exist."
Partially generated by AI.

Made-with: Cursor
- Fix dead nil check in store.go fullScanReplace: use direct field
  access (cve.CveBaseInfo == nil) instead of proto getter which always
  returns non-nil, making the guard dead code
- Fix highestFixedBy lexicographic version comparison: add numeric
  segment-aware compareVersionSegments so "1.10.0" > "1.9.10"
- Optimize preferredCvssVersion to single-pass iteration
- Add EnsureVMExists integration tests covering creation, no-clobber
  behavior, and invalid ID rejection

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The rebase introduced ACK/NACK support but the pipeline was not
sending NACKs on storage errors (v1 UpdateVirtualMachineScan, v2
EnsureVirtualMachineExists, v2 UpsertScan). Added sendVMIndexNACK
calls on all three storage error paths for consistency with the
rest of the pipeline.

Fixed three tests (TestRun_SendsACKOnSuccess, NoACKWhenCapabilityMissing,
NACKOnDBError) that were missing Do callbacks to set vm.Scan after
enrichment, causing failures against the nil-scan guard. Updated v2
error propagation tests to verify NACK behavior. Reverted store.go
nil check to use proto getter to satisfy protogetter linter.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>