amadan
release: retire the legacy read-fallback — qualified keys only (spec §2/§5)
Every app with a route on either production deployment now has account-qualified bucket keys, so the fallback's job is done. Every 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. GetWithFallback/ListWithFallback are gone. GetQualified is the one read helper the runtime uses, so the fallback has exactly one place it could grow back, and a miss on an app key names `carlos accounts migrate -from legacy` in one sentence written once. The hint is unconditional rather than probed: "the runtime never reads a legacy key" is worth having outright rather than with an error-path exception, and the wording carries the condition instead. GetMigrationSource keeps the old shape for the one caller whose access to the account-less space is explicit and permanent. Promote and Rollback now hold a real etag for the object they write (no more create-only dance), and an app still on legacy keys reads as a first promote — carrying history across the two key spaces is the migration verb's job, not a side effect of the next deploy. Adopt SKIPS such an app, printing the store's error so the hint reaches the operator: one app's missing pointer must never stop the rest of the box from installing.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
6 files changed,
+452
−321
internal/release/adopt.go+9 −3internal/release/fallback.go+0 −99internal/release/fallback_test.go+0 −190internal/release/qualified.go+143 −0internal/release/qualified_test.go+278 −0internal/release/release.go+22 −29
diff --git a/internal/release/adopt.go b/internal/release/adopt.go| index 5f6ef1228..0473c7e96 100644 |
| --- a/internal/release/adopt.go |
| +++ b/internal/release/adopt.go |
| @@ -125,7 +125,14 @@ 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. Since the |
| + // read-fallback was retired an app still on legacy keys lands |
| + // here too, so the store's error is printed rather than |
| + // swallowed — it names `accounts migrate -from legacy`, which |
| + // is the difference between "nothing promoted yet" and "this |
| + // deployment never migrated". |
| + 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 +329,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..0e16d5e8d |
| --- /dev/null |
| +++ b/internal/release/qualified.go |
| @@ -0,0 +1,143 @@ |
| +// 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 — the ONE read helper the |
| +// runtime uses, so the retired fallback has exactly one place it could ever |
| +// grow back. Not-found stays os.ErrNotExist-wrapped (every caller's "has this |
| +// app ever shipped here" check keeps working) and gains the migration hint |
| +// below. |
| +// |
| +// 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) |
| +} |
| + |
| +// 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..e1169f592 |
| --- /dev/null |
| +++ b/internal/release/qualified_test.go |
| @@ -0,0 +1,278 @@ |
| +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 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) |
| + } |
| +} |
| + |
| +// 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 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..21e256b2a 100644 |
| --- a/internal/release/release.go |
| +++ b/internal/release/release.go |
| @@ -144,9 +144,10 @@ 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 exactly one caller: the migration verb, which |
| +// copies from here to there. Nothing writes them, and no runtime read |
| +// resolves through them. |
| func LegacyManifestKey(app, version string) string { |
| return legacyAppPrefix(app) + "releases/" + version + "/manifest.json" |
| } |
| @@ -438,12 +439,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 +491,12 @@ 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. |
| + cur, etag, err := s.Get(ctx, key) |
| var old *ChannelPointer |
| switch { |
| case err == nil: |
| @@ -499,14 +504,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 +590,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 |
| @@ -645,7 +639,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 +662,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 +688,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 +696,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) |
| } |