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.

Sign up for GitHub

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

Fix: archive/tar: write too long error when using Docker Desktop containerd backend #275

Merged
merged 1 commit into from
Jun 3, 2024
Merged

Fix: archive/tar: write too long error when using Docker Desktop containerd backend #275

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
52 changes: 5 additions & 47 deletions local/local.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
package local

import (
"crypto/sha256"
"encoding/hex"
"errors"
"fmt"
"io"
"os"
"path/filepath"
"strings"

v1 "github.com/google/go-containerregistry/pkg/v1"
Expand Down Expand Up @@ -110,67 +106,29 @@ func (i *Image) SetOS(osVal string) error {
var emptyHistory = v1.History{Created: v1.Time{Time: imgutil.NormalizedDateTime}}

func (i *Image) AddLayer(path string) error {
diffID, err := calculateChecksum(path)
if err != nil {
return err
}
layer, err := i.addLayerToStore(path, diffID)
layer, err := i.store.AddLayer(path)
if err != nil {
return err
}
return i.AddLayerWithHistory(layer, emptyHistory)
}

func calculateChecksum(path string) (string, error) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function relies on the daemon always outputting uncompressed layers, which is no longer true

f, err := os.Open(filepath.Clean(path))
if err != nil {
return "", fmt.Errorf("failed to open layer at path %s: %w", path, err)
}
defer f.Close()
hasher := sha256.New()
if _, err := io.Copy(hasher, f); err != nil {
return "", fmt.Errorf("failed to calculate checksum for layer at path %s: %w", path, err)
}
return "sha256:" + hex.EncodeToString(hasher.Sum(make([]byte, 0, hasher.Size()))), nil
}

func (i *Image) AddLayerWithDiffID(path, diffID string) error {
layer, err := i.addLayerToStore(path, diffID)
func (i *Image) AddLayerWithDiffID(path, _ string) error {
layer, err := i.store.AddLayer(path)
if err != nil {
return err
}
return i.AddLayerWithHistory(layer, emptyHistory)
}

func (i *Image) AddLayerWithDiffIDAndHistory(path, diffID string, history v1.History) error {
layer, err := i.addLayerToStore(path, diffID)
func (i *Image) AddLayerWithDiffIDAndHistory(path, _ string, history v1.History) error {
layer, err := i.store.AddLayer(path)
if err != nil {
return err
}
return i.AddLayerWithHistory(layer, history)
}

func (i *Image) addLayerToStore(fromPath, withDiffID string) (v1.Layer, error) {
var (
layer v1.Layer
err error
)
diffID, err := v1.NewHash(withDiffID)
if err != nil {
return nil, err
}
layer = newPopulatedLayer(diffID, fromPath, 1)
if err != nil {
return nil, err
}
fi, err := os.Stat(fromPath)
if err != nil {
return nil, err
}
i.store.AddLayer(layer, diffID, fi.Size())
return layer, nil
}

func (i *Image) AddOrReuseLayerWithHistory(path string, diffID string, history v1.History) error {
prevLayerExists, err := i.PreviousImageHasLayer(diffID)
if err != nil {
Expand Down
54 changes: 40 additions & 14 deletions local/store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/docker/docker/pkg/jsonmessage"
registryName "github.com/google/go-containerregistry/pkg/name"
v1 "github.com/google/go-containerregistry/pkg/v1"
"github.com/google/go-containerregistry/pkg/v1/tarball"
"golang.org/x/sync/errgroup"

"github.com/buildpacks/imgutil"
Expand Down Expand Up @@ -288,11 +289,13 @@ func (s *Store) getLayerSize(layer v1.Layer) (int64, error) {
return 0, err
}
knownLayer, layerFound := s.onDiskLayersByDiffID[diffID]
if layerFound {
if layerFound && knownLayer.uncompressedSize != -1 {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the real fix

return knownLayer.uncompressedSize, nil
}
// FIXME: this is a time sink and should be avoided if the daemon accepts OCI layout-formatted tars
// If layer was not seen previously, we need to read it to get the uncompressed size
// In practice, we should not get here
// In practice, we should only get here if layers saved from the daemon via `docker save`
// are output compressed.
layerReader, err := layer.Uncompressed()
if err != nil {
return 0, err
Expand Down Expand Up @@ -450,18 +453,11 @@ func (s *Store) doDownloadLayersFor(identifier string) error {
return err
}

for idx, diffID := range configFile.RootFS.DiffIDs {
for idx := range configFile.RootFS.DiffIDs {
layerPath := filepath.Join(tmpDir, manifest[0].Layers[idx])
hash, err := v1.NewHash(diffID)
if err != nil {
if _, err := s.AddLayer(layerPath); err != nil {
return err
}
layer := newPopulatedLayer(hash, layerPath, 1)
fi, err := os.Stat(layerPath)
if err != nil {
return err
}
s.AddLayer(layer, hash, fi.Size())
}
return nil
}
Expand Down Expand Up @@ -546,9 +542,39 @@ func (s *Store) findLayer(withHash v1.Hash) v1.Layer {
return aLayer.layer
}

func (s *Store) AddLayer(layer v1.Layer, withDiffID v1.Hash, withSize int64) {
s.onDiskLayersByDiffID[withDiffID] = annotatedLayer{
func (s *Store) AddLayer(fromPath string) (v1.Layer, error) {
layer, err := tarball.LayerFromFile(fromPath)
if err != nil {
return nil, err
}
diffID, err := layer.DiffID()
if err != nil {
return nil, err
}
var uncompressedSize int64
fileSize, err := func() (int64, error) {
fi, err := os.Stat(fromPath)
if err != nil {
return -1, err
}
return fi.Size(), nil
}()
if err != nil {
return nil, err
}
compressedSize, err := layer.Size()
if err != nil {
return nil, err
}
if fileSize == compressedSize {
// the layer is compressed, we don't know the uncompressed size
uncompressedSize = -1
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the real fix

} else {
uncompressedSize = fileSize
}
s.onDiskLayersByDiffID[diffID] = annotatedLayer{
layer: layer,
uncompressedSize: withSize,
uncompressedSize: uncompressedSize,
}
return layer, nil
}
17 changes: 0 additions & 17 deletions local/v1_facade.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"bytes"
"fmt"
"io"
"os"
"time"

"github.com/docker/docker/api/types"
Expand Down Expand Up @@ -234,22 +233,6 @@ func newDownloadableEmptyLayer(diffID v1.Hash, store *Store, imageID string) *v1
}
}

func newPopulatedLayer(diffID v1.Hash, fromPath string, uncompressedSize int64) *v1LayerFacade {
return &v1LayerFacade{
diffID: diffID,
uncompressed: func() (io.ReadCloser, error) {
f, err := os.Open(fromPath)
if err != nil {
return nil, err
}
return f, nil
},
uncompressedSize: func() (int64, error) {
return uncompressedSize, nil
},
}
}

func newEmptyLayerListFrom(configFile *v1.ConfigFile, downloadOnAccess bool, withStore *Store, withImageIdentifier string) []v1.Layer {
layers := make([]v1.Layer, len(configFile.RootFS.DiffIDs))
for idx, diffID := range configFile.RootFS.DiffIDs {
Expand Down
Loading