Skip to content

Commit

Permalink
Don't look for the binary digest when pulling layers
Browse files Browse the repository at this point in the history
This code path is usually never triggered because
the annotations are present; and it was broken until recently.

Remove it to simplify the code and analysis.

Signed-off-by: Miloslav Trmač <[email protected]>
  • Loading branch information
mtrmac committed Apr 22, 2024
1 parent 2431799 commit 9fbd0e0
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 43 deletions.
48 changes: 8 additions & 40 deletions pkg/chunked/compression_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,48 +132,16 @@ func readEstargzChunkedManifest(blobStream ImageSourceSeekable, blobSize int64,
return manifestUncompressed, tocOffset, nil
}

// readZstdChunkedManifest reads the zstd:chunked manifest from the seekable stream blobStream. The blob total size must
// be specified.
// This function uses the io.github.containers.zstd-chunked. annotations when specified.
func readZstdChunkedManifest(blobStream ImageSourceSeekable, blobSize int64, tocDigest digest.Digest, annotations map[string]string) ([]byte, []byte, int64, error) {
footerSize := int64(internal.FooterSizeSupported)
if blobSize <= footerSize {
return nil, nil, 0, errors.New("blob too small")
// readZstdChunkedManifest reads the zstd:chunked manifest from the seekable stream blobStream.
func readZstdChunkedManifest(blobStream ImageSourceSeekable, tocDigest digest.Digest, annotations map[string]string) ([]byte, []byte, int64, error) {
offsetMetadata := annotations[internal.ManifestInfoKey]
if offsetMetadata == "" {
return nil, nil, 0, fmt.Errorf("%q annotation missing", internal.ManifestInfoKey)
}

var footerData internal.ZstdChunkedFooterData

if offsetMetadata := annotations[internal.ManifestInfoKey]; offsetMetadata != "" {
var err error
footerData, err = internal.ReadFooterDataFromAnnotations(annotations)
if err != nil {
return nil, nil, 0, err
}
} else {
chunk := ImageSourceChunk{
Offset: uint64(blobSize - footerSize),
Length: uint64(footerSize),
}
parts, errs, err := blobStream.GetBlobAt([]ImageSourceChunk{chunk})
if err != nil {
return nil, nil, 0, err
}
var reader io.ReadCloser
select {
case r := <-parts:
reader = r
case err := <-errs:
return nil, nil, 0, err
}
footer := make([]byte, footerSize)
if _, err := io.ReadFull(reader, footer); err != nil {
return nil, nil, 0, err
}

footerData, err = internal.ReadFooterDataFromBlob(footer)
if err != nil {
return nil, nil, 0, err
}
footerData, err := internal.ReadFooterDataFromAnnotations(annotations)
if err != nil {
return nil, nil, 0, err
}

if footerData.ManifestType != internal.ManifestTypeCRFS {
Expand Down
5 changes: 5 additions & 0 deletions pkg/chunked/internal/compression.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,11 @@ func ZstdWriterWithLevel(dest io.Writer, level int) (*zstd.Encoder, error) {
}

// ZstdChunkedFooterData contains all the data stored in the zstd:chunked footer.
// This footer exists to make the blobs self-describing, our implementation
// never reads it:
// Partial pull security hinges on the TOC digest, and that exists as a layer annotation;
// so we are relying on the layer annotations anyway, and doing so means we can avoid
// a round-trip to fetch this binary footer.
type ZstdChunkedFooterData struct {
ManifestType uint64

Expand Down
4 changes: 2 additions & 2 deletions pkg/chunked/storage_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ func makeConvertFromRawDiffer(ctx context.Context, store storage.Store, blobDige
}

func makeZstdChunkedDiffer(ctx context.Context, store storage.Store, blobSize int64, tocDigest digest.Digest, annotations map[string]string, iss ImageSourceSeekable, storeOpts *types.StoreOptions) (*chunkedDiffer, error) {
manifest, tarSplit, tocOffset, err := readZstdChunkedManifest(iss, blobSize, tocDigest, annotations)
manifest, tarSplit, tocOffset, err := readZstdChunkedManifest(iss, tocDigest, annotations)
if err != nil {
return nil, fmt.Errorf("read zstd:chunked manifest: %w", err)
}
Expand Down Expand Up @@ -1701,7 +1701,7 @@ func (c *chunkedDiffer) ApplyDiff(dest string, options *archive.TarOptions, diff
if tocDigest == nil {
return graphdriver.DriverWithDifferOutput{}, fmt.Errorf("internal error: just-created zstd:chunked missing TOC digest")
}
manifest, tarSplit, tocOffset, err := readZstdChunkedManifest(fileSource, c.blobSize, *tocDigest, annotations)
manifest, tarSplit, tocOffset, err := readZstdChunkedManifest(fileSource, *tocDigest, annotations)
if err != nil {
return graphdriver.DriverWithDifferOutput{}, fmt.Errorf("read zstd:chunked manifest: %w", err)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/chunked/zstdchunked_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ func TestGenerateAndParseManifest(t *testing.T) {
tocDigest, err := toc.GetTOCDigest(annotations)
require.NoError(t, err)
require.NotNil(t, tocDigest)
manifest, _, _, err := readZstdChunkedManifest(s, 8192, *tocDigest, annotations)
manifest, _, _, err := readZstdChunkedManifest(s, *tocDigest, annotations)
if err != nil {
t.Error(err)
}
Expand Down

0 comments on commit 9fbd0e0

Please sign in to comment.