Skip to content

Commit 375dbba

Browse files
committed
cache-server: bound upload sizes so one job can't fill the disk
uploadSingle streamed the request body straight to disk with no limit, and --max-size was only consulted when an entry finalized, so a single upload (or a wave of never-finalized ones) could write far past the cap and fill the host filesystem the per-VM rootfs also lives on. Add a per-entry cap (--max-entry-size, default 10GB) enforced mid-stream via http.MaxBytesReader on both the single-shot and staged-block paths; an over-cap upload now gets a 413 and its partial blob is removed. Track staged (reserved-but-unfinalized) bytes per entry, bound each staged block by the entry's remaining budget, and count in-flight bytes toward --max-size in evictLRU so concurrent uploads can't collectively blow past the total cap. Assemble the block list into a temp file and rename it over the blob only once it is complete and within the cap — previously commitBlockList truncated the live blob before checking that every staged block existed, so a bad commit could destroy a finalized entry.
1 parent 4d6e8d2 commit 375dbba

3 files changed

Lines changed: 206 additions & 19 deletions

File tree

‎cmd/firerunner/main.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,13 +112,18 @@ func cacheServe(args []string) error {
112112
addr := fs.String("addr", "127.0.0.1:8099", "listen address (bind to a guest-facing gateway IP for microVM access; the server is unauthenticated so never expose it on a public interface)")
113113
dir := fs.String("dir", "/var/lib/firerunner/cache", "cache storage directory")
114114
maxSize := fs.String("max-size", "50GB", "evict least-recently-used entries above this total size (e.g. 50GB, 0 for unlimited)")
115+
maxEntry := fs.String("max-entry-size", "10GB", "refuse any single cache entry larger than this (e.g. 10GB, 0 for unlimited)")
115116
if err := fs.Parse(args); err != nil {
116117
return err
117118
}
118119
maxBytes, err := parseSize(*maxSize)
119120
if err != nil {
120121
return fmt.Errorf("--max-size: %w", err)
121122
}
123+
maxEntryBytes, err := parseSize(*maxEntry)
124+
if err != nil {
125+
return fmt.Errorf("--max-entry-size: %w", err)
126+
}
122127
log := config.NewLogger("info", "text")
123128
slog.SetDefault(log)
124129

@@ -127,6 +132,7 @@ func cacheServe(args []string) error {
127132
return err
128133
}
129134
srv.SetMaxSize(maxBytes)
135+
srv.SetMaxEntrySize(maxEntryBytes)
130136
stopJanitor := srv.StartJanitor()
131137
defer stopJanitor()
132138

@@ -215,6 +221,7 @@ cache-server flags:
215221
this unauthenticated server on a public interface)
216222
--dir string cache storage directory (default "/var/lib/firerunner/cache")
217223
--max-size string evict LRU entries above this total size (default "50GB"; 0 = unlimited)
224+
--max-entry-size string refuse any single entry larger than this (default "10GB"; 0 = unlimited)
218225
`)
219226
}
220227

‎internal/cacheserver/cacheserver.go‎

Lines changed: 135 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ import (
5151
"encoding/hex"
5252
"encoding/json"
5353
"encoding/xml"
54+
"errors"
5455
"fmt"
5556
"io"
5657
"log/slog"
@@ -95,10 +96,12 @@ type Server struct {
9596
log *slog.Logger
9697
mux *http.ServeMux
9798

98-
mu sync.Mutex
99-
entries map[uint64]*Entry
100-
nextID uint64
101-
maxSize int64 // 0 = unlimited; total completed-blob bytes to keep
99+
mu sync.Mutex
100+
entries map[uint64]*Entry
101+
nextID uint64
102+
maxSize int64 // 0 = unlimited; total completed-blob bytes to keep
103+
maxEntry int64 // 0 = unlimited; hard cap on a single entry's blob
104+
staged map[uint64]int64 // bytes written so far for a not-yet-finalized entry
102105

103106
// counters (under mu) for observability via /stats and /metrics.
104107
hits uint64
@@ -143,6 +146,23 @@ func (s *Server) SetMaxSize(n int64) {
143146
s.mu.Unlock()
144147
}
145148

149+
// SetMaxEntrySize caps the size of any single cache entry's blob. Uploads that
150+
// exceed it are refused (413) mid-stream rather than after the fact, so one job
151+
// cannot fill the host disk with a single unbounded upload. A non-positive value
152+
// means unlimited. Call before serving.
153+
func (s *Server) SetMaxEntrySize(n int64) {
154+
s.mu.Lock()
155+
s.maxEntry = n
156+
s.mu.Unlock()
157+
}
158+
159+
// entryCap returns the effective per-entry byte cap (0 = unlimited).
160+
func (s *Server) entryCap() int64 {
161+
s.mu.Lock()
162+
defer s.mu.Unlock()
163+
return s.maxEntry
164+
}
165+
146166
// New opens (creating if needed) a cache store rooted at dir and returns a
147167
// ready-to-serve Server. The on-disk index is loaded so caches survive restarts.
148168
func New(dir string, log *slog.Logger) (*Server, error) {
@@ -164,6 +184,7 @@ func New(dir string, log *slog.Logger) (*Server, error) {
164184
dir: dir,
165185
log: log.With("module", "cacheserver"),
166186
entries: make(map[uint64]*Entry),
187+
staged: make(map[uint64]int64),
167188
nextID: 1,
168189
}
169190
if err := s.load(); err != nil {
@@ -331,6 +352,7 @@ func (s *Server) handleFinalize(w http.ResponseWriter, r *http.Request) {
331352
e.Size = fi.Size()
332353
e.Complete = true
333354
e.UsedAt = time.Now().Unix()
355+
delete(s.staged, e.ID) // it now counts as a completed blob, not in-flight
334356
s.saves++
335357
s.evictLRU(e.ID)
336358
if err := s.save(); err != nil {
@@ -396,10 +418,19 @@ func (s *Server) handleUpload(w http.ResponseWriter, r *http.Request) {
396418
}
397419

398420
func (s *Server) uploadSingle(w http.ResponseWriter, r *http.Request, e *Entry) {
399-
if err := writeFile(s.blobPath(e.ID), r.Body); err != nil {
400-
http.Error(w, err.Error(), http.StatusInternalServerError)
421+
body := r.Body
422+
if cap := s.entryCap(); cap > 0 {
423+
body = http.MaxBytesReader(w, r.Body, cap)
424+
}
425+
n, err := writeFile(s.blobPath(e.ID), body)
426+
if err != nil {
427+
_ = os.Remove(s.blobPath(e.ID)) // don't leave an oversized/partial blob squatting disk
428+
s.clearStaged(e.ID)
429+
code, msg := uploadStatus(err)
430+
http.Error(w, msg, code)
401431
return
402432
}
433+
s.setStaged(e.ID, n)
403434
w.WriteHeader(http.StatusCreated)
404435
}
405436

@@ -414,10 +445,25 @@ func (s *Server) uploadBlock(w http.ResponseWriter, r *http.Request, e *Entry) {
414445
http.Error(w, err.Error(), http.StatusInternalServerError)
415446
return
416447
}
417-
if err := writeFile(filepath.Join(dir, hex.EncodeToString([]byte(blockID))), r.Body); err != nil {
418-
http.Error(w, err.Error(), http.StatusInternalServerError)
448+
body := r.Body
449+
if cap := s.entryCap(); cap > 0 {
450+
// Bound the block by the entry's remaining budget so staged blocks can't
451+
// accumulate past the per-entry cap before the block list is committed.
452+
remaining := cap - s.stagedBytes(e.ID)
453+
if remaining < 0 {
454+
remaining = 0
455+
}
456+
body = http.MaxBytesReader(w, r.Body, remaining)
457+
}
458+
blockPath := filepath.Join(dir, hex.EncodeToString([]byte(blockID)))
459+
n, err := writeFile(blockPath, body)
460+
if err != nil {
461+
_ = os.Remove(blockPath)
462+
code, msg := uploadStatus(err)
463+
http.Error(w, msg, code)
419464
return
420465
}
466+
s.addStaged(e.ID, n)
421467
w.WriteHeader(http.StatusCreated)
422468
}
423469

@@ -444,28 +490,58 @@ func (s *Server) commitBlockList(w http.ResponseWriter, r *http.Request, e *Entr
444490
return
445491
}
446492

493+
// Assemble into a temp file and rename over the blob only once it is fully
494+
// built and within the per-entry cap. Streaming straight into the live blob
495+
// would truncate a previously finalized entry the instant a commit arrives,
496+
// even if a staged block is missing or the total exceeds the cap.
447497
dir := s.tmpDir(e.ID)
448-
out, err := os.Create(s.blobPath(e.ID))
498+
tmp := s.blobPath(e.ID) + ".tmp"
499+
out, err := os.OpenFile(tmp, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o600)
449500
if err != nil {
450501
http.Error(w, err.Error(), http.StatusInternalServerError)
451502
return
452503
}
453-
defer out.Close()
504+
cap := s.entryCap()
505+
var total int64
454506
for _, id := range ids {
455507
blockPath := filepath.Join(dir, hex.EncodeToString([]byte(id)))
456508
f, err := os.Open(blockPath)
457509
if err != nil {
510+
out.Close()
511+
_ = os.Remove(tmp)
458512
http.Error(w, "missing staged block: "+err.Error(), http.StatusBadRequest)
459513
return
460514
}
461-
_, err = io.Copy(out, f)
515+
n, err := io.Copy(out, f)
462516
f.Close()
517+
total += n
463518
if err != nil {
519+
out.Close()
520+
_ = os.Remove(tmp)
464521
http.Error(w, err.Error(), http.StatusInternalServerError)
465522
return
466523
}
524+
if cap > 0 && total > cap {
525+
out.Close()
526+
_ = os.Remove(tmp)
527+
_ = os.RemoveAll(dir)
528+
s.clearStaged(e.ID)
529+
http.Error(w, "cache entry exceeds max entry size", http.StatusRequestEntityTooLarge)
530+
return
531+
}
532+
}
533+
if err := out.Close(); err != nil {
534+
_ = os.Remove(tmp)
535+
http.Error(w, err.Error(), http.StatusInternalServerError)
536+
return
537+
}
538+
if err := os.Rename(tmp, s.blobPath(e.ID)); err != nil {
539+
_ = os.Remove(tmp)
540+
http.Error(w, err.Error(), http.StatusInternalServerError)
541+
return
467542
}
468543
_ = os.RemoveAll(dir)
544+
s.setStaged(e.ID, total)
469545
w.WriteHeader(http.StatusCreated)
470546
}
471547

@@ -642,7 +718,9 @@ func (s *Server) blobURL(r *http.Request, kind string, id uint64, sig string) st
642718
// the least-recently-used completed entries (oldest UsedAt first). keepID is the
643719
// entry that just triggered the check and is never evicted, so a single blob
644720
// larger than the cap is still served rather than deleting the thing just
645-
// stored. The caller must hold s.mu; it does not save the index (the caller
721+
// stored. In-flight (staged but not-yet-finalized) bytes count toward the total
722+
// so a wave of concurrent uploads cannot blow past the cap before any of them
723+
// finalizes. The caller must hold s.mu; it does not save the index (the caller
646724
// does). A non-positive cap disables eviction.
647725
func (s *Server) evictLRU(keepID uint64) {
648726
if s.maxSize <= 0 {
@@ -654,6 +732,9 @@ func (s *Server) evictLRU(keepID uint64) {
654732
total += e.Size
655733
}
656734
}
735+
for _, n := range s.staged {
736+
total += n
737+
}
657738
for total > s.maxSize {
658739
var victim *Entry
659740
for _, e := range s.entries {
@@ -665,12 +746,13 @@ func (s *Server) evictLRU(keepID uint64) {
665746
}
666747
}
667748
if victim == nil {
668-
break // only the kept entry is left
749+
break // only the kept entry (and in-flight uploads) are left
669750
}
670751
total -= victim.Size
671752
_ = os.Remove(s.blobPath(victim.ID))
672753
_ = os.RemoveAll(s.tmpDir(victim.ID))
673754
delete(s.entries, victim.ID)
755+
delete(s.staged, victim.ID)
674756
s.evictions++
675757
s.log.Info("evicted cache entry (size cap)", "id", victim.ID, "key", victim.Key, "size", victim.Size)
676758
}
@@ -696,6 +778,7 @@ func (s *Server) gc() {
696778
_ = os.Remove(s.blobPath(id))
697779
_ = os.RemoveAll(s.tmpDir(id))
698780
delete(s.entries, id)
781+
delete(s.staged, id)
699782
removed++
700783
}
701784
}
@@ -749,19 +832,52 @@ func (s *Server) save() error {
749832

750833
// --- small helpers ---
751834

752-
func writeFile(path string, r io.Reader) error {
835+
func writeFile(path string, r io.Reader) (int64, error) {
753836
if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil {
754-
return err
837+
return 0, err
755838
}
756839
f, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o600)
757840
if err != nil {
758-
return err
841+
return 0, err
759842
}
760-
if _, err := io.Copy(f, r); err != nil {
843+
n, err := io.Copy(f, r)
844+
if err != nil {
761845
f.Close()
762-
return err
846+
return n, err
763847
}
764-
return f.Close()
848+
return n, f.Close()
849+
}
850+
851+
// uploadStatus maps an upload write error to an HTTP status: an over-cap body
852+
// (http.MaxBytesError) is a 413, anything else a 500.
853+
func uploadStatus(err error) (int, string) {
854+
var mbe *http.MaxBytesError
855+
if errors.As(err, &mbe) {
856+
return http.StatusRequestEntityTooLarge, "cache entry exceeds max entry size"
857+
}
858+
return http.StatusInternalServerError, err.Error()
859+
}
860+
861+
// staged-byte accounting bounds the disk a not-yet-finalized entry occupies.
862+
func (s *Server) setStaged(id uint64, n int64) {
863+
s.mu.Lock()
864+
s.staged[id] = n
865+
s.mu.Unlock()
866+
}
867+
func (s *Server) addStaged(id uint64, n int64) {
868+
s.mu.Lock()
869+
s.staged[id] += n
870+
s.mu.Unlock()
871+
}
872+
func (s *Server) stagedBytes(id uint64) int64 {
873+
s.mu.Lock()
874+
defer s.mu.Unlock()
875+
return s.staged[id]
876+
}
877+
func (s *Server) clearStaged(id uint64) {
878+
s.mu.Lock()
879+
delete(s.staged, id)
880+
s.mu.Unlock()
765881
}
766882

767883
func newToken() string {

‎internal/cacheserver/cacheserver_test.go‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -447,3 +447,67 @@ func TestStorePermsAreTight(t *testing.T) {
447447
}
448448
}
449449
}
450+
451+
// TestMaxEntrySizeRejectsSingleShot verifies a single-shot PUT over the
452+
// per-entry cap is refused mid-stream (413) and leaves no oversized blob behind,
453+
// so one job cannot fill the host disk with an unbounded upload.
454+
func TestMaxEntrySizeRejectsSingleShot(t *testing.T) {
455+
dir := t.TempDir()
456+
s, err := New(dir, nil)
457+
if err != nil {
458+
t.Fatalf("New: %v", err)
459+
}
460+
s.SetMaxEntrySize(1024)
461+
ts := httptest.NewServer(s)
462+
defer ts.Close()
463+
464+
create := twirp(t, ts.URL, "CreateCacheEntry", createReq{Key: "k", Version: "v"})
465+
uploadURL := create["signed_upload_url"].(string)
466+
467+
req, _ := http.NewRequest(http.MethodPut, uploadURL, bytes.NewReader(bytes.Repeat([]byte("x"), 4096)))
468+
req.Header.Set("x-ms-blob-type", "BlockBlob")
469+
resp, err := http.DefaultClient.Do(req)
470+
if err != nil {
471+
t.Fatalf("PUT: %v", err)
472+
}
473+
resp.Body.Close()
474+
if resp.StatusCode != http.StatusRequestEntityTooLarge {
475+
t.Fatalf("status = %d, want 413", resp.StatusCode)
476+
}
477+
if fi, err := os.Stat(s.blobPath(1)); err == nil {
478+
t.Fatalf("oversized blob left on disk: %d bytes", fi.Size())
479+
}
480+
}
481+
482+
// TestMaxEntrySizeRejectsBlockList verifies staged blocks whose committed total
483+
// exceeds the cap are refused and the live blob path is never created.
484+
func TestMaxEntrySizeRejectsBlockList(t *testing.T) {
485+
dir := t.TempDir()
486+
s, err := New(dir, nil)
487+
if err != nil {
488+
t.Fatalf("New: %v", err)
489+
}
490+
s.SetMaxEntrySize(1024)
491+
ts := httptest.NewServer(s)
492+
defer ts.Close()
493+
494+
create := twirp(t, ts.URL, "CreateCacheEntry", createReq{Key: "big", Version: "v"})
495+
uploadURL := create["signed_upload_url"].(string)
496+
497+
// Stage one 4 KiB block, then commit it: assembled total (4096) > cap (1024).
498+
blockID := "blk-0"
499+
stage := uploadURL + "&comp=block&blockid=" + blockID
500+
req, _ := http.NewRequest(http.MethodPut, stage, bytes.NewReader(bytes.Repeat([]byte("y"), 4096)))
501+
resp, err := http.DefaultClient.Do(req)
502+
if err != nil {
503+
t.Fatalf("stage block: %v", err)
504+
}
505+
resp.Body.Close()
506+
// The block itself already exceeds the remaining budget, so it is refused.
507+
if resp.StatusCode != http.StatusRequestEntityTooLarge {
508+
t.Fatalf("stage status = %d, want 413", resp.StatusCode)
509+
}
510+
if _, err := os.Stat(s.blobPath(1)); err == nil {
511+
t.Fatal("blob created despite over-cap upload")
512+
}
513+
}

0 commit comments

Comments
 (0)