hostfiles: refuse an over-cap tombstone up front rather than walk it (Paul's ruling, 2026-10-15); only skipped entries are exempt from the path cap
Astra's round 3 found that removing an over-cap tombstone's file left over-cap parent directories, which the scan still refuses. It also found that an ancestor rename during the walk could redirect the unlink. Each round of the deep-removal walk opened a new edge, so Paul ruled to refuse such tombstones instead. The rule, exported as RestorableWithinCap (it replaces LiveEntriesWithinCap): - Live entries and tombstones past the cap are ErrTooDeep. The readers (checkManifest), Open, OpenAt and Commit all apply this. - Skipped entries are exempt, because Restore never touches their path. - Moves' cut preflight calls this rule, relocation's pin already validates, and #250's backfill reports such manifests, so no recovery meets one. StageDelete of a path past the cap is now a no-op. The only such entry a continuable manifest holds is a skipped one, and a tombstone for it would be refused. The inherited skipped entry stays, and the scan and commit go through. Removed: removePastCap, its !unix stub, descendCtx, the deep-walk tests and the relocation deep-tombstone test. Restore's tombstone loop is main's again. Tests: - TestASkippedEntryPastTheCapStaysReadableAndATombstoneDoesNot - TestAScanKeepsAnInheritedSkippedEntryPastTheCap Each fails under its mutation: StageDelete erroring past the cap, or tombstones exempted.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
9 files changed,
+82
−522
internal/host/relocationflush_deeptombstone_linux_test.go+0 −111internal/hostfiles/dir_unix.go+0 −14internal/hostfiles/fix4_linux_test.go+2 −2internal/hostfiles/fix5_linux_test.go+50 −256internal/hostfiles/publisher.go+19 −15internal/hostfiles/restore.go+7 −17internal/hostfiles/restore_other.go+0 −17internal/hostfiles/restore_unix.go+0 −86internal/hostfiles/sync.go+4 −4
diff --git a/internal/host/relocationflush_deeptombstone_linux_test.go b/internal/host/relocationflush_deeptombstone_linux_test.go| deleted file mode 100644 |
| index 3ce322a32..000000000 |
| --- a/internal/host/relocationflush_deeptombstone_linux_test.go |
| +++ /dev/null |
| @@ -1,111 +0,0 @@ |
| -//go:build linux |
| - |
| -package host |
| - |
| -import ( |
| - "context" |
| - "encoding/json" |
| - "os" |
| - "path/filepath" |
| - "strings" |
| - "testing" |
| - "time" |
| - |
| - "github.com/carlosframework/platform/internal/hostclaim" |
| - "github.com/carlosframework/platform/internal/hostfiles" |
| - "github.com/carlosframework/platform/internal/moves" |
| - "github.com/carlosframework/platform/internal/registry" |
| - "github.com/carlosframework/platform/internal/release" |
| -) |
| - |
| -// Astra's review of the hostfiles path cap, round 2: a manifest written |
| -// before the cap may tombstone a path past it whose file still stands at |
| -// the destination. Restore removes it (in one linear walk), so the release |
| -// flush's scan does not refuse the tree (ErrTooDeep) and the flush, which |
| -// EndProbe requires, commits. |
| -func TestReleaseFlushCommitsAfterARestoreOfATombstonePastTheCap(t *testing.T) { |
| - ctx := context.Background() |
| - const host = "shop.example" |
| - holder := &hostclaim.Holder{Location: "instances/" + host + "/inc", Incarnation: "inc-1", Session: 6, Generation: "g6"} |
| - prefix := moves.GenerationFilesPrefix(holder.Location, holder.Generation) |
| - data := release.NewDirStore(t.TempDir()) |
| - reg, err := registry.Open(filepath.Join(t.TempDir(), "registry.db")) |
| - if err != nil { |
| - t.Fatal(err) |
| - } |
| - t.Cleanup(func() { reg.Close() }) |
| - if err := reg.Upsert(registry.Route{Host: host, Kind: "service", Addr: "10.0.0.1:3000", Status: "ready", Hibernate: true}); err != nil { |
| - t.Fatal(err) |
| - } |
| - dir, envDir := t.TempDir(), t.TempDir() |
| - if err := os.WriteFile(filepath.Join(envDir, host+".env"), []byte("A=1\n"), 0o600); err != nil { |
| - t.Fatal(err) |
| - } |
| - |
| - // The destination: a file past the cap, its parent exactly at it. |
| - r, err := os.OpenRoot(dir) |
| - if err != nil { |
| - t.Fatal(err) |
| - } |
| - var parts []string |
| - for n := 0; n < hostfiles.MaxPathBytes/2+1; n++ { |
| - if err := r.Mkdir("d", 0o755); err != nil { |
| - t.Fatal(err) |
| - } |
| - next, err := r.OpenRoot("d") |
| - r.Close() |
| - if err != nil { |
| - t.Fatal(err) |
| - } |
| - r, parts = next, append(parts, "d") |
| - } |
| - if err := r.WriteFile("f", []byte("deep"), 0o600); err != nil { |
| - t.Fatal(err) |
| - } |
| - r.Close() |
| - deep := strings.Join(append(parts, "f"), "/") |
| - |
| - // The inherited manifest: one healthy file and the deep tombstone. |
| - p, err := hostfiles.Open(ctx, data, prefix) |
| - if err != nil { |
| - t.Fatal(err) |
| - } |
| - if err := p.Stage(ctx, "healthy.txt", []byte("fine"), 0o644, false); err != nil { |
| - t.Fatal(err) |
| - } |
| - if _, err := p.Commit(ctx); err != nil { |
| - t.Fatal(err) |
| - } |
| - m, _, err := hostfiles.Load(ctx, data, prefix) |
| - if err != nil { |
| - t.Fatal(err) |
| - } |
| - m.Seq++ |
| - m.Entries[deep] = hostfiles.Entry{Seq: m.Seq, Deleted: true} |
| - body, err := json.Marshal(m) |
| - if err != nil { |
| - t.Fatal(err) |
| - } |
| - _, etag, err := data.Get(ctx, hostfiles.ManifestKey(prefix)) |
| - if err != nil { |
| - t.Fatal(err) |
| - } |
| - if _, err := data.Put(ctx, hostfiles.ManifestKey(prefix), body, etag); err != nil { |
| - t.Fatal(err) |
| - } |
| - |
| - rctx, cancel := context.WithTimeout(ctx, 5*time.Second) |
| - defer cancel() |
| - if _, st, err := hostfiles.Restore(rctx, data, prefix, dir, nil); err != nil || st.Removed != 1 { |
| - t.Fatalf("Restore: %+v %v", st, err) |
| - } |
| - f := ReleaseFlushFiles{Data: data, Registry: reg, FilesDir: func(string) string { return dir }, EnvPath: func(h string) string { return filepath.Join(envDir, h+".env") }} |
| - seq, err := f.Flush(ctx, hostclaim.Observed{Host: host, Claim: &hostclaim.Claim{Host: host, Holder: holder}}) |
| - if err != nil { |
| - t.Fatalf("the release flush after the restore: %v", err) |
| - } |
| - got, _, err := hostfiles.Load(ctx, data, prefix) |
| - if err != nil || got.Seq != seq || !got.Entries[deep].Deleted || got.Entries["healthy.txt"].Deleted { |
| - t.Fatalf("after the flush: seq %d/%d, deep %+v, %v", got.Seq, seq, got.Entries[deep], err) |
| - } |
| -} |
diff --git a/internal/hostfiles/dir_unix.go b/internal/hostfiles/dir_unix.go| index ca0e91df0..4148ab92d 100644 |
| --- a/internal/hostfiles/dir_unix.go |
| +++ b/internal/hostfiles/dir_unix.go |
| @@ -3,7 +3,6 @@ |
| package hostfiles |
| import ( |
| - "context" |
| "errors" |
| "io" |
| "os" |
| @@ -110,13 +109,6 @@ func notADirectory(err error) bool { |
| // reported as gone: publishOne relies on descend never leaving the tree. |
| // descend("") returns d itself, so the caller must not close it. |
| func (d *dirHandle) descend(rel string) (sub *dirHandle, gone bool, err error) { |
| - return d.descendCtx(context.Background(), rel) |
| -} |
| - |
| -// descendCtx is descend, checking ctx before each component, so a walk |
| -// thousands of directories deep (Restore's removal of a tombstone past the |
| -// cap, removePastCap) stops with ctx's error when it is cancelled. |
| -func (d *dirHandle) descendCtx(ctx context.Context, rel string) (sub *dirHandle, gone bool, err error) { |
| if rel == "" { |
| return d, false, nil |
| } |
| @@ -128,12 +120,6 @@ func (d *dirHandle) descendCtx(ctx context.Context, rel string) (sub *dirHandle, |
| } |
| cur, owned := d, false |
| for _, part := range parts { |
| - if err := ctx.Err(); err != nil { |
| - if owned { |
| - cur.Close() |
| - } |
| - return nil, false, err |
| - } |
| betweenLookAndOpen() |
| next, err := cur.openDir(part) |
| if owned { |
diff --git a/internal/hostfiles/fix4_linux_test.go b/internal/hostfiles/fix4_linux_test.go| index 535311978..602e03ed5 100644 |
| --- a/internal/hostfiles/fix4_linux_test.go |
| +++ b/internal/hostfiles/fix4_linux_test.go |
| @@ -77,8 +77,8 @@ func TestTheWritersRefuseAPathPastTheCapAsTheReadersDo(t *testing.T) { |
| if err := p.StageSkipped(past, "too-large"); !errors.Is(err, ErrTooDeep) { |
| t.Fatalf("StageSkipped past the cap: %v", err) |
| } |
| - // StageDelete takes no cap: it only tombstones an entry the manifest |
| - // holds, and this one holds none, so it is a no-op. |
| + // StageDelete of a path past the cap is a no-op: a tombstone there |
| + // would be one no reader accepts. |
| if err := p.StageDelete(past); err != nil || p.Dirty() && p.man.Entries[past].Deleted { |
| t.Fatalf("StageDelete of a path the manifest never had: %v", err) |
| } |
diff --git a/internal/hostfiles/fix5_linux_test.go b/internal/hostfiles/fix5_linux_test.go| index a394664a6..18e13546e 100644 |
| --- a/internal/hostfiles/fix5_linux_test.go |
| +++ b/internal/hostfiles/fix5_linux_test.go |
| @@ -10,7 +10,6 @@ import ( |
| "os" |
| "path/filepath" |
| "slices" |
| - "strconv" |
| "strings" |
| "testing" |
| "time" |
| @@ -100,9 +99,7 @@ func TestAnInheritedManifestPastTheCapIsRefusedByOpenAndTheReaders(t *testing.T) |
| // Run over a publisher holding such an entry (one continued before Open |
| // refused them) stops on its first commit with one line, never retrying |
| -// at every tick, and commits nothing. The scan would retire the entry if |
| -// its file were gone (StageDelete takes no cap), so the scan here skips |
| -// the path, as it skips one the caller excludes. |
| +// at every tick, and commits nothing. |
| func TestRunStopsOnAManifestItCannotCommit(t *testing.T) { |
| logs := captureLog(t) |
| s, dir := release.NewDirStore(t.TempDir()), t.TempDir() |
| @@ -121,10 +118,8 @@ func TestRunStopsOnAManifestItCannotCommit(t *testing.T) { |
| write(t, dir, "healthy.txt", "changed") // the first scan stages it, and Commit is attempted |
| ctx, cancel := context.WithCancel(context.Background()) |
| defer cancel() |
| - o := polled |
| - o.Skip = func(rel string, _ bool) bool { return rel == deep } |
| done := make(chan error, 1) |
| - go func() { done <- run(ctx, p, dir, o, nil) }() |
| + go func() { done <- run(ctx, p, dir, polled, nil) }() |
| select { |
| case err := <-done: |
| if !errors.Is(err, ErrTooDeep) { |
| @@ -142,24 +137,26 @@ func TestRunStopsOnAManifestItCannotCommit(t *testing.T) { |
| } |
| } |
| -// Tombstones and skipped entries past the cap are exempt (moves' review of |
| -// a8cebad, #236): Restore writes neither, so a manifest written before the |
| -// cap that holds them is still opened, continued, validated, copied and |
| -// restored, and a recovery that selects it is not stranded. The tombstone |
| -// is removed, as any other tombstone is. |
| -func TestATombstoneOrSkippedEntryPastTheCapStaysReadable(t *testing.T) { |
| +// A skipped entry past the cap is exempt (moves' review of a8cebad, #236): |
| +// Restore never touches its path, so a manifest written before the cap |
| +// that holds one is still opened, continued, validated, copied and |
| +// restored, and a recovery that selects it is not stranded. A tombstone |
| +// past the cap is refused like a live entry (RestorableWithinCap): no |
| +// restore could remove it in bounded work, so a move's cut refuses such a |
| +// manifest before anything starts (#236, Paul's ruling of 2026-10-15). |
| +func TestASkippedEntryPastTheCapStaysReadableAndATombstoneDoesNot(t *testing.T) { |
| ctx := context.Background() |
| s, dir := release.NewDirStore(t.TempDir()), t.TempDir() |
| - dead, skipped := pathOfBytes(MaxPathBytes+1), pathOfBytes(MaxPathBytes+2) |
| + skipped := pathOfBytes(MaxPathBytes + 1) |
| inheritPastTheCap(t, s, func(seq int64) map[string]Entry { |
| - return map[string]Entry{dead: {Seq: seq, Deleted: true}, skipped: {Seq: seq, Skipped: "too-large"}} |
| + return map[string]Entry{skipped: {Seq: seq, Skipped: "too-large"}} |
| }) |
| m, _, err := Load(ctx, s, pfx) |
| if err != nil { |
| t.Fatal(err) |
| } |
| - if err := LiveEntriesWithinCap(m); err != nil { |
| - t.Fatalf("LiveEntriesWithinCap: %v", err) |
| + if err := RestorableWithinCap(m); err != nil { |
| + t.Fatalf("RestorableWithinCap: %v", err) |
| } |
| if err := ValidateManifest(m, []string{pfx}); err != nil { |
| t.Fatalf("ValidateManifest: %v", err) |
| @@ -174,28 +171,49 @@ func TestATombstoneOrSkippedEntryPastTheCapStaysReadable(t *testing.T) { |
| if _, err := CopyManifest(ctx, s, pfx, to, 0, nil); err != nil { |
| t.Fatalf("CopyManifest: %v", err) |
| } |
| - got, st, err := Restore(ctx, s, pfx, dir, nil) |
| + _, st, err := Restore(ctx, s, pfx, dir, nil) |
| if err != nil { |
| t.Fatalf("Restore: %v", err) |
| } |
| - if st.Written != 2 || !slices.Equal(st.Skipped, []string{skipped}) || !got.Entries[dead].Deleted { |
| + if st.Written != 2 || !slices.Equal(st.Skipped, []string{skipped}) { |
| t.Fatalf("restore: %+v", st) |
| } |
| if b, err := os.ReadFile(filepath.Join(dir, "healthy.txt")); err != nil || string(b) != "fine" { |
| t.Fatalf("healthy.txt: %q %v", b, err) |
| } |
| - // A live path past the cap is still refused by the same rule. |
| - m.Entries[pathOfBytes(MaxPathBytes+3)] = liveEntry(m.Seq) |
| - if err := LiveEntriesWithinCap(m); !errors.Is(err, ErrTooDeep) { |
| - t.Fatalf("a live path past the cap: %v", err) |
| + // A tombstone past the cap is refused, as a live entry is, by the |
| + // shared rule, every reader and Open, and nothing is touched. |
| + dead := pathOfBytes(MaxPathBytes + 2) |
| + s2 := release.NewDirStore(t.TempDir()) |
| + before := inheritPastTheCap(t, s2, func(seq int64) map[string]Entry { |
| + return map[string]Entry{dead: {Seq: seq, Deleted: true}} |
| + }) |
| + m2, _, err := Load(ctx, s2, pfx) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := RestorableWithinCap(m2); !errors.Is(err, ErrTooDeep) { |
| + t.Fatalf("RestorableWithinCap over a tombstone past the cap: %v", err) |
| + } |
| + if err := ValidateManifest(m2, []string{pfx}); !errors.Is(err, ErrTooDeep) { |
| + t.Fatalf("ValidateManifest: %v", err) |
| + } |
| + if _, err := Open(ctx, s2, pfx); !errors.Is(err, ErrTooDeep) { |
| + t.Fatalf("Open: %v", err) |
| + } |
| + if _, _, err := Restore(ctx, s2, pfx, t.TempDir(), nil); !errors.Is(err, ErrTooDeep) { |
| + t.Fatalf("Restore: %v", err) |
| + } |
| + if after, _, err := s2.Get(ctx, ManifestKey(pfx)); err != nil || !bytes.Equal(before, after) { |
| + t.Fatalf("the inherited manifest was rewritten: %v", err) |
| } |
| } |
| // Astra's review of the exemption (P2 1): a skipped entry past the cap, |
| -// inherited, has no file after a restore, so the next scan retires it |
| -// (StageDelete), and the commit goes through, rather than every scan |
| -// failing on it. |
| -func TestAScanRetiresAnInheritedSkippedEntryPastTheCap(t *testing.T) { |
| +// inherited, has no file after a restore. The next scan's StageDelete of |
| +// it is a no-op rather than an error or a tombstone no reader accepts, so |
| +// the scan and its commit go through and the entry stays skipped. |
| +func TestAScanKeepsAnInheritedSkippedEntryPastTheCap(t *testing.T) { |
| ctx := context.Background() |
| s, dir := release.NewDirStore(t.TempDir()), t.TempDir() |
| skipped := pathOfBytes(MaxPathBytes + 1) |
| @@ -205,6 +223,7 @@ func TestAScanRetiresAnInheritedSkippedEntryPastTheCap(t *testing.T) { |
| if _, _, err := Restore(ctx, s, pfx, dir, nil); err != nil { |
| t.Fatalf("Restore: %v", err) |
| } |
| + write(t, dir, "healthy.txt", "changed") |
| p, err := Open(ctx, s, pfx) |
| if err != nil { |
| t.Fatalf("Open: %v", err) |
| @@ -217,235 +236,10 @@ func TestAScanRetiresAnInheritedSkippedEntryPastTheCap(t *testing.T) { |
| if err != nil { |
| t.Fatal(err) |
| } |
| - if !m.Entries[skipped].Deleted || m.Entries["healthy.txt"].Deleted { |
| - t.Fatalf("after the scan: skipped %+v, healthy %+v", m.Entries[skipped], m.Entries["healthy.txt"]) |
| - } |
| -} |
| - |
| -// deepLevels is how many directories deep the fixtures below go: the |
| -// parent is exactly MaxPathBytes ("d/d/.../d", 4095 bytes), so a walk may |
| -// enter it, and the file in it ("…/f", 4097 bytes) is past the cap. |
| -const deepLevels = MaxPathBytes/2 + 1 |
| - |
| -// deepTree builds levels directories named "d" under dir, one at a time, |
| -// and returns the open root of the last one (the caller closes it) and |
| -// the path to it. |
| -func deepTree(t *testing.T, dir string, levels int) (*os.Root, []string) { |
| - t.Helper() |
| - r, err := os.OpenRoot(dir) |
| - if err != nil { |
| - t.Fatal(err) |
| - } |
| - var parts []string |
| - for n := 0; n < levels; n++ { |
| - if err := r.Mkdir("d", 0o755); err != nil { |
| - t.Fatal(err) |
| - } |
| - next, err := r.OpenRoot("d") |
| - r.Close() |
| - if err != nil { |
| - t.Fatal(err) |
| - } |
| - r, parts = next, append(parts, "d") |
| - } |
| - return r, parts |
| -} |
| - |
| -// readDeep reads name beneath dir/parts..., one directory at a time, as |
| -// a path past PATH_MAX cannot be opened whole. |
| -func readDeep(dir string, parts []string, name string) ([]byte, error) { |
| - r, err := os.OpenRoot(dir) |
| - if err != nil { |
| - return nil, err |
| - } |
| - for _, d := range parts { |
| - next, err := r.OpenRoot(d) |
| - r.Close() |
| - if err != nil { |
| - return nil, err |
| - } |
| - r = next |
| - } |
| - defer r.Close() |
| - return r.ReadFile(name) |
| -} |
| - |
| -// deepTombstone builds the deep fixture at dir, a file "f" past the cap, |
| -// and inherits a manifest at pfx that tombstones it. It returns the |
| -// tombstoned path and its directories. |
| -func deepTombstone(t *testing.T, s release.Store, dir string) (string, []string) { |
| - t.Helper() |
| - r, parts := deepTree(t, dir, deepLevels) |
| - if err := r.WriteFile("f", []byte("deep"), 0o600); err != nil { |
| - t.Fatal(err) |
| - } |
| - r.Close() |
| - deep := strings.Join(append(parts, "f"), "/") |
| - if withinTreeCap(deep) == nil || withinTreeCap(strings.Join(parts, "/")) != nil { |
| - t.Fatalf("the fixture's file is within the cap, or its parent is not: %d bytes", len(deep)) |
| + if e := m.Entries[skipped]; e.Skipped != "too-large" || e.Deleted { |
| + t.Fatalf("the skipped entry after the scan: %+v", e) |
| } |
| - inheritPastTheCap(t, s, func(seq int64) map[string]Entry { |
| - return map[string]Entry{deep: {Seq: seq, Deleted: true}} |
| - }) |
| - return deep, parts |
| -} |
| - |
| -// restoreWithin runs Restore and fails the test if it has not returned |
| -// within limit, rather than waiting for go test's timeout. |
| -func restoreWithin(t *testing.T, ctx context.Context, limit time.Duration, s release.Store, dir string) (RestoreStats, time.Duration, error) { |
| - t.Helper() |
| - type result struct { |
| - st RestoreStats |
| - err error |
| - } |
| - done, start := make(chan result, 1), time.Now() |
| - go func() { |
| - _, st, err := Restore(ctx, s, pfx, dir, nil) |
| - done <- result{st, err} |
| - }() |
| - select { |
| - case r := <-done: |
| - return r.st, time.Since(start), r.err |
| - case <-time.After(limit): |
| - t.Fatalf("Restore was still running after %v", limit) |
| - return RestoreStats{}, 0, nil |
| - } |
| -} |
| - |
| -// Astra's review of the exemption, rounds 1 and 2: a tombstone past the |
| -// cap whose file still stands at the destination is removed, or the next |
| -// scan refuses the whole tree (ErrTooDeep) and nothing ever commits, |
| -// relocation's release flush among them. It is removed in one walk down |
| -// (removePastCap), linear in depth: removeInside's per-prefix walk is |
| -// cubic and takes minutes at this depth. Every directory stays, and |
| -// restore, scan and commit all go through. |
| -func TestRestoreRemovesATombstonePastTheCapInOneWalk(t *testing.T) { |
| - s, dir := release.NewDirStore(t.TempDir()), t.TempDir() |
| - deep, parts := deepTombstone(t, s, dir) |
| - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) |
| - defer cancel() |
| - st, took, err := restoreWithin(t, ctx, 10*time.Second, s, dir) |
| - if err != nil { |
| - t.Fatalf("Restore: %v", err) |
| - } |
| - t.Logf("restore with a %d-byte tombstone %d directories deep: %v", len(deep), len(parts), took) |
| - if st.Removed != 1 || st.Written != 1 { |
| - t.Fatalf("restore: %+v", st) |
| - } |
| - r, err := os.OpenRoot(dir) |
| - if err != nil { |
| - t.Fatal(err) |
| - } |
| - for _, d := range parts { |
| - next, err := r.OpenRoot(d) |
| - r.Close() |
| - if err != nil { |
| - t.Fatalf("a directory of the deep tree went: %v", err) |
| - } |
| - r = next |
| - } |
| - if _, err := r.Lstat("f"); !errors.Is(err, os.ErrNotExist) { |
| - t.Fatalf("the deep file is still there: %v", err) |
| - } |
| - r.Close() |
| - p, err := Open(context.Background(), s, pfx) |
| - if err != nil { |
| - t.Fatalf("Open: %v", err) |
| - } |
| - if _, err := SyncDir(context.Background(), p, dir, nil); err != nil { |
| - t.Fatalf("scan after the restore: %v", err) |
| - } |
| - commit(t, p) |
| - m, _, err := Load(context.Background(), s, pfx) |
| - if err != nil { |
| - t.Fatal(err) |
| - } |
| - if !m.Entries[deep].Deleted || m.Entries["healthy.txt"].Deleted { |
| - t.Fatalf("after the scan: deep %+v, healthy %+v", m.Entries[deep], m.Entries["healthy.txt"]) |
| - } |
| -} |
| - |
| -// The deep walk checks ctx between components: cancelled part way down, |
| -// Restore stops with ctx's error and removes nothing. |
| -func TestRestoreStopsADeepTombstoneWalkWhenCancelled(t *testing.T) { |
| - s, dir := release.NewDirStore(t.TempDir()), t.TempDir() |
| - _, parts := deepTombstone(t, s, dir) |
| - ctx, cancel := context.WithCancel(context.Background()) |
| - defer cancel() |
| - const at = 1000 |
| - calls := 0 |
| - orig := betweenLookAndOpen |
| - t.Cleanup(func() { betweenLookAndOpen = orig }) |
| - betweenLookAndOpen = func() { |
| - calls++ |
| - if calls == at { |
| - cancel() |
| - } |
| - } |
| - _, _, err := restoreWithin(t, ctx, 10*time.Second, s, dir) |
| - if !errors.Is(err, context.Canceled) { |
| - t.Fatalf("Restore with ctx cancelled part way down: %v", err) |
| - } |
| - if calls != at { |
| - t.Fatalf("the walk went on after the cancel: %d components opened, cancelled at %d", calls, at) |
| - } |
| - if b, err := readDeep(dir, parts, "f"); err != nil || string(b) != "deep" { |
| - t.Fatalf("a cancelled restore removed the deep file: %v", err) |
| - } |
| -} |
| - |
| -// The deep walk never follows a symlink: one standing for a directory |
| -// anywhere on the way (the first, the middle, the parent) is "nothing of |
| -// ours here", and the file it points at, outside the root, stays. |
| -func TestRestoreNeverFollowsASymlinkOnADeepTombstone(t *testing.T) { |
| - for _, k := range []int{0, deepLevels / 2, deepLevels - 1} { |
| - t.Run(strconv.Itoa(k), func(t *testing.T) { |
| - s, dir, outside := release.NewDirStore(t.TempDir()), t.TempDir(), t.TempDir() |
| - // The real file the tombstone names, if the link were followed: |
| - // the rest of the path, built outside the root. |
| - r, rest := deepTree(t, outside, deepLevels-k-1) |
| - if err := r.WriteFile("f", []byte("outside"), 0o600); err != nil { |
| - t.Fatal(err) |
| - } |
| - r.Close() |
| - r, parts := deepTree(t, dir, k) |
| - if err := r.Symlink(outside, "d"); err != nil { |
| - t.Fatal(err) |
| - } |
| - r.Close() |
| - deep := strings.Join(append(append(append(parts, "d"), rest...), "f"), "/") |
| - if strings.Count(deep, "/") != deepLevels || withinTreeCap(deep) == nil { |
| - t.Fatalf("the fixture is not the deep path: %d bytes", len(deep)) |
| - } |
| - inheritPastTheCap(t, s, func(seq int64) map[string]Entry { |
| - return map[string]Entry{deep: {Seq: seq, Deleted: true}} |
| - }) |
| - st, _, err := restoreWithin(t, context.Background(), 10*time.Second, s, dir) |
| - if err != nil { |
| - t.Fatalf("Restore: %v", err) |
| - } |
| - if st.Removed != 0 { |
| - t.Fatalf("restore through a symlink: %+v", st) |
| - } |
| - if b, err := readDeep(outside, rest, "f"); err != nil || string(b) != "outside" { |
| - t.Fatalf("the file outside the root: %q %v", b, err) |
| - } |
| - }) |
| - } |
| -} |
| - |
| -// The census ruling for removePastCap (platform-1e, #236) holds it to a |
| -// plain final name, as descendCtx holds each directory component: an |
| -// empty, "." or ".." final name refuses, and nothing is removed. |
| -func TestRemovePastCapRefusesANameThatIsNotPlain(t *testing.T) { |
| - r, err := os.OpenRoot(t.TempDir()) |
| - if err != nil { |
| - t.Fatal(err) |
| - } |
| - defer r.Close() |
| - for _, rel := range []string{"a/", "a/.", "a/..", ".."} { |
| - if removed, err := removePastCap(context.Background(), r, rel); err == nil || removed { |
| - t.Fatalf("removePastCap(%q): %v, removed %v", rel, err, removed) |
| - } |
| + if e := m.Entries["healthy.txt"]; e.Deleted || e.SHA256 != sum([]byte("changed")) { |
| + t.Fatalf("healthy.txt after the scan: %+v", e) |
| } |
| } |
diff --git a/internal/hostfiles/publisher.go b/internal/hostfiles/publisher.go| index 5fc225dcf..517051dc8 100644 |
| --- a/internal/hostfiles/publisher.go |
| +++ b/internal/hostfiles/publisher.go |
| @@ -57,20 +57,21 @@ func Open(ctx context.Context, s release.Store, prefix string) (*Publisher, erro |
| return &Publisher{s: s, prefix: prefix, man: m, etag: etag, next: m.Seq}, nil |
| } |
| -// LiveEntriesWithinCap holds every live entry path of m to the tree's |
| -// length cap (withinTreeCap): the one rule the readers (checkManifest), |
| -// Open, OpenAt and Commit apply, and the check a caller makes before it |
| -// starts work that will need m restored (a move's cut). A tombstone or a |
| -// skipped entry is exempt: Restore writes neither, so a manifest written |
| -// before the cap that holds one past it stays readable and continuable. |
| -// A live entry past the cap is ErrTooDeep: no restore could recreate it. |
| -func LiveEntriesWithinCap(m Manifest) error { return manifestWithinCap(m) } |
| +// RestorableWithinCap holds every live entry and tombstone path of m to |
| +// the tree's length cap (withinTreeCap): the one rule the readers |
| +// (checkManifest), Open, OpenAt and Commit apply, and the check a caller |
| +// makes before it starts work that will need m restored (a move's cut). A |
| +// skipped entry is exempt: Restore never touches its path, so a manifest |
| +// written before the cap that holds one past it stays readable and |
| +// continuable. A live entry or tombstone past the cap is ErrTooDeep: no |
| +// restore could recreate or remove it in bounded work. |
| +func RestorableWithinCap(m Manifest) error { return manifestWithinCap(m) } |
| -// manifestWithinCap is LiveEntriesWithinCap. Open, OpenAt and Commit call |
| +// manifestWithinCap is RestorableWithinCap. Open, OpenAt and Commit call |
| // it by this name, which the root-read and record-write censuses pin. |
| func manifestWithinCap(m Manifest) error { |
| for path, e := range m.Entries { |
| - if e.Deleted || e.Skipped != "" { |
| + if e.Skipped != "" { |
| continue |
| } |
| if err := withinTreeCap(path); err != nil { |
| @@ -174,16 +175,19 @@ func (p *Publisher) StageSkipped(path, why string) error { |
| } |
| // StageDelete records that path is gone. Deleting a path the manifest never |
| -// had, or already has as deleted, is a no-op. It takes no length cap: it |
| -// only tombstones an entry the manifest already holds, and a tombstone is |
| -// exempt (LiveEntriesWithinCap), so a skipped entry past the cap that a |
| -// manifest written before the cap carried can still be retired when its |
| -// file is gone. |
| +// had, or already has as deleted, is a no-op. So is deleting a path past |
| +// the cap: the only such entry a continuable manifest holds is a skipped |
| +// one, written before the cap, and a tombstone for it would be one no |
| +// reader accepts (RestorableWithinCap). It stays as it is; Restore never |
| +// touches a skipped path. |
| func (p *Publisher) StageDelete(path string) error { |
| clean, err := CleanPath(path) |
| if err != nil { |
| return err |
| } |
| + if withinTreeCap(clean) != nil { |
| + return nil |
| + } |
| p.mu.Lock() |
| defer p.mu.Unlock() |
| if cur, ok := p.man.Entries[clean]; !ok || cur.Deleted { |
diff --git a/internal/hostfiles/restore.go b/internal/hostfiles/restore.go| index dc66c05c8..7dd4af4b5 100644 |
| --- a/internal/hostfiles/restore.go |
| +++ b/internal/hostfiles/restore.go |
| @@ -142,19 +142,8 @@ func restore(ctx context.Context, s release.Store, prefix string, origins []stri |
| return dead[i] < dead[j] |
| }) |
| for _, p := range dead { |
| - var removed bool |
| - if withinTreeCap(p) != nil { |
| - // A tombstone past the cap (one a manifest written before the |
| - // cap may hold) is removed in one linear, cancellable walk that |
| - // syncs its own parent, and is never marked: removeInside and |
| - // the directory sync below resolve every prefix afresh, which |
| - // is cubic in depth. Left standing, the file would make the |
| - // next walk refuse the whole tree (ErrTooDeep). |
| - removed, err = removePastCap(ctx, root, p) |
| - } else { |
| - mark(p) |
| - removed, err = removeInside(root, p) |
| - } |
| + mark(p) |
| + removed, err := removeInside(root, p) |
| if err != nil { |
| return Manifest{}, st, fmt.Errorf("hostfiles: remove %s: %w", p, err) |
| } |
| @@ -275,10 +264,11 @@ func checkManifest(m Manifest, origins []string) error { |
| return fmt.Errorf("hostfiles: platform item %q: %w", name, err) |
| } |
| } |
| - // A live path past the cap: no restore could recreate it, so it is |
| - // refused before anything is touched (tombstones and skipped entries |
| - // are exempt; Restore writes neither). |
| - if err := LiveEntriesWithinCap(m); err != nil { |
| + // A live entry or tombstone past the cap: no restore could recreate or |
| + // remove it in bounded work, so it is refused before anything is |
| + // touched (RestorableWithinCap; skipped entries are exempt, Restore |
| + // never touches them). |
| + if err := RestorableWithinCap(m); err != nil { |
| return err |
| } |
| for p, e := range m.Entries { |
diff --git a/internal/hostfiles/restore_other.go b/internal/hostfiles/restore_other.go| deleted file mode 100644 |
| index 4670f3e07..000000000 |
| --- a/internal/hostfiles/restore_other.go |
| +++ /dev/null |
| @@ -1,17 +0,0 @@ |
| -//go:build !unix |
| - |
| -package hostfiles |
| - |
| -import ( |
| - "context" |
| - "errors" |
| - "os" |
| -) |
| - |
| -// removePastCap needs openat and unlinkat to remove a path past the cap in |
| -// one walk (restore_unix.go). The agent runs on Linux; elsewhere a |
| -// tombstone past the cap is refused rather than removed by a walk without |
| -// bound. |
| -func removePastCap(ctx context.Context, root *os.Root, rel string) (bool, error) { |
| - return false, errors.New("hostfiles: removing a tombstone past the path cap is not supported on this OS") |
| -} |
diff --git a/internal/hostfiles/restore_unix.go b/internal/hostfiles/restore_unix.go| deleted file mode 100644 |
| index 8f935e1d4..000000000 |
| --- a/internal/hostfiles/restore_unix.go |
| +++ /dev/null |
| @@ -1,86 +0,0 @@ |
| -//go:build unix |
| - |
| -package hostfiles |
| - |
| -import ( |
| - "context" |
| - "errors" |
| - "os" |
| - "path" |
| - "strconv" |
| - "strings" |
| - |
| - "golang.org/x/sys/unix" |
| -) |
| - |
| -// removePastCap is removeInside for a tombstone past MaxPathBytes, one a |
| -// manifest written before the cap may hold. removeInside resolves every |
| -// prefix of rel afresh (realParents, then os.Root), which is cubic in |
| -// depth; this walks down once instead: each directory is opened beneath |
| -// the one before it with O_NOFOLLOW (descendCtx), so it is O(depth) |
| -// syscalls, no symlink is ever followed and nothing outside root is |
| -// reached, and ctx is checked between components. |
| -// |
| -// The semantics are removeInside's. A missing component, or one that is a |
| -// symlink or anything but a directory, means nothing of ours is there: not |
| -// an error, nothing removed. So is a missing final name. A final name that |
| -// is a directory is left alone. |
| -// |
| -// Durability: Restore's directory sync re-resolves every touched directory |
| -// from the root, which is the same cubic walk, so a path past the cap is |
| -// never marked. Instead the parent, once reached, is fsynced here through |
| -// the descriptor already held, whether or not this call removed anything |
| -// (an earlier attempt may have removed the name and failed before its |
| -// sync). Only the parent's entries change, so when this returns nil the |
| -// removal is durable. |
| -func removePastCap(ctx context.Context, root *os.Root, rel string) (bool, error) { |
| - f, err := root.Open(".") |
| - if err != nil { |
| - return false, err |
| - } |
| - defer f.Close() |
| - top, err := handleOf(f) |
| - if err != nil { |
| - return false, err |
| - } |
| - parent, name := path.Split(rel) |
| - if name == "" || name == "." || name == ".." { |
| - // descendCtx refuses such a directory component; the final name is |
| - // held to the same rule, so only a plain name is ever unlinked. |
| - return false, errors.New("hostfiles: path component " + strconv.Quote(name) + " in " + strconv.Quote(rel) + " is not a plain name") |
| - } |
| - dir, gone, err := top.descendCtx(ctx, strings.TrimSuffix(parent, "/")) |
| - if err != nil || gone { |
| - return false, err |
| - } |
| - if dir != top { |
| - defer dir.Close() |
| - } |
| - if err := ctx.Err(); err != nil { |
| - return false, err |
| - } |
| - removed := false |
| - st, err := dir.look(name) |
| - switch { |
| - case errors.Is(err, unix.ENOENT): |
| - case err != nil: |
| - return false, &os.PathError{Op: "fstatat", Path: name, Err: err} |
| - case isDir(st): |
| - // A directory now: a later entry lives under it, and the tombstone |
| - // is for a file that no longer exists. |
| - default: |
| - err := unix.Unlinkat(int(dir.f.Fd()), name, 0) |
| - switch { |
| - case err == nil: |
| - removed = true |
| - case errors.Is(err, unix.ENOENT), errors.Is(err, unix.EISDIR): |
| - // Gone, or swapped for a directory, since the look. |
| - default: |
| - return false, &os.PathError{Op: "unlinkat", Path: name, Err: err} |
| - } |
| - } |
| - if err := dir.f.Sync(); err != nil { |
| - return false, err |
| - } |
| - return removed, nil |
| -} |
diff --git a/internal/hostfiles/sync.go b/internal/hostfiles/sync.go| index 23e7edffb..49a4962fb 100644 |
| --- a/internal/hostfiles/sync.go |
| +++ b/internal/hostfiles/sync.go |
| @@ -79,10 +79,10 @@ func SyncDir(ctx context.Context, p *Publisher, dir string, skip func(rel string |
| // the one cap every writer of a new entry (Stage, StageSkipped, the |
| // watcher's publishOne, a walk) applies, and that Commit and every reader |
| // (checkManifest: Restore, CopyManifest, ValidateManifest) apply to live |
| -// entries (LiveEntriesWithinCap), so no manifest is written that a reader |
| -// refuses. A tombstone or skipped entry past it, from a manifest written |
| -// before the cap, is exempt: StageDelete retires one, and Restore neither |
| -// writes nor walks it. Every path it admits can be opened by one openat2 |
| +// entries and tombstones (RestorableWithinCap), so no manifest is written |
| +// that a reader refuses. A skipped entry past it, from a manifest written |
| +// before the cap, is exempt: Restore never touches it, and StageDelete |
| +// leaves it as it is. Every path it admits can be opened by one openat2 |
| // beneath the restored root (Present). It also bounds depth (about 2048 |
| // components of one byte), and with it a walk's open directories and its |
| // path strings. |