diff --git a/copy/single.go b/copy/single.go index 9e892943b4..4c363d4bac 100644 --- a/copy/single.go +++ b/copy/single.go @@ -695,11 +695,16 @@ func (ic *imageCopier) copyLayer(ctx context.Context, srcInfo types.BlobInfo, to requiredCompression = ic.compressionFormat } + var tocDigest digest.Digest + // Check if we have a chunked layer in storage that's based on that blob. These layers are stored by their TOC digest. - tocDigest, err := chunkedToc.GetTOCDigest(srcInfo.Annotations) + d, err := chunkedToc.GetTOCDigest(srcInfo.Annotations) if err != nil { return types.BlobInfo{}, "", err } + if d != nil { + tocDigest = *d + } reused, reusedBlob, err := ic.c.dest.TryReusingBlobWithOptions(ctx, srcInfo, private.TryReusingBlobOptions{ Cache: ic.c.blobInfoCache, @@ -718,7 +723,11 @@ func (ic *imageCopier) copyLayer(ctx context.Context, srcInfo types.BlobInfo, to if reused { logrus.Debugf("Skipping blob %s (already present):", srcInfo.Digest) func() { // A scope for defer - bar := ic.c.createProgressBar(pool, false, types.BlobInfo{Digest: reusedBlob.Digest, Size: 0}, "blob", "skipped: already exists") + label := "skipped: already exists" + if reusedBlob.MatchedByTOCDigest { + label = "skipped: already exists (found by TOC)" + } + bar := ic.c.createProgressBar(pool, false, types.BlobInfo{Digest: reusedBlob.Digest, Size: 0}, "blob", label) defer bar.Abort(false) bar.mark100PercentComplete() }() @@ -751,7 +760,10 @@ func (ic *imageCopier) copyLayer(ctx context.Context, srcInfo types.BlobInfo, to wrapped: ic.c.rawSource, bar: bar, } - uploadedBlob, err := ic.c.dest.PutBlobPartial(ctx, &proxy, srcInfo, ic.c.blobInfoCache) + uploadedBlob, err := ic.c.dest.PutBlobPartial(ctx, &proxy, srcInfo, private.PutBlobPartialOptions{ + Cache: ic.c.blobInfoCache, + LayerIndex: layerIndex, + }) if err == nil { if srcInfo.Size != -1 { refill := srcInfo.Size - bar.Current() diff --git a/internal/imagedestination/stubs/put_blob_partial.go b/internal/imagedestination/stubs/put_blob_partial.go index 0dc6bd5af7..bbb53c198f 100644 --- a/internal/imagedestination/stubs/put_blob_partial.go +++ b/internal/imagedestination/stubs/put_blob_partial.go @@ -4,7 +4,6 @@ import ( "context" "fmt" - "github.com/containers/image/v5/internal/blobinfocache" "github.com/containers/image/v5/internal/private" "github.com/containers/image/v5/types" ) @@ -39,7 +38,7 @@ func (stub NoPutBlobPartialInitialize) SupportsPutBlobPartial() bool { // It is available only if SupportsPutBlobPartial(). // Even if SupportsPutBlobPartial() returns true, the call can fail, in which case the caller // should fall back to PutBlobWithOptions. -func (stub NoPutBlobPartialInitialize) PutBlobPartial(ctx context.Context, chunkAccessor private.BlobChunkAccessor, srcInfo types.BlobInfo, cache blobinfocache.BlobInfoCache2) (private.UploadedBlob, error) { +func (stub NoPutBlobPartialInitialize) PutBlobPartial(ctx context.Context, chunkAccessor private.BlobChunkAccessor, srcInfo types.BlobInfo, options private.PutBlobPartialOptions) (private.UploadedBlob, error) { return private.UploadedBlob{}, fmt.Errorf("internal error: PutBlobPartial is not supported by the %q transport", stub.transportName) } diff --git a/internal/private/private.go b/internal/private/private.go index 7037755bfc..562adbea8d 100644 --- a/internal/private/private.go +++ b/internal/private/private.go @@ -55,7 +55,7 @@ type ImageDestinationInternalOnly interface { // It is available only if SupportsPutBlobPartial(). // Even if SupportsPutBlobPartial() returns true, the call can fail, in which case the caller // should fall back to PutBlobWithOptions. - PutBlobPartial(ctx context.Context, chunkAccessor BlobChunkAccessor, srcInfo types.BlobInfo, cache blobinfocache.BlobInfoCache2) (UploadedBlob, error) + PutBlobPartial(ctx context.Context, chunkAccessor BlobChunkAccessor, srcInfo types.BlobInfo, options PutBlobPartialOptions) (UploadedBlob, error) // TryReusingBlobWithOptions checks whether the transport already contains, or can efficiently reuse, a blob, and if so, applies it to the current destination // (e.g. if the blob is a filesystem layer, this signifies that the changes it describes need to be applied again when composing a filesystem tree). @@ -100,6 +100,12 @@ type PutBlobOptions struct { LayerIndex *int // If the blob is a layer, a zero-based index of the layer within the image; nil otherwise. } +// PutBlobPartialOptions are used in PutBlobPartial. +type PutBlobPartialOptions struct { + Cache blobinfocache.BlobInfoCache2 // Cache to use and/or update. + LayerIndex int // A zero-based index of the layer within the image (PutBlobPartial is only called with layer-like blobs, not configs) +} + // TryReusingBlobOptions are used in TryReusingBlobWithOptions. type TryReusingBlobOptions struct { Cache blobinfocache.BlobInfoCache2 // Cache to use and/or update. @@ -118,7 +124,7 @@ type TryReusingBlobOptions struct { PossibleManifestFormats []string // A set of possible manifest formats; at least one should support the reused layer blob. RequiredCompression *compression.Algorithm // If set, reuse blobs with a matching algorithm as per implementations in internal/imagedestination/impl.helpers.go OriginalCompression *compression.Algorithm // May be nil to indicate “uncompressed” or “unknown”. - TOCDigest *digest.Digest // If specified, the blob can be looked up in the destination also by its TOC digest. + TOCDigest digest.Digest // If specified, the blob can be looked up in the destination also by its TOC digest. } // ReusedBlob is information about a blob reused in a destination. @@ -130,6 +136,8 @@ type ReusedBlob struct { // a differently-compressed blob. CompressionOperation types.LayerCompression // Compress/Decompress, matching the reused blob; PreserveOriginal if N/A CompressionAlgorithm *compression.Algorithm // Algorithm if compressed, nil if decompressed or N/A + + MatchedByTOCDigest bool // Whether the layer was reused/matched by TOC digest. Used only for UI purposes. } // ImageSourceChunk is a portion of a blob. diff --git a/manifest/oci.go b/manifest/oci.go index 6d5acb45d8..548994ffaf 100644 --- a/manifest/oci.go +++ b/manifest/oci.go @@ -9,7 +9,6 @@ import ( compressiontypes "github.com/containers/image/v5/pkg/compression/types" "github.com/containers/image/v5/types" ociencspec "github.com/containers/ocicrypt/spec" - chunkedToc "github.com/containers/storage/pkg/chunked/toc" "github.com/opencontainers/go-digest" "github.com/opencontainers/image-spec/specs-go" imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" @@ -260,44 +259,9 @@ func (m *OCI1) ImageID(diffIDs []digest.Digest) (string, error) { if err := m.Config.Digest.Validate(); err != nil { return "", err } - - // If there is any layer that is using partial content, we calculate the image ID - // in a different way since the diffID cannot be validated as for regular pulled images. - for _, layer := range m.Layers { - toc, err := chunkedToc.GetTOCDigest(layer.Annotations) - if err != nil { - return "", fmt.Errorf("error looking up annotation for layer %q: %w", layer.Digest, err) - } - if toc != nil { - return m.calculateImageIDForPartialImage(diffIDs) - } - } - return m.Config.Digest.Hex(), nil } -func (m *OCI1) calculateImageIDForPartialImage(diffIDs []digest.Digest) (string, error) { - newID := digest.Canonical.Digester() - for i, layer := range m.Layers { - diffID := diffIDs[i] - _, err := newID.Hash().Write([]byte(diffID.Hex())) - if err != nil { - return "", fmt.Errorf("error writing diffID %q: %w", diffID, err) - } - toc, err := chunkedToc.GetTOCDigest(layer.Annotations) - if err != nil { - return "", fmt.Errorf("error looking up annotation for layer %q: %w", layer.Digest, err) - } - if toc != nil { - _, err = newID.Hash().Write([]byte(toc.Hex())) - if err != nil { - return "", fmt.Errorf("error writing TOC %q: %w", toc, err) - } - } - } - return newID.Digest().Hex(), nil -} - // CanChangeLayerCompression returns true if we can compress/decompress layers with mimeType in the current image // (and the code can handle that). // NOTE: Even if this returns true, the relevant format might not accept all compression algorithms; the set of accepted diff --git a/oci/archive/oci_dest.go b/oci/archive/oci_dest.go index 6ca618e351..a3eb5d7a1b 100644 --- a/oci/archive/oci_dest.go +++ b/oci/archive/oci_dest.go @@ -6,7 +6,6 @@ import ( "io" "os" - "github.com/containers/image/v5/internal/blobinfocache" "github.com/containers/image/v5/internal/imagedestination" "github.com/containers/image/v5/internal/imagedestination/impl" "github.com/containers/image/v5/internal/private" @@ -120,8 +119,8 @@ func (d *ociArchiveImageDestination) PutBlobWithOptions(ctx context.Context, str // It is available only if SupportsPutBlobPartial(). // Even if SupportsPutBlobPartial() returns true, the call can fail, in which case the caller // should fall back to PutBlobWithOptions. -func (d *ociArchiveImageDestination) PutBlobPartial(ctx context.Context, chunkAccessor private.BlobChunkAccessor, srcInfo types.BlobInfo, cache blobinfocache.BlobInfoCache2) (private.UploadedBlob, error) { - return d.unpackedDest.PutBlobPartial(ctx, chunkAccessor, srcInfo, cache) +func (d *ociArchiveImageDestination) PutBlobPartial(ctx context.Context, chunkAccessor private.BlobChunkAccessor, srcInfo types.BlobInfo, options private.PutBlobPartialOptions) (private.UploadedBlob, error) { + return d.unpackedDest.PutBlobPartial(ctx, chunkAccessor, srcInfo, options) } // TryReusingBlobWithOptions checks whether the transport already contains, or can efficiently reuse, a blob, and if so, applies it to the current destination diff --git a/openshift/openshift_dest.go b/openshift/openshift_dest.go index 50a5339e1b..656f4518d1 100644 --- a/openshift/openshift_dest.go +++ b/openshift/openshift_dest.go @@ -12,7 +12,6 @@ import ( "github.com/containers/image/v5/docker" "github.com/containers/image/v5/docker/reference" - "github.com/containers/image/v5/internal/blobinfocache" "github.com/containers/image/v5/internal/imagedestination" "github.com/containers/image/v5/internal/imagedestination/impl" "github.com/containers/image/v5/internal/imagedestination/stubs" @@ -128,8 +127,8 @@ func (d *openshiftImageDestination) PutBlobWithOptions(ctx context.Context, stre // It is available only if SupportsPutBlobPartial(). // Even if SupportsPutBlobPartial() returns true, the call can fail, in which case the caller // should fall back to PutBlobWithOptions. -func (d *openshiftImageDestination) PutBlobPartial(ctx context.Context, chunkAccessor private.BlobChunkAccessor, srcInfo types.BlobInfo, cache blobinfocache.BlobInfoCache2) (private.UploadedBlob, error) { - return d.docker.PutBlobPartial(ctx, chunkAccessor, srcInfo, cache) +func (d *openshiftImageDestination) PutBlobPartial(ctx context.Context, chunkAccessor private.BlobChunkAccessor, srcInfo types.BlobInfo, options private.PutBlobPartialOptions) (private.UploadedBlob, error) { + return d.docker.PutBlobPartial(ctx, chunkAccessor, srcInfo, options) } // TryReusingBlobWithOptions checks whether the transport already contains, or can efficiently reuse, a blob, and if so, applies it to the current destination diff --git a/pkg/blobcache/dest.go b/pkg/blobcache/dest.go index f32fccf805..663df11dd9 100644 --- a/pkg/blobcache/dest.go +++ b/pkg/blobcache/dest.go @@ -9,7 +9,6 @@ import ( "path/filepath" "sync" - "github.com/containers/image/v5/internal/blobinfocache" "github.com/containers/image/v5/internal/imagedestination" "github.com/containers/image/v5/internal/imagedestination/impl" "github.com/containers/image/v5/internal/private" @@ -227,8 +226,8 @@ func (d *blobCacheDestination) SupportsPutBlobPartial() bool { // It is available only if SupportsPutBlobPartial(). // Even if SupportsPutBlobPartial() returns true, the call can fail, in which case the caller // should fall back to PutBlobWithOptions. -func (d *blobCacheDestination) PutBlobPartial(ctx context.Context, chunkAccessor private.BlobChunkAccessor, srcInfo types.BlobInfo, cache blobinfocache.BlobInfoCache2) (private.UploadedBlob, error) { - return d.destination.PutBlobPartial(ctx, chunkAccessor, srcInfo, cache) +func (d *blobCacheDestination) PutBlobPartial(ctx context.Context, chunkAccessor private.BlobChunkAccessor, srcInfo types.BlobInfo, options private.PutBlobPartialOptions) (private.UploadedBlob, error) { + return d.destination.PutBlobPartial(ctx, chunkAccessor, srcInfo, options) } // TryReusingBlobWithOptions checks whether the transport already contains, or can efficiently reuse, a blob, and if so, applies it to the current destination diff --git a/storage/storage_dest.go b/storage/storage_dest.go index 9fa7f4a86a..5783981b4d 100644 --- a/storage/storage_dest.go +++ b/storage/storage_dest.go @@ -16,7 +16,6 @@ import ( "sync/atomic" "github.com/containers/image/v5/docker/reference" - "github.com/containers/image/v5/internal/blobinfocache" "github.com/containers/image/v5/internal/imagedestination/impl" "github.com/containers/image/v5/internal/imagedestination/stubs" "github.com/containers/image/v5/internal/private" @@ -81,19 +80,34 @@ type storageImageDestination struct { // _During the concurrent TryReusingBlob/PutBlob/* calls_ (but not necessarily during the final Commit) // uses must hold storageImageDestination.lock. type storageImageDestinationLockProtected struct { - uncompressedOrTocDigest map[digest.Digest]digest.Digest // Mapping from layer blobsums to their corresponding DiffIDs or TOC IDs. - fileSizes map[digest.Digest]int64 // Mapping from layer blobsums to their sizes - filenames map[digest.Digest]string // Mapping from layer blobsums to names of files we used to hold them - currentIndex int // The index of the layer to be committed (i.e., lower indices have already been committed) - indexToAddedLayerInfo map[int]addedLayerInfo // Mapping from layer (by index) to blob to add to the image - blobAdditionalLayer map[digest.Digest]storage.AdditionalLayer // Mapping from layer blobsums to their corresponding additional layer - diffOutputs map[digest.Digest]*graphdriver.DriverWithDifferOutput // Mapping from digest to differ output + currentIndex int // The index of the layer to be committed (i.e., lower indices have already been committed) + indexToAddedLayerInfo map[int]addedLayerInfo // Mapping from layer (by index) to blob to add to the image + + // In general, a layer is identified either by (compressed) digest, or by TOC digest. + // When creating a layer, the c/storage layer metadata and image IDs must _only_ be based on trusted values + // we have computed ourselves. (Layer reuse can then look up against such trusted values, but it might not + // recompute those values for incomding layers — the point of the reuse is that we don’t need to consume the incoming layer.) + + // Layer identification: For a layer, at least one of indexToTOCDigest and blobDiffIDs must be available before commitLayer is called. + // The presence of an indexToTOCDigest is what decides how the layer is identified, i.e. which fields must be trusted. + blobDiffIDs map[digest.Digest]digest.Digest // Mapping from layer blobsums to their corresponding DiffIDs + indexToTOCDigest map[int]digest.Digest // Mapping from layer index to a TOC Digest, IFF the layer was created/found/reused by TOC digest + + // Layer data: Before commitLayer is called, either at least one of (diffOutputs, blobAdditionalLayer, filenames) + // should be available; or indexToTOCDigest/blobDiffIDs should be enough to locate an existing c/storage layer. + // They are looked up in the order they are mentioned above. + diffOutputs map[int]*graphdriver.DriverWithDifferOutput // Mapping from layer index to a partially-pulled layer intermediate data + blobAdditionalLayer map[digest.Digest]storage.AdditionalLayer // Mapping from layer blobsums to their corresponding additional layer + // Mapping from layer blobsums to names of files we used to hold them. If set, fileSizes and blobDiffIDs must also be set. + filenames map[digest.Digest]string + // Mapping from layer blobsums to their sizes. If set, filenames and blobDiffIDs must also be set. + fileSizes map[digest.Digest]int64 } // addedLayerInfo records data about a layer to use in this image. type addedLayerInfo struct { - digest digest.Digest - emptyLayer bool // The layer is an “empty”/“throwaway” one, and may or may not be physically represented in various transport / storage systems. false if the manifest type does not have the concept. + digest digest.Digest // Mandatory, the digest of the layer. + emptyLayer bool // The layer is an “empty”/“throwaway” one, and may or may not be physically represented in various transport / storage systems. false if the manifest type does not have the concept. } // newImageDestination sets us up to write a new image, caching blobs in a temporary directory until @@ -130,12 +144,13 @@ func newImageDestination(sys *types.SystemContext, imageRef storageReference) (* }, indexToStorageID: make(map[int]string), lockProtected: storageImageDestinationLockProtected{ - uncompressedOrTocDigest: make(map[digest.Digest]digest.Digest), - fileSizes: make(map[digest.Digest]int64), - filenames: make(map[digest.Digest]string), - indexToAddedLayerInfo: make(map[int]addedLayerInfo), - blobAdditionalLayer: make(map[digest.Digest]storage.AdditionalLayer), - diffOutputs: make(map[digest.Digest]*graphdriver.DriverWithDifferOutput), + indexToAddedLayerInfo: make(map[int]addedLayerInfo), + blobDiffIDs: make(map[digest.Digest]digest.Digest), + indexToTOCDigest: make(map[int]digest.Digest), + diffOutputs: make(map[int]*graphdriver.DriverWithDifferOutput), + blobAdditionalLayer: make(map[digest.Digest]storage.AdditionalLayer), + filenames: make(map[digest.Digest]string), + fileSizes: make(map[digest.Digest]int64), }, } dest.Compat = impl.AddCompat(dest) @@ -236,7 +251,7 @@ func (s *storageImageDestination) putBlobToPendingFile(stream io.Reader, blobinf // Record information about the blob. s.lock.Lock() - s.lockProtected.uncompressedOrTocDigest[blobDigest] = diffID.Digest() + s.lockProtected.blobDiffIDs[blobDigest] = diffID.Digest() s.lockProtected.fileSizes[blobDigest] = counter.Count s.lockProtected.filenames[blobDigest] = filename s.lock.Unlock() @@ -278,7 +293,7 @@ func (f *zstdFetcher) GetBlobAt(chunks []chunked.ImageSourceChunk) (chan io.Read // It is available only if SupportsPutBlobPartial(). // Even if SupportsPutBlobPartial() returns true, the call can fail, in which case the caller // should fall back to PutBlobWithOptions. -func (s *storageImageDestination) PutBlobPartial(ctx context.Context, chunkAccessor private.BlobChunkAccessor, srcInfo types.BlobInfo, cache blobinfocache.BlobInfoCache2) (private.UploadedBlob, error) { +func (s *storageImageDestination) PutBlobPartial(ctx context.Context, chunkAccessor private.BlobChunkAccessor, srcInfo types.BlobInfo, options private.PutBlobPartialOptions) (private.UploadedBlob, error) { fetcher := zstdFetcher{ chunkAccessor: chunkAccessor, ctx: ctx, @@ -295,13 +310,25 @@ func (s *storageImageDestination) PutBlobPartial(ctx context.Context, chunkAcces return private.UploadedBlob{}, err } + if out.TOCDigest == "" && out.UncompressedDigest == "" { + return private.UploadedBlob{}, errors.New("internal error: ApplyDiffWithDiffer succeeded with neither TOCDigest nor UncompressedDigest set") + } + blobDigest := srcInfo.Digest s.lock.Lock() - s.lockProtected.uncompressedOrTocDigest[blobDigest] = blobDigest - s.lockProtected.fileSizes[blobDigest] = 0 - s.lockProtected.filenames[blobDigest] = "" - s.lockProtected.diffOutputs[blobDigest] = out + if out.UncompressedDigest != "" { + // The computation of UncompressedDigest means the whole layer has been consumed; while doing that, chunked.GetDiffer is + // responsible for ensuring blobDigest has been validated. + s.lockProtected.blobDiffIDs[blobDigest] = out.UncompressedDigest + } else { + // Don’t identify layers by TOC if UncompressedDigest is available. + // - Using UncompressedDigest allows image reuse with non-partially-pulled layers + // - If UncompressedDigest has been computed, that means the layer was read completely, and the TOC has been created from scratch. + // That TOC is quite unlikely to match with any other TOC value. + s.lockProtected.indexToTOCDigest[options.LayerIndex] = out.TOCDigest + } + s.lockProtected.diffOutputs[options.LayerIndex] = out s.lock.Unlock() return private.UploadedBlob{ @@ -330,68 +357,79 @@ func (s *storageImageDestination) TryReusingBlobWithOptions(ctx context.Context, }) } -// tryReusingBlobAsPending implements TryReusingBlobWithOptions for (digest, size or -1), filling s.uncompressedOrTocDigest and other metadata. +// tryReusingBlobAsPending implements TryReusingBlobWithOptions for (blobDigest, size or -1), filling s.blobDiffIDs and other metadata. // The caller must arrange the blob to be eventually committed using s.commitLayer(). -func (s *storageImageDestination) tryReusingBlobAsPending(digest digest.Digest, size int64, options *private.TryReusingBlobOptions) (bool, private.ReusedBlob, error) { +func (s *storageImageDestination) tryReusingBlobAsPending(blobDigest digest.Digest, size int64, options *private.TryReusingBlobOptions) (bool, private.ReusedBlob, error) { // lock the entire method as it executes fairly quickly s.lock.Lock() defer s.lock.Unlock() if options.SrcRef != nil { // Check if we have the layer in the underlying additional layer store. - aLayer, err := s.imageRef.transport.store.LookupAdditionalLayer(digest, options.SrcRef.String()) + aLayer, err := s.imageRef.transport.store.LookupAdditionalLayer(blobDigest, options.SrcRef.String()) if err != nil && !errors.Is(err, storage.ErrLayerUnknown) { - return false, private.ReusedBlob{}, fmt.Errorf(`looking for compressed layers with digest %q and labels: %w`, digest, err) + return false, private.ReusedBlob{}, fmt.Errorf(`looking for compressed layers with digest %q and labels: %w`, blobDigest, err) } else if err == nil { - // Record the uncompressed value so that we can use it to calculate layer IDs. - s.lockProtected.uncompressedOrTocDigest[digest] = aLayer.UncompressedDigest() - s.lockProtected.blobAdditionalLayer[digest] = aLayer + s.lockProtected.blobDiffIDs[blobDigest] = aLayer.UncompressedDigest() + s.lockProtected.blobAdditionalLayer[blobDigest] = aLayer return true, private.ReusedBlob{ - Digest: digest, + Digest: blobDigest, Size: aLayer.CompressedSize(), }, nil } } - if digest == "" { + if blobDigest == "" { return false, private.ReusedBlob{}, errors.New(`Can not check for a blob with unknown digest`) } - if err := digest.Validate(); err != nil { + if err := blobDigest.Validate(); err != nil { return false, private.ReusedBlob{}, fmt.Errorf("Can not check for a blob with invalid digest: %w", err) } + if options.TOCDigest != "" { + if err := options.TOCDigest.Validate(); err != nil { + return false, private.ReusedBlob{}, fmt.Errorf("Can not check for a blob with invalid digest: %w", err) + } + } + + // Check if we have a wasn't-compressed layer in storage that's based on that blob. // Check if we've already cached it in a file. - if size, ok := s.lockProtected.fileSizes[digest]; ok { + if size, ok := s.lockProtected.fileSizes[blobDigest]; ok { + // s.lockProtected.blobDiffIDs is set either by putBlobToPendingFile or in createNewLayer when creating the + // filenames/fileSizes entry. return true, private.ReusedBlob{ - Digest: digest, + Digest: blobDigest, Size: size, }, nil } - // Check if we have a wasn't-compressed layer in storage that's based on that blob. - layers, err := s.imageRef.transport.store.LayersByUncompressedDigest(digest) + layers, err := s.imageRef.transport.store.LayersByUncompressedDigest(blobDigest) if err != nil && !errors.Is(err, storage.ErrLayerUnknown) { - return false, private.ReusedBlob{}, fmt.Errorf(`looking for layers with digest %q: %w`, digest, err) + return false, private.ReusedBlob{}, fmt.Errorf(`looking for layers with digest %q: %w`, blobDigest, err) } if len(layers) > 0 { - // Save this for completeness. - s.lockProtected.uncompressedOrTocDigest[digest] = layers[0].UncompressedDigest + s.lockProtected.blobDiffIDs[blobDigest] = blobDigest return true, private.ReusedBlob{ - Digest: digest, + Digest: blobDigest, Size: layers[0].UncompressedSize, }, nil } // Check if we have a was-compressed layer in storage that's based on that blob. - layers, err = s.imageRef.transport.store.LayersByCompressedDigest(digest) + layers, err = s.imageRef.transport.store.LayersByCompressedDigest(blobDigest) if err != nil && !errors.Is(err, storage.ErrLayerUnknown) { - return false, private.ReusedBlob{}, fmt.Errorf(`looking for compressed layers with digest %q: %w`, digest, err) + return false, private.ReusedBlob{}, fmt.Errorf(`looking for compressed layers with digest %q: %w`, blobDigest, err) } if len(layers) > 0 { - // Record the uncompressed value so that we can use it to calculate layer IDs. - s.lockProtected.uncompressedOrTocDigest[digest] = layers[0].UncompressedDigest + // LayersByCompressedDigest only finds layers which were created from a full layer blob, and extracting that + // always sets UncompressedDigest. + diffID := layers[0].UncompressedDigest + if diffID == "" { + return false, private.ReusedBlob{}, fmt.Errorf("internal error: compressed layer %q (for compressed digest %q) does not have an uncompressed digest", layers[0].ID, blobDigest.String()) + } + s.lockProtected.blobDiffIDs[blobDigest] = diffID return true, private.ReusedBlob{ - Digest: digest, + Digest: blobDigest, Size: layers[0].CompressedSize, }, nil } @@ -400,23 +438,23 @@ func (s *storageImageDestination) tryReusingBlobAsPending(digest digest.Digest, // Because we must return the size, which is unknown for unavailable compressed blobs, the returned BlobInfo refers to the // uncompressed layer, and that can happen only if options.CanSubstitute, or if the incoming manifest already specifies the size. if options.CanSubstitute || size != -1 { - if uncompressedDigest := options.Cache.UncompressedDigest(digest); uncompressedDigest != "" && uncompressedDigest != digest { + if uncompressedDigest := options.Cache.UncompressedDigest(blobDigest); uncompressedDigest != "" && uncompressedDigest != blobDigest { layers, err := s.imageRef.transport.store.LayersByUncompressedDigest(uncompressedDigest) if err != nil && !errors.Is(err, storage.ErrLayerUnknown) { return false, private.ReusedBlob{}, fmt.Errorf(`looking for layers with digest %q: %w`, uncompressedDigest, err) } if len(layers) > 0 { if size != -1 { - s.lockProtected.uncompressedOrTocDigest[digest] = layers[0].UncompressedDigest + s.lockProtected.blobDiffIDs[blobDigest] = uncompressedDigest return true, private.ReusedBlob{ - Digest: digest, + Digest: blobDigest, Size: size, }, nil } if !options.CanSubstitute { - return false, private.ReusedBlob{}, fmt.Errorf("Internal error: options.CanSubstitute was expected to be true for blob with digest %s", digest) + return false, private.ReusedBlob{}, fmt.Errorf("Internal error: options.CanSubstitute was expected to be true for blob with digest %s", blobDigest) } - s.lockProtected.uncompressedOrTocDigest[uncompressedDigest] = layers[0].UncompressedDigest + s.lockProtected.blobDiffIDs[uncompressedDigest] = uncompressedDigest return true, private.ReusedBlob{ Digest: uncompressedDigest, Size: layers[0].UncompressedSize, @@ -425,23 +463,21 @@ func (s *storageImageDestination) tryReusingBlobAsPending(digest digest.Digest, } } - tocDigest := digest - if options.TOCDigest != nil { - tocDigest = *options.TOCDigest - } + if options.TOCDigest != "" && options.LayerIndex != nil { + // Check if we have a chunked layer in storage with the same TOC digest. + layers, err := s.imageRef.transport.store.LayersByTOCDigest(options.TOCDigest) - // Check if we have a chunked layer in storage with the same TOC digest. - layers, err = s.imageRef.transport.store.LayersByTOCDigest(tocDigest) - if err != nil && !errors.Is(err, storage.ErrLayerUnknown) { - return false, private.ReusedBlob{}, fmt.Errorf(`looking for layers with TOC digest %q: %w`, tocDigest, err) - } - if len(layers) > 0 { - // Save this for completeness. - s.lockProtected.uncompressedOrTocDigest[digest] = layers[0].TOCDigest - return true, private.ReusedBlob{ - Digest: layers[0].TOCDigest, - Size: layers[0].UncompressedSize, - }, nil + if err != nil && !errors.Is(err, storage.ErrLayerUnknown) { + return false, private.ReusedBlob{}, fmt.Errorf(`looking for layers with TOC digest %q: %w`, options.TOCDigest, err) + } + if len(layers) > 0 { + s.lockProtected.indexToTOCDigest[*options.LayerIndex] = options.TOCDigest + return true, private.ReusedBlob{ + Digest: blobDigest, + Size: layers[0].UncompressedSize, + MatchedByTOCDigest: true, + }, nil + } } // Nope, we don't have it. @@ -468,28 +504,59 @@ func (s *storageImageDestination) computeID(m manifest.Manifest) string { continue } blobSum := m.FSLayers[i].BlobSum - diffID, ok := s.lockProtected.uncompressedOrTocDigest[blobSum] + diffID, ok := s.lockProtected.blobDiffIDs[blobSum] if !ok { + // this can, in principle, legitimately happen when a layer is reused by TOC. logrus.Infof("error looking up diffID for layer %q", blobSum.String()) return "" } diffIDs = append([]digest.Digest{diffID}, diffIDs...) } - case *manifest.Schema2: + case *manifest.Schema2, *manifest.OCI1: // We know the ID calculation doesn't actually use the diffIDs, so we don't need to populate // the diffID list. - case *manifest.OCI1: - for _, l := range m.Layers { - diffIDs = append(diffIDs, l.Digest) - } default: return "" } - id, err := m.ImageID(diffIDs) + + // We want to use the same ID for “the same” images, but without risking unwanted sharing / malicious image corruption. + // + // Traditionally that means the same ~config digest, as computed by m.ImageID; + // but if we pull a layer by TOC, we verify the layer against neither the (compressed) blob digest in the manifest, + // nor against the config’s RootFS.DiffIDs. We don’t really want to do either, to allow partial layer pulls where we never see + // most of the data. + // + // So, if a layer is pulled by TOC (and we do validate against the TOC), the fact that we used the TOC, and the value of the TOC, + // must enter into the image ID computation. + // But for images where no TOC was used, continue to use IDs computed the traditional way, to maximize image reuse on upgrades, + // and to introduce the changed behavior only when partial pulls are used. + // + // Note that it’s not 100% guaranteed that an image pulled by TOC uses an OCI manifest; consider + // (skopeo copy --format v2s2 docker://…/zstd-chunked-image containers-storage:… ). So this is not happening only in the OCI case above. + ordinaryImageID, err := m.ImageID(diffIDs) if err != nil { return "" } - return id + tocIDInput := "" + hasLayerPulledByTOC := false + for i := range m.LayerInfos() { + layerValue := "" // An empty string is not a valid digest, so this is unambiguous with the TOC case. + tocDigest, ok := s.lockProtected.indexToTOCDigest[i] // "" if not a TOC + if ok { + hasLayerPulledByTOC = true + layerValue = tocDigest.String() + } + tocIDInput += layerValue + "|" // "|" can not be present in a TOC digest, so this is an unambiguous separator. + } + + if !hasLayerPulledByTOC { + return ordinaryImageID + } + // ordinaryImageID is a digest of a config, which is a JSON value. + // To avoid the risk of collisions, start the input with @ so that the input is not a valid JSON. + tocImageID := digest.FromString("@With TOC:" + tocIDInput).Hex() + logrus.Debugf("Ordinary storage image ID %s; a layer was looked up by TOC, so using image ID %s", ordinaryImageID, tocImageID) + return tocImageID } // getConfigBlob exists only to let us retrieve the configuration blob so that the manifest package can dig @@ -566,20 +633,25 @@ func (s *storageImageDestination) queueOrCommit(index int, info addedLayerInfo) return nil } -// getDiffIDOrTOCDigest returns the diffID for the specified digest or the digest for the TOC, if known. -func (s *storageImageDestination) getDiffIDOrTOCDigest(uncompressedDigest digest.Digest) (digest.Digest, bool) { +// singleLayerIDComponent returns a single layer’s the input to computing a layer (chain) ID, +// and an indication whether the input already has the shape of a layer ID. +// It returns ("", false) if the layer is not found at all (which should never happen) +func (s *storageImageDestination) singleLayerIDComponent(layerIndex int, blobDigest digest.Digest) (string, bool) { s.lock.Lock() defer s.lock.Unlock() - if d, found := s.lockProtected.diffOutputs[uncompressedDigest]; found { - return d.TOCDigest, found + if d, found := s.lockProtected.indexToTOCDigest[layerIndex]; found { + return "@TOC=" + d.Hex(), false // "@" is not a valid start of a digest.Digest, so this is unambiguous. + } + + if d, found := s.lockProtected.blobDiffIDs[blobDigest]; found { + return d.Hex(), true // This looks like chain IDs, and it uses the traditional value. } - d, found := s.lockProtected.uncompressedOrTocDigest[uncompressedDigest] - return d, found + return "", false } // commitLayer commits the specified layer with the given index to the storage. -// size can usually be -1; it can be provided if the layer is not known to be already present in uncompressedOrTocDigest. +// size can usually be -1; it can be provided if the layer is not known to be already present in blobDiffIDs. // // If the layer cannot be committed yet, the function returns (true, nil). // @@ -614,16 +686,21 @@ func (s *storageImageDestination) commitLayer(index int, info addedLayerInfo, si // Check if there's already a layer with the ID that we'd give to the result of applying // this layer blob to its parent, if it has one, or the blob's hex value otherwise. - // The diffIDOrTOCDigest refers either to the DiffID or the digest of the TOC. - diffIDOrTOCDigest, haveDiffIDOrTOCDigest := s.getDiffIDOrTOCDigest(info.digest) - if !haveDiffIDOrTOCDigest { - // Check if it's elsewhere and the caller just forgot to pass it to us in a PutBlob(), - // or to even check if we had it. - // Use none.NoCache to avoid a repeated DiffID lookup in the BlobInfoCache; a caller + // The layerID refers either to the DiffID or the digest of the TOC. + layerIDComponent, layerIDComponentStandalone := s.singleLayerIDComponent(index, info.digest) + if layerIDComponent == "" { + // Check if it's elsewhere and the caller just forgot to pass it to us in a PutBlob() / TryReusingBlob() / … + // + // Use none.NoCache to avoid a repeated DiffID lookup in the BlobInfoCache: a caller // that relies on using a blob digest that has never been seen by the store had better call // TryReusingBlob; not calling PutBlob already violates the documented API, so there’s only // so far we are going to accommodate that (if we should be doing that at all). - logrus.Debugf("looking for diffID or TOC digest for blob %+v", info.digest) + // + // We are also ignoring lookups by TOC, and other non-trivial situations. + // Those can only happen using the c/image/internal/private API, + // so those internal callers should be fixed to follow the API instead of expanding this fallback. + logrus.Debugf("looking for diffID for blob=%+v", info.digest) + // Use tryReusingBlobAsPending, not the top-level TryReusingBlobWithOptions, to prevent recursion via queueOrCommit. has, _, err := s.tryReusingBlobAsPending(info.digest, size, &private.TryReusingBlobOptions{ Cache: none.NoCache, @@ -633,16 +710,18 @@ func (s *storageImageDestination) commitLayer(index int, info addedLayerInfo, si return false, fmt.Errorf("checking for a layer based on blob %q: %w", info.digest.String(), err) } if !has { - return false, fmt.Errorf("error determining uncompressed digest or TOC digest for blob %q", info.digest.String()) + return false, fmt.Errorf("error determining uncompressed digest for blob %q", info.digest.String()) } - diffIDOrTOCDigest, haveDiffIDOrTOCDigest = s.getDiffIDOrTOCDigest(info.digest) - if !haveDiffIDOrTOCDigest { - return false, fmt.Errorf("we have blob %q, but don't know its uncompressed or TOC digest", info.digest.String()) + + layerIDComponent, layerIDComponentStandalone = s.singleLayerIDComponent(index, info.digest) + if layerIDComponent == "" { + return false, fmt.Errorf("we have blob %q, but don't know its layer ID", info.digest.String()) } } - id := diffIDOrTOCDigest.Hex() - if parentLayer != "" { - id = digest.Canonical.FromString(parentLayer + "+" + diffIDOrTOCDigest.Hex()).Hex() + + id := layerIDComponent + if !layerIDComponentStandalone || parentLayer != "" { + id = digest.Canonical.FromString(parentLayer + "+" + layerIDComponent).Hex() } if layer, err2 := s.imageRef.transport.store.Layer(id); layer != nil && err2 == nil { // There's already a layer that should have the right contents, just reuse it. @@ -650,96 +729,134 @@ func (s *storageImageDestination) commitLayer(index int, info addedLayerInfo, si return false, nil } + layer, err := s.createNewLayer(index, info.digest, parentLayer, id) + if err != nil { + return false, err + } + if layer == nil { + return true, nil + } + s.indexToStorageID[index] = layer.ID + return false, nil +} + +// createNewLayer creates a new layer newLayerID for (index, layerDigest) on top of parentLayer (which may be ""). +// If the layer cannot be committed yet, the function returns (nil, nil). +func (s *storageImageDestination) createNewLayer(index int, layerDigest digest.Digest, parentLayer, newLayerID string) (*storage.Layer, error) { s.lock.Lock() - diffOutput, ok := s.lockProtected.diffOutputs[info.digest] + diffOutput, ok := s.lockProtected.diffOutputs[index] s.lock.Unlock() if ok { - if s.manifest == nil { - logrus.Debugf("Skipping commit for TOC=%q, manifest not yet available", id) - return true, nil - } + var untrustedUncompressedDigest digest.Digest + if diffOutput.UncompressedDigest == "" { + if s.manifest == nil { + logrus.Debugf("Skipping commit for layer %q, manifest not yet available", newLayerID) + return nil, nil + } - man, err := manifest.FromBlob(s.manifest, manifest.GuessMIMEType(s.manifest)) - if err != nil { - return false, fmt.Errorf("parsing manifest: %w", err) - } + man, err := manifest.FromBlob(s.manifest, manifest.GuessMIMEType(s.manifest)) + if err != nil { + return nil, fmt.Errorf("parsing manifest: %w", err) + } - cb, err := s.getConfigBlob(man.ConfigInfo()) - if err != nil { - return false, err - } + cb, err := s.getConfigBlob(man.ConfigInfo()) + if err != nil { + return nil, err + } - // retrieve the expected uncompressed digest from the config blob. - configOCI := &imgspecv1.Image{} - if err := json.Unmarshal(cb, configOCI); err != nil { - return false, err - } - if index >= len(configOCI.RootFS.DiffIDs) { - return false, fmt.Errorf("index %d out of range for configOCI.RootFS.DiffIDs", index) + // retrieve the expected uncompressed digest from the config blob. + configOCI := &imgspecv1.Image{} + if err := json.Unmarshal(cb, configOCI); err != nil { + return nil, err + } + if index >= len(configOCI.RootFS.DiffIDs) { + return nil, fmt.Errorf("index %d out of range for configOCI.RootFS.DiffIDs", index) + } + untrustedUncompressedDigest = configOCI.RootFS.DiffIDs[index] } - layer, err := s.imageRef.transport.store.CreateLayer(id, parentLayer, nil, "", false, nil) + layer, err := s.imageRef.transport.store.CreateLayer(newLayerID, parentLayer, nil, "", false, nil) if err != nil { - return false, err + return nil, err } - // let the storage layer know what was the original uncompressed layer. flags := make(map[string]interface{}) - flags[expectedLayerDiffIDFlag] = configOCI.RootFS.DiffIDs[index] - logrus.Debugf("Setting uncompressed digest to %q for layer %q", configOCI.RootFS.DiffIDs[index], id) + if untrustedUncompressedDigest != "" { + flags[expectedLayerDiffIDFlag] = untrustedUncompressedDigest + logrus.Debugf("Setting uncompressed digest to %q for layer %q", untrustedUncompressedDigest, newLayerID) + } options := &graphdriver.ApplyDiffWithDifferOpts{ Flags: flags, } - if err := s.imageRef.transport.store.ApplyDiffFromStagingDirectory(layer.ID, diffOutput.Target, diffOutput, options); err != nil { _ = s.imageRef.transport.store.Delete(layer.ID) - return false, err + return nil, err } - - s.indexToStorageID[index] = layer.ID - return false, nil + return layer, nil } s.lock.Lock() - al, ok := s.lockProtected.blobAdditionalLayer[info.digest] + al, ok := s.lockProtected.blobAdditionalLayer[layerDigest] s.lock.Unlock() if ok { - layer, err := al.PutAs(id, parentLayer, nil) + layer, err := al.PutAs(newLayerID, parentLayer, nil) if err != nil && !errors.Is(err, storage.ErrDuplicateID) { - return false, fmt.Errorf("failed to put layer from digest and labels: %w", err) + return nil, fmt.Errorf("failed to put layer from digest and labels: %w", err) } - s.indexToStorageID[index] = layer.ID - return false, nil + return layer, nil } // Check if we previously cached a file with that blob's contents. If we didn't, // then we need to read the desired contents from a layer. + var trustedUncompressedDigest, trustedOriginalDigest digest.Digest // For storage.LayerOptions s.lock.Lock() - filename, ok := s.lockProtected.filenames[info.digest] + tocDigest := s.lockProtected.indexToTOCDigest[index] // "" if not set + optionalDiffID := s.lockProtected.blobDiffIDs[layerDigest] // "" if not set + filename, gotFilename := s.lockProtected.filenames[layerDigest] s.lock.Unlock() - if !ok { - // Try to find the layer with contents matching that blobsum. - layer := "" - layers, err2 := s.imageRef.transport.store.LayersByUncompressedDigest(diffIDOrTOCDigest) - if err2 == nil && len(layers) > 0 { - layer = layers[0].ID + if gotFilename && tocDigest == "" { + // If tocDigest != "", if we now happen to find a layerDigest match, the newLayerID has already been computed as TOC-based, + // and we don't know the relationship of the layerDigest and TOC digest. + // We could recompute newLayerID to be DiffID-based and use the file, but such a within-image layer + // reuse is expected to be pretty rare; instead, ignore the unexpected file match and proceed to the + // originally-planned TOC match. + + // Because tocDigest == "", optionaldiffID must have been set; and even if it weren’t, PutLayer will recompute the digest from the stream. + trustedUncompressedDigest = optionalDiffID + trustedOriginalDigest = layerDigest // The code setting .filenames[layerDigest] is responsible for the contents matching. + } else { + // Try to find the layer with contents matching the data we use. + var layer *storage.Layer // = nil + if tocDigest != "" { + layers, err2 := s.imageRef.transport.store.LayersByTOCDigest(tocDigest) + if err2 == nil && len(layers) > 0 { + layer = &layers[0] + } else { + return nil, fmt.Errorf("locating layer for TOC digest %q: %w", tocDigest, err2) + } } else { - layers, err2 = s.imageRef.transport.store.LayersByCompressedDigest(info.digest) + // Because tocDigest == "", optionaldiffID must have been set + layers, err2 := s.imageRef.transport.store.LayersByUncompressedDigest(optionalDiffID) if err2 == nil && len(layers) > 0 { - layer = layers[0].ID + layer = &layers[0] + } else { + layers, err2 = s.imageRef.transport.store.LayersByCompressedDigest(layerDigest) + if err2 == nil && len(layers) > 0 { + layer = &layers[0] + } + } + if layer == nil { + return nil, fmt.Errorf("locating layer for blob %q: %w", layerDigest, err2) } - } - if layer == "" { - return false, fmt.Errorf("locating layer for blob %q: %w", info.digest, err2) } // Read the layer's contents. noCompression := archive.Uncompressed diffOptions := &storage.DiffOptions{ Compression: &noCompression, } - diff, err2 := s.imageRef.transport.store.Diff("", layer, diffOptions) + diff, err2 := s.imageRef.transport.store.Diff("", layer.ID, diffOptions) if err2 != nil { - return false, fmt.Errorf("reading layer %q for blob %q: %w", layer, info.digest, err2) + return nil, fmt.Errorf("reading layer %q for blob %q: %w", layer.ID, layerDigest, err2) } // Copy the layer diff to a file. Diff() takes a lock that it holds // until the ReadCloser that it returns is closed, and PutLayer() wants @@ -749,41 +866,65 @@ func (s *storageImageDestination) commitLayer(index int, info addedLayerInfo, si file, err := os.OpenFile(filename, os.O_CREATE|os.O_TRUNC|os.O_WRONLY|os.O_EXCL, 0o600) if err != nil { diff.Close() - return false, fmt.Errorf("creating temporary file %q: %w", filename, err) + return nil, fmt.Errorf("creating temporary file %q: %w", filename, err) } // Copy the data to the file. // TODO: This can take quite some time, and should ideally be cancellable using // ctx.Done(). - _, err = io.Copy(file, diff) + fileSize, err := io.Copy(file, diff) diff.Close() file.Close() if err != nil { - return false, fmt.Errorf("storing blob to file %q: %w", filename, err) + return nil, fmt.Errorf("storing blob to file %q: %w", filename, err) + } + + if optionalDiffID == "" && layer.UncompressedDigest != "" { + optionalDiffID = layer.UncompressedDigest + } + // The stream we have is uncompressed, this matches contents of the stream. + // If tocDigest != "", trustedUncompressedDigest might still be ""; in that case PutLayer will compute the value from the stream. + trustedUncompressedDigest = optionalDiffID + // FIXME? trustedOriginalDigest could be set to layerDigest IF tocDigest == "" (otherwise layerDigest is untrusted). + // But for c/storage to reasonably use it (as a CompressedDigest value), we should also ensure the CompressedSize of the created + // layer is correct, and the API does not currently make it possible (.CompressedSize is set from the input stream). + // + // We can legitimately set storage.LayerOptions.OriginalDigest to "", + // but that would just result in PutLayer computing the digest of the input stream == optionalDiffID. + // So, instead, set .OriginalDigest to the value we know already, to avoid that digest computation. + // FIXME? If both trustedUncompressedDigest and trustedOriginalDigest are "", PutLayer currently digests the uncompressed layer + // twice. We could compute the digest here, but fixing that in c/storage would be more general. + trustedOriginalDigest = optionalDiffID + + // Allow using the already-collected layer contents without extracting the layer again. + // + // This only matches against the uncompressed digest. + // We don’t have the original compressed data here to trivially set filenames[layerDigest]. + // In particular we can’t achieve the correct Layer.CompressedSize value with the current c/storage API. + // Within-image layer reuse is probably very rare, for now we prefer to avoid that complexity. + if trustedUncompressedDigest != "" { + s.lock.Lock() + s.lockProtected.blobDiffIDs[trustedUncompressedDigest] = trustedUncompressedDigest + s.lockProtected.filenames[trustedUncompressedDigest] = filename + s.lockProtected.fileSizes[trustedUncompressedDigest] = fileSize + s.lock.Unlock() } - // Make sure that we can find this file later, should we need the layer's - // contents again. - s.lock.Lock() - s.lockProtected.filenames[info.digest] = filename - s.lock.Unlock() } // Read the cached blob and use it as a diff. file, err := os.Open(filename) if err != nil { - return false, fmt.Errorf("opening file %q: %w", filename, err) + return nil, fmt.Errorf("opening file %q: %w", filename, err) } defer file.Close() // Build the new layer using the diff, regardless of where it came from. // TODO: This can take quite some time, and should ideally be cancellable using ctx.Done(). - layer, _, err := s.imageRef.transport.store.PutLayer(id, parentLayer, nil, "", false, &storage.LayerOptions{ - OriginalDigest: info.digest, - UncompressedDigest: diffIDOrTOCDigest, + layer, _, err := s.imageRef.transport.store.PutLayer(newLayerID, parentLayer, nil, "", false, &storage.LayerOptions{ + OriginalDigest: trustedOriginalDigest, + UncompressedDigest: trustedUncompressedDigest, }, file) if err != nil && !errors.Is(err, storage.ErrDuplicateID) { - return false, fmt.Errorf("adding layer with blob %q: %w", info.digest, err) + return nil, fmt.Errorf("adding layer with blob %q: %w", layerDigest, err) } - - s.indexToStorageID[index] = layer.ID - return false, nil + return layer, nil } // Commit marks the process of storing the image as successful and asks for the image to be persisted. diff --git a/storage/storage_src.go b/storage/storage_src.go index 1b8fa07b5b..febfc226a4 100644 --- a/storage/storage_src.go +++ b/storage/storage_src.go @@ -301,16 +301,15 @@ func (s *storageImageSource) LayerInfosForCopy(ctx context.Context, instanceDige if err != nil { return nil, fmt.Errorf("reading layer %q in image %q: %w", layerID, s.image.ID, err) } - if layer.UncompressedDigest == "" && layer.TOCDigest == "" { - return nil, fmt.Errorf("uncompressed digest and TOC digest for layer %q is unknown", layerID) - } if layer.UncompressedSize < 0 { return nil, fmt.Errorf("uncompressed size for layer %q is unknown", layerID) } blobDigest := layer.UncompressedDigest - - if layer.TOCDigest != "" { + if blobDigest == "" { + if layer.TOCDigest == "" { + return nil, fmt.Errorf("uncompressed digest and TOC digest for layer %q is unknown", layerID) + } if layer.Flags == nil || layer.Flags[expectedLayerDiffIDFlag] == nil { return nil, fmt.Errorf("TOC digest %q for layer %q is present but %q flag is not set", layer.TOCDigest, layerID, expectedLayerDiffIDFlag) }