amadan
settings-1 final fixes: an unreadable channel makes the digests report incomplete
DigestReport returns a channel whose pointer cannot be read or verified with Err set instead of skipping it; the console door and the direct-store CLI mark the account unknown with that error, so the none-left line never prints on an incomplete report (Astra r1 #4).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
6 files changed,
+176
−5
channels_digests.go+8 −1channels_digests_test.go+47 −0internal/console/apidigests.go+8 −1internal/console/apidigests_test.go+57 −0internal/release/digests.go+8 −3internal/release/digests_test.go+48 −0
diff --git a/channels_digests.go b/channels_digests.go| index 03ee07c92..43fe79dd0 100644 |
| --- a/channels_digests.go |
| +++ b/channels_digests.go |
| @@ -308,7 +308,14 @@ func localPointerDigests(ctx context.Context, s release.Store, verb string, ids |
| break |
| } |
| for _, st := range states { |
| - if !st.HeadDigest { |
| + switch { |
| + case st.Err != "": |
| + // A channel whose pointer could not be read leaves the |
| + // account unknown, as the console's report does. |
| + if row.Error == "" { |
| + row.Error = st.Err |
| + } |
| + case !st.HeadDigest: |
| rep.Channels = append(rep.Channels, apiDigestChannel{Account: acct, App: app, Channel: st.Channel, Version: st.Version, Straggler: st.Straggler}) |
| } |
| } |
diff --git a/channels_digests_test.go b/channels_digests_test.go| index d29bdef57..5d940c111 100644 |
| --- a/channels_digests_test.go |
| +++ b/channels_digests_test.go |
| @@ -8,6 +8,7 @@ import ( |
| "fmt" |
| "net/http" |
| "net/http/httptest" |
| + "os" |
| "strings" |
| "testing" |
| "time" |
| @@ -343,3 +344,49 @@ func TestChannelsDigestsDirectStore(t *testing.T) { |
| t.Fatalf("unverifiable marker read as off or done:\n%s", out) |
| } |
| } |
| + |
| +// Astra r1 #4, direct-store: every readable channel carries digests, but one |
| +// pointer cannot be verified. The account is reported unknown with that |
| +// channel's error, and the none-left line never prints. |
| +func TestChannelsDigestsDirectStoreUnreadableChannel(t *testing.T) { |
| + s, pub := directDigestStore(t) // bab: marker on, km/stable at v1 with no digest |
| + ctx := context.Background() |
| + priv, err := release.ParseSigningKey(os.Getenv("CARLOS_RELEASE_KEY"), "test") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := release.Promote(ctx, s, priv, "bab", "km", "beta", "v1"); err != nil { // under the marker: digests |
| + t.Fatal(err) |
| + } |
| + key := release.ChannelKey("bab", "km", "stable") |
| + _, etag, err := s.Get(ctx, key) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := s.Put(ctx, key, []byte("junk"), etag); err != nil { |
| + t.Fatal(err) |
| + } |
| + rows, _, err := release.DigestReport(ctx, s, pub, "bab", "km") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + cause := "" |
| + for _, r := range rows { |
| + switch { |
| + case r.Channel == "stable": |
| + cause = r.Err |
| + case !r.HeadDigest: |
| + t.Fatalf("control: %s carries no digest, so none-left would be withheld anyway", r.Channel) |
| + } |
| + } |
| + if cause == "" { |
| + t.Fatal("control: the junk pointer read as a channel state") |
| + } |
| + out := runDigests(t, "--account-id", "bab") |
| + if want := fmt.Sprintf(textStatusError, "bab", cause); !strings.Contains(out, want) { |
| + t.Fatalf("output lacks %q:\n%s", want, out) |
| + } |
| + if strings.Contains(out, textStatusNoneLeft) { |
| + t.Fatalf("an incomplete report printed the none-left line:\n%s", out) |
| + } |
| +} |
diff --git a/internal/console/apidigests.go b/internal/console/apidigests.go| index be3d9266b..7c68ff5a3 100644 |
| --- a/internal/console/apidigests.go |
| +++ b/internal/console/apidigests.go |
| @@ -239,7 +239,14 @@ func (s *Server) handleAPISystemPointerDigests(w http.ResponseWriter, r *http.Re |
| break |
| } |
| for _, st := range states { |
| - if !st.HeadDigest { |
| + switch { |
| + case st.Err != "": |
| + // Unknown, not "has digests": the account's report is |
| + // incomplete, so the CLI never calls the fleet ready. |
| + if row.Error == "" { |
| + row.Error = st.Err |
| + } |
| + case !st.HeadDigest: |
| chRows = append(chRows, chRow{Account: a.Sqid, App: app, Channel: st.Channel, Version: st.Version, Straggler: st.Straggler}) |
| } |
| } |
diff --git a/internal/console/apidigests_test.go b/internal/console/apidigests_test.go| index 072c1c801..6e226932c 100644 |
| --- a/internal/console/apidigests_test.go |
| +++ b/internal/console/apidigests_test.go |
| @@ -155,6 +155,63 @@ func TestSystemDigestsDoorListsOnlyHeadsWithoutDigests(t *testing.T) { |
| } |
| } |
| +// Astra r1 #4: a channel whose pointer cannot be verified is not skipped as |
| +// if it carried digests. The account's row carries the error, so the CLI |
| +// reports it unknown and never calls the fleet ready. |
| +func TestSystemDigestsDoorReportsAChannelItCannotRead(t *testing.T) { |
| + fx := newDigestsFixture(t) |
| + ctx := context.Background() |
| + key := release.ChannelKey(release.OpsAccount, "km", "beta") |
| + _, etag, err := fx.store.Get(ctx, key) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := fx.store.Put(ctx, key, []byte("junk"), etag); err != nil { |
| + t.Fatal(err) |
| + } |
| + st, out := getRaw(t, fx.ts.URL+"/api/cli/system/pointer-digests", fx.operator) |
| + if st != http.StatusOK { |
| + t.Fatalf("report: %d %s", st, out) |
| + } |
| + var rep struct { |
| + Accounts []struct{ Account, Error string } `json:"accounts"` |
| + Channels []struct{ Account, App, Channel string } `json:"channels"` |
| + } |
| + if err := json.Unmarshal([]byte(out), &rep); err != nil { |
| + t.Fatalf("report body %s: %v", out, err) |
| + } |
| + rows, _, err := release.DigestReport(ctx, fx.store, fx.pub, release.OpsAccount, "km") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + want := "" |
| + for _, r := range rows { |
| + if r.Channel == "beta" { |
| + want = r.Err |
| + } |
| + } |
| + if want == "" { |
| + t.Fatal("control: the junk pointer read as a channel state") |
| + } |
| + found := false |
| + for _, a := range rep.Accounts { |
| + if a.Account == release.OpsAccount { |
| + found = true |
| + if a.Error != want { |
| + t.Fatalf("bab's row error = %q, want %q", a.Error, want) |
| + } |
| + } |
| + } |
| + if !found { |
| + t.Fatalf("no row for bab: %s", out) |
| + } |
| + for _, c := range rep.Channels { |
| + if c.Channel == "beta" { |
| + t.Fatalf("the unreadable channel was listed as a state: %s", out) |
| + } |
| + } |
| +} |
| + |
| func TestChannelsDoorCarriesDigestState(t *testing.T) { |
| fx := newDigestsFixture(t) |
| st, out := getRaw(t, fx.ts.URL+"/api/cli/apps/bab/km/channels", fx.member) |
diff --git a/internal/release/digests.go b/internal/release/digests.go| index 040856b2b..b9568a49f 100644 |
| --- a/internal/release/digests.go |
| +++ b/internal/release/digests.go |
| @@ -130,7 +130,10 @@ func (e *ResignNeededError) Error() string { |
| } |
| // ChannelDigestState is one channel's line in the straggler report (spec |
| -// §4.6 "Watch for stragglers"). |
| +// §4.6 "Watch for stragglers"). Err set means the channel's pointer could not |
| +// be read or verified: its digest state is unknown, and every other field is |
| +// zero. A report with such a line is incomplete, so it never says the fleet |
| +// is ready for CARLOS_REQUIRE_POINTER_DIGEST (Astra r1 #4). |
| type ChannelDigestState struct { |
| Channel string `json:"channel"` |
| Version string `json:"version"` |
| @@ -138,6 +141,7 @@ type ChannelDigestState struct { |
| HeadDigest bool `json:"head_digest"` |
| HistoryMissing int `json:"history_missing"` |
| Straggler bool `json:"straggler"` |
| + Err string `json:"error,omitempty"` |
| } |
| // DigestStateOf is a verified pointer's line. A straggler is a pointer |
| @@ -156,8 +160,8 @@ func DigestStateOf(p *ChannelPointer, marker *DigestMarker) ChannelDigestState { |
| } |
| // DigestReport is every channel of one app, plus the account's marker. A |
| -// pointer that does not verify is skipped here: `carlos channels` already |
| -// shows it as an error row. |
| +// pointer that cannot be read or does not verify is never skipped: it comes |
| +// back as a line with Err set, its state unknown. |
| func DigestReport(ctx context.Context, s Store, pub ed25519.PublicKey, acct, app string) ([]ChannelDigestState, *DigestMarker, error) { |
| marker, err := ReadDigestMarker(ctx, s, pub, acct) |
| if err != nil { |
| @@ -176,6 +180,7 @@ func DigestReport(ctx context.Context, s Store, pub ed25519.PublicKey, acct, app |
| } |
| p, err := Channel(ctx, s, pub, acct, app, name) |
| if err != nil { |
| + out = append(out, ChannelDigestState{Channel: name, Err: fmt.Sprintf("%s: %v", ChannelKey(acct, app, name), err)}) |
| continue |
| } |
| out = append(out, DigestStateOf(p, marker)) |
diff --git a/internal/release/digests_test.go b/internal/release/digests_test.go| index 959dc2241..5b4ff2a80 100644 |
| --- a/internal/release/digests_test.go |
| +++ b/internal/release/digests_test.go |
| @@ -4,6 +4,7 @@ import ( |
| "bytes" |
| "context" |
| "errors" |
| + "strings" |
| "testing" |
| ) |
| @@ -55,3 +56,50 @@ func TestValidDigest(t *testing.T) { |
| } |
| } |
| } |
| + |
| +// failingGetStore fails every Get of one key, as a transient bucket error |
| +// would, while listing still names it. |
| +type failingGetStore struct { |
| + Store |
| + key string |
| +} |
| + |
| +var errTransient = errors.New("transient read failure") |
| + |
| +func (f failingGetStore) Get(ctx context.Context, key string) ([]byte, string, error) { |
| + if key == f.key { |
| + return nil, "", errTransient |
| + } |
| + return f.Store.Get(ctx, key) |
| +} |
| + |
| +// Astra r1 #4: a channel whose pointer cannot be read is not skipped. It |
| +// comes back with Err set, so no consumer can count it as carrying digests. |
| +func TestDigestReportMarksAChannelItCannotRead(t *testing.T) { |
| + base := NewDirStore(t.TempDir()) |
| + ctx := context.Background() |
| + priv, pub := testKeys(t) |
| + shipRelease(t, base, "bab", "km", "v1", adoptPlatform, []byte("one")) |
| + if err := Promote(ctx, base, priv, "bab", "km", "beta", "v1"); err != nil { // no digests |
| + t.Fatal(err) |
| + } |
| + markerOn(t, base, priv, "bab") |
| + if err := Promote(ctx, base, priv, "bab", "km", "canary/x", "v1"); err != nil { // digests |
| + t.Fatal(err) |
| + } |
| + s := failingGetStore{Store: base, key: ChannelKey("bab", "km", "beta")} |
| + rows, _, err := DigestReport(ctx, s, pub, "bab", "km") |
| + if err != nil || len(rows) != 2 { |
| + t.Fatalf("report = %+v, %v; want both channels", rows, err) |
| + } |
| + got := map[string]ChannelDigestState{} |
| + for _, r := range rows { |
| + got[r.Channel] = r |
| + } |
| + if b := got["beta"]; b.Err == "" || !strings.Contains(b.Err, errTransient.Error()) || b.HeadDigest { |
| + t.Fatalf("beta = %+v; want its read error, never a digest state", b) |
| + } |
| + if c := got["canary/x"]; c.Err != "" || !c.HeadDigest { |
| + t.Fatalf("canary/x = %+v; want it read as carrying digests", c) |
| + } |
| +} |