fix: push an empty index when deleting the last referrer with SkipRef…

…errersGC (#593) Fix: #592 Signed-off-by: Lixia (Sylvia) Lei <[email protected]>
oras-project · Oct 20, 2023 · bfef8c5 · bfef8c5
1 parent f570766
commit bfef8c5
Show file tree

Hide file tree

Showing 2 changed files with 572 additions and 64 deletions.
diff --git a/registry/remote/repository.go b/registry/remote/repository.go
@@ -1425,26 +1425,25 @@ func (s *manifestStore) indexReferrersForPush(ctx context.Context, desc ocispec.
 func (s *manifestStore) updateReferrersIndex(ctx context.Context, subject ocispec.Descriptor, change referrerChange) (err error) {
 	referrersTag := buildReferrersTag(subject)
 
-	skipDelete := s.repo.SkipReferrersGC
-	var oldIndexDesc ocispec.Descriptor
-	var referrers []ocispec.Descriptor
+	var oldIndexDesc *ocispec.Descriptor
+	var oldReferrers []ocispec.Descriptor
 	prepare := func() error {
 		// 1. pull the original referrers list using the referrers tag schema
-		var err error
-		oldIndexDesc, referrers, err = s.repo.referrersFromIndex(ctx, referrersTag)
+		indexDesc, referrers, err := s.repo.referrersFromIndex(ctx, referrersTag)
 		if err != nil {
 			if errors.Is(err, errdef.ErrNotFound) {
-				// no old index found, skip delete
-				skipDelete = true
+				// valid case: no old referrers index
 				return nil
 			}
 			return err
 		}
+		oldIndexDesc = &indexDesc
+		oldReferrers = referrers
 		return nil
 	}
 	update := func(referrerChanges []referrerChange) error {
 		// 2. apply the referrer changes on the referrers list
-		updatedReferrers, err := applyReferrerChanges(referrers, referrerChanges)
+		updatedReferrers, err := applyReferrerChanges(oldReferrers, referrerChanges)
 		if err != nil {
 			if err == errNoReferrerUpdate {
 				return nil
@@ -1453,7 +1452,12 @@ func (s *manifestStore) updateReferrersIndex(ctx context.Context, subject ocispe
 		}
 
 		// 3. push the updated referrers list using referrers tag schema
-		if len(updatedReferrers) > 0 {
+		if len(updatedReferrers) > 0 || s.repo.SkipReferrersGC {
+			// push a new index in either case:
+			// 1. the referrers list has been updated with a non-zero size
+			// 2. OR the updated referrers list is empty but referrers GC
+			//    is skipped, in this case an empty index should still be pushed
+			//    as the old index won't get deleted
 			newIndexDesc, newIndex, err := generateIndex(updatedReferrers)
 			if err != nil {
 				return fmt.Errorf("failed to generate referrers index for referrers tag %s: %w", referrersTag, err)
@@ -1463,14 +1467,15 @@ func (s *manifestStore) updateReferrersIndex(ctx context.Context, subject ocispe
 			}
 		}
 
-		// 4. delete the dangling original referrers index
-		if !skipDelete {
-			if err := s.repo.delete(ctx, oldIndexDesc, true); err != nil {
-				return &ReferrersError{
-					Op:      opDeleteReferrersIndex,
-					Err:     fmt.Errorf("failed to delete dangling referrers index %s for referrers tag %s: %w", oldIndexDesc.Digest.String(), referrersTag, err),
-					Subject: subject,
-				}
+		// 4. delete the dangling original referrers index, if applicable
+		if s.repo.SkipReferrersGC || oldIndexDesc == nil {
+			return nil
+		}
+		if err := s.repo.delete(ctx, *oldIndexDesc, true); err != nil {
+			return &ReferrersError{
+				Op:      opDeleteReferrersIndex,
+				Err:     fmt.Errorf("failed to delete dangling referrers index %s for referrers tag %s: %w", oldIndexDesc.Digest.String(), referrersTag, err),
+				Subject: subject,
 			}
 		}
 		return nil