diff --git a/cnb_image.go b/cnb_image.go index cac042cf..2348ab46 100644 --- a/cnb_image.go +++ b/cnb_image.go @@ -10,7 +10,6 @@ import ( v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/mutate" "github.com/google/go-containerregistry/pkg/v1/tarball" - "github.com/google/go-containerregistry/pkg/v1/validate" ) // CNBImageCore wraps a v1.Image and provides most of the methods necessary for the image to satisfy the Image interface. @@ -184,11 +183,6 @@ func (i *CNBImageCore) UnderlyingImage() v1.Image { return i.Image } -func (i *CNBImageCore) Valid() bool { - err := validate.Image(i.Image) - return err == nil -} - // TBD Deprecated: Variant func (i *CNBImageCore) Variant() (string, error) { configFile, err := getConfigFile(i.Image) diff --git a/layout/layout.go b/layout/layout.go index dd40b75b..b8100bb5 100644 --- a/layout/layout.go +++ b/layout/layout.go @@ -66,6 +66,11 @@ func (i *Image) Identifier() (imgutil.Identifier, error) { return newLayoutIdentifier(i.repoPath, hash) } +func (i *Image) Valid() bool { + // layout images may be invalid if they are missing layer data + return true +} + func (i *Image) Delete() error { return os.RemoveAll(i.repoPath) } diff --git a/local/local.go b/local/local.go index e18cf276..29017377 100644 --- a/local/local.go +++ b/local/local.go @@ -46,6 +46,11 @@ func (i *Image) Identifier() (imgutil.Identifier, error) { }, nil } +func (i *Image) Valid() bool { + // local images are actually always invalid, because they are missing a manifest, but let's ignore that for now + return true +} + // GetLayer returns an io.ReadCloser with uncompressed layer data. // The layer will always have data, even if that means downloading ALL the image layers from the daemon. func (i *Image) GetLayer(diffID string) (io.ReadCloser, error) { diff --git a/local/local_test.go b/local/local_test.go index 148591c0..134248fa 100644 --- a/local/local_test.go +++ b/local/local_test.go @@ -2101,10 +2101,13 @@ func testImage(t *testing.T, when spec.G, it spec.S) { }) when("previous image is configured and layers are reused", func() { + var prevImageName string + it.Before(func() { var err error - prevImg, err := local.NewImage(newTestImageName(), dockerClient) + prevImageName = newTestImageName() + prevImg, err := local.NewImage(prevImageName, dockerClient) h.AssertNil(t, err) prevImgBase, err := h.CreateSingleFileLayerTar("/root", "root", daemonOS) @@ -2112,7 +2115,6 @@ func testImage(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, prevImg.AddLayer(prevImgBase)) h.AssertNil(t, prevImg.Save()) - defer h.DockerRmi(dockerClient, prevImg.Name()) img, err = local.NewImage(repoName, dockerClient, local.WithPreviousImage(prevImg.Name())) h.AssertNil(t, err) @@ -2131,6 +2133,10 @@ func testImage(t *testing.T, when spec.G, it spec.S) { }) it("creates an archive that can be imported and has correct diffIDs", saveFileTest) + + it.After(func() { + defer h.DockerRmi(dockerClient, prevImageName) + }) }) when("base image is configured", func() { diff --git a/local/store.go b/local/store.go index 8811ad6f..22c62ffe 100644 --- a/local/store.go +++ b/local/store.go @@ -216,24 +216,10 @@ func (s *Store) addImageToTar(tw *tar.Writer, image v1.Image, withName string) e blankIdx int ) for _, layer := range layers { - var layerName string - size, err := layer.Size() + layerName, err := s.addLayerToTar(tw, layer, &blankIdx) if err != nil { return err } - if size == -1 { // layer facade fronting empty layer - layerName = fmt.Sprintf("blank_%d", blankIdx) - blankIdx++ - hdr := &tar.Header{Name: layerName, Mode: 0644, Size: 0} - if err := tw.WriteHeader(hdr); err != nil { - return err - } - } else { - layerName, err = s.addLayerToTar(tw, layer) - if err != nil { - return err - } - } layerPaths = append(layerPaths, layerName) } @@ -250,32 +236,46 @@ func (s *Store) addImageToTar(tw *tar.Writer, image v1.Image, withName string) e return addTextToTar(tw, manifestJSON, "manifest.json") } -func (s *Store) addLayerToTar(tw *tar.Writer, layer v1.Layer) (string, error) { - layerDiffID, err := layer.DiffID() +func (s *Store) addLayerToTar(tw *tar.Writer, layer v1.Layer, blankIdx *int) (string, error) { + // If the layer is a previous image layer that hasn't been downloaded yet, + // cause ALL the previous image layers to be downloaded by grabbing the ReadCloser. + layerReader, err := layer.Uncompressed() if err != nil { return "", err } - withName := fmt.Sprintf("/%s.tar", layerDiffID.String()) + defer layerReader.Close() - uncompressedSize, err := s.getLayerSize(layer) + var layerName string + size, err := layer.Size() if err != nil { return "", err } - hdr := &tar.Header{Name: withName, Mode: 0644, Size: uncompressedSize} - if err = tw.WriteHeader(hdr); err != nil { + if size == -1 { // it's a base (always empty) layer + layerName = fmt.Sprintf("blank_%d", blankIdx) + *blankIdx++ + hdr := &tar.Header{Name: layerName, Mode: 0644, Size: 0} + return layerName, tw.WriteHeader(hdr) + } + // it's a populated layer + layerDiffID, err := layer.DiffID() + if err != nil { return "", err } + layerName = fmt.Sprintf("/%s.tar", layerDiffID.String()) - layerReader, err := layer.Uncompressed() + uncompressedSize, err := s.getLayerSize(layer) if err != nil { return "", err } - defer layerReader.Close() + hdr := &tar.Header{Name: layerName, Mode: 0644, Size: uncompressedSize} + if err = tw.WriteHeader(hdr); err != nil { + return "", err + } if _, err = io.Copy(tw, layerReader); err != nil { return "", err } - return withName, nil + return layerName, nil } // getLayerSize returns the uncompressed layer size. @@ -405,7 +405,7 @@ func (s *Store) doDownloadLayersFor(identifier string) error { imageReader, err := s.dockerClient.ImageSave(ctx, []string{identifier}) if err != nil { - return fmt.Errorf("saving base image with ID %q from the docker daemon: %w", identifier, err) + return fmt.Errorf("saving image with ID %q from the docker daemon: %w", identifier, err) } defer ensureReaderClosed(imageReader) diff --git a/local/v1_facade.go b/local/v1_facade.go index 13678ba7..0feeae77 100644 --- a/local/v1_facade.go +++ b/local/v1_facade.go @@ -182,9 +182,9 @@ func toV1Config(dockerCfg *container.Config) v1.Config { var _ v1.Layer = &v1LayerFacade{} type v1LayerFacade struct { - diffID v1.Hash - uncompressed func() (io.ReadCloser, error) - size func() (int64, error) + diffID v1.Hash + uncompressed func() (io.ReadCloser, error) + uncompressedSize func() (int64, error) } func newEmptyLayer(diffID v1.Hash, store *Store) *v1LayerFacade { @@ -197,7 +197,7 @@ func newEmptyLayer(diffID v1.Hash, store *Store) *v1LayerFacade { } return io.NopCloser(bytes.NewReader([]byte{})), nil }, - size: func() (int64, error) { + uncompressedSize: func() (int64, error) { layer, err := store.LayerByDiffID(diffID) if err == nil { return layer.Size() @@ -224,24 +224,17 @@ func newDownloadableEmptyLayer(diffID v1.Hash, store *Store, imageID string) *v1 } return nil, err }, - size: func() (int64, error) { + uncompressedSize: func() (int64, error) { layer, err := store.LayerByDiffID(diffID) if err == nil { return layer.Size() } - if err = store.downloadLayersFor(imageID); err != nil { - return -1, err - } - layer, err = store.LayerByDiffID(diffID) - if err == nil { - return layer.Size() - } - return -1, err + return -1, nil }, } } -func newPopulatedLayer(diffID v1.Hash, fromPath string, sentinelSize int64) *v1LayerFacade { +func newPopulatedLayer(diffID v1.Hash, fromPath string, uncompressedSize int64) *v1LayerFacade { return &v1LayerFacade{ diffID: diffID, uncompressed: func() (io.ReadCloser, error) { @@ -251,8 +244,8 @@ func newPopulatedLayer(diffID v1.Hash, fromPath string, sentinelSize int64) *v1L } return f, nil }, - size: func() (int64, error) { - return sentinelSize, nil + uncompressedSize: func() (int64, error) { + return uncompressedSize, nil }, } } @@ -285,9 +278,10 @@ func (l *v1LayerFacade) Uncompressed() (io.ReadCloser, error) { return l.uncompressed() } -// Size returns a sentinel value indicating if the layer has data. +// Size returns the uncompressed size. +// If the layer is missing local data, it returns a sentinel value of -1. func (l *v1LayerFacade) Size() (int64, error) { - return l.size() + return l.uncompressedSize() } func (l *v1LayerFacade) MediaType() (v1types.MediaType, error) { diff --git a/remote/remote.go b/remote/remote.go index a031137c..dd03b5a9 100644 --- a/remote/remote.go +++ b/remote/remote.go @@ -74,10 +74,6 @@ func (i *Image) Identifier() (imgutil.Identifier, error) { } // Valid returns true if the (saved) image is valid. -// It differs from CNBImageCore.Valid() in that the latter validates the current working image (not what is already saved). -// Additionally, when the repoName for the image is a manifest list, this method validates the entire index. -// Finally, this method uses validate.Fast, whereas CNBImageCore.Valid() does not. -// FIXME: see if this can be combined with CNBImageCore.Valid() func (i *Image) Valid() bool { return i.valid() == nil }