zdd-4 task 2: staged manifest siblings keep the canonical bytes
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
4 files changed,
+92
−36
internal/release/adopt.go+7 −17internal/release/release.go+13 −5internal/release/staged_raw_test.go+58 −0internal/release/versions.go+14 −14
diff --git a/internal/release/adopt.go b/internal/release/adopt.go| index 82dc9f422..5fade535d 100644 |
| --- a/internal/release/adopt.go |
| +++ b/internal/release/adopt.go |
| @@ -7,7 +7,6 @@ package release |
| import ( |
| "context" |
| "crypto/ed25519" |
| - "encoding/json" |
| "errors" |
| "fmt" |
| "io" |
| @@ -1642,7 +1641,7 @@ func ensureStaged(ctx context.Context, s Store, optDir, acct, app, ver, host str |
| // already-staged version's manifest is re-fetched — kept cheap by |
| // only ever taking it when the sibling itself is absent. |
| } |
| - m, err := GetManifest(ctx, s, acct, app, ver) |
| + m, raw, err := GetManifestRaw(ctx, s, acct, app, ver) |
| if err != nil { |
| if staged { |
| // The binary IS staged — this function's whole promise — so a |
| @@ -1672,16 +1671,12 @@ func ensureStaged(ctx context.Context, s Store, optDir, acct, app, ver, host str |
| return nil, err |
| } |
| } |
| - // The sibling write is best-effort: a failure here is not fatal to |
| - // staging the binary — log and continue. A binary that runs with an |
| - // unreadable capability sibling is the leave-alone state, which is safe |
| - // (ReadStagedManifest's nil-nil-on-absence contract), and |
| + // The sibling is the CANONICAL bytes (spec §4.2): what the bucket holds, |
| + // unknown fields and all, never a re-marshal by whichever binary staged |
| + // it. The write is best-effort: a failure here is not fatal to staging the |
| + // binary. A binary with an unreadable sibling is the leave-alone state, |
| + // which is safe (ReadStagedManifest's nil-nil-on-absence contract), and |
| // backfillManifest gets another chance at it on every later pass. |
| - raw, err := json.MarshalIndent(m, "", " ") |
| - if err != nil { |
| - fmt.Printf(" %s: marshal staged manifest for %s: %v\n", host, ver, err) |
| - return m, nil |
| - } |
| if err := StageManifest(optDir, acct, ver, app, raw); err != nil { |
| fmt.Printf(" %s: stage manifest sibling for %s: %v\n", host, ver, err) |
| } |
| @@ -1705,16 +1700,11 @@ func backfillManifest(ctx context.Context, s Store, optDir, acct, app, ver, host |
| if _, err := os.Stat(StagedManifestPath(optDir, acct, ver, app)); err == nil { |
| return |
| } |
| - m, err := GetManifest(ctx, s, acct, app, ver) |
| + _, raw, err := GetManifestRaw(ctx, s, acct, app, ver) |
| if err != nil { |
| fmt.Printf(" %s: back-fill manifest for %s: %v\n", host, ver, err) |
| return |
| } |
| - raw, err := json.MarshalIndent(m, "", " ") |
| - if err != nil { |
| - fmt.Printf(" %s: marshal staged manifest for %s: %v\n", host, ver, err) |
| - return |
| - } |
| if err := StageManifest(optDir, acct, ver, app, raw); err != nil { |
| fmt.Printf(" %s: stage manifest sibling for %s: %v\n", host, ver, err) |
| } |
diff --git a/internal/release/release.go b/internal/release/release.go| index 6d7182466..7ac46582a 100644 |
| --- a/internal/release/release.go |
| +++ b/internal/release/release.go |
| @@ -682,21 +682,29 @@ func putImmutable(ctx context.Context, s Store, key string, body []byte) error { |
| // 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) { |
| + m, _, err := GetManifestRaw(ctx, s, acct, app, version) |
| + return m, err |
| +} |
| + |
| +// GetManifestRaw is GetManifest that also returns the exact bytes it read, so |
| +// a box can keep them (the staged sibling) rather than a re-marshal that |
| +// drops every field this binary does not know (spec §4.2). |
| +func GetManifestRaw(ctx context.Context, s Store, acct, app, version string) (*Manifest, []byte, error) { |
| raw, _, err := GetQualified(ctx, s, ManifestKey(acct, app, version)) |
| if err != nil { |
| - return nil, err |
| + return nil, nil, err |
| } |
| var m Manifest |
| if err := json.Unmarshal(raw, &m); err != nil { |
| - return nil, fmt.Errorf("manifest %s/%s: %v", app, version, err) |
| + return nil, nil, fmt.Errorf("manifest %s/%s: %v", app, version, err) |
| } |
| if err := validateManifest(&m); err != nil { |
| - return nil, fmt.Errorf("manifest %s/%s: %w", app, version, err) |
| + return nil, nil, fmt.Errorf("manifest %s/%s: %w", app, version, err) |
| } |
| if m.App != app || m.Version != version { |
| - return nil, fmt.Errorf("manifest at %s names %s/%s", ManifestKey(acct, app, version), m.App, m.Version) |
| + return nil, nil, fmt.Errorf("manifest at %s names %s/%s", ManifestKey(acct, app, version), m.App, m.Version) |
| } |
| - return &m, nil |
| + return &m, raw, nil |
| } |
| // Promote points a channel at a shipped version with the deployment's root |
diff --git a/internal/release/staged_raw_test.go b/internal/release/staged_raw_test.go| new file mode 100644 |
| index 000000000..66fe4a6ce |
| --- /dev/null |
| +++ b/internal/release/staged_raw_test.go |
| @@ -0,0 +1,58 @@ |
| +package release |
| + |
| +import ( |
| + "bytes" |
| + "context" |
| + "os" |
| + "testing" |
| +) |
| + |
| +func TestGetManifestRawReturnsTheBytesItParsed(t *testing.T) { |
| + s := NewDirStore(t.TempDir()) |
| + raw := putRawRelease(t, s, "bab", "km", "v1", []byte("bin"), `"swap":"hold","from_a_newer_shipper":{"x":1},`) |
| + m, got, err := GetManifestRaw(context.Background(), s, "bab", "km", "v1") |
| + if err != nil { |
| + t.Fatalf("GetManifestRaw: %v", err) |
| + } |
| + if !bytes.Equal(got, raw) || m.Swap != DemandHold { |
| + t.Fatalf("GetManifestRaw = %+v, %s; want the stored bytes and swap hold", m, got) |
| + } |
| +} |
| + |
| +// Spec §4.2: copies staged from now on are lossless. A field this binary does |
| +// not know survives in the sibling. |
| +func TestStagedSiblingKeepsTheCanonicalBytes(t *testing.T) { |
| + s := NewDirStore(t.TempDir()) |
| + ctx := context.Background() |
| + opt := t.TempDir() |
| + raw := putRawRelease(t, s, "bab", "km", "v1", []byte("bin"), `"swap":"hold","from_a_newer_shipper":{"x":1},`) |
| + if _, err := ensureStaged(ctx, s, opt, "bab", "km", "v1", "h.example.com"); err != nil { |
| + t.Fatalf("ensureStaged: %v", err) |
| + } |
| + got, err := os.ReadFile(StagedManifestPath(opt, "bab", "v1", "km")) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if !bytes.Equal(got, raw) { |
| + t.Fatalf("staged sibling is not the canonical bytes:\n got %s\nwant %s", got, raw) |
| + } |
| + m, err := ReadStagedManifest(opt, "bab", "v1", "km") |
| + if err != nil || m == nil || m.Swap != DemandHold { |
| + t.Fatalf("ReadStagedManifest = %+v, %v; want the canonical manifest back", m, err) |
| + } |
| +} |
| + |
| +func TestBackfilledSiblingKeepsTheCanonicalBytes(t *testing.T) { |
| + s := NewDirStore(t.TempDir()) |
| + ctx := context.Background() |
| + opt := t.TempDir() |
| + raw := putRawRelease(t, s, "bab", "km", "v1", []byte("bin"), `"from_a_newer_shipper":true,`) |
| + if err := StageArtifact(opt, "bab", "v1", "km", []byte("bin")); err != nil { |
| + t.Fatal(err) |
| + } |
| + backfillManifest(ctx, s, opt, "bab", "km", "v1", "h.example.com") |
| + got, err := os.ReadFile(StagedManifestPath(opt, "bab", "v1", "km")) |
| + if err != nil || !bytes.Equal(got, raw) { |
| + t.Fatalf("back-filled sibling = %s, %v; want the canonical bytes %s", got, err, raw) |
| + } |
| +} |
diff --git a/internal/release/versions.go b/internal/release/versions.go| index f33475e20..c2952e1f1 100644 |
| --- a/internal/release/versions.go |
| +++ b/internal/release/versions.go |
| @@ -169,13 +169,14 @@ func StagedManifestPath(optDir, acct, ver, app string) string { |
| // suite (no test kills a fault injected between the write and the rename); |
| // it is the same primitive StageArtifact already trusts for the binary. |
| // |
| -// raw is whatever bytes the caller already has (ensureStaged's |
| -// json.MarshalIndent of the fetched Manifest) — this is the ONE write path |
| -// that skips StageArtifact, so it repeats StageArtifact's three charset |
| -// gates itself rather than relying on a caller that staged the binary |
| -// first: the back-fill branch calls this with no StageArtifact call in the |
| -// same pass, and the reserved-word/traversal protection those gates give |
| -// the binary's path must cover this sibling path too. |
| +// raw is the canonical manifest's bytes as the store returned them |
| +// (GetManifestRaw), so the sibling is lossless (zero-downtime deploys spec |
| +// §4.2). This is the ONE write path that skips StageArtifact, so it repeats |
| +// StageArtifact's three charset gates itself rather than relying on a caller |
| +// that staged the binary first: the back-fill branch calls this with no |
| +// StageArtifact call in the same pass, and the reserved-word/traversal |
| +// protection those gates give the binary's path must cover this sibling path |
| +// too. |
| func StageManifest(optDir, acct, ver, app string, raw []byte) error { |
| if !ValidVersion(ver) { |
| return fmt.Errorf("invalid version %q (allowed: letters, digits, . _ -)", ver) |
| @@ -233,13 +234,12 @@ func StageManifest(optDir, acct, ver, app string, raw []byte) error { |
| // reserved-word gate under keymail's own identity, and its capabilities |
| // (host_role's User=, runtime_dir, ports) would land on keymail's unit. |
| // |
| -// The sibling is a deliberately LOSSY re-marshal of the bucket's manifest: |
| -// json.MarshalIndent drops any top-level key this binary's Manifest struct |
| -// doesn't know, and caps.Set's decoder notes (unknown fields inside a known |
| -// capability) are unexported and never survive the round trip. The bucket |
| -// object stays the canonical copy; this sibling exists only so the converge |
| -// pass can read a running version's capabilities without a bucket round |
| -// trip on every tick. |
| +// A sibling staged by this binary or later is the canonical manifest's exact |
| +// bytes. One staged by an older binary is a LOSSY re-marshal: it drops any |
| +// top-level key that binary's Manifest struct did not know, `swap` included. |
| +// So the bucket object stays the authority for anything a box acts on |
| +// (CanonicalDemand); this sibling exists so the converge pass can read a |
| +// running version's capabilities without a bucket round trip on every tick. |
| func ReadStagedManifest(optDir, acct, ver, app string) (*Manifest, error) { |
| raw, err := os.ReadFile(StagedManifestPath(optDir, acct, ver, app)) |
| if err != nil { |