Skip to content

Commit

Permalink
Fix: always preserve the digest when base image is provided and no me…
Browse files Browse the repository at this point in the history
…dia types (or other modifications)

are requested

Signed-off-by: Natalie Arellano <narellano@vmware.com>
  • Loading branch information
natalieparellano committed Feb 2, 2024
1 parent a65b1a9 commit 9cef4d4
Show file tree
Hide file tree
Showing 6 changed files with 177 additions and 71 deletions.
12 changes: 6 additions & 6 deletions cnb_image.go
Original file line number Diff line number Diff line change
Expand Up @@ -393,23 +393,23 @@ func (i *CNBImageCore) ReuseLayer(diffID string) error {
}
idx, err := getLayerIndex(diffID, i.previousImage)
if err != nil {
return err
return fmt.Errorf("failed to get layer index: %w", err)
}
previousHistory, err := getHistory(idx, i.previousImage)
if err != nil {
return err
return fmt.Errorf("failed to get history: %w", err)
}
return i.ReuseLayerWithHistory(diffID, previousHistory)
}

func getLayerIndex(forDiffID string, fromImage v1.Image) (int, error) {
layerHash, err := v1.NewHash(forDiffID)
if err != nil {
return -1, err
return -1, fmt.Errorf("failed to get layer hash: %w", err)
}
configFile, err := getConfigFile(fromImage)
if err != nil {
return -1, err
return -1, fmt.Errorf("failed to get config file: %w", err)
}
for idx, configHash := range configFile.RootFS.DiffIDs {
if layerHash.String() == configHash.String() {
Expand All @@ -433,11 +433,11 @@ func getHistory(forIndex int, fromImage v1.Image) (v1.History, error) {
func (i *CNBImageCore) ReuseLayerWithHistory(diffID string, history v1.History) error {
layerHash, err := v1.NewHash(diffID)
if err != nil {
return err
return fmt.Errorf("failed to get layer hash: %w", err)
}
layer, err := i.previousImage.LayerByDiffID(layerHash)
if err != nil {
return err
return fmt.Errorf("failed to get layer by diffID: %w", err)
}
if i.preserveHistory {
history.Created = v1.Time{Time: i.createdAt}
Expand Down
29 changes: 29 additions & 0 deletions layout/layout_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,16 @@ func testImage(t *testing.T, when spec.G, it spec.S) {
h.AssertError(t, err, "has no layers")
})
})

when("existing config has extra fields", func() {
it("returns an unmodified digest", func() {
img, err := layout.NewImage(imagePath, layout.FromBaseImagePath(filepath.Join("testdata", "layout", "busybox-latest")))
h.AssertNil(t, err)
digest, err := img.Digest()
h.AssertNil(t, err)
h.AssertEq(t, digest.String(), "sha256:f75f3d1a317fc82c793d567de94fc8df2bece37acd5f2bd364a0d91a0d1f3dab")
})
})
})

when("#WithMediaTypes", func() {
Expand All @@ -273,6 +283,25 @@ func testImage(t *testing.T, when spec.G, it spec.S) {
h.AssertNil(t, img.Save())
h.AssertDockerMediaTypes(t, img) // after saving
})

when("using a sparse image", func() {
it("sets the requested media types", func() {
img, err := layout.NewImage(
imagePath,
layout.FromBaseImagePath(sparseBaseImagePath),
layout.WithMediaTypes(imgutil.OCITypes),
)
h.AssertNil(t, err)
h.AssertOCIMediaTypes(t, img) // before saving
// add a random layer
path, diffID, _ := h.RandomLayer(t, tmpDir)
err = img.AddLayerWithDiffID(path, diffID)
h.AssertNil(t, err)
h.AssertOCIMediaTypes(t, img) // after adding a layer
h.AssertNil(t, img.Save())
h.AssertOCIMediaTypes(t, img) // after saving
})
})
})

