Harden the vulnerability refresh pipeline against long-running scans - #228
Merged
Conversation
Add the Vulnerabilities:MaxParallelScans option (validated 1..8, default 1) and process image versions in batches of that size, running only the provider calls in parallel while every database interaction stays on a single flow. Raise the vulnerability request timeout default from 30 to 120 seconds because first-time Trivy scans of large images regularly exceed the shorter window. Let the scan cleanup mark abandoned Running scan runs as Failed on startup and on every cleanup cycle, using twice the run type interval with a floor of 24 hours as the stale threshold. Closes #170
Add ScheduledBackgroundServiceTests together with a test double that drives the scheduled loop, covering the startup hook ordering, the logged startup failure, and the cancellation path during shutdown. Extend the scan cleanup tests with abandoned observed image and untyped runs and with a refresh interval whose doubled value exceeds the minimum threshold of 24 hours. All lines and branches added by this branch are covered now.
|
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
Vulnerabilities:MaxParallelScans(default1, validated1..8, added toappsettings.json) and process image versions in batches of that size: the provider calls of a batch run concurrently throughTask.WhenAll, while loading findings, upserting, andSaveChangesAsyncstay strictly sequential on one flow. The value is additionally clamped to1..8at runtime.Vulnerabilities:RequestTimeoutSecondsdefault from30to120inVulnerabilityOptions,appsettings.json,README.md, andDOCKER.md. The validator range of1..300is unchanged.Runningscan runs that never completed asFailedinScanCleanupBackgroundService, settingCompletedAtUtcand an explanatoryErrorMessage, and log the repaired count under a new event id2021.ScheduledBackgroundService.ExecuteStartupAsync, a virtual, exception-guarded startup hook that runs before the scheduled loop independently ofShouldExecuteImmediately. The cleanup service uses it so abandoned runs are repaired right after a restart without pulling the deletion pass onto the startup path.Why
A Trivy scan of a large image can take minutes, and the vulnerability refresh processed every image version strictly sequentially, so one cycle could exceed
Scanning:VulnerabilityRefreshIntervalMinutes. The 30 second request timeout was too tight for first-time scans of large images. When the process crashed mid-run, theScanRunrow stayed in statusRunningforever, because only the normal completion path was covered so far.Linked issues
Closes #170
Review notes
OwnImageBaseScanIntervalMinutes,RuntimeImageUpdateScanIntervalMinutes,VulnerabilityRefreshIntervalMinutes), with a floor of 24 hours. With the default intervals the floor applies to every type, and a deliberately long refresh interval still gets a proportionally longer grace period instead of being cut off at a fixed 24 hours.DockerUpdateGuardDbContextis never touched from a parallel task. OnlyIVulnerabilityProvider.GetVulnerabilitiesAsyncruns concurrently; the image reference is formatted before the task starts and the results are applied one by one afterwards, so the per-imagefinalStatusaggregation keeps the previousPartial/Failedsemantics.MaxParallelScans = 1the batch size is one and the behavior is identical to the previous sequential loop. One difference exists at higher values: a provider exception no longer stops the remaining calls of the same batch, sinceTask.WhenAllobserves all of them before the run is marked as failed.MaxParallelScans = 2viaConcurrencyTrackingVulnerabilityProvider, plus stale-run repair on both the cleanup cycle and the startup step.DOCKER.mddid not document the timeout before, so the two vulnerability keys were added to its JSON path table.Follow-up work
None