fix(meta): delete repo meta when the last manifest is removed

storage.maxRepos is enforced against metaDB.CountRepos(), but no code
path removed a repo's meta record on delete. RemoveRepoReference drops
the reference and writes the record back; DeleteRepoMeta is only called
by ParseStorage at startup, for repos already absent from storage.

The count therefore never decreases. Once a registry reaches maxRepos,
pushes creating a new repository are rejected until restart, even after
repos are deleted. GC does not help: CleanupRepo removes storage without
touching metadb.

OnDeleteManifest now calls DeleteRepoMeta when the repo's index has no
manifests left.

- Emptiness is taken from the index, not from repo meta: untagged
  content such as a signature referrer leaves no tag to count.
- GetIndexContent expects the caller to hold the lock (as ParseStorage
  does), so the read is wrapped in RLock/RUnlock. Both callers of
  OnDeleteManifest run after DeleteImageManifest released its lock.
- Errors are logged, not returned: the manifest delete already
  succeeded, and ParseStorage corrects a stale record on next start.
- Repos still holding content keep their record and keep counting.

Tests (all fail without the change):

- pkg/meta: emptied repo drops its meta and stops counting; a repo with
  another manifest keeps both. Error paths covered via mocks.
- pkg/api: partial delete keeps the slot, full delete releases it,
  emptied name is reusable, release survives restart, holds with GC on.
- test/blackbox: end-to-end case, plus a guard that only one slot is
  released.

Signed-off-by: Bachir Khiati <bachir.khiati@gmail.com>

Author: Aluchir

pkg/api/quota_test.go

@@ -6,7 +6,9 @@ import (
 	"net/http"
 	"sync"
 	"testing"
+	"time"
 
+	ispec "github.com/opencontainers/image-spec/specs-go/v1"
 	. "github.com/smartystreets/goconvey/convey"
 	"gopkg.in/resty.v1"
 
@@ -136,3 +138,172 @@ func TestQuotaConcurrency(t *testing.T) {
 		})
 	})
 }
