Refactor signature handling in putSignaturesToSigstoreAttachment and getSigstoreAttachmentManifest

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

Author: bitoku

image/oci/layout/oci_dest.go

@@ -315,33 +315,6 @@ 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
@@ -409,9 +382,9 @@ func (d *ociImageDestination) PutSignaturesWithFormat(ctx context.Context, signa
 }
 
 func (d *ociImageDestination) putSignaturesToSigstoreAttachment(ctx context.Context, signatures []signature.Sigstore, manifestDigest digest.Digest) error {
-	var signConfig imgspecv1.Image // Most fields empty by default
-
-	signManifest, err := d.ref.getSigstoreAttachmentManifest(manifestDigest, &d.index, d.sharedBlobDir)
+	var signConfig imgspecv1.Image     // Most fields empty by default
+	var oldConfigDigest *digest.Digest // It is used for cleanup when updated
+	signManifest, signDesc, err := d.ref.getSigstoreAttachmentManifest(manifestDigest, &d.index, d.sharedBlobDir)
 	if err != nil {
 		return err
 	}
@@ -423,6 +396,7 @@ func (d *ociImageDestination) putSignaturesToSigstoreAttachment(ctx context.Cont
 		}, nil)
 		signConfig.RootFS.Type = "layers"
 	} else {
+		oldConfigDigest = &signManifest.Config.Digest
 		logrus.Debugf("Fetching sigstore attachment config %s", signManifest.Config.Digest.String())
 		configBlob, err := d.ref.getOCIDescriptorContents(signManifest.Config.Digest, iolimits.MaxConfigBodySize, d.sharedBlobDir)
 		if err != nil {
@@ -500,25 +474,17 @@ func (d *ociImageDestination) putSignaturesToSigstoreAttachment(ctx context.Cont
 	if err != nil {
 		return err
 	}
-	oldDesc, err := d.addSignatureManifest(&imgspecv1.Descriptor{
+	d.addManifest(&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.blobDeleteCandidates.AddSeq(maps.Keys(referencedBlobs))
+	if signDesc != nil && oldConfigDigest != nil {
+		d.blobDeleteCandidates.Add(signDesc.Digest)
+		d.blobDeleteCandidates.Add(*oldConfigDigest)
 	}
 	return nil
 }

image/oci/layout/oci_src.go

@@ -240,7 +240,7 @@ func (s *ociImageSource) GetSignaturesWithFormat(ctx context.Context, instanceDi
 		instanceDigest = &s.descriptor.Digest
 	}
 
-	ociManifest, err := s.ref.getSigstoreAttachmentManifest(*instanceDigest, s.index, s.sharedBlobDir)
+	ociManifest, _, err := s.ref.getSigstoreAttachmentManifest(*instanceDigest, s.index, s.sharedBlobDir)
 	if err != nil {
 		return nil, err
 	}

image/oci/layout/oci_transport.go

@@ -338,10 +338,10 @@ func sigstoreAttachmentTag(d digest.Digest) (string, error) {
 	return strings.Replace(d.String(), ":", "-", 1) + ".sig", nil
 }
 
-func (ref ociReference) getSigstoreAttachmentManifest(d digest.Digest, idx *imgspecv1.Index, sharedBlobDir string) (*manifest.OCI1, error) {
+func (ref ociReference) getSigstoreAttachmentManifest(d digest.Digest, idx *imgspecv1.Index, sharedBlobDir string) (*manifest.OCI1, *imgspecv1.Descriptor, error) {
 	signTag, err := sigstoreAttachmentTag(d)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 	var signDesc *imgspecv1.Descriptor
 	for _, m := range idx.Manifests {
@@ -352,26 +352,26 @@ func (ref ociReference) getSigstoreAttachmentManifest(d digest.Digest, idx *imgs
 	}
 	if signDesc == nil {
 		// No signature found
-		return nil, nil
+		return nil, nil, nil
 	}
 	if signDesc.MediaType != imgspecv1.MediaTypeImageManifest {
-		return nil, fmt.Errorf("unexpected MIME type for sigstore attachment manifest %s: %q",
+		return nil, nil, fmt.Errorf("unexpected MIME type for sigstore attachment manifest %s: %q",
 			signTag, signDesc.MediaType)
 	}
 	blobReader, _, err := ref.getBlob(signDesc.Digest, sharedBlobDir)
 	if err != nil {
-		return nil, fmt.Errorf("failed to get Blob %s: %w", signTag, err)
+		return nil, nil, fmt.Errorf("failed to get Blob %s: %w", signTag, err)
 	}
 	defer blobReader.Close()
 	signBlob, err := iolimits.ReadAtMost(blobReader, iolimits.MaxManifestBodySize)
 	if err != nil {
-		return nil, fmt.Errorf("failed to read blob: %w", err)
+		return nil, nil, fmt.Errorf("failed to read blob: %w", err)
 	}
 	res, err := manifest.OCI1FromManifest(signBlob)
 	if err != nil {
-		return nil, fmt.Errorf("parsing manifest %s: %w", signDesc.Digest, err)
+		return nil, nil, fmt.Errorf("parsing manifest %s: %w", signDesc.Digest, err)
 	}
-	return res, nil
+	return res, signDesc, nil
 }
 
 func (ref ociReference) getBlob(d digest.Digest, sharedBlobDir string) (io.ReadCloser, int64, error) {