Don't store old signature manifest

Signed-off-by: Ayato Tokubi <[email protected]>

Author: bitoku

image/oci/layout/oci_dest.go

@@ -313,6 +313,33 @@ func (d *ociImageDestination) addManifest(desc *imgspecv1.Descriptor) {
 	d.index.Manifests = append(slices.Clone(d.index.Manifests), *desc)
 }
 
+// addSignatureManifest is similar to addManifest, but replace the entry based on imgspecv1.AnnotationRefName
+// and returns the old digest to delete it later.
+func (d *ociImageDestination) addSignatureManifest(desc *imgspecv1.Descriptor) (*imgspecv1.Descriptor, error) {
+	if desc.Annotations == nil || desc.Annotations[imgspecv1.AnnotationRefName] == "" {
+		return nil, errors.New("cannot add signature manifest without ref.name")
+	}
+	for i, m := range d.index.Manifests {
+		if m.Annotations[imgspecv1.AnnotationRefName] == desc.Annotations[imgspecv1.AnnotationRefName] {
+			// Replace it completely.
+			oldDesc := d.index.Manifests[i]
+			d.index.Manifests[i] = *desc
+			return &oldDesc, nil
+		}
+	}
+	// It shouldn't happen, but if there's no entry with the same ref name, but the same digest, just replace it.
+	for i, m := range d.index.Manifests {
+		if m.Digest == desc.Digest && m.Annotations[imgspecv1.AnnotationRefName] == "" {
+			// Replace it completely.
+			d.index.Manifests[i] = *desc
+			return nil, nil
+		}
+	}
+	// It's a new entry to be added to the index. Use slices.Clone() to avoid a remote dependency on how d.index was created.
+	d.index.Manifests = append(slices.Clone(d.index.Manifests), *desc)
+	return nil, nil
+}
+
 // CommitWithOptions marks the process of storing the image as successful and asks for the image to be persisted.
 // WARNING: This does not have any transactional semantics:
 // - Uploaded data MAY be visible to others before CommitWithOptions() is called
@@ -324,7 +351,7 @@ func (d *ociImageDestination) CommitWithOptions(ctx context.Context, options pri
 	if err != nil {
 		return err
 	}
-	// Delete unreferenced blobs (e.g. old signature manifest config)
+	// Delete unreferenced blobs (e.g. old signature manifest and its config)
 	if !d.blobsToDelete.Empty() {
 		count := make(map[digest.Digest]int)
 		err = d.ref.countBlobsReferencedByIndex(count, &d.index, d.sharedBlobDir)
@@ -471,15 +498,26 @@ func (d *ociImageDestination) putSignaturesToSigstoreAttachment(ctx context.Cont
 	if err != nil {
 		return err
 	}
-	d.addManifest(&imgspecv1.Descriptor{
+	oldDesc, err := d.addSignatureManifest(&imgspecv1.Descriptor{
 		MediaType: signManifest.MediaType,
 		Digest:    signDigest,
 		Size:      int64(len(signManifestBlob)),
 		Annotations: map[string]string{
 			imgspecv1.AnnotationRefName: signTag,
 		},
 	})
-
+	if err != nil {
+		return err
+	}
+	// If it overwrote an existing signature manifest, delete blobs referenced by the old manifest.
+	if oldDesc != nil {
+		referencedBlobs := make(map[digest.Digest]int)
+		err = d.ref.countBlobsForDescriptor(referencedBlobs, oldDesc, d.sharedBlobDir)
+		if err != nil {
+			return fmt.Errorf("error counting blobs for digest %s: %w", oldDesc.Digest.String(), err)
+		}
+		d.blobsToDelete.AddSeq(maps.Keys(referencedBlobs))
+	}
 	return nil
 }
 

image/oci/layout/oci_dest_test.go

@@ -301,6 +301,7 @@ func TestPutSignaturesWithFormatTwice(t *testing.T) {
 	require.NoError(t, err)
 	require.Len(t, sign, 2)
 	require.Equal(t, sigstoreSign, sign[0])
+	require.Equal(t, sigstoreSign2, sign[1])
 }
 
 // TestPutSignaturesWithFormatNilDigest tests error handling when instanceDigest is nil