Cache the parsed DiffID values from the config

... so that we parse them at most once per image, not once
per layer.

Should not change behavior (apart from rewording one error message).

Signed-off-by: Miloslav Trmač <[email protected]>

storage/storage_dest.go

@@ -54,14 +54,15 @@ type storageImageDestination struct {
 	stubs.ImplementsPutBlobPartial
 	stubs.AlwaysSupportsSignatures
 
-	imageRef       storageReference
-	directory      string                   // Temporary directory where we store blobs until Commit() time
-	nextTempFileID atomic.Int32             // A counter that we use for computing filenames to assign to blobs
-	manifest       []byte                   // Manifest contents, temporary
-	manifestDigest digest.Digest            // Valid if len(manifest) != 0
-	signatures     []byte                   // Signature contents, temporary
-	signatureses   map[digest.Digest][]byte // Instance signature contents, temporary
-	metadata       storageImageMetadata     // Metadata contents being built
+	imageRef              storageReference
+	directory             string                   // Temporary directory where we store blobs until Commit() time
+	nextTempFileID        atomic.Int32             // A counter that we use for computing filenames to assign to blobs
+	manifest              []byte                   // Manifest contents, temporary
+	manifestDigest        digest.Digest            // Valid if len(manifest) != 0
+	untrustedDiffIDValues []digest.Digest          // From config’s RootFS.DiffIDs, valid if not nil
+	signatures            []byte                   // Signature contents, temporary
+	signatureses          map[digest.Digest][]byte // Instance signature contents, temporary
+	metadata              storageImageMetadata     // Metadata contents being built
 
 	// Mapping from layer (by index) to the associated ID in the storage.
 	// It's protected *implicitly* since `commitLayer()`, at any given
@@ -925,40 +926,49 @@ func (s *storageImageDestination) createNewLayer(index int, layerDigest digest.D
 // If the value is not yet available (but it can be available after s.manifets is set), it returns ("", nil).
 // WARNING: We don’t validate the DiffID value against the layer contents; it must not be used for any deduplication.
 func (s *storageImageDestination) untrustedLayerDiffID(layerIndex int) (digest.Digest, error) {
+	// At this point, we are either inside the multi-threaded scope of HasThreadSafePutBlob, and
+	// nothing is writing to s.manifest yet, or PutManifest has been called and s.manifest != nil.
+	// Either way this function does not need the protection of s.lock.
 	if s.manifest == nil {
 		logrus.Debugf("Skipping commit for layer %d, manifest not yet available", layerIndex)
 		return "", nil
 	}
 
-	mt := manifest.GuessMIMEType(s.manifest)
-	if mt != imgspecv1.MediaTypeImageManifest {
-		// We could, in principle, build an ImageSource, support arbitrary image formats using image.FromUnparsedImage,
-		// and then use types.Image.OCIConfig so that we can parse the image.
-		//
-		// In practice, this should, right now, only matter for pulls of OCI images (this code path implies that a layer has annotation),
-		// while converting to a non-OCI formats, using a manual (skopeo copy) or something similar, not (podman pull).
-		// So it is not implemented yet.
-		return "", fmt.Errorf("determining DiffID for manifest type %q is not yet supported", mt)
-	}
-	man, err := manifest.FromBlob(s.manifest, mt)
-	if err != nil {
-		return "", fmt.Errorf("parsing manifest: %w", err)
-	}
+	if s.untrustedDiffIDValues == nil {
+		mt := manifest.GuessMIMEType(s.manifest)
+		if mt != imgspecv1.MediaTypeImageManifest {
+			// We could, in principle, build an ImageSource, support arbitrary image formats using image.FromUnparsedImage,
+			// and then use types.Image.OCIConfig so that we can parse the image.
+			//
+			// In practice, this should, right now, only matter for pulls of OCI images (this code path implies that a layer has annotation),
+			// while converting to a non-OCI formats, using a manual (skopeo copy) or something similar, not (podman pull).
+			// So it is not implemented yet.
+			return "", fmt.Errorf("determining DiffID for manifest type %q is not yet supported", mt)
+		}
+		man, err := manifest.FromBlob(s.manifest, mt)
+		if err != nil {
+			return "", fmt.Errorf("parsing manifest: %w", err)
+		}
 
-	cb, err := s.getConfigBlob(man.ConfigInfo())
-	if err != nil {
-		return "", err
-	}
+		cb, err := s.getConfigBlob(man.ConfigInfo())
+		if err != nil {
+			return "", err
+		}
 
-	// retrieve the expected uncompressed digest from the config blob.
-	configOCI := &imgspecv1.Image{}
-	if err := json.Unmarshal(cb, configOCI); err != nil {
-		return "", err
+		// retrieve the expected uncompressed digest from the config blob.
+		configOCI := &imgspecv1.Image{}
+		if err := json.Unmarshal(cb, configOCI); err != nil {
+			return "", err
+		}
+		s.untrustedDiffIDValues = slices.Clone(configOCI.RootFS.DiffIDs)
+		if s.untrustedDiffIDValues == nil { // Unlikely but possible in theory…
+			s.untrustedDiffIDValues = []digest.Digest{}
+		}
 	}
-	if layerIndex >= len(configOCI.RootFS.DiffIDs) {
-		return "", fmt.Errorf("index %d out of range for configOCI.RootFS.DiffIDs", layerIndex)
+	if layerIndex >= len(s.untrustedDiffIDValues) {
+		return "", fmt.Errorf("image config has only %d DiffID values, but a layer with index %d exists", len(s.untrustedDiffIDValues), layerIndex)
 	}
-	return configOCI.RootFS.DiffIDs[layerIndex], nil
+	return s.untrustedDiffIDValues[layerIndex], nil
 }
 
 // Commit marks the process of storing the image as successful and asks for the image to be persisted.