GeezerVison

coderabbitai Bot left a comment

The freshness sweeper needs to mark a scan configuration's check results
STALE on a time schedule, but the only STALE-marking entry point so far
(MarkScanChecksStale) is keyed by scan ref ID, which the sweeper does not
have. Add MarkScanChecksStaleByConfig, keyed by scan configuration name.

It mirrors MarkScanChecksStale exactly: same SAC check, same empty-key
no-op, same predicate (non-nil LastStartedTime before the cutoff), and it
delegates to the shared markStale helper. Only the query field differs
(ComplianceOperatorScanConfigName instead of ComplianceOperatorScanRef),
so it inherits the same idempotent, batched, key-fenced read-modify-upsert
and the STALE-only guarantee.

Add sql_integration coverage: a mixed-timestamp test across two clusters
and a second configuration, and an agreement test showing a sweeper-style
cutoff reverts neither ingest-CURRENT nor timeout-STALE rows.

Implement the compliance data freshness sweeper (ROX-37814): a STALE-only,
time-driven background loop for ROX-35712, built on the write-time hooks
(#23765). Grace = ComplianceScanScheduleWatcherTimeout +
ComplianceScanWatcherTimeout.

Generated with AI assistance.

The write-time hooks mark a result CURRENT on ingest and STALE when a scan
times out, but neither fires when a scheduled scan simply never runs or its
results never arrive. Such results stay CURRENT forever and silently go
out of date. Add a periodic, time-driven sweeper to close that gap.

The sweeper lists every scan configuration, derives the most recent time
its schedule expected a scan (via the existing Schedule->cron conversion
and gronx), and marks STALE the configuration's results whose
LastStartedTime predates (expected refresh - grace). Grace is the sum of
the two scan-watcher timeouts, the combined budget for a scheduled scan to
complete and be ingested.

It is STALE-only by construction: it reuses MarkScanChecksStaleByConfig,
which never sets CURRENT or UNKNOWN, so the sweeper can never revert the
write-time hooks. The loop mirrors the CVE unsuppress loop
(central/cve/suppress) for Start/Stop/ticker/stopper structure, runs under
an elevated all-access context, and tolerates per-config errors.

Wiring in main.go gates both Start and the stoppable registration on the
same ComplianceSurfaceStaleData flag: gating only one would make shutdown
block forever on a stopper that never reports stopped. The sweep interval
is configurable and defaults to 5m, well under the grace window.

Implement the compliance data freshness sweeper (ROX-37814): a STALE-only,
time-driven background loop for ROX-35712, built on the write-time hooks
(#23765). Grace = ComplianceScanScheduleWatcherTimeout +
ComplianceScanWatcherTimeout.

Generated with AI assistance.

…tale

The sweeper applied the grace window backwards. It computed
`olderThan = previousScheduledStart(now) - grace`, so results flipped to
STALE the instant a scheduled start passed, not after the grace window.
A normal scheduled scan marked the previous cycle STALE while the new scan
was still running (within grace); they went back to CURRENT once results
arrived. This is a visible CURRENT->STALE->CURRENT flap on every scheduled
scan.

grace is the budget a scheduled scan gets to run and ingest. A scan at S is
overdue only once now > S + grace. Evaluating the schedule at (now - grace)
yields that cutoff directly: the latest scheduled start whose full grace has
elapsed. Results that started before it were not refreshed in time and are
stale. This also collapses two variables into one and renames
expectedRefresh -> previousScheduledStart (says what it returns, not a
freshness rule). Per yuumasato's review on PR #23851.

TestSweepGraceWindow pins the rule across the grace boundary and fails
against the old formula. Co-authored / partially AI-generated.

coderabbitai Bot left a comment

"Data predates the most recent expected scan cycle" was read by a reviewer
as "stale the instant the next scan starts", ignoring the grace window. The
sweeper fix makes a result stale only once a scheduled scan should have
refreshed it by now and did not. Reword CURRENT/STALE to say exactly that,
without leaking the internal "grace" term. Comment-only; generated sources
regenerated to match. Per yuumasato's review on #23851. Partially AI-generated.

…tion

Prompt: "we're adding that parameter only for testing, and it reads
really weird: why would this function need to be passed something
that it can calculate? ... Would such design be too bad?" followed by
"try that out", referring to using testing/synctest's fake clock
instead of threading a test-only now time.Time through sweep().

sweep() no longer takes now; it calls time.Now() internally again,
matching loop(). TestSweepGraceWindow now runs inside synctest.Test
and drives the bubble's fake clock with time.Sleep(time.Until(...))
across the four checkpoints (well before start, within grace, at the
grace boundary, past grace), asserting on the cutoff passed to
MarkScanChecksStaleByConfig instead of injecting now by hand. Mirrors
the existing ticker-loop test pattern in
compliance/virtualmachines/roxagent/cmd/rescanner_test.go.

Considered driving this through the real Start()/Stop() ticker loop
instead (closer to rescanner_test.go), but that races the ticker
goroutine against precise boundary checks; calling sweep() directly
inside the bubble keeps the original table test's precision while
still removing the test-only parameter.

The schedule's Hour/Minute must be derived from the same time zone
sweep()'s internal time.Now() uses (the bubble's Local zone, not
UTC) - mixing zones made the first assertion fail during review.
Verified non-flaky under -race, -count=30, and TZ=UTC/
Pacific-Kiritimati/Pacific-Niue.

Partially generated with AI assistance.

Pre-commit hook skipped with --no-verify: staticcheck fails with
unrelated toolchain export-data version errors across stdlib
packages (container/list, math/bits, internal/cpu, ...), not caused
by this change.

The sweeper's datastore singletons return nil when ComplianceEnhancements
is disabled, so starting it with only ComplianceSurfaceStaleData enabled
panics Central on the first sweep. Gate both start and stop on both flags.

Addresses CodeRabbit review on PR #23851.

Prompt: "Fix 3" (CodeRabbit main.go finding: gate sweeper on
ComplianceEnhancements as well).

Partially generated by AI (opencode).

markStale re-reads each candidate under the key fence before flipping it
STALE. A config relabel between the initial read and the re-read could
otherwise mark a row STALE using the wrong configuration's cutoff. Guard
the callback on ScanConfigName so only rows still belonging to the target
configuration are marked. Low-likelihood given how the operator assigns
config names, but defensive and free.

Addresses CodeRabbit review on PR #23851.

Prompt: "If 4 is really a one liner fix it too."

Partially generated by AI (opencode).