more gracefully handle other types of resources, like .sig files

sidecar/internal/registry/registry.go

@@ -4,10 +4,12 @@ package registry
 
 import (
 	"context"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"io"
 	"net/http"
+	"slices"
 	"time"
 )
 
@@ -33,10 +35,37 @@ func New(baseURL string) *Client {
 	}
 }
 
+// gone lists the distribution error codes that mean the delete has nothing left
+// to do: the repository or the manifest is already absent. zot returns
+// NAME_UNKNOWN with a 400, not a 404, once a repo's last tag is reaped and the
+// repo itself disappears — a cosign .sig tag outliving its image hits this.
+var gone = []string{"NAME_UNKNOWN", "MANIFEST_UNKNOWN"}
+
+// isGone reports whether an error body carries one of the `gone` codes. Matching
+// on the body rather than the status keeps a genuinely malformed request, which
+// is also a 400, retryable and visible.
+func isGone(body []byte) bool {
+	var payload struct {
+		Errors []struct {
+			Code string `json:"code"`
+		} `json:"errors"`
+	}
+	if err := json.Unmarshal(body, &payload); err != nil {
+		return false
+	}
+	for _, e := range payload.Errors {
+		if slices.Contains(gone, e.Code) {
+			return true
+		}
+	}
+	return false
+}
+
 // DeleteManifest issues DELETE /v2/<repo>/manifests/<tag>; zot resolves the tag
-// to its digest. 200/202/204/404 all mean the tag is gone; any other status is
-// a transient error to retry on the next tick. ctx bounds the request, so a
-// wedged registry cannot outlive a shutdown.
+// to its digest. 200/202/204/404, and any error naming a missing repo or
+// manifest, all mean the tag is gone; any other status is a transient error to
+// retry on the next tick. ctx bounds the request, so a wedged registry cannot
+// outlive a shutdown.
 func (c *Client) DeleteManifest(ctx context.Context, repo, tag string) error {
 	endpoint := fmt.Sprintf("%s/v2/%s/manifests/%s", c.baseURL, repo, tag)
 	req, err := http.NewRequestWithContext(ctx, http.MethodDelete, endpoint, nil)
@@ -49,10 +78,14 @@ func (c *Client) DeleteManifest(ctx context.Context, repo, tag string) error {
 	}
 	defer func() { _ = resp.Body.Close() }()
 	body, _ := io.ReadAll(resp.Body)
-	switch resp.StatusCode {
-	case http.StatusOK, http.StatusAccepted, http.StatusNoContent, http.StatusNotFound:
+	switch {
+	case resp.StatusCode == http.StatusOK,
+		resp.StatusCode == http.StatusAccepted,
+		resp.StatusCode == http.StatusNoContent,
+		resp.StatusCode == http.StatusNotFound,
+		isGone(body):
 		return nil
-	case http.StatusMethodNotAllowed:
+	case resp.StatusCode == http.StatusMethodNotAllowed:
 		return fmt.Errorf("DELETE %s: %w", endpoint, ErrManifestReferenced)
 	default:
 		// zot describes the failure in the body; %q keeps it on one log line.

sidecar/internal/registry/registry_test.go

@@ -71,6 +71,63 @@ func TestDeleteManifestReferencedByIndex(t *testing.T) {
 	}
 }
 
+// Once a repo's last tag is reaped the repo itself disappears, and zot answers a
+// delete against it with 400/NAME_UNKNOWN rather than 404. A cosign .sig tag
+// outliving its image lands here, so it has to read as success — otherwise the
+// row is never dropped and the reaper retries it every tick forever.
+func TestDeleteManifestRepoAlreadyGone(t *testing.T) {
+	cases := []struct {
+		name    string
+		status  int
+		body    string
+		wantErr bool
+	}{
+		{
+			name:   "NAME_UNKNOWN",
+			status: http.StatusBadRequest,
+			body:   `{"errors":[{"code":"NAME_UNKNOWN","message":"repository name not known to registry","detail":{"name":"c1e2d663-74a5-47d1-a395-d1b8cdacb137"}}]}`,
+		},
+		{
+			name:   "MANIFEST_UNKNOWN",
+			status: http.StatusBadRequest,
+			body:   `{"errors":[{"code":"MANIFEST_UNKNOWN","message":"manifest unknown"}]}`,
+		},
+		{
+			// A malformed request is also a 400, and that one is worth retrying
+			// and surfacing rather than silently untracking.
+			name:    "other 400 stays an error",
+			status:  http.StatusBadRequest,
+			body:    `{"errors":[{"code":"UNSUPPORTED","message":"the operation is unsupported"}]}`,
+			wantErr: true,
+		},
+		{
+			name:    "unparseable body stays an error",
+			status:  http.StatusBadRequest,
+			body:    `not json`,
+			wantErr: true,
+		},
+	}
+
+	for _, tc := range cases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				w.WriteHeader(tc.status)
+				_, _ = w.Write([]byte(tc.body))
+			}))
+			defer srv.Close()
+
+			err := New(srv.URL).DeleteManifest(context.Background(), "foo/bar", "sha256-abc.sig")
+			if tc.wantErr && err == nil {
+				t.Fatal("expected error, got nil")
+			}
+			if !tc.wantErr && err != nil {
+				t.Fatalf("expected nil, got error: %v", err)
+			}
+		})
+	}
+}
+
 // Port 1 refuses connections, so http.Client.Do fails and the error surfaces.
 func TestDeleteManifestTransportError(t *testing.T) {
 	c := New("http://127.0.0.1:1")