when("#WithPreviousImage", func() {
Expand Down
38 changes: 17 additions & 21 deletions layout/new.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,25 +13,33 @@ func NewImage(path string, ops ...ImageOption) (*Image, error) {
for _, op := range ops {
op(options)
}

options.Platform = processDefaultPlatformOption(options.Platform)

var err error
if options.PreviousImageRepoName != "" {
options.PreviousImage, err = newImageFromPath(options.PreviousImageRepoName, options.Platform, imgutil.DefaultTypes)

if options.BaseImage == nil && options.BaseImageRepoName != "" { // options.BaseImage supersedes options.BaseImageRepoName
options.BaseImage, err = newImageFromPath(options.BaseImageRepoName, options.Platform)
if err != nil {
return nil, err
}
}
options.MediaTypes = imgutil.GetPreferredMediaTypes(*options)
if options.BaseImage != nil {
options.BaseImage, err = newImageFacadeFrom(options.BaseImage, options.MediaTypes)
if err != nil {
return nil, err
}
}

if options.BaseImage == nil && options.BaseImageRepoName != "" { // options.BaseImage supersedes options.BaseImageRepoName
options.BaseImage, err = newImageFromPath(options.BaseImageRepoName, options.Platform, imgutil.DefaultTypes)
if options.PreviousImageRepoName != "" {
options.PreviousImage, err = newImageFromPath(options.PreviousImageRepoName, options.Platform)
if err != nil {
return nil, err
}
}
options.MediaTypes = imgutil.GetPreferredMediaTypes(*options)
if options.BaseImage != nil {
options.BaseImage, err = imgutil.EnsureMediaTypes(options.BaseImage, options.MediaTypes) // FIXME: this can move into imgutil constructor
if options.PreviousImage != nil {
options.PreviousImage, err = newImageFacadeFrom(options.PreviousImage, options.MediaTypes)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -63,7 +71,7 @@ func processDefaultPlatformOption(requestedPlatform imgutil.Platform) imgutil.Pl
// newImageFromPath creates a layout image from the given path.
// * If an image index for multiple platforms exists, it will try to select the image according to the platform provided.
// * If the image does not exist, then nothing is returned.
func newImageFromPath(path string, withPlatform imgutil.Platform, withMediaTypes imgutil.MediaTypes) (v1.Image, error) {
func newImageFromPath(path string, withPlatform imgutil.Platform) (v1.Image, error) {
if !imageExists(path) {
return nil, nil
}
Expand All @@ -80,19 +88,7 @@ func newImageFromPath(path string, withPlatform imgutil.Platform, withMediaTypes
if err != nil {
return nil, fmt.Errorf("failed to load image from index: %w", err)
}

// ensure layers will not error when accessed if there is no underlying data
manifestFile, err := image.Manifest()
if err != nil {
return nil, err
}
configFile, err := image.ConfigFile()
if err != nil {
return nil, err
}
return imgutil.EnsureMediaTypesAndLayers(image, withMediaTypes, func(idx int, layer v1.Layer) (v1.Layer, error) {
return newLayerOrFacadeFrom(*configFile, *manifestFile, idx, layer)
})
return image, nil
}

// imageFromIndex creates a v1.Image from the given Image Index, selecting the image manifest
Expand Down
94 changes: 94 additions & 0 deletions layout/v1_facade.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,102 @@ import (
"io"

v1 "github.com/google/go-containerregistry/pkg/v1"

"github.com/buildpacks/imgutil"
)

type v1ImageFacade struct {
v1.Image
diffIDMap map[v1.Hash]v1.Layer
digestMap map[v1.Hash]v1.Layer
}

func newImageFacadeFrom(original v1.Image, withMediaTypes imgutil.MediaTypes) (v1.Image, error) {
configFile, err := original.ConfigFile()
if err != nil {
return nil, fmt.Errorf("failed to get config: %w", err)
}
manifestFile, err := original.Manifest()
if err != nil {
return nil, fmt.Errorf("failed to get manifest: %w", err)
}
originalLayers, err := original.Layers()
if err != nil {
return nil, fmt.Errorf("failed to get layers: %w", err)
}

ensureLayers := func(idx int, layer v1.Layer) (v1.Layer, error) {
return newLayerOrFacadeFrom(*configFile, *manifestFile, idx, layer)
}
// first, ensure media types
image, mutated, err := imgutil.EnsureMediaTypesAndLayers(original, withMediaTypes, ensureLayers) // if no media types are requested, this does nothing
if err != nil {
return nil, fmt.Errorf("failed to ensure media types: %w", err)
}
// then, ensure layers
if mutated {
// layers are wrapped in a facade, it is possible to call layer.Compressed or layer.Uncompressed without error
return image, nil
}
// we didn't mutate the image (possibly to preserve the digest), we must wrap the image in a facade
facade := &v1ImageFacade{
Image: original,
diffIDMap: make(map[v1.Hash]v1.Layer),
digestMap: make(map[v1.Hash]v1.Layer),
}
for idx, l := range originalLayers {
layer, err := newLayerOrFacadeFrom(*configFile, *manifestFile, idx, l)
if err != nil {
return nil, err
}
diffID, err := layer.DiffID()
if err != nil {
return nil, err
}
facade.diffIDMap[diffID] = layer
digest, err := layer.Digest()
if err != nil {
return nil, err
}
facade.digestMap[digest] = layer
}

return facade, nil
}

func (i *v1ImageFacade) Layers() ([]v1.Layer, error) {
var layers []v1.Layer
configFile, err := i.ConfigFile()
if err != nil {
return nil, err
}
if configFile == nil {
return nil, nil
}
for _, diffID := range configFile.RootFS.DiffIDs {
l, err := i.LayerByDiffID(diffID)
if err != nil {
return nil, err
}
layers = append(layers, l)
}
return layers, nil
}

func (i *v1ImageFacade) LayerByDiffID(h v1.Hash) (v1.Layer, error) {
if layer, ok := i.diffIDMap[h]; ok {
return layer, nil
}
return nil, fmt.Errorf("failed to find layer with diffID %s", h) // shouldn't get here
}

func (i *v1ImageFacade) LayerByDigest(h v1.Hash) (v1.Layer, error) {
if layer, ok := i.digestMap[h]; ok {
return layer, nil
}
return nil, fmt.Errorf("failed to find layer with digest %s", h) // shouldn't get here
}

type v1LayerFacade struct {
v1.Layer
diffID v1.Hash
Expand Down
Loading

0 comments on commit 9cef4d4

Please sign in to comment.