mirror of
https://tangled.org/evan.jarrett.net/at-container-registry
synced 2026-09-20 01:04:15 +00:00
scanner: reclaim scan directories a killed process left behind
buildOCILayout already removes its scan dir on every error path, and syft.go
defers the stereoscope generator's Cleanup. What neither can do is clean up
after a process that dies mid-scan: the deferred call never runs, and nothing
afterwards ever looks at what was left. Every restart therefore leaks the
in-flight layout and extraction permanently, and a restart is routine — a
deploy is one.
On seamark-hold that reached 8.8 GB of orphaned scan-*, syft-scan-* and
syft-cataloger-* directories under a 20 GB disk, at which point the disk was
97% full and scans began failing on it:
failed to load OCI image: unable to populate layer cache
dir="/var/lib/seamark/scanner/tmp/syft-scan-1546187834/..."
: no space left on device
failed to download layer 5: failed to write blob:
write /var/lib/seamark/scanner/tmp/scan-4160414849/blobs/sha256/...
: no space left on device
The leaked directories cluster at the scanner's restart timestamps, which is
what identifies the killed process rather than the error paths as the source.
Manual removal reclaimed 8.8 GB and took the disk from 97% to 50%.
Startup is where this belongs: it is the one moment the previous process is
known to be gone, and it is immediately after the event that caused the leak.
The sweep runs in WorkerPool.Start after TMPDIR is set and before any worker
can dequeue, so nothing it removes can be work in progress here.
Three constraints shape what it will touch:
- Only the three per-job prefixes, only as direct children, only
directories. The Grype database lives beside the tmp dir at
<parent>/vulndb and go-getter unpacks into grype-dl underneath it; both
are state the scanner needs and neither matches a prefix. The prefixes now
have one definition each, used by both the creator and the sweeper, so
renaming a directory cannot silently take it out of the sweep's scope.
- An age threshold, vuln.sweep_max_age, default 1h. A second scanner sharing
the directory has an in-flight scan-* dir that is minutes old, and
scanner.job_timeout is 8m, so an hour clears both with room to spare. 0
disables the sweep rather than removing a peer's live work.
- Nothing is fatal. A stat or removal failure is a WARN and the sweep moves
on, so a permission problem in the tmp dir cannot keep the scanner from
starting.
The sweep only runs at startup, so a scanner that is killed twice between
deploys carries the first leak until its next restart. That is the tradeoff
for never racing a live peer; a periodic sweep would be the follow-up if
processes ever live long enough for it to matter.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TA9D4DjaLZTvzQ7dJbu4eg
This commit is contained in:
co-authored by
Claude Opus 5
parent
bd208661c6
commit
265e533ad3
@@ -47,3 +47,9 @@ vuln:
|
||||
# reached 561 MiB RSS with the memory limit applied. Images above this are
|
||||
# rejected before any blob is downloaded.
|
||||
max_image_size: 536870912
|
||||
# Reclaim scan directories left behind by a scanner that was killed
|
||||
# mid-scan (a restart or a deploy). Those never run their own cleanup, and
|
||||
# on this host they reached 8.8 GB on a 20 GB disk before scans started
|
||||
# failing with "no space left on device". Above the 8m job timeout, so a
|
||||
# scan in flight elsewhere in the same directory is never touched.
|
||||
sweep_max_age: 1h
|
||||
|
||||
@@ -110,6 +110,7 @@ a YAML file or pure env vars with the `SCANNER_` prefix. Run with
|
||||
| `vuln.db_path` | `SCANNER_VULN_DB_PATH` | `/var/lib/atcr-scanner/vulndb` | Directory for the Grype vulnerability database. |
|
||||
| `vuln.tmp_dir` | `SCANNER_VULN_TMP_DIR` | `/var/lib/atcr-scanner/tmp` | Directory for layer extraction and DB download. Also exported as `TMPDIR`; point it at a large partition, **not** tmpfs. |
|
||||
| `vuln.max_image_size`| `SCANNER_VULN_MAX_IMAGE_SIZE`| `2147483648` (2 GiB) | Max total compressed image size. Larger images are skipped with an error. `0` = no limit. |
|
||||
| `vuln.sweep_max_age`| `SCANNER_VULN_SWEEP_MAX_AGE` | `1h` | Age threshold for the startup sweep of `vuln.tmp_dir`. Leftover `scan-*`, `syft-scan-*` and `syft-cataloger-*` directories older than this are removed before workers start; a scan killed by a restart never cleans up after itself. Keep it above the longest scan so a second scanner sharing the directory keeps its live work. `0` disables the sweep. |
|
||||
| `server.addr` | `SCANNER_SERVER_ADDR` | `:9090` | Listen address for the scanner's health endpoint. |
|
||||
|
||||
Both `hold.url` and `hold.secret` are required; `LoadConfig` errors out if either is
|
||||
@@ -420,6 +421,11 @@ admin endpoint `POST /admin/api/scan-backfill` instead.
|
||||
- **Layer extraction or Grype DB download fails mid-process.** `vuln.tmp_dir` is too
|
||||
small or on tmpfs. Point it at a large persistent partition; the scanner sets
|
||||
`TMPDIR` to this directory.
|
||||
- **`vuln.tmp_dir` fills up with `scan-*` / `syft-scan-*` directories.** These are
|
||||
scans the process was killed in the middle of, which never ran their own cleanup.
|
||||
The startup sweep reclaims them on the next restart; if it is not doing so, check
|
||||
that `vuln.sweep_max_age` is not `0` and look for `Swept scanner tmp dir` in the
|
||||
startup logs.
|
||||
- **SBOM present but no vulnerability counts.** `vuln.enabled` is false on the scanner,
|
||||
or the Grype DB failed to initialize (check startup logs).
|
||||
- **Helm/attestation artifacts show "scanning isn't applied".** Expected — these are
|
||||
|
||||
@@ -68,6 +68,13 @@ type VulnConfig struct {
|
||||
|
||||
// Maximum total compressed image size in bytes. Images exceeding this are skipped. 0 = no limit.
|
||||
MaxImageSize int64 `yaml:"max_image_size" comment:"Maximum total compressed image size in bytes. 0 = no limit. Default: 2 GiB."`
|
||||
|
||||
// Age threshold for the startup sweep of tmp_dir. A scan killed mid-flight
|
||||
// (a restart, a deploy) never runs its own cleanup, so its layout and
|
||||
// extraction directories stay on disk forever; the sweep reclaims the ones
|
||||
// older than this. It must stay well above the longest a scan can take, so
|
||||
// that a second scanner sharing the directory keeps its live work.
|
||||
SweepMaxAge time.Duration `yaml:"sweep_max_age" comment:"Age threshold for the startup sweep of stale scan directories in tmp_dir (scan-*, syft-scan-*, syft-cataloger-*), which a scan interrupted by a restart leaves behind. Must stay above the longest scan so a second scanner sharing the directory keeps its live work. 0 disables the sweep. Default: 1h."`
|
||||
}
|
||||
|
||||
// setScannerDefaults registers all default values on the given Viper instance.
|
||||
@@ -92,6 +99,7 @@ func setScannerDefaults(v *viper.Viper) {
|
||||
v.SetDefault("vuln.db_path", "/var/lib/atcr-scanner/vulndb")
|
||||
v.SetDefault("vuln.tmp_dir", "/var/lib/atcr-scanner/tmp")
|
||||
v.SetDefault("vuln.max_image_size", 2*1024*1024*1024) // 2 GiB
|
||||
v.SetDefault("vuln.sweep_max_age", "1h")
|
||||
|
||||
// Log shipper defaults
|
||||
v.SetDefault("log_shipper.batch_size", 100)
|
||||
|
||||
@@ -62,7 +62,7 @@ type ociIndex struct {
|
||||
// waiting out the HTTP client's own per-request timeout, and every exit from
|
||||
// here removes the scan directory — the deadline path included.
|
||||
func buildOCILayout(ctx context.Context, job *scanner.ScanJob, tmpDir, secret string, maxBytes int64) (string, func(), error) {
|
||||
scanDir, err := os.MkdirTemp(tmpDir, "scan-*")
|
||||
scanDir, err := os.MkdirTemp(tmpDir, scanDirPrefix+"*")
|
||||
if err != nil {
|
||||
return "", nil, fmt.Errorf("failed to create temp directory: %w", err)
|
||||
}
|
||||
|
||||
@@ -0,0 +1,130 @@
|
||||
package scan
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"io/fs"
|
||||
"log/slog"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"time"
|
||||
)
|
||||
|
||||
// The three directory shapes a scan leaves directly under the configured tmp
|
||||
// dir. Everything created here is per-job and disposable:
|
||||
//
|
||||
// scan-* buildOCILayout's downloaded OCI layout
|
||||
// syft-scan-* stereoscope's extraction root (syftTempDirName)
|
||||
// syft-cataloger-* Syft's own cataloging scratch, created inside itself
|
||||
//
|
||||
// Only these are ever swept. The Grype database lives beside the tmp dir at
|
||||
// <parent>/vulndb and go-getter unpacks its downloads into a grype-dl
|
||||
// directory underneath it; both are state the scanner needs and neither
|
||||
// matches a prefix here.
|
||||
const (
|
||||
scanDirPrefix = "scan-"
|
||||
// syftTempDirName is the root prefix handed to stereoscope's
|
||||
// TempDirGenerator in syft.go, which appends "-" and a random suffix.
|
||||
syftTempDirName = "syft-scan"
|
||||
// syftCatalogerPrefix is Syft's, not ours: it opens its own generator
|
||||
// during cataloging. Named here because the sweep is the only thing that
|
||||
// reclaims one after a crash.
|
||||
syftCatalogerPrefix = "syft-cataloger-"
|
||||
)
|
||||
|
||||
// sweepPrefixes is the full set, defined once so the creators above and the
|
||||
// sweeper below cannot drift apart.
|
||||
var sweepPrefixes = []string{scanDirPrefix, syftTempDirName + "-", syftCatalogerPrefix}
|
||||
|
||||
// sweepTmpDir removes scan leftovers older than maxAge from tmpDir.
|
||||
//
|
||||
// The per-job error paths already clean up after themselves; what they cannot
|
||||
// do is clean up after a process that dies mid-scan. A restart — a deploy
|
||||
// included — permanently orphans whatever was in flight, and on seamark-hold
|
||||
// that reached 8.8 GB on a 20 GB disk before scans started failing with "no
|
||||
// space left on device". Nothing else ever reclaims these, so startup does.
|
||||
//
|
||||
// Only direct children matching sweepPrefixes are considered, and only
|
||||
// directories: the tmp dir is shared with state the scanner needs. maxAge is
|
||||
// what makes this safe to run while another scanner shares the directory —
|
||||
// its in-flight scan-* dir is minutes old, not hours. A maxAge of zero or less
|
||||
// disables the sweep entirely rather than removing a peer's live work.
|
||||
//
|
||||
// Nothing here is fatal. Every failure is logged and the sweep moves on, so a
|
||||
// permission problem in the tmp dir cannot stop the scanner from starting.
|
||||
func sweepTmpDir(tmpDir string, maxAge time.Duration) {
|
||||
if tmpDir == "" || maxAge <= 0 {
|
||||
return
|
||||
}
|
||||
|
||||
entries, err := os.ReadDir(tmpDir)
|
||||
if err != nil {
|
||||
// A tmp dir that does not exist yet is the normal first-boot state,
|
||||
// not something to warn about.
|
||||
if !errors.Is(err, fs.ErrNotExist) {
|
||||
slog.Warn("Failed to sweep scanner tmp dir", "dir", tmpDir, "error", err)
|
||||
}
|
||||
return
|
||||
}
|
||||
|
||||
cutoff := time.Now().Add(-maxAge)
|
||||
var removed int
|
||||
var reclaimed int64
|
||||
for _, entry := range entries {
|
||||
if !entry.IsDir() || !sweepable(entry.Name()) {
|
||||
continue
|
||||
}
|
||||
info, err := entry.Info()
|
||||
if err != nil {
|
||||
slog.Warn("Failed to stat stale scan directory", "name", entry.Name(), "error", err)
|
||||
continue
|
||||
}
|
||||
if info.ModTime().After(cutoff) {
|
||||
continue
|
||||
}
|
||||
|
||||
// Measured before the removal, because afterwards there is nothing
|
||||
// left to measure. Best effort: a directory that cannot be walked is
|
||||
// still worth removing, it just goes unaccounted for.
|
||||
path := filepath.Join(tmpDir, entry.Name())
|
||||
size := dirSize(path)
|
||||
if err := os.RemoveAll(path); err != nil {
|
||||
slog.Warn("Failed to remove stale scan directory", "dir", path, "error", err)
|
||||
continue
|
||||
}
|
||||
removed++
|
||||
reclaimed += size
|
||||
}
|
||||
|
||||
slog.Info("Swept scanner tmp dir",
|
||||
"dir", tmpDir,
|
||||
"removed", removed,
|
||||
"bytes", reclaimed,
|
||||
"older_than", maxAge)
|
||||
}
|
||||
|
||||
// sweepable reports whether a directory name is one of ours.
|
||||
func sweepable(name string) bool {
|
||||
for _, prefix := range sweepPrefixes {
|
||||
if strings.HasPrefix(name, prefix) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// dirSize sums the file sizes under path, ignoring anything it cannot read.
|
||||
// It exists only to put a number in the sweep's log line.
|
||||
func dirSize(path string) int64 {
|
||||
var total int64
|
||||
_ = filepath.WalkDir(path, func(_ string, d fs.DirEntry, err error) error {
|
||||
if err != nil || d.IsDir() {
|
||||
return nil //nolint:nilerr // an unreadable subtree is skipped, not fatal
|
||||
}
|
||||
if info, err := d.Info(); err == nil {
|
||||
total += info.Size()
|
||||
}
|
||||
return nil
|
||||
})
|
||||
return total
|
||||
}
|
||||
@@ -0,0 +1,148 @@
|
||||
package scan
|
||||
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
// staleAge is comfortably past the maxAge every case below sweeps with, so a
|
||||
// "stale" fixture is stale regardless of filesystem timestamp granularity.
|
||||
const staleAge = 3 * time.Hour
|
||||
|
||||
// mkStaleDir creates a directory under parent holding one file of the given
|
||||
// size, then backdates it so the sweep sees it as old.
|
||||
func mkStaleDir(t *testing.T, parent, name string, size int, age time.Duration) string {
|
||||
t.Helper()
|
||||
|
||||
path := filepath.Join(parent, name)
|
||||
if err := os.MkdirAll(path, 0o755); err != nil {
|
||||
t.Fatalf("mkdir %s: %v", path, err)
|
||||
}
|
||||
if size > 0 {
|
||||
if err := os.WriteFile(filepath.Join(path, "blob"), make([]byte, size), 0o644); err != nil {
|
||||
t.Fatalf("write blob in %s: %v", path, err)
|
||||
}
|
||||
}
|
||||
when := time.Now().Add(-age)
|
||||
if err := os.Chtimes(path, when, when); err != nil {
|
||||
t.Fatalf("chtimes %s: %v", path, err)
|
||||
}
|
||||
return path
|
||||
}
|
||||
|
||||
// TestSweepTmpDirRemovesOnlyStaleScanDirs is the whole contract of the sweep:
|
||||
// the three shapes a dead scan leaves behind go, and everything else in the
|
||||
// tmp dir stays. The preserved entries are not hypothetical — the Grype
|
||||
// database lives at <parent>/vulndb and go-getter unpacks into grype-dl under
|
||||
// the tmp dir, so a sweep that took either would cost a full DB download on
|
||||
// every restart.
|
||||
func TestSweepTmpDirRemovesOnlyStaleScanDirs(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
dir string
|
||||
age time.Duration
|
||||
wantGone bool
|
||||
}{
|
||||
{name: "orphaned OCI layout", dir: "scan-1234567890", age: staleAge, wantGone: true},
|
||||
{name: "orphaned stereoscope extraction", dir: "syft-scan-1546187834", age: staleAge, wantGone: true},
|
||||
{name: "orphaned syft cataloging scratch", dir: "syft-cataloger-99", age: staleAge, wantGone: true},
|
||||
{name: "scan a peer may still be running", dir: "scan-inflight", age: time.Minute},
|
||||
{name: "grype download dir", dir: "grype-dl", age: staleAge},
|
||||
{name: "vulndb alongside the tmp dir", dir: "vulndb", age: staleAge},
|
||||
{name: "unrelated directory", dir: "cache", age: staleAge},
|
||||
}
|
||||
|
||||
tmpDir := t.TempDir()
|
||||
for _, tt := range tests {
|
||||
mkStaleDir(t, tmpDir, tt.dir, 512, tt.age)
|
||||
}
|
||||
// A file, not a directory, that matches a swept prefix. Only directories
|
||||
// are ours; a stray file is somebody else's business.
|
||||
stray := filepath.Join(tmpDir, "scan-notes.txt")
|
||||
if err := os.WriteFile(stray, []byte("hello"), 0o644); err != nil {
|
||||
t.Fatalf("write %s: %v", stray, err)
|
||||
}
|
||||
when := time.Now().Add(-staleAge)
|
||||
if err := os.Chtimes(stray, when, when); err != nil {
|
||||
t.Fatalf("chtimes %s: %v", stray, err)
|
||||
}
|
||||
|
||||
sweepTmpDir(tmpDir, time.Hour)
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
_, err := os.Stat(filepath.Join(tmpDir, tt.dir))
|
||||
switch {
|
||||
case tt.wantGone && err == nil:
|
||||
t.Errorf("%s survived the sweep; nothing else ever reclaims it", tt.dir)
|
||||
case !tt.wantGone && err != nil:
|
||||
t.Errorf("%s was removed by the sweep: %v", tt.dir, err)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
if _, err := os.Stat(stray); err != nil {
|
||||
t.Errorf("scan-notes.txt was removed by the sweep: %v", err)
|
||||
}
|
||||
if _, err := os.Stat(tmpDir); err != nil {
|
||||
t.Errorf("the tmp dir itself was removed: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestSweepTmpDirDisabledAndMissing covers the two ways the sweep is asked to
|
||||
// do nothing. Neither may touch anything, and neither may fail: this runs on
|
||||
// the startup path, before any worker exists to report an error.
|
||||
func TestSweepTmpDirDisabledAndMissing(t *testing.T) {
|
||||
t.Run("zero max age disables the sweep", func(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
stale := mkStaleDir(t, tmpDir, "scan-old", 16, staleAge)
|
||||
|
||||
sweepTmpDir(tmpDir, 0)
|
||||
|
||||
if _, err := os.Stat(stale); err != nil {
|
||||
t.Errorf("sweep ran with max age 0 and removed %s: %v", stale, err)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("tmp dir does not exist", func(t *testing.T) {
|
||||
sweepTmpDir(filepath.Join(t.TempDir(), "never-created"), time.Hour)
|
||||
})
|
||||
|
||||
t.Run("empty tmp dir path", func(t *testing.T) {
|
||||
sweepTmpDir("", time.Hour)
|
||||
})
|
||||
}
|
||||
|
||||
// TestSweepTmpDirContinuesPastAnUnremovableEntry is the "never fatal" half of
|
||||
// the contract. One directory the process cannot remove must not cost the
|
||||
// sweep the rest of the reclaim, and must not stop the scanner from starting.
|
||||
func TestSweepTmpDirContinuesPastAnUnremovableEntry(t *testing.T) {
|
||||
if os.Geteuid() == 0 {
|
||||
t.Skip("root ignores the directory permissions this test relies on")
|
||||
}
|
||||
|
||||
tmpDir := t.TempDir()
|
||||
|
||||
// A stale scan dir made read-only, so RemoveAll cannot unlink what is
|
||||
// inside it and fails partway through, the way a root-owned leftover or a
|
||||
// busy mount would.
|
||||
locked := mkStaleDir(t, tmpDir, "scan-locked", 32, staleAge)
|
||||
if err := os.Chmod(locked, 0o555); err != nil {
|
||||
t.Fatalf("chmod %s: %v", locked, err)
|
||||
}
|
||||
// Restore write permission or t.TempDir's own cleanup fails too.
|
||||
t.Cleanup(func() { _ = os.Chmod(locked, 0o755) })
|
||||
|
||||
removable := mkStaleDir(t, tmpDir, "syft-scan-removable", 32, staleAge)
|
||||
|
||||
sweepTmpDir(tmpDir, time.Hour)
|
||||
|
||||
if _, err := os.Stat(locked); err != nil {
|
||||
t.Errorf("expected the unremovable dir to still be there, got: %v", err)
|
||||
}
|
||||
if _, err := os.Stat(removable); err == nil {
|
||||
t.Error("the sweep stopped at the unremovable entry instead of continuing")
|
||||
}
|
||||
}
|
||||
@@ -29,7 +29,7 @@ func generateSBOM(ctx context.Context, ociLayoutDir, sourceRef string) (*sbom.SB
|
||||
slog.Info("Generating SBOM with Syft", "ociLayout", ociLayoutDir, "source", sourceRef)
|
||||
|
||||
// Create stereoscope OCI directory provider
|
||||
tmpGen := file.NewTempDirGenerator("syft-scan")
|
||||
tmpGen := file.NewTempDirGenerator(syftTempDirName)
|
||||
defer tmpGen.Cleanup()
|
||||
|
||||
provider := oci.NewDirectoryProvider(tmpGen, ociLayoutDir)
|
||||
|
||||
@@ -61,6 +61,12 @@ func (wp *WorkerPool) Start(ctx context.Context) {
|
||||
os.Setenv("TMPDIR", wp.cfg.Vuln.TmpDir)
|
||||
}
|
||||
|
||||
// Reclaim what earlier processes left behind before any worker starts
|
||||
// filling the directory again. A scan that is interrupted by a restart
|
||||
// never runs its own cleanup, so without this every restart leaks the
|
||||
// in-flight layout and extraction permanently.
|
||||
sweepTmpDir(wp.cfg.Vuln.TmpDir, wp.cfg.Vuln.SweepMaxAge)
|
||||
|
||||
// Initialize vuln database on startup if enabled
|
||||
if wp.cfg.Vuln.Enabled {
|
||||
go func() {
|
||||
|
||||
Reference in New Issue
Block a user