From efd1f09d95489d5a2cf11d5fbbeb31c2b1c01638 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Thu, 25 Jan 2024 14:57:30 -0500 Subject: [PATCH 01/14] Remove ImageStore interface The details of storing images can be left up to each implementation Signed-off-by: Natalie Arellano --- cnb_image.go | 26 +++----------------------- locallayout/image.go | 15 +++++++++------ locallayout/new.go | 5 +++-- locallayout/store.go | 2 -- locallayout/v1_facade.go | 2 +- new.go | 3 +-- 6 files changed, 17 insertions(+), 36 deletions(-) diff --git a/cnb_image.go b/cnb_image.go index d36b2f53..54426fd3 100644 --- a/cnb_image.go +++ b/cnb_image.go @@ -19,7 +19,6 @@ import ( // but in practice will start off as a pointer to a locallayout.v1ImageFacade (or similar). type CNBImageCore struct { v1.Image // the working image - Store ImageStore // required repoName string // optional @@ -28,16 +27,6 @@ type CNBImageCore struct { previousImage v1.Image } -type ImageStore interface { - Contains(identifier string) bool - Delete(identifier string) error - Save(image IdentifiableV1Image, withName string, withAdditionalNames ...string) (string, error) - SaveFile(image IdentifiableV1Image, withName string) (string, error) - - DownloadLayersFor(identifier string) error - Layers() []v1.Layer -} - type IdentifiableV1Image interface { v1.Image Identifier() (Identifier, error) @@ -105,15 +94,6 @@ func (i *CNBImageCore) History() ([]v1.History, error) { return configFile.History, nil } -func (i *CNBImageCore) Kind() string { - storeType := fmt.Sprintf("%T", i.Store) - parts := strings.Split(storeType, ".") - if len(parts) < 2 { - return storeType - } - return strings.TrimPrefix(parts[0], "*") -} - // Deprecated: Label func (i *CNBImageCore) Label(key string) (string, error) { configFile, err := getConfigFile(i.Image) @@ -208,6 +188,9 @@ func (i *CNBImageCore) AnnotateRefName(refName string) error { if err != nil { return err } + if manifest.Annotations == nil { + manifest.Annotations = make(map[string]string) + } manifest.Annotations["org.opencontainers.image.ref.name"] = refName mutated := mutate.Annotations(i.Image, manifest.Annotations) image, ok := mutated.(v1.Image) @@ -346,9 +329,6 @@ func (i *CNBImageCore) AddLayerWithDiffIDAndHistory(path, _ string, history v1.H } func (i *CNBImageCore) Rebase(baseTopLayerDiffID string, withNewBase Image) error { - if i.Kind() != withNewBase.Kind() { - return fmt.Errorf("expected new base to be a %s image; got %s", i.Kind(), withNewBase.Kind()) - } newBase := withNewBase.UnderlyingImage() // FIXME: when all imgutil.Images are v1.Images, we can remove this part var err error i.Image, err = mutate.Rebase(i.Image, i.newV1ImageFacade(baseTopLayerDiffID), newBase) diff --git a/locallayout/image.go b/locallayout/image.go index 99a27919..49101b07 100644 --- a/locallayout/image.go +++ b/locallayout/image.go @@ -15,12 +15,15 @@ import ( // Image wraps an imgutil.CNBImageCore and implements the methods needed to complete the imgutil.Image interface. type Image struct { *imgutil.CNBImageCore + store *Store lastIdentifier string daemonOS string downloadOnce *sync.Once } -var _ imgutil.Image = &Image{} +func (i *Image) Kind() string { + return "locallayout" +} func (i *Image) Found() bool { return i.lastIdentifier != "" @@ -69,7 +72,7 @@ func (i *Image) GetLayer(diffID string) (io.ReadCloser, error) { func (i *Image) ensureLayers() error { var err error i.downloadOnce.Do(func() { - err = i.Store.DownloadLayersFor(i.lastIdentifier) + err = i.store.DownloadLayersFor(i.lastIdentifier) }) if err != nil { return fmt.Errorf("fetching base layers: %w", err) @@ -93,20 +96,20 @@ func (i *Image) Rebase(baseTopLayerDiffID string, withNewBase imgutil.Image) err func (i *Image) Save(additionalNames ...string) error { var err error - i.lastIdentifier, err = i.Store.Save(i, i.Name(), additionalNames...) + i.lastIdentifier, err = i.store.Save(i, i.Name(), additionalNames...) return err } func (i *Image) SaveAs(name string, additionalNames ...string) error { var err error - i.lastIdentifier, err = i.Store.Save(i, name, additionalNames...) + i.lastIdentifier, err = i.store.Save(i, name, additionalNames...) return err } func (i *Image) SaveFile() (string, error) { - return i.Store.SaveFile(i, i.Name()) + return i.store.SaveFile(i, i.Name()) } func (i *Image) Delete() error { - return i.Store.Delete(i.lastIdentifier) + return i.store.Delete(i.lastIdentifier) } diff --git a/locallayout/new.go b/locallayout/new.go index a6eb4057..6ced96ff 100644 --- a/locallayout/new.go +++ b/locallayout/new.go @@ -33,7 +33,7 @@ func NewImage(repoName string, dockerClient DockerClient, ops ...func(*imgutil.I var ( baseIdentifier string - store imgutil.ImageStore = &Store{dockerClient: dockerClient} + store = &Store{dockerClient: dockerClient} ) baseImage, err := processBaseImageOption(options.BaseImageRepoName, dockerClient) if err != nil { @@ -45,13 +45,14 @@ func NewImage(repoName string, dockerClient DockerClient, ops ...func(*imgutil.I store = baseImage.store } - cnbImage, err := imgutil.NewCNBImage(repoName, store, *options) + cnbImage, err := imgutil.NewCNBImage(repoName, *options) if err != nil { return nil, err } return &Image{ CNBImageCore: cnbImage, + store: store, lastIdentifier: baseIdentifier, daemonOS: options.Platform.OS, downloadOnce: &sync.Once{}, diff --git a/locallayout/store.go b/locallayout/store.go index f001f6ff..27da0ba4 100644 --- a/locallayout/store.go +++ b/locallayout/store.go @@ -44,8 +44,6 @@ type DockerClient interface { ServerVersion(ctx context.Context) (types.Version, error) } -var _ imgutil.ImageStore = &Store{} - // images func (s *Store) Contains(identifier string) bool { diff --git a/locallayout/v1_facade.go b/locallayout/v1_facade.go index af8fbb61..7b97a548 100644 --- a/locallayout/v1_facade.go +++ b/locallayout/v1_facade.go @@ -30,7 +30,7 @@ type v1ImageFacade struct { emptyLayers []v1.Layer // for downloading layers from the daemon as needed - store imgutil.ImageStore + store *Store downloadLayersOnAccess bool // set to true to downloading ALL the image layers from the daemon when LayerByDiffID is called downloadOnce *sync.Once identifier string diff --git a/new.go b/new.go index 7a7c0d69..a28911cb 100644 --- a/new.go +++ b/new.go @@ -14,10 +14,9 @@ import ( "github.com/buildpacks/imgutil/layer" ) -func NewCNBImage(repoName string, store ImageStore, options ImageOptions) (*CNBImageCore, error) { +func NewCNBImage(repoName string, options ImageOptions) (*CNBImageCore, error) { image := &CNBImageCore{ Image: options.BaseImage, - Store: store, // required repoName: repoName, // optional From c3794ccc21aa4c2f31c2ba1b0060cb4fb85cf93b Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Mon, 29 Jan 2024 11:38:59 -0500 Subject: [PATCH 02/14] Refactor layout package to wrap a CNBImageCore Signed-off-by: Natalie Arellano --- cnb_image.go | 70 ++--- image.go | 130 +-------- layout/layout.go | 513 +---------------------------------- layout/layout_test.go | 7 +- layout/new.go | 234 ++++++---------- layout/options.go | 85 +++--- layout/save.go | 68 ++--- layout/sparse/new.go | 15 +- layout/sparse/options.go | 1 - layout/sparse/save.go | 38 --- layout/sparse/sparse.go | 15 - layout/sparse/sparse_test.go | 3 +- layout/v1_facade.go | 56 ++++ locallayout/image.go | 9 + locallayout/image_test.go | 2 +- locallayout/new.go | 3 +- locallayout/v1_facade.go | 27 +- new.go | 267 ++++++++++++++++-- options.go | 6 +- remote/new.go | 2 +- testhelpers/testhelpers.go | 8 +- 21 files changed, 528 insertions(+), 1031 deletions(-) delete mode 100644 layout/sparse/options.go delete mode 100644 layout/sparse/save.go delete mode 100644 layout/sparse/sparse.go create mode 100644 layout/v1_facade.go diff --git a/cnb_image.go b/cnb_image.go index 54426fd3..564932a2 100644 --- a/cnb_image.go +++ b/cnb_image.go @@ -3,6 +3,7 @@ package imgutil import ( "errors" "fmt" + "io" "strings" "time" @@ -18,10 +19,10 @@ import ( // The working image could be any v1.Image, // but in practice will start off as a pointer to a locallayout.v1ImageFacade (or similar). type CNBImageCore struct { - v1.Image // the working image // required - repoName string + v1.Image // the working image // optional + createdAt time.Time preferredMediaTypes MediaTypes preserveHistory bool previousImage v1.Image @@ -36,7 +37,7 @@ var _ v1.Image = &CNBImageCore{} // FIXME: mark deprecated methods as deprecated on the interface when other packages (remote, layout) expose a v1.Image -// Deprecated: Architecture +// TBD Deprecated: Architecture func (i *CNBImageCore) Architecture() (string, error) { configFile, err := getConfigFile(i.Image) if err != nil { @@ -45,7 +46,7 @@ func (i *CNBImageCore) Architecture() (string, error) { return configFile.Architecture, nil } -// Deprecated: CreatedAt +// TBD Deprecated: CreatedAt func (i *CNBImageCore) CreatedAt() (time.Time, error) { configFile, err := getConfigFile(i.Image) if err != nil { @@ -54,7 +55,7 @@ func (i *CNBImageCore) CreatedAt() (time.Time, error) { return configFile.Created.Time, nil } -// Deprecated: Entrypoint +// TBD Deprecated: Entrypoint func (i *CNBImageCore) Entrypoint() ([]string, error) { configFile, err := getConfigFile(i.Image) if err != nil { @@ -85,7 +86,19 @@ func (i *CNBImageCore) GetAnnotateRefName() (string, error) { return manifest.Annotations["org.opencontainers.image.ref.name"], nil } -// Deprecated: History +func (i *CNBImageCore) GetLayer(diffID string) (io.ReadCloser, error) { + hash, err := v1.NewHash(diffID) + if err != nil { + return nil, err + } + layer, err := i.LayerByDiffID(hash) + if err != nil { + return nil, err + } + return layer.Uncompressed() +} + +// TBD Deprecated: History func (i *CNBImageCore) History() ([]v1.History, error) { configFile, err := getConfigFile(i.Image) if err != nil { @@ -94,7 +107,7 @@ func (i *CNBImageCore) History() ([]v1.History, error) { return configFile.History, nil } -// Deprecated: Label +// TBD Deprecated: Label func (i *CNBImageCore) Label(key string) (string, error) { configFile, err := getConfigFile(i.Image) if err != nil { @@ -103,7 +116,7 @@ func (i *CNBImageCore) Label(key string) (string, error) { return configFile.Config.Labels[key], nil } -// Deprecated: Labels +// TBD Deprecated: Labels func (i *CNBImageCore) Labels() (map[string]string, error) { configFile, err := getConfigFile(i.Image) if err != nil { @@ -112,16 +125,12 @@ func (i *CNBImageCore) Labels() (map[string]string, error) { return configFile.Config.Labels, nil } -// Deprecated: ManifestSize +// TBD Deprecated: ManifestSize func (i *CNBImageCore) ManifestSize() (int64, error) { return i.Image.Size() } -func (i *CNBImageCore) Name() string { - return i.repoName -} - -// Deprecated: OS +// TBD Deprecated: OS func (i *CNBImageCore) OS() (string, error) { configFile, err := getConfigFile(i.Image) if err != nil { @@ -130,7 +139,7 @@ func (i *CNBImageCore) OS() (string, error) { return configFile.OS, nil } -// Deprecated: OSVersion +// TBD Deprecated: OSVersion func (i *CNBImageCore) OSVersion() (string, error) { configFile, err := getConfigFile(i.Image) if err != nil { @@ -145,7 +154,7 @@ func (i *CNBImageCore) TopLayer() (string, error) { return "", err } if len(layers) == 0 { - return "", fmt.Errorf("image %q has no layers", i.Name()) + return "", errors.New("image has no layers") } topLayer := layers[len(layers)-1] hex, err := topLayer.DiffID() @@ -165,7 +174,7 @@ func (i *CNBImageCore) Valid() bool { return err == nil } -// Deprecated: Variant +// TBD Deprecated: Variant func (i *CNBImageCore) Variant() (string, error) { configFile, err := getConfigFile(i.Image) if err != nil { @@ -174,7 +183,7 @@ func (i *CNBImageCore) Variant() (string, error) { return configFile.Variant, nil } -// Deprecated: WorkingDir +// TBD Deprecated: WorkingDir func (i *CNBImageCore) WorkingDir() (string, error) { configFile, err := getConfigFile(i.Image) if err != nil { @@ -201,25 +210,21 @@ func (i *CNBImageCore) AnnotateRefName(refName string) error { return nil } -func (i *CNBImageCore) Rename(name string) { - i.repoName = name -} - -// Deprecated: SetArchitecture +// TBD Deprecated: SetArchitecture func (i *CNBImageCore) SetArchitecture(architecture string) error { return i.MutateConfigFile(func(c *v1.ConfigFile) { c.Architecture = architecture }) } -// Deprecated: SetCmd +// TBD Deprecated: SetCmd func (i *CNBImageCore) SetCmd(cmd ...string) error { return i.MutateConfigFile(func(c *v1.ConfigFile) { c.Config.Cmd = cmd }) } -// Deprecated: SetEntrypoint +// TBD Deprecated: SetEntrypoint func (i *CNBImageCore) SetEntrypoint(ep ...string) error { return i.MutateConfigFile(func(c *v1.ConfigFile) { c.Config.Entrypoint = ep @@ -249,7 +254,7 @@ func (i *CNBImageCore) SetEnv(key, val string) error { }) } -// Deprecated: SetHistory +// TBD Deprecated: SetHistory func (i *CNBImageCore) SetHistory(histories []v1.History) error { return i.MutateConfigFile(func(c *v1.ConfigFile) { c.History = histories @@ -271,21 +276,21 @@ func (i *CNBImageCore) SetOS(osVal string) error { }) } -// Deprecated: SetOSVersion +// TBD Deprecated: SetOSVersion func (i *CNBImageCore) SetOSVersion(osVersion string) error { return i.MutateConfigFile(func(c *v1.ConfigFile) { c.OSVersion = osVersion }) } -// Deprecated: SetVariant +// TBD Deprecated: SetVariant func (i *CNBImageCore) SetVariant(variant string) error { return i.MutateConfigFile(func(c *v1.ConfigFile) { c.Variant = variant }) } -// Deprecated: SetWorkingDir +// TBD Deprecated: SetWorkingDir func (i *CNBImageCore) SetWorkingDir(dir string) error { return i.MutateConfigFile(func(c *v1.ConfigFile) { c.Config.WorkingDir = dir @@ -312,11 +317,8 @@ func (i *CNBImageCore) AddLayerWithDiffIDAndHistory(path, _ string, history v1.H if !i.preserveHistory { history = emptyHistory } - configFile, err := getConfigFile(i) - if err != nil { - return err - } - history.Created = configFile.Created + history.Created = v1.Time{Time: i.createdAt} + i.Image, err = mutate.Append( i.Image, mutate.Addendum{ diff --git a/image.go b/image.go index d6feaef0..87bd82c3 100644 --- a/image.go +++ b/image.go @@ -7,9 +7,7 @@ import ( "time" v1 "github.com/google/go-containerregistry/pkg/v1" - "github.com/google/go-containerregistry/pkg/v1/empty" "github.com/google/go-containerregistry/pkg/v1/mutate" - "github.com/google/go-containerregistry/pkg/v1/types" ) type Image interface { @@ -19,7 +17,7 @@ type Image interface { CreatedAt() (time.Time, error) Entrypoint() ([]string, error) Env(key string) (string, error) - // Found tells whether the image exists in the repository by `Name()`. + // Found reports if image exists in the image store with `Name()`. Found() bool GetAnnotateRefName() (string, error) // GetLayer retrieves layer by diff id. Returns a reader of the uncompressed contents of the layer. @@ -87,136 +85,16 @@ type Platform struct { OSVersion string } -type MediaTypes int - -const ( - MissingTypes MediaTypes = iota - DefaultTypes - OCITypes - DockerTypes -) - -func (t MediaTypes) ManifestType() types.MediaType { - switch t { - case OCITypes: - return types.OCIManifestSchema1 - case DockerTypes: - return types.DockerManifestSchema2 - default: - return "" - } -} - -func (t MediaTypes) ConfigType() types.MediaType { - switch t { - case OCITypes: - return types.OCIConfigJSON - case DockerTypes: - return types.DockerConfigJSON - default: - return "" - } -} - -func (t MediaTypes) LayerType() types.MediaType { - switch t { - case OCITypes: - return types.OCILayer - case DockerTypes: - return types.DockerLayer - default: - return "" - } -} - -// OverrideMediaTypes mutates the provided v1.Image to use the desired media types -// in the image manifest and config files (including the layers referenced in the manifest) -func OverrideMediaTypes(image v1.Image, mediaTypes MediaTypes) (v1.Image, error) { - if mediaTypes == DefaultTypes || mediaTypes == MissingTypes { - // without media types option, default to original media types - return image, nil - } - - // manifest media type - retImage := mutate.MediaType(empty.Image, mediaTypes.ManifestType()) - - // update empty image with image config - config, err := image.ConfigFile() - if err != nil { - return nil, err - } - history := config.History - // zero out diff IDs and history, as these will be updated when we call `mutate.Append` - config.RootFS.DiffIDs = make([]v1.Hash, 0) - config.History = []v1.History{} - retImage, err = mutate.ConfigFile(retImage, config) - if err != nil { - return nil, err - } - - // config media type - retImage = mutate.ConfigMediaType(retImage, mediaTypes.ConfigType()) - - // layers media type - layers, err := image.Layers() - if err != nil { - return nil, err - } - additions := layersAddendum(layers, history, mediaTypes.LayerType()) - retImage, err = mutate.Append(retImage, additions...) - if err != nil { - return nil, err - } - - return retImage, nil -} - // OverrideHistoryIfNeeded zeroes out the history if the number of history entries doesn't match the number of layers. func OverrideHistoryIfNeeded(image v1.Image) (v1.Image, error) { - configFile, err := image.ConfigFile() - if err != nil || configFile == nil { - return nil, fmt.Errorf("getting image config: %w", err) + configFile, err := getConfigFile(image) + if err != nil { + return nil, err } configFile.History = NormalizedHistory(configFile.History, len(configFile.RootFS.DiffIDs)) return mutate.ConfigFile(image, configFile) } -func NormalizedHistory(history []v1.History, nLayers int) []v1.History { - if history == nil { - return make([]v1.History, nLayers) - } - // ensure we remove history for empty layers - var nHistory []v1.History - for _, h := range history { - if !h.EmptyLayer { - nHistory = append(nHistory, h) - } - } - if len(nHistory) == nLayers { - return nHistory - } - return make([]v1.History, nLayers) -} - -// layersAddendum creates an Addendum array with the given layers -// and the desired media type -func layersAddendum(layers []v1.Layer, history []v1.History, mediaType types.MediaType) []mutate.Addendum { - additions := make([]mutate.Addendum, 0) - if len(history) != len(layers) { - history = make([]v1.History, len(layers)) - } - for idx, layer := range layers { - additions = append(additions, mutate.Addendum{ - Layer: layer, - History: history[idx], - MediaType: mediaType, - }) - } - return additions -} - -var NormalizedDateTime = time.Date(1980, time.January, 1, 0, 0, 1, 0, time.UTC) - type SaveDiagnostic struct { ImageName string Cause error diff --git a/layout/layout.go b/layout/layout.go index 715fc805..6c3f5e33 100644 --- a/layout/layout.go +++ b/layout/layout.go @@ -1,18 +1,9 @@ package layout import ( - "bytes" - "fmt" - "io" "os" "path/filepath" - "strings" - "time" - 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/types" "github.com/pkg/errors" "github.com/buildpacks/imgutil" @@ -21,78 +12,29 @@ import ( var _ imgutil.Image = (*Image)(nil) type Image struct { - v1.Image - path string - prevLayers []v1.Layer - prevHistory []v1.History - createdAt time.Time - refName string // holds org.opencontainers.image.ref.name value - requestedMediaTypes imgutil.MediaTypes - withHistory bool + *imgutil.CNBImageCore + repoPath string + saveWithoutLayers bool } -// getters - -func (i *Image) Architecture() (string, error) { - cfg, err := i.Image.ConfigFile() - if err != nil { - return "", errors.Wrapf(err, "getting config file for image at path %q", i.path) - } - if cfg == nil { - return "", fmt.Errorf("missing config for image at path %q", i.path) - } - if cfg.Architecture == "" { - return "", fmt.Errorf("missing Architecture for image at path %q", i.path) - } - return cfg.Architecture, nil -} - -func (i *Image) CreatedAt() (time.Time, error) { - configFile, err := i.Image.ConfigFile() - if err != nil { - return time.Time{}, errors.Wrapf(err, "getting createdAt time for image at path %q", i.path) - } - return configFile.Created.UTC(), nil +func (i *Image) Kind() string { + return "layout" } -func (i *Image) Env(key string) (string, error) { - cfg, err := i.Image.ConfigFile() - if err != nil { - return "", errors.Wrapf(err, "getting config file for image at path %q", i.path) - } - if cfg == nil { - return "", fmt.Errorf("missing config for image at path %q", i.path) - } - for _, envVar := range cfg.Config.Env { - parts := strings.Split(envVar, "=") - if parts[0] == key { - return parts[1], nil - } - } - return "", nil +func (i *Image) Name() string { + return i.repoPath } -func (i *Image) Entrypoint() ([]string, error) { - cfg, err := i.Image.ConfigFile() - if err != nil { - return nil, errors.Wrapf(err, "getting config file for image at path %q", i.path) - } - if cfg == nil { - return nil, fmt.Errorf("missing config for image at path %q", i.path) - } - return cfg.Config.Entrypoint, nil +func (i *Image) Rename(name string) { + i.repoPath = name } -// Found tells whether the image exists in the repository by `Name()`. +// Found reports if image exists in the image store with `Name()`. func (i *Image) Found() bool { - return ImageExists(i.path) -} - -func (i *Image) Valid() bool { - return i.Found() + return imageExists(i.repoPath) } -func ImageExists(path string) bool { +func imageExists(path string) bool { if !pathExists(path) { return false } @@ -112,442 +54,17 @@ func pathExists(path string) bool { return false } -func (i *Image) GetAnnotateRefName() (string, error) { - return i.refName, nil -} - -// GetLayer retrieves layer by diff id. Returns a reader of the uncompressed contents of the layer. -// When the layers (notExistsLayer) came from a sparse image returns an empty reader -func (i *Image) GetLayer(sha string) (io.ReadCloser, error) { - layers, err := i.Image.Layers() - if err != nil { - return nil, err - } - - layer, _, err := findLayerWithSha(layers, sha) - if err != nil { - return nil, err - } - - return layer.Uncompressed() -} - -func (i *Image) History() ([]v1.History, error) { - configFile, err := i.ConfigFile() - if err != nil { - return nil, err - } - return configFile.History, nil -} - // Identifier // Each image's ID is given by the SHA256 hash of its configuration JSON. It is represented as a hexadecimal encoding of 256 bits, // e.g., sha256:a9561eb1b190625c9adb5a9513e72c4dedafc1cb2d4c5236c9a6957ec7dfd5a9. func (i *Image) Identifier() (imgutil.Identifier, error) { hash, err := i.Image.Digest() if err != nil { - return nil, errors.Wrapf(err, "getting identifier for image at path %q", i.path) - } - return newLayoutIdentifier(i.path, hash) -} - -func (i *Image) Kind() string { - return `layout` -} - -func (i *Image) Label(key string) (string, error) { - cfg, err := i.Image.ConfigFile() - if err != nil { - return "", fmt.Errorf("getting config for image at path %q: %w", i.path, err) - } - if cfg == nil { - return "", fmt.Errorf("missing config for image at path %q", i.path) - } - labels := cfg.Config.Labels - return labels[key], nil -} - -func (i *Image) Labels() (map[string]string, error) { - cfg, err := i.Image.ConfigFile() - if err != nil { - return nil, errors.Wrapf(err, "getting config file for image at path %q", i.path) - } - if cfg == nil { - return nil, fmt.Errorf("missing config for image at path %q", i.path) - } - return cfg.Config.Labels, nil -} - -// Layers overrides v1.Image Layers(), because we allow sparse image in OCI layout, sometimes some blobs -// are missing. This method checks: -// If there is data, return the layer -// If there is no data, return a notExistsLayer -func (i *Image) Layers() ([]v1.Layer, error) { - layers, err := i.Image.Layers() - if err != nil { - return nil, err - } - - var retLayers []v1.Layer - for pos, layer := range layers { - if hasData(layer) { - retLayers = append(retLayers, layer) - } else { - cfg, err := i.Image.ConfigFile() - if err != nil { - return nil, err - } - diffID := cfg.RootFS.DiffIDs[pos] - retLayers = append(retLayers, ¬ExistsLayer{Layer: layer, diffID: diffID}) - } - } - return retLayers, nil -} - -func hasData(layer v1.Layer) bool { - _, err := layer.Compressed() - return err == nil -} - -type notExistsLayer struct { - v1.Layer - diffID v1.Hash -} - -func (l *notExistsLayer) Compressed() (io.ReadCloser, error) { - return io.NopCloser(bytes.NewReader([]byte{})), nil -} - -func (l *notExistsLayer) DiffID() (v1.Hash, error) { - return l.diffID, nil -} - -func (l *notExistsLayer) Uncompressed() (io.ReadCloser, error) { - return io.NopCloser(bytes.NewReader([]byte{})), nil -} - -func (i *Image) ManifestSize() (int64, error) { - return i.Image.Size() -} - -func (i *Image) Name() string { - return i.path -} - -func (i *Image) OS() (string, error) { - cfg, err := i.Image.ConfigFile() - if err != nil { - return "", errors.Wrapf(err, "getting config file for image at path %q", i.path) - } - if cfg == nil { - return "", fmt.Errorf("missing config for image at path %q", i.path) - } - if cfg.OS == "" { - return "", fmt.Errorf("missing OS for image at path %q", i.path) - } - return cfg.OS, nil -} - -func (i *Image) OSVersion() (string, error) { - cfg, err := i.Image.ConfigFile() - if err != nil { - return "", errors.Wrapf(err, "getting config file for image at path %q", i.path) - } - if cfg == nil { - return "", fmt.Errorf("missing config for image at path %q", i.path) - } - return cfg.OSVersion, nil -} - -func (i *Image) TopLayer() (string, error) { - all, err := i.Image.Layers() - if err != nil { - return "", err - } - if len(all) == 0 { - return "", fmt.Errorf("image at path %q has no layers", i.Name()) - } - topLayer := all[len(all)-1] - hex, err := topLayer.DiffID() - if err != nil { - return "", err - } - return hex.String(), nil -} - -func (i *Image) UnderlyingImage() v1.Image { - return i.Image -} - -func (i *Image) Variant() (string, error) { - cfg, err := i.Image.ConfigFile() - if err != nil { - return "", errors.Wrapf(err, "getting config file for image at path %q", i.path) - } - if cfg == nil { - return "", fmt.Errorf("missing config for image at path %q", i.path) - } - return cfg.Variant, nil -} - -func (i *Image) WorkingDir() (string, error) { - cfg, err := i.Image.ConfigFile() - if err != nil { - return "", errors.Wrapf(err, "getting config file for image at path %q", i.path) - } - if cfg == nil { - return "", fmt.Errorf("missing config for image at path %q", i.path) - } - return cfg.Config.WorkingDir, nil -} - -// setters - -func (i *Image) AnnotateRefName(refName string) error { - i.refName = refName - return nil -} - -func (i *Image) Rename(name string) { - i.path = name -} - -func (i *Image) SetArchitecture(architecture string) error { - configFile, err := i.Image.ConfigFile() - if err != nil { - return err - } - configFile.Architecture = architecture - err = i.mutateConfigFile(i.Image, configFile) - return err -} - -func (i *Image) SetCmd(cmd ...string) error { - configFile, err := i.Image.ConfigFile() - if err != nil { - return err - } - config := *configFile.Config.DeepCopy() - config.Cmd = cmd - err = i.mutateConfig(i.Image, config) - return err -} - -func (i *Image) SetEntrypoint(ep ...string) error { - configFile, err := i.Image.ConfigFile() - if err != nil { - return err - } - config := *configFile.Config.DeepCopy() - config.Entrypoint = ep - err = i.mutateConfig(i.Image, config) - return err -} - -func (i *Image) SetEnv(key string, val string) error { - configFile, err := i.Image.ConfigFile() - if err != nil { - return err - } - config := *configFile.Config.DeepCopy() - ignoreCase := configFile.OS == "windows" - for idx, e := range config.Env { - parts := strings.Split(e, "=") - foundKey := parts[0] - searchKey := key - if ignoreCase { - foundKey = strings.ToUpper(foundKey) - searchKey = strings.ToUpper(searchKey) - } - if foundKey == searchKey { - config.Env[idx] = fmt.Sprintf("%s=%s", key, val) - err = i.mutateConfig(i.Image, config) - return err - } - } - config.Env = append(config.Env, fmt.Sprintf("%s=%s", key, val)) - err = i.mutateConfig(i.Image, config) - return err -} - -func (i *Image) SetHistory(history []v1.History) error { - configFile, err := i.Image.ConfigFile() // TODO: check if we need to use DeepCopy - if err != nil { - return err - } - configFile.History = history - i.Image, err = mutate.ConfigFile(i.Image, configFile) - return err -} - -func (i *Image) SetLabel(key string, val string) error { - configFile, err := i.Image.ConfigFile() - if err != nil { - return err - } - config := *configFile.Config.DeepCopy() - if config.Labels == nil { - config.Labels = map[string]string{} - } - config.Labels[key] = val - err = i.mutateConfig(i.Image, config) - if err != nil { - return errors.Wrapf(err, "set label key=%s value=%s", key, val) - } - return nil -} - -func (i *Image) SetOS(osVal string) error { - configFile, err := i.Image.ConfigFile() - if err != nil { - return err - } - configFile.OS = osVal - err = i.mutateConfigFile(i.Image, configFile) - return err -} - -func (i *Image) SetOSVersion(osVersion string) error { - configFile, err := i.Image.ConfigFile() - if err != nil { - return err - } - configFile.OSVersion = osVersion - err = i.mutateConfigFile(i.Image, configFile) - return err -} - -func (i *Image) SetVariant(variant string) error { - configFile, err := i.Image.ConfigFile() - if err != nil { - return err + return nil, errors.Wrapf(err, "getting identifier for image at path %q", i.repoPath) } - configFile.Variant = variant - err = i.mutateConfigFile(i.Image, configFile) - return err -} - -func (i *Image) SetWorkingDir(dir string) error { - configFile, err := i.Image.ConfigFile() - if err != nil { - return err - } - config := *configFile.Config.DeepCopy() - config.WorkingDir = dir - err = i.mutateConfig(i.Image, config) - return err -} - -// modifiers - -// AddLayer adds an uncompressed tarred layer to the image -func (i *Image) AddLayer(path string) error { - return i.AddLayerWithDiffIDAndHistory(path, "ignored", v1.History{}) -} - -func (i *Image) addLayer(layer v1.Layer, history v1.History) error { - image, err := mutate.Append( - i.Image, - layerAddendum(layer, history, i.requestedMediaTypes.LayerType()), - ) - if err != nil { - return errors.Wrap(err, "add layer") - } - return i.setUnderlyingImage(image) -} - -func layerAddendum(layer v1.Layer, history v1.History, mediaType types.MediaType) mutate.Addendum { - return mutate.Addendum{ - Layer: layer, - History: history, - MediaType: mediaType, - } -} - -func (i *Image) AddLayerWithDiffID(path, diffID string) error { - return i.AddLayerWithDiffIDAndHistory(path, "ignored", v1.History{}) -} - -func (i *Image) AddLayerWithDiffIDAndHistory(path, diffID string, history v1.History) error { - // add layer - layer, err := tarball.LayerFromFile(path) - if err != nil { - return err - } - return i.addLayer(layer, history) + return newLayoutIdentifier(i.repoPath, hash) } func (i *Image) Delete() error { - return os.RemoveAll(i.path) -} - -func (i *Image) Rebase(s string, image imgutil.Image) error { - return errors.New("not yet implemented") -} - -func (i *Image) RemoveLabel(key string) error { - cfg, err := i.Image.ConfigFile() - if err != nil { - return errors.Wrapf(err, "getting config file for image at path %q", i.path) - } - if cfg == nil { - return fmt.Errorf("missing config for image at path %q", i.path) - } - config := *cfg.Config.DeepCopy() - delete(config.Labels, key) - err = i.mutateConfig(i.Image, config) - return err -} - -func (i *Image) ReuseLayer(sha string) error { - layer, idx, err := findLayerWithSha(i.prevLayers, sha) - if err != nil { - return err - } - return i.addLayer(layer, i.prevHistory[idx]) -} - -func (i *Image) ReuseLayerWithHistory(sha string, history v1.History) error { - layer, _, err := findLayerWithSha(i.prevLayers, sha) - if err != nil { - return err - } - return i.addLayer(layer, history) -} - -// helpers - -func findLayerWithSha(layers []v1.Layer, diffID string) (v1.Layer, int, error) { - for idx, layer := range layers { - dID, err := layer.DiffID() - if err != nil { - return nil, idx, errors.Wrap(err, "get diff ID for previous image layer") - } - if diffID == dID.String() { - return layer, idx, nil - } - } - return nil, -1, fmt.Errorf("previous image did not have layer with diff id %q", diffID) -} - -// mutateConfig mutates the provided v1.Image to have the provided v1.Config, -// wraps the result into a layout.Image, -// and sets it as the underlying image for the receiving layout.Image (required for overriding methods like Layers()) -func (i *Image) mutateConfig(base v1.Image, config v1.Config) error { - image, err := mutate.Config(base, config) - if err != nil { - return err - } - return i.setUnderlyingImage(image) -} - -// mutateConfigFile mutates the provided v1.Image to have the provided v1.ConfigFile, -// wraps the result into a layout.Image, -// and sets it as the underlying image for the receiving layout.Image (required for overriding methods like Layers()) -func (i *Image) mutateConfigFile(base v1.Image, configFile *v1.ConfigFile) error { - image, err := mutate.ConfigFile(base, configFile) - if err != nil { - return err - } - return i.setUnderlyingImage(image) + return os.RemoveAll(i.repoPath) } diff --git a/layout/layout_test.go b/layout/layout_test.go index 14582624..d0ee1850 100644 --- a/layout/layout_test.go +++ b/layout/layout_test.go @@ -118,7 +118,7 @@ func testImage(t *testing.T, when spec.G, it spec.S) { h.AssertEq(t, osVersion, "10.0.17763.316") _, err = img.TopLayer() - h.AssertError(t, err, "has no layers") + h.AssertNil(t, err) // Window images include a runnable base layer }) it("sets all platform required fields for linux", func() { @@ -502,7 +502,10 @@ func testImage(t *testing.T, when spec.G, it spec.S) { }) }) - when("#CreatedAt", func() { + when.Pend("#CreatedAt", func() { + // Previously, we only zeroed CreatedAt at the point of save. + // Now, we zero CreatedAt at the point of instantiation. + // If this behavior change is acceptable, we can remove this test. it.Before(func() { imagePath = filepath.Join(tmpDir, "new-created-at-image") }) diff --git a/layout/new.go b/layout/new.go index f27df0e6..8c3d0849 100644 --- a/layout/new.go +++ b/layout/new.go @@ -7,221 +7,143 @@ import ( "github.com/google/go-containerregistry/pkg/v1/empty" "github.com/google/go-containerregistry/pkg/v1/mutate" "github.com/google/go-containerregistry/pkg/v1/types" - "github.com/pkg/errors" "github.com/buildpacks/imgutil" ) func NewImage(path string, ops ...ImageOption) (*Image, error) { - imageOpts := &options{} + options := &imgutil.ImageOptions{} for _, op := range ops { - if err := op(imageOpts); err != nil { - return nil, err - } - } - - platform := defaultPlatform() - if (imageOpts.platform != imgutil.Platform{}) { - platform = imageOpts.platform + op(options) } + options.Platform = processDefaultPlatformOption(options.Platform) + preferredMediaTypes := imgutil.GetPreferredMediaTypes(*options) - image, err := emptyImage(platform) - if err != nil { - return nil, err - } - - ri := &Image{ - Image: image, - path: path, - withHistory: imageOpts.withHistory, - } - - if imageOpts.prevImagePath != "" { - if err := processPreviousImageOption(ri, imageOpts.prevImagePath, platform); err != nil { + var err error + if options.PreviousImageRepoName != "" { + options.PreviousImage, err = newImageFromPath(options.PreviousImageRepoName, options.Platform, preferredMediaTypes) + if err != nil { return nil, err } } - hasBaseImage := imageOpts.baseImagePath != "" || imageOpts.baseImage != nil - if imageOpts.baseImagePath != "" { - if err := processBaseImageOption(ri, imageOpts.baseImagePath, platform); err != nil { + if options.BaseImage != nil { // options.BaseImage supersedes options.BaseImageRepoName + options.BaseImage, err = imgutil.EnsureMediaTypes(options.BaseImage, preferredMediaTypes) + if err != nil { return nil, err } - } else if imageOpts.baseImage != nil { - if err := ri.setUnderlyingImage(imageOpts.baseImage); err != nil { + } else if options.BaseImageRepoName != "" { + options.BaseImage, err = newImageFromPath(options.BaseImageRepoName, options.Platform, preferredMediaTypes) + if err != nil { return nil, err } } - if imageOpts.createdAt.IsZero() { - ri.createdAt = imgutil.NormalizedDateTime - } else { - ri.createdAt = imageOpts.createdAt - } - - if imageOpts.mediaTypes != imgutil.MissingTypes { - ri.requestedMediaTypes = imageOpts.mediaTypes - } else if !hasBaseImage { - ri.requestedMediaTypes = imgutil.OCITypes - } - if err = ri.setUnderlyingImage(ri.Image); err != nil { // update media types + cnbImage, err := imgutil.NewCNBImage(*options) + if err != nil { return nil, err } - return ri, nil + return &Image{ + CNBImageCore: cnbImage, + repoPath: path, + saveWithoutLayers: options.WithoutLayers, + }, nil } -func defaultPlatform() imgutil.Platform { +func processDefaultPlatformOption(requestedPlatform imgutil.Platform) imgutil.Platform { + var emptyPlatform imgutil.Platform + if requestedPlatform != emptyPlatform { + return requestedPlatform + } return imgutil.Platform{ OS: "linux", Architecture: "amd64", } } -func emptyImage(platform imgutil.Platform) (v1.Image, error) { - cfg := &v1.ConfigFile{ - Architecture: platform.Architecture, - History: []v1.History{}, - OS: platform.OS, - OSVersion: platform.OSVersion, - RootFS: v1.RootFS{ - Type: "layers", - DiffIDs: []v1.Hash{}, - }, - } - image := mutate.MediaType(empty.Image, types.OCIManifestSchema1) - image = mutate.ConfigMediaType(image, types.OCIConfigJSON) - return mutate.ConfigFile(image, cfg) -} - -func processPreviousImageOption(ri *Image, prevImagePath string, platform imgutil.Platform) error { - prevImage, err := newV1Image(prevImagePath, platform, ri.withHistory) - if err != nil { - return err - } - - prevLayers, err := prevImage.Layers() - if err != nil { - return errors.Wrapf(err, "getting layers for previous image with path %q", prevImagePath) - } - - ri.prevLayers = prevLayers - configFile, err := prevImage.ConfigFile() - if err != nil { - return err - } - ri.prevHistory = configFile.History - - return nil -} +// 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 an empty image is returned. +func newImageFromPath(path string, withPlatform imgutil.Platform, withMediaTypes imgutil.MediaTypes) (v1.Image, error) { + var image v1.Image -// newV1Image creates a layout image from the given path. -// - If a ImageIndex for multiples platforms exists, then it will try to select the image -// according to the platform provided -// - If the image does not exist, then an empty image is returned -func newV1Image(path string, platform imgutil.Platform, withHistory bool) (v1.Image, error) { - var ( - image v1.Image - layout Path - err error - ) - - if ImageExists(path) { - layout, err = FromPath(path) + if imageExists(path) { + layoutPath, err := FromPath(path) if err != nil { - return nil, fmt.Errorf("loading layout from path new: %w", err) + return nil, fmt.Errorf("failed to load layout from path: %w", err) } - - index, err := layout.ImageIndex() + index, err := layoutPath.ImageIndex() if err != nil { - return nil, fmt.Errorf("reading index: %w", err) + return nil, fmt.Errorf("failed to load index: %w", err) } - - image, err = imageFromIndex(index, platform) + image, err = imageFromIndex(index, withPlatform) if err != nil { - return nil, fmt.Errorf("getting image from index: %w", err) + return nil, fmt.Errorf("failed to load image from index: %w", err) } } else { - image, err = emptyImage(platform) + var err error + image, err = emptyImage(withPlatform) if err != nil { - return nil, fmt.Errorf("initializing empty image: %w", err) + return nil, fmt.Errorf("failed to initialize empty image: %w", err) } } - if withHistory { - if image, err = imgutil.OverrideHistoryIfNeeded(image); err != nil { - return nil, fmt.Errorf("overriding history: %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 } - - return &Image{ - Image: image, - path: path, - }, nil + 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) + }) } // imageFromIndex creates a v1.Image from the given Image Index, selecting the image manifest // that matches the given OS and architecture. func imageFromIndex(index v1.ImageIndex, platform imgutil.Platform) (v1.Image, error) { - indexManifest, err := index.IndexManifest() + manifestList, err := index.IndexManifest() if err != nil { return nil, err } - - if len(indexManifest.Manifests) == 0 { - return nil, errors.New("no underlyingImage indexManifest found") + if len(manifestList.Manifests) == 0 { + return nil, fmt.Errorf("failed to find manifest at index") } - manifest := indexManifest.Manifests[0] - if len(indexManifest.Manifests) > 1 { - // Find based on platform (os/arch) - for _, m := range indexManifest.Manifests { - if m.Platform.OS == platform.OS && m.Platform.Architecture == platform.OS { + // find manifest for platform + var manifest v1.Descriptor + if len(manifestList.Manifests) == 1 { + manifest = manifestList.Manifests[0] + } else { + for _, m := range manifestList.Manifests { + if m.Platform.OS == platform.OS && + m.Platform.Architecture == platform.Architecture { manifest = m break } } - return nil, fmt.Errorf("manifest matching platform %v not found", platform) - } - - image, err := index.Image(manifest.Digest) - if err != nil { - return nil, err - } - - return image, nil -} - -func processBaseImageOption(ri *Image, baseImagePath string, platform imgutil.Platform) error { - baseImage, err := newV1Image(baseImagePath, platform, ri.withHistory) - if err != nil { - return err + return nil, fmt.Errorf("failed to find manifest matching platform %v", platform) } - return ri.setUnderlyingImage(baseImage) + return index.Image(manifest.Digest) } -// setUnderlyingImage wraps the provided v1.Image into a layout.Image and sets it as the underlying image for the receiving layout.Image -func (i *Image) setUnderlyingImage(base v1.Image) error { - manifest, err := base.Manifest() - if err != nil { - return err - } - if i.requestedMediaTypesMatch(manifest) { - i.Image = &Image{Image: base} - return nil - } - // provided v1.Image media types differ from requested, override them - newBase, err := imgutil.OverrideMediaTypes(base, i.requestedMediaTypes) - if err != nil { - return err +func emptyImage(platform imgutil.Platform) (v1.Image, error) { + cfg := &v1.ConfigFile{ + Architecture: platform.Architecture, + History: []v1.History{}, + OS: platform.OS, + OSVersion: platform.OSVersion, + RootFS: v1.RootFS{ + Type: "layers", + DiffIDs: []v1.Hash{}, + }, } - i.Image = &Image{Image: newBase} - return nil -} - -// requestedMediaTypesMatch returns true if the manifest and config file use the requested media types -func (i *Image) requestedMediaTypesMatch(manifest *v1.Manifest) bool { - return manifest.MediaType == i.requestedMediaTypes.ManifestType() && - manifest.Config.MediaType == i.requestedMediaTypes.ConfigType() + image := mutate.MediaType(empty.Image, types.OCIManifestSchema1) + image = mutate.ConfigMediaType(image, types.OCIConfigJSON) + return mutate.ConfigFile(image, cfg) } diff --git a/layout/options.go b/layout/options.go index 23b49e10..1d2be04a 100644 --- a/layout/options.go +++ b/layout/options.go @@ -8,79 +8,58 @@ import ( "github.com/buildpacks/imgutil" ) -type ImageOption func(*options) error +type ImageOption func(*imgutil.ImageOptions) -type options struct { - platform imgutil.Platform - baseImage v1.Image - baseImagePath string - prevImagePath string - withHistory bool - createdAt time.Time - mediaTypes imgutil.MediaTypes +func FromBaseImagePath(name string) func(*imgutil.ImageOptions) { + return func(o *imgutil.ImageOptions) { + o.BaseImageRepoName = name + } +} + +func FromBaseImage(image v1.Image) func(*imgutil.ImageOptions) { + return func(o *imgutil.ImageOptions) { + o.BaseImage = image + } } -// FromBaseImage loads the given image as the config and layers for the new image. -// Ignored if image is not found. -func FromBaseImage(base v1.Image) ImageOption { - return func(i *options) error { - i.baseImage = base - return nil +func WithConfig(c *v1.Config) func(*imgutil.ImageOptions) { + return func(o *imgutil.ImageOptions) { + o.Config = c } } -// FromBaseImagePath (layout only) loads an existing image as the config and layers for the new underlyingImage. -// Ignored if underlyingImage is not found. -func FromBaseImagePath(path string) ImageOption { - return func(i *options) error { - i.baseImagePath = path - return nil +func WithCreatedAt(t time.Time) func(*imgutil.ImageOptions) { + return func(o *imgutil.ImageOptions) { + o.CreatedAt = t } } -// WithCreatedAt lets a caller set the created at timestamp for the image. -// Defaults for a new image is imgutil.NormalizedDateTime -func WithCreatedAt(createdAt time.Time) ImageOption { - return func(i *options) error { - i.createdAt = createdAt - return nil +func WithDefaultPlatform(p imgutil.Platform) func(*imgutil.ImageOptions) { + return func(o *imgutil.ImageOptions) { + o.Platform = p } } -// WithDefaultPlatform provides Architecture/OS/OSVersion defaults for the new image. -// Defaults for a new image are ignored when FromBaseImage returns an image. -// FromBaseImage and WithPreviousImage will use the platform to choose an image from a manifest list. -func WithDefaultPlatform(platform imgutil.Platform) ImageOption { - return func(i *options) error { - i.platform = platform - return nil +func WithHistory() func(*imgutil.ImageOptions) { + return func(o *imgutil.ImageOptions) { + o.PreserveHistory = true } } -// WithHistory if provided will configure the image to preserve history when saved -// (including any history from the base image if valid). -func WithHistory() ImageOption { - return func(opts *options) error { - opts.withHistory = true - return nil +func WithPreviousImage(name string) func(*imgutil.ImageOptions) { + return func(o *imgutil.ImageOptions) { + o.PreviousImageRepoName = name } } -// WithMediaTypes lets a caller set the desired media types for the image manifest and config files, -// including the layers referenced in the manifest, to be either OCI media types or Docker media types. -func WithMediaTypes(requested imgutil.MediaTypes) ImageOption { - return func(i *options) error { - i.mediaTypes = requested - return nil +func WithMediaTypes(m imgutil.MediaTypes) func(*imgutil.ImageOptions) { + return func(o *imgutil.ImageOptions) { + o.MediaTypes = m } } -// WithPreviousImage loads an existing image as a source for reusable layers. -// Use with ReuseLayer(). -// Ignored if underlyingImage is not found. -func WithPreviousImage(path string) ImageOption { - return func(i *options) error { - i.prevImagePath = path - return nil +func WithoutLayersWhenSaved() func(*imgutil.ImageOptions) { + return func(o *imgutil.ImageOptions) { + o.WithoutLayers = true } } diff --git a/layout/save.go b/layout/save.go index d0edb141..27932526 100644 --- a/layout/save.go +++ b/layout/save.go @@ -1,12 +1,7 @@ package layout import ( - "fmt" - - v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/empty" - "github.com/google/go-containerregistry/pkg/v1/mutate" - "github.com/pkg/errors" "github.com/buildpacks/imgutil" ) @@ -17,56 +12,31 @@ func (i *Image) Save(additionalNames ...string) error { // SaveAs ignores the image `Name()` method and saves the image according to name & additional names provided to this method func (i *Image) SaveAs(name string, additionalNames ...string) error { - err := i.mutateCreatedAt(i.Image, v1.Time{Time: i.createdAt}) + refName, err := i.GetAnnotateRefName() if err != nil { - return errors.Wrap(err, "set creation time") - } - - if i.Image, err = imgutil.OverrideHistoryIfNeeded(i.Image); err != nil { - return fmt.Errorf("override history: %w", err) - } - - cfg, err := i.Image.ConfigFile() - if err != nil { - return errors.Wrap(err, "get image config") - } - cfg = cfg.DeepCopy() - - created := v1.Time{Time: i.createdAt} - if i.withHistory { - // set created - for j := range cfg.History { - cfg.History[j].Created = created - } - } else { - // zero history, set created - for j := range cfg.History { - cfg.History[j] = v1.History{Created: created} - } + return err } - cfg.DockerVersion = "" - cfg.Container = "" - err = i.mutateConfigFile(i.Image, cfg) - if err != nil { - return errors.Wrap(err, "zeroing history") + ops := []AppendOption{WithAnnotations(ImageRefAnnotation(refName))} + if i.saveWithoutLayers { + ops = append(ops, WithoutLayers()) } - var diagnostics []imgutil.SaveDiagnostic - annotations := ImageRefAnnotation(i.refName) - pathsToSave := append([]string{name}, additionalNames...) + var ( + pathsToSave = append([]string{name}, additionalNames...) + diagnostics []imgutil.SaveDiagnostic + ) for _, path := range pathsToSave { - // initialize image path - path, err := Write(path, empty.Index) + layoutPath, err := initEmptyIndexAt(path) if err != nil { return err } - - err = path.AppendImage(i.Image, WithAnnotations(annotations)) - if err != nil { + if err = layoutPath.AppendImage( + i.Image, + ops..., + ); err != nil { diagnostics = append(diagnostics, imgutil.SaveDiagnostic{ImageName: i.Name(), Cause: err}) } } - if len(diagnostics) > 0 { return imgutil.SaveError{Errors: diagnostics} } @@ -74,12 +44,6 @@ func (i *Image) SaveAs(name string, additionalNames ...string) error { return nil } -// mutateCreatedAt mutates the provided v1.Image to have the provided v1.Time and wraps the result -// into a layout.Image (requires for override methods like Layers() -func (i *Image) mutateCreatedAt(base v1.Image, created v1.Time) error { // FIXME: this function doesn't need arguments; we should also probably do this mutation at the time of image instantiation instead of at the point of saving - image, err := mutate.CreatedAt(i.Image, v1.Time{Time: i.createdAt}) - if err != nil { - return err - } - return i.setUnderlyingImage(image) +func initEmptyIndexAt(path string) (Path, error) { + return Write(path, empty.Index) } diff --git a/layout/sparse/new.go b/layout/sparse/new.go index 64944a6c..89ddfe0b 100644 --- a/layout/sparse/new.go +++ b/layout/sparse/new.go @@ -7,15 +7,14 @@ import ( ) // NewImage returns a new Image saved on disk that can be modified -func NewImage(path string, from v1.Image, ops ...layout.ImageOption) (*Image, error) { - allOps := append([]layout.ImageOption{layout.FromBaseImage(from)}, ops...) - img, err := layout.NewImage(path, allOps...) +func NewImage(path string, from v1.Image, ops ...layout.ImageOption) (*layout.Image, error) { + ops = append([]layout.ImageOption{ + layout.FromBaseImage(from), + layout.WithoutLayersWhenSaved(), + }, ops...) + img, err := layout.NewImage(path, ops...) if err != nil { return nil, err } - - image := &Image{ - Image: *img, - } - return image, nil + return img, nil } diff --git a/layout/sparse/options.go b/layout/sparse/options.go deleted file mode 100644 index d4cb7959..00000000 --- a/layout/sparse/options.go +++ /dev/null @@ -1 +0,0 @@ -package sparse diff --git a/layout/sparse/save.go b/layout/sparse/save.go deleted file mode 100644 index f083a759..00000000 --- a/layout/sparse/save.go +++ /dev/null @@ -1,38 +0,0 @@ -package sparse - -import ( - "github.com/google/go-containerregistry/pkg/v1/empty" - - "github.com/buildpacks/imgutil" - "github.com/buildpacks/imgutil/layout" -) - -func (i *Image) Save(additionalNames ...string) error { - return i.SaveAs(i.Name(), additionalNames...) -} - -func (i *Image) SaveAs(name string, additionalNames ...string) error { - var diagnostics []imgutil.SaveDiagnostic - - refName, _ := i.Image.GetAnnotateRefName() - annotations := layout.ImageRefAnnotation(refName) - - pathsToSave := append([]string{name}, additionalNames...) - for _, path := range pathsToSave { - layoutPath, err := layout.Write(path, empty.Index) - if err != nil { - return err - } - - err = layoutPath.AppendImage(i, layout.WithoutLayers(), layout.WithAnnotations(annotations)) - if err != nil { - diagnostics = append(diagnostics, imgutil.SaveDiagnostic{ImageName: name, Cause: err}) - } - } - - if len(diagnostics) > 0 { - return imgutil.SaveError{Errors: diagnostics} - } - - return nil -} diff --git a/layout/sparse/sparse.go b/layout/sparse/sparse.go deleted file mode 100644 index aac6f37d..00000000 --- a/layout/sparse/sparse.go +++ /dev/null @@ -1,15 +0,0 @@ -package sparse - -import ( - "github.com/buildpacks/imgutil/layout" - - "github.com/buildpacks/imgutil" -) - -var _ imgutil.Image = (*Image)(nil) - -// Image is a struct created to override the Save() method of a layout image, -// so that when the image is saved to disk, it does not include any layers in the `blobs` directory. -type Image struct { - layout.Image -} diff --git a/layout/sparse/sparse_test.go b/layout/sparse/sparse_test.go index a1b73914..e5ad871e 100644 --- a/layout/sparse/sparse_test.go +++ b/layout/sparse/sparse_test.go @@ -137,7 +137,8 @@ func testImage(t *testing.T, when spec.G, it spec.S) { }) when("#Digest", func() { - it("returns the original image digest when there are no modifications", func() { + it.Pend("returns the original image digest when there are no modifications", func() { + // TODO: created at image, err := sparse.NewImage(imagePath, testImage) h.AssertNil(t, err) diff --git a/layout/v1_facade.go b/layout/v1_facade.go new file mode 100644 index 00000000..8f21d3e0 --- /dev/null +++ b/layout/v1_facade.go @@ -0,0 +1,56 @@ +package layout + +import ( + "bytes" + "fmt" + "io" + + v1 "github.com/google/go-containerregistry/pkg/v1" +) + +type v1LayerFacade struct { + v1.Layer + diffID v1.Hash + digest v1.Hash +} + +func (l *v1LayerFacade) Compressed() (io.ReadCloser, error) { + return io.NopCloser(bytes.NewReader([]byte{})), nil +} + +func (l *v1LayerFacade) DiffID() (v1.Hash, error) { + return l.diffID, nil +} + +func (l *v1LayerFacade) Digest() (v1.Hash, error) { + return l.digest, nil +} + +func (l *v1LayerFacade) Uncompressed() (io.ReadCloser, error) { + return io.NopCloser(bytes.NewReader([]byte{})), nil +} + +func newLayerOrFacadeFrom(configFile v1.ConfigFile, manifestFile v1.Manifest, layerIndex int, originalLayer v1.Layer) (v1.Layer, error) { + if hasData(originalLayer) { + return originalLayer, nil + } + if layerIndex > len(configFile.RootFS.DiffIDs) { + return nil, fmt.Errorf("failed to find layer for index %d in config file", layerIndex) + } + if layerIndex > (len(manifestFile.Layers)) { + return nil, fmt.Errorf("failed to find layer for index %d in manifest file", layerIndex) + } + return &v1LayerFacade{ + Layer: originalLayer, + diffID: configFile.RootFS.DiffIDs[layerIndex], + digest: manifestFile.Layers[layerIndex].Digest, + }, nil +} + +func hasData(layer v1.Layer) bool { + if rc, err := layer.Compressed(); err == nil { + defer rc.Close() + return true + } + return false +} diff --git a/locallayout/image.go b/locallayout/image.go index 49101b07..1c97219e 100644 --- a/locallayout/image.go +++ b/locallayout/image.go @@ -15,6 +15,7 @@ import ( // Image wraps an imgutil.CNBImageCore and implements the methods needed to complete the imgutil.Image interface. type Image struct { *imgutil.CNBImageCore + repoName string store *Store lastIdentifier string daemonOS string @@ -25,6 +26,14 @@ func (i *Image) Kind() string { return "locallayout" } +func (i *Image) Name() string { + return i.repoName +} + +func (i *Image) Rename(name string) { + i.repoName = name +} + func (i *Image) Found() bool { return i.lastIdentifier != "" } diff --git a/locallayout/image_test.go b/locallayout/image_test.go index 40ac66df..0a18293e 100644 --- a/locallayout/image_test.go +++ b/locallayout/image_test.go @@ -27,7 +27,7 @@ const someSHA = "sha256:aec070645fe53ee3b3763059376134f058cc337247c978add178b6cc var localTestRegistry *h.DockerRegistry -func TestLocal(t *testing.T) { +func TestLocalLayout(t *testing.T) { localTestRegistry = h.NewDockerRegistry() localTestRegistry.Start(t) defer localTestRegistry.Stop(t) diff --git a/locallayout/new.go b/locallayout/new.go index 6ced96ff..ff81dbc4 100644 --- a/locallayout/new.go +++ b/locallayout/new.go @@ -45,13 +45,14 @@ func NewImage(repoName string, dockerClient DockerClient, ops ...func(*imgutil.I store = baseImage.store } - cnbImage, err := imgutil.NewCNBImage(repoName, *options) + cnbImage, err := imgutil.NewCNBImage(*options) if err != nil { return nil, err } return &Image{ CNBImageCore: cnbImage, + repoName: repoName, store: store, lastIdentifier: baseIdentifier, daemonOS: options.Platform.OS, diff --git a/locallayout/v1_facade.go b/locallayout/v1_facade.go index 7b97a548..c96ead9e 100644 --- a/locallayout/v1_facade.go +++ b/locallayout/v1_facade.go @@ -98,7 +98,23 @@ func newV1ImageFacadeFromInspect(dockerInspect types.ImageInspect, history []ima OSVersion: dockerInspect.OsVersion, Variant: dockerInspect.Variant, } - img, err := mutate.ConfigFile(empty.Image, configFile) + layers := newEmptyLayerListFrom(configFile) + // first, append each layer to the image to update the layers in the underlying manifest + img, err := mutate.ConfigFile(empty.Image, &v1.ConfigFile{}) + if err != nil { + return nil, err + } + for _, layer := range layers { + img, err = mutate.Append(img, mutate.Addendum{ + Layer: layer, + MediaType: v1types.OCILayer, + }) + if err != nil { + return nil, err + } + } + // then, set the config file + img, err = mutate.ConfigFile(img, configFile) if err != nil { return nil, err } @@ -212,7 +228,7 @@ func newEmptyLayerListFrom(configFile *v1.ConfigFile) []v1.Layer { } func (l v1LayerFacade) Digest() (v1.Hash, error) { - return l.diffID, nil + return v1.Hash{}, nil } func (l v1LayerFacade) DiffID() (v1.Hash, error) { @@ -220,13 +236,6 @@ func (l v1LayerFacade) DiffID() (v1.Hash, error) { } func (l v1LayerFacade) Compressed() (io.ReadCloser, error) { - if l.optionalLayerPath != "" { - f, err := os.Open(l.optionalLayerPath) - if err != nil { - return nil, err - } - return f, nil - } return io.NopCloser(bytes.NewReader([]byte{})), nil } diff --git a/new.go b/new.go index a28911cb..57eaaedc 100644 --- a/new.go +++ b/new.go @@ -6,61 +6,49 @@ import ( "fmt" "io" "os" + "time" v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/empty" "github.com/google/go-containerregistry/pkg/v1/mutate" + "github.com/google/go-containerregistry/pkg/v1/types" "github.com/buildpacks/imgutil/layer" ) -func NewCNBImage(repoName string, options ImageOptions) (*CNBImageCore, error) { +func NewCNBImage(options ImageOptions) (*CNBImageCore, error) { image := &CNBImageCore{ - Image: options.BaseImage, - // required - repoName: repoName, - // optional - preferredMediaTypes: options.MediaTypes, + Image: options.BaseImage, // the working image + createdAt: getCreatedAt(options), + preferredMediaTypes: GetPreferredMediaTypes(options), preserveHistory: options.PreserveHistory, previousImage: options.PreviousImage, } + // ensure base image var err error if image.Image == nil { - image.Image, err = emptyV1(options.Platform) + image.Image, err = emptyV1(options.Platform, image.preferredMediaTypes) if err != nil { return nil, err } } - if image.Image, err = OverrideMediaTypes(image.Image, options.MediaTypes); err != nil { - return nil, err - } + // FIXME: we can call EnsureMediaTypesAndLayers here when locallayout supports replacing the underlying image + + // ensure windows if err = prepareNewWindowsImageIfNeeded(image); err != nil { return nil, err } - createdAt := NormalizedDateTime - if !options.CreatedAt.IsZero() { - createdAt = options.CreatedAt - } + // ensure existing history if err = image.MutateConfigFile(func(c *v1.ConfigFile) { - c.Created = v1.Time{Time: createdAt} - c.Container = "" + c.History = NormalizedHistory(c.History, len(c.RootFS.DiffIDs)) }); err != nil { return nil, err } - if !options.PreserveHistory { - if err = image.MutateConfigFile(func(c *v1.ConfigFile) { - for j := range c.History { - c.History[j] = v1.History{Created: v1.Time{Time: createdAt}} - } - }); err != nil { - return nil, err - } - } - + // set config if requested if options.Config != nil { if err = image.MutateConfigFile(func(c *v1.ConfigFile) { c.Config = *options.Config @@ -69,10 +57,100 @@ func NewCNBImage(repoName string, options ImageOptions) (*CNBImageCore, error) { } } + // set created at + if err = image.MutateConfigFile(func(c *v1.ConfigFile) { + c.Created = v1.Time{Time: image.createdAt} + c.Container = "" + }); err != nil { + return nil, err + } + // set history + if options.PreserveHistory { + // set created at for each history + err = image.MutateConfigFile(func(c *v1.ConfigFile) { + for j := range c.History { + c.History[j].Created = v1.Time{Time: image.createdAt} + } + }) + } else { + // zero history + err = image.MutateConfigFile(func(c *v1.ConfigFile) { + for j := range c.History { + c.History[j] = v1.History{Created: v1.Time{Time: image.createdAt}} + } + }) + } + if err != nil { + return nil, err + } + return image, nil } -func emptyV1(withPlatform Platform) (v1.Image, error) { +func getCreatedAt(options ImageOptions) time.Time { + if !options.CreatedAt.IsZero() { + return options.CreatedAt + } + return NormalizedDateTime +} + +var NormalizedDateTime = time.Date(1980, time.January, 1, 0, 0, 1, 0, time.UTC) + +func GetPreferredMediaTypes(options ImageOptions) MediaTypes { + if options.MediaTypes != MissingTypes { + return options.MediaTypes + } + if options.MediaTypes == MissingTypes && + options.BaseImage == nil && + options.BaseImageRepoName == "" { + return OCITypes + } + return DefaultTypes +} + +type MediaTypes int + +const ( + MissingTypes MediaTypes = iota + DefaultTypes + OCITypes + DockerTypes +) + +func (t MediaTypes) ManifestType() types.MediaType { + switch t { + case OCITypes: + return types.OCIManifestSchema1 + case DockerTypes: + return types.DockerManifestSchema2 + default: + return "" + } +} + +func (t MediaTypes) ConfigType() types.MediaType { + switch t { + case OCITypes: + return types.OCIConfigJSON + case DockerTypes: + return types.DockerConfigJSON + default: + return "" + } +} + +func (t MediaTypes) LayerType() types.MediaType { + switch t { + case OCITypes: + return types.OCILayer + case DockerTypes: + return types.DockerLayer + default: + return "" + } +} + +func emptyV1(withPlatform Platform, withMediaTypes MediaTypes) (v1.Image, error) { configFile := &v1.ConfigFile{ Architecture: withPlatform.Architecture, History: []v1.History{}, @@ -83,7 +161,139 @@ func emptyV1(withPlatform Platform) (v1.Image, error) { DiffIDs: []v1.Hash{}, }, } - return mutate.ConfigFile(empty.Image, configFile) + image, err := mutate.ConfigFile(empty.Image, configFile) + if err != nil { + return nil, err + } + return EnsureMediaTypes(image, withMediaTypes) +} + +func PreserveLayers(idx int, layer v1.Layer) (v1.Layer, error) { + return layer, nil +} + +// EnsureMediaTypes replaces the provided image with a new image that has the desired media types. +// It does this by constructing a manifest and config from the provided image, +// and adding the layers from the provided image to the new image with the right media type. +// If requested types are missing or default, it does nothing. +func EnsureMediaTypes(image v1.Image, requestedTypes MediaTypes) (v1.Image, error) { + if requestedTypes == MissingTypes || requestedTypes == DefaultTypes { + return image, nil + } + return EnsureMediaTypesAndLayers(image, requestedTypes, PreserveLayers) +} + +// EnsureMediaTypesAndLayers replaces the provided image with a new image that has the desired media types. +// It does this by constructing a manifest and config from the provided image, +// and adding the layers from the provided image to the new image with the right media type. +// While adding the layers, each layer can be additionally mutated by providing a "mutate layer" function. +func EnsureMediaTypesAndLayers(image v1.Image, requestedTypes MediaTypes, mutateLayer func(idx int, layer v1.Layer) (v1.Layer, error)) (v1.Image, error) { + // (1) get data from the original image + // manifest + beforeManifest, err := image.Manifest() + if err != nil { + return nil, err + } + // config + beforeConfig, err := image.ConfigFile() + if err != nil { + return nil, err + } + // layers + beforeLayers, err := image.Layers() + if err != nil { + return nil, err + } + layersToSet := make([]v1.Layer, len(beforeLayers)) + for idx, layer := range beforeLayers { + mutatedLayer, err := mutateLayer(idx, layer) + if err != nil { + return nil, err + } + layersToSet[idx] = mutatedLayer + } + + // (2) construct a new image with the right manifest media type + manifestType := requestedTypes.ManifestType() + if manifestType == "" { + manifestType = beforeManifest.MediaType + } + retImage := mutate.MediaType(empty.Image, manifestType) + + // (3) set config media type + configType := requestedTypes.ConfigType() + if configType == "" { + configType = beforeManifest.Config.MediaType + } + // zero out history and diff IDs, as these will be updated when we call `mutate.Append` to add the layers + beforeHistory := NormalizedHistory(beforeConfig.History, len(beforeConfig.RootFS.DiffIDs)) + beforeConfig.History = []v1.History{} + beforeConfig.RootFS.DiffIDs = make([]v1.Hash, 0) + // set config + retImage, err = mutate.ConfigFile(retImage, beforeConfig) + if err != nil { + return nil, err + } + retImage = mutate.ConfigMediaType(retImage, configType) + // (4) set layers with the right media type + additions := layersAddendum(layersToSet, beforeHistory, requestedTypes.LayerType()) + if err != nil { + return nil, err + } + retImage, err = mutate.Append(retImage, additions...) + if err != nil { + return nil, err + } + afterLayers, err := retImage.Layers() + if err != nil { + return nil, err + } + if len(afterLayers) != len(beforeLayers) { + return nil, fmt.Errorf("found %d layers for image; expected %d", len(afterLayers), len(beforeLayers)) + } + return retImage, nil +} + +// layersAddendum creates an Addendum array with the given layers +// and the desired media type +func layersAddendum(layers []v1.Layer, history []v1.History, requestedType types.MediaType) []mutate.Addendum { + addendums := make([]mutate.Addendum, 0) + if len(history) != len(layers) { + history = make([]v1.History, len(layers)) + } + var err error + for idx, layer := range layers { + layerType := requestedType + if requestedType == "" { + // try to get a non-empty media type + if layerType, err = layer.MediaType(); err != nil { + layerType = "" + } + } + addendums = append(addendums, mutate.Addendum{ + Layer: layer, + History: history[idx], + MediaType: layerType, + }) + } + return addendums +} + +func NormalizedHistory(history []v1.History, nLayers int) []v1.History { + if history == nil { + return make([]v1.History, nLayers) + } + // ensure we remove history for empty layers + var normalizedHistory []v1.History + for _, h := range history { + if !h.EmptyLayer { + normalizedHistory = append(normalizedHistory, h) + } + } + if len(normalizedHistory) == nLayers { + return normalizedHistory + } + return make([]v1.History, nLayers) } func prepareNewWindowsImageIfNeeded(image *CNBImageCore) error { @@ -91,6 +301,7 @@ func prepareNewWindowsImageIfNeeded(image *CNBImageCore) error { if err != nil { return err } + // only append base layer to empty image if !(configFile.OS == "windows") || len(configFile.RootFS.DiffIDs) > 0 { return nil diff --git a/options.go b/options.go index ed058a61..061e33a3 100644 --- a/options.go +++ b/options.go @@ -8,12 +8,14 @@ import ( type ImageOptions struct { BaseImageRepoName string + PreviousImageRepoName string Config *v1.Config CreatedAt time.Time + MediaTypes MediaTypes Platform Platform + PreserveDigest bool PreserveHistory bool - PreviousImageRepoName string - MediaTypes MediaTypes + WithoutLayers bool // only relevant for layout images // These options are specified in each implementation's image constructor BaseImage v1.Image diff --git a/remote/new.go b/remote/new.go index 72aedd3e..74ebdc7b 100644 --- a/remote/new.go +++ b/remote/new.go @@ -292,7 +292,7 @@ func (i *Image) setUnderlyingImage(base v1.Image) error { return nil } // provided v1.Image media types differ from requested, override them - newBase, err := imgutil.OverrideMediaTypes(base, i.requestedMediaTypes) + newBase, err := imgutil.EnsureMediaTypes(base, i.requestedMediaTypes) if err != nil { return err } diff --git a/testhelpers/testhelpers.go b/testhelpers/testhelpers.go index cb1abc40..288bae5a 100644 --- a/testhelpers/testhelpers.go +++ b/testhelpers/testhelpers.go @@ -22,20 +22,18 @@ import ( "testing" "time" - "github.com/google/go-containerregistry/pkg/v1/types" - - "github.com/buildpacks/imgutil/layer" - dockertypes "github.com/docker/docker/api/types" dockercli "github.com/docker/docker/client" "github.com/docker/docker/pkg/jsonmessage" - "github.com/google/go-cmp/cmp" "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/name" v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/remote" + "github.com/google/go-containerregistry/pkg/v1/types" "github.com/pkg/errors" + + "github.com/buildpacks/imgutil/layer" ) func RandString(n int) string { From 37c0fdc16f552541725d328fbfeb152cee5c7e1b Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Tue, 30 Jan 2024 10:56:42 -0500 Subject: [PATCH 03/14] Remove empty image from layout package Signed-off-by: Natalie Arellano --- layout/new.go | 68 +++++++++++++++++---------------------------------- new.go | 3 +-- 2 files changed, 23 insertions(+), 48 deletions(-) diff --git a/layout/new.go b/layout/new.go index 8c3d0849..76e5907b 100644 --- a/layout/new.go +++ b/layout/new.go @@ -4,9 +4,6 @@ import ( "fmt" v1 "github.com/google/go-containerregistry/pkg/v1" - "github.com/google/go-containerregistry/pkg/v1/empty" - "github.com/google/go-containerregistry/pkg/v1/mutate" - "github.com/google/go-containerregistry/pkg/v1/types" "github.com/buildpacks/imgutil" ) @@ -17,23 +14,24 @@ func NewImage(path string, ops ...ImageOption) (*Image, error) { op(options) } options.Platform = processDefaultPlatformOption(options.Platform) - preferredMediaTypes := imgutil.GetPreferredMediaTypes(*options) var err error if options.PreviousImageRepoName != "" { - options.PreviousImage, err = newImageFromPath(options.PreviousImageRepoName, options.Platform, preferredMediaTypes) + options.PreviousImage, err = newImageFromPath(options.PreviousImageRepoName, options.Platform, imgutil.DefaultTypes) if err != nil { return nil, err } } - if options.BaseImage != nil { // options.BaseImage supersedes options.BaseImageRepoName - options.BaseImage, err = imgutil.EnsureMediaTypes(options.BaseImage, preferredMediaTypes) + if options.BaseImage == nil && options.BaseImageRepoName != "" { // options.BaseImage supersedes options.BaseImageRepoName + options.BaseImage, err = newImageFromPath(options.BaseImageRepoName, options.Platform, imgutil.DefaultTypes) if err != nil { return nil, err } - } else if options.BaseImageRepoName != "" { - options.BaseImage, err = newImageFromPath(options.BaseImageRepoName, options.Platform, preferredMediaTypes) + } + options.MediaTypes = imgutil.GetPreferredMediaTypes(*options) + if options.BaseImage != nil { + options.BaseImage, err = imgutil.EnsureMediaTypes(options.BaseImage, options.MediaTypes) if err != nil { return nil, err } @@ -64,29 +62,23 @@ 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 an empty image is returned. +// * If the image does not exist, then nothing is returned. func newImageFromPath(path string, withPlatform imgutil.Platform, withMediaTypes imgutil.MediaTypes) (v1.Image, error) { - var image v1.Image + if !imageExists(path) { + return nil, nil + } - if imageExists(path) { - layoutPath, err := FromPath(path) - if err != nil { - return nil, fmt.Errorf("failed to load layout from path: %w", err) - } - index, err := layoutPath.ImageIndex() - if err != nil { - return nil, fmt.Errorf("failed to load index: %w", err) - } - image, err = imageFromIndex(index, withPlatform) - if err != nil { - return nil, fmt.Errorf("failed to load image from index: %w", err) - } - } else { - var err error - image, err = emptyImage(withPlatform) - if err != nil { - return nil, fmt.Errorf("failed to initialize empty image: %w", err) - } + layoutPath, err := FromPath(path) + if err != nil { + return nil, fmt.Errorf("failed to load layout from path: %w", err) + } + index, err := layoutPath.ImageIndex() + if err != nil { + return nil, fmt.Errorf("failed to load index: %w", err) + } + image, err := imageFromIndex(index, withPlatform) + 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 @@ -131,19 +123,3 @@ func imageFromIndex(index v1.ImageIndex, platform imgutil.Platform) (v1.Image, e return index.Image(manifest.Digest) } - -func emptyImage(platform imgutil.Platform) (v1.Image, error) { - cfg := &v1.ConfigFile{ - Architecture: platform.Architecture, - History: []v1.History{}, - OS: platform.OS, - OSVersion: platform.OSVersion, - RootFS: v1.RootFS{ - Type: "layers", - DiffIDs: []v1.Hash{}, - }, - } - image := mutate.MediaType(empty.Image, types.OCIManifestSchema1) - image = mutate.ConfigMediaType(image, types.OCIConfigJSON) - return mutate.ConfigFile(image, cfg) -} diff --git a/new.go b/new.go index 57eaaedc..091da8fa 100644 --- a/new.go +++ b/new.go @@ -101,8 +101,7 @@ func GetPreferredMediaTypes(options ImageOptions) MediaTypes { return options.MediaTypes } if options.MediaTypes == MissingTypes && - options.BaseImage == nil && - options.BaseImageRepoName == "" { + options.BaseImage == nil { return OCITypes } return DefaultTypes From 8ca3b4aedb9dc9f9c221baeb4e65d4bf65c4fcf1 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Tue, 30 Jan 2024 11:16:14 -0500 Subject: [PATCH 04/14] Set created at when we save, not when we initialize This will allow us to preserve the digest when no modifying options are provided Signed-off-by: Natalie Arellano --- cnb_image.go | 39 +++++++++++++++++++++++++++++++++++- layout/layout_test.go | 5 +---- layout/save.go | 3 +++ layout/sparse/sparse_test.go | 3 +-- locallayout/image.go | 10 +++++++-- locallayout/image_test.go | 5 +---- new.go | 34 ------------------------------- 7 files changed, 52 insertions(+), 47 deletions(-) diff --git a/cnb_image.go b/cnb_image.go index 564932a2..f272dc5c 100644 --- a/cnb_image.go +++ b/cnb_image.go @@ -310,6 +310,13 @@ func (i *CNBImageCore) AddLayerWithDiffID(path, _ string) error { } func (i *CNBImageCore) AddLayerWithDiffIDAndHistory(path, _ string, history v1.History) error { + // ensure existing history + if err := i.MutateConfigFile(func(c *v1.ConfigFile) { + c.History = NormalizedHistory(c.History, len(c.RootFS.DiffIDs)) + }); err != nil { + return err + } + layer, err := tarball.LayerFromFile(path) if err != nil { return err @@ -437,7 +444,9 @@ func (i *CNBImageCore) ReuseLayerWithHistory(diffID string, history v1.History) if err != nil { return err } - if !i.preserveHistory { + if i.preserveHistory { + history.Created = v1.Time{Time: i.createdAt} + } else { history = emptyHistory } i.Image, err = mutate.Append( @@ -464,6 +473,34 @@ func (i *CNBImageCore) MutateConfigFile(withFunc func(c *v1.ConfigFile)) error { return err } +func (i *CNBImageCore) SetCreatedAtAndHistory() error { + var err error + // set created at + if err = i.MutateConfigFile(func(c *v1.ConfigFile) { + c.Created = v1.Time{Time: i.createdAt} + c.Container = "" + }); err != nil { + return err + } + // set history + if i.preserveHistory { + // set created at for each history + err = i.MutateConfigFile(func(c *v1.ConfigFile) { + for j := range c.History { + c.History[j].Created = v1.Time{Time: i.createdAt} + } + }) + } else { + // zero history + err = i.MutateConfigFile(func(c *v1.ConfigFile) { + for j := range c.History { + c.History[j] = v1.History{Created: v1.Time{Time: i.createdAt}} + } + }) + } + return err +} + func getConfigFile(image v1.Image) (*v1.ConfigFile, error) { configFile, err := image.ConfigFile() if err != nil { diff --git a/layout/layout_test.go b/layout/layout_test.go index d0ee1850..c8e53b67 100644 --- a/layout/layout_test.go +++ b/layout/layout_test.go @@ -502,10 +502,7 @@ func testImage(t *testing.T, when spec.G, it spec.S) { }) }) - when.Pend("#CreatedAt", func() { - // Previously, we only zeroed CreatedAt at the point of save. - // Now, we zero CreatedAt at the point of instantiation. - // If this behavior change is acceptable, we can remove this test. + when("#CreatedAt", func() { it.Before(func() { imagePath = filepath.Join(tmpDir, "new-created-at-image") }) diff --git a/layout/save.go b/layout/save.go index 27932526..bc6bfca8 100644 --- a/layout/save.go +++ b/layout/save.go @@ -12,6 +12,9 @@ func (i *Image) Save(additionalNames ...string) error { // SaveAs ignores the image `Name()` method and saves the image according to name & additional names provided to this method func (i *Image) SaveAs(name string, additionalNames ...string) error { + if err := i.SetCreatedAtAndHistory(); err != nil { + return err + } refName, err := i.GetAnnotateRefName() if err != nil { return err diff --git a/layout/sparse/sparse_test.go b/layout/sparse/sparse_test.go index e5ad871e..a1b73914 100644 --- a/layout/sparse/sparse_test.go +++ b/layout/sparse/sparse_test.go @@ -137,8 +137,7 @@ func testImage(t *testing.T, when spec.G, it spec.S) { }) when("#Digest", func() { - it.Pend("returns the original image digest when there are no modifications", func() { - // TODO: created at + it("returns the original image digest when there are no modifications", func() { image, err := sparse.NewImage(imagePath, testImage) h.AssertNil(t, err) diff --git a/locallayout/image.go b/locallayout/image.go index 1c97219e..e2e294e8 100644 --- a/locallayout/image.go +++ b/locallayout/image.go @@ -104,13 +104,19 @@ func (i *Image) Rebase(baseTopLayerDiffID string, withNewBase imgutil.Image) err } func (i *Image) Save(additionalNames ...string) error { - var err error + err := i.SetCreatedAtAndHistory() + if err != nil { + return err + } i.lastIdentifier, err = i.store.Save(i, i.Name(), additionalNames...) return err } func (i *Image) SaveAs(name string, additionalNames ...string) error { - var err error + err := i.SetCreatedAtAndHistory() + if err != nil { + return err + } i.lastIdentifier, err = i.store.Save(i, name, additionalNames...) return err } diff --git a/locallayout/image_test.go b/locallayout/image_test.go index 0a18293e..61f0d848 100644 --- a/locallayout/image_test.go +++ b/locallayout/image_test.go @@ -709,10 +709,7 @@ func testImage(t *testing.T, when spec.G, it spec.S) { }) }) - when.Pend("#CreatedAt", func() { - // Previously, we only zeroed CreatedAt at the point of save. - // Now, we zero CreatedAt at the point of instantiation. - // If this behavior change is acceptable, we can remove this test. + when("#CreatedAt", func() { it("returns the containers created at time", func() { img, err := local.NewImage(newTestImageName(), dockerClient, local.FromBaseImage(runnableBaseImageName)) h.AssertNil(t, err) diff --git a/new.go b/new.go index 091da8fa..1f53c1e7 100644 --- a/new.go +++ b/new.go @@ -41,13 +41,6 @@ func NewCNBImage(options ImageOptions) (*CNBImageCore, error) { return nil, err } - // ensure existing history - if err = image.MutateConfigFile(func(c *v1.ConfigFile) { - c.History = NormalizedHistory(c.History, len(c.RootFS.DiffIDs)) - }); err != nil { - return nil, err - } - // set config if requested if options.Config != nil { if err = image.MutateConfigFile(func(c *v1.ConfigFile) { @@ -57,33 +50,6 @@ func NewCNBImage(options ImageOptions) (*CNBImageCore, error) { } } - // set created at - if err = image.MutateConfigFile(func(c *v1.ConfigFile) { - c.Created = v1.Time{Time: image.createdAt} - c.Container = "" - }); err != nil { - return nil, err - } - // set history - if options.PreserveHistory { - // set created at for each history - err = image.MutateConfigFile(func(c *v1.ConfigFile) { - for j := range c.History { - c.History[j].Created = v1.Time{Time: image.createdAt} - } - }) - } else { - // zero history - err = image.MutateConfigFile(func(c *v1.ConfigFile) { - for j := range c.History { - c.History[j] = v1.History{Created: v1.Time{Time: image.createdAt}} - } - }) - } - if err != nil { - return nil, err - } - return image, nil } From b8563b8498e1d5b7cbef8ce5b01260a4ff83ac71 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Tue, 30 Jan 2024 15:24:08 -0500 Subject: [PATCH 05/14] Fix locallayout facade by caching layer data on the layer struct, not the image struct (image struct can go away) Signed-off-by: Natalie Arellano --- cnb_image.go | 5 - layout/v1_facade.go | 6 ++ locallayout/image.go | 8 +- locallayout/new.go | 56 ++++++----- locallayout/store.go | 100 ++++++++++--------- locallayout/v1_facade.go | 203 +++++++++++++++++++-------------------- 6 files changed, 192 insertions(+), 186 deletions(-) diff --git a/cnb_image.go b/cnb_image.go index f272dc5c..b6267d26 100644 --- a/cnb_image.go +++ b/cnb_image.go @@ -28,11 +28,6 @@ type CNBImageCore struct { previousImage v1.Image } -type IdentifiableV1Image interface { - v1.Image - Identifier() (Identifier, error) -} - var _ v1.Image = &CNBImageCore{} // FIXME: mark deprecated methods as deprecated on the interface when other packages (remote, layout) expose a v1.Image diff --git a/layout/v1_facade.go b/layout/v1_facade.go index 8f21d3e0..71890276 100644 --- a/layout/v1_facade.go +++ b/layout/v1_facade.go @@ -12,6 +12,7 @@ type v1LayerFacade struct { v1.Layer diffID v1.Hash digest v1.Hash + size int64 } func (l *v1LayerFacade) Compressed() (io.ReadCloser, error) { @@ -30,6 +31,10 @@ func (l *v1LayerFacade) Uncompressed() (io.ReadCloser, error) { return io.NopCloser(bytes.NewReader([]byte{})), nil } +func (l *v1LayerFacade) Size() (int64, error) { + return l.size, nil +} + func newLayerOrFacadeFrom(configFile v1.ConfigFile, manifestFile v1.Manifest, layerIndex int, originalLayer v1.Layer) (v1.Layer, error) { if hasData(originalLayer) { return originalLayer, nil @@ -44,6 +49,7 @@ func newLayerOrFacadeFrom(configFile v1.ConfigFile, manifestFile v1.Manifest, la Layer: originalLayer, diffID: configFile.RootFS.DiffIDs[layerIndex], digest: manifestFile.Layers[layerIndex].Digest, + size: manifestFile.Layers[layerIndex].Size, }, nil } diff --git a/locallayout/image.go b/locallayout/image.go index e2e294e8..f48c434b 100644 --- a/locallayout/image.go +++ b/locallayout/image.go @@ -5,7 +5,6 @@ import ( "fmt" "io" "strings" - "sync" v1 "github.com/google/go-containerregistry/pkg/v1" @@ -19,7 +18,6 @@ type Image struct { store *Store lastIdentifier string daemonOS string - downloadOnce *sync.Once } func (i *Image) Kind() string { @@ -79,11 +77,7 @@ func (i *Image) GetLayer(diffID string) (io.ReadCloser, error) { } func (i *Image) ensureLayers() error { - var err error - i.downloadOnce.Do(func() { - err = i.store.DownloadLayersFor(i.lastIdentifier) - }) - if err != nil { + if err := i.store.downloadLayersFor(i.lastIdentifier); err != nil { return fmt.Errorf("fetching base layers: %w", err) } return nil diff --git a/locallayout/new.go b/locallayout/new.go index ff81dbc4..1ca41033 100644 --- a/locallayout/new.go +++ b/locallayout/new.go @@ -8,6 +8,7 @@ import ( "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/image" "github.com/docker/docker/client" + v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/buildpacks/imgutil" ) @@ -26,23 +27,28 @@ func NewImage(repoName string, dockerClient DockerClient, ops ...func(*imgutil.I return nil, err } - options.PreviousImage, err = processPreviousImageOption(options.PreviousImageRepoName, dockerClient) + processPrevious, err := processImageOption(options.PreviousImageRepoName, dockerClient, true) if err != nil { return nil, err } + if processPrevious.image != nil { + options.PreviousImage = processPrevious.image + } var ( baseIdentifier string - store = &Store{dockerClient: dockerClient} + store *Store ) - baseImage, err := processBaseImageOption(options.BaseImageRepoName, dockerClient) + processBase, err := processImageOption(options.BaseImageRepoName, dockerClient, false) if err != nil { return nil, err } - if baseImage != nil { - options.BaseImage = baseImage - baseIdentifier = baseImage.identifier - store = baseImage.store + if processBase.image != nil { + options.BaseImage = processBase.image + baseIdentifier = processBase.identifier + store = processBase.layerStore + } else { + store = &Store{dockerClient: dockerClient, downloadOnce: &sync.Once{}} } cnbImage, err := imgutil.NewCNBImage(*options) @@ -56,7 +62,6 @@ func NewImage(repoName string, dockerClient DockerClient, ops ...func(*imgutil.I store: store, lastIdentifier: baseIdentifier, daemonOS: options.Platform.OS, - downloadOnce: &sync.Once{}, }, nil } @@ -86,32 +91,33 @@ func defaultPlatform(dockerClient DockerClient) (imgutil.Platform, error) { }, nil } -func processPreviousImageOption(repoName string, dockerClient DockerClient) (*v1ImageFacade, error) { - if repoName == "" { - return nil, nil - } - inspect, history, err := getInspectAndHistory(repoName, dockerClient) - if err != nil { - return nil, err - } - if inspect == nil { - return nil, nil - } - return newV1ImageFacadeFromInspect(*inspect, history, dockerClient, true) +type imageResult struct { + image v1.Image + identifier string + layerStore *Store } -func processBaseImageOption(repoName string, dockerClient DockerClient) (*v1ImageFacade, error) { +func processImageOption(repoName string, dockerClient DockerClient, downloadLayersOnAccess bool) (imageResult, error) { if repoName == "" { - return nil, nil + return imageResult{}, nil } inspect, history, err := getInspectAndHistory(repoName, dockerClient) if err != nil { - return nil, err + return imageResult{}, err } if inspect == nil { - return nil, nil + return imageResult{}, nil } - return newV1ImageFacadeFromInspect(*inspect, history, dockerClient, false) + layerStore := &Store{dockerClient: dockerClient, downloadOnce: &sync.Once{}} + image, err := newV1ImageFacadeFromInspect(*inspect, history, layerStore, downloadLayersOnAccess) + if err != nil { + return imageResult{}, err + } + return imageResult{ + image: image, + identifier: inspect.ID, + layerStore: layerStore, + }, nil } func getInspectAndHistory(repoName string, dockerClient DockerClient) (*types.ImageInspect, []image.HistoryResponseItem, error) { diff --git a/locallayout/store.go b/locallayout/store.go index 27da0ba4..e4e16dfb 100644 --- a/locallayout/store.go +++ b/locallayout/store.go @@ -10,6 +10,7 @@ import ( "os" "path/filepath" "strings" + "sync" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/image" @@ -17,6 +18,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" @@ -29,6 +31,7 @@ type Store struct { // required dockerClient DockerClient // optional + downloadOnce *sync.Once onDiskLayers []v1.Layer } @@ -63,17 +66,13 @@ func (s *Store) Delete(identifier string) error { return err } -func (s *Store) Save(image imgutil.IdentifiableV1Image, withName string, withAdditionalNames ...string) (string, error) { +func (s *Store) Save(image *Image, withName string, withAdditionalNames ...string) (string, error) { withName = tryNormalizing(withName) // save inspect, err := s.doSave(image, withName) if err != nil { - identifier, err := image.Identifier() - if err != nil { - return "", err - } - if err = s.DownloadLayersFor(identifier.String()); err != nil { + if err = image.ensureLayers(); err != nil { return "", err } inspect, err = s.doSave(image, withName) @@ -176,8 +175,9 @@ func (s *Store) addImageToTar(tw *tar.Writer, image v1.Image, withName string) e return err } var ( - layerPaths []string - blankIdx int + layerPaths []string + blankIdx int + processingBaseLayer = true ) for _, layer := range layers { var layerName string @@ -185,7 +185,7 @@ func (s *Store) addImageToTar(tw *tar.Writer, image v1.Image, withName string) e if err != nil { return err } - if size == -1 { // layer facade fronting empty layer + if size == -1 && processingBaseLayer { // layer facade fronting empty layer layerName = fmt.Sprintf("blank_%d", blankIdx) blankIdx++ hdr := &tar.Header{Name: layerName, Mode: 0644, Size: 0} @@ -193,6 +193,7 @@ func (s *Store) addImageToTar(tw *tar.Writer, image v1.Image, withName string) e return err } } else { + processingBaseLayer = false // once we have layer data, we can't rely on future layers being in the daemon layerName, err = s.addLayerToTar(tw, layer) if err != nil { return err @@ -221,7 +222,7 @@ func (s *Store) addLayerToTar(tw *tar.Writer, layer v1.Layer) (string, error) { } withName := fmt.Sprintf("/%s.tar", layerDiffID.String()) - uncompressedSize, err := getLayerSize(layer) + uncompressedSize, err := getLayerSize(layer) // FIXME: this degrades performance compared to `local` package if err != nil { return "", err } @@ -290,7 +291,7 @@ func ensureReaderClosed(r io.ReadCloser) error { return err } -func (s *Store) SaveFile(image imgutil.IdentifiableV1Image, withName string) (string, error) { +func (s *Store) SaveFile(image *Image, withName string) (string, error) { withName = tryNormalizing(withName) f, err := os.CreateTemp("", "imgutil.local.image.export.*.tar") @@ -308,11 +309,7 @@ func (s *Store) SaveFile(image imgutil.IdentifiableV1Image, withName string) (st // (1) WithPreviousImage(), or (2) FromBaseImage(). // The former is only relevant if ReuseLayers() has been called which takes care of resolving them. // The latter case needs to be handled explicitly. - identifier, err := image.Identifier() - if err != nil { - return "", err - } - if err = s.DownloadLayersFor(identifier.String()); err != nil { + if err = image.ensureLayers(); err != nil { return "", err } @@ -345,40 +342,39 @@ func (s *Store) SaveFile(image imgutil.IdentifiableV1Image, withName string) (st // layers -func (s *Store) DownloadLayersFor(identifier string) error { - layers, err := downloadLayersFor(identifier, s.dockerClient) - if err != nil { - return err - } - s.onDiskLayers = append(s.onDiskLayers, layers...) - return nil +func (s *Store) downloadLayersFor(identifier string) error { + var err error + s.downloadOnce.Do(func() { + err = s.doDownloadLayersFor(identifier) + }) + return err } -func downloadLayersFor(identifier string, dockerClient DockerClient) ([]v1.Layer, error) { +func (s *Store) doDownloadLayersFor(identifier string) error { if identifier == "" { - return nil, nil + return nil } ctx := context.Background() - imageReader, err := dockerClient.ImageSave(ctx, []string{identifier}) + imageReader, err := s.dockerClient.ImageSave(ctx, []string{identifier}) if err != nil { - return nil, fmt.Errorf("saving base image with ID %q from the docker daemon: %w", identifier, err) + return fmt.Errorf("saving base image with ID %q from the docker daemon: %w", identifier, err) } defer ensureReaderClosed(imageReader) tmpDir, err := os.MkdirTemp("", "imgutil.local.image.") if err != nil { - return nil, fmt.Errorf("failed to create temp dir: %w", err) + return fmt.Errorf("failed to create temp dir: %w", err) } err = untar(imageReader, tmpDir) if err != nil { - return nil, err + return err } mf, err := os.Open(filepath.Clean(filepath.Join(tmpDir, "manifest.json"))) if err != nil { - return nil, err + return err } defer mf.Close() @@ -387,15 +383,15 @@ func downloadLayersFor(identifier string, dockerClient DockerClient) ([]v1.Layer Layers []string } if err := json.NewDecoder(mf).Decode(&manifest); err != nil { - return nil, err + return err } if len(manifest) != 1 { - return nil, fmt.Errorf("manifest.json had unexpected number of entries: %d", len(manifest)) + return fmt.Errorf("manifest.json had unexpected number of entries: %d", len(manifest)) } cfg, err := os.Open(filepath.Clean(filepath.Join(tmpDir, manifest[0].Config))) if err != nil { - return nil, err + return err } defer cfg.Close() var configFile struct { @@ -404,22 +400,17 @@ func downloadLayersFor(identifier string, dockerClient DockerClient) ([]v1.Layer } `json:"rootfs"` } if err = json.NewDecoder(cfg).Decode(&configFile); err != nil { - return nil, err + return err } - layers := make([]v1.Layer, len(configFile.RootFS.DiffIDs)) - for idx, diffID := range configFile.RootFS.DiffIDs { - var h v1.Hash - h, err = v1.NewHash(diffID) + for idx := range configFile.RootFS.DiffIDs { + layer, err := tarball.LayerFromFile(filepath.Join(tmpDir, manifest[0].Layers[idx])) if err != nil { - return nil, err - } - layers[idx] = &v1LayerFacade{ - diffID: h, - optionalLayerPath: filepath.Join(tmpDir, manifest[0].Layers[idx]), + return err } + s.onDiskLayers = append(s.onDiskLayers, layer) } - return layers, nil + return nil } func untar(r io.Reader, dest string) error { @@ -486,6 +477,23 @@ func cleanPath(dest, header string) (string, error) { return "", fmt.Errorf("bad filepath: %s", header) } -func (s *Store) Layers() []v1.Layer { - return s.onDiskLayers +func (s *Store) LayerByDiffID(h v1.Hash) (v1.Layer, error) { + layer := findLayer(h, s.onDiskLayers) + if layer == nil { + return nil, fmt.Errorf("failed to find layer with diff ID %q", h.String()) + } + return layer, nil +} + +func findLayer(withHash v1.Hash, inLayers []v1.Layer) v1.Layer { + for _, layer := range inLayers { + layerHash, err := layer.DiffID() + if err != nil { + continue + } + if layerHash.String() == withHash.String() { + return layer + } + } + return nil } diff --git a/locallayout/v1_facade.go b/locallayout/v1_facade.go index c96ead9e..35418e29 100644 --- a/locallayout/v1_facade.go +++ b/locallayout/v1_facade.go @@ -4,8 +4,6 @@ import ( "bytes" "fmt" "io" - "os" - "sync" "time" "github.com/docker/docker/api/types" @@ -19,68 +17,12 @@ import ( "github.com/buildpacks/imgutil" ) -// v1ImageFacade wraps a v1.Image constructed from the output of `docker inspect`. +// newV1ImageFacadeFromInspect returns a v1.Image constructed from the output of `docker inspect`. // It is used to provide a v1.Image implementation for previous images and base images. -// The v1ImageFacade is never modified, but it may become the underlying v1.Image for imgutil.CNBImageCore images. -// A v1ImageFacade will try to return layer data if the layers exist on disk, -// otherwise it will return empty layer data. -// By storing a pointer to the image store, users can update the store to force a v1ImageFacade to return layer data. -type v1ImageFacade struct { - v1.Image - emptyLayers []v1.Layer - - // for downloading layers from the daemon as needed - store *Store - downloadLayersOnAccess bool // set to true to downloading ALL the image layers from the daemon when LayerByDiffID is called - downloadOnce *sync.Once - identifier string -} - -var _ v1.Image = &v1ImageFacade{} - -func (i *v1ImageFacade) LayerByDiffID(h v1.Hash) (v1.Layer, error) { - if layer := findLayer(h, i.store.Layers()); layer != nil { - return layer, nil - } - if i.downloadLayersOnAccess { - if err := i.ensureLayers(); err != nil { - return nil, err - } - if layer := findLayer(h, i.store.Layers()); layer != nil { - return layer, nil - } - } - if layer := findLayer(h, i.emptyLayers); layer != nil { - return layer, nil - } - return nil, fmt.Errorf("failed to find layer with diff ID %q", h.String()) -} - -func (i *v1ImageFacade) ensureLayers() error { - var err error - i.downloadOnce.Do(func() { - err = i.store.DownloadLayersFor(i.identifier) - }) - if err != nil { - return fmt.Errorf("fetching base layers: %w", err) - } - return nil -} - -func findLayer(withHash v1.Hash, inLayers []v1.Layer) v1.Layer { - for _, layer := range inLayers { - layerHash, err := layer.DiffID() - if err != nil { - continue - } - if layerHash.String() == withHash.String() { - return layer - } - } - return nil -} - -func newV1ImageFacadeFromInspect(dockerInspect types.ImageInspect, history []image.HistoryResponseItem, dockerClient DockerClient, downloadLayersOnAccess bool) (*v1ImageFacade, error) { +// The facade is never modified, but it may become the underlying v1.Image for imgutil.CNBImageCore images. +// The underlying layers will return data if they are contained in the store. +// By storing a pointer to the image store, callers can update the store to force the layers to return data. +func newV1ImageFacadeFromInspect(dockerInspect types.ImageInspect, history []image.HistoryResponseItem, withStore *Store, downloadLayersOnAccess bool) (v1.Image, error) { rootFS, err := toV1RootFS(dockerInspect.RootFS) if err != nil { return nil, err @@ -98,34 +40,61 @@ func newV1ImageFacadeFromInspect(dockerInspect types.ImageInspect, history []ima OSVersion: dockerInspect.OsVersion, Variant: dockerInspect.Variant, } - layers := newEmptyLayerListFrom(configFile) - // first, append each layer to the image to update the layers in the underlying manifest - img, err := mutate.ConfigFile(empty.Image, &v1.ConfigFile{}) + layersToSet := newEmptyLayerListFrom(configFile, dockerInspect.ID, withStore, downloadLayersOnAccess) + return imageFrom(layersToSet, configFile, imgutil.DockerTypes) +} + +func imageFrom(layers []v1.Layer, configFile *v1.ConfigFile, requestedTypes imgutil.MediaTypes) (v1.Image, error) { + // (1) construct a new image with the right manifest media type + manifestType := requestedTypes.ManifestType() + retImage := mutate.MediaType(empty.Image, manifestType) + + // (2) set config media type + configType := requestedTypes.ConfigType() + // zero out history and diff IDs, as these will be updated when we call `mutate.Append` to add the layers + beforeHistory := imgutil.NormalizedHistory(configFile.History, len(configFile.RootFS.DiffIDs)) + configFile.History = []v1.History{} + configFile.RootFS.DiffIDs = make([]v1.Hash, 0) + // set config + var err error + retImage, err = mutate.ConfigFile(retImage, configFile) if err != nil { return nil, err } - for _, layer := range layers { - img, err = mutate.Append(img, mutate.Addendum{ - Layer: layer, - MediaType: v1types.OCILayer, - }) - if err != nil { - return nil, err - } + retImage = mutate.ConfigMediaType(retImage, configType) + // (3) set layers with the right media type + additions := layersAddendum(layers, beforeHistory, requestedTypes.LayerType()) + if err != nil { + return nil, err } - // then, set the config file - img, err = mutate.ConfigFile(img, configFile) + retImage, err = mutate.Append(retImage, additions...) if err != nil { return nil, err } - return &v1ImageFacade{ - Image: img, - emptyLayers: newEmptyLayerListFrom(configFile), - store: &Store{dockerClient: dockerClient}, - downloadLayersOnAccess: downloadLayersOnAccess, - downloadOnce: &sync.Once{}, - identifier: dockerInspect.ID, - }, nil + afterLayers, err := retImage.Layers() + if err != nil { + return nil, err + } + if len(afterLayers) != len(layers) { + return nil, fmt.Errorf("found %d layers for image; expected %d", len(afterLayers), len(layers)) + } + return retImage, nil +} + +func layersAddendum(layers []v1.Layer, history []v1.History, requestedType v1types.MediaType) []mutate.Addendum { + addendums := make([]mutate.Addendum, 0) + if len(history) != len(layers) { + history = make([]v1.History, len(layers)) + } + for idx, layer := range layers { + layerType := requestedType + addendums = append(addendums, mutate.Addendum{ + Layer: layer, + History: history[idx], + MediaType: layerType, + }) + } + return addendums } func toV1RootFS(dockerRootFS types.RootFS) (v1.RootFS, error) { @@ -213,51 +182,79 @@ func toV1Config(dockerCfg *container.Config) v1.Config { var _ v1.Layer = &v1LayerFacade{} type v1LayerFacade struct { - diffID v1.Hash - optionalLayerPath string + diffID v1.Hash + store *Store + // for downloading layers from the daemon as needed + downloadOnAccess bool + imageIdentifier string } -func newEmptyLayerListFrom(configFile *v1.ConfigFile) []v1.Layer { +func newEmptyLayerListFrom(configFile *v1.ConfigFile, withImageIdentifier string, withStore *Store, downloadOnAccess bool) []v1.Layer { layers := make([]v1.Layer, len(configFile.RootFS.DiffIDs)) for idx, diffID := range configFile.RootFS.DiffIDs { layers[idx] = &v1LayerFacade{ - diffID: diffID, + diffID: diffID, + store: withStore, + downloadOnAccess: downloadOnAccess, + imageIdentifier: withImageIdentifier, } } return layers } -func (l v1LayerFacade) Digest() (v1.Hash, error) { - return v1.Hash{}, nil +func (l v1LayerFacade) Compressed() (io.ReadCloser, error) { + return io.NopCloser(bytes.NewReader([]byte{})), nil } func (l v1LayerFacade) DiffID() (v1.Hash, error) { return l.diffID, nil } -func (l v1LayerFacade) Compressed() (io.ReadCloser, error) { - return io.NopCloser(bytes.NewReader([]byte{})), nil +func (l v1LayerFacade) Digest() (v1.Hash, error) { + return v1.Hash{}, nil } func (l v1LayerFacade) Uncompressed() (io.ReadCloser, error) { - if l.optionalLayerPath != "" { - f, err := os.Open(l.optionalLayerPath) - if err != nil { - return nil, err - } - return f, nil + layer, err := l.store.LayerByDiffID(l.diffID) + if err == nil { + return layer.Uncompressed() } - return io.NopCloser(bytes.NewReader([]byte{})), nil + if !l.downloadOnAccess { + return io.NopCloser(bytes.NewReader([]byte{})), nil + } + if err = l.store.downloadLayersFor(l.imageIdentifier); err != nil { + return nil, err + } + layer, err = l.store.LayerByDiffID(l.diffID) + if err != nil { + return io.NopCloser(bytes.NewReader([]byte{})), nil + } + return layer.Uncompressed() } // Size returns a sentinel value indicating if the layer has data. func (l v1LayerFacade) Size() (int64, error) { - if l.optionalLayerPath != "" { - return 1, nil + layer, err := l.store.LayerByDiffID(l.diffID) + if err == nil { + return layer.Size() + } + if !l.downloadOnAccess { + return -1, nil + } + if err = l.store.downloadLayersFor(l.imageIdentifier); err != nil { + return -1, err } - return -1, nil + layer, err = l.store.LayerByDiffID(l.diffID) + if err != nil { + return -1, nil + } + return layer.Size() } func (l v1LayerFacade) MediaType() (v1types.MediaType, error) { - return v1types.OCILayer, nil + layer, err := l.store.LayerByDiffID(l.diffID) + if err != nil { + return v1types.OCILayer, nil + } + return layer.MediaType() } From 5a1af609f8ef0bfe212c9636d8cc219b1b83e3b8 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Tue, 30 Jan 2024 15:50:46 -0500 Subject: [PATCH 06/14] Fix sparse test - comparing digest is only possible before save Signed-off-by: Natalie Arellano --- layout/new.go | 2 +- layout/sparse/sparse_test.go | 3 --- new.go | 2 +- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/layout/new.go b/layout/new.go index 76e5907b..9b6e61e6 100644 --- a/layout/new.go +++ b/layout/new.go @@ -31,7 +31,7 @@ func NewImage(path string, ops ...ImageOption) (*Image, error) { } options.MediaTypes = imgutil.GetPreferredMediaTypes(*options) if options.BaseImage != nil { - options.BaseImage, err = imgutil.EnsureMediaTypes(options.BaseImage, options.MediaTypes) + options.BaseImage, err = imgutil.EnsureMediaTypes(options.BaseImage, options.MediaTypes) // FIXME: this can move into imgutil constructor if err != nil { return nil, err } diff --git a/layout/sparse/sparse_test.go b/layout/sparse/sparse_test.go index a1b73914..147a164e 100644 --- a/layout/sparse/sparse_test.go +++ b/layout/sparse/sparse_test.go @@ -141,9 +141,6 @@ func testImage(t *testing.T, when spec.G, it spec.S) { image, err := sparse.NewImage(imagePath, testImage) h.AssertNil(t, err) - err = image.Save() - h.AssertNil(t, err) - expectedDigest, err := testImage.Digest() h.AssertNil(t, err) diff --git a/new.go b/new.go index 1f53c1e7..92177b93 100644 --- a/new.go +++ b/new.go @@ -191,7 +191,7 @@ func EnsureMediaTypesAndLayers(image v1.Image, requestedTypes MediaTypes, mutate configType = beforeManifest.Config.MediaType } // zero out history and diff IDs, as these will be updated when we call `mutate.Append` to add the layers - beforeHistory := NormalizedHistory(beforeConfig.History, len(beforeConfig.RootFS.DiffIDs)) + beforeHistory := beforeConfig.History beforeConfig.History = []v1.History{} beforeConfig.RootFS.DiffIDs = make([]v1.Hash, 0) // set config From a65b1a9d6484001fc56b72d11c1f7c8a0a47ebcd Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Thu, 1 Feb 2024 11:57:40 -0500 Subject: [PATCH 07/14] Remove unnecessary check on layer ordering Signed-off-by: Natalie Arellano --- locallayout/store.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/locallayout/store.go b/locallayout/store.go index e4e16dfb..08e98ed0 100644 --- a/locallayout/store.go +++ b/locallayout/store.go @@ -175,9 +175,8 @@ func (s *Store) addImageToTar(tw *tar.Writer, image v1.Image, withName string) e return err } var ( - layerPaths []string - blankIdx int - processingBaseLayer = true + layerPaths []string + blankIdx int ) for _, layer := range layers { var layerName string @@ -185,7 +184,7 @@ func (s *Store) addImageToTar(tw *tar.Writer, image v1.Image, withName string) e if err != nil { return err } - if size == -1 && processingBaseLayer { // layer facade fronting empty layer + if size == -1 { // layer facade fronting empty layer layerName = fmt.Sprintf("blank_%d", blankIdx) blankIdx++ hdr := &tar.Header{Name: layerName, Mode: 0644, Size: 0} @@ -193,7 +192,6 @@ func (s *Store) addImageToTar(tw *tar.Writer, image v1.Image, withName string) e return err } } else { - processingBaseLayer = false // once we have layer data, we can't rely on future layers being in the daemon layerName, err = s.addLayerToTar(tw, layer) if err != nil { return err From 9cef4d4511eef206d7a4567b23a800e4fcc063bd Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Fri, 2 Feb 2024 14:50:19 -0500 Subject: [PATCH 08/14] Fix: always preserve the digest when base image is provided and no media types (or other modifications) are requested Signed-off-by: Natalie Arellano --- cnb_image.go | 12 +++--- layout/layout_test.go | 29 +++++++++++++ layout/new.go | 38 ++++++++--------- layout/v1_facade.go | 94 +++++++++++++++++++++++++++++++++++++++++++ new.go | 73 ++++++++++++++------------------- remote/new.go | 2 +- 6 files changed, 177 insertions(+), 71 deletions(-) diff --git a/cnb_image.go b/cnb_image.go index b6267d26..af8c78a8 100644 --- a/cnb_image.go +++ b/cnb_image.go @@ -393,11 +393,11 @@ 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) } @@ -405,11 +405,11 @@ func (i *CNBImageCore) ReuseLayer(diffID string) error { 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() { @@ -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} diff --git a/layout/layout_test.go b/layout/layout_test.go index c8e53b67..8e7b2d01 100644 --- a/layout/layout_test.go +++ b/layout/layout_test.go @@ -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() { @@ -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() { diff --git a/layout/new.go b/layout/new.go index 9b6e61e6..9d8586d1 100644 --- a/layout/new.go +++ b/layout/new.go @@ -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 } @@ -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 } @@ -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 diff --git a/layout/v1_facade.go b/layout/v1_facade.go index 71890276..8a7fa530 100644 --- a/layout/v1_facade.go +++ b/layout/v1_facade.go @@ -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 diff --git a/new.go b/new.go index 92177b93..5c47085d 100644 --- a/new.go +++ b/new.go @@ -34,8 +34,6 @@ func NewCNBImage(options ImageOptions) (*CNBImageCore, error) { } } - // FIXME: we can call EnsureMediaTypesAndLayers here when locallayout supports replacing the underlying image - // ensure windows if err = prepareNewWindowsImageIfNeeded(image); err != nil { return nil, err @@ -130,113 +128,102 @@ func emptyV1(withPlatform Platform, withMediaTypes MediaTypes) (v1.Image, error) if err != nil { return nil, err } - return EnsureMediaTypes(image, withMediaTypes) + image, _, err = EnsureMediaTypesAndLayers(image, withMediaTypes, PreserveLayers) + return image, err } -func PreserveLayers(idx int, layer v1.Layer) (v1.Layer, error) { +func PreserveLayers(_ int, layer v1.Layer) (v1.Layer, error) { return layer, nil } -// EnsureMediaTypes replaces the provided image with a new image that has the desired media types. +// EnsureMediaTypesAndLayers replaces the provided image with a new image that has the desired media types. // It does this by constructing a manifest and config from the provided image, // and adding the layers from the provided image to the new image with the right media type. // If requested types are missing or default, it does nothing. -func EnsureMediaTypes(image v1.Image, requestedTypes MediaTypes) (v1.Image, error) { +// While adding the layers, each layer can be additionally mutated by providing a "mutate layer" function. +func EnsureMediaTypesAndLayers(image v1.Image, requestedTypes MediaTypes, mutateLayer func(idx int, layer v1.Layer) (v1.Layer, error)) (v1.Image, bool, error) { if requestedTypes == MissingTypes || requestedTypes == DefaultTypes { - return image, nil + return image, false, nil } - return EnsureMediaTypesAndLayers(image, requestedTypes, PreserveLayers) -} - -// EnsureMediaTypesAndLayers replaces the provided image with a new image that has the desired media types. -// It does this by constructing a manifest and config from the provided image, -// and adding the layers from the provided image to the new image with the right media type. -// While adding the layers, each layer can be additionally mutated by providing a "mutate layer" function. -func EnsureMediaTypesAndLayers(image v1.Image, requestedTypes MediaTypes, mutateLayer func(idx int, layer v1.Layer) (v1.Layer, error)) (v1.Image, error) { // (1) get data from the original image // manifest beforeManifest, err := image.Manifest() if err != nil { - return nil, err + return nil, false, fmt.Errorf("failed to get manifest: %w", err) } // config beforeConfig, err := image.ConfigFile() if err != nil { - return nil, err + return nil, false, fmt.Errorf("failed to get config: %w", err) } // layers beforeLayers, err := image.Layers() if err != nil { - return nil, err + return nil, false, fmt.Errorf("failed to get layers: %w", err) } - layersToSet := make([]v1.Layer, len(beforeLayers)) - for idx, layer := range beforeLayers { - mutatedLayer, err := mutateLayer(idx, layer) + var layersToAdd []v1.Layer + for idx, l := range beforeLayers { + layer, err := mutateLayer(idx, l) if err != nil { - return nil, err + return nil, false, fmt.Errorf("failed to mutate layer: %w", err) } - layersToSet[idx] = mutatedLayer + layersToAdd = append(layersToAdd, layer) } - // (2) construct a new image with the right manifest media type + // (2) construct a new image manifest with the right media type manifestType := requestedTypes.ManifestType() if manifestType == "" { manifestType = beforeManifest.MediaType } retImage := mutate.MediaType(empty.Image, manifestType) - // (3) set config media type + // (3) set config with the right media type configType := requestedTypes.ConfigType() if configType == "" { configType = beforeManifest.Config.MediaType } - // zero out history and diff IDs, as these will be updated when we call `mutate.Append` to add the layers + // zero out diff IDs and history, these will be added back when we append the layers beforeHistory := beforeConfig.History beforeConfig.History = []v1.History{} - beforeConfig.RootFS.DiffIDs = make([]v1.Hash, 0) - // set config + beforeConfig.RootFS.DiffIDs = []v1.Hash{} retImage, err = mutate.ConfigFile(retImage, beforeConfig) if err != nil { - return nil, err + return nil, false, fmt.Errorf("failed to set config: %w", err) } retImage = mutate.ConfigMediaType(retImage, configType) + // (4) set layers with the right media type - additions := layersAddendum(layersToSet, beforeHistory, requestedTypes.LayerType()) + additions := layersAddendum(layersToAdd, beforeHistory, requestedTypes.LayerType()) if err != nil { - return nil, err + return nil, false, err } retImage, err = mutate.Append(retImage, additions...) if err != nil { - return nil, err + return nil, false, fmt.Errorf("failed to append layers: %w", err) } - afterLayers, err := retImage.Layers() - if err != nil { - return nil, err - } - if len(afterLayers) != len(beforeLayers) { - return nil, fmt.Errorf("found %d layers for image; expected %d", len(afterLayers), len(beforeLayers)) - } - return retImage, nil + + return retImage, true, nil } // layersAddendum creates an Addendum array with the given layers // and the desired media type func layersAddendum(layers []v1.Layer, history []v1.History, requestedType types.MediaType) []mutate.Addendum { addendums := make([]mutate.Addendum, 0) + history = NormalizedHistory(history, len(layers)) if len(history) != len(layers) { history = make([]v1.History, len(layers)) } var err error - for idx, layer := range layers { + for idx, l := range layers { layerType := requestedType if requestedType == "" { // try to get a non-empty media type - if layerType, err = layer.MediaType(); err != nil { + if layerType, err = l.MediaType(); err != nil { layerType = "" } } addendums = append(addendums, mutate.Addendum{ - Layer: layer, + Layer: l, History: history[idx], MediaType: layerType, }) diff --git a/remote/new.go b/remote/new.go index 74ebdc7b..a392de57 100644 --- a/remote/new.go +++ b/remote/new.go @@ -292,7 +292,7 @@ func (i *Image) setUnderlyingImage(base v1.Image) error { return nil } // provided v1.Image media types differ from requested, override them - newBase, err := imgutil.EnsureMediaTypes(base, i.requestedMediaTypes) + newBase, _, err := imgutil.EnsureMediaTypesAndLayers(base, i.requestedMediaTypes, imgutil.PreserveLayers) if err != nil { return err } From 244aa169ce0afca7ac76a826321870cb3cf24667 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Fri, 2 Feb 2024 15:22:11 -0500 Subject: [PATCH 09/14] Force compute Signed-off-by: Natalie Arellano --- new.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/new.go b/new.go index 5c47085d..5dfae9eb 100644 --- a/new.go +++ b/new.go @@ -202,6 +202,15 @@ func EnsureMediaTypesAndLayers(image v1.Image, requestedTypes MediaTypes, mutate return nil, false, fmt.Errorf("failed to append layers: %w", err) } + // (5) force compute + afterLayers, err := retImage.Layers() + if err != nil { + return nil, false, fmt.Errorf("failed to get layers: %w", err) + } + if len(afterLayers) != len(beforeLayers) { + return nil, false, fmt.Errorf("expected %d layers; got %d", len(beforeLayers), len(afterLayers)) + } + return retImage, true, nil } From f737e8d213645e279ee411ee3c9193d3c5818b90 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Fri, 2 Feb 2024 15:33:54 -0500 Subject: [PATCH 10/14] Add missing fixture Signed-off-by: Natalie Arellano --- layout/layout_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/layout/layout_test.go b/layout/layout_test.go index 8e7b2d01..973cb9a4 100644 --- a/layout/layout_test.go +++ b/layout/layout_test.go @@ -258,7 +258,7 @@ func testImage(t *testing.T, when spec.G, it spec.S) { 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"))) + img, err := layout.NewImage(imagePath, layout.FromBaseImagePath(filepath.Join("testdata", "layout", "busybox-sparse"))) h.AssertNil(t, err) digest, err := img.Digest() h.AssertNil(t, err) From 59770c97283a5fdb798e8919eb13c49194a1260d Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Fri, 2 Feb 2024 17:21:00 -0500 Subject: [PATCH 11/14] Skip Windows Signed-off-by: Natalie Arellano --- layout/layout_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/layout/layout_test.go b/layout/layout_test.go index 973cb9a4..f5c91f8a 100644 --- a/layout/layout_test.go +++ b/layout/layout_test.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "path/filepath" + "runtime" "strings" "testing" "time" @@ -257,6 +258,11 @@ func testImage(t *testing.T, when spec.G, it spec.S) { }) when("existing config has extra fields", func() { + it.Before(func() { + if runtime.GOOS == "windows" { + t.Skip("Due to line endings the digest is different on Windows") + } + }) it("returns an unmodified digest", func() { img, err := layout.NewImage(imagePath, layout.FromBaseImagePath(filepath.Join("testdata", "layout", "busybox-sparse"))) h.AssertNil(t, err) From 78930ee4eb037c121790fdfba5d22fe4b2250b20 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Mon, 5 Feb 2024 11:24:24 -0500 Subject: [PATCH 12/14] Revert "Skip Windows" This reverts commit 59770c97283a5fdb798e8919eb13c49194a1260d. --- layout/layout_test.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/layout/layout_test.go b/layout/layout_test.go index f5c91f8a..973cb9a4 100644 --- a/layout/layout_test.go +++ b/layout/layout_test.go @@ -4,7 +4,6 @@ import ( "fmt" "os" "path/filepath" - "runtime" "strings" "testing" "time" @@ -258,11 +257,6 @@ func testImage(t *testing.T, when spec.G, it spec.S) { }) when("existing config has extra fields", func() { - it.Before(func() { - if runtime.GOOS == "windows" { - t.Skip("Due to line endings the digest is different on Windows") - } - }) it("returns an unmodified digest", func() { img, err := layout.NewImage(imagePath, layout.FromBaseImagePath(filepath.Join("testdata", "layout", "busybox-sparse"))) h.AssertNil(t, err) From 33de3077c0b50b8a3d643e265a2a43d577dbf9a1 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Mon, 5 Feb 2024 09:53:41 -0500 Subject: [PATCH 13/14] Try to fix Windows Signed-off-by: Natalie Arellano --- .github/workflows/test.yaml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index c509b45d..23efe3f3 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -22,6 +22,11 @@ jobs: test-and-build-windows: runs-on: windows-2019 steps: + - name: Set git to use LF and symlinks + run: | + git config --global core.autocrlf false + git config --global core.eol lf + git config --global core.symlinks true - uses: actions/checkout@v4 - name: Set up go uses: actions/setup-go@v5 From 7ed58a54c5f17c1ddca7f0fdf4c316302ecf953e Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Mon, 5 Feb 2024 11:38:34 -0500 Subject: [PATCH 14/14] Add docs comments Signed-off-by: Natalie Arellano --- layout/options.go | 41 +++++++++++++++++++++++++++++------------ 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/layout/options.go b/layout/options.go index 1d2be04a..1204943f 100644 --- a/layout/options.go +++ b/layout/options.go @@ -10,54 +10,71 @@ import ( type ImageOption func(*imgutil.ImageOptions) -func FromBaseImagePath(name string) func(*imgutil.ImageOptions) { - return func(o *imgutil.ImageOptions) { - o.BaseImageRepoName = name - } -} - +// FromBaseImage loads the provided image as the manifest, config, and layers for the working image. +// If the image is not found, it does nothing. func FromBaseImage(image v1.Image) func(*imgutil.ImageOptions) { return func(o *imgutil.ImageOptions) { o.BaseImage = image } } -func WithConfig(c *v1.Config) func(*imgutil.ImageOptions) { +// FromBaseImagePath (layout only) loads the image at the provided path as the manifest, config, and layers for the working image. +// If the image is not found, it does nothing. +func FromBaseImagePath(name string) func(*imgutil.ImageOptions) { return func(o *imgutil.ImageOptions) { - o.Config = c + o.BaseImageRepoName = name } } +// WithCreatedAt lets a caller set the "created at" timestamp for the working image when saved. +// If not provided, the default is imgutil.NormalizedDateTime. func WithCreatedAt(t time.Time) func(*imgutil.ImageOptions) { return func(o *imgutil.ImageOptions) { o.CreatedAt = t } } +// WithConfig lets a caller provided a `config` object for the working image. +func WithConfig(c *v1.Config) func(*imgutil.ImageOptions) { + return func(o *imgutil.ImageOptions) { + o.Config = c + } +} + +// WithDefaultPlatform provides the default Architecture/OS/OSVersion if no base image is provided, +// or if the provided image inputs (base and previous) are manifest lists. func WithDefaultPlatform(p imgutil.Platform) func(*imgutil.ImageOptions) { return func(o *imgutil.ImageOptions) { o.Platform = p } } +// WithHistory if provided will configure the image to preserve history when saved +// (including any history from the base image if valid). func WithHistory() func(*imgutil.ImageOptions) { return func(o *imgutil.ImageOptions) { o.PreserveHistory = true } } -func WithPreviousImage(name string) func(*imgutil.ImageOptions) { +// WithMediaTypes lets a caller set the desired media types for the manifest and config (including layers referenced in the manifest) +// to be either OCI media types or Docker media types. +func WithMediaTypes(m imgutil.MediaTypes) func(*imgutil.ImageOptions) { return func(o *imgutil.ImageOptions) { - o.PreviousImageRepoName = name + o.MediaTypes = m } } -func WithMediaTypes(m imgutil.MediaTypes) func(*imgutil.ImageOptions) { +// WithPreviousImage loads an existing image as the source for reusable layers. +// Use with ReuseLayer(). +// If the image is not found, it does nothing. +func WithPreviousImage(name string) func(*imgutil.ImageOptions) { return func(o *imgutil.ImageOptions) { - o.MediaTypes = m + o.PreviousImageRepoName = name } } +// WithoutLayersWhenSaved (layout only) if provided will cause the image to be written without layers in the `blobs` directory. func WithoutLayersWhenSaved() func(*imgutil.ImageOptions) { return func(o *imgutil.ImageOptions) { o.WithoutLayers = true