Merge pull request #1894 from mtrmac/drop-annotations

Add FIXMEs about handling of zstd:chunked blob annotations on blob changes

copy/compression.go

@@ -176,7 +176,7 @@ func (ic *imageCopier) bpcRecompressCompressed(stream *sourceStream, detected bp
  recompressed, annotations := ic.compressedStream(decompressed, *ic.compressionFormat)
  // Note: recompressed must be closed on all return paths.
  stream.reader = recompressed
- stream.info = types.BlobInfo{ // FIXME? Should we preserve more data in src.info?
+ stream.info = types.BlobInfo{ // FIXME? Should we preserve more data in src.info? Notably the current approach correctly removes zstd:chunked metadata annotations.
  Digest: "",
  Size: -1,
  }
@@ -203,7 +203,7 @@ func (ic *imageCopier) bpcDecompressCompressed(stream *sourceStream, detected bp
  }
  // Note: s must be closed on all return paths.
  stream.reader = s
- stream.info = types.BlobInfo{ // FIXME? Should we preserve more data in src.info?
+ stream.info = types.BlobInfo{ // FIXME? Should we preserve more data in src.info? Notably the current approach correctly removes zstd:chunked metadata annotations.
  Digest: "",
  Size: -1,
  }

copy/single.go

@@ -739,9 +739,9 @@ func updatedBlobInfoFromReuse(inputInfo types.BlobInfo, reusedBlob private.Reuse
  res := types.BlobInfo{
  Digest: reusedBlob.Digest,
  Size: reusedBlob.Size,
- URLs: nil, // This _must_ be cleared if Digest changes; clear it in other cases as well, to preserve previous behavior.
- Annotations: inputInfo.Annotations,
- MediaType: inputInfo.MediaType, // Mostly irrelevant, MediaType is updated based on Compression*/CryptoOperation.
+ URLs: nil,  // This _must_ be cleared if Digest changes; clear it in other cases as well, to preserve previous behavior.
+ Annotations: inputInfo.Annotations, // FIXME: This should remove zstd:chunked annotations (but those annotations being left with incorrect values should not break pulls)
+ MediaType: inputInfo.MediaType,  // Mostly irrelevant, MediaType is updated based on Compression*/CryptoOperation.
  CompressionOperation: reusedBlob.CompressionOperation,
  CompressionAlgorithm: reusedBlob.CompressionAlgorithm,
  CryptoOperation: inputInfo.CryptoOperation, // Expected to be unset anyway.

pkg/blobcache/src.go

@@ -157,7 +157,7 @@ func (s *blobCacheSource) LayerInfosForCopy(ctx context.Context, instanceDigest
  case types.Compress:
  info.MediaType = v1.MediaTypeImageLayerGzip
  info.CompressionAlgorithm = &compression.Gzip
- case types.Decompress:
+ case types.Decompress: // FIXME: This should remove zstd:chunked annotations (but those annotations being left with incorrect values should not break pulls)
  info.MediaType = v1.MediaTypeImageLayer
  info.CompressionAlgorithm = nil
  }

pkg/compression/internal/types.go

@@ -44,21 +44,21 @@ func (c Algorithm) InternalUnstableUndocumentedMIMEQuestionMark() string {
 }
 
 // AlgorithmCompressor returns the compressor field of algo.
-// This is a function instead of a public method so that it is only callable from by code
+// This is a function instead of a public method so that it is only callable by code
 // that is allowed to import this internal subpackage.
 func AlgorithmCompressor(algo Algorithm) CompressorFunc {
  return algo.compressor
 }
 
 // AlgorithmDecompressor returns the decompressor field of algo.
-// This is a function instead of a public method so that it is only callable from by code
+// This is a function instead of a public method so that it is only callable by code
 // that is allowed to import this internal subpackage.
 func AlgorithmDecompressor(algo Algorithm) DecompressorFunc {
  return algo.decompressor
 }
 
 // AlgorithmPrefix returns the prefix field of algo.
-// This is a function instead of a public method so that it is only callable from by code
+// This is a function instead of a public method so that it is only callable by code
 // that is allowed to import this internal subpackage.
 func AlgorithmPrefix(algo Algorithm) []byte {
  return algo.prefix

storage/storage_src.go

@@ -293,7 +293,7 @@ func buildLayerInfosForCopy(manifestInfos []manifest.LayerInfo, physicalInfos []
  if nextPhysical >= len(physicalInfos) {
  return nil, fmt.Errorf("expected more than %d physical layers to exist", len(physicalInfos))
  }
- res[i] = physicalInfos[nextPhysical]
+ res[i] = physicalInfos[nextPhysical] // FIXME? Should we preserve more data in manifestInfos? Notably the current approach correctly removes zstd:chunked metadata annotations.
  nextPhysical++
  }
  }