+
+// startQuotaServerAt starts a quota-enforcing registry on a given root directory, so a test can restart
+// the server over the same storage.
+func startQuotaServerAt(t *testing.T, maxRepos int, rootDir string, gc bool) (string, func()) {
+	t.Helper()
+
+	conf := config.New()
+	conf.HTTP.Port = "0"
+	conf.Storage.RootDirectory = rootDir
+	conf.Storage.MaxRepos = maxRepos
+	// Dedupe keeps its cache open past Shutdown, blocking a restart on the same dir.
+	conf.Storage.Dedupe = false
+
+	if gc {
+		conf.Storage.GC = true
+		conf.Storage.GCDelay = 1 * time.Second
+		conf.Storage.GCInterval = 2 * time.Second
+	}
+
+	ctlr := api.NewController(conf)
+	ctlrManager := test.NewControllerManager(ctlr)
+	baseURL := ctlrManager.StartAndWait()
+
+	return baseURL, func() { ctlrManager.StopServer() }
+}
+
+func manifestDigest(t *testing.T, baseURL, repo, ref string) string {
+	t.Helper()
+
+	resp, err := resty.R().
+		SetHeader("Accept", ispec.MediaTypeImageManifest).
+		Get(baseURL + "/v2/" + repo + "/manifests/" + ref)
+	So(err, ShouldBeNil)
+
+	return resp.Header().Get("Docker-Content-Digest")
+}
+
+// uploadNewRepo pushes a complete image into a new repo.
+func uploadNewRepo(t *testing.T, baseURL, repo string) error {
+	t.Helper()
+
+	return UploadImage(CreateRandomImage(), baseURL, repo, "v1")
+}
+
+// pushNewRepoStatus reports how the quota answers a new-repo push. Only a refusal is meaningful: the
+// manifest is sent without blobs, so an allowed push fails later in the handler. Use uploadNewRepo to
+// assert acceptance.
+func pushNewRepoStatus(t *testing.T, baseURL, repo string) int {
+	t.Helper()
+
+	img := CreateRandomImage()
+
+	manifestBody, err := json.Marshal(img.Manifest)
+	So(err, ShouldBeNil)
+
+	resp, err := resty.R().
+		SetHeader("Content-Type", ispec.MediaTypeImageManifest).
+		SetBody(manifestBody).
+		Put(baseURL + "/v2/" + repo + "/manifests/v1")
+	So(err, ShouldBeNil)
+
+	return resp.StatusCode()
+}
+
+// TestQuotaSlotReleasedOnDelete checks that a partial delete keeps the slot, a full delete releases it,
+// and the remaining images stay pullable.
+func TestQuotaSlotReleasedOnDelete(t *testing.T) {
+	Convey("Given a registry at its repo limit", t, func() {
+		rootDir := t.TempDir()
+		baseURL, stop := startQuotaServerAt(t, 3, rootDir, false)
+		defer stop()
+
+		first := CreateRandomImage()
+		second := CreateRandomImage()
+
+		So(UploadImage(first, baseURL, "shared", "v1"), ShouldBeNil)
+		So(UploadImage(second, baseURL, "shared", "v2"), ShouldBeNil)
+		So(UploadImage(CreateRandomImage(), baseURL, "second", "v1"), ShouldBeNil)
+		So(UploadImage(CreateRandomImage(), baseURL, "third", "v1"), ShouldBeNil)
+
+		So(pushNewRepoStatus(t, baseURL, "overflow"), ShouldEqual, http.StatusTooManyRequests)
+
+		Convey("Deleting one tag of a two-tag repo releases nothing", func() {
+			resp, err := resty.R().Delete(baseURL + "/v2/shared/manifests/" + first.DigestStr())
+			So(err, ShouldBeNil)
+			So(resp.StatusCode(), ShouldEqual, http.StatusAccepted)
+
+			// The sibling tag is untouched, and still serves the same bytes.
+			So(manifestDigest(t, baseURL, "shared", "v2"), ShouldEqual, second.DigestStr())
+
+			// The repo still holds content, so it still holds its slot.
+			So(pushNewRepoStatus(t, baseURL, "overflow"), ShouldEqual, http.StatusTooManyRequests)
+
+			Convey("Deleting the last tag releases the slot, and the name is reusable", func() {
+				resp, err := resty.R().Delete(baseURL + "/v2/shared/manifests/" + second.DigestStr())
+				So(err, ShouldBeNil)
+				So(resp.StatusCode(), ShouldEqual, http.StatusAccepted)
+
+				Convey("A differently named repo can take the freed slot", func() {
+					So(uploadNewRepo(t, baseURL, "afterfree"), ShouldBeNil)
+				})
+
+				Convey("The emptied name can be reused, and round-trips intact", func() {
+					reused := CreateRandomImage()
+					So(UploadImage(reused, baseURL, "shared", "again"), ShouldBeNil)
+					So(manifestDigest(t, baseURL, "shared", "again"), ShouldEqual, reused.DigestStr())
+				})
+			})
+		})
+	})
+}
+
+// TestQuotaSlotStaysReleasedAcrossRestart checks the startup reparse does not hand back a slot that a
+// delete released.
+func TestQuotaSlotStaysReleasedAcrossRestart(t *testing.T) {
+	Convey("Given a repo emptied while at the limit", t, func() {
+		rootDir := t.TempDir()
+		baseURL, stop := startQuotaServerAt(t, 2, rootDir, false)
+
+		doomed := CreateRandomImage()
+		keeper := CreateRandomImage()
+
+		So(UploadImage(doomed, baseURL, "doomed", "v1"), ShouldBeNil)
+		So(UploadImage(keeper, baseURL, "keeper", "v1"), ShouldBeNil)
+		So(pushNewRepoStatus(t, baseURL, "extra"), ShouldEqual, http.StatusTooManyRequests)
+
+		resp, err := resty.R().Delete(baseURL + "/v2/doomed/manifests/" + doomed.DigestStr())
+		So(err, ShouldBeNil)
+		So(resp.StatusCode(), ShouldEqual, http.StatusAccepted)
+
+		stop()
+
+		Convey("The slot is still free after a restart, and the survivor is intact", func() {
+			restartedURL, stopAgain := startQuotaServerAt(t, 2, rootDir, false)
+			defer stopAgain()
+
+			So(manifestDigest(t, restartedURL, "keeper", "v1"), ShouldEqual, keeper.DigestStr())
+			So(uploadNewRepo(t, restartedURL, "extra"), ShouldBeNil)
+		})
+	})
+}
+
+// TestQuotaSlotReleaseWithGCEnabled runs the release path with GC active, since GC also writes to
+// metadata.
+func TestQuotaSlotReleaseWithGCEnabled(t *testing.T) {
+	Convey("Given a registry with GC enabled and at its limit", t, func() {
+		rootDir := t.TempDir()
+		baseURL, stop := startQuotaServerAt(t, 2, rootDir, true)
+		defer stop()
+
+		doomed := CreateRandomImage()
+		keeper := CreateRandomImage()
+
+		So(UploadImage(doomed, baseURL, "doomed", "v1"), ShouldBeNil)
+		So(UploadImage(keeper, baseURL, "keeper", "v1"), ShouldBeNil)
+
+		resp, err := resty.R().Delete(baseURL + "/v2/doomed/manifests/" + doomed.DigestStr())
+		So(err, ShouldBeNil)
+		So(resp.StatusCode(), ShouldEqual, http.StatusAccepted)
+
+		So(uploadNewRepo(t, baseURL, "extra"), ShouldBeNil)
+
+		// Let several GC cycles run over the emptied repo.
+		time.Sleep(6 * time.Second)
+
+		So(manifestDigest(t, baseURL, "keeper", "v1"), ShouldEqual, keeper.DigestStr())
+		So(pushNewRepoStatus(t, baseURL, "yetanother"), ShouldEqual, http.StatusTooManyRequests)
+	})
+}

pkg/meta/hooks.go

