amadan
moves: a standing condition never holds gone back; unexpected and unreadable local paths are left, and a gone without a usable seal, valid evidence or a movable damaged record pages once and records nothing
Task 1 fix round 1. recordRetired states the rule once: a transient read (the judge unreadable, a seal read error, the versioned store's factory, ListVersions, the corrupt scan's and createRetired's I/O) returns an error and retries; a local path that is not its type (Identify's new ErrUnexpectedType, or a live link that is not a symlink, a programs file that is not regular) is left as "unexpected type", and one that cannot be read (an Lstat, Readlink or chain error) as "unreadable layout"; anything else writes nothing, logs, and lets the gone report through. A seal that is absent, does not parse or fails validation, evidence gone built that does not validate, and a damaged retired.json the link refuses (a directory) each also page once per host, move and rule (MV-O264 to MV-O267, pending Paul's review). ValidateRetired requires the bucket, its region and the tombstone owner, and the live link's identity a symlink, the programs file's regular. The layout tests that assume a non-root uid skip as root, naming the root runner. The seam audit walks the module root (nested modules aside), and SetTrustForE2E's zero minimum names Task 9a.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
9 files changed,
+502
−84
approved_moves_copy_test.go+5 −1docs/superpowers/plans/2026-10-06-moves-phase1-copy.md+11 −0internal/host/movescleanup_retired.go+100 −26internal/host/movescleanup_retired_test.go+239 −9internal/offlinecleanup/layout_linux.go+3 −2internal/offlinecleanup/layout_linux_test.go+53 −20internal/offlinecleanup/seams_test.go+34 −22internal/offlinecleanup/wire.go+36 −4internal/offlinecleanup/wire_test.go+21 −0
diff --git a/approved_moves_copy_test.go b/approved_moves_copy_test.go| index 26df05443..1ec1a606a 100644 |
| --- a/approved_moves_copy_test.go |
| +++ b/approved_moves_copy_test.go |
| @@ -114,6 +114,7 @@ var movesCopyIDs = []string{ |
| "MV-O247", "MV-O248", "MV-O249", "MV-O250", "MV-O251", "MV-O252", "MV-O253", "MV-O254", "MV-O255", "MV-O256", |
| "MV-O257", "MV-O258", |
| "MV-O259", "MV-O260", "MV-O261", "MV-O262", "MV-O263", |
| + "MV-O264", "MV-O265", "MV-O266", "MV-O267", |
| } |
| // copyRow is one row of the record. The File cell is the file the sentence is |
| @@ -1305,7 +1306,7 @@ var movesScanFiles = []string{ |
| "internal/host/movesoffline_stage.go", |
| "internal/host/movesoffline_reports.go", |
| // Self-serve moves plan 4 Task 1: the cleanup obligation recorded at |
| - // gone (its log lines only; no operator sentence). |
| + // gone, and its skipped-cleanup pages (MV-O264 to MV-O267). |
| "internal/host/movescleanup_retired.go", |
| // Self-serve moves plan 2 Task 10a: the exported end-to-end bridge (test |
| // infrastructure in a non-test file). |
| @@ -1502,6 +1503,9 @@ var movesReturnedSentences = map[string]map[string]bool{ |
| // The box's move pages (Plan 6 Task 5, MV-N385 to MV-N390) and M-C20, |
| // the operators' sentence for a stray process (the stray page's body). |
| "internal/host/movespaging.go": {"rolledBackPage": true, "strayPage": true, "strayLine": true, "startRefusedPage": true}, |
| + // A gone that records no cleanup obligation pages once per rule |
| + // (self-serve moves plan 4 Task 1 fix round 1, MV-O264 to MV-O267). |
| + "internal/host/movescleanup_retired.go": {"cleanupSkippedPage": true, "cleanupSkippedBody": true}, |
| // The offline sealer's lost-stop page, its two causes and the lease |
| // states the first names (offline move Task 4, MV-O66 to MV-O75). |
| "internal/host/movesoffline_seal.go": {"offlineLostStopPage": true, "offlineLeaseCause": true, "offlinePastSealCause": true, "offlineLeaseText": true}, |
diff --git a/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md b/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md| index 7e76ecbdf..48d4b4262 100644 |
| --- a/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md |
| +++ b/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md |
| @@ -817,3 +817,14 @@ Task 6 answers MV-O263 from internal/edge/servedby.go, the file its row now name |
| | MV-O261 | internal/edge/holding.go (self-serve moves plan 3 Task 2) | This account is temporarily offline. Please check back in a few minutes. | pending | |
| | MV-O262 | internal/edge/holding.go (self-serve moves plan 3 Task 2) | This is taking longer than expected. Please keep checking back. | pending | |
| | MV-O263 | internal/edge/servedby.go (self-serve moves plan 3 Task 2; the probe refusal of Task 6) | not served here | pending | |
| + |
| +## Self-serve moves plan 4 |
| + |
| +Task 1 fix round 1 (pending Paul's review): the pages of a gone that records no cleanup obligation for a reason an operator must see, one subject per rule (MV-O264 a seal that is absent, does not parse or fails validation; MV-O265 evidence the box built that does not validate; MV-O266 a damaged retired.json that cannot be moved aside), each with the body MV-O267. Each fires once per host and move; the move's gone report proceeds and nothing is deleted. |
| + |
| +| id | file | text | status | |
| +|---|---|---|---| |
| +| MV-O264 | internal/host/movescleanup_retired.go (self-serve moves plan 4 Task 1 fix round 1) | cleanup of <host> skipped: no usable seal | pending | |
| +| MV-O265 | internal/host/movescleanup_retired.go (self-serve moves plan 4 Task 1 fix round 1) | cleanup of <host> skipped: its evidence does not validate | pending | |
| +| MV-O266 | internal/host/movescleanup_retired.go (self-serve moves plan 4 Task 1 fix round 1) | cleanup of <host> skipped: its damaged record cannot be moved aside | pending | |
| +| MV-O267 | internal/host/movescleanup_retired.go (self-serve moves plan 4 Task 1 fix round 1) | the move <id> finished, but nothing will clean up its source replica or what it left on <box>. clean them up by hand. | pending | |
diff --git a/internal/host/movescleanup_retired.go b/internal/host/movescleanup_retired.go| index aa413fe45..7ea79ff70 100644 |
| --- a/internal/host/movescleanup_retired.go |
| +++ b/internal/host/movescleanup_retired.go |
| @@ -21,6 +21,7 @@ import ( |
| "time" |
| "github.com/carlosframework/platform/internal/activator" |
| + "github.com/carlosframework/platform/internal/alerting" |
| "github.com/carlosframework/platform/internal/lease" |
| "github.com/carlosframework/platform/internal/moves" |
| "github.com/carlosframework/platform/internal/offlinecleanup" |
| @@ -34,13 +35,27 @@ const retiredNoticeEvery = 24 * time.Hour |
| // recordRetired writes retired.json for rec's move at offline-finishing, |
| // on the sealing source box whose row is gone (offlineSourceGone's caller |
| -// checks both), when the lifetime is tracked. It answers nil when it |
| -// writes nothing for a standing reason (no judge, another region, an |
| -// untracked lifetime, a version id that names no one version), so the gone |
| -// report is written as before this plan, and an error, which holds the |
| -// gone report back to the next pass, when a read fails or the evidence is |
| -// not whole. A file already there is never rewritten: it is scanned |
| -// (scanRetired), which keeps a damaged one aside behind a corrupt stub. |
| +// checks both), when the lifetime is tracked. A file already there is |
| +// never rewritten: it is scanned (scanRetired), which keeps a damaged one |
| +// aside behind a corrupt stub. |
| +// |
| +// The rule, once: a standing condition never holds gone back. |
| +// - A transient read returns an error, and the gone report waits for the |
| +// next pass: a lifetime the judge cannot read, a seal read error, the |
| +// versioned store's factory, ListVersions (and a listing with no single |
| +// current entry for a key), the corrupt scan's and createRetired's |
| +// I/O. |
| +// - A local path that cannot be read, or is not the file type its name |
| +// says, is left with its reason ("unreadable layout", "unexpected |
| +// type") in the obligation, which is still written. |
| +// - Anything else writes nothing, logs once a day and answers nil, so |
| +// the gone report is written as before this plan: no judge, another |
| +// region, no bucket or directories, an untracked lifetime, a version |
| +// id that names no one version; and, paged once per host, move and |
| +// rule as well, a seal that is absent, does not parse or fails |
| +// validation, evidence gone itself built that does not validate (a |
| +// bug or a misconfigured box), and a damaged retired.json that cannot |
| +// be moved aside (a directory there). |
| func (w *movesWorker) recordRetired(ctx context.Context, s release.Store, rec provision.Record) error { |
| cfg := w.cfg |
| m, host := rec.Move, rec.Host |
| @@ -62,7 +77,11 @@ func (w *movesWorker) recordRetired(ctx context.Context, s release.Store, rec pr |
| if path == "" { |
| return none("the host or the move id is not a usable name") |
| } |
| - if found, err := scanRetired(path, cfg.BoxID, host, m.ID); err != nil || found.Retired != nil || found.Stub != nil { |
| + found, err := scanRetired(path, cfg.BoxID, host, m.ID) |
| + if errors.Is(err, errRetiredUnmovable) { |
| + return w.skipCleanup(ctx, host, m.ID, pageRuleCleanupUnmovable, "%v", err) |
| + } |
| + if err != nil || found.Retired != nil || found.Stub != nil { |
| return err |
| } |
| @@ -84,11 +103,11 @@ func (w *movesWorker) recordRetired(ctx context.Context, s release.Store, rec pr |
| return err |
| } |
| seal, err := decodeSeal(raw, host, m.ID) |
| - if err != nil { |
| - return fmt.Errorf("offline move %s: the seal: %w", m.ID, err) |
| - } |
| if seal == nil { |
| - return fmt.Errorf("offline move %s: the seal is missing at finishing", m.ID) |
| + if err == nil { |
| + err = errors.New("the seal is missing") |
| + } |
| + return w.skipCleanup(ctx, host, m.ID, pageRuleCleanupNoSeal, "no usable seal: %v", err) |
| } |
| prefix := activator.HibernatePrefix(cfg.OwnPrefix, host) + "/" |
| @@ -110,13 +129,10 @@ func (w *movesWorker) recordRetired(ctx context.Context, s release.Store, rec pr |
| if unusable != "" { |
| return none("%s lists a version id that names no one version (the bucket wrote it unversioned or suspended)", unusable) |
| } |
| - local, left, err := w.localInventory(host) |
| - if err != nil { |
| - return fmt.Errorf("offline move %s: %w", m.ID, err) |
| - } |
| + local, left := w.localInventory(host) |
| r := cfg.retiredAtGone(obs.Rec, cur, seal, prefix, inventory, local, left) |
| if err := offlinecleanup.ValidateRetired(r); err != nil { |
| - return fmt.Errorf("offline move %s: %w", m.ID, err) |
| + return w.skipCleanup(ctx, host, m.ID, pageRuleCleanupInvalid, "the evidence gone built does not validate: %v", err) |
| } |
| if err := createRetired(path, r); err != nil { |
| return fmt.Errorf("offline move %s: write %s: %w", m.ID, path, err) |
| @@ -201,8 +217,11 @@ func inventoryOf(prefix string, listing []lease.ObjectVersion) (inv []offlinecle |
| // named nowhere; present in a directory whose chain is root-only, in the |
| // inventory with its identity (root removes it); present otherwise, left |
| // as "writable layout" (on every built box the programs file is here: the |
| -// agent's unit has ReadWritePaths=/data). |
| -func (w *movesWorker) localInventory(host string) ([]offlinecleanup.LocalPath, []offlinecleanup.KeptPath, error) { |
| +// agent's unit has ReadWritePaths=/data). A path that is not the file type |
| +// its name says is left as "unexpected type", and one the agent cannot |
| +// read (its Lstat, its link or its directory chain) as "unreadable |
| +// layout": a layout problem never holds gone back. |
| +func (w *movesWorker) localInventory(host string) ([]offlinecleanup.LocalPath, []offlinecleanup.KeptPath) { |
| chain := w.cfg.rootOnlyChain |
| if chain == nil { |
| chain = offlinecleanup.RootOnlyChain |
| @@ -213,16 +232,28 @@ func (w *movesWorker) localInventory(host string) ([]offlinecleanup.LocalPath, [ |
| {offlinecleanup.WhichLiveLink, release.LiveLink(w.cfg.OptDir, host)}, |
| {offlinecleanup.WhichProgramsFile, release.ProgramsPath(w.cfg.DataDir, host)}, |
| } { |
| - id, ok, err := offlinecleanup.Identify(p.path) |
| - if err != nil { |
| - return nil, nil, fmt.Errorf("the %s: %w", p.which, err) |
| + keep := func(reason string, err error) { |
| + left = append(left, offlinecleanup.KeptPath{Which: p.which, Reason: reason}) |
| + w.noticeEvery(host+"#retired-"+p.which, retiredNoticeEvery, "moves: %s: the %s is left (%s): %v", host, p.which, reason, err) |
| } |
| - if !ok { |
| + id, ok, err := offlinecleanup.Identify(p.path) |
| + switch { |
| + case errors.Is(err, offlinecleanup.ErrUnexpectedType): |
| + keep(offlinecleanup.ReasonUnexpectedType, err) |
| + continue |
| + case err != nil: |
| + keep(offlinecleanup.ReasonUnreadableLayout, err) |
| + continue |
| + case !ok: |
| + continue |
| + case id.Mode != offlinecleanup.WantMode(p.which): |
| + keep(offlinecleanup.ReasonUnexpectedType, fmt.Errorf("%s is a %s", p.path, id.Mode)) |
| continue |
| } |
| rootOnly, err := chain(filepath.Dir(p.path)) |
| if err != nil { |
| - return nil, nil, fmt.Errorf("the %s's directory: %w", p.which, err) |
| + keep(offlinecleanup.ReasonUnreadableLayout, err) |
| + continue |
| } |
| if rootOnly { |
| inv = append(inv, offlinecleanup.LocalPath{Which: p.which, Identity: id}) |
| @@ -230,7 +261,41 @@ func (w *movesWorker) localInventory(host string) ([]offlinecleanup.LocalPath, [ |
| left = append(left, offlinecleanup.KeptPath{Which: p.which, Reason: offlinecleanup.ReasonWritableLayout}) |
| } |
| } |
| - return inv, left, nil |
| + return inv, left |
| +} |
| + |
| +// The pages of a gone that records no obligation for a reason an operator |
| +// must see, one rule each, fired once per host and move (firePage). |
| +const ( |
| + pageRuleCleanupNoSeal = "cleanup-no-seal" |
| + pageRuleCleanupInvalid = "cleanup-invalid" |
| + pageRuleCleanupUnmovable = "cleanup-unmovable" |
| +) |
| + |
| +// skipCleanup is a standing condition with a page: nothing is written, the |
| +// reason is logged once a day, the rule's page fires once, and nil lets |
| +// the gone report through. |
| +func (w *movesWorker) skipCleanup(ctx context.Context, host, op, rule, why string, args ...any) error { |
| + w.noticeEvery(host+"#retired-"+rule, retiredNoticeEvery, "moves: %s: offline move %s: no cleanup obligation: "+why, append([]any{host, op}, args...)...) |
| + subject, body := cleanupSkippedPage(rule, host, op, w.cfg.Box) |
| + w.firePage(ctx, host, op, rule, alerting.LevelWarning, subject, body) |
| + return nil |
| +} |
| + |
| +// cleanupSkippedPage is the subject of each rule's page (MV-O264 to |
| +// MV-O266) and their body (MV-O267). |
| +func cleanupSkippedPage(rule, host, id, box string) (subject, body string) { |
| + switch rule { |
| + case pageRuleCleanupNoSeal: |
| + return fmt.Sprintf("cleanup of %s skipped: no usable seal", host), cleanupSkippedBody(id, box) |
| + case pageRuleCleanupInvalid: |
| + return fmt.Sprintf("cleanup of %s skipped: its evidence does not validate", host), cleanupSkippedBody(id, box) |
| + } |
| + return fmt.Sprintf("cleanup of %s skipped: its damaged record cannot be moved aside", host), cleanupSkippedBody(id, box) |
| +} |
| + |
| +func cleanupSkippedBody(id, box string) string { |
| + return fmt.Sprintf("the move %s finished, but nothing will clean up its source replica or what it left on %s. clean them up by hand.", id, box) |
| } |
| // retiredScan is what scanRetired found at a move's retired.json: nothing |
| @@ -290,9 +355,15 @@ func scanRetired(path, box, host, op string) (retiredScan, error) { |
| return retiredScan{Stub: stub, MovedTo: moved}, err |
| } |
| +// errRetiredUnmovable is a damaged retired.json the scan cannot move |
| +// aside: the link is refused (a directory there, which no hard link may |
| +// name), so it stays where it is and nothing is recorded. |
| +var errRetiredUnmovable = errors.New("the damaged retired.json cannot be moved aside") |
| + |
| // moveAsideCorrupt links path to the first free retired.json.corrupt-<n> |
| // and then removes path. A name already holding this very file (a crash |
| -// after the link) is where it stays. |
| +// after the link) is where it stays. A refused link is |
| +// errRetiredUnmovable. |
| func moveAsideCorrupt(path string) (string, error) { |
| fi, err := os.Lstat(path) |
| if err != nil { |
| @@ -308,6 +379,9 @@ func moveAsideCorrupt(path string) (string, error) { |
| if err == nil { |
| break |
| } |
| + if errors.Is(err, fs.ErrPermission) { |
| + return "", fmt.Errorf("%w: %v", errRetiredUnmovable, err) |
| + } |
| if !errors.Is(err, fs.ErrExist) { |
| return "", err |
| } |
diff --git a/internal/host/movescleanup_retired_test.go b/internal/host/movescleanup_retired_test.go| index 09b56c43d..91bc47e89 100644 |
| --- a/internal/host/movescleanup_retired_test.go |
| +++ b/internal/host/movescleanup_retired_test.go |
| @@ -247,17 +247,20 @@ func TestRetiredJSONWrittenOnceAtGoneBySealer(t *testing.T) { |
| t.Fatalf("listings %q: a second pass listed the bucket again", l) |
| } |
| - // An identity error writes nothing and no gone report: the live link's |
| - // directory is a regular file, so its Lstat fails with ENOTDIR. |
| + // A path that cannot be read is a layout problem, never a reason to hold |
| + // gone back: the live link's directory is a regular file, so its Lstat |
| + // fails with ENOTDIR, and the link is left as an unreadable layout. |
| rw2 := newRetiredWorld(t, true) |
| writeFile(t, filepath.Join(rw2.ie.opt, "live"), "not a directory") |
| - err := rw2.gone(rw2.ie, nil) |
| - if !errors.Is(err, syscall.ENOTDIR) { |
| - t.Fatalf("an identity error answered %v", err) |
| + if err := rw2.gone(rw2.ie, nil); err != nil { |
| + t.Fatalf("an identity error held gone back: %v", err) |
| } |
| - rw2.noRetired(rw2.ie) |
| - if rw2.goneReported("ie-a") { |
| - t.Fatal("a gone report after an identity error") |
| + r2, ok := rw2.retired(rw2.ie) |
| + if !ok || len(r2.LocalInventory) != 0 || fmt.Sprint(r2.LocalLeft) != fmt.Sprint([]offlinecleanup.KeptPath{{Which: offlinecleanup.WhichLiveLink, Reason: "unreadable layout"}}) { |
| + t.Fatalf("after ENOTDIR: %v %+v", ok, r2.LocalLeft) |
| + } |
| + if !rw2.goneReported("ie-a") { |
| + t.Fatal("no gone report after an unreadable layout") |
| } |
| } |
| @@ -291,7 +294,10 @@ func TestUntrackedLifetimeWritesNoRetiredAndReportsGone(t *testing.T) { |
| } |
| }) |
| t.Run("a migrating record", func(t *testing.T) { |
| - rw := newRetiredWorld(t, false) |
| + // Incarnation 1 with a live slot at 1 that vouches for the |
| + // migration: the judge answers valid, and only the migrating |
| + // clause makes the lifetime untracked. |
| + rw := newRetiredWorld(t, true) |
| rec, etag, err := provision.GetRecordETag(ctx, rw.s, rw.acct, "app", offHost) |
| if err != nil { |
| t.Fatal(err) |
| @@ -300,6 +306,21 @@ func TestUntrackedLifetimeWritesNoRetiredAndReportsGone(t *testing.T) { |
| if err := provision.UpdateRecord(ctx, rw.s, *rec, etag); err != nil { |
| t.Fatal(err) |
| } |
| + if _, etag, err = provision.GetRecordETag(ctx, rw.s, rw.acct, "app", offHost); err != nil { |
| + t.Fatal(err) |
| + } |
| + _, slotETag, err := provision.ReadHostIncarnation(ctx, rw.s, offHost) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + vouch := provision.HostIncarnation{Incarnation: 1, State: provision.SlotLive, Account: rw.acct, App: "app", Intent: "migration lazy-1", RecordETag: etag} |
| + if _, err := provision.WriteHostIncarnation(ctx, rw.s, offHost, vouch, slotETag); err != nil { |
| + t.Fatal(err) |
| + } |
| + obs := rw.judge.Observe(ctx, rw.s, rw.acct, "app", offHost) |
| + if obs.Verdict != provision.LifetimeValid || obs.Rec == nil || obs.Rec.Incarnation != 1 || obs.Slot == nil || obs.Slot.Incarnation != 1 || obs.Rec.IncarnationMigrating == "" { |
| + t.Fatalf("fixture: the judge answers %d (%+v, slot %+v)", obs.Verdict, obs.Rec, obs.Slot) |
| + } |
| if err := rw.gone(rw.ie, nil); err != nil { |
| t.Fatal(err) |
| } |
| @@ -516,6 +537,9 @@ func TestWritablePathsAreLeftNotRequested(t *testing.T) { |
| return rw |
| } |
| t.Run("a directory the test uid owns", func(t *testing.T) { |
| + if os.Getuid() == 0 { |
| + t.Skip("runs as root: the temp directory is root's here; the root runner covers root-owned layouts") |
| + } |
| rw := setup(t) |
| if err := rw.gone(rw.ie, nil); err != nil { |
| t.Fatal(err) |
| @@ -704,3 +728,209 @@ func TestWriteRetiredRaisesRevAndReplaces(t *testing.T) { |
| t.Fatalf("temporary files left: %v", ents) |
| } |
| } |
| + |
| +// pages is the pager b's worker fires through: every page's subject. |
| +func (rw *retiredWorld) pages(b *offBox) *[]string { |
| + var got []string |
| + b.w.cfg.Page = func(_ context.Context, level, subject, body string) error { |
| + got = append(got, level+": "+subject+" | "+body) |
| + return nil |
| + } |
| + return &got |
| +} |
| + |
| +// The local paths' kinds that are neither readable nor what their name |
| +// says: each is left with its reason, and gone proceeds. |
| +func TestUnexpectedAndUnreadableLocalPathsAreLeft(t *testing.T) { |
| + t.Run("unexpected types", func(t *testing.T) { |
| + rw := newRetiredWorld(t, true) |
| + link := release.LiveLink(rw.ie.opt, offHost) |
| + if err := os.MkdirAll(link, 0o755); err != nil { // a directory where the link belongs |
| + t.Fatal(err) |
| + } |
| + programs := release.ProgramsPath(rw.ie.dd, offHost) |
| + if err := os.MkdirAll(filepath.Dir(programs), 0o755); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := os.Symlink("elsewhere.json", programs); err != nil { // a link where the file belongs |
| + t.Fatal(err) |
| + } |
| + rw.ie.w.cfg.rootOnlyChain = func(string) (bool, error) { return true, nil } |
| + if err := rw.gone(rw.ie, nil); err != nil { |
| + t.Fatal(err) |
| + } |
| + r, ok := rw.retired(rw.ie) |
| + want := []offlinecleanup.KeptPath{{Which: offlinecleanup.WhichLiveLink, Reason: "unexpected type"}, {Which: offlinecleanup.WhichProgramsFile, Reason: "unexpected type"}} |
| + if !ok || len(r.LocalInventory) != 0 || fmt.Sprint(r.LocalLeft) != fmt.Sprint(want) { |
| + t.Fatalf("%v: inventory %+v, left %+v", ok, r.LocalInventory, r.LocalLeft) |
| + } |
| + if !rw.goneReported("ie-a") { |
| + t.Fatal("no gone report") |
| + } |
| + }) |
| + t.Run("a chain that cannot be read", func(t *testing.T) { |
| + rw := newRetiredWorld(t, true) |
| + writeFile(t, release.ProgramsPath(rw.ie.dd, offHost), "{}") |
| + rw.ie.w.cfg.rootOnlyChain = func(string) (bool, error) { return false, syscall.EACCES } |
| + if err := rw.gone(rw.ie, nil); err != nil { |
| + t.Fatal(err) |
| + } |
| + r, _ := rw.retired(rw.ie) |
| + if fmt.Sprint(r.LocalLeft) != fmt.Sprint([]offlinecleanup.KeptPath{{Which: offlinecleanup.WhichProgramsFile, Reason: "unreadable layout"}}) { |
| + t.Fatalf("left %+v", r.LocalLeft) |
| + } |
| + }) |
| + t.Run("a directory the agent cannot search", func(t *testing.T) { |
| + if os.Getuid() == 0 { |
| + t.Skip("runs as root: a mode bit refuses root nothing; the root runner covers root-owned layouts") |
| + } |
| + rw := newRetiredWorld(t, true) |
| + programs := release.ProgramsPath(rw.ie.dd, offHost) |
| + writeFile(t, programs, "{}") |
| + if err := os.Chmod(filepath.Dir(programs), 0); err != nil { |
| + t.Fatal(err) |
| + } |
| + t.Cleanup(func() { os.Chmod(filepath.Dir(programs), 0o755) }) |
| + if err := rw.gone(rw.ie, nil); err != nil { |
| + t.Fatal(err) |
| + } |
| + r, _ := rw.retired(rw.ie) |
| + if fmt.Sprint(r.LocalLeft) != fmt.Sprint([]offlinecleanup.KeptPath{{Which: offlinecleanup.WhichProgramsFile, Reason: "unreadable layout"}}) { |
| + t.Fatalf("left %+v", r.LocalLeft) |
| + } |
| + }) |
| +} |
| + |
| +// failingKey fails every read of one key while on. |
| +type failingKey struct { |
| + release.Store |
| + key string |
| + on bool |
| +} |
| + |
| +func (s *failingKey) Get(ctx context.Context, key string) ([]byte, string, error) { |
| + if s.on && key == s.key { |
| + return nil, "", errors.New("read: injected") |
| + } |
| + return s.Store.Get(ctx, key) |
| +} |
| + |
| +func TestNoUsableSealSkipsAndPages(t *testing.T) { |
| + ctx := context.Background() |
| + sealKey := moves.OfflineSealKey(offHost, offID) |
| + cases := map[string]func(rw *retiredWorld){ |
| + "absent": func(rw *retiredWorld) { |
| + if err := rw.s.Delete(ctx, sealKey); err != nil { |
| + t.Fatal(err) |
| + } |
| + }, |
| + "does not parse": func(rw *retiredWorld) { |
| + if err := moves.PutReplace(ctx, rw.s, sealKey, []byte("{not json")); err != nil { |
| + t.Fatal(err) |
| + } |
| + }, |
| + "fails validation": func(rw *retiredWorld) { |
| + if err := moves.PutReplace(ctx, rw.s, sealKey, []byte(`{"v":1,"op":"`+offID+`","host":"`+offHost+`","box":"ie-a","role":"sealer","phase":"sealed"}`)); err != nil { |
| + t.Fatal(err) |
| + } |
| + }, |
| + } |
| + for name, spoil := range cases { |
| + t.Run(name, func(t *testing.T) { |
| + rw := newRetiredWorld(t, true) |
| + spoil(rw) |
| + pages := rw.pages(rw.ie) |
| + for range 2 { |
| + if err := rw.gone(rw.ie, nil); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + rw.noRetired(rw.ie) |
| + if !rw.goneReported("ie-a") { |
| + t.Fatal("no gone report") |
| + } |
| + if len(*pages) != 1 || !strings.HasPrefix((*pages)[0], "warning: cleanup of shop.example.com skipped: no usable seal | ") { |
| + t.Fatalf("pages %q", *pages) |
| + } |
| + }) |
| + } |
| + t.Run("a read error retries", func(t *testing.T) { |
| + rw := newRetiredWorld(t, true) |
| + pages := rw.pages(rw.ie) |
| + fk := &failingKey{Store: rw.s, key: sealKey, on: true} |
| + if err := rw.gone(rw.ie, fk); err == nil { |
| + t.Fatal("a seal read error did not hold gone back") |
| + } |
| + rw.noRetired(rw.ie) |
| + if rw.goneReported("ie-a") || len(*pages) != 0 { |
| + t.Fatalf("gone reported %v, pages %q", rw.goneReported("ie-a"), *pages) |
| + } |
| + fk.on = false |
| + if err := rw.gone(rw.ie, fk); err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, ok := rw.retired(rw.ie); !ok || !rw.goneReported("ie-a") { |
| + t.Fatal("the next pass wrote neither") |
| + } |
| + }) |
| +} |
| + |
| +// A standing condition never holds gone back: evidence that does not |
| +// validate (here a box with no bucket region) and a damaged record that |
| +// cannot be moved aside (a directory) each write nothing, page once, and |
| +// let gone through. |
| +func TestStandingConditionsPageAndNeverHoldGone(t *testing.T) { |
| + t.Run("evidence that does not validate", func(t *testing.T) { |
| + rw := newRetiredWorld(t, true) |
| + rw.ie.w.cfg.OwnBucketRegion = "" |
| + rw.ie.w.cfg.Versioned = func(context.Context, string, string) (lease.VersionedStore, error) { return rw.fake, nil } |
| + rw.fake.Put(retBucket, retPrefix+"db", []byte("x")) |
| + pages := rw.pages(rw.ie) |
| + for range 2 { |
| + if err := rw.gone(rw.ie, nil); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + rw.noRetired(rw.ie) |
| + if !rw.goneReported("ie-a") { |
| + t.Fatal("no gone report") |
| + } |
| + if len(*pages) != 1 || !strings.HasPrefix((*pages)[0], "warning: cleanup of shop.example.com skipped: its evidence does not validate | ") { |
| + t.Fatalf("pages %q", *pages) |
| + } |
| + }) |
| + t.Run("a damaged record that cannot be moved aside", func(t *testing.T) { |
| + rw := newRetiredWorld(t, true) |
| + path := retiredPath(rw.ie) |
| + if err := os.MkdirAll(path, 0o700); err != nil { // retired.json is a directory |
| + t.Fatal(err) |
| + } |
| + pages := rw.pages(rw.ie) |
| + for range 2 { |
| + if err := rw.gone(rw.ie, nil); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + if fi, err := os.Lstat(path); err != nil || !fi.IsDir() { |
| + t.Fatalf("the directory was touched: %v", err) |
| + } |
| + if _, err := os.Lstat(path + ".corrupt-1"); !errors.Is(err, os.ErrNotExist) { |
| + t.Fatal("something was moved aside") |
| + } |
| + if !rw.goneReported("ie-a") { |
| + t.Fatal("no gone report") |
| + } |
| + if len(*pages) != 1 || !strings.HasPrefix((*pages)[0], "warning: cleanup of shop.example.com skipped: its damaged record cannot be moved aside | ") { |
| + t.Fatalf("pages %q", *pages) |
| + } |
| + }) |
| +} |
| + |
| +func TestCleanupSkippedPageNamesTheMoveAndTheBox(t *testing.T) { |
| + for _, rule := range []string{pageRuleCleanupNoSeal, pageRuleCleanupInvalid, pageRuleCleanupUnmovable} { |
| + subject, body := cleanupSkippedPage(rule, offHost, offID, "ie-a") |
| + if !strings.HasPrefix(subject, "cleanup of shop.example.com skipped: ") || !strings.Contains(body, offID) || !strings.Contains(body, "ie-a") { |
| + t.Errorf("%s: %q / %q", rule, subject, body) |
| + } |
| + } |
| +} |
diff --git a/internal/offlinecleanup/layout_linux.go b/internal/offlinecleanup/layout_linux.go| index dde1addc0..a4646e3d3 100644 |
| --- a/internal/offlinecleanup/layout_linux.go |
| +++ b/internal/offlinecleanup/layout_linux.go |
| @@ -16,7 +16,8 @@ import ( |
| // Identify is what path is now, read by Lstat (a link is never followed): |
| // its device, inode and type, and a link's target. An absent path answers |
| // false; any other failure, or a file that is neither a symlink nor a |
| -// regular file, is an error and fails the caller's step. |
| +// regular file, is an error: ErrUnexpectedType for another file type, |
| +// which the caller keeps the path for, and the failure itself otherwise. |
| func Identify(path string) (FileIdentity, bool, error) { |
| fi, err := os.Lstat(path) |
| if errors.Is(err, fs.ErrNotExist) { |
| @@ -42,7 +43,7 @@ func Identify(path string) (FileIdentity, bool, error) { |
| case fi.Mode().IsRegular(): |
| id.Mode = ModeRegular |
| default: |
| - return FileIdentity{}, false, fmt.Errorf("%s is %v, neither a symlink nor a regular file", path, fi.Mode().Type()) |
| + return FileIdentity{}, false, fmt.Errorf("%s is %v: %w", path, fi.Mode().Type(), ErrUnexpectedType) |
| } |
| return id, true, nil |
| } |
diff --git a/internal/offlinecleanup/layout_linux_test.go b/internal/offlinecleanup/layout_linux_test.go| index b1118fe3d..0ed44dd56 100644 |
| --- a/internal/offlinecleanup/layout_linux_test.go |
| +++ b/internal/offlinecleanup/layout_linux_test.go |
| @@ -3,6 +3,7 @@ |
| package offlinecleanup |
| import ( |
| + "errors" |
| "os" |
| "path/filepath" |
| "syscall" |
| @@ -26,6 +27,16 @@ func rootOwnedOnThisSystem(t *testing.T, paths ...string) { |
| } |
| } |
| +// nonRoot skips a half whose premise is a uid other than 0: as root (the |
| +// root runner, scripts/*-root-runner.sh) a temporary directory is root's |
| +// and a mode bit refuses nothing. |
| +func nonRoot(t *testing.T) { |
| + t.Helper() |
| + if os.Getuid() == 0 { |
| + t.Skip("runs as root: this half asserts what a non-root uid sees; the root runner covers the root-owned halves") |
| + } |
| +} |
| + |
| // trusted is a temporary directory the root-only checks treat as root's: |
| // the test's own uid stands for uid 0 inside it and nowhere else. |
| func trusted(t *testing.T) string { |
| @@ -98,9 +109,20 @@ func TestIdentifyUsesLstat(t *testing.T) { |
| if _, ok, err := Identify(filepath.Join(file, "under")); ok || err == nil { |
| t.Fatalf("ENOTDIR answered %v %v", ok, err) |
| } |
| - // Another file type is not an identity this plan records. |
| - if _, _, err := Identify(dir); err == nil { |
| - t.Fatal("a directory has an identity") |
| + // Another file type is not an identity this plan records: a directory |
| + // answers ErrUnexpectedType, which the caller keeps the path for. |
| + if _, ok, err := Identify(dir); ok || !errors.Is(err, ErrUnexpectedType) { |
| + t.Fatalf("a directory answered %v %v", ok, err) |
| + } |
| + fifo := filepath.Join(dir, "fifo") |
| + if err := syscall.Mkfifo(fifo, 0o600); err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, ok, err := Identify(fifo); ok || !errors.Is(err, ErrUnexpectedType) { |
| + t.Fatalf("a fifo answered %v %v", ok, err) |
| + } |
| + if _, _, err := Identify(filepath.Join(file, "under")); errors.Is(err, ErrUnexpectedType) { |
| + t.Fatal("ENOTDIR is not a file type") |
| } |
| } |
| @@ -151,9 +173,12 @@ func TestRootOnlyChain(t *testing.T) { |
| t.Fatalf("an absent chain answered %v %v", ok, err) |
| } |
| // A temporary directory outside the trust: owned by the test's uid. |
| - if ok, err := RootOnlyChain(t.TempDir()); ok || err != nil { |
| - t.Fatalf("a temp directory answered %v %v", ok, err) |
| - } |
| + t.Run("a temp directory, as a non-root uid", func(t *testing.T) { |
| + nonRoot(t) |
| + if ok, err := RootOnlyChain(t.TempDir()); ok || err != nil { |
| + t.Fatalf("a temp directory answered %v %v", ok, err) |
| + } |
| + }) |
| if _, err := RootOnlyChain("relative/dir"); err == nil { |
| t.Fatal("a relative path was judged") |
| } |
| @@ -215,20 +240,10 @@ func TestRootOwnedJudgesTheEntryUnderItsOpenedParent(t *testing.T) { |
| } |
| // A temporary directory outside the trust: the test owns it. |
| - tmp := t.TempDir() |
| - write(t, filepath.Join(tmp, "mine"), 0o644) |
| - if err := os.Symlink("mine", filepath.Join(tmp, "link")); err != nil { |
| - t.Fatal(err) |
| - } |
| - tp := open(t, tmp) |
| - if ok, err := RootOnlyDir(tp); ok || err != nil { |
| - t.Fatalf("a temp directory: %v %v", ok, err) |
| - } |
| - for _, n := range []string{"mine", "link", "absent"} { |
| - if ok, err := RootOwned(tp, n); ok || err != nil { |
| - t.Errorf("RootOwned(tmp, %s) = %v %v", n, ok, err) |
| - } |
| - } |
| + t.Run("a temp directory, as a non-root uid", func(t *testing.T) { |
| + nonRoot(t) |
| + tmpHalf(t) |
| + }) |
| // A closed parent: the fstat fails, and says so. |
| closed := open(t, base) |
| closed.Close() |
| @@ -248,10 +263,28 @@ func TestRootOwnedJudgesTheEntryUnderItsOpenedParent(t *testing.T) { |
| }) |
| } |
| +func tmpHalf(t *testing.T) { |
| + tmp := t.TempDir() |
| + write(t, filepath.Join(tmp, "mine"), 0o644) |
| + if err := os.Symlink("mine", filepath.Join(tmp, "link")); err != nil { |
| + t.Fatal(err) |
| + } |
| + tp := open(t, tmp) |
| + if ok, err := RootOnlyDir(tp); ok || err != nil { |
| + t.Fatalf("a temp directory: %v %v", ok, err) |
| + } |
| + for _, n := range []string{"mine", "link", "absent"} { |
| + if ok, err := RootOwned(tp, n); ok || err != nil { |
| + t.Errorf("RootOwned(tmp, %s) = %v %v", n, ok, err) |
| + } |
| + } |
| +} |
| + |
| // The trust is a uid standing for 0 inside its bases only: a sibling that |
| // shares the base's name as a string prefix, the base's parent and any |
| // other directory keep the real rule, and restore ends it. |
| func TestTrustForE2EAppliesOnlyInsideItsBases(t *testing.T) { |
| + nonRoot(t) // the sibling and the parent are refused for their owner, which as root is 0 |
| parent := t.TempDir() |
| base := filepath.Join(parent, "root") |
| mkdir(t, base, 0o755) |
diff --git a/internal/offlinecleanup/seams_test.go b/internal/offlinecleanup/seams_test.go| index 97e60612a..abe9f7b99 100644 |
| --- a/internal/offlinecleanup/seams_test.go |
| +++ b/internal/offlinecleanup/seams_test.go |
| @@ -16,13 +16,14 @@ import ( |
| // The test-only seams of the cleanup (plan 4): production code a test uses |
| // to stand in for root, a clock, a fault or a schedule. Each is listed with |
| // the only places allowed to set it, and the audit below walks every Go |
| -// file under internal/ and fails on any other setter. A later task that |
| +// file of the module (nested modules, testdata and hidden directories |
| +// aside) and fails on any other setter. A later task that |
| // adds a seam adds its entry here. |
| // |
| // A seam is one of: |
| // - call: a function whose every reference (a call or a value) must sit |
| -// inside one of Setters, named "<dir under internal>.<func>" (a method |
| -// as "<dir>.<Type>.<method>"); |
| +// inside one of Setters, named "<dir>.<func>" with dir relative to the |
| +// module root (a method as "<dir>.<Type>.<method>"); |
| // - field: a struct field (Owner the struct's package directory) set by a |
| // composite literal key or an assignment, only in the _test.go files of |
| // the directories named in TestDirs; |
| @@ -30,7 +31,7 @@ import ( |
| // outside its declaration. |
| type seam struct { |
| Kind string // call, field, var |
| - Owner string // the declaring package's directory under internal/ |
| + Owner string // the declaring package's directory, relative to the module root |
| Name string |
| Setters []string // call: the functions allowed to reference it |
| TestDirs []string // field: the directories whose _test.go files may set it |
| @@ -43,15 +44,17 @@ type seam struct { |
| } |
| var e2eSeams = []seam{ |
| - {Kind: "call", Owner: "offlinecleanup", Name: "SetTrustForE2E", Setters: []string{"host.NewE2E"}, OwnTests: true, MinRefs: 0, |
| + // MinRefs 0 until Task 9a wires host.NewE2E to call it; Task 9a raises |
| + // it to 1. |
| + {Kind: "call", Owner: "internal/offlinecleanup", Name: "SetTrustForE2E", Setters: []string{"internal/host.NewE2E"}, OwnTests: true, MinRefs: 0, |
| Why: "the root-only checks' trust root for the e2e bridge; the bridge calls it from Task 9a, and nothing else ever may"}, |
| - {Kind: "var", Owner: "offlinecleanup", Name: "RetrySchedule", |
| + {Kind: "var", Owner: "internal/offlinecleanup", Name: "RetrySchedule", |
| Why: "the resend schedule lease and moves share; a test schedule is built with NewBackoff, never by assigning this"}, |
| - {Kind: "field", Owner: "host", Name: "rootOnlyChain", TestDirs: []string{"host"}, |
| + {Kind: "field", Owner: "internal/host", Name: "rootOnlyChain", TestDirs: []string{"internal/host"}, |
| Why: "moveCfg's injection of offlinecleanup.RootOnlyChain for the gone evidence's local paths (Task 1); host tests only"}, |
| } |
| -const modulePath = "github.com/carlosframework/platform/internal/" |
| +const modulePath = "github.com/carlosframework/platform/" |
| type seamRef struct { |
| file, fn string |
| @@ -59,8 +62,7 @@ type seamRef struct { |
| test bool |
| } |
| -// auditSeams parses every Go file under root (a directory standing for |
| -// internal/) and returns, per seam name, the references it found that the |
| +// auditSeams parses every Go file under root (the module root) and returns, per seam name, the references it found that the |
| // seam does not permit, and the count of permitted references. |
| func auditSeams(t *testing.T, root string, seams []seam) (bad map[string][]string, permitted map[string]int) { |
| t.Helper() |
| @@ -71,9 +73,12 @@ func auditSeams(t *testing.T, root string, seams []seam) (bad map[string][]strin |
| return err |
| } |
| if d.IsDir() { |
| - if n := d.Name(); p != root && (n == "testdata" || strings.HasPrefix(n, ".") || strings.HasPrefix(n, "_")) { |
| + if n := d.Name(); p != root && (n == "testdata" || n == "node_modules" || strings.HasPrefix(n, ".") || strings.HasPrefix(n, "_")) { |
| return filepath.SkipDir |
| } |
| + if _, err := os.Stat(filepath.Join(p, "go.mod")); p != root && err == nil { |
| + return filepath.SkipDir // a nested module (launchctx) imports none of this one |
| + } |
| return nil |
| } |
| if !strings.HasSuffix(p, ".go") { |
| @@ -87,7 +92,7 @@ func auditSeams(t *testing.T, root string, seams []seam) (bad map[string][]strin |
| dir := filepath.ToSlash(filepath.Dir(rel)) |
| test := strings.HasSuffix(p, "_test.go") |
| ownPkg := !strings.HasSuffix(f.Name.Name, "_test") // an external test package is not the declaring package |
| - imports := map[string]string{} // local name -> dir under internal/ |
| + imports := map[string]string{} // local name -> dir under the module root |
| for _, im := range f.Imports { |
| path, _ := strconv.Unquote(im.Path.Value) |
| if !strings.HasPrefix(path, modulePath) { |
| @@ -277,12 +282,15 @@ func recvName(e ast.Expr) string { |
| } |
| func TestEveryE2ESeamHasOneSetter(t *testing.T) { |
| - root, err := filepath.Abs("..") |
| + root, err := filepath.Abs(filepath.Join("..", "..")) |
| if err != nil { |
| t.Fatal(err) |
| } |
| - if _, err := os.Stat(filepath.Join(root, "offlinecleanup", "seams_test.go")); err != nil { |
| - t.Fatalf("the audit does not stand in internal/: %v", err) |
| + if _, err := os.Stat(filepath.Join(root, "internal", "offlinecleanup", "seams_test.go")); err != nil { |
| + t.Fatalf("the audit does not stand at the module root: %v", err) |
| + } |
| + if _, err := os.Stat(filepath.Join(root, "go.mod")); err != nil { |
| + t.Fatalf("the module root has no go.mod: %v", err) |
| } |
| bad, permitted := auditSeams(t, root, e2eSeams) |
| names := make([]string, 0, len(bad)) |
| @@ -307,27 +315,31 @@ func TestEveryE2ESeamHasOneSetter(t *testing.T) { |
| func TestSeamAuditFindsForbiddenSetters(t *testing.T) { |
| root := t.TempDir() |
| files := map[string]string{ |
| - "offlinecleanup/x.go": `package offlinecleanup |
| + "internal/offlinecleanup/x.go": `package offlinecleanup |
| var RetrySchedule = 1 |
| func SetTrustForE2E() func() { return nil } |
| func own() { RetrySchedule = 2 } |
| `, |
| - "host/e2e_bridge.go": `package host |
| + "internal/host/e2e_bridge.go": `package host |
| import oc "github.com/carlosframework/platform/internal/offlinecleanup" |
| type moveCfg struct{ rootOnlyChain func() } |
| func NewE2E() { defer oc.SetTrustForE2E()() } |
| func (w *worker) run() { oc.SetTrustForE2E() } |
| func prod() { _ = moveCfg{rootOnlyChain: nil} } |
| `, |
| - "host/x_test.go": `package host |
| + "internal/host/x_test.go": `package host |
| func fixture(c *moveCfg) { c.rootOnlyChain = nil } |
| `, |
| - "moves/y.go": `package moves |
| + "internal/moves/y.go": `package moves |
| import "github.com/carlosframework/platform/internal/offlinecleanup" |
| var f = offlinecleanup.SetTrustForE2E |
| func g() { offlinecleanup.RetrySchedule = 3; p := &offlinecleanup.RetrySchedule; _ = p; x := offlinecleanup.RetrySchedule; _ = x } |
| `, |
| } |
| + // A setter outside internal/ is found too, and a nested module is not walked. |
| + files["main.go"] = "package main\nimport \"github.com/carlosframework/platform/internal/offlinecleanup\"\nfunc main() { offlinecleanup.SetTrustForE2E() }\n" |
| + files["launchctx/go.mod"] = "module other\n" |
| + files["launchctx/z.go"] = "package z\nimport \"github.com/carlosframework/platform/internal/offlinecleanup\"\nfunc z() { offlinecleanup.SetTrustForE2E() }\n" |
| for name, body := range files { |
| p := filepath.Join(root, name) |
| if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { |
| @@ -339,9 +351,9 @@ func g() { offlinecleanup.RetrySchedule = 3; p := &offlinecleanup.RetrySchedule; |
| } |
| bad, permitted := auditSeams(t, root, e2eSeams) |
| want := map[string][]string{ |
| - "SetTrustForE2E": {"host/e2e_bridge.go:5 in worker.run", "moves/y.go:3 in <package scope>"}, |
| - "RetrySchedule": {"moves/y.go:4 in g", "moves/y.go:4 in g", "offlinecleanup/x.go:4 in own"}, |
| - "rootOnlyChain": {"host/e2e_bridge.go:6 in prod"}, |
| + "SetTrustForE2E": {"internal/host/e2e_bridge.go:5 in worker.run", "internal/moves/y.go:3 in <package scope>", "main.go:3 in main"}, |
| + "RetrySchedule": {"internal/moves/y.go:4 in g", "internal/moves/y.go:4 in g", "internal/offlinecleanup/x.go:4 in own"}, |
| + "rootOnlyChain": {"internal/host/e2e_bridge.go:6 in prod"}, |
| } |
| for n, w := range want { |
| got := append([]string(nil), bad[n]...) |
diff --git a/internal/offlinecleanup/wire.go b/internal/offlinecleanup/wire.go| index f7ced1495..f03638c7d 100644 |
| --- a/internal/offlinecleanup/wire.go |
| +++ b/internal/offlinecleanup/wire.go |
| @@ -95,10 +95,33 @@ const ( |
| var whiches = map[string]bool{WhichLiveLink: true, WhichProgramsFile: true} |
| -// ReasonWritableLayout keeps a path whose directory chain is not |
| -// root-only: no exclusion against a writer sharing the agent's uid is |
| -// possible there. |
| -const ReasonWritableLayout = "writable layout" |
| +// The reasons a local path is left (KeptPath.Reason) rather than |
| +// requested of root. |
| +const ( |
| + // ReasonWritableLayout keeps a path whose directory chain is not |
| + // root-only: no exclusion against a writer sharing the agent's uid is |
| + // possible there. |
| + ReasonWritableLayout = "writable layout" |
| + // ReasonUnexpectedType keeps a path that is not the file type its name |
| + // says (the live link is a symlink, the programs file a regular file). |
| + ReasonUnexpectedType = "unexpected type" |
| + // ReasonUnreadableLayout keeps a path the agent could not Lstat, or |
| + // whose directory chain it could not read. |
| + ReasonUnreadableLayout = "unreadable layout" |
| +) |
| + |
| +// ErrUnexpectedType is Identify's answer for a file that is neither a |
| +// symlink nor a regular file. |
| +var ErrUnexpectedType = errors.New("offlinecleanup: neither a symlink nor a regular file") |
| + |
| +// WantMode is the file type each local path must be: the live link a |
| +// symlink, the programs file a regular file. |
| +func WantMode(which string) string { |
| + if which == WhichLiveLink { |
| + return ModeSymlink |
| + } |
| + return ModeRegular |
| +} |
| // The file types an identity records. |
| const ( |
| @@ -292,6 +315,12 @@ func ValidateRetired(r Retired) error { |
| return errors.New("retired: incarnation 0 is not a tracked lifetime") |
| case r.From == r.To: |
| return fmt.Errorf("retired: from and to are both %q", r.From) |
| + case r.Bucket == "": |
| + return errors.New("retired: no bucket") |
| + case r.BucketRegion == "": |
| + return errors.New("retired: no bucket region") |
| + case r.TombstoneOwner == "": |
| + return errors.New("retired: no tombstone owner") |
| } |
| if !states[r.State] { |
| return fmt.Errorf("retired: state %q is not a state of the file", r.State) |
| @@ -329,6 +358,9 @@ func ValidateRetired(r Retired) error { |
| if err := validIdentity(p.Identity); err != nil { |
| return fmt.Errorf("retired: %s: %w", p.Which, err) |
| } |
| + if p.Identity.Mode != WantMode(p.Which) { |
| + return fmt.Errorf("retired: %s is a %s, want a %s", p.Which, p.Identity.Mode, WantMode(p.Which)) |
| + } |
| } |
| for _, p := range r.LocalLeft { |
| if err := validWhich(p.Which, seen); err != nil { |
diff --git a/internal/offlinecleanup/wire_test.go b/internal/offlinecleanup/wire_test.go| index 983232a0a..6cc5bbba9 100644 |
| --- a/internal/offlinecleanup/wire_test.go |
| +++ b/internal/offlinecleanup/wire_test.go |
| @@ -99,6 +99,16 @@ func TestValidateRetired(t *testing.T) { |
| {"superseding outside cleaning", func(r *Retired) { r.State = StateFinished; r.Superseding = SupersededRow }, "superseding"}, |
| {"superseding with nothing in flight", func(r *Retired) { r.State = StateCleaning; r.Superseding = SupersededRow }, "superseding"}, |
| {"written at", func(r *Retired) { r.WrittenAt = time.Time{} }, "written"}, |
| + {"no bucket", func(r *Retired) { r.Bucket = "" }, "bucket"}, |
| + {"no bucket region", func(r *Retired) { r.BucketRegion = "" }, "region"}, |
| + {"no tombstone owner", func(r *Retired) { r.TombstoneOwner = "" }, "tombstone"}, |
| + {"a live link that is a regular file", func(r *Retired) { |
| + r.LocalInventory[0].Identity = FileIdentity{Dev: 1, Inode: 2, Mode: ModeRegular} |
| + }, "live-link"}, |
| + {"a programs file that is a symlink", func(r *Retired) { |
| + r.LocalLeft = nil |
| + r.LocalInventory = append(r.LocalInventory, LocalPath{Which: WhichProgramsFile, Identity: FileIdentity{Dev: 1, Inode: 3, Mode: ModeSymlink, LinkTarget: "/x"}}) |
| + }, "programs-file"}, |
| {"prefix without a slash", func(r *Retired) { r.Prefix = strings.TrimSuffix(r.Prefix, "/") }, "prefix"}, |
| {"duplicate key", func(r *Retired) { r.BucketInventory[1].Key = r.BucketInventory[0].Key }, "twice"}, |
| {"key outside the prefix", func(r *Retired) { k0(r).Key = "instances/shop.example.co/db" }, "prefix"}, |
| @@ -186,6 +196,17 @@ func TestValidateRetired(t *testing.T) { |
| func(r *Retired) { r.Keys = map[string]KeyOutcome{k0(r).Key: {State: KeyUnsettled, Done: 2}} }, |
| func(r *Retired) { r.LocalInventory, r.LocalLeft = nil, nil; r.BucketInventory = nil }, |
| func(r *Retired) { r.Rev, r.PublishedRev = 4, 4 }, |
| + func(r *Retired) { |
| + r.LocalLeft = []KeptPath{{Which: WhichProgramsFile, Reason: ReasonUnexpectedType}} |
| + }, |
| + func(r *Retired) { |
| + r.LocalLeft = nil |
| + r.LocalInventory = append(r.LocalInventory, LocalPath{Which: WhichProgramsFile, Identity: FileIdentity{Dev: 1, Inode: 3, Mode: ModeRegular}}) |
| + }, |
| + func(r *Retired) { |
| + r.LocalInventory = nil |
| + r.LocalLeft = []KeptPath{{Which: WhichLiveLink, Reason: ReasonUnreadableLayout}, {Which: WhichProgramsFile, Reason: ReasonWritableLayout}} |
| + }, |
| // A tie in LastModified is ordered by Seq descending. |
| func(r *Retired) { |
| k := k0(r) |