From b88c6dac5bd0bdf6008cf2f2e43ff8099e2fa436 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 4 Nov 2026 16:22:48 +0100 Subject: [PATCH] CAT-AS1: distribution removes an index's platforms with it, but those a kept index names The sweep asks remove now instead of deleting over the store's door, so what its door did for an index is the holder's: a removal for retention removes the platform manifests no tagged or held index of the repository names, and refuses a manifest such an index names. Claude-Session: https://claude.ai/code/session_01JG73KJNrH5EgFXMKKYSrig --- .../cmd/distribution-tools/main.go | 2 +- .../internal/holder/holder_test.go | 67 ++++++++++ .../distribution/internal/holder/platforms.go | 117 ++++++++++++++++++ modules/distribution/internal/holder/verbs.go | 6 +- 4 files changed, 189 insertions(+), 3 deletions(-) create mode 100644 modules/distribution/internal/holder/platforms.go diff --git a/modules/distribution/cmd/distribution-tools/main.go b/modules/distribution/cmd/distribution-tools/main.go index e5cd3dee..b16a18d0 100644 --- a/modules/distribution/cmd/distribution-tools/main.go +++ b/modules/distribution/cmd/distribution-tools/main.go @@ -306,7 +306,7 @@ func tools(v *holder.Verbs, windows kept[holder.Window], callers func() (holder. } return versionsAnswer(v, str(args, "kind"), a), nil }}, - stdio.Tool{Name: seat + "." + holder.VerbRemove, Description: "Remove a tagged version (the builder), or for retention an untagged one by its digest (the artifact sweep); says removed.", + stdio.Tool{Name: seat + "." + holder.VerbRemove, Description: "Remove a tagged version (the builder), or for retention an untagged one by its digest (the artifact sweep; an index with the platform manifests no tagged or held index names, and never a manifest such an index names); says removed.", Input: obj(map[string]any{"name": s("name"), "version": s("a tagged version"), "tag": s("its tag"), "digest": s("for retention"), "reason": s("why")}, "name", "reason"), Run: func(args map[string]any) (any, error) { v, err := now() diff --git a/modules/distribution/internal/holder/holder_test.go b/modules/distribution/internal/holder/holder_test.go index ec1ba9b6..eb589a9c 100644 --- a/modules/distribution/internal/holder/holder_test.go +++ b/modules/distribution/internal/holder/holder_test.go @@ -89,6 +89,7 @@ type fakeCatalog struct { tags map[string]string // name:version -> digest removed []string mirrorErr error + indexes map[string][]string // name@digest -> the platform manifests it names } func (c *fakeCatalog) Versions(name string) ([]string, error) { @@ -112,6 +113,9 @@ func (c *fakeCatalog) RemoveDigest(name, digest string) error { c.removed = append(c.removed, name+"@"+digest) return nil } +func (c *fakeCatalog) Platforms(name, digest string) ([]string, error) { + return c.indexes[name+"@"+digest], nil +} func (c *fakeCatalog) Mirror(source, digest string) (string, error) { if c.mirrorErr != nil { return "", c.mirrorErr @@ -359,6 +363,69 @@ func TestTheHolderOfTheTwoImageSeats(t *testing.T) { t.Errorf("the builder's removal of a tagged version: %v", err) } }) + // CAT-AS1: what the sweep did over the store's door is the holder's: a removal for retention of an index removes + // its platform manifests first, but those a tagged or held index of the repository names; a manifest such an index + // names is refused; OCI.Platforms reads an index's from the store. + t.Run("an index's platforms go with it, but those a kept index names", func(t *testing.T) { + d := func(s string) string { return "sha256:" + strings.Repeat(s, 64) } + sweep := "novox.artifact-store-tools" + name := "upstream/docker.io/library/alpine" + for _, c := range []struct { + case_, remove string + tagged, held string // the kept index, by a tag or a hold; none when empty + removed []string + refused string + }{ + {case_: "no kept index", remove: d("1"), removed: []string{d("a"), d("b"), d("1")}}, + {case_: "a tagged index shares a platform", remove: d("1"), tagged: d("2"), removed: []string{d("a"), d("1")}}, + {case_: "a held index shares a platform", remove: d("1"), held: d("2"), removed: []string{d("a"), d("1")}}, + {case_: "a platform of a held index", remove: d("b"), held: d("2"), refused: "is kept by the index " + d("2")}, + {case_: "an image that is no index", remove: d("c"), removed: []string{d("c")}}, + } { + b := newBed(true) + b.cat.indexes = map[string][]string{name + "@" + d("1"): {d("a"), d("b")}, name + "@" + d("2"): {d("b")}} + if c.tagged != "" { + b.cat.tags[name+":3.20"] = c.tagged + } + if c.held != "" { + if err := b.v.HoldOne(ImageSeat, sweep, HoldRequest{Name: name, Digest: c.held}); err != nil { + t.Fatal(err) + } + } + err := b.v.Remove(ImageSeat, sweep, RemoveRequest{Name: name, Digest: c.remove, Reason: ReasonRetention}) + var removed []string + for _, r := range b.cat.removed { + removed = append(removed, strings.TrimPrefix(r, name+"@")) + } + if c.refused != "" { + if err == nil || !strings.Contains(err.Error(), c.refused) || len(removed) != 0 { + t.Errorf("%s: %v, removed %v; want refused saying %q and nothing removed", c.case_, err, removed, c.refused) + } + continue + } + if err != nil || !reflect.DeepEqual(removed, c.removed) { + t.Errorf("%s: %v, removed %v, want %v", c.case_, err, removed, c.removed) + } + } + store := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/v2/" + name + "/manifests/" + d("1"): + _, _ = w.Write([]byte(`{"mediaType":"application/vnd.oci.image.index.v1+json","manifests":[{"digest":"` + d("a") + `"},{"digest":"` + d("b") + `"}]}`)) + case "/v2/" + name + "/manifests/" + d("c"): + _, _ = w.Write([]byte(`{"mediaType":"application/vnd.oci.image.manifest.v1+json","layers":[]}`)) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer store.Close() + u, _ := url.Parse(store.URL) + o := &OCI{Base: u} + for digest, want := range map[string][]string{d("1"): {d("a"), d("b")}, d("c"): nil, d("9"): nil} { + if got, err := o.Platforms(name, digest); err != nil || !reflect.DeepEqual(got, want) { + t.Errorf("Platforms(%s) = %v, %v, want %v", digest[:10], got, err, want) + } + } + }) t.Run("the store's port: reads open, writes from distribution alone, and the direct-write window", func(t *testing.T) { store := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusCreated) })) defer store.Close() diff --git a/modules/distribution/internal/holder/platforms.go b/modules/distribution/internal/holder/platforms.go new file mode 100644 index 00000000..8cc0bf2e --- /dev/null +++ b/modules/distribution/internal/holder/platforms.go @@ -0,0 +1,117 @@ +package holder + +// **An index's platform manifests go with it, unless a kept index names them** (CAT-AS1: what the artifact sweep did +// over the store's door, ADR 0257, is the holder's now that the sweep asks `remove`). A removal for retention of an +// index removes first every platform manifest it names that no other kept index of the repository names, then the +// index; a kept index is one a tag names or one held (`hold`). A manifest a kept index names is never removed for +// retention, so holding an index keeps its platforms too. + +import ( + "encoding/json" + "fmt" + "net/http" + "strings" +) + +// spared is every manifest of name a kept index keeps on seat, but the one being removed (except): each tagged +// manifest and held digest, and the platform manifests each of those names. +func (v *Verbs) spared(seat, name, except string) (map[string]string, error) { + keepers := map[string]string{} // digest -> why it is kept + tags, err := v.Catalog.Versions(name) + if err != nil { + return nil, err + } + for _, t := range tags { + d, err := v.Catalog.Digest(name, t) + if err != nil { + return nil, err + } + if d != "" { + keepers[d] = "the tag " + t + } + } + holds, err := v.Holds.List() + if err != nil { + return nil, err + } + for _, h := range holds { + if h.Seat == seat && h.Name == name { + keepers[h.Digest] = "the hold of " + h.Digest + } + } + out := map[string]string{} + for d, why := range keepers { + if d == except { + continue + } + out[d] = why + children, err := v.Catalog.Platforms(name, d) + if err != nil { + return nil, fmt.Errorf("reading the kept %s@%s: %w", name, d, err) + } + for _, c := range children { + out[c] = "the index " + d + ", kept by " + why + } + } + return out, nil +} + +// removeForRetention removes name@digest and, for an index, the platform manifests no kept index names; a manifest a +// kept index names is refused. Any read that fails removes nothing. +func (v *Verbs) removeForRetention(seat, name, digest string) error { + children, err := v.Catalog.Platforms(name, digest) + if err != nil { + return fmt.Errorf("remove: the store cannot read %s@%s, so nothing is removed: %w", name, digest, err) + } + spared, err := v.spared(seat, name, digest) + if err != nil { + return fmt.Errorf("remove: which manifests a kept index of %s names cannot be read, so nothing is removed: %w", name, err) + } + if why, kept := spared[digest]; kept { + return fmt.Errorf("remove: refused: %s@%s is kept by %s", name, digest, why) + } + for _, c := range children { + if _, kept := spared[c]; kept { + continue + } + if err := v.Catalog.RemoveDigest(name, c); err != nil { + return fmt.Errorf("remove: the store did not remove %s@%s, a platform of %s: %w", name, c, digest, err) + } + } + if err := v.Catalog.RemoveDigest(name, digest); err != nil { + return fmt.Errorf("remove: the store did not remove %s@%s: %w", name, digest, err) + } + return nil +} + +// Platforms is the manifests an index names, by digest: none for a manifest that is not an index, or one the store +// does not hold. +func (o *OCI) Platforms(name, digest string) ([]string, error) { + resp, b, err := o.do(http.MethodGet, "/v2/"+name+"/manifests/"+digest, "", nil, + "application/vnd.oci.image.index.v1+json", "application/vnd.docker.distribution.manifest.list.v2+json", + mediaManifest, "application/vnd.docker.distribution.manifest.v2+json") + if err != nil { + return nil, err + } + if resp.StatusCode == http.StatusNotFound { + return nil, nil + } + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("the store answered %d for %s@%s", resp.StatusCode, name, digest) + } + var doc struct { + Manifests []struct { + Digest string `json:"digest"` + } `json:"manifests"` + } + if err := json.Unmarshal(b, &doc); err != nil { + return nil, fmt.Errorf("%s@%s is not a manifest: %w", name, digest, err) + } + var out []string + for _, m := range doc.Manifests { + if strings.HasPrefix(m.Digest, "sha256:") { + out = append(out, m.Digest) + } + } + return out, nil +} diff --git a/modules/distribution/internal/holder/verbs.go b/modules/distribution/internal/holder/verbs.go index 30b1c7bd..18d02d1c 100644 --- a/modules/distribution/internal/holder/verbs.go +++ b/modules/distribution/internal/holder/verbs.go @@ -20,6 +20,8 @@ type Catalog interface { Remove(name, version string) error // RemoveDigest deletes an untagged manifest of name by its digest (retention). RemoveDigest(name, digest string) error + // Platforms are the manifests an index of name names; none for a manifest that is not an index or is not held. + Platforms(name, digest string) ([]string, error) // Mirror copies an outside image, pinned by digest, into upstream//, answering the reference. Mirror(source, digest string) (string, error) // Reference is the seat's answer for name at digest, known before anything is copied. @@ -132,8 +134,8 @@ func (v *Verbs) Remove(seat, caller string, r RemoveRequest) error { } else if held { return fmt.Errorf("remove: refused: %s@%s is held; release it first", r.Name, r.Digest) } - if err := v.Catalog.RemoveDigest(r.Name, r.Digest); err != nil { - return fmt.Errorf("remove: the store did not remove %s@%s: %w", r.Name, r.Digest, err) + if err := v.removeForRetention(seat, r.Name, r.Digest); err != nil { + return err } default: if r.Version == "" || !strings.Contains(r.Version, "-") {