amadan
review fix S1: an import's rollback retires the instance key it minted
An aborted import left the key its verify launch minted in <host>.env, where it outranks configuration. A second import of the host, whose owner had configured the old process's key first, was still handed the first import's key. - The swap records what the host's own env file held before the import (no file, a file with no key, a file with a key) in the import journal's new env_before column, read just before the swap's transaction. A read that fails defers the swap. - The fenced rollback's step 3 retires what the import created, before it removes the local data: the whole file (and the adoption record beside it) when there was none, the key lines alone when the file had no key, nothing when the key was there before or the journal row predates the record. With the row gone it follows the same marker rule as the database. - The retirement is its own hook (activator.RetireSeedKey), not part of the shared purge. A failure keeps the rollback where it was. - Journal rows from before the column read as unknown and retire nothing. Removing the key is within main's rules for it: create-only protects data sealed with the key, and the rollback destroys that data under the same fence. <host>.env is an installation artifact, retired with its installation. Tests: verify, abort, finish, configure B, reimport is handed B; a key and env lines that predated the import survive; the swap records and defers; the rollback passes the journal's value on both paths and waits on a failed retirement; the journal column migrates and is hashed when set. The record-write census re-reviews the five units this touches.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
14 files changed,
+856
−33
internal/activator/seedkey.go+164 −0internal/activator/seedkey_linux_test.go+274 −0internal/host/seedwiring.go+3 −1internal/host/seedwiring_linux_test.go+36 −0internal/host/seedwiring_test.go+6 −2internal/provision/plan9_b8_census_test.go+10 −5internal/provision/seedreconcile.go+18 −4internal/provision/seedreconcile_test.go+5 −1internal/provision/seedrollback.go+26 −5internal/provision/seedrollback_test.go+164 −1internal/provision/seedswap.go+14 −1internal/provision/seedswap_test.go+3 −0internal/registry/importjournal.go+50 −13internal/registry/importjournal_test.go+83 −0
diff --git a/internal/activator/seedkey.go b/internal/activator/seedkey.go| new file mode 100644 |
| index 000000000..5918303f6 |
| --- /dev/null |
| +++ b/internal/activator/seedkey.go |
| @@ -0,0 +1,164 @@ |
| +package activator |
| + |
| +import ( |
| + "errors" |
| + "fmt" |
| + "os" |
| + "path/filepath" |
| + "strings" |
| + |
| + "github.com/carlosframework/platform/internal/instancefs" |
| + "github.com/carlosframework/platform/internal/instancekey" |
| + "github.com/carlosframework/platform/internal/registry" |
| +) |
| + |
| +// An import and the instance key (instancekey.go; merge review S1). |
| +// |
| +// The platform mints CARLOS_INSTANCE_KEY into the instance's own |
| +// <host>.env at its first launch, create-only, and a key there outranks one |
| +// the app's configuration sets (instancekey.Effective). An import's first |
| +// launch is its verify request, so an import that is then aborted has left |
| +// a key behind. Left in place it would outlive the rollback: a second |
| +// import of the same host, whose owner configured the old process's key |
| +// before verifying, would still be handed the first import's key, and the |
| +// values the old process sealed would not open. |
| +// |
| +// So the swap records what the own env file held before the import |
| +// (SeedEnvBefore, kept in the import journal), and the fenced rollback |
| +// retires what the import created and nothing else (RetireSeedKey). |
| +// |
| +// Removing a minted key is within the key's rules. Create-only means a key |
| +// is never replaced under data sealed with it. Here the data goes too: the |
| +// rollback removes the import's database and deletes its replica, under the |
| +// same fence. <host>.env is an installation artifact, retired with its |
| +// installation, as a move retires it on the box it leaves. |
| + |
| +// seedEnvDir is the directory holding the own env file of the nested row |
| +// whose database is dbPath, and the file's name. |
| +func seedEnvDir(dbPath, host string) (dir, name string, err error) { |
| + dir, _, err = splitDBPath(dbPath) |
| + if err != nil { |
| + return "", "", err |
| + } |
| + if host == "" || host != filepath.Base(host) { |
| + return "", "", fmt.Errorf("activator: %q is not a host name", host) |
| + } |
| + return dir, host + ".env", nil |
| +} |
| + |
| +// SeedEnvBefore reports what the own env file of the nested row at dbPath |
| +// holds now, as one of registry's EnvBefore values: no file, a file with no |
| +// instance key, or a file that carries one. The swap calls it before the |
| +// import's row exists, so before anything can launch the import. The file |
| +// is read through instancefs, as every nested read of it is. A file that |
| +// cannot be read (a link, a FIFO, over the limit) is an error: the caller |
| +// cannot tell, and does not swap. |
| +func SeedEnvBefore(dbPath, host string) (string, error) { |
| + dir, name, err := seedEnvDir(dbPath, host) |
| + if err != nil { |
| + return "", err |
| + } |
| + adminTokenMu.Lock() |
| + defer adminTokenMu.Unlock() |
| + raw, exists, err := readSeedEnv(dir, name) |
| + switch { |
| + case err != nil: |
| + return "", err |
| + case !exists: |
| + return registry.EnvBeforeAbsent, nil |
| + case parseEnvBytes(raw)[InstanceKeyVar] != "": |
| + return registry.EnvBeforeKey, nil |
| + } |
| + return registry.EnvBeforeNoKey, nil |
| +} |
| + |
| +// readSeedEnv reads dir/name beneath dir. An instance directory that is not |
| +// there holds no file. |
| +func readSeedEnv(dir, name string) (raw []byte, exists bool, err error) { |
| + if _, err := os.Lstat(dir); errors.Is(err, os.ErrNotExist) { |
| + return nil, false, nil |
| + } |
| + raw, err = instancefs.ReadBeneathLimit(dir, name, ownEnvReadLimit) |
| + if errors.Is(err, os.ErrNotExist) { |
| + return nil, false, nil |
| + } |
| + if err != nil { |
| + return nil, false, fmt.Errorf("read %s: %w", filepath.Join(dir, name), err) |
| + } |
| + return raw, true, nil |
| +} |
| + |
| +// RetireSeedKey retires the instance key an import created, for the nested |
| +// row at dbPath, by what the swap recorded (before): |
| +// |
| +// - EnvBeforeAbsent: there was no env file, so the one there now is the |
| +// import's whole. It is removed, and with it the adoption record beside |
| +// it, which belongs to the same installation. |
| +// - EnvBeforeNoKey: the file was there with no key. Its CARLOS_INSTANCE_KEY |
| +// lines are the import's and are removed; every other line stays, byte |
| +// for byte. |
| +// - EnvBeforeKey: the key was there before the import. Nothing is touched. |
| +// - "": the journal row predates the record. Nothing is known, and nothing |
| +// is touched. |
| +// |
| +// The caller holds the host fenced and stopped (the import rollback's step |
| +// 3), so no launch reads the file meanwhile; adminTokenMu keeps the |
| +// startup backfill's append out. It is idempotent: a second run finds |
| +// nothing of the import's left. |
| +func RetireSeedKey(dbPath, host, before string) error { |
| + switch before { |
| + case registry.EnvBeforeAbsent, registry.EnvBeforeNoKey: |
| + case registry.EnvBeforeKey, "": |
| + return nil |
| + default: |
| + return fmt.Errorf("activator: %s: %q is not a recorded env state", host, before) |
| + } |
| + dir, name, err := seedEnvDir(dbPath, host) |
| + if err != nil { |
| + return err |
| + } |
| + adminTokenMu.Lock() |
| + defer adminTokenMu.Unlock() |
| + if _, err := os.Lstat(dir); errors.Is(err, os.ErrNotExist) { |
| + return nil |
| + } |
| + if before == registry.EnvBeforeAbsent { |
| + for _, n := range []string{name, host + instancekey.AdoptionSuffix} { |
| + if err := instancefs.RemoveAllBeneath(dir, n); err != nil { |
| + return fmt.Errorf("retire %s: %w", filepath.Join(dir, n), err) |
| + } |
| + } |
| + return syncDir(dir) |
| + } |
| + raw, exists, err := readSeedEnv(dir, name) |
| + if err != nil || !exists { |
| + return err |
| + } |
| + kept, removed := withoutInstanceKey(raw) |
| + if !removed { |
| + return nil |
| + } |
| + if err := instancefs.WriteAtomic(dir, name, kept, 0o600); err != nil { |
| + return fmt.Errorf("retire the instance key in %s: %w", filepath.Join(dir, name), err) |
| + } |
| + return syncDir(dir) |
| +} |
| + |
| +// withoutInstanceKey is raw with every line that sets CARLOS_INSTANCE_KEY |
| +// removed, by instancekey.ParseEnv's reading of a line; the other lines are |
| +// kept exactly. removed is whether any line went. |
| +func withoutInstanceKey(raw []byte) (kept []byte, removed bool) { |
| + lines := strings.Split(string(raw), "\n") |
| + out := lines[:0] |
| + for _, line := range lines { |
| + t := strings.TrimSpace(line) |
| + if t != "" && !strings.HasPrefix(t, "#") { |
| + if k, _, ok := strings.Cut(t, "="); ok && strings.TrimSpace(k) == InstanceKeyVar { |
| + removed = true |
| + continue |
| + } |
| + } |
| + out = append(out, line) |
| + } |
| + return []byte(strings.Join(out, "\n")), removed |
| +} |
diff --git a/internal/activator/seedkey_linux_test.go b/internal/activator/seedkey_linux_test.go| new file mode 100644 |
| index 000000000..f0c86d276 |
| --- /dev/null |
| +++ b/internal/activator/seedkey_linux_test.go |
| @@ -0,0 +1,274 @@ |
| +//go:build linux |
| + |
| +package activator |
| + |
| +import ( |
| + "context" |
| + "crypto/sha256" |
| + "encoding/hex" |
| + "os" |
| + "path/filepath" |
| + "strings" |
| + "testing" |
| + |
| + "github.com/carlosframework/platform/internal/instancectl" |
| + "github.com/carlosframework/platform/internal/instancekey" |
| + "github.com/carlosframework/platform/internal/provision" |
| + "github.com/carlosframework/platform/internal/registry" |
| + "github.com/carlosframework/platform/internal/seeds" |
| +) |
| + |
| +// importedRoute is nestedBackend's row as an import's swap leaves it: a |
| +// fresh seed id, swapped, its snapshot in the box's seed reader. |
| +func importedRoute(t *testing.T, b *ExecBackend, rt registry.Route) registry.Route { |
| + t.Helper() |
| + id, _ := seeds.NewID() |
| + snap := filepath.Join(t.TempDir(), "snap.db") |
| + if err := writeMinimalSQLite(snap); err != nil { |
| + t.Fatal(err) |
| + } |
| + body, _ := os.ReadFile(snap) |
| + sum := sha256.Sum256(body) |
| + rt.SeedSource = seeds.RowSeed{ID: id, Kind: seeds.KindImport, SHA256: hex.EncodeToString(sum[:]), Bytes: int64(len(body)), State: seeds.StateSwapped}.Encode() |
| + b.Seeds = &fakeSeedObjects{body: map[string][]byte{id: body}} |
| + return rt |
| +} |
| + |
| +// launchKey is one launch of rt as doWake makes it (the restore, then the |
| +// run role's preparation): the key the app is handed, and the key the |
| +// instance's own env file holds ("" for none). |
| +func launchKey(t *testing.T, b *ExecBackend, rt registry.Route) (handed, own string) { |
| + t.Helper() |
| + if fresh, err := b.Restore(context.Background(), rt); err != nil || fresh { |
| + t.Fatalf("Restore = fresh %v, %v", fresh, err) |
| + } |
| + _, ctl, logs, env, err := b.prepare(rt, instancectl.RoleRun) |
| + if err != nil { |
| + t.Fatalf("prepare: %v", err) |
| + } |
| + logs.Close() |
| + if ctl != nil { |
| + ctl.Close() |
| + } |
| + raw, err := os.ReadFile(filepath.Join(provision.InstanceDir(b.DataDir, rt.Host), rt.Host+".env")) |
| + if err != nil && !os.IsNotExist(err) { |
| + t.Fatal(err) |
| + } |
| + return env[InstanceKeyVar], parseEnvBytes(raw)[InstanceKeyVar] |
| +} |
| + |
| +// Merge review S1, the reported sequence: an import is verified (its first |
| +// launch mints key A), aborted and rolled back; the owner then configures |
| +// the old process's key B and imports the host again. The second import is |
| +// handed B. Before the fix it was handed A, which the first import's |
| +// rollback left in <host>.env, where it outranks configuration. |
| +func TestAnAbortedImportsKeyDoesNotOutliveItsRollback(t *testing.T) { |
| + noOwnReplica(t) |
| + b, rt := nestedBackend(t, &fakeControl{inv: "inv1"}, &fakeUnit{}) |
| + dir := provision.InstanceDir(b.DataDir, rt.Host) |
| + |
| + // The swap, before the import's row exists: this host has no env file. |
| + before, err := SeedEnvBefore(rt.DBPath, rt.Host) |
| + if err != nil || before != registry.EnvBeforeAbsent { |
| + t.Fatalf("SeedEnvBefore = %q, %v; want absent", before, err) |
| + } |
| + // Verify: the import's first launch mints key A. |
| + first := importedRoute(t, b, rt) |
| + keyA, ownA := launchKey(t, b, first) |
| + if !wellFormedInstanceKey(keyA) || ownA != keyA { |
| + t.Fatal("the import's first launch did not mint a key into its own env file") |
| + } |
| + // The adoption record a helper would have written for this writer. |
| + if err := os.WriteFile(filepath.Join(dir, rt.Host+instancekey.AdoptionSuffix), []byte("{}\n"), 0o600); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + // Abort: the fenced rollback's step 3, as seedwiring wires it. |
| + if err := RetireSeedKey(rt.DBPath, rt.Host, before); err != nil { |
| + t.Fatalf("RetireSeedKey: %v", err) |
| + } |
| + if err := PurgeSeedLocal(rt.DBPath); err != nil { |
| + t.Fatal(err) |
| + } |
| + for _, n := range []string{rt.Host + ".env", rt.Host + instancekey.AdoptionSuffix} { |
| + if _, err := os.Lstat(filepath.Join(dir, n)); !os.IsNotExist(err) { |
| + t.Fatalf("%s survived the rollback (%v): it was the import's", n, err) |
| + } |
| + } |
| + if err := RetireSeedKey(rt.DBPath, rt.Host, before); err != nil { |
| + t.Fatalf("a second retirement: %v", err) |
| + } |
| + |
| + // The owner configures the old process's key B; the config sync |
| + // delivers it. |
| + keyB := strings.Repeat("B", 43) + "=" |
| + if err := os.WriteFile(filepath.Join(dir, rt.Host+".config.env"), []byte(InstanceKeyVar+"="+keyB+"\n"), 0o600); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + // Reimport, same host: its swap finds no env file again, and its first |
| + // launch is handed B. Nothing is minted while configuration sets it. |
| + if before, err := SeedEnvBefore(rt.DBPath, rt.Host); err != nil || before != registry.EnvBeforeAbsent { |
| + t.Fatalf("SeedEnvBefore at the reimport = %q, %v; want absent", before, err) |
| + } |
| + second := importedRoute(t, b, rt) |
| + handed, own := launchKey(t, b, second) |
| + if handed != keyB { |
| + if handed == keyA { |
| + t.Fatal("the reimport was handed the aborted import's key, not the configured one") |
| + } |
| + t.Fatalf("the reimport was handed a key that is not the configured one") |
| + } |
| + if own != "" { |
| + t.Fatal("a key was minted into the own env file while configuration sets one") |
| + } |
| +} |
| + |
| +// Merge review S1: a key, and any env content, that predated the import |
| +// survives its rollback. Only what the import created is retired. |
| +func TestRetireSeedKeyLeavesWhatPredatesTheImport(t *testing.T) { |
| + token := "CARLOS_ADMIN_TOKEN=" + strings.Repeat("e", 64) |
| + t.Run("a key that was there before", func(t *testing.T) { |
| + noOwnReplica(t) |
| + b, rt := nestedBackend(t, &fakeControl{inv: "inv1"}, &fakeUnit{}) |
| + dir := provision.InstanceDir(b.DataDir, rt.Host) |
| + keyP := strings.Repeat("P", 43) + "=" |
| + predating := []byte(token + "\n" + InstanceKeyVar + "=" + keyP + "\n") |
| + if err := os.MkdirAll(dir, 0o700); err != nil { |
| + t.Fatal(err) |
| + } |
| + envPath := filepath.Join(dir, rt.Host+".env") |
| + if err := os.WriteFile(envPath, predating, 0o600); err != nil { |
| + t.Fatal(err) |
| + } |
| + adoption := filepath.Join(dir, rt.Host+instancekey.AdoptionSuffix) |
| + if err := os.WriteFile(adoption, []byte("{}\n"), 0o600); err != nil { |
| + t.Fatal(err) |
| + } |
| + before, err := SeedEnvBefore(rt.DBPath, rt.Host) |
| + if err != nil || before != registry.EnvBeforeKey { |
| + t.Fatalf("SeedEnvBefore = %q, %v; want key", before, err) |
| + } |
| + // The import launches with the key that was there: nothing minted. |
| + if handed, _ := launchKey(t, b, importedRoute(t, b, rt)); handed != keyP { |
| + t.Fatal("the import was not handed the key that predated it") |
| + } |
| + if err := RetireSeedKey(rt.DBPath, rt.Host, before); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := PurgeSeedLocal(rt.DBPath); err != nil { |
| + t.Fatal(err) |
| + } |
| + if got, err := os.ReadFile(envPath); err != nil || string(got) != string(predating) { |
| + t.Fatalf("the env file that predated the import changed: %v", err) |
| + } |
| + if _, err := os.Stat(adoption); err != nil { |
| + t.Fatalf("the adoption record that predated the import went: %v", err) |
| + } |
| + }) |
| + t.Run("an env file with no key", func(t *testing.T) { |
| + noOwnReplica(t) |
| + b, rt := nestedBackend(t, &fakeControl{inv: "inv1"}, &fakeUnit{}) |
| + dir := provision.InstanceDir(b.DataDir, rt.Host) |
| + if err := os.MkdirAll(dir, 0o700); err != nil { |
| + t.Fatal(err) |
| + } |
| + envPath := filepath.Join(dir, rt.Host+".env") |
| + // No trailing newline, a comment, and a line the platform does not own. |
| + predating := "# set by hand\n" + token + "\nOTHER=1" |
| + if err := os.WriteFile(envPath, []byte(predating), 0o600); err != nil { |
| + t.Fatal(err) |
| + } |
| + adoption := filepath.Join(dir, rt.Host+instancekey.AdoptionSuffix) |
| + if err := os.WriteFile(adoption, []byte("{}\n"), 0o600); err != nil { |
| + t.Fatal(err) |
| + } |
| + before, err := SeedEnvBefore(rt.DBPath, rt.Host) |
| + if err != nil || before != registry.EnvBeforeNoKey { |
| + t.Fatalf("SeedEnvBefore = %q, %v; want no-key", before, err) |
| + } |
| + handed, own := launchKey(t, b, importedRoute(t, b, rt)) |
| + if !wellFormedInstanceKey(handed) || own != handed { |
| + t.Fatal("the import's launch did not mint a key into the file") |
| + } |
| + if err := RetireSeedKey(rt.DBPath, rt.Host, before); err != nil { |
| + t.Fatal(err) |
| + } |
| + got, err := os.ReadFile(envPath) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + // Every line that was there is there, in order; the key is not. |
| + if string(got) != predating+"\n" { |
| + t.Fatalf("after the retirement the file is not what predated the import plus its newline (%d bytes, want %d)", len(got), len(predating)+1) |
| + } |
| + if st, _ := os.Stat(envPath); st.Mode().Perm() != 0o600 { |
| + t.Fatalf("mode = %o, want 600", st.Mode().Perm()) |
| + } |
| + if _, err := os.Stat(adoption); err != nil { |
| + t.Fatalf("the adoption record went with a file that predated the import: %v", err) |
| + } |
| + if err := RetireSeedKey(rt.DBPath, rt.Host, before); err != nil { |
| + t.Fatal(err) |
| + } |
| + if again, _ := os.ReadFile(envPath); string(again) != string(got) { |
| + t.Fatal("a second retirement changed the file") |
| + } |
| + }) |
| + t.Run("a journal row that does not say", func(t *testing.T) { |
| + noOwnReplica(t) |
| + b, rt := nestedBackend(t, &fakeControl{inv: "inv1"}, &fakeUnit{}) |
| + handed, own := launchKey(t, b, importedRoute(t, b, rt)) |
| + if err := RetireSeedKey(rt.DBPath, rt.Host, ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, now := launchKeyFile(t, b, rt); now != own || own != handed { |
| + t.Fatal("a key was retired on no record of what predated the import") |
| + } |
| + }) |
| + t.Run("nothing there", func(t *testing.T) { |
| + b, rt := nestedBackend(t, &fakeControl{}, &fakeUnit{}) |
| + for _, before := range []string{registry.EnvBeforeAbsent, registry.EnvBeforeNoKey, registry.EnvBeforeKey, ""} { |
| + if err := RetireSeedKey(rt.DBPath, rt.Host, before); err != nil { |
| + t.Fatalf("%q with no instance directory: %v", before, err) |
| + } |
| + } |
| + if err := RetireSeedKey(rt.DBPath, rt.Host, "whatever"); err == nil { |
| + t.Fatal("an unknown env state was accepted") |
| + } |
| + _ = b |
| + }) |
| +} |
| + |
| +// launchKeyFile reads the own env file's key without launching. |
| +func launchKeyFile(t *testing.T, b *ExecBackend, rt registry.Route) (exists bool, key string) { |
| + t.Helper() |
| + raw, err := os.ReadFile(filepath.Join(provision.InstanceDir(b.DataDir, rt.Host), rt.Host+".env")) |
| + if os.IsNotExist(err) { |
| + return false, "" |
| + } |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + return true, parseEnvBytes(raw)[InstanceKeyVar] |
| +} |
| + |
| +// The swap does not go ahead on an env file it cannot read: a link there is |
| +// refused, never followed, and never read as "no file". |
| +func TestSeedEnvBeforeRefusesALink(t *testing.T) { |
| + b, rt := nestedBackend(t, &fakeControl{}, &fakeUnit{}) |
| + dir := provision.InstanceDir(b.DataDir, rt.Host) |
| + if err := os.MkdirAll(dir, 0o700); err != nil { |
| + t.Fatal(err) |
| + } |
| + target := filepath.Join(t.TempDir(), "elsewhere.env") |
| + if err := os.WriteFile(target, []byte(InstanceKeyVar+"=x\n"), 0o600); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := os.Symlink(target, filepath.Join(dir, rt.Host+".env")); err != nil { |
| + t.Fatal(err) |
| + } |
| + if before, err := SeedEnvBefore(rt.DBPath, rt.Host); err == nil { |
| + t.Fatalf("SeedEnvBefore = %q over a symlink, want an error", before) |
| + } |
| +} |
diff --git a/internal/host/seedwiring.go b/internal/host/seedwiring.go| index 85faac55e..62e71bdf9 100644 |
| --- a/internal/host/seedwiring.go |
| +++ b/internal/host/seedwiring.go |
| @@ -129,7 +129,7 @@ type seedBox struct { |
| // The import swap's and rollback's hooks (spec §5.4, §5.6) are armed only |
| // with claim control off: a claim-armed box refuses seeds until #250, so it |
| // never swaps, and an import journaled before claims came on stays gated |
| -// and untouched there. The rollback's six hooks are armed together or not |
| +// and untouched there. The rollback's seven hooks are armed together or not |
| // at all (provision runs no rollback missing one); without a fence from the |
| // activator none is. |
| // |
| @@ -158,12 +158,14 @@ func wireSeedPlacement(eb *activator.ExecBackend, claimArmed bool, openData, ope |
| o, _, err := seeds.ReadLegacyOwners(ctx, deploy) |
| return o, err |
| } |
| + h.EnvBefore = activator.SeedEnvBefore |
| if box.Fence != nil && box.Swept != nil && box.NextFenceGen != nil { |
| fence := box.Fence |
| h.FenceForRollback = fenceForRollback(fence, box.Swept) |
| h.LiftFence = func(host string, gen uint64) { fence.LiftFence(host, gen) } |
| h.RemoveLocal = func(rt registry.Route) error { return activator.PurgeSeedLocal(rt.DBPath) } |
| h.LocalSeedID = activator.ReadSeedMarker |
| + h.RetireKey = activator.RetireSeedKey |
| h.DeleteReplica = deleteReplica(data, box.Prefix) |
| h.NextFenceGen = box.NextFenceGen |
| } |
diff --git a/internal/host/seedwiring_linux_test.go b/internal/host/seedwiring_linux_test.go| index a316f2c8d..cd7d36002 100644 |
| --- a/internal/host/seedwiring_linux_test.go |
| +++ b/internal/host/seedwiring_linux_test.go |
| @@ -61,3 +61,39 @@ func TestWiredRollbackReadsAndRemovesTheLocalSeed(t *testing.T) { |
| t.Fatalf("a second removal: %v", err) |
| } |
| } |
| + |
| +// Merge review S1: the wired EnvBefore and RetireKey are the activator's |
| +// read of the host's own env file and its retirement of the import's key. |
| +// A file the import created goes; a key that was there before stays. |
| +func TestWiredSwapAndRollbackKnowTheImportsInstanceKey(t *testing.T) { |
| + h := wiredSeedHooks(t, &fakeFence{gens: map[string]uint64{}}, restartTestRegistry(t), |
| + release.NewDirStore(t.TempDir()), release.NewDirStore(t.TempDir())) |
| + const host = "shop-1.example.com" |
| + dir := filepath.Join(t.TempDir(), host) |
| + if err := os.MkdirAll(dir, 0o700); err != nil { |
| + t.Fatal(err) |
| + } |
| + db, env := filepath.Join(dir, host+".db"), filepath.Join(dir, host+".env") |
| + if got, err := h.EnvBefore(db, host); err != nil || got != registry.EnvBeforeAbsent { |
| + t.Fatalf("EnvBefore with no file = %q, %v", got, err) |
| + } |
| + body := activator.InstanceKeyVar + "=minted-by-the-import\n" |
| + if err := os.WriteFile(env, []byte(body), 0o600); err != nil { |
| + t.Fatal(err) |
| + } |
| + if got, err := h.EnvBefore(db, host); err != nil || got != registry.EnvBeforeKey { |
| + t.Fatalf("EnvBefore with a key = %q, %v", got, err) |
| + } |
| + if err := h.RetireKey(db, host, registry.EnvBeforeKey); err != nil { |
| + t.Fatal(err) |
| + } |
| + if got, err := os.ReadFile(env); err != nil || string(got) != body { |
| + t.Fatalf("a key that predated the import was touched: %q, %v", got, err) |
| + } |
| + if err := h.RetireKey(db, host, registry.EnvBeforeAbsent); err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := os.Lstat(env); !os.IsNotExist(err) { |
| + t.Fatalf("the env file the import created survived (%v)", err) |
| + } |
| +} |
diff --git a/internal/host/seedwiring_test.go b/internal/host/seedwiring_test.go| index aad57d368..79f3b33a3 100644 |
| --- a/internal/host/seedwiring_test.go |
| +++ b/internal/host/seedwiring_test.go |
| @@ -493,7 +493,7 @@ func TestRollbackFenceAfterRestart(t *testing.T) { |
| } |
| } |
| -// The rollback's six hooks are all or none (provision skips a rollback |
| +// The rollback's seven hooks are all or none (provision skips a rollback |
| // missing any), so production wiring must set every one; with claim control |
| // on, seeds are refused until #250 and none of the swap's or the |
| // rollback's hooks is armed. |
| @@ -510,6 +510,7 @@ func TestWireSeedPlacementArmsTheSwapAndTheRollback(t *testing.T) { |
| "FenceForRollback": h.FenceForRollback != nil, "LiftFence": h.LiftFence != nil, |
| "RemoveLocal": h.RemoveLocal != nil, "DeleteReplica": h.DeleteReplica != nil, |
| "NextFenceGen": h.NextFenceGen != nil, "LocalSeedID": h.LocalSeedID != nil, |
| + "RetireKey": h.RetireKey != nil, |
| } |
| } |
| @@ -532,6 +533,9 @@ func TestWireSeedPlacementArmsTheSwapAndTheRollback(t *testing.T) { |
| if h.LegacyOwners == nil { |
| t.Fatal("LegacyOwners is not wired") |
| } |
| + if h.EnvBefore == nil { |
| + t.Fatal("EnvBefore is not wired: no import swaps") |
| + } |
| body := `{"v":1,"names":{"old.example.com":{"account":"bbw","app":"shop"}}}` |
| if _, err := deploy.Put(ctx, seeds.LegacyOwnersKey, []byte(body), ""); err != nil { |
| t.Fatal(err) |
| @@ -561,7 +565,7 @@ func TestWireSeedPlacementArmsTheSwapAndTheRollback(t *testing.T) { |
| t.Errorf("the rollback hook %s is armed with claim control on", name) |
| } |
| } |
| - if h.NestedOK != nil || h.LegacyOwners != nil { |
| + if h.NestedOK != nil || h.LegacyOwners != nil || h.EnvBefore != nil { |
| t.Error("the swap's hooks are armed with claim control on") |
| } |
| }) |
diff --git a/internal/provision/plan9_b8_census_test.go b/internal/provision/plan9_b8_census_test.go| index c71306a89..e57845be4 100644 |
| --- a/internal/provision/plan9_b8_census_test.go |
| +++ b/internal/provision/plan9_b8_census_test.go |
| @@ -1127,15 +1127,20 @@ var plan9B8CensusInventory = []censusEntry{ |
| // ready to swapped through SetRecordSeed (SeedSwapped), after the registry transaction committed. |
| {"internal/provision/seedswap.go", "markSwapped", "record-writer", "key:SetRecordSeed", "8d3c844b0370e574b73a7bbdf92d1749cd69ddc343357b5b450059e713d035d2"}, |
| // The swap: it names the journal row's record key, and its one record write before the transaction is a refusal through SetRecordSeed (SeedRefuse), which is all it writes with claims on. |
| - {"internal/provision/seedswap.go", "swapImport", "record-writer", "key:RecordKey,key:SetRecordSeed", "0becc84c60d4e6833f13ceafbc79a45d521d65b5c30d0559b3bf7958cfa60be4"}, |
| + // Re-reviewed for merge review S1 (2026-10-13): before its registry transaction the swap asks the box what the host's own env file holds (SeedHooks.EnvBefore) and keeps the answer in the journal row; a read that fails defers the swap. No record write was added. |
| + {"internal/provision/seedswap.go", "swapImport", "record-writer", "key:RecordKey,key:SetRecordSeed", "af87cf52f0588a695e273f6719c74ad9738db80cbb07282e0dac2a993e610016"}, |
| // RecordKey here is the journal row's column naming its record; a registry transaction, no bucket write. |
| - {"internal/registry/importjournal.go", "Registry.SwapForImport", "key-site", "key:RecordKey", "4068b67e5d24bd900c77f1fceeeec7503f6d6d0365490e4fdec03edaf2c4ecf7"}, |
| + // Re-reviewed for merge review S1 (2026-10-13): the journal row also stores EnvBefore (one of three values, or empty), in the same transaction; still no bucket write. |
| + {"internal/registry/importjournal.go", "Registry.SwapForImport", "key-site", "key:RecordKey", "f55fae3ef3a731c6ea57d82573c214082316a2c7e01255b0e8f549a6d4f25474"}, |
| // Reads the journal's record_key column into the snapshot digest; no bucket write. |
| - {"internal/registry/importjournal.go", "hashImportJournal", "key-site", "key:recordKey", "ebc3baa0bec6ed12e0ae4f3dc49b66707e60faee204b6c1ea754deca6fb52820"}, |
| + // Re-reviewed for merge review S1 (2026-10-13): also folds env_before into the digest, when set; no bucket write. |
| + {"internal/registry/importjournal.go", "hashImportJournal", "key-site", "key:recordKey", "7c60d298ba3a000711feb49c5a2a6964d78c645ee5bbf0253a8d244f19920a46"}, |
| // Scans a journal row, its record key among the columns; no bucket write. |
| - {"internal/registry/importjournal.go", "scanJournalRow", "key-site", "key:RecordKey", "25c53df2c8851091eb8b1f36c824a0b1625c38b40d6c4c12249b708039c5734b"}, |
| + // Re-reviewed for merge review S1 (2026-10-13): scans env_before too; no bucket write. |
| + {"internal/registry/importjournal.go", "scanJournalRow", "key-site", "key:RecordKey", "752a3b638a5580ea0a1d03f3b031b5669539d1cfc899f0a520c51e2c845da983"}, |
| // The journal row's struct; RecordKey is a field naming the import's record. |
| - {"internal/registry/importjournal.go", "type ImportJournal", "key-site", "key:RecordKey", "72300f5bb8d4be7e72c91bb93b097f6ae897b782ad6582db11c07b969cd3baab"}, |
| + // Re-reviewed for merge review S1 (2026-10-13): gains the EnvBefore field; RecordKey is unchanged. |
| + {"internal/registry/importjournal.go", "type ImportJournal", "key-site", "key:RecordKey", "905b3ca8c1a9366d8c4eb310d1fdbfb34c14cc57f30c722f424c3dd46c8d4515"}, |
| // Writes control/seeds-protocol.json by CAS (classified in hostops' mutation classifier); no record key. |
| {"internal/seeds/floor.go", "PublishFloor", "nonrecord", "sel:Put", "3085ce28914a709f1a55c5c430fe9ef9f66b94f1c6a54694d092dcff272978fe"}, |
| // END seeded instances |
diff --git a/internal/provision/seedreconcile.go b/internal/provision/seedreconcile.go| index d5b46b3aa..c45c8083b 100644 |
| --- a/internal/provision/seedreconcile.go |
| +++ b/internal/provision/seedreconcile.go |
| @@ -27,9 +27,16 @@ type SeedHooks struct { |
| // the next pass; it never refuses (only the intent does). Nil: the |
| // intent alone decides. |
| NestedOK func() error |
| + // EnvBefore reports what the host's own env file holds, at the nested |
| + // path beside dbPath, as one of registry's EnvBefore values. The swap |
| + // asks it before the import's row exists and keeps the answer in the |
| + // journal, so a rollback can tell the instance key the import's first |
| + // launch minted from one that was there before (RetireKey). Nil, or an |
| + // error, defers the swap. |
| + EnvBefore func(dbPath, host string) (string, error) |
| - // The rollback's box effects (spec §5.6, seedrollback.go). All six or |
| - // none: a box without them leaves every abandoned import journaled, |
| + // The rollback's box effects (spec §5.6, seedrollback.go). All seven |
| + // or none: a box without them leaves every abandoned import journaled, |
| // gated and untouched. |
| // |
| // FenceForRollback fences and stops host under the rollback's fence |
| @@ -55,6 +62,13 @@ type SeedHooks struct { |
| // ("" for none or no marker), read through instancefs. An error is a |
| // marker that could not be read. |
| LocalSeedID func(dbPath string) (string, error) |
| + // RetireKey retires the instance key the import created in the host's |
| + // own env file beside dbPath, by the journal's EnvBefore: the whole |
| + // file when there was none, its key lines when the file had no key, |
| + // nothing when the key was there before or the journal does not say. |
| + // It never removes a key or a line that predates the import, and it is |
| + // not part of RemoveLocal. Nothing there is not an error. |
| + RetireKey func(dbPath, host, envBefore string) error |
| } |
| type seedVerdict int |
| @@ -210,10 +224,10 @@ func finishAbortedSeed(ctx context.Context, s release.Store, reg *registry.Regis |
| log.Printf("provision: %s: aborted seed %s rolled back (no box had swapped it)", rec.Host, rec.Seed.ID) |
| } |
| -// rollbackArmed reports whether h has the rollback's six box hooks. |
| +// rollbackArmed reports whether h has the rollback's seven box hooks. |
| func rollbackArmed(h *SeedHooks) bool { |
| return h != nil && h.FenceForRollback != nil && h.LiftFence != nil && h.RemoveLocal != nil && h.DeleteReplica != nil && |
| - h.NextFenceGen != nil && h.LocalSeedID != nil |
| + h.NextFenceGen != nil && h.LocalSeedID != nil && h.RetireKey != nil |
| } |
| // purgeAbortedFromLocal removes an aborted --from seed's local database |
diff --git a/internal/provision/seedreconcile_test.go b/internal/provision/seedreconcile_test.go| index 6751eb2d8..afeddd5f3 100644 |
| --- a/internal/provision/seedreconcile_test.go |
| +++ b/internal/provision/seedreconcile_test.go |
| @@ -244,7 +244,7 @@ func TestReconcileNeverRemovesAnAbortedImportsRow(t *testing.T) { |
| } |
| } |
| -// diskBox is the rollback's six box hooks over a real data directory: the |
| +// diskBox is the rollback's seven box hooks over a real data directory: the |
| // marker and the database are files, read and removed as the activator's |
| // ReadSeedMarker and PurgeSeedLocal do (their own tests are the activator's |
| // seedpurge_linux_test.go). It records the steps in order. |
| @@ -310,6 +310,10 @@ func (b *diskBox) hooks(h *SeedHooks) { |
| b.t.Error("the no-journal path deleted a replica") |
| return nil |
| } |
| + h.RetireKey = func(string, string, string) error { |
| + b.t.Error("the no-journal path retired an instance key: only an import's rollback does") |
| + return nil |
| + } |
| } |
| // placeLocal writes a database and its <db>.seed marker naming id at db, |
diff --git a/internal/provision/seedrollback.go b/internal/provision/seedrollback.go| index c52dd33e6..48fbd7c15 100644 |
| --- a/internal/provision/seedrollback.go |
| +++ b/internal/provision/seedrollback.go |
| @@ -18,7 +18,8 @@ import ( |
| // |
| // swapped 1. close the gate (journal rolling-back, with a fence generation) |
| // rolling-back 2. fence and stop the host under the journal's generation |
| -// 3. remove the import's local data |
| +// 3. retire the instance key the import created, then |
| +// remove the import's local data |
| // 4. put the original route back (journal cleanup-pending) |
| // 5. lift the fence |
| // cleanup-pending 6. delete this seed's snapshot and the host's replica |
| @@ -34,6 +35,16 @@ import ( |
| // path is still removed when the database's <db>.seed marker names this |
| // seed and the host still has no row. |
| // |
| +// Step 3 retires the import's instance key first (merge review S1). The |
| +// key the import's first launch minted into the host's own env file would |
| +// otherwise outlive the rollback and outrank the key a second import of |
| +// the host configures. The journal's EnvBefore, recorded at the swap, says |
| +// what was there before the import: only what the import created goes |
| +// (SeedHooks.RetireKey). With the row gone the key is retired under the |
| +// same marker rule as the database, and before it, so the marker that |
| +// proves the installation is the import's is still there if the key's |
| +// removal has to be tried again. |
| +// |
| // rec is the pass's view of the journal's record (nil when gone). It chose |
| // the rollback; the destructive cleanup reads the record again (step 6). |
| func rollbackImport(ctx context.Context, s release.Store, reg *registry.Registry, cfg ReconcileConfig, j registry.ImportJournal, rec *Record) { |
| @@ -42,8 +53,7 @@ func rollbackImport(ctx context.Context, s release.Store, reg *registry.Registry |
| log.Printf("provision: dry-run: would roll back %s's import %s", j.Host, j.SeedID) |
| return |
| } |
| - if h == nil || h.FenceForRollback == nil || h.LiftFence == nil || h.RemoveLocal == nil || h.DeleteReplica == nil || h.NextFenceGen == nil || |
| - h.LocalSeedID == nil { |
| + if !rollbackArmed(h) { |
| log.Printf("provision: %s: import %s cannot roll back on this box: the activator is not wired; it stays journaled and gated", j.Host, j.SeedID) |
| return |
| } |
| @@ -144,6 +154,10 @@ func rollBackRoute(ctx context.Context, s release.Store, reg *registry.Registry, |
| log.Printf("provision: %s: import %s is not rolled back: the route changed under the fence, so nothing there is touched", j.Host, j.SeedID) |
| return j, false |
| default: |
| + if err := h.RetireKey(row.DBPath, j.Host, j.EnvBefore); err != nil { |
| + log.Printf("provision: %s: import %s: retire its instance key: %v", j.Host, j.SeedID, err) |
| + return j, false |
| + } |
| if err := h.RemoveLocal(*row); err != nil { |
| log.Printf("provision: %s: import %s: remove its local data: %v", j.Host, j.SeedID, err) |
| return j, false |
| @@ -227,8 +241,9 @@ func cleanUpImport(ctx context.Context, s release.Store, reg *registry.Registry, |
| // has that file open. The database is removed only when its <db>.seed |
| // marker names this seed and the host still has no row. A marker that is |
| // absent or names another seed does not prove the data is the import's, |
| -// so the file is left. ok false defers the rollback, with the fence |
| -// lifted. |
| +// so the file is left. The import's instance key is retired by the same |
| +// rule, just before the database, and left with it. ok false defers the |
| +// rollback, with the fence lifted. |
| func removeOrphanedLocal(reg *registry.Registry, cfg ReconcileConfig, j registry.ImportJournal) (ok bool) { |
| h := cfg.Seeds |
| db := NestedDBPath(cfg.DataDir, j.Host) |
| @@ -254,6 +269,12 @@ func removeOrphanedLocal(reg *registry.Registry, cfg ReconcileConfig, j registry |
| log.Printf("provision: %s: import %s: a route appeared at the host, so its local database is left and the rollback waits", j.Host, j.SeedID) |
| return false |
| } |
| + // The key before the database: the marker just read is what says this |
| + // installation is the import's, and RemoveLocal removes it. |
| + if err := h.RetireKey(db, j.Host, j.EnvBefore); err != nil { |
| + log.Printf("provision: %s: import %s: retire its instance key: %v", j.Host, j.SeedID, err) |
| + return false |
| + } |
| if err := h.RemoveLocal(registry.Route{Host: j.Host, Kind: "instance", DBPath: db}); err != nil { |
| log.Printf("provision: %s: import %s: remove its local data: %v", j.Host, j.SeedID, err) |
| return false |
diff --git a/internal/provision/seedrollback_test.go b/internal/provision/seedrollback_test.go| index 7ac0f5ae2..cb7c7e80d 100644 |
| --- a/internal/provision/seedrollback_test.go |
| +++ b/internal/provision/seedrollback_test.go |
| @@ -23,6 +23,9 @@ type rollbackRecorder struct { |
| fenceErr error |
| gen uint64 |
| marker string // the seed id the local database's marker names |
| + // retired is each RetireKey call, "<db path> <host> <env before>". It |
| + // is kept apart from steps, whose sequences predate the hook. |
| + retired []string |
| } |
| func (r *rollbackRecorder) hooks(h *SeedHooks) { |
| @@ -35,6 +38,10 @@ func (r *rollbackRecorder) hooks(h *SeedHooks) { |
| h.RemoveLocal = func(rt registry.Route) error { r.steps = append(r.steps, "remove-local:"+rt.Kind); return nil } |
| h.DeleteReplica = func(_ context.Context, host string) error { r.steps = append(r.steps, "delete-replica"); return nil } |
| h.LocalSeedID = func(string) (string, error) { return r.marker, nil } |
| + h.RetireKey = func(db, host, before string) error { |
| + r.retired = append(r.retired, db+" "+host+" "+before) |
| + return nil |
| + } |
| } |
| func TestRollbackAfterAbortRestoresTheRowExactly(t *testing.T) { |
| @@ -270,10 +277,14 @@ type rollbackBox struct { |
| noRoute bool // the import's row was removed: no route comes back |
| marker string // the seed id the local database's <db>.seed names |
| markErr error // reading that marker fails |
| + // retired is each RetireKey call's env-before value, kept apart from |
| + // steps; key is whether the import's minted key is still on the box. |
| + retired []string |
| + key bool |
| } |
| func newRollbackBox(t *testing.T, reg *registry.Registry, original registry.Route) *rollbackBox { |
| - return &rollbackBox{t: t, reg: reg, original: original, proceed: true, fenced: map[string]uint64{}, local: true, replica: true} |
| + return &rollbackBox{t: t, reg: reg, original: original, proceed: true, fenced: map[string]uint64{}, local: true, replica: true, key: true} |
| } |
| func (b *rollbackBox) trip(step string) error { |
| @@ -378,6 +389,29 @@ func (b *rollbackBox) hooks(h *SeedHooks) { |
| b.local = false |
| return b.trip("remove-local-done") |
| } |
| + h.RetireKey = func(dbPath, host, before string) error { |
| + b.retired = append(b.retired, before) |
| + if _, ok := b.fenced[host]; !ok { |
| + b.t.Errorf("retired the instance key of a host that is not fenced and stopped") |
| + } |
| + if j := b.journal(); j == nil || j.State != registry.JournalRollingBack || j.EnvBefore != before { |
| + b.t.Errorf("retired the instance key by %q under journal %+v", before, j) |
| + } |
| + if dbPath != NestedDBPath("/data", host) { |
| + b.t.Errorf("retired the instance key beside %s", dbPath) |
| + } |
| + // Before the local data: its marker is the missing-row path's |
| + // evidence that the installation is the import's. (A resumed pass |
| + // retires again after the data went; that is a no-op.) |
| + if b.key && !b.local { |
| + b.t.Errorf("the instance key was first retired after the local data was removed") |
| + } |
| + if err := b.trip("retire-key"); err != nil { |
| + return err |
| + } |
| + b.key = false |
| + return nil |
| + } |
| h.LocalSeedID = func(dbPath string) (string, error) { |
| b.steps = append(b.steps, "read-marker") |
| if dbPath != NestedDBPath("/data", b.original.Host) { |
| @@ -529,6 +563,16 @@ func (x *rollbackScene) wantRolledBack(t *testing.T) { |
| if x.box.local || x.box.replica || len(x.box.fenced) != 0 { |
| t.Fatalf("local %v replica %v fenced %v", x.box.local, x.box.replica, x.box.fenced) |
| } |
| + // The key the import's verify minted is retired, by what the swap |
| + // recorded: this host had no env file before the import (importCfg). |
| + if x.box.key || len(x.box.retired) == 0 { |
| + t.Fatalf("the import's instance key was not retired (calls %q)", x.box.retired) |
| + } |
| + for _, before := range x.box.retired { |
| + if before != registry.EnvBeforeAbsent { |
| + t.Fatalf("retired by %q, want what the swap recorded (%q)", before, registry.EnvBeforeAbsent) |
| + } |
| + } |
| if st := seedState(t, x.raw, x.rec); st.ID != x.rec.Seed.ID || st.State != seeds.StateRolledBack { |
| t.Fatalf("record seed = %+v, want rolled-back", st) |
| } |
| @@ -570,6 +614,7 @@ func TestRollbackResumesAfterAFailureBetweenAnyTwoSteps(t *testing.T) { |
| }{ |
| {name: "closing the gate", trigger: `CREATE TRIGGER fault BEFORE UPDATE OF state ON import_journal WHEN NEW.state = 'rolling-back'` + raise, after: registry.JournalSwapped}, |
| {name: "fencing", hook: "fence", after: registry.JournalRollingBack}, |
| + {name: "retiring the instance key", hook: "retire-key", after: registry.JournalRollingBack}, |
| {name: "removing the local data", hook: "remove-local", after: registry.JournalRollingBack}, |
| {name: "after removing the local data", hook: "remove-local-done", after: registry.JournalRollingBack}, |
| {name: "restoring the route", trigger: `CREATE TRIGGER fault BEFORE UPDATE OF state ON import_journal WHEN NEW.state = 'cleanup-pending'` + raise, after: registry.JournalRollingBack}, |
| @@ -1006,6 +1051,11 @@ func TestRollbackOfAMissingRowDecidesTheLocalDataFromItsMarker(t *testing.T) { |
| if x.box.local == c.removed || (count(x.box.steps, "remove-local") == 1) != c.removed { |
| t.Fatalf("local %v steps %q; want removed %v", x.box.local, x.box.steps, c.removed) |
| } |
| + // The instance key follows the database, by the same marker: |
| + // retired with it, left with it (merge review S1). |
| + if x.box.key == c.removed || (len(x.box.retired) == 1) != c.removed { |
| + t.Fatalf("key left %v, retire calls %q; want retired %v", x.box.key, x.box.retired, c.removed) |
| + } |
| if len(x.box.fenced) != 0 { |
| t.Fatalf("a fence was left: %v", x.box.fenced) |
| } |
| @@ -1160,3 +1210,116 @@ func TestRollbackWaitsForAFenceGeneration(t *testing.T) { |
| }) |
| } |
| } |
| + |
| +// Merge review S1: the swap records what the host's own env file held |
| +// before the import, in the journal, and swaps only when it could read it. |
| +func TestSwapRecordsTheEnvFileBeforeTheImport(t *testing.T) { |
| + for _, before := range []string{registry.EnvBeforeAbsent, registry.EnvBeforeNoKey, registry.EnvBeforeKey} { |
| + t.Run(before, func(t *testing.T) { |
| + s, sqid := shopSetup(t) |
| + rec := importRecord(t, s, sqid) |
| + reg := regWith(t, addrRow(rec, sqid)) |
| + cfg := importCfg(t, nil) |
| + var asked []string |
| + cfg.Seeds.EnvBefore = func(db, host string) (string, error) { |
| + // Asked before the import's row exists. |
| + if rt := mustGet(t, reg, rec.Host); rt.ImportGate != "" || rt.Kind != "service" && rt.Addr == "" { |
| + t.Errorf("the env file was read after the swap: %+v", rt) |
| + } |
| + asked = append(asked, db+" "+host) |
| + return before, nil |
| + } |
| + if err := Reconcile(ctx, s, reg, cfg); err != nil { |
| + t.Fatal(err) |
| + } |
| + j, err := reg.ImportJournal(rec.Host) |
| + if err != nil || j == nil || j.EnvBefore != before { |
| + t.Fatalf("journal = %+v, %v; want env before %q", j, err, before) |
| + } |
| + if want := []string{NestedDBPath("/data", rec.Host) + " " + rec.Host}; !reflect.DeepEqual(asked, want) { |
| + t.Fatalf("asked %q, want %q", asked, want) |
| + } |
| + }) |
| + } |
| + for name, hook := range map[string]func(string, string) (string, error){ |
| + "the file cannot be read": func(string, string) (string, error) { return "", errors.New("a symlink") }, |
| + "no reader": nil, |
| + } { |
| + t.Run(name, func(t *testing.T) { |
| + s, sqid := shopSetup(t) |
| + rec := importRecord(t, s, sqid) |
| + reg := regWith(t, addrRow(rec, sqid)) |
| + original := *mustGet(t, reg, rec.Host) |
| + cfg := importCfg(t, nil) |
| + cfg.Seeds.EnvBefore = hook |
| + if err := Reconcile(ctx, s, reg, cfg); err != nil { |
| + t.Fatal(err) |
| + } |
| + if j, _ := reg.ImportJournal(rec.Host); j != nil { |
| + t.Fatalf("swapped without knowing what the env file held: %+v", j) |
| + } |
| + if got := mustGet(t, reg, rec.Host); !reflect.DeepEqual(*got, original) { |
| + t.Fatalf("row = %+v, want the original untouched", got) |
| + } |
| + if st := seedState(t, s, rec); st.State != seeds.StateReady { |
| + t.Fatalf("record = %s, want ready: a deferred swap refuses nothing", st.State) |
| + } |
| + }) |
| + } |
| +} |
| + |
| +// Merge review S1: the rollback hands the journal's record of the env file |
| +// to the key's retirement, for the import's own row, whatever it says. A |
| +// key that was there before the import is the hook's to leave (the |
| +// activator's RetireSeedKey; its own tests cover the file). |
| +func TestRollbackRetiresTheKeyByWhatTheSwapRecorded(t *testing.T) { |
| + for _, before := range []string{registry.EnvBeforeAbsent, registry.EnvBeforeNoKey, registry.EnvBeforeKey} { |
| + t.Run(before, func(t *testing.T) { |
| + s, sqid := shopSetup(t) |
| + rec := importRecord(t, s, sqid) |
| + reg := regWith(t, addrRow(rec, sqid)) |
| + cfg := importCfg(t, nil) |
| + cfg.Seeds.EnvBefore = func(string, string) (string, error) { return before, nil } |
| + rr := &rollbackRecorder{proceed: true} |
| + rr.hooks(cfg.Seeds) |
| + Reconcile(ctx, s, reg, cfg) // swap |
| + if _, err := SetRecordSeed(ctx, s, rec.Account, rec.App, rec.Host, SeedAbort(rec.Seed.ID)); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := Reconcile(ctx, s, reg, cfg); err != nil { |
| + t.Fatal(err) |
| + } |
| + want := []string{NestedDBPath("/data", rec.Host) + " " + rec.Host + " " + before} |
| + if !reflect.DeepEqual(rr.retired, want) { |
| + t.Fatalf("retired %q, want %q", rr.retired, want) |
| + } |
| + }) |
| + } |
| +} |
| + |
| +// A retirement that fails keeps the rollback where it was, fenced, with |
| +// the local data and the gated row in place; the next pass tries again. |
| +func TestRollbackWaitsWhileTheKeyCannotBeRetired(t *testing.T) { |
| + x := abortedImport(t) |
| + retire := x.cfg.Seeds.RetireKey |
| + fail := true |
| + x.cfg.Seeds.RetireKey = func(db, host, before string) error { |
| + if fail { |
| + return errors.New("the env file is a symlink") |
| + } |
| + return retire(db, host, before) |
| + } |
| + x.pass(t) |
| + if st := x.journalState(t); st != registry.JournalRollingBack { |
| + t.Fatalf("journal = %q, want rolling-back", st) |
| + } |
| + if !x.box.local || !x.box.key || count(x.box.steps, "remove-local") != 0 { |
| + t.Fatalf("local %v key %v steps %q: the rollback went on past a key it could not retire", x.box.local, x.box.key, x.box.steps) |
| + } |
| + if got := mustGet(t, x.reg, x.rec.Host); got.ImportGate != registry.ImportGateClosed { |
| + t.Fatalf("row = %+v, want the closed import row", got) |
| + } |
| + fail = false |
| + x.pass(t) |
| + x.wantRolledBack(t) |
| +} |
diff --git a/internal/provision/seedswap.go b/internal/provision/seedswap.go| index edb29c0cd..2fc4dd352 100644 |
| --- a/internal/provision/seedswap.go |
| +++ b/internal/provision/seedswap.go |
| @@ -242,12 +242,25 @@ func swapImport(ctx context.Context, s release.Store, reg *registry.Registry, cf |
| return // the gate logs its own refusal; retried every pass |
| } |
| } |
| + // What the host's own env file holds before the import's row exists, |
| + // for the journal: the rollback retires only the instance key the |
| + // import's launches created (merge review S1). Read last, so nothing |
| + // can launch the import between this read and the transaction. |
| + if cfg.Seeds.EnvBefore == nil { |
| + log.Printf("provision: %s: import deferred: this box cannot read the instance's env file", rec.Host) |
| + return |
| + } |
| + envBefore, err := cfg.Seeds.EnvBefore(next.DBPath, rec.Host) |
| + if err != nil { |
| + log.Printf("provision: %s: import deferred: the instance's env file could not be read: %v", rec.Host, err) |
| + return |
| + } |
| now := time.Now |
| if cfg.Now != nil { |
| now = cfg.Now |
| } |
| journal := registry.ImportJournal{Host: rec.Host, SeedID: rec.Seed.ID, Original: *existing, |
| - RecordKey: RecordKey(rec.Account, rec.App, rec.Host), At: now().Unix()} |
| + RecordKey: RecordKey(rec.Account, rec.App, rec.Host), At: now().Unix(), EnvBefore: envBefore} |
| if err := reg.SwapForImport(journal, next); err != nil { |
| log.Printf("provision: %s: import swap deferred: %v", rec.Host, err) |
| return |
diff --git a/internal/provision/seedswap_test.go b/internal/provision/seedswap_test.go| index 54eff2125..c4d9e07fa 100644 |
| --- a/internal/provision/seedswap_test.go |
| +++ b/internal/provision/seedswap_test.go |
| @@ -62,6 +62,9 @@ func importCfg(t *testing.T, owners map[string]seeds.LegacyOwner) ReconcileConfi |
| cfg.Seeds.LegacyOwners = func(context.Context) (*seeds.LegacyOwners, error) { |
| return &seeds.LegacyOwners{V: 1, Names: owners}, nil |
| } |
| + // The host has no env file before the import, as a host the platform |
| + // never ran has none. |
| + cfg.Seeds.EnvBefore = func(string, string) (string, error) { return registry.EnvBeforeAbsent, nil } |
| return cfg |
| } |
diff --git a/internal/registry/importjournal.go b/internal/registry/importjournal.go| index 6ff16b997..5dc58b389 100644 |
| --- a/internal/registry/importjournal.go |
| +++ b/internal/registry/importjournal.go |
| @@ -8,6 +8,7 @@ import ( |
| "fmt" |
| "io" |
| "reflect" |
| + "strings" |
| ) |
| // The import journal (seeded-instances spec §5.7): one row per host this |
| @@ -27,8 +28,23 @@ const ImportGateClosed = "closed" |
| // hold: no row or another seed's, a row already there, a changed route. |
| var ErrImportJournal = errors.New("registry: the import journal does not allow this") |
| +// What the host's own env file (<host>.env, at the nested path the swap |
| +// gives the import's row) held when the swap ran, before any launch of the |
| +// import: ImportJournal.EnvBefore. The platform mints the instance key into |
| +// that file at the import's first launch, create-only, and a key there |
| +// outranks the app's configuration, so a rollback has to know whether the |
| +// key it finds is the import's own (it retires it) or was there before (it |
| +// never touches it). "" is a journal row written before this was recorded: |
| +// nothing is known, so nothing is retired. |
| +const ( |
| + EnvBeforeAbsent = "absent" // no env file: every byte in it later is the import's |
| + EnvBeforeNoKey = "no-key" // an env file with no instance key: a key in it later is the import's |
| + EnvBeforeKey = "key" // an env file that already carried a key: the import minted none |
| +) |
| + |
| // ImportJournal is one import_journal row. Original is the route the swap |
| -// replaced, exactly as it was stored. |
| +// replaced, exactly as it was stored. EnvBefore is one of the EnvBefore |
| +// values, or "" for unknown. |
| type ImportJournal struct { |
| Host string |
| SeedID string |
| @@ -37,19 +53,28 @@ type ImportJournal struct { |
| State string |
| FenceGen uint64 |
| At int64 |
| + EnvBefore string |
| } |
| func (r *Registry) migrateImportJournal() error { |
| - _, err := r.sql.Exec(`CREATE TABLE IF NOT EXISTS import_journal ( |
| + if _, err := r.sql.Exec(`CREATE TABLE IF NOT EXISTS import_journal ( |
| host TEXT PRIMARY KEY, |
| seed_id TEXT NOT NULL, |
| original TEXT NOT NULL, |
| record_key TEXT NOT NULL DEFAULT '', |
| state TEXT NOT NULL, |
| fence_gen INTEGER NOT NULL DEFAULT 0, |
| - at INTEGER NOT NULL DEFAULT 0 |
| -);`) |
| - return err |
| + at INTEGER NOT NULL DEFAULT 0, |
| + env_before TEXT NOT NULL DEFAULT '' |
| +);`); err != nil { |
| + return err |
| + } |
| + // A journal table from before env_before gains it; its rows read "". |
| + if _, err := r.sql.Exec(`ALTER TABLE import_journal ADD COLUMN env_before TEXT NOT NULL DEFAULT ''`); err != nil && |
| + !strings.Contains(err.Error(), "duplicate column") { |
| + return err |
| + } |
| + return nil |
| } |
| type queryer interface { |
| @@ -61,12 +86,12 @@ type txer interface { |
| Exec(query string, args ...any) (sql.Result, error) |
| } |
| -const journalCols = `host, seed_id, original, record_key, state, fence_gen, at` |
| +const journalCols = `host, seed_id, original, record_key, state, fence_gen, at, env_before` |
| func scanJournalRow(scan func(...any) error) (ImportJournal, error) { |
| var j ImportJournal |
| var orig string |
| - if err := scan(&j.Host, &j.SeedID, &orig, &j.RecordKey, &j.State, &j.FenceGen, &j.At); err != nil { |
| + if err := scan(&j.Host, &j.SeedID, &orig, &j.RecordKey, &j.State, &j.FenceGen, &j.At, &j.EnvBefore); err != nil { |
| return j, err |
| } |
| if err := json.Unmarshal([]byte(orig), &j.Original); err != nil { |
| @@ -130,17 +155,23 @@ func hashImportJournal(db *sql.DB, h io.Writer) error { |
| defer rows.Close() |
| first := true |
| for rows.Next() { |
| - var host, seedID, orig, recordKey, state string |
| + var host, seedID, orig, recordKey, state, envBefore string |
| var fenceGen uint64 |
| var at int64 |
| - if err := rows.Scan(&host, &seedID, &orig, &recordKey, &state, &fenceGen, &at); err != nil { |
| + if err := rows.Scan(&host, &seedID, &orig, &recordKey, &state, &fenceGen, &at, &envBefore); err != nil { |
| return err |
| } |
| if first { |
| fmt.Fprint(h, "\x1dimport_journal\x1e") |
| first = false |
| } |
| - fmt.Fprintf(h, "%q\x1f%q\x1f%q\x1f%q\x1f%q\x1f%d\x1f%d\x1e", host, seedID, orig, recordKey, state, fenceGen, at) |
| + fmt.Fprintf(h, "%q\x1f%q\x1f%q\x1f%q\x1f%q\x1f%d\x1f%d", host, seedID, orig, recordKey, state, fenceGen, at) |
| + // env_before is hashed only when set, tagged, so a row written |
| + // before the column existed digests as it did. |
| + if envBefore != "" { |
| + fmt.Fprintf(h, "\x1fenv_before=%q", envBefore) |
| + } |
| + fmt.Fprint(h, "\x1e") |
| } |
| return rows.Err() |
| } |
| @@ -225,11 +256,17 @@ func seedColumnID(col string) string { |
| // can never be decided, so it is never written. The original must be one |
| // the journal can put back: not claim-owned (claims are off for imports, |
| // spec §3.4), not held by an operation, not itself gated, and unchanged by |
| -// its JSON round trip. |
| +// its JSON round trip. j.EnvBefore is stored as given: one of the EnvBefore |
| +// values, or "" when the caller could not say. |
| func (r *Registry) SwapForImport(j ImportJournal, next Route) error { |
| if j.Host == "" || j.SeedID == "" || j.RecordKey == "" || j.Original.Host != j.Host || !IsImportRow(normalizeRoute(next), j) { |
| return fmt.Errorf("%w: the swap for %s is not well formed", ErrImportJournal, j.Host) |
| } |
| + switch j.EnvBefore { |
| + case "", EnvBeforeAbsent, EnvBeforeNoKey, EnvBeforeKey: |
| + default: |
| + return fmt.Errorf("%w: the swap for %s is not well formed", ErrImportJournal, j.Host) |
| + } |
| if j.Original.ClaimOwnership != (ClaimOwnership{}) || j.Original.HoldOp != "" || j.Original.ImportGate != "" { |
| return fmt.Errorf("%w: the route at %s is not one an import replaces", ErrImportJournal, j.Host) |
| } |
| @@ -260,8 +297,8 @@ func (r *Registry) SwapForImport(j ImportJournal, next Route) error { |
| if err := insertRouteTx(tx, normalizeRoute(next)); err != nil { |
| return err |
| } |
| - _, err = tx.Exec(`INSERT INTO import_journal (`+journalCols+`) VALUES (?,?,?,?,?,0,?)`, |
| - j.Host, j.SeedID, string(orig), j.RecordKey, JournalSwapped, j.At) |
| + _, err = tx.Exec(`INSERT INTO import_journal (`+journalCols+`) VALUES (?,?,?,?,?,0,?,?)`, |
| + j.Host, j.SeedID, string(orig), j.RecordKey, JournalSwapped, j.At, j.EnvBefore) |
| return err |
| }) |
| } |
diff --git a/internal/registry/importjournal_test.go b/internal/registry/importjournal_test.go| index b0dc4bc3e..98145814c 100644 |
| --- a/internal/registry/importjournal_test.go |
| +++ b/internal/registry/importjournal_test.go |
| @@ -806,3 +806,86 @@ func TestARegistryFromEitherSideGainsTheOthersColumns(t *testing.T) { |
| }) |
| } |
| } |
| + |
| +// Merge review S1: the journal keeps what the host's own env file held |
| +// before the import (EnvBefore), through every state, so the rollback can |
| +// tell the key the import minted from one that predates it. |
| +func TestImportJournalKeepsEnvBefore(t *testing.T) { |
| + for _, before := range []string{"", EnvBeforeAbsent, EnvBeforeNoKey, EnvBeforeKey} { |
| + reg := openTestRegistry(t) |
| + old := legacyRow() |
| + if err := reg.Upsert(old); err != nil { |
| + t.Fatal(err) |
| + } |
| + j := ImportJournal{Host: old.Host, SeedID: importSeedID, RecordKey: importRecordKey, Original: mustRoute(t, reg, old.Host), At: 1, EnvBefore: before} |
| + if err := reg.SwapForImport(j, swapRow()); err != nil { |
| + t.Fatalf("%q: %v", before, err) |
| + } |
| + if got := mustJournal(t, reg, old.Host); got == nil || got.EnvBefore != before { |
| + t.Fatalf("after the swap: %+v, want env before %q", got, before) |
| + } |
| + if err := reg.CloseImportGate(old.Host, j.SeedID, 7); err != nil { |
| + t.Fatal(err) |
| + } |
| + if got := mustJournal(t, reg, old.Host); got.EnvBefore != before || got.State != JournalRollingBack { |
| + t.Fatalf("rolling back: %+v, want env before %q", got, before) |
| + } |
| + if js, err := reg.ImportJournals(); err != nil || len(js) != 1 || js[0].EnvBefore != before { |
| + t.Fatalf("ImportJournals = %+v, %v", js, err) |
| + } |
| + } |
| + reg := openTestRegistry(t) |
| + old := legacyRow() |
| + if err := reg.Upsert(old); err != nil { |
| + t.Fatal(err) |
| + } |
| + j := ImportJournal{Host: old.Host, SeedID: importSeedID, RecordKey: importRecordKey, Original: mustRoute(t, reg, old.Host), At: 1, EnvBefore: "maybe"} |
| + if err := reg.SwapForImport(j, swapRow()); !errors.Is(err, ErrImportJournal) { |
| + t.Fatalf("a swap with an env state that is none of the recorded ones: %v", err) |
| + } |
| +} |
| + |
| +// A journal table from before env_before gains the column on open, and its |
| +// rows read as "nothing recorded". Such a row digests as it did; a |
| +// recorded value moves the snapshot digest. |
| +func TestImportJournalEnvBeforeMigratesAndIsHashedWhenSet(t *testing.T) { |
| + path := filepath.Join(t.TempDir(), "registry.db") |
| + r, err := Open(path) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + old := legacyRow() |
| + if err := r.Upsert(old); err != nil { |
| + t.Fatal(err) |
| + } |
| + j := ImportJournal{Host: old.Host, SeedID: importSeedID, RecordKey: importRecordKey, Original: mustRoute(t, r, old.Host), At: 1} |
| + if err := r.SwapForImport(j, swapRow()); err != nil { |
| + t.Fatal(err) |
| + } |
| + unset := mustDigest(t, r.SnapshotFingerprint) |
| + if _, err := r.sql.Exec(`ALTER TABLE import_journal DROP COLUMN env_before`); err != nil { |
| + t.Fatal(err) |
| + } |
| + r.Close() |
| + for i := 0; i < 2; i++ { |
| + if r, err = Open(path); err != nil { |
| + t.Fatalf("open %d: %v", i, err) |
| + } |
| + if got := mustJournal(t, r, old.Host); got == nil || got.EnvBefore != "" || got.SeedID != importSeedID { |
| + t.Fatalf("open %d: journal %+v", i, got) |
| + } |
| + if got := mustDigest(t, r.SnapshotFingerprint); got != unset { |
| + t.Fatalf("open %d: a row with nothing recorded digests %s, was %s", i, got, unset) |
| + } |
| + if i == 0 { |
| + r.Close() |
| + } |
| + } |
| + defer r.Close() |
| + if _, err := r.sql.Exec(`UPDATE import_journal SET env_before = ? WHERE host = ?`, EnvBeforeAbsent, old.Host); err != nil { |
| + t.Fatal(err) |
| + } |
| + if mustDigest(t, r.SnapshotFingerprint) == unset { |
| + t.Fatal("a recorded env state did not move SnapshotFingerprint") |
| + } |
| +} |