amadan
accounts: name the legacy key space, so ops apps can leave it (#8)
`accounts migrate` could not express "move this into ops". A bare move
into ops is refused — ops is also the default source, and the verb will
not guess a direction — and -from only took a sqid, which the
account-less key space does not have. So every app still on apps/<app>/…
was stranded there, and the read-fallback could never be retired: there
was no way to get the objects onto qualified keys.
-from legacy names that key space. It is checked before ValidAccount,
because acctRe is [a-z]{3,16} and "legacy" satisfies it; the word is safe
as a sentinel only because the frozen sqid alphabet omits i, l and o, so
Sqid() can never emit it. TestLegacySourceIsNotASqid derives the emitted
character set from the encoder and fails the day that stops being true.
The refusal now names the fix instead of leaving the operator to find it,
and "legacy" is rejected as a DESTINATION — emptying that key space is
the point of the verb.
The e2e test asserts on KEYS, not on what resolves: ops already serves a
legacy app through the read-fallback, so release.Channel answers v1
whether or not anything migrated. Putting the objects at the qualified
keys is the thing being tested.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
4 files changed,
+210
−4
accounts.go+37 −4e2e_test.go+130 −0internal/domains/sqid_test.go+29 −0internal/release/fallback.go+14 −0
diff --git a/accounts.go b/accounts.go| index 95a586843..e86d7da2c 100644 |
| --- a/accounts.go |
| +++ b/accounts.go |
| @@ -235,12 +235,15 @@ func runAccountsMigrate(args []string) { |
| regPath := fs.String("registry", defaultRegistry(), "registry database") |
| app := fs.String("app", "", "app to move") |
| to := fs.String("to", "", "destination account sqid") |
| - from := fs.String("from", "", "source account sqid (default: the legacy, account-less keys)") |
| + from := fs.String("from", "", "source account sqid, or \"legacy\" for the pre-accounts keys (default: ops, then legacy)") |
| dryRun := fs.Bool("dry-run", false, "print the plan and touch nothing") |
| fs.Parse(args) |
| if *app == "" || *to == "" { |
| log.Fatal("usage: carlos accounts migrate -app <a> -to <sqid> [-from <sqid>] [-dry-run]") |
| } |
| + if *to == release.LegacySource { |
| + log.Fatalf("accounts migrate: %q is a source, never a destination — the account-less key space is what this verb exists to empty", release.LegacySource) |
| + } |
| if !release.ValidAccount(*to) { |
| log.Fatalf("accounts migrate: %q is not an account sqid", *to) |
| } |
| @@ -416,6 +419,26 @@ func putOverwrite(ctx context.Context, s release.Store, key string, body []byte) |
| // Returns the copies to make and the account the app is being moved FROM (for |
| // the post-copy verification). |
| func migrationPlan(ctx context.Context, store release.Store, app, from, to, dst string) ([]copyOp, string) { |
| + // The account-less keys, named explicitly. Checked BEFORE ValidAccount: |
| + // acctRe is [a-z]{3,16}, which "legacy" satisfies, so the order is what |
| + // keeps the sentinel from being read as an account sqid. |
| + if from == release.LegacySource { |
| + src := release.LegacyAppPrefix(app) |
| + keys, err := store.List(ctx, src) |
| + if err != nil { |
| + log.Fatalf("accounts migrate: list %s: %v", src, err) |
| + } |
| + if len(keys) == 0 { |
| + log.Fatalf("accounts migrate: nothing under %s — %s has no legacy objects to move", src, app) |
| + } |
| + fmt.Printf("source: %s (the legacy, account-less keys)\n", src) |
| + var out []copyOp |
| + for _, k := range keys { |
| + rel := strings.TrimPrefix(k, src) |
| + out = append(out, copyOp{k, dst + rel, isChannelPointer(rel)}) |
| + } |
| + return out, release.LegacySource |
| + } |
| if from != "" { |
| if !release.ValidAccount(from) { |
| log.Fatalf("accounts migrate: %q is not an account sqid", from) |
| @@ -437,7 +460,10 @@ func migrationPlan(ctx context.Context, store release.Store, app, from, to, dst |
| } |
| if to == release.OpsAccount { |
| - log.Fatalf("accounts migrate: moving into ops needs an explicit -from — ops is also the default SOURCE, and guessing which way you meant is not something this verb will do") |
| + log.Fatalf("accounts migrate: moving into ops needs an explicit -from — ops is also the default\n" + |
| + "SOURCE, and guessing which way you meant is not something this verb will do.\n" + |
| + "For an app still on the pre-accounts keys (apps/<app>/…), name them:\n\n" + |
| + " carlos accounts migrate -app <app> -from legacy -to bab\n") |
| } |
| opsPrefix := release.AppPrefix(release.OpsAccount, app) |
| legacyPrefix := release.LegacyAppPrefix(app) |
| @@ -507,9 +533,16 @@ func verifyMigratedPointers(ctx context.Context, store release.Store, app, srcAc |
| log.Fatalf("accounts migrate: verify %s: %v", key, err) |
| } |
| // What a reader on the OLD account resolves right now, by the same |
| - // precedence the runtime uses. |
| + // precedence the runtime uses. A legacy source has no qualified key at |
| + // all, so both halves of the fallback are the legacy one — passing the |
| + // sentinel to ChannelKey would build apps/legacy/… and quietly depend |
| + // on that never existing. |
| + srcKey := release.ChannelKey(srcAcct, app, channel) |
| + if srcAcct == release.LegacySource { |
| + srcKey = release.LegacyChannelKey(app, channel) |
| + } |
| want, _, _, err := release.GetWithFallback(ctx, store, |
| - release.ChannelKey(srcAcct, app, channel), release.LegacyChannelKey(app, channel)) |
| + srcKey, release.LegacyChannelKey(app, channel)) |
| if err != nil { |
| log.Fatalf("accounts migrate: verify: re-read %s/%s at the source: %v", app, channel, err) |
| } |
diff --git a/e2e_test.go b/e2e_test.go| index 4c886df5b..f464c752c 100644 |
| --- a/e2e_test.go |
| +++ b/e2e_test.go |
| @@ -1175,3 +1175,133 @@ func TestAddUnitBackingRequiresDBForHibernate(t *testing.T) { |
| } |
| } |
| } |
| + |
| +// An app still on the pre-accounts keys can be moved INTO ops. Until `-from |
| +// legacy` existed there was no way to say it: a bare move into ops is refused |
| +// (ops is also the default source, and the verb will not guess a direction), |
| +// and -from only took a sqid, which the account-less key space does not have. |
| +// That left every ops app stranded on legacy keys and made the read-fallback |
| +// impossible to retire. |
| +func TestAccountsMigrateLegacyIntoOps(t *testing.T) { |
| + goBin, err := exec.LookPath("go") |
| + if err != nil { |
| + t.Skip("go toolchain not found") |
| + } |
| + tmp := t.TempDir() |
| + carlosBin := filepath.Join(tmp, "carlos") |
| + build := exec.Command(goBin, "build", "-o", carlosBin, ".") |
| + build.Env = os.Environ() |
| + if out, err := build.CombinedOutput(); err != nil { |
| + t.Fatalf("build carlos: %v\n%s", err, out) |
| + } |
| + storeDir := filepath.Join(tmp, "store") |
| + regPath := filepath.Join(tmp, "registry.db") |
| + store := release.NewDirStore(storeDir) |
| + ctx := context.Background() |
| + |
| + seed, pubB64, err := release.GenerateKey() |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + priv, err := release.ParseSigningKey(seed, "e2e") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + pub, err := release.ParsePublicKey(pubB64, "e2e") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + env := append(os.Environ(), |
| + "CARLOS_DEPLOYMENT_DIR="+storeDir, |
| + "CARLOS_RELEASE_PUBKEY="+pubB64) |
| + run := func(args ...string) (string, error) { |
| + t.Helper() |
| + cmd := exec.Command(carlosBin, args...) |
| + cmd.Env = env |
| + out, err := cmd.CombinedOutput() |
| + return string(out), err |
| + } |
| + |
| + // A purely legacy app: the deployment store holds oldapp ONLY at the |
| + // account-less keys, which is the shape every deployment predating accounts |
| + // is in. Built in a scratch store and copied down, because a pointer's |
| + // signature covers app/channel/version rather than the key path — so these |
| + // are genuine legacy objects, signed by the same key a real deployment used. |
| + scratch := release.NewDirStore(filepath.Join(tmp, "scratch")) |
| + m := release.Manifest{Schema: 1, App: "oldapp", Version: "v1", Kind: "binary", |
| + Artifacts: map[string]release.Artifact{"linux-amd64": {File: "oldapp"}}} |
| + if err := release.Ship(ctx, scratch, release.OpsAccount, m, map[string][]byte{"linux-amd64": []byte("v1 bytes")}); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := release.Promote(ctx, scratch, priv, release.OpsAccount, "oldapp", "stable", "v1"); err != nil { |
| + t.Fatal(err) |
| + } |
| + qualified, err := scratch.List(ctx, release.AppPrefix(release.OpsAccount, "oldapp")) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + for _, k := range qualified { |
| + body, _, err := scratch.Get(ctx, k) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + legacy := release.LegacyAppPrefix("oldapp") + strings.TrimPrefix(k, release.AppPrefix(release.OpsAccount, "oldapp")) |
| + if _, err := store.Put(ctx, legacy, body, ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + // Sanity, and the reason this test asserts on KEYS rather than on what |
| + // resolves: ops already "serves" oldapp through the read-fallback, so |
| + // release.Channel would answer v1 whether or not anything migrated. The |
| + // migration's job is to put the objects at the qualified keys so the |
| + // fallback can eventually be retired — that is the thing to assert. |
| + opsKey := release.ChannelKey(release.OpsAccount, "oldapp", "stable") |
| + if _, _, err := store.Get(ctx, opsKey); err == nil { |
| + t.Fatal("fixture: the qualified key already exists — the migration would prove nothing") |
| + } |
| + |
| + // A bare move into ops is still refused — the ambiguity guard stays — and |
| + // the refusal now NAMES the way out. |
| + out, err := run("accounts", "migrate", "-registry", regPath, "-app", "oldapp", "-to", release.OpsAccount) |
| + if err == nil { |
| + t.Fatalf("a bare move into ops was accepted:\n%s", out) |
| + } |
| + if !strings.Contains(out, "-from legacy") { |
| + t.Errorf("the refusal does not name the fix:\n%s", out) |
| + } |
| + |
| + // Naming the legacy source works, and the destination then serves what the |
| + // source served. |
| + out, err = run("accounts", "migrate", "-registry", regPath, "-app", "oldapp", |
| + "-from", release.LegacySource, "-to", release.OpsAccount) |
| + if err != nil { |
| + t.Fatalf("legacy -> ops migration: %v\n%s", err, out) |
| + } |
| + if _, _, err := store.Get(ctx, opsKey); err != nil { |
| + t.Fatalf("the qualified pointer %s does not exist after the migration: %v\n%s", opsKey, err, out) |
| + } |
| + p, err := release.Channel(ctx, store, pub, release.OpsAccount, "oldapp", "stable") |
| + if err != nil { |
| + t.Fatalf("ops does not serve oldapp after the migration: %v\n%s", err, out) |
| + } |
| + if p.Version != "v1" { |
| + t.Errorf("ops serves %s, want v1", p.Version) |
| + } |
| + // The whole app moved, not just the pointer: the manifest and every |
| + // artifact must be at qualified keys too, or retiring the fallback would |
| + // take the app down. |
| + moved, err := store.List(ctx, release.AppPrefix(release.OpsAccount, "oldapp")) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if len(moved) != len(qualified) { |
| + t.Errorf("migrated %d objects, want all %d", len(moved), len(qualified)) |
| + } |
| + |
| + // "legacy" is a source, never a destination: the point of the verb is to |
| + // empty that key space, not to fill it. |
| + if out, err := run("accounts", "migrate", "-registry", regPath, "-app", "oldapp", |
| + "-to", release.LegacySource); err == nil { |
| + t.Fatalf("migrating INTO legacy was accepted:\n%s", out) |
| + } |
| +} |
diff --git a/internal/domains/sqid_test.go b/internal/domains/sqid_test.go| index 0f9b4263f..77a4b8f81 100644 |
| --- a/internal/domains/sqid_test.go |
| +++ b/internal/domains/sqid_test.go |
| @@ -5,6 +5,7 @@ import ( |
| "testing" |
| "github.com/carlosframework/platform/internal/domains" |
| + "github.com/carlosframework/platform/internal/release" |
| ) |
| func TestSqidGolden(t *testing.T) { |
| @@ -40,3 +41,31 @@ func TestSqidGolden(t *testing.T) { |
| goldenSeen[s] = id |
| } |
| } |
| + |
| +// The frozen alphabet omits i, l and o, so no id encodes to "legacy" — which is |
| +// what lets release.LegacySource use that word as a sentinel for the |
| +// account-less key space in `carlos accounts migrate -from legacy`. If the |
| +// alphabet ever gains an "l", the sentinel silently becomes a real account's |
| +// sqid and the verb starts reading one as the other. Fail here first. |
| +// |
| +// The emitted character set is derived from Sqid itself rather than read off |
| +// the constant: this test lives outside the package, and asserting on what the |
| +// encoder actually produces is the stronger check anyway. |
| +func TestLegacySourceIsNotASqid(t *testing.T) { |
| + emitted := map[rune]bool{} |
| + for id := int64(1); id < 20000; id++ { |
| + for _, r := range domains.Sqid(id) { |
| + emitted[r] = true |
| + } |
| + } |
| + if len(emitted) != 23 { |
| + t.Fatalf("saw %d distinct characters over 20k ids, want the full 23 — widen the range", len(emitted)) |
| + } |
| + for _, r := range release.LegacySource { |
| + if !emitted[r] { |
| + return // a character the encoder can never emit: no id can collide |
| + } |
| + } |
| + t.Fatalf("every character of %q is one the sqid encoder emits — the sentinel can collide with a real account sqid", |
| + release.LegacySource) |
| +} |
diff --git a/internal/release/fallback.go b/internal/release/fallback.go| index 5d585dd56..cd0e017d7 100644 |
| --- a/internal/release/fallback.go |
| +++ b/internal/release/fallback.go |
| @@ -23,6 +23,20 @@ import ( |
| // a caller that genuinely has no account in hand passes. |
| const OpsAccount = "bab" |
| +// LegacySource names the account-less key space, so an operator can point at |
| +// it. Before accounts existed every app lived at apps/<app>/…, and those keys |
| +// carry no sqid — which left `accounts migrate` with no way to spell "move |
| +// THIS into ops": the verb refuses a bare move into ops (ops is also its |
| +// default source, and guessing the direction is not its job), and -from only |
| +// took a sqid the legacy space does not have. Ops apps were stranded on legacy |
| +// keys, and the read-fallback could never be retired. |
| +// |
| +// Usable as a sentinel because the frozen encoder's alphabet omits i, l and o, |
| +// so Sqid() can never emit "legacy" for any id. TestLegacySourceIsNotASqid |
| +// pins that, because the day the alphabet changes is the day this silently |
| +// becomes a real account's name. |
| +const LegacySource = "legacy" |
| + |
| // acctRe pins an account sqid to what the frozen encoder emits: lowercase |
| // letters only, at least three of them (the encoder's offset guarantees it). |
| // It exists so a caller can't smuggle a path segment — or an empty string — |