@@ -2,9 +2,11 @@ package meta
 
 import (
 	"context"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"slices"
+	"time"
 
 	godigest "github.com/opencontainers/go-digest"
 	v1 "github.com/opencontainers/image-spec/specs-go/v1"
@@ -15,6 +17,7 @@ import (
 	"zotregistry.dev/zot/v2/pkg/log"
 	mTypes "zotregistry.dev/zot/v2/pkg/meta/types"
 	"zotregistry.dev/zot/v2/pkg/storage"
+	storageTypes "zotregistry.dev/zot/v2/pkg/storage/types"
 )
 
 // priorTagManifest records where MetaDB believed each tag pointed before a digest PUT with tag=
@@ -280,9 +283,52 @@ func OnDeleteManifest(repo, reference, mediaType string, digest godigest.Digest,
 		return err
 	}
 
+	deleteRepoMetaIfEmpty(repo, imgStore, metaDB, log)
+
 	return nil
 }
 
+// deleteRepoMetaIfEmpty removes a repo's meta once its last manifest is gone. RemoveRepoReference only
+// drops the reference, so the record would otherwise keep counting towards maxRepos until the startup
+// reparse. Emptiness comes from the index, since untagged content leaves no tag behind.
+func deleteRepoMetaIfEmpty(repo string, imgStore storageTypes.ImageStore, metaDB mTypes.MetaDB, log log.Logger) {
+	var lockLatency time.Time
+
+	imgStore.RLock(&lockLatency)
+	indexContent, err := imgStore.GetIndexContent(repo)
+	imgStore.RUnlock(&lockLatency)
+
+	if err != nil {
+		log.Debug().Err(err).Str("repository", repo).Str("component", "metadb").
+			Msg("failed to read index while checking whether repo is empty")
+
+		return
+	}
+
+	var index v1.Index
+
+	if err := json.Unmarshal(indexContent, &index); err != nil {
+		log.Debug().Err(err).Str("repository", repo).Str("component", "metadb").
+			Msg("failed to parse index while checking whether repo is empty")
+
+		return
+	}
+
+	if len(index.Manifests) > 0 {
+		return
+	}
+
+	if err := metaDB.DeleteRepoMeta(repo); err != nil {
+		log.Error().Err(err).Str("repository", repo).Str("component", "metadb").
+			Msg("failed to delete meta for emptied repo")
+
+		return
+	}
+
+	log.Debug().Str("repository", repo).Str("component", "metadb").
+		Msg("removed meta for emptied repo")
+}
+
 // OnGetManifest is called when a manifest is downloaded. It increments the download counter on that manifest.
 func OnGetManifest(name, reference, mediaType string, body []byte,
 	storeController storage.StoreController, metaDB mTypes.MetaDB, log log.Logger,

pkg/meta/hooks_internal_test.go

@@ -260,3 +260,60 @@ func TestRollbackDigestManifestTags(t *testing.T) {
 		})
 	})
 }
+
+func TestDeleteRepoMetaIfEmpty(t *testing.T) {
+	Convey("deleteRepoMetaIfEmpty", t, func() {
+		logger := log.NewTestLogger()
+		emptyIndex := []byte(`{"schemaVersion":2,"manifests":[]}`)
+
+		Convey("An unreadable index leaves the meta alone", func() {
+			called := false
+			imgStore := mocks.MockedImageStore{
+				GetIndexContentFn: func(repo string) ([]byte, error) { return nil, errHookInternal },
+			}
+			metaDB := mocks.MetaDBMock{
+				DeleteRepoMetaFn: func(repo string) error { called = true; return nil },
+			}
+
+			deleteRepoMetaIfEmpty("repo", imgStore, metaDB, logger)
+			So(called, ShouldBeFalse)
+		})
+
+		Convey("An unparseable index leaves the meta alone", func() {
+			called := false
+			imgStore := mocks.MockedImageStore{
+				GetIndexContentFn: func(repo string) ([]byte, error) { return []byte("not json"), nil },
+			}
+			metaDB := mocks.MetaDBMock{
+				DeleteRepoMetaFn: func(repo string) error { called = true; return nil },
+			}
+
+			deleteRepoMetaIfEmpty("repo", imgStore, metaDB, logger)
+			So(called, ShouldBeFalse)
+		})
+
+		Convey("A failed delete is swallowed, since the manifest is already gone", func() {
+			imgStore := mocks.MockedImageStore{
+				GetIndexContentFn: func(repo string) ([]byte, error) { return emptyIndex, nil },
+			}
+			metaDB := mocks.MetaDBMock{
+				DeleteRepoMetaFn: func(repo string) error { return errHookInternal },
+			}
+
+			So(func() { deleteRepoMetaIfEmpty("repo", imgStore, metaDB, logger) }, ShouldNotPanic)
+		})
+
+		Convey("An empty index deletes the meta for that repo", func() {
+			var deleted string
+			imgStore := mocks.MockedImageStore{
+				GetIndexContentFn: func(repo string) ([]byte, error) { return emptyIndex, nil },
+			}
+			metaDB := mocks.MetaDBMock{
+				DeleteRepoMetaFn: func(repo string) error { deleted = repo; return nil },
+			}
+
+			deleteRepoMetaIfEmpty("repo", imgStore, metaDB, logger)
+			So(deleted, ShouldEqual, "repo")
+		})
+	})
+}