Support ReplaceOp in Signatures (#3315)

Before this change, calling WithReplaceOp didn't do anything for signatures and only affected attestations. I believe this is because the implementation of Dedupe and Replace was copied to 6 different methods, so they weren't getting updated. I've extracted the core of the impl into a dedupeAndReplace method on opts, which makes each impl just a one line call with either signatures or attestations passed in. Signed-off-by: Jon Johnson <jon.johnson@chainguard.dev>
sigstore · Oct 29, 2023 · 9400476 · 9400476
1 parent 67558de
commit 9400476
Showing 1 changed file with 24 additions and 104 deletions.
diff --git a/pkg/oci/mutate/mutate.go b/pkg/oci/mutate/mutate.go
@@ -204,47 +204,12 @@ type signedImage struct {
 
 // Signatures implements oci.SignedImage
 func (si *signedImage) Signatures() (oci.Signatures, error) {
-	base, err := si.SignedImage.Signatures()
-	if err != nil {
-		return nil, err
-	} else if si.sig == nil {
-		return base, nil
-	}
-	if si.so.dd != nil {
-		if existing, err := si.so.dd.Find(base, si.sig); err != nil {
-			return nil, err
-		} else if existing != nil {
-			// Just return base if the signature is redundant
-			return base, nil
-		}
-	}
-	return AppendSignatures(base, si.sig)
+	return si.so.dedupeAndReplace(si.sig, si.SignedImage.Signatures)
 }
 
 // Attestations implements oci.SignedImage
 func (si *signedImage) Attestations() (oci.Signatures, error) {
-	base, err := si.SignedImage.Attestations()
-	if err != nil {
-		return nil, err
-	} else if si.att == nil {
-		return base, nil
-	}
-	if si.so.dd != nil {
-		if existing, err := si.so.dd.Find(base, si.att); err != nil {
-			return nil, err
-		} else if existing != nil {
-			// Just return base if the signature is redundant
-			return base, nil
-		}
-	}
-	if si.so.ro != nil {
-		replace, err := si.so.ro.Replace(base, si.att)
-		if err != nil {
-			return nil, err
-		}
-		return ReplaceSignatures(replace)
-	}
-	return AppendSignatures(base, si.att)
+	return si.so.dedupeAndReplace(si.att, si.SignedImage.Attestations)
 }
 
 // Attachment implements oci.SignedImage
@@ -298,47 +263,12 @@ type signedImageIndex struct {
 
 // Signatures implements oci.SignedImageIndex
 func (sii *signedImageIndex) Signatures() (oci.Signatures, error) {
-	base, err := sii.ociSignedImageIndex.Signatures()
-	if err != nil {
-		return nil, err
-	} else if sii.sig == nil {
-		return base, nil
-	}
-	if sii.so.dd != nil {
-		if existing, err := sii.so.dd.Find(base, sii.sig); err != nil {
-			return nil, err
-		} else if existing != nil {
-			// Just return base if the signature is redundant
-			return base, nil
-		}
-	}
-	return AppendSignatures(base, sii.sig)
+	return sii.so.dedupeAndReplace(sii.sig, sii.ociSignedImageIndex.Signatures)
 }
 
 // Attestations implements oci.SignedImageIndex
 func (sii *signedImageIndex) Attestations() (oci.Signatures, error) {
-	base, err := sii.ociSignedImageIndex.Attestations()
-	if err != nil {
-		return nil, err
-	} else if sii.att == nil {
-		return base, nil
-	}
-	if sii.so.dd != nil {
-		if existing, err := sii.so.dd.Find(base, sii.att); err != nil {
-			return nil, err
-		} else if existing != nil {
-			// Just return base if the signature is redundant
-			return base, nil
-		}
-	}
-	if sii.so.ro != nil {
-		replace, err := sii.so.ro.Replace(base, sii.att)
-		if err != nil {
-			return nil, err
-		}
-		return ReplaceSignatures(replace)
-	}
-	return AppendSignatures(base, sii.att)
+	return sii.so.dedupeAndReplace(sii.att, sii.ociSignedImageIndex.Attestations)
 }
 
 // Attachment implements oci.SignedImageIndex
@@ -403,53 +333,43 @@ func (si *signedUnknown) Digest() (v1.Hash, error) {
 
 // Signatures implements oci.SignedEntity
 func (si *signedUnknown) Signatures() (oci.Signatures, error) {
-	base, err := si.SignedEntity.Signatures()
-	if err != nil {
-		return nil, err
-	} else if si.sig == nil {
-		return base, nil
-	}
-	if si.so.dd != nil {
-		if existing, err := si.so.dd.Find(base, si.sig); err != nil {
-			return nil, err
-		} else if existing != nil {
-			// Just return base if the signature is redundant
-			return base, nil
-		}
-	}
-	return AppendSignatures(base, si.sig)
+	return si.so.dedupeAndReplace(si.sig, si.SignedEntity.Signatures)
 }
 
 // Attestations implements oci.SignedEntity
 func (si *signedUnknown) Attestations() (oci.Signatures, error) {
-	base, err := si.SignedEntity.Attestations()
+	return si.so.dedupeAndReplace(si.att, si.SignedEntity.Attestations)
+}
+
+// Attachment implements oci.SignedEntity
+func (si *signedUnknown) Attachment(attName string) (oci.File, error) {
+	if f, ok := si.attachments[attName]; ok {
+		return f, nil
+	}
+	return nil, fmt.Errorf("attachment %q not found", attName)
+}
+
+func (so *signOpts) dedupeAndReplace(sig oci.Signature, basefn func() (oci.Signatures, error)) (oci.Signatures, error) {
+	base, err := basefn()
 	if err != nil {
 		return nil, err
-	} else if si.att == nil {
+	} else if sig == nil {
 		return base, nil
 	}
-	if si.so.dd != nil {
-		if existing, err := si.so.dd.Find(base, si.att); err != nil {
+	if so.dd != nil {
+		if existing, err := so.dd.Find(base, sig); err != nil {
 			return nil, err
 		} else if existing != nil {
 			// Just return base if the signature is redundant
 			return base, nil
 		}
 	}
-	if si.so.ro != nil {
-		replace, err := si.so.ro.Replace(base, si.att)
+	if so.ro != nil {
+		replace, err := so.ro.Replace(base, sig)
 		if err != nil {
 			return nil, err
 		}
 		return ReplaceSignatures(replace)
 	}
-	return AppendSignatures(base, si.att)
-}
-
-// Attachment implements oci.SignedEntity
-func (si *signedUnknown) Attachment(attName string) (oci.File, error) {
-	if f, ok := si.attachments[attName]; ok {
-		return f, nil
-	}
-	return nil, fmt.Errorf("attachment %q not found", attName)
+	return AppendSignatures(base, sig)
 }