amadan
release: an artifacts-only version is not a release for the first-ship rule (rootfs Plan 1 Task 3, controller ruling on 2.3c)
appKind now counts only manifests that exist but do not decode or validate. A version with no manifest object at all (a ship that died before publishing; the manifest is written last) no longer blocks the first ship, which writes the marker and publishes, so one crashed first ship cannot leave an app that only someone with bucket access can unblock. A present but unreadable manifest still refuses (fail closed).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 files changed,
+90
−37
internal/release/rootfs_ship.go+28 −20internal/release/rootfs_ship_test.go+62 −17
diff --git a/internal/release/rootfs_ship.go b/internal/release/rootfs_ship.go| index 5b8c819a0..15712f4d1 100644 |
| --- a/internal/release/rootfs_ship.go |
| +++ b/internal/release/rootfs_ship.go |
| @@ -46,38 +46,45 @@ func AppKind(ctx context.Context, s Store, acct, app string) (AppKindInfo, error |
| return info, err |
| } |
| -// appKind is AppKind that also counts the versions VersionCandidates listed, |
| -// so the first-ship rule can tell an empty app from one whose releases are |
| -// all unreadable (preflight 2.3c). |
| +// appKind is AppKind that also counts the manifests it skipped because they |
| +// exist but do not decode or validate, so the first-ship rule can tell an |
| +// app with no release from one whose releases cannot be read (preflight |
| +// 2.3c, as refined by the controller). A version with no manifest object at |
| +// all (artifacts only: a ship that died before publishing, since the manifest |
| +// is written last) is not a release and is not counted. |
| func appKind(ctx context.Context, s Store, acct, app string) (AppKindInfo, int, error) { |
| vers, _, err := VersionCandidates(ctx, s, acct, app, 20) |
| if err != nil { |
| return AppKindInfo{}, 0, err |
| } |
| + unreadable := 0 |
| for _, v := range vers { |
| raw, _, err := GetQualified(ctx, s, ManifestKey(acct, app, v)) |
| if errors.Is(err, os.ErrNotExist) { |
| continue |
| } |
| if err != nil { |
| - return AppKindInfo{}, len(vers), err |
| + return AppKindInfo{}, unreadable, err |
| } |
| var m Manifest |
| if err := json.Unmarshal(raw, &m); err != nil { |
| log.Printf("app kind: manifest %s/%s: decode error: %v", app, v, err) |
| + unreadable++ |
| continue |
| } |
| if err := validateManifest(&m); err != nil { |
| log.Printf("app kind: manifest %s/%s: validation error: %v", app, v, err) |
| + unreadable++ |
| continue |
| } |
| if m.App != app || m.Version != v { |
| log.Printf("app kind: manifest %s/%s: declares wrong app/version", app, v) |
| + unreadable++ |
| continue |
| } |
| - return AppKindInfo{Kind: m.Kind, Version: m.Version, Hibernate: m.Hibernate}, len(vers), nil |
| + return AppKindInfo{Kind: m.Kind, Version: m.Version, Hibernate: m.Hibernate}, unreadable, nil |
| } |
| - return AppKindInfo{}, len(vers), nil |
| + return AppKindInfo{}, unreadable, nil |
| } |
| // EstablishedKind is AppKind, and when the app has no readable release, the |
| @@ -157,11 +164,11 @@ type AppKindMarker struct { |
| // AppKindMarkerKey is where an app's kind marker lives in the bucket. |
| func AppKindMarkerKey(acct, app string) string { return appPrefix(acct, app) + "kind.json" } |
| -// errAppKindUnreadable refuses a first ship to an app that lists versions |
| -// but has no readable manifest and no marker: its kind is unknown, so no |
| -// ship may fix it (preflight 2.3c). The console answers it as any other |
| +// errAppKindUnreadable refuses a first ship to an app that has manifests but |
| +// none readable, and no marker: its kind is unknown, so no ship may fix it |
| +// (preflight 2.3c). The console answers it as any other |
| // failure to check the app's releases. |
| -var errAppKindUnreadable = errors.New("the app lists versions but none has a readable manifest, so its kind is unknown") |
| +var errAppKindUnreadable = errors.New("the app has release manifests but none is readable, so its kind is unknown") |
| func validMarkerKind(kind string) bool { |
| return kind == "binary" || kind == "static" || kind == KindRootfs |
| @@ -213,20 +220,21 @@ func kindRefusal(m *Manifest, have, haveVersion string, haveHibernate, first boo |
| } |
| // shipKindState is what a kind check reads, once: the app's newest readable |
| -// release, how many versions were listed, and the marker when the release |
| +// release, how many manifests exist but could not be read, and the marker |
| +// when the release |
| // does not decide. |
| type shipKindState struct { |
| - info AppKindInfo |
| - listed int |
| - marker *AppKindMarker |
| + info AppKindInfo |
| + unreadable int |
| + marker *AppKindMarker |
| } |
| func readShipKindState(ctx context.Context, s Store, acct string, m *Manifest) (shipKindState, error) { |
| - info, listed, err := appKind(ctx, s, acct, m.App) |
| + info, unreadable, err := appKind(ctx, s, acct, m.App) |
| if err != nil { |
| return shipKindState{}, err |
| } |
| - st := shipKindState{info: info, listed: listed} |
| + st := shipKindState{info: info, unreadable: unreadable} |
| if info.Kind != "" && info.Version != m.Version { |
| return st, nil |
| } |
| @@ -235,11 +243,11 @@ func readShipKindState(ctx context.Context, s Store, acct string, m *Manifest) ( |
| } |
| // decide answers the refusal for m, and whether m is the app's first ship |
| -// (no release, no marker, nothing listed), which must write the marker. |
| +// (no release, no marker, no unreadable manifest), which must write the marker. |
| // Another release decides; else the marker does, with no version skip (a |
| // re-ship of the marker's version as the other kind is still the other |
| -// kind); else a readable re-ship of m's own version passes; else an app that |
| -// lists anything is refused (preflight 2.3c). |
| +// kind); else a readable re-ship of m's own version passes; else an app with |
| +// a manifest that exists but cannot be read is refused (preflight 2.3c). |
| func (st shipKindState) decide(m *Manifest) (first bool, err error) { |
| switch { |
| case st.info.Kind != "" && st.info.Version != m.Version: |
| @@ -248,7 +256,7 @@ func (st shipKindState) decide(m *Manifest) (first bool, err error) { |
| return false, kindRefusal(m, st.marker.Kind, st.marker.Version, st.marker.Hibernate, true) |
| case st.info.Kind != "": |
| return false, nil |
| - case st.listed > 0: |
| + case st.unreadable > 0: |
| return false, errAppKindUnreadable |
| } |
| return true, nil |
diff --git a/internal/release/rootfs_ship_test.go b/internal/release/rootfs_ship_test.go| index ac8d5f1d5..92ff522ab 100644 |
| --- a/internal/release/rootfs_ship_test.go |
| +++ b/internal/release/rootfs_ship_test.go |
| @@ -223,29 +223,74 @@ func TestCheckShipKindSkipsAnUnreadableManifestButFailsClosedOnTheStore(t *testi |
| } |
| } |
| -// Preflight 2.3c: an app that lists versions but has no readable manifest |
| -// and no marker is refused, never given a marker of the incoming kind. |
| -func TestFirstShipRefusesAnAppWhoseReleasesAreAllUnreadable(t *testing.T) { |
| +// Preflight 2.3c, as the controller refined it: an app with a manifest that |
| +// exists but cannot be read or does not validate, and no marker, is refused, |
| +// never given a marker of the incoming kind. |
| +func TestFirstShipRefusesAnAppWhoseManifestsAreUnreadable(t *testing.T) { |
| + ctx := context.Background() |
| + for name, body := range map[string]string{ |
| + "not json": "{not json", |
| + "invalid": `{"schema":1,"app":"shop","version":"0.1.0-1","kind":"nonsense","artifacts":{}}`, |
| + "another app": `{"schema":1,"app":"blog","version":"0.1.0-1","kind":"binary","artifacts":{"linux-arm64":{"file":"linux-arm64","sha256":"` + strings.Repeat("a", 64) + `","size":1}}}`, |
| + } { |
| + s := newShipCountingStore(t) |
| + if _, err := s.DirStore.Put(ctx, ManifestKey("bab", "shop", "0.1.0-1"), []byte(body), ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + // An artifacts-only version beside it does not make the app readable. |
| + if _, err := s.DirStore.Put(ctx, ArtifactKey("bab", "shop", "0.1.0-0", adoptPlatform), []byte("bin"), ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + m, paths := spooledRootfs(t, "0.1.0-2", false, "image") |
| + for call, err := range map[string]error{ |
| + "CheckShipKind": CheckShipKind(ctx, s, "bab", &m), |
| + "establishShipKind": establishShipKind(ctx, s, "bab", &m), |
| + "ShipSpooled": ShipSpooled(ctx, s, "bab", m, paths), |
| + "Ship": Ship(ctx, s, "bab", binaryManifest("0.1.0-3"), binaryFiles()), |
| + } { |
| + if !errors.Is(err, errAppKindUnreadable) { |
| + t.Errorf("%s: %s: err = %v, want errAppKindUnreadable", name, call, err) |
| + } |
| + } |
| + if len(s.puts)+len(s.putFroms) != 0 { |
| + t.Errorf("%s: writes after the refusal: puts %v, putFroms %v", name, s.puts, s.putFroms) |
| + } |
| + } |
| +} |
| + |
| +// A version with no manifest object at all (a ship that died before it |
| +// published; the manifest is written last) is not a release: the next first |
| +// ship writes the marker and publishes, of either kind, and the crashed |
| +// version itself can be shipped again. |
| +func TestFirstShipOverAnArtifactsOnlyVersionWritesTheMarker(t *testing.T) { |
| ctx := context.Background() |
| s := newShipCountingStore(t) |
| - if _, err := s.DirStore.Put(ctx, ManifestKey("bab", "shop", "0.1.0-1"), []byte("{not json"), ""); err != nil { |
| + if _, err := s.DirStore.Put(ctx, ArtifactKey("bab", "shop", "0.1.0-1", adoptPlatform), []byte("bin"), ""); err != nil { |
| t.Fatal(err) |
| } |
| - m, paths := spooledRootfs(t, "0.1.0-2", false, "image") |
| - for name, err := range map[string]error{ |
| - "CheckShipKind": CheckShipKind(ctx, s, "bab", &m), |
| - "establishShipKind": establishShipKind(ctx, s, "bab", &m), |
| - "ShipSpooled": ShipSpooled(ctx, s, "bab", m, paths), |
| - } { |
| - if !errors.Is(err, errAppKindUnreadable) { |
| - t.Errorf("%s: err = %v, want errAppKindUnreadable", name, err) |
| - } |
| + m, paths := spooledRootfs(t, "0.1.0-2", true, "image") |
| + if err := CheckShipKind(ctx, s, "bab", &m); err != nil { |
| + t.Fatalf("CheckShipKind: %v", err) |
| + } |
| + if err := ShipSpooled(ctx, s, "bab", m, paths); err != nil { |
| + t.Fatalf("ShipSpooled over an artifacts-only version: %v", err) |
| } |
| - if p, _ := s.counts(AppKindMarkerKey("bab", "shop")); p != 0 { |
| - t.Errorf("the marker was put %d times", p) |
| + if mk := readMarker(t, s, "bab", "shop"); mk == nil || mk.Kind != KindRootfs || mk.Version != "0.1.0-2" || !mk.Hibernate { |
| + t.Fatalf("marker = %+v", mk) |
| + } |
| + if _, err := GetManifest(ctx, s, "bab", "shop", "0.1.0-2"); err != nil { |
| + t.Fatalf("not published: %v", err) |
| + } |
| + // The crashed version, re-shipped with its own bytes, after the marker. |
| + r := newShipCountingStore(t) |
| + if _, err := r.DirStore.Put(ctx, ArtifactKey("bab", "shop", "0.1.0-1", adoptPlatform), []byte("bin"), ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := Ship(ctx, r, "bab", binaryManifest("0.1.0-1"), binaryFiles()); err != nil { |
| + t.Fatalf("re-ship of the crashed version: %v", err) |
| } |
| - if p, f := s.counts(ArtifactKey("bab", "shop", "0.1.0-2", RootfsArtifactFile)); p+f != 0 { |
| - t.Errorf("the image was written") |
| + if mk := readMarker(t, r, "bab", "shop"); mk == nil || mk.Kind != "binary" || mk.Version != "0.1.0-1" { |
| + t.Fatalf("marker = %+v", mk) |
| } |
| } |