diff --git a/modules/artifact-store-tools/cmd/artifact-store-tools/holder_test.go b/modules/artifact-store-tools/cmd/artifact-store-tools/holder_test.go index a0effbb3..37b7d553 100644 --- a/modules/artifact-store-tools/cmd/artifact-store-tools/holder_test.go +++ b/modules/artifact-store-tools/cmd/artifact-store-tools/holder_test.go @@ -124,11 +124,9 @@ func (d *memDoor) seat(key string, body any) (json.RawMessage, error) { d.held[ref] = true return json.Marshal(map[string]any{"held": true}) case "release": - if !d.held[ref] { - return nil, fmt.Errorf("release: %s@%s is not held on %s", in.Name, in.Digest, seat) - } + released := d.held[ref] // what is not held answers released false, as the seat's definition says delete(d.held, ref) - return json.Marshal(map[string]any{"released": true}) + return json.Marshal(map[string]any{"released": released}) case "remove": if d.held[ref] { return nil, fmt.Errorf("remove: refused: %s@%s is held; release it first", in.Name, in.Digest) diff --git a/modules/artifact-store-tools/cmd/artifact-store-tools/sweepseats.go b/modules/artifact-store-tools/cmd/artifact-store-tools/sweepseats.go index 20519cc7..ecb4e373 100644 --- a/modules/artifact-store-tools/cmd/artifact-store-tools/sweepseats.go +++ b/modules/artifact-store-tools/cmd/artifact-store-tools/sweepseats.go @@ -93,19 +93,16 @@ func (h *Holder) holdOnSeat(reference, why string) (bool, error) { } // Retention is the reason of a removal by digest, the sweep's alone (design 51 *Who may call which verb*): the holder -// removes an untagged thing only for it. The seat's definition names no such value yet (a request to mesh-contracts). -const Retention = "retention" - -// notHeld is the holder's refusal of a release of what it does not hold: nothing to let go. -const notHeld = "is not held" +// removes an untagged thing only for it. The seat's definition names it. +const Retention = reg.RemoveInputReasonRetention // removeOnSeat lets reference go: a hold an earlier sweep asked while it was kept is released first (the holder -// refuses to remove a held thing), then it is removed for retention. why goes with the release; the hand act records it. +// refuses to remove a held thing; one not held answers released false), then it is removed for retention. why goes with +// the release; the hand act records it. func (h *Holder) removeOnSeat(reference, why string) error { - err := h.askSeat(reference, reg.VerbRelease, func(name, digest string) any { + if err := h.askSeat(reference, reg.VerbRelease, func(name, digest string) any { return reg.ReleaseInput{Name: name, Digest: digest, Reason: why} - }, ®.ReleaseAnswer{}) - if err != nil && !strings.Contains(err.Error(), notHeld) { + }, ®.ReleaseAnswer{}); err != nil { return err } return h.askSeat(reference, reg.VerbRemove, func(name, digest string) any { diff --git a/modules/distribution/cmd/distribution-tools/main.go b/modules/distribution/cmd/distribution-tools/main.go index b16a18d0..2385b3e9 100644 --- a/modules/distribution/cmd/distribution-tools/main.go +++ b/modules/distribution/cmd/distribution-tools/main.go @@ -350,10 +350,11 @@ func tools(v *holder.Verbs, windows kept[holder.Window], callers func() (holder. if err != nil { return nil, err } - if err := v.Release(seat, stdio.CallerOf(args), holder.HoldRequest{Name: in.Name, Digest: in.Digest, Why: in.Reason}); err != nil { + released, err := v.Release(seat, stdio.CallerOf(args), holder.HoldRequest{Name: in.Name, Digest: in.Digest, Why: in.Reason}) + if err != nil { return nil, err } - return reg.ReleaseAnswer{Released: true}, nil + return reg.ReleaseAnswer{Released: released}, nil }}, ) } @@ -434,7 +435,11 @@ func tools(v *holder.Verbs, windows kept[holder.Window], callers func() (holder. return nil, errors.New("a release by hand is a hand act: say why") } // The caller proved on this call (the operator's session) is carried on; distribution's own never. - return map[string]any{"released": name}, v.Release(str(args, "seat"), stdio.CallerOf(args), holder.HoldRequest{Name: name, Digest: str(args, "digest"), Why: str(args, "why")}) + released, err := v.Release(str(args, "seat"), stdio.CallerOf(args), holder.HoldRequest{Name: name, Digest: str(args, "digest"), Why: str(args, "why")}) + if err == nil && !released { + return map[string]any{"released": false, "note": name + " is not held: nothing was let go"}, nil + } + return map[string]any{"released": name}, err } return v.HeldNow() }}, diff --git a/modules/distribution/internal/holder/holder_test.go b/modules/distribution/internal/holder/holder_test.go index eb589a9c..5e15abfa 100644 --- a/modules/distribution/internal/holder/holder_test.go +++ b/modules/distribution/internal/holder/holder_test.go @@ -281,10 +281,12 @@ func TestTheHolderOfTheTwoImageSeats(t *testing.T) { return b.v.HoldOne(ArtifactSeat, "novox.mesh-build-go", HoldRequest{Name: "m/a", Kind: KindArchive, Digest: part.Digest}) }, "not on the holder's caller list"}, {"a release from a caller not on the list", func(b *bed) error { - return b.v.Release(ArtifactSeat, "novox.mesh-build-go", HoldRequest{Name: "m/a", Digest: part.Digest}) + _, err := b.v.Release(ArtifactSeat, "novox.mesh-build-go", HoldRequest{Name: "m/a", Digest: part.Digest}) + return err }, "not on the holder's caller list"}, {"a release in distribution's own name", func(b *bed) error { - return b.v.Release(ArtifactSeat, "novox.distribution", HoldRequest{Name: "m/a", Digest: part.Digest}) + _, err := b.v.Release(ArtifactSeat, "novox.distribution", HoldRequest{Name: "m/a", Digest: part.Digest}) + return err }, "in its own name"}, {"a mirror by no digest", func(b *bed) error { _, err := b.v.Mirror(ImageSeat, "novox.mesh-build-image", MirrorRequest{Source: "docker.io/library/nats:2"}) @@ -348,8 +350,12 @@ func TestTheHolderOfTheTwoImageSeats(t *testing.T) { if err := b.v.HoldOne(ArtifactSeat, sweep, HoldRequest{Name: "m/b", Kind: KindArchive, Digest: part.Digest}); err != nil { t.Fatal(err) } - if err := b.v.Release(ArtifactSeat, "person.jochen", HoldRequest{Name: "m/b", Digest: part.Digest, Why: "by hand"}); err != nil { - t.Errorf("the operator's session releasing through registry_holds: %v", err) + if released, err := b.v.Release(ArtifactSeat, "person.jochen", HoldRequest{Name: "m/b", Digest: part.Digest, Why: "by hand"}); err != nil || !released { + t.Errorf("the operator's session releasing through registry_holds: %v, %v", released, err) + } + // A release of what is not held answers released false, never an error, and says nothing (the seat's definition). + if released, err := b.v.Release(ArtifactSeat, sweep, HoldRequest{Name: "m/b", Digest: part.Digest, Why: "eligible"}); err != nil || released { + t.Errorf("a release of what is not held: %v, %v; want false and no error", released, err) } var events []string for _, s := range b.said { diff --git a/modules/distribution/internal/holder/verbs.go b/modules/distribution/internal/holder/verbs.go index 18d02d1c..cc90a063 100644 --- a/modules/distribution/internal/holder/verbs.go +++ b/modules/distribution/internal/holder/verbs.go @@ -8,6 +8,8 @@ import ( "slices" "strings" "time" + + reg "git.novox.be/novox/mesh-contracts/go/seats/containerimageregistry" ) // Catalog is what the store answers of what it holds. @@ -109,7 +111,8 @@ type RemoveRequest struct { } // ReasonRetention is the reason of the sweep's removal of an untagged version (ADR 0610 ยง3). -const ReasonRetention = "retention" +// The seat definitions name it (the removal reasons' enum), so the sweep and the holder share one value. +const ReasonRetention = reg.RemoveInputReasonRetention // Remove is `remove`: the builder removes a tagged version; the artifact sweep's holder an untagged bundle, archive or // copied base with the reason retention, which a hold keeps. Every removal says `removed` with its reason. @@ -191,25 +194,25 @@ func (v *Verbs) HoldOne(seat, caller string, r HoldRequest) error { // Release is `release`: the artifact sweep's holder, or the operator's session through registry_holds (a hand act, // recorded by the tool); distribution's own account is never on its list. -func (v *Verbs) Release(seat, caller string, r HoldRequest) error { +func (v *Verbs) Release(seat, caller string, r HoldRequest) (bool, error) { if CallerModule(caller) == Self && !strings.HasPrefix(caller, "person.") { - return fmt.Errorf("release: refused for %s: distribution cannot release in its own name", caller) + return false, fmt.Errorf("release: refused for %s: distribution cannot release in its own name", caller) } if err := v.Callers.Allowed(VerbRelease, caller); err != nil { - return err + return false, err } key := Hold{Seat: seat, Name: r.Name, Digest: r.Digest}.Key() h, held, err := v.Holds.Get(key) if err != nil { - return err + return false, err } if !held { - return fmt.Errorf("release: %s@%s is not held on %s", r.Name, r.Digest, seat) + return false, nil // nothing held: the outcome asked holds already, and nothing is said (the seat's definition) } if err := v.Holds.Delete(key); err != nil { - return err + return false, err } - return v.Say(seat, EventReleased, Event{Name: h.Name, Kind: h.Kind, Digest: h.Digest, Holder: Self, By: caller, Reason: r.Why}) + return true, v.Say(seat, EventReleased, Event{Name: h.Name, Kind: h.Kind, Digest: h.Digest, Holder: Self, By: caller, Reason: r.Why}) } // Tags is `tags` on the image seat: the tags of an image.