Allow falling back from partial pulls if the metadata is too large

... but not if the fallback would be convert_images, again
creating too large metadata.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>

Author: mtrmac

pkg/chunked/compression_linux.go

@@ -45,7 +45,7 @@ func typeToTarType(t string) (byte, error) {
 }
 
 // readEstargzChunkedManifest reads the estargz manifest from the seekable stream blobStream.
-// It may return an error matching ErrFallbackToOrdinaryLayerDownload.
+// It may return an error matching ErrFallbackToOrdinaryLayerDownload / errFallbackCanConvert.
 func readEstargzChunkedManifest(blobStream ImageSourceSeekable, blobSize int64, tocDigest digest.Digest) ([]byte, int64, error) {
 	// information on the format here https://github.com/containerd/stargz-snapshotter/blob/main/docs/stargz-estargz.md
 	footerSize := int64(51)
@@ -58,7 +58,7 @@ func readEstargzChunkedManifest(blobStream ImageSourceSeekable, blobSize int64,
 	if err != nil {
 		var badRequestErr ErrBadRequest
 		if errors.As(err, &badRequestErr) {
-			err = newErrFallbackToOrdinaryLayerDownload(err)
+			err = errFallbackCanConvert{newErrFallbackToOrdinaryLayerDownload(err)}
 		}
 		return nil, 0, err
 	}
@@ -90,14 +90,15 @@ func readEstargzChunkedManifest(blobStream ImageSourceSeekable, blobSize int64,
 	size := int64(blobSize - footerSize - tocOffset)
 	// set a reasonable limit
 	if size > maxTocSize {
-		return nil, 0, errors.New("manifest too big")
+		// Not errFallbackCanConvert: we would still use too much memory.
+		return nil, 0, newErrFallbackToOrdinaryLayerDownload(fmt.Errorf("estargz manifest too big to process in memory (%d bytes)", size))
 	}
 
 	streamsOrErrors, err = getBlobAt(blobStream, ImageSourceChunk{Offset: uint64(tocOffset), Length: uint64(size)})
 	if err != nil {
 		var badRequestErr ErrBadRequest
 		if errors.As(err, &badRequestErr) {
-			err = newErrFallbackToOrdinaryLayerDownload(err)
+			err = errFallbackCanConvert{newErrFallbackToOrdinaryLayerDownload(err)}
 		}
 		return nil, 0, err
 	}
@@ -158,7 +159,7 @@ func readEstargzChunkedManifest(blobStream ImageSourceSeekable, blobSize int64,
 
 // readZstdChunkedManifest reads the zstd:chunked manifest from the seekable stream blobStream.
 // Returns (manifest blob, parsed manifest, tar-split blob or nil, manifest offset).
-// It may return an error matching ErrFallbackToOrdinaryLayerDownload.
+// It may return an error matching ErrFallbackToOrdinaryLayerDownload / errFallbackCanConvert.
 func readZstdChunkedManifest(blobStream ImageSourceSeekable, tocDigest digest.Digest, annotations map[string]string) (_ []byte, _ *minimal.TOC, _ []byte, _ int64, retErr error) {
 	offsetMetadata := annotations[minimal.ManifestInfoKey]
 	if offsetMetadata == "" {
@@ -184,10 +185,12 @@ func readZstdChunkedManifest(blobStream ImageSourceSeekable, tocDigest digest.Di
 
 	// set a reasonable limit
 	if manifestChunk.Length > maxTocSize {
-		return nil, nil, nil, 0, errors.New("manifest too big")
+		// Not errFallbackCanConvert: we would still use too much memory.
+		return nil, nil, nil, 0, newErrFallbackToOrdinaryLayerDownload(fmt.Errorf("zstd:chunked manifest too big to process in memory (%d bytes compressed)", manifestChunk.Length))
 	}
 	if manifestLengthUncompressed > maxTocSize {
-		return nil, nil, nil, 0, errors.New("manifest too big")
+		// Not errFallbackCanConvert: we would still use too much memory.
+		return nil, nil, nil, 0, newErrFallbackToOrdinaryLayerDownload(fmt.Errorf("zstd:chunked manifest too big to process in memory (%d bytes uncompressed)", manifestLengthUncompressed))
 	}
 
 	chunks := []ImageSourceChunk{manifestChunk}
@@ -199,7 +202,7 @@ func readZstdChunkedManifest(blobStream ImageSourceSeekable, tocDigest digest.Di
 	if err != nil {
 		var badRequestErr ErrBadRequest
 		if errors.As(err, &badRequestErr) {
-			err = newErrFallbackToOrdinaryLayerDownload(err)
+			err = errFallbackCanConvert{newErrFallbackToOrdinaryLayerDownload(err)}
 		}
 		return nil, nil, nil, 0, err
 	}

pkg/chunked/storage_linux.go

@@ -225,13 +225,38 @@ func GetDiffer(ctx context.Context, store storage.Store, blobDigest digest.Diges
 		if !pullOptions.convertImages {
 			return nil, err
 		}
+		var canConvertErr errFallbackCanConvert
+		if !errors.As(err, &canConvertErr) {
+			// We are supposed to use makeConvertFromRawDiffer, but that would not work.
+			// Fail, and make sure the error does _not_ match ErrFallbackToOrdinaryLayerDownload: use only the error text,
+			// discard all type information.
+			return nil, fmt.Errorf("neither a partial pull nor convert_images is possible: %s", err.Error())
+		}
 		logrus.Debugf("Created differ to convert blob %q", blobDigest)
 		return makeConvertFromRawDiffer(store, blobDigest, blobSize, iss, pullOptions)
 	}
 
 	return differ, nil
 }
 
+// errFallbackCanConvert is an an error type _accompanying_ ErrFallbackToOrdinaryLayerDownload
+// within getProperDiffer, to mark that using makeConvertFromRawDiffer makes sense.
+// This is used to distinguish between cases where the environment does not support partial pulls
+// (e.g. a registry does not support range requests) and convert_images is still possible,
+// from cases where the image content is unacceptable for partial pulls (e.g. exceeds memory limits)
+// and convert_images would not help.
+type errFallbackCanConvert struct {
+	err error
+}
+
+func (e errFallbackCanConvert) Error() string {
+	return e.err.Error()
+}
+
+func (e errFallbackCanConvert) Unwrap() error {
+	return e.err
+}
+
 // getProperDiffer is an implementation detail of GetDiffer.
 // It returns a “proper” differ (not a convert_images one) if possible.
 // May return an error matching ErrFallbackToOrdinaryLayerDownload if a fallback to an alternative
@@ -271,10 +296,13 @@ func getProperDiffer(store storage.Store, blobDigest digest.Digest, blobSize int
 		return differ, nil
 
 	default: // no TOC
+		message := "no TOC found"
 		if !pullOptions.convertImages {
-			return nil, newErrFallbackToOrdinaryLayerDownload(errors.New("no TOC found and convert_images is not configured"))
+			message = "no TOC found and convert_images is not configured"
+		}
+		return nil, errFallbackCanConvert{
+			newErrFallbackToOrdinaryLayerDownload(errors.New(message)),
 		}
-		return nil, newErrFallbackToOrdinaryLayerDownload(errors.New("no TOC found"))
 	}
 }
 
@@ -301,10 +329,10 @@ func makeConvertFromRawDiffer(store storage.Store, blobDigest digest.Digest, blo
 }
 
 // makeZstdChunkedDiffer sets up a chunkedDiffer for a zstd:chunked layer.
-// It may return an error matching ErrFallbackToOrdinaryLayerDownload.
+// It may return an error matching ErrFallbackToOrdinaryLayerDownload / errFallbackCanConvert.
 func makeZstdChunkedDiffer(store storage.Store, blobSize int64, tocDigest digest.Digest, annotations map[string]string, iss ImageSourceSeekable, pullOptions pullOptions) (*chunkedDiffer, error) {
 	manifest, toc, tarSplit, tocOffset, err := readZstdChunkedManifest(iss, tocDigest, annotations)
-	if err != nil { // May be ErrFallbackToOrdinaryLayerDownload
+	if err != nil { // May be ErrFallbackToOrdinaryLayerDownload / errFallbackCanConvert
 		return nil, fmt.Errorf("read zstd:chunked manifest: %w", err)
 	}
 
@@ -315,7 +343,9 @@ func makeZstdChunkedDiffer(store storage.Store, blobSize int64, tocDigest digest
 			return nil, fmt.Errorf("computing size from tar-split: %w", err)
 		}
 	} else if !pullOptions.insecureAllowUnpredictableImageContents { // With no tar-split, we can't compute the traditional UncompressedDigest.
-		return nil, newErrFallbackToOrdinaryLayerDownload(fmt.Errorf("zstd:chunked layers without tar-split data don't support partial pulls with guaranteed consistency with non-partial pulls"))
+		return nil, errFallbackCanConvert{
+			newErrFallbackToOrdinaryLayerDownload(fmt.Errorf("zstd:chunked layers without tar-split data don't support partial pulls with guaranteed consistency with non-partial pulls")),
+		}
 	}
 
 	layersCache, err := getLayersCache(store)
@@ -344,14 +374,16 @@ func makeZstdChunkedDiffer(store storage.Store, blobSize int64, tocDigest digest
 }
 
 // makeEstargzChunkedDiffer sets up a chunkedDiffer for an estargz layer.
-// It may return an error matching ErrFallbackToOrdinaryLayerDownload.
+// It may return an error matching ErrFallbackToOrdinaryLayerDownload / errFallbackCanConvert.
 func makeEstargzChunkedDiffer(store storage.Store, blobSize int64, tocDigest digest.Digest, iss ImageSourceSeekable, pullOptions pullOptions) (*chunkedDiffer, error) {
 	if !pullOptions.insecureAllowUnpredictableImageContents { // With no tar-split, we can't compute the traditional UncompressedDigest.
-		return nil, newErrFallbackToOrdinaryLayerDownload(fmt.Errorf("estargz layers don't support partial pulls with guaranteed consistency with non-partial pulls"))
+		return nil, errFallbackCanConvert{
+			newErrFallbackToOrdinaryLayerDownload(fmt.Errorf("estargz layers don't support partial pulls with guaranteed consistency with non-partial pulls")),
+		}
 	}
 
 	manifest, tocOffset, err := readEstargzChunkedManifest(iss, blobSize, tocDigest)
-	if err != nil { // May be ErrFallbackToOrdinaryLayerDownload
+	if err != nil { // May be ErrFallbackToOrdinaryLayerDownload / errFallbackCanConvert
 		return nil, fmt.Errorf("read zstd:chunked manifest: %w", err)
 	}
 	layersCache, err := getLayersCache(store)