amadan
merge: retire the legacy read-fallback — the runtime reads qualified keys only
Every app with a route on both production fleets was migrated to account-qualified keys this morning (flagship: console, platform-site, carlosframework via -from legacy; tito: titogo, pointer byte-verified), so the fallback's job is done and every read it still performed was a chance for a stale legacy object to shadow reality. GetQualified is the runtime's read path; a miss names carlos accounts migrate -from legacy — but only for apps with NOTHING under their qualified prefix, so a migrated app waiting on its first stable promotion (kass, until Aug 5) gets a plain skip, not hourly advice. The migration verb and the console's name-claim check keep their EXPLICIT legacy access forever: they are the upgrade path. Legacy keys mint nothing in reconcile any more, and legacy tripwire stores pin that no runtime path — pointer, artifact staging, static extraction — ever builds a legacy key. Reviewed twice on Opus; the one CRITICAL (reconcile retiring a mis-stamped canary host) closed via live pre-flight on both fleets: tito's reconcile is inert and the flagship has zero Platform+Canary rows, recorded in the test that pins the shape.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
19 files changed,
+1295
−424
README.md+15 −8accounts.go+19 −16cli.go+2 −2docs/superpowers/specs/2026-08-02-multi-account-design.md+124 −1e2e_test.go+47 −20internal/appconfig/environments.go+3 −2internal/console/server.go+7 −5internal/console/view.go+9 −7internal/domains/reconcile.go+45 −29internal/domains/reconcile_test.go+113 −4internal/edge/static.go+1 −3internal/edge/static_test.go+76 −0internal/release/adopt.go+47 −3internal/release/fallback.go+0 −99internal/release/fallback_test.go+0 −190internal/release/qualified.go+184 −0internal/release/qualified_test.go+550 −0internal/release/release.go+52 −32internal/vet/vet.go+1 −3
diff --git a/README.md b/README.md| index 071fc93a4..fabd0c7b9 100644 |
| --- a/README.md |
| +++ b/README.md |
| @@ -132,10 +132,9 @@ the multi-account migration. |
| **Bucket layout, and the migration.** App objects live under |
| `apps/<account-sqid>/<app>/…`, and that key path *is* the app→account linkage |
| -— there is no separate manifest. Existing deployments still have their objects |
| -at the legacy `apps/<app>/…` keys, so **every read path tries the qualified |
| -key first and falls back to the legacy one**; writes are always qualified. |
| -Moving an app is a verb: |
| +— there is no separate manifest. **Every read path resolves those qualified |
| +keys and nothing else** (the legacy read-fallback was retired on 2026-08-03, |
| +once both deployments were migrated). Moving an app is a verb: |
| carlos accounts migrate -app kass -to <sqid> -dry-run # prints the plan |
| carlos accounts migrate -app kass -to <sqid> |
| @@ -147,10 +146,18 @@ carlos binary deliberately never opens its database — so `console |
| migrate-app` completes the move; `carlos accounts migrate` prints that exact |
| command when it finishes. Both halves are idempotent. |
| -**It never deletes a legacy object.** Deleting them — and removing the |
| -fallback code — is a separate operator step, gated on verification: every |
| -channel pointer must resolve from the qualified keys with the fallback |
| -disabled, and one adopt pass must be a no-op, first. |
| +**Upgrading from a pre-accounts binary?** Your objects are all at the old |
| +account-less `apps/<app>/…` keys, which nothing reads any more — so the apps |
| +will go quiet rather than serve something stale, and every miss tells you so |
| +by name: |
| + |
| + carlos accounts migrate -app <app> -from legacy -to bab # -to <sqid> for a tenant |
| + |
| +That `-from legacy` source is permanent. However long a deployment sits on an |
| +old build, this is the way off those keys. |
| + |
| +**It never deletes a legacy object.** Deleting them is a separate operator |
| +step, and an unhurried one now that nothing resolves through them. |
| ## App config & secrets |
diff --git a/accounts.go b/accounts.go| index e86d7da2c..3a4679a2e 100644 |
| --- a/accounts.go |
| +++ b/accounts.go |
| @@ -403,18 +403,20 @@ func putOverwrite(ctx context.Context, s release.Store, key string, body []byte) |
| // most dangerous decision the verb makes. |
| // |
| // Once this binary is adopted, every ship and promote writes to the |
| -// account-qualified keys and every read resolves qualified-first — so an app |
| -// that has not been migrated yet ends up with objects at BOTH prefixes, and |
| -// its LEGACY pointer goes stale the first time anyone deploys. Copying the |
| -// legacy prefix at that point would move a stale pointer into the new |
| -// account, lose every post-adopt release, and let the next agent tick |
| -// silently downgrade the app to a pre-adopt build. |
| +// account-qualified keys — so an app that was half-migrated when the |
| +// accounts build landed has objects at BOTH prefixes, and its LEGACY pointer |
| +// went stale the first time anyone deployed. Copying the legacy prefix at |
| +// that point would move a stale pointer into the new account, lose every |
| +// post-adopt release, and let the next agent tick silently downgrade the app |
| +// to a pre-adopt build. |
| // |
| -// So the default source is not a prefix but the same PER-KEY PRECEDENCE the |
| -// runtime uses (release.GetWithFallback): for each relative key, the |
| -// ops-qualified object wins, and legacy-only objects still come across so no |
| -// rollback target is lost. `-from` overrides it with a single explicit |
| -// prefix, for moving between two non-ops accounts. |
| +// So the default source is not a prefix but a PER-KEY PRECEDENCE: for each |
| +// relative key, the ops-qualified object wins, and legacy-only objects still |
| +// come across so no rollback target is lost. (That precedence outlived the |
| +// read-fallback it was modelled on — the runtime resolves qualified keys only |
| +// now, but the buckets this verb repairs are exactly the ones with objects |
| +// under both.) `-from` overrides it with a single explicit prefix, for moving |
| +// between two non-ops accounts. |
| // |
| // Returns the copies to make and the account the app is being moved FROM (for |
| // the post-copy verification). |
| @@ -487,7 +489,7 @@ func migrationPlan(ctx context.Context, store release.Store, app, from, to, dst |
| } |
| // Qualified first, then legacy for anything it did not cover — the |
| - // per-key precedence GetWithFallback applies on every read. |
| + // per-key precedence described above. |
| seen := map[string]bool{} |
| var out []copyOp |
| for _, k := range opsKeys { |
| @@ -532,16 +534,17 @@ func verifyMigratedPointers(ctx context.Context, store release.Store, app, srcAc |
| if err != nil { |
| 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. A legacy source has no qualified key at |
| - // all, so both halves of the fallback are the legacy one — passing the |
| + // What the OLD account still holds, by this verb's own per-key |
| + // precedence — the migration's explicit read of the legacy space, |
| + // which outlives the retired runtime fallback. A legacy source has no |
| + // qualified key at all, so both halves 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, |
| + want, _, _, err := release.GetMigrationSource(ctx, store, |
| 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/cli.go b/cli.go| index 91be37344..b5adf5c8f 100644 |
| --- a/cli.go |
| +++ b/cli.go |
| @@ -863,8 +863,8 @@ func runChannels(args []string) { |
| } |
| acct := acctOf() |
| s := openStore() |
| - keys, prefix, _, err := release.ListWithFallback(context.Background(), s, |
| - release.ChannelsPrefix(acct, *app), release.LegacyChannelsPrefix(*app)) |
| + prefix := release.ChannelsPrefix(acct, *app) |
| + keys, err := s.List(context.Background(), prefix) |
| if err != nil { |
| log.Fatal(err) |
| } |
diff --git a/docs/superpowers/specs/2026-08-02-multi-account-design.md b/docs/superpowers/specs/2026-08-02-multi-account-design.md| index 24b4dade0..530998f0e 100644 |
| --- a/docs/superpowers/specs/2026-08-02-multi-account-design.md |
| +++ b/docs/superpowers/specs/2026-08-02-multi-account-design.md |
| @@ -142,6 +142,11 @@ qualified key first and falls back to the legacy unqualified |
| `apps/<app>/…`. Writes always use the qualified key. The fallback is |
| deleted once both deployments are migrated and verified (§5). |
| +> **Retired 2026-08-03.** Both deployments are migrated and the fallback is |
| +> gone; the runtime reads account-qualified keys only. `accounts migrate |
| +> -from legacy` keeps its explicit access to the account-less space |
| +> forever. See §7c. |
| + |
| ### Permissions collapse into membership |
| The console's `grants` table (address → app) is **replaced by |
| @@ -209,6 +214,14 @@ then delete the legacy objects. |
| pointer must resolve, and one adopt pass must be a no-op, from the |
| qualified keys with the fallback *disabled* in a dry run. |
| +> **Gate passed 2026-08-03** for every app with a route on either |
| +> deployment: console, platform-site and carlosframework migrated |
| +> `-from legacy -to bab`; kass and keymail earlier, to `bad`/`bae`; titogo |
| +> migrated on the Tito deployment and its pointer byte-verified under |
| +> `bab`. The fallback was retired the same day (§7c). Deleting the legacy |
| +> OBJECTS remains a separate operator step — no code path has ever removed |
| +> one, and none does now. |
| + |
| **Then, and only then**, `PLATFORM_APPS_DOMAIN=oncarlos.com` gets set on |
| the flagship — so every app mints exactly one platform domain, under its |
| real account, with a durable sqid. That ordering is the whole reason |
| @@ -223,7 +236,12 @@ domains shipped inert. |
| - **Sqid**: existing frozen goldens still pass (the encoder is unchanged; |
| only what feeds it changes). |
| - **Bucket keys**: table-driven over qualified/legacy paths, including |
| - the fallback and its removal. |
| + the fallback and its removal. Post-§7c that means: a qualified read does |
| + not answer from a legacy object; a miss names `accounts migrate -from |
| + legacy` and stays `os.ErrNotExist`-wrapped; a legacy-only app is |
| + invisible to every runtime read path — pinned by a tripwire store that |
| + fails the test if anything so much as touches an unqualified prefix — |
| + while `GetMigrationSource` still reads it. |
| - **Console**: membership gates every account-scoped route; non-member |
| gets a byte-identical 404; owner-only actions refuse members; the last |
| owner cannot be removed; invite tokens are single-use, expiring, and |
| @@ -240,6 +258,9 @@ domains shipped inert. |
| 3. Create accounts; migrate kass and keymail; leave platform components |
| under `ops`. |
| 4. Verification gate; delete legacy objects; remove the fallback. |
| + *(Gate passed and fallback removed 2026-08-03 — §7c. Deleting the legacy |
| + objects is still outstanding, and deliberately unhurried: they are inert |
| + now that nothing reads them.)* |
| 5. Enable `PLATFORM_APPS_DOMAIN` on the flagship. |
| 6. Woodstar migrates as the first app *born* into its own account. |
| @@ -302,6 +323,108 @@ reaches every box, not just the console that performed it); anything that |
| **allocates** against it must still count it (`NextAccountID`). Backwards |
| in either direction resurrects a deleted account or reissues its identity. |
| +## 7c. The read-fallback is retired (2026-08-03) |
| + |
| +The migration seam described in §2 is gone. **Every runtime read path |
| +resolves account-qualified keys and nothing else**, and a legacy-only app |
| +is invisible to all of them: it does not resolve, does not list, does not |
| +adopt, and does not mint a route. |
| + |
| +**Why now.** The §5 gate passed for every app with a route on either |
| +deployment. Past that point the fallback had no job left, and each read it |
| +performed was another chance for a stale legacy object to shadow reality — |
| +the class of failure that nearly forced a rollback during the migration |
| +itself, when a legacy pointer left behind by a half-migrated app would have |
| +downgraded a live app on the next agent tick. |
| + |
| +**What explicitly survives, and must keep surviving.** |
| + |
| +- `carlos accounts migrate -from legacy` reads the account-less space |
| + directly, forever. It is the upgrade path for any deployment coming from |
| + a pre-accounts binary — a self-hosted box that sits on an old build for a |
| + year still has all of its data at `apps/<app>/…`, and the day it upgrades |
| + is the day it needs this verb. Its per-key precedence (qualified wins, |
| + legacy-only keys still come across) outlived the fallback it was modelled |
| + on, because the buckets it repairs are exactly the ones holding objects |
| + under both prefixes. |
| +- The console's **app-name claim check** still refuses a name whose legacy |
| + ops objects exist. Handing that name to a stranger would put their app in |
| + the way of the migration those objects are still owed. |
| + |
| +Neither is a resolution path: nothing falls back, both are named. |
| + |
| +**A miss is actionable, in one place.** Every runtime read of an OBJECT — |
| +manifest, channel pointer, artifact — goes through `release.GetQualified`, so |
| +the sentence a miss needs is written once: its not-found names |
| +`carlos accounts migrate -app <app> -from legacy -to <acct>` and stays |
| +`os.ErrNotExist`-wrapped, so every "has this app ever shipped here" check |
| +keeps working. The hint is **unconditional rather than probed**: confirming |
| +it would mean reading a legacy key on the error path, and "the runtime never |
| +reads a legacy key" is a property worth holding outright rather than with an |
| +exception. The wording carries the condition instead, so it stays honest for |
| +an app that simply has no such channel. Non-app keys get no hint at all. |
| + |
| +Two kinds of read do not pass through it, and the claim is stated with them |
| +rather than around them. **Listings** (`VersionRung`, `carlos channels`, the |
| +console's channel and release views) call `List` on a qualified prefix; there |
| +is nothing to fall back to, since a prefix either holds objects or the app has |
| +none here, and both are answers. **`Promote`'s read-modify-write** calls |
| +`GetQualified` but discards the hint, because an absent pointer is the |
| +legitimate first promotion of a fresh channel. |
| + |
| +What holds without qualification is the narrower, checkable property: no |
| +runtime read anywhere builds a legacy key. The **tripwire tests** enforce it |
| +at the `Store`, not at any one helper — and because a tripwire only guards the |
| +paths a test routes through it, the two artifact downloads |
| +(`adopt.ensureStaged`, `edge.ensureExtracted`) each get their own pass. Both |
| +were mutation-checked: reintroducing a fallback at either site fails its test. |
| + |
| +**`carlos promote` says it too.** `PromoteOnLadder` refuses an app with |
| +nothing at all under its qualified prefix, naming the migration. Without that |
| +the ladder answers first — "this build is on no rung yet", true and beside the |
| +point. The condition is *nothing qualified at all*, not *no qualified |
| +releases*: an app that has shipped since accounts landed is not unmigrated |
| +however new the channel is. Both readings are computed from the qualified |
| +prefix; neither touches a legacy key. |
| + |
| +**Leftover legacy objects mint nothing and claim nothing.** Reconciliation |
| +used to read `apps/<app>/…` as "ops, until migrated"; it no longer treats |
| +those keys as app→account linkage. A legacy canary pointer mints no canary |
| +host (and one already minted from a legacy pointer is retired by the |
| +ordinary pointerless-canary rule), and a leftover copy under a migrated app |
| +no longer makes ops an owner of its objects — which used to drag an |
| +unstamped route onto ops rather than the account the app now lives in. |
| +Platform apexes still mint, because minting follows the operator's ROUTE, a |
| +real record of intent; such an apex serves nothing until the app is |
| +migrated, which is the honest report of where its objects are. |
| + |
| +**The cost, stated.** A promote against an app still on legacy keys is now a |
| +FIRST promote at the qualified key: its legacy history does not come |
| +forward. Carrying history across the two key spaces is the migration verb's |
| +job, not a side effect of the next deploy — and it is why a miss names that |
| +verb. In exchange, promote and rollback hold a real etag for the object they |
| +write, so the CAS is honest rather than a create-only workaround. |
| + |
| +**Adopt skips, loudly — but only where the advice applies.** A legacy-only app |
| +leaves adopt printing the store's error, which carries the hint, and moving on. |
| +It never fails the pass: one app's missing pointer must not stop every other |
| +app on the box from installing, which is already how a channel with nothing |
| +promoted behaves. |
| + |
| +A missing pointer has two causes and only one of them is a migration. A |
| +MIGRATED app waiting on its first promotion to a channel is ordinary — kass |
| +follows `stable` on the flagship, whose pointer does not exist until the bake |
| +window ends — and an hourly pass that told it to migrate would be wrong every |
| +hour for days. So adopt applies `AppHasQualifiedObjects`, the same rule |
| +`PromoteOnLadder` uses: nothing under the qualified prefix gets the hint, |
| +anything under it gets the plain "channel has no release yet". One listing per |
| +(account, app) per pass, cached; a listing that FAILS suppresses the advice |
| +rather than inventing it, since a bucket the box cannot list is not evidence |
| +an app never migrated. |
| + |
| +**Deleting the legacy objects is still a separate operator step.** No code |
| +path has ever removed one and none does now. They are inert. |
| + |
| ## 8. Future work (named, not built) |
| - **Roles beyond owner/member** — read-only members, deploy-but-not-invite. |
diff --git a/e2e_test.go b/e2e_test.go| index 28ca3b04b..90b280edc 100644 |
| --- a/e2e_test.go |
| +++ b/e2e_test.go |
| @@ -18,7 +18,6 @@ package main_test |
| import ( |
| "context" |
| "crypto/ed25519" |
| - "crypto/sha256" |
| "encoding/json" |
| "fmt" |
| "io" |
| @@ -565,24 +564,35 @@ func TestMultiAccountEndToEnd(t *testing.T) { |
| t.Fatalf("Promote blog for %s: %v", name, err) |
| } |
| } |
| - // And a legacy app: objects at the account-less keys, exactly as an |
| - // unmigrated deployment's bucket has them. |
| - legacyTar := e2eSiteTar(t, "legacy site") |
| - legacyManifest := release.Manifest{Schema: 1, App: "oldsite", Version: "v1", Kind: "static", |
| - Artifacts: map[string]release.Artifact{"site.tar.gz": { |
| - File: "site.tar.gz", SHA256: fmt.Sprintf("%x", sha256.Sum256(legacyTar)), Size: int64(len(legacyTar))}}} |
| - raw, err := json.MarshalIndent(legacyManifest, "", " ") |
| - if err != nil { |
| + // And a legacy app: manifest, artifact and signed pointer at the |
| + // account-less keys, exactly as an unmigrated deployment's bucket has |
| + // them. 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")) |
| + oldm := release.Manifest{Schema: 1, App: "oldsite", Version: "v1", Kind: "static", |
| + Artifacts: map[string]release.Artifact{"site.tar.gz": {}}} |
| + if err := release.Ship(ctx, scratch, release.OpsAccount, oldm, |
| + map[string][]byte{"site.tar.gz": e2eSiteTar(t, "legacy site")}); err != nil { |
| t.Fatal(err) |
| } |
| - if _, err := store.Put(ctx, release.LegacyArtifactKey("oldsite", "v1", "site.tar.gz"), legacyTar, ""); err != nil { |
| + if err := release.Promote(ctx, scratch, priv, release.OpsAccount, "oldsite", "stable", "v1"); err != nil { |
| t.Fatal(err) |
| } |
| - if _, err := store.Put(ctx, release.LegacyManifestKey("oldsite", "v1"), raw, ""); err != nil { |
| + staged, err := scratch.List(ctx, release.AppPrefix(release.OpsAccount, "oldsite")) |
| + if err != nil { |
| t.Fatal(err) |
| } |
| - if err := release.Promote(ctx, store, priv, release.OpsAccount, "oldsite", "stable", "v1"); err != nil { |
| - t.Fatal(err) |
| + for _, k := range staged { |
| + body, _, err := scratch.Get(ctx, k) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + legacy := release.LegacyAppPrefix("oldsite") + strings.TrimPrefix(k, release.AppPrefix(release.OpsAccount, "oldsite")) |
| + if _, err := store.Put(ctx, legacy, body, ""); err != nil { |
| + t.Fatal(err) |
| + } |
| } |
| for _, rt := range []registry.Route{ |
| @@ -610,12 +620,19 @@ func TestMultiAccountEndToEnd(t *testing.T) { |
| for host, want := range map[string]string{ |
| "blog.kass.test": "kass blog", |
| "blog.woodstar.test": "woodstar blog", |
| - "old.test": "legacy site", // served through the read-fallback |
| } { |
| if code, body := e2eGet(t, srv.URL, host, "/"); code != 200 || body != want { |
| t.Errorf("GET %s / = %d %q, want 200 %q", host, code, body, want) |
| } |
| } |
| + // The legacy app serves NOTHING. Its objects are all still at the |
| + // account-less keys, and since the read-fallback was retired no read path |
| + // resolves through them — the edge cannot find a pointer, so the route is |
| + // a route to nowhere until somebody runs `accounts migrate -from legacy`. |
| + // Serving it would be the stale-shadow risk the retirement removed. |
| + if code, body := e2eGet(t, srv.URL, "old.test", "/"); code == 200 { |
| + t.Errorf("GET old.test / = %d %q — a legacy-only app is still being served", code, body) |
| + } |
| // One reconcile pass: accounts materialize with the bucket's ids, and each |
| // app mints its platform domain under the account its KEY PATHS say it |
| @@ -635,13 +652,19 @@ func TestMultiAccountEndToEnd(t *testing.T) { |
| for host, want := range map[string]string{ |
| "blog." + accounts["Kass"] + ".oncarlos.test": "kass blog", |
| "blog." + accounts["Woodstar"] + ".oncarlos.test": "woodstar blog", |
| - "oldsite.bab.oncarlos.test": "legacy site", |
| } { |
| if code, body := e2eGet(t, srv.URL, host, "/"); code != 200 || body != want { |
| t.Errorf("GET %s / = %d %q, want 200 %q — the platform domain must serve its own account's build", |
| host, code, body, want) |
| } |
| } |
| + // oldsite's apex still mints: minting follows the operator's ROUTE, which |
| + // is a real record of intent, not the bucket objects. What it must not do |
| + // is serve — the account it mints under is where the app's objects OUGHT |
| + // to be, and the empty page is the honest report of them not being there. |
| + if code, body := e2eGet(t, srv.URL, "oldsite.bab.oncarlos.test", "/"); code == 200 { |
| + t.Errorf("GET oldsite.bab.oncarlos.test / = %d %q — a legacy-only app is still being served", code, body) |
| + } |
| } |
| // TestAccountsCLI drives the `carlos accounts` verbs as an operator would: |
| @@ -1251,15 +1274,19 @@ func TestAccountsMigrateLegacyIntoOps(t *testing.T) { |
| 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") |
| } |
| + // Before the move, oldapp is INVISIBLE: the read-fallback is retired, so |
| + // nothing resolves it and the miss names the exact command that fixes it. |
| + // This is the state a deployment lands in by upgrading from a |
| + // pre-accounts binary — the reason this verb has to keep working forever. |
| + if _, err := release.Channel(ctx, store, pub, release.OpsAccount, "oldapp", "stable"); err == nil { |
| + t.Fatal("a legacy-only app resolved — the read-fallback is supposed to be retired") |
| + } else if !strings.Contains(err.Error(), "carlos accounts migrate -app oldapp -from legacy -to bab") { |
| + t.Errorf("the miss does not name the fix:\n%v", err) |
| + } |
| // A bare move into ops is still refused — the ambiguity guard stays — and |
| // the refusal now NAMES the way out. |
diff --git a/internal/appconfig/environments.go b/internal/appconfig/environments.go| index 3fdcb21fa..552d19bca 100644 |
| --- a/internal/appconfig/environments.go |
| +++ b/internal/appconfig/environments.go |
| @@ -30,8 +30,9 @@ func EnvironmentKey(acct, app, name string) string { |
| // EnvironmentsPrefix is the list prefix behind "what named environments does |
| // this app have" — every apps/<acct>/<app>/environments/<name>.json key |
| -// shares it. Unlike release.ChannelsPrefix there is no legacy fallback: |
| -// environments are new, so there is nothing pre-migration to fall back to. |
| +// shares it. Environments were born account-scoped and never had a legacy |
| +// unqualified shape — which, since the read-fallback's retirement, is now |
| +// true of every read path rather than a distinction worth drawing. |
| func EnvironmentsPrefix(acct, app string) string { |
| return release.AppPrefix(acct, app) + "environments/" |
| } |
diff --git a/internal/console/server.go b/internal/console/server.go| index af9dde602..53fb6adf1 100644 |
| --- a/internal/console/server.go |
| +++ b/internal/console/server.go |
| @@ -1122,11 +1122,13 @@ func (s *Server) handleNewApp(w http.ResponseWriter, r *http.Request) { |
| ctx, cancel := context.WithTimeout(r.Context(), 10*time.Second) |
| defer cancel() |
| // The account's own prefix, plus — for ops only — the legacy unqualified |
| - // one an unmigrated app's objects are still at. Missing the legacy prefix |
| - // would hand out a name whose releases already exist, which is precisely |
| - // the "orphan bucket objects" case this check was added for. For a |
| - // non-ops account the legacy prefix belongs to somebody else's app and |
| - // must NOT block the name. |
| + // one an unmigrated app's objects are still at. This one legacy read |
| + // OUTLIVES the retired read-fallback, and deliberately: it is explicit |
| + // rather than a resolution path, and a name whose legacy objects still |
| + // exist is not free. Handing it out would put a stranger's app in the way |
| + // of the `accounts migrate -from legacy` that is still owed to those |
| + // objects. For a non-ops account the legacy prefix belongs to somebody |
| + // else's app and must NOT block the name. |
| prefixes := []string{release.AppPrefix(acct.Sqid, name)} |
| if acct.ID == OpsAccountID { |
| prefixes = append(prefixes, release.LegacyAppPrefix(name)) |
diff --git a/internal/console/view.go b/internal/console/view.go| index 9b61c1ca2..096959ac1 100644 |
| --- a/internal/console/view.go |
| +++ b/internal/console/view.go |
| @@ -273,12 +273,14 @@ func assembleCanaries(channels []ChannelView, routes []RouteView) []CanaryView { |
| return out |
| } |
| -// assembleChannels lists the app's channel pointers — account-qualified, |
| -// falling back to the legacy unqualified prefix for an app that hasn't been |
| -// migrated yet — and resolves each one. |
| +// assembleChannels lists the app's account-qualified channel pointers and |
| +// resolves each one. An app whose objects are still at the legacy unqualified |
| +// keys lists as empty here, exactly as the fleet sees it — the console showing |
| +// channels the boxes cannot follow is the disagreement the retired |
| +// read-fallback used to create. |
| func assembleChannels(ctx context.Context, s release.Store, pub ed25519.PublicKey, acct, app string, now time.Time) []ChannelView { |
| - keys, prefix, _, err := release.ListWithFallback(ctx, s, |
| - release.ChannelsPrefix(acct, app), release.LegacyChannelsPrefix(app)) |
| + prefix := release.ChannelsPrefix(acct, app) |
| + keys, err := s.List(ctx, prefix) |
| if err != nil { |
| return []ChannelView{{Name: "(all)", Err: "listing channels: " + err.Error()}} |
| } |
| @@ -354,8 +356,8 @@ const versionsCap = 20 |
| // capped at versionsCap. Unreadable manifests degrade to a row error. |
| func Versions(s release.Store, acct, app string) ([]VersionView, error) { |
| ctx := context.Background() |
| - keys, prefix, _, err := release.ListWithFallback(ctx, s, |
| - release.ReleasesPrefix(acct, app), release.LegacyReleasesPrefix(app)) |
| + prefix := release.ReleasesPrefix(acct, app) |
| + keys, err := s.List(ctx, prefix) |
| if err != nil { |
| return nil, err |
| } |
diff --git a/internal/domains/reconcile.go b/internal/domains/reconcile.go| index fe691bea7..6f1250be9 100644 |
| --- a/internal/domains/reconcile.go |
| +++ b/internal/domains/reconcile.go |
| @@ -126,12 +126,12 @@ func Reconcile(ctx context.Context, s release.Store, reg *registry.Registry, cfg |
| } |
| // --- 1b. app→account linkage, from the KEY PATH -------------------------- |
| - // apps/<sqid>/<app>/… says the app belongs to that account; a legacy |
| - // unqualified apps/<app>/… belongs to ops until it is migrated. The two |
| - // shapes are told apart by whether the first segment is a KNOWN account |
| - // sqid, which is why the account set had to be read first — app names and |
| - // account sqids live in different namespaces, so there is no ambiguity |
| - // once the set is known. |
| + // apps/<sqid>/<app>/… says the app belongs to that account. A legacy |
| + // unqualified apps/<app>/… key says nothing at all since the read-fallback |
| + // was retired — the runtime cannot read those objects, so they must not |
| + // mint a route or claim an app (appKeyOwner). The shapes are told apart by |
| + // whether the first segment is a KNOWN account sqid, which is why the |
| + // account set had to be read first. |
| // owners is app name -> the accounts that hold objects for it. A LIST, not |
| // one account: the same app name legitimately exists in several accounts, |
| // and collapsing them to one would re-home every tenant's app onto |
| @@ -348,42 +348,58 @@ func AccountManifestSqid(key string) (string, bool) { |
| } |
| // appKeyOwner reads the app→account linkage straight out of a bucket key: |
| -// apps/<sqid>/<app>/… belongs to that account, apps/<app>/… is a legacy |
| -// unqualified key and belongs to ops until it is migrated. known is the set |
| -// of materialized account sqids — the only thing that tells the two shapes |
| -// apart, which is why accounts are read first. |
| +// apps/<sqid>/<app>/… belongs to that account. known is the set of |
| +// materialized account sqids, which is what makes a key path unambiguous — |
| +// hence accounts being read first. |
| // |
| -// Keys with nothing after the app segment (apps/<app>) are not linkage: an |
| -// object sitting directly at the app prefix says nothing about ownership. |
| -// It also returns the app's key prefix, so callers can parse what comes after |
| -// it without re-deciding which of the two shapes they are looking at. |
| +// A LEGACY unqualified apps/<app>/… key is not linkage. It used to be (it |
| +// meant "ops, until this app is migrated"), but the read-fallback's |
| +// retirement on 2026-08-03 made those objects unreadable to the runtime, and |
| +// bytes nobody can serve say nothing about who owns an app. Acting on them |
| +// here would be worse than useless: reconciliation would mint routes in front |
| +// of builds no box can install, and a leftover copy under a migrated app |
| +// would claim it back for ops. Nothing removes legacy objects, so this stays |
| +// true for as long as they sit there. |
| +// |
| +// Keys with nothing after the app segment (apps/<sqid>/<app>) are not linkage |
| +// either: an object sitting directly at the app prefix says nothing about |
| +// ownership. It also returns the app's key prefix, so callers can parse what |
| +// comes after it without rebuilding it. |
| +// |
| +// Requiring a KNOWN sqid also closes a latent bug the old legacy branch hid: |
| +// a genuinely qualified key whose account had not materialized yet fell |
| +// through to it and came back as (ops, "<sqid>") — the account's own sqid |
| +// read as an app NAME, on any box whose account manifests reconcile had not |
| +// reached. That minted apps out of account ids. Now it returns nothing and |
| +// the next pass, with the account materialized, reads the key correctly. |
| func appKeyOwner(key string, known map[string]bool) (acct, app, prefix string, ok bool) { |
| rest, ok := strings.CutPrefix(key, "apps/") |
| if !ok { |
| return "", "", "", false |
| } |
| - first, rest, ok := strings.Cut(rest, "/") |
| - if !ok || first == "" { |
| + acct, rest, ok = strings.Cut(rest, "/") |
| + if !ok || !known[acct] { // legacy apps/<app>/…, or an account we know nothing about |
| return "", "", "", false |
| } |
| app, _, hasThird := strings.Cut(rest, "/") |
| + if !hasThird || app == "" { |
| + return "", "", "", false |
| + } |
| // The one place the two namespaces can genuinely collide: an app whose |
| - // NAME happens to equal a materialized account sqid (an app called "bab" |
| - // on a legacy key). What follows disambiguates it — a qualified key's |
| - // third segment is the app name, a legacy key's second segment is always |
| - // one of the release plane's own words. |
| - if !known[first] || app == "channels" || app == "releases" { |
| - return OpsSqid, first, "apps/" + first + "/", true // apps/<app>/… |
| - } |
| - if !hasThird || app == "" { // apps/<sqid>/<app>/… |
| + // NAME happens to equal a materialized account sqid — a legacy key for an |
| + // app called "bab" is apps/bab/channels/…, which parses as ops's app |
| + // "channels". A qualified key's third segment is the app name; a legacy |
| + // key's second is always one of the release plane's own words, so those |
| + // two words are never an app here. |
| + if app == "channels" || app == "releases" { |
| return "", "", "", false |
| } |
| - return first, app, "apps/" + first + "/" + app + "/", true |
| + return acct, app, "apps/" + acct + "/" + app + "/", true |
| } |
| -// canaryPointer reports whether key is a canary channel pointer — qualified |
| -// (apps/<sqid>/<app>/channels/canary/<name>.json) or legacy |
| -// (apps/<app>/channels/canary/<name>.json) — and its app + canary name. |
| +// canaryPointer reports whether key is an account-qualified canary channel |
| +// pointer (apps/<sqid>/<app>/channels/canary/<name>.json), and its app + |
| +// canary name. A legacy unqualified pointer is not one: see appKeyOwner. |
| func canaryPointer(key string, known map[string]bool) (app, name string, ok bool) { |
| _, app, prefix, ok := appKeyOwner(key, known) |
| if !ok { |
| @@ -412,7 +428,7 @@ type appKey struct{ acct, app string } |
| // The rules, in order: |
| // - the row's own account already holds objects: it is right, keep it; |
| // - exactly one account holds them: adopt it (this is what makes a |
| -// migration land, and what puts a legacy app under ops); |
| +// migration land); |
| // - several do, and the row names none of them: leave it alone. Picking one |
| // would re-home somebody's app onto another tenant's bytes, which is far |
| // worse than a row that reconciliation declines to touch. |
diff --git a/internal/domains/reconcile_test.go b/internal/domains/reconcile_test.go| index 6f9475d7e..54daecb30 100644 |
| --- a/internal/domains/reconcile_test.go |
| +++ b/internal/domains/reconcile_test.go |
| @@ -182,8 +182,10 @@ func TestReconcileRetiresTombstonedAccounts(t *testing.T) { |
| } |
| // The app→account linkage IS the bucket key path: an app under |
| -// apps/<sqid>/<app>/… belongs to that account; a legacy unqualified |
| -// apps/<app>/… belongs to ops until it is migrated. |
| +// apps/<sqid>/<app>/… belongs to that account. A legacy unqualified |
| +// apps/<app>/… key is not linkage at all any more — the runtime cannot read |
| +// it, so it says nothing about who owns what. oldapp here still lands under |
| +// ops, but from its ROUTE's default, not from the leftover object. |
| func TestReconcileDerivesAccountFromKeyPath(t *testing.T) { |
| s, reg := dirStore(t), regWith(t, |
| registry.Route{Host: "kass.training", Kind: "instance", App: "kass", Status: "ready", Addr: "127.0.0.1:9000"}, |
| @@ -209,6 +211,113 @@ func TestReconcileDerivesAccountFromKeyPath(t *testing.T) { |
| } |
| } |
| +// A leftover legacy object must not claim an app for ops. This is the shape |
| +// every migrated app's bucket is in — the migration copies and deliberately |
| +// never deletes — and the route was added before any of it, so it carries no |
| +// account yet. |
| +// |
| +// It used to go the other way: the legacy leftover made OPS an owner of |
| +// kass's objects, an unstamped route reads as ops, ops was among the owners, |
| +// and the route got stamped ops — pointing adopt at a prefix holding nothing |
| +// but the copy nobody serves. With legacy keys ignored there is exactly one |
| +// owner, the account the app actually lives in. |
| +func TestReconcileIgnoresLeftoverLegacyKeys(t *testing.T) { |
| + s, reg := dirStore(t), regWith(t, |
| + registry.Route{Host: "kass.training", Kind: "static", App: "kass", |
| + Status: "ready", Channel: "stable"}) |
| + sqid := writeAccountManifest(t, s, 793481920157, "Kass") |
| + putKey(t, s, "apps/"+sqid+"/kass/channels/stable.json") |
| + putKey(t, s, "apps/kass/channels/stable.json") |
| + if err := domains.Reconcile(ctx, s, reg, cfg(t)); err != nil { |
| + t.Fatal(err) |
| + } |
| + if src := mustGet(t, reg, "kass.training"); src.Account != sqid { |
| + t.Errorf("source route account = %q, want %s — a legacy leftover claimed the app for ops", |
| + src.Account, sqid) |
| + } |
| + mustGet(t, reg, "kass."+sqid+".oncarlos.test") // the apex mints under the real account |
| +} |
| + |
| +// A legacy canary pointer mints no canary host. The host would resolve a |
| +// channel the fleet cannot read, so it would serve a 404 under a URL the |
| +// console advertises as live. |
| +func TestReconcileMintsNothingForALegacyCanaryPointer(t *testing.T) { |
| + s, reg := dirStore(t), regWith(t, registry.Route{Host: "kass.training", Kind: "static", |
| + App: "kass", Status: "ready", Channel: "stable"}) |
| + putKey(t, s, "apps/kass/channels/canary/pricing.json") |
| + if err := domains.Reconcile(ctx, s, reg, cfg(t)); err != nil { |
| + t.Fatal(err) |
| + } |
| + if rt, err := reg.Get("pricing.kass.bab.oncarlos.test"); err != nil || rt != nil { |
| + t.Errorf("a legacy canary pointer minted %+v (%v) — nothing can serve it", rt, err) |
| + } |
| +} |
| + |
| +// PINNED, NOT ENDORSED. A platform-minted canary host stamped with one |
| +// account while its app's real canary pointer lives under another is removed |
| +// on the next pass. The stamp is what decides, and a single stray object |
| +// under the stamped account is enough to hold it there: resolveRouteAccount's |
| +// first rule is "the row's own account already holds objects: it is right, |
| +// keep it", so step 1b never re-stamps the row onto the account the pointer |
| +// is actually under, and the retirement loop then finds no pointer for the |
| +// (account, app) pair the row names. |
| +// |
| +// This shape predates the fallback's retirement — the retirement only changed |
| +// which pointers count — and a live pre-flight on 2026-08-03 found it does not |
| +// occur on either fleet: tito's reconcile is inert (no apps domain configured) |
| +// and the flagship registry holds zero Platform+Canary rows. It is pinned here |
| +// so the behaviour is known rather than discovered by a deleted host. |
| +// |
| +// If it ever does bite, the fix belongs in the STAMPING rule (a row whose |
| +// stamped account holds no pointer for its own channel is not "right"), never |
| +// in the retirement loop — softening the loop would strand every genuinely |
| +// retired canary host forever. |
| +func TestReconcileRemovesACanaryHostStampedWithTheWrongAccount(t *testing.T) { |
| + s, reg := dirStore(t), regWith(t, |
| + registry.Route{Host: "blog.kass.test", Kind: "static", App: "blog", |
| + Status: "ready", Channel: "stable", Account: "bab"}, |
| + registry.Route{Host: "pricing.blog.bab.oncarlos.test", Kind: "static", App: "blog", |
| + Status: "ready", Channel: "canary/pricing", Platform: true, Canary: true, Account: "bab"}, |
| + ) |
| + sqid := writeAccountManifest(t, s, 793481920157, "Kass") |
| + // One stray object under ops pins both rows to bab... |
| + putKey(t, s, "apps/bab/blog/releases/v1/manifest.json") |
| + // ...while the canary pointer the fleet actually follows is the tenant's. |
| + putKey(t, s, "apps/"+sqid+"/blog/channels/canary/pricing.json") |
| + if err := domains.Reconcile(ctx, s, reg, cfg(t)); err != nil { |
| + t.Fatal(err) |
| + } |
| + if rt, err := reg.Get("pricing.blog.bab.oncarlos.test"); err != nil || rt != nil { |
| + t.Errorf("canary route survived: %+v %v — if this now passes, the stamping rule changed; "+ |
| + "re-read the comment above before deleting this test", rt, err) |
| + } |
| + // The row that pinned it keeps its stamp, which is the mechanism at work. |
| + if src := mustGet(t, reg, "blog.kass.test"); src.Account != "bab" { |
| + t.Errorf("source route account = %q, want bab", src.Account) |
| + } |
| +} |
| + |
| +// A platform-minted canary host whose pointer only ever existed at the legacy |
| +// keys is retired like any other pointerless canary. It is the same removal |
| +// rule, reached because legacy keys stopped counting as live pointers: the |
| +// host resolved to a build the fleet cannot install, so keeping it alive |
| +// would advertise a URL that 404s. |
| +func TestReconcileRetiresCanaryHostLeftOnALegacyPointer(t *testing.T) { |
| + s, reg := dirStore(t), regWith(t, |
| + registry.Route{Host: "kass.training", Kind: "static", App: "kass", |
| + Status: "ready", Channel: "stable"}, |
| + registry.Route{Host: "pricing.kass.bab.oncarlos.test", Kind: "static", App: "kass", |
| + Status: "ready", Channel: "canary/pricing", Platform: true, Canary: true, Account: "bab"}, |
| + ) |
| + putKey(t, s, "apps/kass/channels/canary/pricing.json") |
| + if err := domains.Reconcile(ctx, s, reg, cfg(t)); err != nil { |
| + t.Fatal(err) |
| + } |
| + if rt, err := reg.Get("pricing.kass.bab.oncarlos.test"); err != nil || rt != nil { |
| + t.Errorf("canary host on a legacy-only pointer survived: %+v %v", rt, err) |
| + } |
| +} |
| + |
| // A malformed account manifest is one skipped line, never a failed pass, and |
| // never a guessed id. |
| func TestReconcileSkipsMalformedAccountManifest(t *testing.T) { |
| @@ -306,7 +415,7 @@ func TestReconcileMintsCanaryHost(t *testing.T) { |
| App: "kass", Status: "ready", Channel: "stable"}) |
| // The pointer's existence is what reconcile reads — content is not its |
| // business (the edge verifies signatures when serving). |
| - if _, err := s.Put(ctx, "apps/kass/channels/canary/pricing.json", []byte("{}"), ""); err != nil { |
| + if _, err := s.Put(ctx, "apps/bab/kass/channels/canary/pricing.json", []byte("{}"), ""); err != nil { |
| t.Fatal(err) |
| } |
| if err := domains.Reconcile(ctx, s, reg, cfg(t)); err != nil { |
| @@ -336,7 +445,7 @@ func TestReconcileMintsCanaryHost(t *testing.T) { |
| func TestReconcileSkipsCanaryMintForInstanceApps(t *testing.T) { |
| s, reg := dirStore(t), regWith(t, registry.Route{Host: "kass.training", Kind: "instance", |
| App: "kass", Status: "ready", Addr: "127.0.0.1:9000"}) |
| - if _, err := s.Put(ctx, "apps/kass/channels/canary/pricing.json", []byte("{}"), ""); err != nil { |
| + if _, err := s.Put(ctx, "apps/bab/kass/channels/canary/pricing.json", []byte("{}"), ""); err != nil { |
| t.Fatal(err) |
| } |
| if err := domains.Reconcile(ctx, s, reg, cfg(t)); err != nil { |
diff --git a/internal/edge/static.go b/internal/edge/static.go| index 585f90791..43b134cfe 100644 |
| --- a/internal/edge/static.go |
| +++ b/internal/edge/static.go |
| @@ -229,9 +229,7 @@ func (s *staticSite) ensureExtracted(ctx context.Context, version string) error |
| if art == nil { |
| return fmt.Errorf("release %s/%s has no %s artifact", s.app, version, siteArtifactFile) |
| } |
| - data, _, _, err := release.GetWithFallback(ctx, s.store, |
| - release.ArtifactKey(s.acct, s.app, version, art.File), |
| - release.LegacyArtifactKey(s.app, version, art.File)) |
| + data, _, err := release.GetQualified(ctx, s.store, release.ArtifactKey(s.acct, s.app, version, art.File)) |
| if err != nil { |
| return err |
| } |
diff --git a/internal/edge/static_test.go b/internal/edge/static_test.go| index 9f391bc20..3bd4188ce 100644 |
| --- a/internal/edge/static_test.go |
| +++ b/internal/edge/static_test.go |
| @@ -9,6 +9,7 @@ import ( |
| "net/http/httptest" |
| "os" |
| "path/filepath" |
| + "strings" |
| "testing" |
| "github.com/carlosframework/platform/internal/registry" |
| @@ -233,3 +234,78 @@ func TestEdgeServesStaticRouteWhenEnabled(t *testing.T) { |
| t.Fatalf("post-enable GET = %d %q, want 200 %q", code, body, "via the edge") |
| } |
| } |
| + |
| +// legacyTripwire fails the test if the edge asks the store for anything under |
| +// one app's LEGACY unqualified prefix. The release package has its own; the |
| +// edge needs one of its own too, because a tripwire only guards the paths a |
| +// test actually routes through it — and the static extract is where a |
| +// fallback would put unmigrated bytes in front of real traffic. |
| +type legacyTripwire struct { |
| + release.Store |
| + t *testing.T |
| + app string |
| +} |
| + |
| +func (l *legacyTripwire) trip(what, key string) { |
| + l.t.Helper() |
| + if strings.HasPrefix(key, release.LegacyAppPrefix(l.app)) { |
| + l.t.Errorf("the edge %s a legacy key: %s — the read-fallback is retired", what, key) |
| + } |
| +} |
| + |
| +func (l *legacyTripwire) Get(ctx context.Context, key string) ([]byte, string, error) { |
| + l.trip("read", key) |
| + return l.Store.Get(ctx, key) |
| +} |
| + |
| +func (l *legacyTripwire) List(ctx context.Context, prefix string) ([]string, error) { |
| + l.trip("listed", prefix) |
| + return l.Store.List(ctx, prefix) |
| +} |
| + |
| +// The static extract never falls back to a legacy artifact. The fixture is |
| +// the half-migrated shape that makes the read observable: pointer and |
| +// manifest qualified (so the handler resolves and gets as far as wanting the |
| +// tar), the tar itself only at the legacy key. A fallback reintroduced in |
| +// ensureExtracted would serve those bytes; qualified-only serves an error. |
| +// |
| +// Without this, a fallback in ensureExtracted passes the whole suite. |
| +func TestStaticExtractNeverReadsALegacyArtifact(t *testing.T) { |
| + ctx := context.Background() |
| + dir := release.NewDirStore(filepath.Join(t.TempDir(), "store")) |
| + priv, pub := testKeys(t) |
| + |
| + scratch := release.NewDirStore(filepath.Join(t.TempDir(), "scratch")) |
| + shipSite(t, scratch, "site", "v1", map[string]string{"index.html": "legacy bytes"}) |
| + if err := release.Promote(ctx, scratch, priv, release.OpsAccount, "site", "canary/x", "v1"); err != nil { |
| + t.Fatalf("Promote: %v", err) |
| + } |
| + keys, err := scratch.List(ctx, release.AppPrefix(release.OpsAccount, "site")) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + for _, k := range keys { |
| + body, _, err := scratch.Get(ctx, k) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + dst := k |
| + if strings.HasSuffix(k, siteArtifactFile) { |
| + dst = release.LegacyAppPrefix("site") + strings.TrimPrefix(k, release.AppPrefix(release.OpsAccount, "site")) |
| + } |
| + if _, err := dir.Put(ctx, dst, body, ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + |
| + s := &legacyTripwire{Store: dir, t: t, app: "site"} |
| + rt := registry.Route{Host: "site.example.com", Kind: "static", App: "site", Channel: "canary/x", Status: "ready"} |
| + h := newStaticHandler(rt, s, pub, filepath.Join(t.TempDir(), "cache")) |
| + h.(*staticSite).recheck = 0 |
| + srv := httptest.NewServer(h) |
| + defer srv.Close() |
| + |
| + if code, body := getPath(t, srv.URL, "site.example.com", "/"); code == 200 { |
| + t.Errorf("GET / = %d %q — the edge served an artifact that only exists at the legacy key", code, body) |
| + } |
| +} |
diff --git a/internal/release/adopt.go b/internal/release/adopt.go| index 5f6ef1228..895ecf69d 100644 |
| --- a/internal/release/adopt.go |
| +++ b/internal/release/adopt.go |
| @@ -113,6 +113,19 @@ func PassFilter(ctx context.Context, reg *registry.Registry, s Store, pub ed2551 |
| // silently install it on both. |
| type groupKey struct{ acct, app, channel string } |
| pointers := map[groupKey]*ChannelPointer{} |
| + // Whether an app has anything under its qualified prefix, cached for the |
| + // pass: one listing per (account, app), not one per channel. It decides |
| + // which of the two missing-pointer lines below an app gets. |
| + type acctApp struct{ acct, app string } |
| + // migratedState caches one pass's answer to "has this app anything under |
| + // its qualified prefix", together with the listing failure when there was |
| + // one — a failed listing must neither invent the migrate advice nor claim |
| + // the healthy skip (2026-08-03 review). |
| + type migratedState struct { |
| + has bool |
| + listErr error |
| + } |
| + migrated := map[acctApp]migratedState{} |
| for _, rt := range routes { |
| if !adopts(rt) { |
| continue |
| @@ -125,7 +138,39 @@ func PassFilter(ctx context.Context, reg *registry.Registry, s Store, pub ed2551 |
| if err != nil { |
| pointers[k] = nil // resolved-and-failed: don't retry per host |
| if errors.Is(err, os.ErrNotExist) { |
| - fmt.Printf(" %s/%s/%s: channel has no release yet\n", k.acct, k.app, k.channel) |
| + // SKIP, never abort: one app with no pointer must not stop |
| + // every other app on the box from installing. |
| + // |
| + // Two deployments produce a missing pointer, and only one of |
| + // them should be told to migrate. A channel with nothing |
| + // promoted yet is ordinary — a migrated app waiting on its |
| + // first stable promotion sits here for days — and gets the |
| + // plain line. An app with NOTHING under its qualified prefix |
| + // upgraded without migrating, and gets the store's error, |
| + // which names the verb. Advice that fires every hour on a |
| + // healthy fleet is advice nobody reads. |
| + ak := acctApp{k.acct, k.app} |
| + st, cached := migrated[ak] |
| + if !cached { |
| + has, lerr := AppHasQualifiedObjects(ctx, s, k.acct, k.app) |
| + // A listing we could not perform suppresses the advice |
| + // rather than inventing it: a bucket this box cannot list |
| + // is not evidence that an app never migrated. It is not |
| + // evidence of a healthy skip either, so the failure rides |
| + // along and is printed instead of a claim this box never |
| + // established (realistic trigger: GetObject IAM without |
| + // ListBucket). |
| + st = migratedState{has: has || lerr != nil, listErr: lerr} |
| + migrated[ak] = st |
| + } |
| + switch { |
| + case st.listErr != nil: |
| + fmt.Printf(" %s/%s/%s: channel state unknown (could not list the app's prefix: %v)\n", k.acct, k.app, k.channel, st.listErr) |
| + case st.has: |
| + fmt.Printf(" %s/%s/%s: channel has no release yet\n", k.acct, k.app, k.channel) |
| + default: |
| + fmt.Printf(" %s/%s/%s: channel has no release yet (%v)\n", k.acct, k.app, k.channel, err) |
| + } |
| } else { |
| fmt.Printf(" %s/%s/%s: channel pointer rejected: %v\n", k.acct, k.app, k.channel, err) |
| } |
| @@ -322,8 +367,7 @@ func ensureStaged(ctx context.Context, s Store, optDir, acct, app, ver string) e |
| // catches up. |
| return fmt.Errorf("release %s has no %s build — skipping", ver, adoptPlatform) |
| } |
| - body, _, _, err := GetWithFallback(ctx, s, |
| - ArtifactKey(acct, app, ver, art.File), LegacyArtifactKey(app, ver, art.File)) |
| + body, _, err := GetQualified(ctx, s, ArtifactKey(acct, app, ver, art.File)) |
| if err != nil { |
| return fmt.Errorf("download %s: %w", ver, err) |
| } |
diff --git a/internal/release/fallback.go b/internal/release/fallback.go| deleted file mode 100644 |
| index cd0e017d7..000000000 |
| --- a/internal/release/fallback.go |
| +++ /dev/null |
| @@ -1,99 +0,0 @@ |
| -// The migration seam. App identity became account-scoped (bucket keys gained |
| -// an account-sqid segment), but the DATA moves days after the CODE lands — |
| -// there is deliberately no flag day. So every read path tries the qualified |
| -// key first and falls back to the legacy unqualified one; every write path |
| -// uses the qualified key only. |
| -// |
| -// This whole file is temporary by design: it is deleted once both deployments |
| -// are migrated and verified (spec §5's gate — every channel pointer resolves |
| -// qualified with the fallback disabled, and one adopt pass is a no-op). |
| -// Deleting the LEGACY OBJECTS is an operator step behind that gate; nothing |
| -// in the code ever removes one. |
| -package release |
| - |
| -import ( |
| - "context" |
| - "errors" |
| - "os" |
| - "regexp" |
| -) |
| - |
| -// OpsAccount is the built-in operator account's sqid — registry id 1 under |
| -// the frozen encoder. It is the default account for CLI-added apps, and what |
| -// 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 — |
| -// into a bucket key. |
| -var acctRe = regexp.MustCompile(`^[a-z]{3,16}$`) |
| - |
| -// ValidAccount reports whether a is usable as the account segment of a bucket |
| -// key. |
| -func ValidAccount(a string) bool { return acctRe.MatchString(a) } |
| - |
| -// GetWithFallback reads key, and on NOT-FOUND reads legacy instead. It |
| -// reports which one answered, so callers can log the migration's progress and |
| -// so a writer knows whether it holds an etag for the key it is about to write |
| -// (it does not: a legacy read's etag belongs to a different object, and the |
| -// qualified Put must be create-only). |
| -// |
| -// A store error that is not "not found" is returned as-is and never retried |
| -// against legacy. Masking a broken bucket as a fallback miss would make it |
| -// look like an unmigrated app — and the migration would then happily "verify" |
| -// an app it never actually read. |
| -func GetWithFallback(ctx context.Context, s Store, key, legacy string) (body []byte, etag string, usedLegacy bool, err error) { |
| - body, etag, err = s.Get(ctx, key) |
| - if err == nil { |
| - return body, etag, false, nil |
| - } |
| - if !errors.Is(err, os.ErrNotExist) { |
| - return nil, "", false, err |
| - } |
| - body, etag, err = s.Get(ctx, legacy) |
| - if err != nil { |
| - // Both absent: report the miss against the key the caller asked for, |
| - // still os.ErrNotExist-wrapped so errors.Is keeps working. |
| - return nil, "", false, err |
| - } |
| - return body, etag, true, nil |
| -} |
| - |
| -// ListWithFallback lists prefix, and falls back to the legacy prefix when the |
| -// qualified one holds nothing. It returns the prefix that answered so callers |
| -// can trim key names correctly. |
| -// |
| -// "Empty" is the miss condition here rather than an error, because listing a |
| -// prefix with no objects under it is not an error in either store — which |
| -// also means a genuinely empty qualified prefix reads as unmigrated. That is |
| -// the honest reading during the window: an app with nothing under its |
| -// qualified prefix has not moved yet. |
| -func ListWithFallback(ctx context.Context, s Store, prefix, legacy string) (keys []string, prefixUsed string, usedLegacy bool, err error) { |
| - keys, err = s.List(ctx, prefix) |
| - if err != nil { |
| - return nil, "", false, err |
| - } |
| - if len(keys) > 0 { |
| - return keys, prefix, false, nil |
| - } |
| - keys, err = s.List(ctx, legacy) |
| - if err != nil { |
| - return nil, "", false, err |
| - } |
| - return keys, legacy, true, nil |
| -} |
diff --git a/internal/release/fallback_test.go b/internal/release/fallback_test.go| deleted file mode 100644 |
| index 4e370321a..000000000 |
| --- a/internal/release/fallback_test.go |
| +++ /dev/null |
| @@ -1,190 +0,0 @@ |
| -package release |
| - |
| -import ( |
| - "context" |
| - "encoding/json" |
| - "errors" |
| - "os" |
| - "strings" |
| - "testing" |
| -) |
| - |
| -// erroringStore is a Store whose every operation fails with a NON-not-found |
| -// error — the "bucket on fire" case the fallback must never swallow. |
| -type erroringStore struct{ err error } |
| - |
| -func (e *erroringStore) Get(context.Context, string) ([]byte, string, error) { |
| - return nil, "", e.err |
| -} |
| -func (e *erroringStore) Put(context.Context, string, []byte, string) (string, error) { |
| - return "", e.err |
| -} |
| -func (e *erroringStore) List(context.Context, string) ([]string, error) { return nil, e.err } |
| - |
| -func TestKeysAreAccountScoped(t *testing.T) { |
| - if got := ChannelKey("kqmv", "kass", "stable"); got != "apps/kqmv/kass/channels/stable.json" { |
| - t.Errorf("ChannelKey = %q", got) |
| - } |
| - if got := ManifestKey("bab", "console", "abc1234"); got != "apps/bab/console/releases/abc1234/manifest.json" { |
| - t.Errorf("ManifestKey = %q", got) |
| - } |
| - if got := ArtifactKey("kqmv", "kass", "v1", "kass"); got != "apps/kqmv/kass/releases/v1/kass" { |
| - t.Errorf("ArtifactKey = %q", got) |
| - } |
| - // The legacy shapes stay reachable by name, for the fallback and the |
| - // migration verb only. |
| - if got := LegacyChannelKey("kass", "stable"); got != "apps/kass/channels/stable.json" { |
| - t.Errorf("LegacyChannelKey = %q", got) |
| - } |
| - if got := LegacyManifestKey("console", "abc1234"); got != "apps/console/releases/abc1234/manifest.json" { |
| - t.Errorf("LegacyManifestKey = %q", got) |
| - } |
| - if got := LegacyArtifactKey("kass", "v1", "kass"); got != "apps/kass/releases/v1/kass" { |
| - t.Errorf("LegacyArtifactKey = %q", got) |
| - } |
| -} |
| - |
| -// An empty account must never silently rebuild a legacy key: that would make |
| -// an unqualified write look like a successful qualified one, and the |
| -// migration would then never notice the app had not moved. |
| -func TestEmptyAccountDoesNotRebuildALegacyKey(t *testing.T) { |
| - if got := ChannelKey("", "kass", "stable"); got == LegacyChannelKey("kass", "stable") { |
| - t.Errorf("ChannelKey(\"\", ...) = %q — an empty account rebuilt the legacy key", got) |
| - } |
| -} |
| - |
| -func TestGetWithFallbackPrefersQualifiedThenFallsBack(t *testing.T) { |
| - s := NewDirStore(t.TempDir()) |
| - ctx := context.Background() |
| - if _, err := s.Put(ctx, "apps/kass/channels/stable.json", []byte("legacy"), ""); err != nil { |
| - t.Fatal(err) |
| - } |
| - body, _, used, err := GetWithFallback(ctx, s, "apps/kqmv/kass/channels/stable.json", "apps/kass/channels/stable.json") |
| - if err != nil || string(body) != "legacy" || !used { |
| - t.Fatalf("fallback: body=%q used=%v err=%v", body, used, err) |
| - } |
| - if _, err := s.Put(ctx, "apps/kqmv/kass/channels/stable.json", []byte("qualified"), ""); err != nil { |
| - t.Fatal(err) |
| - } |
| - body, _, used, err = GetWithFallback(ctx, s, "apps/kqmv/kass/channels/stable.json", "apps/kass/channels/stable.json") |
| - if err != nil || string(body) != "qualified" || used { |
| - t.Fatalf("qualified must win: body=%q used=%v err=%v", body, used, err) |
| - } |
| -} |
| - |
| -// A store error that is NOT not-found must surface, never be masked as a |
| -// fallback miss — masking would make a broken bucket look like an unmigrated |
| -// app, and the migration's verification gate would then pass on a lie. |
| -func TestGetWithFallbackPropagatesRealErrors(t *testing.T) { |
| - s := &erroringStore{err: errors.New("bucket on fire")} |
| - _, _, _, err := GetWithFallback(context.Background(), s, "a", "b") |
| - if err == nil || !strings.Contains(err.Error(), "on fire") { |
| - t.Fatalf("want the real error, got %v", err) |
| - } |
| -} |
| - |
| -// Neither key present is an honest not-found, so callers can keep using |
| -// errors.Is(err, os.ErrNotExist) as "this app has never shipped here". |
| -func TestGetWithFallbackMissesAsNotExist(t *testing.T) { |
| - s := NewDirStore(t.TempDir()) |
| - _, _, _, err := GetWithFallback(context.Background(), s, "apps/kqmv/kass/x.json", "apps/kass/x.json") |
| - if !errors.Is(err, os.ErrNotExist) { |
| - t.Fatalf("both-absent err = %v, want os.ErrNotExist-wrapped", err) |
| - } |
| -} |
| - |
| -func TestListWithFallback(t *testing.T) { |
| - s := NewDirStore(t.TempDir()) |
| - ctx := context.Background() |
| - if _, err := s.Put(ctx, "apps/kass/channels/stable.json", []byte("x"), ""); err != nil { |
| - t.Fatal(err) |
| - } |
| - keys, prefix, used, err := ListWithFallback(ctx, s, "apps/kqmv/kass/channels/", "apps/kass/channels/") |
| - if err != nil || len(keys) != 1 || !used || prefix != "apps/kass/channels/" { |
| - t.Fatalf("fallback list = %v prefix=%q used=%v err=%v", keys, prefix, used, err) |
| - } |
| - if _, err := s.Put(ctx, "apps/kqmv/kass/channels/beta.json", []byte("y"), ""); err != nil { |
| - t.Fatal(err) |
| - } |
| - keys, prefix, used, err = ListWithFallback(ctx, s, "apps/kqmv/kass/channels/", "apps/kass/channels/") |
| - if err != nil || len(keys) != 1 || used || prefix != "apps/kqmv/kass/channels/" { |
| - t.Fatalf("qualified list must win: %v prefix=%q used=%v err=%v", keys, prefix, used, err) |
| - } |
| -} |
| - |
| -// Promote against an app whose pointer still lives at the legacy key must |
| -// carry that pointer's history forward AND land the new pointer at the |
| -// QUALIFIED key — the read falls back, the write never does. |
| -func TestPromoteReadsLegacyPointerAndWritesQualified(t *testing.T) { |
| - s := NewDirStore(t.TempDir()) |
| - ctx := context.Background() |
| - priv, pub := testKeys(t) |
| - |
| - // Ship two versions under the qualified prefix, then hand-place a legacy |
| - // pointer at v1 (the shape a pre-migration bucket is in). |
| - shipRelease(t, s, "kqmv", "myapp", "v1", "linux-amd64", []byte("v1")) |
| - shipRelease(t, s, "kqmv", "myapp", "v2", "linux-amd64", []byte("v2")) |
| - legacy, err := signPointer(priv, &ChannelPointer{App: "myapp", Channel: "stable", Version: "v1"}) |
| - if err != nil { |
| - t.Fatal(err) |
| - } |
| - if _, err := s.Put(ctx, LegacyChannelKey("myapp", "stable"), legacy, ""); err != nil { |
| - t.Fatal(err) |
| - } |
| - |
| - if err := Promote(ctx, s, priv, "kqmv", "myapp", "stable", "v2"); err != nil { |
| - t.Fatalf("Promote over a legacy pointer: %v", err) |
| - } |
| - if _, _, err := s.Get(ctx, ChannelKey("kqmv", "myapp", "stable")); err != nil { |
| - t.Fatalf("promote did not write the qualified key: %v", err) |
| - } |
| - ptr, err := Channel(ctx, s, pub, "kqmv", "myapp", "stable") |
| - if err != nil { |
| - t.Fatal(err) |
| - } |
| - if ptr.Version != "v2" || len(ptr.History) == 0 || ptr.History[0] != "v1" { |
| - t.Fatalf("pointer = %s history %v, want v2 over v1 (legacy history carried forward)", ptr.Version, ptr.History) |
| - } |
| - // Nothing in this plan deletes a legacy object: the old pointer is still |
| - // there, now shadowed by the qualified one. |
| - if _, _, err := s.Get(ctx, LegacyChannelKey("myapp", "stable")); err != nil { |
| - t.Errorf("promote deleted the legacy pointer: %v", err) |
| - } |
| -} |
| - |
| -// The whole point of the fallback: an app that has never been migrated still |
| -// resolves its channel and its manifest through the account-scoped API. |
| -func TestChannelAndManifestFallBackToLegacyKeys(t *testing.T) { |
| - s := NewDirStore(t.TempDir()) |
| - ctx := context.Background() |
| - priv, pub := testKeys(t) |
| - |
| - // A wholly legacy app: manifest, artifact and pointer all unqualified. |
| - m := Manifest{Schema: 1, App: "old", Version: "v1", Kind: "binary", |
| - Artifacts: map[string]Artifact{"linux-amd64": {File: "old", SHA256: etagOf([]byte("bin")), Size: 3}}} |
| - raw, err := json.MarshalIndent(m, "", " ") |
| - if err != nil { |
| - t.Fatal(err) |
| - } |
| - if _, err := s.Put(ctx, LegacyArtifactKey("old", "v1", "old"), []byte("bin"), ""); err != nil { |
| - t.Fatal(err) |
| - } |
| - if _, err := s.Put(ctx, LegacyManifestKey("old", "v1"), raw, ""); err != nil { |
| - t.Fatal(err) |
| - } |
| - ptr, err := signPointer(priv, &ChannelPointer{App: "old", Channel: "stable", Version: "v1"}) |
| - if err != nil { |
| - t.Fatal(err) |
| - } |
| - if _, err := s.Put(ctx, LegacyChannelKey("old", "stable"), ptr, ""); err != nil { |
| - t.Fatal(err) |
| - } |
| - |
| - if _, err := GetManifest(ctx, s, "bab", "old", "v1"); err != nil { |
| - t.Fatalf("GetManifest did not fall back: %v", err) |
| - } |
| - got, err := Channel(ctx, s, pub, "bab", "old", "stable") |
| - if err != nil || got.Version != "v1" { |
| - t.Fatalf("Channel did not fall back: %+v %v", got, err) |
| - } |
| -} |
diff --git a/internal/release/qualified.go b/internal/release/qualified.go| new file mode 100644 |
| index 000000000..2fdb1f685 |
| --- /dev/null |
| +++ b/internal/release/qualified.go |
| @@ -0,0 +1,184 @@ |
| +// The account-qualified key space, and the sentinel that names what came |
| +// before it. |
| +// |
| +// App identity is account-scoped: every bucket key carries an account sqid |
| +// (apps/<sqid>/<app>/…). It was not always so. While the data caught up with |
| +// the code, every read path tried the qualified key and FELL BACK to the |
| +// legacy unqualified apps/<app>/… shape, so there was no flag day. |
| +// |
| +// **The read-fallback was retired on 2026-08-03** (spec §2/§5), once every |
| +// app with a route on either production deployment had been migrated and |
| +// verified. Its job was done, and each read it still performed was another |
| +// chance for a stale legacy object to shadow reality — the failure mode that |
| +// nearly forced a rollback during the migration itself. The runtime now reads |
| +// account-qualified keys and nothing else. |
| +// |
| +// What remains is the other half of the bargain. No code path ever deleted a |
| +// legacy object, and a deployment upgrading straight from a pre-accounts |
| +// binary still has all of its data at the legacy keys — so |
| +// `carlos accounts migrate -from legacy` reads them EXPLICITLY, forever |
| +// (GetMigrationSource), and a qualified miss names that command instead of |
| +// 404ing mysteriously (GetQualified). |
| +package release |
| + |
| +import ( |
| + "context" |
| + "errors" |
| + "fmt" |
| + "os" |
| + "regexp" |
| + "strings" |
| +) |
| + |
| +// OpsAccount is the built-in operator account's sqid — registry id 1 under |
| +// the frozen encoder. It is the default account for CLI-added apps, and what |
| +// 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 — |
| +// into a bucket key. |
| +var acctRe = regexp.MustCompile(`^[a-z]{3,16}$`) |
| + |
| +// ValidAccount reports whether a is usable as the account segment of a bucket |
| +// key. |
| +func ValidAccount(a string) bool { return acctRe.MatchString(a) } |
| + |
| +// GetQualified reads an account-qualified key. Not-found stays |
| +// os.ErrNotExist-wrapped (every caller's "has this app ever shipped here" |
| +// check keeps working) and gains the migration hint below. |
| +// |
| +// It is where every runtime read of an OBJECT goes — manifests, channel |
| +// pointers, artifacts — so the sentence a miss needs is written once. Two |
| +// kinds of read do not pass through it, and neither is a place the fallback |
| +// could grow back: |
| +// |
| +// - LISTINGS (VersionRung, `carlos channels`, the console's channel and |
| +// release views) call s.List on a prefix built by ChannelsPrefix or |
| +// ReleasesPrefix. There is nothing to fall back to: a prefix either has |
| +// objects under it or the app has none here, and both are answers. |
| +// - Promote's read-modify-write of a channel pointer calls GetQualified but |
| +// DISCARDS the hint, because an absent pointer is the legitimate first |
| +// promotion of a fresh channel. PromoteOnLadder supplies the hint from |
| +// the whole app's prefix instead, where the distinction is computable. |
| +// |
| +// What is true without qualification: no read anywhere in the runtime builds |
| +// a legacy key. The tripwire tests are what keep that honest, and they guard |
| +// the store rather than any one helper — the artifact downloads in |
| +// adopt.ensureStaged and edge.ensureExtracted each get their own pass, |
| +// because a tripwire only guards the paths a test routes through it. |
| +// |
| +// A store error that is not "not found" is returned untouched and never |
| +// annotated: an unreachable bucket is not an unmigrated app, and pointing an |
| +// operator at `accounts migrate` when the bucket is broken sends them at the |
| +// wrong problem. |
| +func GetQualified(ctx context.Context, s Store, key string) (body []byte, etag string, err error) { |
| + body, etag, err = s.Get(ctx, key) |
| + if err != nil && errors.Is(err, os.ErrNotExist) { |
| + return nil, "", fmt.Errorf("%w%s", err, migrationHint(key)) |
| + } |
| + return body, etag, err |
| +} |
| + |
| +// migrationHint turns a miss on an account-qualified APP key into the exact |
| +// command that fixes it, for the one deployment shape that produces a |
| +// surprising 404: upgraded from a pre-accounts binary, objects still at |
| +// apps/<app>/…, nothing migrated. Every runtime read funnels through |
| +// GetQualified, so this sentence is written once and reaches all of them. |
| +// |
| +// It is deliberately unconditional rather than probing the legacy key first. |
| +// Confirming the hint would mean the runtime reading a legacy key again — on |
| +// the error path only, but "the runtime never reads a legacy key" is a |
| +// property worth having outright rather than with an exception, and it is |
| +// what the tripwire test can then pin. The wording carries the condition |
| +// instead, so it stays honest for an app that simply has no such channel. |
| +// |
| +// "" for anything that is not an app key: a status snapshot or an account |
| +// manifest must not advertise a command that cannot apply to it. |
| +func migrationHint(key string) string { |
| + rest, ok := strings.CutPrefix(key, "apps/") |
| + if !ok { |
| + return "" |
| + } |
| + acct, rest, ok := strings.Cut(rest, "/") |
| + if !ok || !ValidAccount(acct) { |
| + return "" |
| + } |
| + app, _, ok := strings.Cut(rest, "/") |
| + if !ok || !ValidApp(app) { |
| + return "" |
| + } |
| + return fmt.Sprintf(" — if this deployment predates account-scoped bucket keys its objects "+ |
| + "are still at %s, which the runtime no longer reads; move them once with: "+ |
| + "carlos accounts migrate -app %s -from legacy -to %s", |
| + legacyAppPrefix(app), app, acct) |
| +} |
| + |
| +// AppHasQualifiedObjects reports whether app has ANYTHING under its |
| +// account-qualified prefix. It is the whole distinction between the two |
| +// deployments a missing object can mean: |
| +// |
| +// - nothing there at all — this deployment upgraded from a pre-accounts |
| +// binary and never migrated. `accounts migrate -from legacy` is the fix, |
| +// and saying so is the point of the hint. |
| +// - something there, but not this key — ordinary. A migrated app waiting on |
| +// its first promotion to a channel is in exactly this state, and telling |
| +// it to migrate would be wrong every hour of every day until somebody |
| +// promotes. |
| +// |
| +// "Anything at all" rather than "any releases": an app that has shipped since |
| +// accounts landed is not unmigrated however new the channel is. Both readings |
| +// come from the qualified prefix; neither touches a legacy key. |
| +// |
| +// Callers that ask per channel should cache it per (account, app) — one |
| +// listing answers for every channel an app has. |
| +func AppHasQualifiedObjects(ctx context.Context, s Store, acct, app string) (bool, error) { |
| + keys, err := s.List(ctx, AppPrefix(acct, app)) |
| + return len(keys) > 0, err |
| +} |
| + |
| +// GetMigrationSource reads key, and on NOT-FOUND reads legacy instead. It is |
| +// the shape the retired read-fallback had, kept for the ONE caller whose |
| +// access to the account-less key space is explicit and permanent: `carlos |
| +// accounts migrate`, which must be able to move an app off those keys however |
| +// long a deployment waits before upgrading. Nothing in a serving path may |
| +// call it. |
| +// |
| +// It reports which key answered, so the verb's post-copy verification knows |
| +// what it just compared against. |
| +// |
| +// A store error that is not "not found" is returned as-is and never retried |
| +// against legacy. Masking a broken bucket as a fallback miss would make it |
| +// look like an unmigrated app — and the migration would then happily "verify" |
| +// an app it never actually read. |
| +func GetMigrationSource(ctx context.Context, s Store, key, legacy string) (body []byte, etag string, usedLegacy bool, err error) { |
| + body, etag, err = s.Get(ctx, key) |
| + if err == nil { |
| + return body, etag, false, nil |
| + } |
| + if !errors.Is(err, os.ErrNotExist) { |
| + return nil, "", false, err |
| + } |
| + body, etag, err = s.Get(ctx, legacy) |
| + if err != nil { |
| + // Both absent: report the miss against the key the caller asked for, |
| + // still os.ErrNotExist-wrapped so errors.Is keeps working. |
| + return nil, "", false, err |
| + } |
| + return body, etag, true, nil |
| +} |
diff --git a/internal/release/qualified_test.go b/internal/release/qualified_test.go| new file mode 100644 |
| index 000000000..ea53feed1 |
| --- /dev/null |
| +++ b/internal/release/qualified_test.go |
| @@ -0,0 +1,550 @@ |
| +package release |
| + |
| +import ( |
| + "bytes" |
| + "context" |
| + "encoding/json" |
| + "errors" |
| + "fmt" |
| + "os" |
| + "strings" |
| + "testing" |
| + |
| + "github.com/carlosframework/platform/internal/registry" |
| +) |
| + |
| +// erroringStore is a Store whose every operation fails with a NON-not-found |
| +// error — the "bucket on fire" case a read must never swallow. |
| +type erroringStore struct{ err error } |
| + |
| +func (e *erroringStore) Get(context.Context, string) ([]byte, string, error) { |
| + return nil, "", e.err |
| +} |
| +func (e *erroringStore) Put(context.Context, string, []byte, string) (string, error) { |
| + return "", e.err |
| +} |
| +func (e *erroringStore) List(context.Context, string) ([]string, error) { return nil, e.err } |
| + |
| +// legacyTripwire fails the test if anything asks the store for a key or |
| +// prefix under one app's LEGACY unqualified prefix. It is how "the runtime |
| +// does not read legacy keys" is pinned as a property rather than asserted one |
| +// call site at a time: a fallback that crept back into any read path would |
| +// trip it, whatever shape it took. |
| +type legacyTripwire struct { |
| + Store |
| + t *testing.T |
| + app string |
| +} |
| + |
| +func (l *legacyTripwire) tripped(what, key string) { |
| + l.t.Helper() |
| + if strings.HasPrefix(key, LegacyAppPrefix(l.app)) { |
| + l.t.Errorf("the runtime %s a legacy key: %s — the read-fallback is retired", what, key) |
| + } |
| +} |
| + |
| +func (l *legacyTripwire) Get(ctx context.Context, key string) ([]byte, string, error) { |
| + l.tripped("read", key) |
| + return l.Store.Get(ctx, key) |
| +} |
| + |
| +func (l *legacyTripwire) List(ctx context.Context, prefix string) ([]string, error) { |
| + l.tripped("listed", prefix) |
| + return l.Store.List(ctx, prefix) |
| +} |
| + |
| +func TestKeysAreAccountScoped(t *testing.T) { |
| + if got := ChannelKey("kqmv", "kass", "stable"); got != "apps/kqmv/kass/channels/stable.json" { |
| + t.Errorf("ChannelKey = %q", got) |
| + } |
| + if got := ManifestKey("bab", "console", "abc1234"); got != "apps/bab/console/releases/abc1234/manifest.json" { |
| + t.Errorf("ManifestKey = %q", got) |
| + } |
| + if got := ArtifactKey("kqmv", "kass", "v1", "kass"); got != "apps/kqmv/kass/releases/v1/kass" { |
| + t.Errorf("ArtifactKey = %q", got) |
| + } |
| + // The legacy shapes stay reachable BY NAME — no read path resolves |
| + // through them any more, but `accounts migrate -from legacy` still has to |
| + // be able to spell them, forever. |
| + if got := LegacyChannelKey("kass", "stable"); got != "apps/kass/channels/stable.json" { |
| + t.Errorf("LegacyChannelKey = %q", got) |
| + } |
| + if got := LegacyManifestKey("console", "abc1234"); got != "apps/console/releases/abc1234/manifest.json" { |
| + t.Errorf("LegacyManifestKey = %q", got) |
| + } |
| + if got := LegacyArtifactKey("kass", "v1", "kass"); got != "apps/kass/releases/v1/kass" { |
| + t.Errorf("LegacyArtifactKey = %q", got) |
| + } |
| +} |
| + |
| +// An empty account must never silently rebuild a legacy key: that would make |
| +// an unqualified write look like a successful qualified one, and the |
| +// migration would then never notice the app had not moved. |
| +func TestEmptyAccountDoesNotRebuildALegacyKey(t *testing.T) { |
| + if got := ChannelKey("", "kass", "stable"); got == LegacyChannelKey("kass", "stable") { |
| + t.Errorf("ChannelKey(\"\", ...) = %q — an empty account rebuilt the legacy key", got) |
| + } |
| +} |
| + |
| +// The retirement, at its smallest: a legacy object at the matching |
| +// unqualified key does not answer a qualified read. It used to. |
| +func TestGetQualifiedDoesNotFallBack(t *testing.T) { |
| + s := NewDirStore(t.TempDir()) |
| + ctx := context.Background() |
| + if _, err := s.Put(ctx, "apps/kass/channels/stable.json", []byte("legacy"), ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, _, err := GetQualified(ctx, s, "apps/kqmv/kass/channels/stable.json"); !errors.Is(err, os.ErrNotExist) { |
| + t.Fatalf("a legacy object answered a qualified read: err = %v", err) |
| + } |
| + if _, err := s.Put(ctx, "apps/kqmv/kass/channels/stable.json", []byte("qualified"), ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + body, _, err := GetQualified(ctx, s, "apps/kqmv/kass/channels/stable.json") |
| + if err != nil || string(body) != "qualified" { |
| + t.Fatalf("qualified read: body=%q err=%v", body, err) |
| + } |
| +} |
| + |
| +// A miss is actionable: a deployment that upgraded from a pre-accounts binary |
| +// without migrating gets the exact command, not a mystery 404. It is still |
| +// os.ErrNotExist-wrapped, so every caller's "has this app ever shipped here" |
| +// check keeps working. |
| +func TestQualifiedMissNamesTheMigrationCommand(t *testing.T) { |
| + s := NewDirStore(t.TempDir()) |
| + _, _, err := GetQualified(context.Background(), s, ChannelKey("kqmv", "kass", "stable")) |
| + if !errors.Is(err, os.ErrNotExist) { |
| + t.Fatalf("miss = %v, want os.ErrNotExist-wrapped", err) |
| + } |
| + if !strings.Contains(err.Error(), "carlos accounts migrate -app kass -from legacy -to kqmv") { |
| + t.Errorf("miss does not name the fix:\n%s", err) |
| + } |
| +} |
| + |
| +// The hint is only for app keys. A miss on anything else (status snapshots, |
| +// account manifests) must not advertise a command that cannot apply to it. |
| +func TestNonAppKeyMissCarriesNoMigrationHint(t *testing.T) { |
| + s := NewDirStore(t.TempDir()) |
| + _, _, err := GetQualified(context.Background(), s, "status/fleet.json") |
| + if !errors.Is(err, os.ErrNotExist) { |
| + t.Fatalf("miss = %v, want os.ErrNotExist-wrapped", err) |
| + } |
| + if strings.Contains(err.Error(), "accounts migrate") { |
| + t.Errorf("status/fleet.json miss suggested a migration:\n%s", err) |
| + } |
| +} |
| + |
| +// A store error that is NOT not-found must surface untouched, and must never |
| +// acquire a migration hint: telling an operator to migrate because the bucket |
| +// is unreachable sends them at the wrong problem. |
| +func TestGetQualifiedPropagatesRealErrors(t *testing.T) { |
| + s := &erroringStore{err: errors.New("bucket on fire")} |
| + _, _, err := GetQualified(context.Background(), s, ChannelKey("kqmv", "kass", "stable")) |
| + if err == nil || !strings.Contains(err.Error(), "on fire") { |
| + t.Fatalf("want the real error, got %v", err) |
| + } |
| + if strings.Contains(err.Error(), "accounts migrate") { |
| + t.Errorf("a broken bucket was reported as an unmigrated app:\n%s", err) |
| + } |
| +} |
| + |
| +// THE test the retirement exists for: an app whose objects are ONLY at the |
| +// legacy keys is invisible to every runtime read path. Nothing resolves it, |
| +// nothing lists it, and — the tripwire — nothing so much as touches its |
| +// legacy prefix on the way to finding that out. |
| +// |
| +// Adopt SKIPS such an app rather than failing the pass: one app's missing |
| +// pointer must not stop every other app on the box from installing, which is |
| +// already how a channel with nothing promoted behaves. The loudness lives in |
| +// the error text (asserted here), which adopt prints per channel. |
| +func TestLegacyOnlyAppIsInvisibleToTheRuntime(t *testing.T) { |
| + dir := NewDirStore(t.TempDir()) |
| + ctx := context.Background() |
| + priv, pub := testKeys(t) |
| + s := &legacyTripwire{Store: dir, t: t, app: "old"} |
| + |
| + // A wholly legacy app: manifest, artifact and pointer all unqualified. |
| + m := Manifest{Schema: 1, App: "old", Version: "v1", Kind: "binary", |
| + Artifacts: map[string]Artifact{"linux-amd64": {File: "old", SHA256: etagOf([]byte("bin")), Size: 3}}} |
| + raw, err := json.MarshalIndent(m, "", " ") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := dir.Put(ctx, LegacyArtifactKey("old", "v1", "old"), []byte("bin"), ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := dir.Put(ctx, LegacyManifestKey("old", "v1"), raw, ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + ptr, err := signPointer(priv, &ChannelPointer{App: "old", Channel: "stable", Version: "v1"}) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := dir.Put(ctx, LegacyChannelKey("old", "stable"), ptr, ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + if _, err := GetManifest(ctx, s, OpsAccount, "old", "v1"); !errors.Is(err, os.ErrNotExist) { |
| + t.Errorf("GetManifest resolved a legacy-only app: %v", err) |
| + } else if !strings.Contains(err.Error(), "-from legacy") { |
| + t.Errorf("GetManifest miss is not actionable:\n%s", err) |
| + } |
| + if _, err := Channel(ctx, s, pub, OpsAccount, "old", "stable"); !errors.Is(err, os.ErrNotExist) { |
| + t.Errorf("Channel resolved a legacy-only app: %v", err) |
| + } else if !strings.Contains(err.Error(), "carlos accounts migrate -app old -from legacy -to bab") { |
| + t.Errorf("Channel miss is not actionable:\n%s", err) |
| + } |
| + // Listing paths see nothing either, so the console and `carlos channels` |
| + // show an empty app rather than a row they cannot resolve. |
| + keys, err := s.List(ctx, ChannelsPrefix(OpsAccount, "old")) |
| + if err != nil || len(keys) != 0 { |
| + t.Errorf("channels listing = %v (err %v), want empty", keys, err) |
| + } |
| + rung, err := VersionRung(ctx, s, pub, OpsAccount, "old", "v1") |
| + if err != nil || rung != "" { |
| + t.Errorf("VersionRung = %q (err %v), want no rung", rung, err) |
| + } |
| +} |
| + |
| +// A tripwire only guards the paths a test routes through it, so the ARTIFACT |
| +// download — the read that puts bytes on a box — gets its own pass. Its |
| +// fixture is the half-migrated shape that makes the read observable: pointer |
| +// and manifest qualified (so the pass gets as far as staging), the artifact |
| +// itself only at the legacy key. A fallback reintroduced in ensureStaged |
| +// would find those bytes and stage them; qualified-only cannot, so the host |
| +// is left alone and the pass survives. |
| +// |
| +// Without this, a fallback in ensureStaged passes the whole suite. |
| +func TestAdoptNeverDownloadsALegacyArtifact(t *testing.T) { |
| + dir := NewDirStore(t.TempDir()) |
| + ctx := context.Background() |
| + priv, pub := testKeys(t) |
| + opt, reg := t.TempDir(), testRegistry(t) |
| + |
| + // Built in a scratch store, then placed key by key: the qualified pointer |
| + // and manifest go down as they are, the artifact only in its legacy shape. |
| + scratch := NewDirStore(t.TempDir()) |
| + shipRelease(t, scratch, OpsAccount, "half", "v1", adoptPlatform, []byte("build one")) |
| + if err := Promote(ctx, scratch, priv, OpsAccount, "half", "canary/x", "v1"); err != nil { |
| + t.Fatal(err) |
| + } |
| + art := "" |
| + keys, err := scratch.List(ctx, AppPrefix(OpsAccount, "half")) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + for _, k := range keys { |
| + body, _, err := scratch.Get(ctx, k) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + dst := k |
| + if strings.Contains(k, "/releases/") && !strings.HasSuffix(k, "manifest.json") { |
| + art = k |
| + dst = LegacyAppPrefix("half") + strings.TrimPrefix(k, AppPrefix(OpsAccount, "half")) |
| + } |
| + if _, err := dir.Put(ctx, dst, body, ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + if art == "" { |
| + t.Fatal("fixture: no artifact key found") |
| + } |
| + if _, _, err := dir.Get(ctx, art); !errors.Is(err, os.ErrNotExist) { |
| + t.Fatalf("fixture: the qualified artifact exists (%v) — the test would prove nothing", err) |
| + } |
| + |
| + s := &legacyTripwire{Store: dir, t: t, app: "half"} |
| + addRoute(t, reg, "half.example.com", "half", "canary/x", false) |
| + if err := Pass(ctx, reg, s, pub, opt, func(string) error { return nil }); err != nil { |
| + t.Fatalf("one app's missing artifact must not fail the pass: %v", err) |
| + } |
| + if _, err := os.Stat(VersionPath(opt, OpsAccount, "v1", "half")); err == nil { |
| + t.Error("adopt staged a build whose artifact only exists at the legacy key") |
| + } |
| + if v, _ := LiveVersion(opt, "half.example.com"); v != "" { |
| + t.Errorf("host repointed to %q on a build that was never staged", v) |
| + } |
| +} |
| + |
| +// The other half of the bargain: the migration verb's access to legacy keys |
| +// is EXPLICIT, and outlives the fallback. It is the upgrade path for any |
| +// deployment arriving from a pre-accounts binary, so it works forever. |
| +func TestGetMigrationSourceStillReadsLegacy(t *testing.T) { |
| + s := NewDirStore(t.TempDir()) |
| + ctx := context.Background() |
| + if _, err := s.Put(ctx, LegacyChannelKey("old", "stable"), []byte("legacy"), ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + body, _, usedLegacy, err := GetMigrationSource(ctx, s, |
| + ChannelKey(OpsAccount, "old", "stable"), LegacyChannelKey("old", "stable")) |
| + if err != nil || string(body) != "legacy" || !usedLegacy { |
| + t.Fatalf("migration source: body=%q usedLegacy=%v err=%v", body, usedLegacy, err) |
| + } |
| + // Qualified still wins where both exist — that precedence is what stops |
| + // the verb copying a stale legacy pointer over a live qualified one. |
| + if _, err := s.Put(ctx, ChannelKey(OpsAccount, "old", "stable"), []byte("qualified"), ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + body, _, usedLegacy, err = GetMigrationSource(ctx, s, |
| + ChannelKey(OpsAccount, "old", "stable"), LegacyChannelKey("old", "stable")) |
| + if err != nil || string(body) != "qualified" || usedLegacy { |
| + t.Fatalf("qualified must win: body=%q usedLegacy=%v err=%v", body, usedLegacy, err) |
| + } |
| +} |
| + |
| +// A MIGRATED app waiting on its first promotion to a channel is ordinary, |
| +// and must not be told to migrate. This is kass's live shape on the flagship: |
| +// fully migrated to its own account, with a route following stable whose |
| +// pointer does not exist until the bake window ends — so the hourly adopt |
| +// would otherwise nag about migrating an app that is already migrated, every |
| +// hour, forever. |
| +// |
| +// The distinction is the same one PromoteOnLadder makes, and it is computed |
| +// from the qualified prefix alone: nothing there at all means the deployment |
| +// never migrated; anything there means this channel simply has no pointer yet. |
| +func TestAdoptDoesNotSuggestMigrationForAMigratedApp(t *testing.T) { |
| + s := NewDirStore(t.TempDir()) |
| + ctx := context.Background() |
| + priv, pub := testKeys(t) |
| + opt, reg := t.TempDir(), testRegistry(t) |
| + |
| + // Migrated and live on its own account — just not on stable yet. |
| + shipRelease(t, s, "bad", "kass", "v1", adoptPlatform, []byte("build one")) |
| + if err := Promote(ctx, s, priv, "bad", "kass", "canary/x", "v1"); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := reg.Upsert(registry.Route{Host: "kass.bad.oncarlos.test", Kind: "instance", |
| + App: "kass", Account: "bad", Channel: "stable"}); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + var out bytes.Buffer |
| + withCapturedStdout(t, &out, func() { |
| + if err := Pass(ctx, reg, s, pub, opt, func(string) error { return nil }); err != nil { |
| + t.Errorf("Pass: %v", err) |
| + } |
| + }) |
| + // The PLAIN line exactly — "channel has no release yet" followed by |
| + // nothing. A bare Contains on the prefix would also match the hint line |
| + // ("channel has no release yet (…migrate…)"), making the positive half of |
| + // this test vacuous (2026-08-03 review): only pinning the line ending |
| + // keeps it a real assertion about WHICH branch printed. |
| + if !strings.Contains(out.String(), "channel has no release yet\n") { |
| + t.Errorf("want the plain skip line and nothing appended to it, got:\n%s", out.String()) |
| + } |
| + if strings.Contains(out.String(), "accounts migrate") { |
| + t.Errorf("a migrated app was told to migrate:\n%s", out.String()) |
| + } |
| +} |
| + |
| +// countingStore counts listings of one exact prefix. |
| +type countingStore struct { |
| + Store |
| + prefix string |
| + n int |
| +} |
| + |
| +func (c *countingStore) List(ctx context.Context, prefix string) ([]string, error) { |
| + if prefix == c.prefix { |
| + c.n++ |
| + } |
| + return c.Store.List(ctx, prefix) |
| +} |
| + |
| +// The check that decides the line costs one listing per (account, app) per |
| +// pass, not one per channel. A box following several of an app's channels — |
| +// the ordinary shape for a canary fleet — must not pay for the distinction |
| +// once per route. |
| +func TestAdoptListsAnAppsPrefixOncePerPass(t *testing.T) { |
| + dir := NewDirStore(t.TempDir()) |
| + ctx := context.Background() |
| + priv, pub := testKeys(t) |
| + opt, reg := t.TempDir(), testRegistry(t) |
| + |
| + shipRelease(t, dir, "bad", "kass", "v1", adoptPlatform, []byte("build one")) |
| + if err := Promote(ctx, dir, priv, "bad", "kass", "canary/x", "v1"); err != nil { |
| + t.Fatal(err) |
| + } |
| + // Three routes, three channels with no pointer, one app. |
| + for i, ch := range []string{"stable", "beta", "edge"} { |
| + if err := reg.Upsert(registry.Route{Host: fmt.Sprintf("h%d.example.com", i), |
| + Kind: "instance", App: "kass", Account: "bad", Channel: ch}); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + |
| + s := &countingStore{Store: dir, prefix: AppPrefix("bad", "kass")} |
| + var out bytes.Buffer |
| + withCapturedStdout(t, &out, func() { |
| + if err := Pass(ctx, reg, s, pub, opt, func(string) error { return nil }); err != nil { |
| + t.Errorf("Pass: %v", err) |
| + } |
| + }) |
| + if s.n != 1 { |
| + t.Errorf("listed %s %d times in one pass, want 1", s.prefix, s.n) |
| + } |
| + if strings.Contains(out.String(), "accounts migrate") { |
| + t.Errorf("a migrated app was told to migrate:\n%s", out.String()) |
| + } |
| +} |
| + |
| +// The other side of the same line: an app with NOTHING under its qualified |
| +// prefix is the deployment that upgraded without migrating, and it gets the |
| +// verb by name. Reserving the advice is what makes it worth reading. |
| +func TestAdoptSuggestsMigrationForAnUnmigratedApp(t *testing.T) { |
| + dir := NewDirStore(t.TempDir()) |
| + ctx := context.Background() |
| + priv, pub := testKeys(t) |
| + opt, reg := t.TempDir(), testRegistry(t) |
| + |
| + scratch := NewDirStore(t.TempDir()) |
| + shipRelease(t, scratch, OpsAccount, "old", "v1", adoptPlatform, []byte("build one")) |
| + if err := Promote(ctx, scratch, priv, OpsAccount, "old", "stable", "v1"); err != nil { |
| + t.Fatal(err) |
| + } |
| + keys, err := scratch.List(ctx, AppPrefix(OpsAccount, "old")) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + for _, k := range keys { |
| + body, _, err := scratch.Get(ctx, k) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + legacy := LegacyAppPrefix("old") + strings.TrimPrefix(k, AppPrefix(OpsAccount, "old")) |
| + if _, err := dir.Put(ctx, legacy, body, ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + |
| + s := &legacyTripwire{Store: dir, t: t, app: "old"} |
| + addRoute(t, reg, "old.example.com", "old", "stable", false) |
| + var out bytes.Buffer |
| + withCapturedStdout(t, &out, func() { |
| + if err := Pass(ctx, reg, s, pub, opt, func(string) error { return nil }); err != nil { |
| + t.Errorf("Pass: %v", err) |
| + } |
| + }) |
| + if !strings.Contains(out.String(), "carlos accounts migrate -app old -from legacy -to bab") { |
| + t.Errorf("an unmigrated app was not told how to migrate:\n%s", out.String()) |
| + } |
| +} |
| + |
| +// Promoting an app on a deployment that never migrated must say SO. Left to |
| +// the ladder, the answer is "stable is entered from beta only, and this build |
| +// is on no rung yet" — true (nothing qualified means no rung) and entirely |
| +// beside the point. The operator's actual problem is that this deployment's |
| +// objects are all at the account-less keys. |
| +// |
| +// The test drives it through the tripwire: the distinction is computed from |
| +// the qualified prefix alone, never by peeking at legacy. |
| +func TestPromoteOnLadderNamesTheMigrationForAnUnmigratedApp(t *testing.T) { |
| + dir := NewDirStore(t.TempDir()) |
| + ctx := context.Background() |
| + priv, _ := testKeys(t) |
| + s := &legacyTripwire{Store: dir, t: t, app: "old"} |
| + |
| + // A wholly legacy app, built in a scratch store and copied down. |
| + scratch := NewDirStore(t.TempDir()) |
| + shipRelease(t, scratch, OpsAccount, "old", "v1", adoptPlatform, []byte("build one")) |
| + if err := Promote(ctx, scratch, priv, OpsAccount, "old", "stable", "v1"); err != nil { |
| + t.Fatal(err) |
| + } |
| + keys, err := scratch.List(ctx, AppPrefix(OpsAccount, "old")) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + for _, k := range keys { |
| + body, _, err := scratch.Get(ctx, k) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + legacy := LegacyAppPrefix("old") + strings.TrimPrefix(k, AppPrefix(OpsAccount, "old")) |
| + if _, err := dir.Put(ctx, legacy, body, ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + |
| + _, err = PromoteOnLadder(ctx, s, priv, OpsAccount, "old", "stable", "v1", PromoteOptions{}) |
| + if err == nil { |
| + t.Fatal("promoting an unmigrated app succeeded") |
| + } |
| + if !strings.Contains(err.Error(), "carlos accounts migrate -app old -from legacy -to bab") { |
| + t.Errorf("the refusal does not name the fix:\n%v", err) |
| + } |
| + if strings.Contains(err.Error(), "no rung yet") { |
| + t.Errorf("the ladder answered instead of the migration:\n%v", err) |
| + } |
| +} |
| + |
| +// The control for the test above, and the reason the condition is "nothing |
| +// qualified AT ALL" rather than "no qualified releases": an app that HAS |
| +// shipped under this account is not unmigrated, however new the target |
| +// channel is. It gets the ladder's answer, with no migration advice. |
| +func TestPromoteOnLadderKeepsTheLadderRefusalForAShippedApp(t *testing.T) { |
| + s := NewDirStore(t.TempDir()) |
| + ctx := context.Background() |
| + priv, _ := testKeys(t) |
| + |
| + shipRelease(t, s, OpsAccount, "fresh", "v1", adoptPlatform, []byte("build one")) |
| + _, err := PromoteOnLadder(ctx, s, priv, OpsAccount, "fresh", "stable", "v1", PromoteOptions{}) |
| + if err == nil { |
| + t.Fatal("a fresh build entered stable directly") |
| + } |
| + if !strings.Contains(err.Error(), "no rung yet") { |
| + t.Errorf("want the ladder's refusal, got:\n%v", err) |
| + } |
| + if strings.Contains(err.Error(), "accounts migrate") { |
| + t.Errorf("a shipped app was told to migrate:\n%v", err) |
| + } |
| +} |
| + |
| +// A store error that is not "not found" is never retried against legacy: |
| +// masking a broken bucket as a fallback miss would make it look like an |
| +// unmigrated app, and the migration would then "verify" an app it never read. |
| +func TestGetMigrationSourcePropagatesRealErrors(t *testing.T) { |
| + s := &erroringStore{err: errors.New("bucket on fire")} |
| + _, _, _, err := GetMigrationSource(context.Background(), s, "a", "b") |
| + if err == nil || !strings.Contains(err.Error(), "on fire") { |
| + t.Fatalf("want the real error, got %v", err) |
| + } |
| +} |
| + |
| +// Promote against an app whose pointer only exists at the legacy key does not |
| +// see it: the new pointer is a FIRST promote at the qualified key, and the |
| +// legacy history does not come forward. That is the retirement's real cost, |
| +// stated — carrying history across is what `accounts migrate` is for, and it |
| +// is why a miss names that verb. The legacy object is still never deleted. |
| +func TestPromoteDoesNotInheritALegacyPointer(t *testing.T) { |
| + s := NewDirStore(t.TempDir()) |
| + ctx := context.Background() |
| + priv, pub := testKeys(t) |
| + |
| + shipRelease(t, s, "kqmv", "myapp", "v1", "linux-amd64", []byte("v1")) |
| + shipRelease(t, s, "kqmv", "myapp", "v2", "linux-amd64", []byte("v2")) |
| + legacy, err := signPointer(priv, &ChannelPointer{App: "myapp", Channel: "stable", Version: "v1"}) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := s.Put(ctx, LegacyChannelKey("myapp", "stable"), legacy, ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + if err := Promote(ctx, s, priv, "kqmv", "myapp", "stable", "v2"); err != nil { |
| + t.Fatalf("Promote: %v", err) |
| + } |
| + ptr, err := Channel(ctx, s, pub, "kqmv", "myapp", "stable") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if ptr.Version != "v2" { |
| + t.Fatalf("pointer = %s, want v2", ptr.Version) |
| + } |
| + if len(ptr.History) != 0 { |
| + t.Errorf("history = %v, want empty — a legacy pointer must not be inherited", ptr.History) |
| + } |
| + if _, _, err := s.Get(ctx, LegacyChannelKey("myapp", "stable")); err != nil { |
| + t.Errorf("promote deleted the legacy pointer: %v", err) |
| + } |
| +} |
diff --git a/internal/release/release.go b/internal/release/release.go| index a621bdb8b..9964b9cea 100644 |
| --- a/internal/release/release.go |
| +++ b/internal/release/release.go |
| @@ -144,9 +144,14 @@ func AppPrefix(acct, app string) string { return appPrefix(acct, app) } |
| func appPrefix(acct, app string) string { return "apps/" + acct + "/" + app + "/" } |
| -// The legacy, account-less key shapes. They exist for exactly two callers: |
| -// the read-fallback (fallback.go) and the migration verb, which copies from |
| -// here to there. Nothing writes them any more. |
| +// The legacy, account-less key shapes. Since the read-fallback was retired |
| +// (qualified.go) they exist for the migration verb, which copies from here to |
| +// there, and for tests building a pre-accounts bucket. Nothing writes them, |
| +// and no runtime read resolves through them. |
| +// |
| +// The two LIST prefixes that used to live here (channels/, releases/) are |
| +// gone with the fallback that was their only caller. A dead builder naming a |
| +// shape nothing may resolve is an invitation to resolve it. |
| func LegacyManifestKey(app, version string) string { |
| return legacyAppPrefix(app) + "releases/" + version + "/manifest.json" |
| } |
| @@ -156,10 +161,9 @@ func LegacyArtifactKey(app, version, file string) string { |
| func LegacyChannelKey(app, channel string) string { |
| return legacyAppPrefix(app) + "channels/" + channel + ".json" |
| } |
| -func LegacyChannelsPrefix(app string) string { return legacyAppPrefix(app) + "channels/" } |
| -func LegacyReleasesPrefix(app string) string { return legacyAppPrefix(app) + "releases/" } |
| -// LegacyAppPrefix is everything an unmigrated app owns in the bucket. |
| +// LegacyAppPrefix is everything an unmigrated app owns in the bucket — what |
| +// `accounts migrate -from legacy` lists and copies. |
| func LegacyAppPrefix(app string) string { return legacyAppPrefix(app) } |
| func legacyAppPrefix(app string) string { return "apps/" + app + "/" } |
| @@ -438,12 +442,11 @@ func putImmutable(ctx context.Context, s Store, key string, body []byte) error { |
| } |
| // GetManifest fetches and validates a release manifest, requiring it to name |
| -// the app/version its key claims. Reads the account-qualified key, falling |
| -// back to the legacy unqualified one for an app that hasn't been migrated |
| -// yet. Not-found is os.ErrNotExist-wrapped. |
| +// the app/version its key claims. Reads the account-qualified key and only |
| +// that one. Not-found is os.ErrNotExist-wrapped, and names `accounts migrate |
| +// -from legacy` for a deployment whose objects never moved. |
| func GetManifest(ctx context.Context, s Store, acct, app, version string) (*Manifest, error) { |
| - raw, _, _, err := GetWithFallback(ctx, s, |
| - ManifestKey(acct, app, version), LegacyManifestKey(app, version)) |
| + raw, _, err := GetQualified(ctx, s, ManifestKey(acct, app, version)) |
| if err != nil { |
| return nil, err |
| } |
| @@ -491,7 +494,18 @@ func Promote(ctx context.Context, s Store, priv ed25519.PrivateKey, acct, app, c |
| } |
| pub, _ := priv.Public().(ed25519.PublicKey) |
| key := ChannelKey(acct, app, channel) |
| - cur, etag, usedLegacy, err := GetWithFallback(ctx, s, key, LegacyChannelKey(app, channel)) |
| + // The qualified key and nothing else, so the etag CAS holds against the |
| + // object actually being written. An app still on legacy keys reads as a |
| + // FIRST promote here and its history does not come forward — carrying |
| + // history across the two key spaces is `accounts migrate`'s job, not a |
| + // side effect of the next deploy. |
| + // |
| + // GetQualified for uniform plumbing, though the hint it attaches is |
| + // DISCARDED below: an absent pointer is the legitimate first promotion of |
| + // a fresh channel, and an app that has never been promoted here is not |
| + // thereby unmigrated. PromoteOnLadder makes that distinction, where the |
| + // whole app's prefix is in view. |
| + cur, etag, err := GetQualified(ctx, s, key) |
| var old *ChannelPointer |
| switch { |
| case err == nil: |
| @@ -499,14 +513,6 @@ func Promote(ctx context.Context, s Store, priv ed25519.PrivateKey, acct, app, c |
| if err != nil { |
| return fmt.Errorf("promote: existing pointer at %s: %w", key, err) |
| } |
| - // A legacy read's etag belongs to a DIFFERENT object, so the qualified |
| - // Put must be create-only. The history still comes forward: promoting |
| - // an unmigrated app moves it to the qualified key without losing its |
| - // rollback targets, and leaves the legacy object untouched (shadowed, |
| - // not deleted — deletion is the operator's gated step). |
| - if usedLegacy { |
| - etag = "" |
| - } |
| case errors.Is(err, os.ErrNotExist): |
| old, etag = &ChannelPointer{}, "" // first promote: create-only Put |
| default: |
| @@ -593,10 +599,7 @@ func PromoteOnLadder(ctx context.Context, s Store, priv ed25519.PrivateKey, acct |
| pub, _ := priv.Public().(ed25519.PublicKey) |
| // A repeat is a no-op, checked before anything else: re-running a command |
| - // must not fail, whichever rung the build has since climbed to. Only the |
| - // ACCOUNT-QUALIFIED pointer counts — an app still on a legacy key has not |
| - // been written to its qualified key yet, and short-circuiting here would |
| - // leave it there. |
| + // must not fail, whichever rung the build has since climbed to. |
| if cur, _, err := s.Get(ctx, ChannelKey(acct, app, channel)); err == nil { |
| if p, verr := verifyPointer(pub, cur, app, channel); verr == nil && p.Version == version { |
| res.NoOp = true |
| @@ -607,6 +610,24 @@ func PromoteOnLadder(ctx context.Context, s Store, priv ed25519.PrivateKey, acct |
| return res, fmt.Errorf("promote: read %s: %w", ChannelKey(acct, app, channel), err) |
| } |
| + // An app with NOTHING under its account-qualified prefix cannot be |
| + // promoted — Promote needs a shipped manifest there — so refuse now, while |
| + // the reason is still nameable. Left to run on, the ladder answers first |
| + // with "this build is on no rung yet", which is true (nothing qualified |
| + // means no rung) and entirely beside the point on a deployment that |
| + // upgraded from a pre-accounts binary and never migrated. Promote's own |
| + // read cannot say it either: an absent pointer is the legitimate first |
| + // promotion of a fresh channel, so the miss has to stay silent there. |
| + // |
| + // AppHasQualifiedObjects is the same test adopt applies to decide whether |
| + // a missing pointer deserves the advice — one rule, two callers. |
| + if has, err := AppHasQualifiedObjects(ctx, s, acct, app); err != nil { |
| + return res, fmt.Errorf("promote: list %s: %w", AppPrefix(acct, app), err) |
| + } else if !has { |
| + return res, fmt.Errorf("promote: nothing has ever been shipped to %s under account %s%s", |
| + app, acct, migrationHint(AppPrefix(acct, app))) |
| + } |
| + |
| from, err := VersionRung(ctx, s, pub, acct, app, version) |
| if err != nil { |
| return res, fmt.Errorf("promote: %w", err) |
| @@ -645,7 +666,8 @@ func VersionRung(ctx context.Context, s Store, pub ed25519.PublicKey, acct, app, |
| if len(pub) != ed25519.PublicKeySize { |
| return "", fmt.Errorf("version rung: no usable release public key") |
| } |
| - keys, prefix, _, err := ListWithFallback(ctx, s, ChannelsPrefix(acct, app), LegacyChannelsPrefix(app)) |
| + prefix := ChannelsPrefix(acct, app) |
| + keys, err := s.List(ctx, prefix) |
| if err != nil { |
| return "", fmt.Errorf("list channels of %s: %w", app, err) |
| } |
| @@ -667,10 +689,11 @@ func VersionRung(ctx context.Context, s Store, pub ed25519.PublicKey, acct, app, |
| // Channel resolves a channel's current pointer, verifying its signature — |
| // the one read path the fleet uses, so nothing unsigned ever reaches a box. |
| -// Not-found is os.ErrNotExist-wrapped. |
| +// Account-qualified only: an app whose pointer is still at the legacy key |
| +// does not resolve here, it resolves after `accounts migrate`. Not-found is |
| +// os.ErrNotExist-wrapped and says exactly that. |
| func Channel(ctx context.Context, s Store, pub ed25519.PublicKey, acct, app, channel string) (*ChannelPointer, error) { |
| - raw, _, _, err := GetWithFallback(ctx, s, |
| - ChannelKey(acct, app, channel), LegacyChannelKey(app, channel)) |
| + raw, _, err := GetQualified(ctx, s, ChannelKey(acct, app, channel)) |
| if err != nil { |
| return nil, err |
| } |
| @@ -692,7 +715,7 @@ func Rollback(ctx context.Context, s Store, priv ed25519.PrivateKey, acct, app, |
| } |
| pub, _ := priv.Public().(ed25519.PublicKey) |
| key := ChannelKey(acct, app, channel) |
| - cur, etag, usedLegacy, err := GetWithFallback(ctx, s, key, LegacyChannelKey(app, channel)) |
| + cur, etag, err := GetQualified(ctx, s, key) |
| if err != nil { |
| return fmt.Errorf("rollback: %w", err) |
| } |
| @@ -700,9 +723,6 @@ func Rollback(ctx context.Context, s Store, priv ed25519.PrivateKey, acct, app, |
| if err != nil { |
| return fmt.Errorf("rollback: existing pointer at %s: %w", key, err) |
| } |
| - if usedLegacy { |
| - etag = "" // the qualified key doesn't exist yet: create-only (see Promote) |
| - } |
| if len(old.History) == 0 { |
| return fmt.Errorf("rollback: %s/%s has no previous version recorded", app, channel) |
| } |
diff --git a/internal/vet/vet.go b/internal/vet/vet.go| index 50b635fc5..5a929f2e8 100644 |
| --- a/internal/vet/vet.go |
| +++ b/internal/vet/vet.go |
| @@ -64,9 +64,7 @@ func Check(ctx context.Context, s release.Store, pub ed25519.PublicKey, acct, ap |
| platform := runtime.GOOS + "-" + runtime.GOARCH |
| for _, k := range keys { |
| a := m.Artifacts[k] |
| - b, _, _, err := release.GetWithFallback(ctx, s, |
| - release.ArtifactKey(acct, app, version, a.File), |
| - release.LegacyArtifactKey(app, version, a.File)) |
| + b, _, err := release.GetQualified(ctx, s, release.ArtifactKey(acct, app, version, a.File)) |
| if err != nil { |
| problems = append(problems, fmt.Sprintf("artifact %s (%s): %v", k, a.File, err)) |
| continue |