Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Apr 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions copy/compression.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand All @@ -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,
}
Expand Down
6 changes: 3 additions & 3 deletions copy/single.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion pkg/blobcache/src.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/compression/internal/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion storage/storage_src.go
Original file line number Diff line number Diff line change
Expand Up @@ -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++
}
}
Expand Down