From 725d3c91420a3ca83a7095339c93107dfd521adf Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Fri, 19 Apr 2024 10:29:07 -0400 Subject: [PATCH 1/3] Override Image method instead of renaming us This will prevent us from accidentally forgetting to use the locally defined method in the future Signed-off-by: Natalie Arellano --- cnb_index.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cnb_index.go b/cnb_index.go index db8aa6e9..87297b7e 100644 --- a/cnb_index.go +++ b/cnb_index.go @@ -41,7 +41,7 @@ func (h *CNBIndex) getConfigFileFrom(digest name.Digest) (v1.ConfigFile, error) if err != nil { return v1.ConfigFile{}, err } - image, err := h.getImage(hash) + image, err := h.Image(hash) if err != nil { return v1.ConfigFile{}, err } @@ -57,7 +57,7 @@ func (h *CNBIndex) getManifestFileFrom(digest name.Digest) (v1.Manifest, error) if err != nil { return v1.Manifest{}, err } - image, err := h.getImage(hash) + image, err := h.Image(hash) if err != nil { return v1.Manifest{}, err } @@ -179,7 +179,7 @@ func (h *CNBIndex) mutateExistingImage(digest name.Digest, withFunc func(image v if err != nil { return err } - image, err := h.getImage(hash) + image, err := h.Image(hash) if err != nil { return err } @@ -194,7 +194,7 @@ func (h *CNBIndex) mutateExistingImage(digest name.Digest, withFunc func(image v return nil } -func (h *CNBIndex) getImage(hash v1.Hash) (v1.Image, error) { +func (h *CNBIndex) Image(hash v1.Hash) (v1.Image, error) { index, err := h.IndexManifest() if err != nil { return nil, err @@ -202,7 +202,7 @@ func (h *CNBIndex) getImage(hash v1.Hash) (v1.Image, error) { if !indexContains(index.Manifests, hash) { return nil, fmt.Errorf("failed to find image with digest %s in index", hash.String()) } - return h.Image(hash) + return h.ImageIndex.Image(hash) } func indexContains(manifests []v1.Descriptor, hash v1.Hash) bool { From d382bdb4ff198debabfcbfa2af19615e864c0b1b Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Fri, 19 Apr 2024 11:05:06 -0400 Subject: [PATCH 2/3] Remove insecure as it is not used Signed-off-by: Natalie Arellano --- cnb_index.go | 1 - new.go | 1 - remote/new_test.go | 1 - 3 files changed, 3 deletions(-) diff --git a/cnb_index.go b/cnb_index.go index 87297b7e..98e2dfdc 100644 --- a/cnb_index.go +++ b/cnb_index.go @@ -31,7 +31,6 @@ type CNBIndex struct { // local options XdgPath string // push options - Insecure bool KeyChain authn.Keychain RepoName string } diff --git a/new.go b/new.go index f0166395..8ddc44cc 100644 --- a/new.go +++ b/new.go @@ -295,7 +295,6 @@ func prepareNewWindowsImageIfNeeded(image *CNBImageCore) error { func NewCNBIndex(repoName string, v1Index v1.ImageIndex, ops IndexOptions) (*CNBIndex, error) { index := &CNBIndex{ ImageIndex: v1Index, - Insecure: ops.Insecure, RepoName: repoName, XdgPath: ops.XdgPath, KeyChain: ops.KeyChain, diff --git a/remote/new_test.go b/remote/new_test.go index 7424b1a7..e35915b1 100644 --- a/remote/new_test.go +++ b/remote/new_test.go @@ -52,7 +52,6 @@ func testRemoteNew(t *testing.T, when spec.G, it spec.S) { imgIx, ok := idx.(*remote.ImageIndex) h.AssertEq(t, ok, true) - h.AssertEq(t, imgIx.Insecure, true) h.AssertEq(t, imgIx.XdgPath, xdgPath) h.AssertEq(t, imgIx.RepoName, "busybox:1.36-musl") }) From 3255562438f50effb5d729a725496437272381a7 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Fri, 19 Apr 2024 11:33:50 -0400 Subject: [PATCH 3/3] Try to simplify index options by putting them all on one struct Signed-off-by: Natalie Arellano --- cnb_index.go | 16 +++---- index.go | 2 +- layout/new.go | 2 +- layout/new_test.go | 4 +- options.go | 102 +++++++++++++++++---------------------------- remote/new_test.go | 12 +++--- 6 files changed, 56 insertions(+), 82 deletions(-) diff --git a/cnb_index.go b/cnb_index.go index 98e2dfdc..41c7b3e0 100644 --- a/cnb_index.go +++ b/cnb_index.go @@ -280,24 +280,24 @@ func newEmptyLayoutPath(indexType types.MediaType, path string) (layout.Path, er // Push Publishes ImageIndex to the registry assuming every image it referes exists in registry. // // It will only push the IndexManifest to registry. -func (h *CNBIndex) Push(ops ...func(*IndexPushOptions) error) error { - var pushOps = &IndexPushOptions{} +func (h *CNBIndex) Push(ops ...func(*IndexOptions) error) error { + var pushOps = &IndexOptions{} for _, op := range ops { if err := op(pushOps); err != nil { return err } } - if pushOps.Format != "" { - if !pushOps.Format.IsIndex() { - return ErrUnknownMediaType(pushOps.Format) + if pushOps.MediaType != "" { + if !pushOps.MediaType.IsIndex() { + return ErrUnknownMediaType(pushOps.MediaType) } existingType, err := h.ImageIndex.MediaType() if err != nil { return err } - if pushOps.Format != existingType { - h.ImageIndex = mutate.IndexMediaType(h.ImageIndex, pushOps.Format) + if pushOps.MediaType != existingType { + h.ImageIndex = mutate.IndexMediaType(h.ImageIndex, pushOps.MediaType) } } @@ -319,7 +319,7 @@ func (h *CNBIndex) Push(ops ...func(*IndexPushOptions) error) error { multiWriteTagables := map[name.Reference]remote.Taggable{ ref: taggableIndex, } - for _, tag := range pushOps.Tags { + for _, tag := range pushOps.DestinationTags { multiWriteTagables[ref.Context().Tag(tag)] = taggableIndex } diff --git a/index.go b/index.go index 3d504700..8a585236 100644 --- a/index.go +++ b/index.go @@ -29,7 +29,7 @@ type ImageIndex interface { AddManifest(image v1.Image) RemoveManifest(digest name.Digest) error - Push(ops ...func(options *IndexPushOptions) error) error + Push(ops ...func(options *IndexOptions) error) error SaveDir() error DeleteDir() error } diff --git a/layout/new.go b/layout/new.go index 50362d4f..11d11ce7 100644 --- a/layout/new.go +++ b/layout/new.go @@ -105,7 +105,7 @@ func NewIndex(repoName, path string, ops ...imgutil.IndexOption) (idx *ImageInde } if idxOps.BaseIndex == nil { - switch idxOps.Format { + switch idxOps.MediaType { case types.DockerManifestList: idxOps.BaseIndex = imgutil.NewEmptyDockerIndex() default: diff --git a/layout/new_test.go b/layout/new_test.go index 1bfd54c3..453b76b8 100644 --- a/layout/new_test.go +++ b/layout/new_test.go @@ -69,7 +69,7 @@ func testLayoutNewImageIndex(t *testing.T, when spec.G, it spec.S) { idx, err = layout.NewIndex( repoName, tempDir, - imgutil.WithFormat(types.DockerManifestList), + imgutil.WithMediaType(types.DockerManifestList), ) h.AssertNil(t, err) }) @@ -87,7 +87,7 @@ func testLayoutNewImageIndex(t *testing.T, when spec.G, it spec.S) { idx, err = layout.NewIndex( failingName, tempDir, - imgutil.PullInsecure(), + imgutil.WithInsecure(), ) h.AssertNotNil(t, err) h.AssertError(t, err, fmt.Sprintf("could not parse reference: %s", failingName)) diff --git a/options.go b/options.go index 91608cb4..b3a6beeb 100644 --- a/options.go +++ b/options.go @@ -101,37 +101,28 @@ func WithPreviousImage(name string) func(*ImageOptions) { type IndexOption func(options *IndexOptions) error -type PushOption func(*IndexPushOptions) error - -type IndexPushOptions struct { - IndexFormatOptions - IndexRemoteOptions - Purge bool - Tags []string // Tags with which the index should be pushed to registry -} - -type IndexFormatOptions struct { - Format types.MediaType // The media type for the index (oci or docker) -} - -type IndexRemoteOptions struct { - Insecure bool -} - type IndexOptions struct { - XdgPath string BaseImageIndexRepoName string - KeyChain authn.Keychain - IndexFormatOptions - IndexRemoteOptions + MediaType types.MediaType + LayoutIndexOptions + RemoteIndexOptions + IndexPushOptions // These options must be specified in each implementation's image index constructor BaseIndex v1.ImageIndex } -// IndexOptions +type LayoutIndexOptions struct { + XdgPath string +} -// FromBaseImageIndex loads the ImageIndex at the provided path for the working image index. +type RemoteIndexOptions struct { + KeyChain authn.Keychain + Insecure bool +} + +// FromBaseImageIndex sets the name to use when loading the index. +// It used to either construct the path (if using layout) or the repo name (if using remote). // If the index is not found, it does nothing. func FromBaseImageIndex(name string) func(*IndexOptions) error { return func(o *IndexOptions) error { @@ -140,8 +131,7 @@ func FromBaseImageIndex(name string) func(*IndexOptions) error { } } -// FromBaseImageIndexInstance loads the provided image index for the working image index. -// If the index is not found, it does nothing. +// FromBaseImageIndexInstance sets the provided image index as the working image index. func FromBaseImageIndexInstance(index v1.ImageIndex) func(options *IndexOptions) error { return func(o *IndexOptions) error { o.BaseIndex = index @@ -149,10 +139,13 @@ func FromBaseImageIndexInstance(index v1.ImageIndex) func(options *IndexOptions) } } -// WithKeychain fetches Index from registry with keychain -func WithKeychain(keychain authn.Keychain) func(options *IndexOptions) error { +// WithMediaType specifies the media type for the image index. +func WithMediaType(mediaType types.MediaType) func(options *IndexOptions) error { return func(o *IndexOptions) error { - o.KeyChain = keychain + if !mediaType.IsIndex() { + return fmt.Errorf("unsupported media type encountered: '%s'", mediaType) + } + o.MediaType = mediaType return nil } } @@ -165,57 +158,39 @@ func WithXDGRuntimePath(xdgPath string) func(options *IndexOptions) error { } } -// PullInsecure If true, pulls images from insecure registry -func PullInsecure() func(options *IndexOptions) error { +// WithKeychain fetches Index from registry with keychain +func WithKeychain(keychain authn.Keychain) func(options *IndexOptions) error { return func(o *IndexOptions) error { - o.Insecure = true + o.KeyChain = keychain return nil } } -// WithFormat Create the image index with the following format -func WithFormat(format types.MediaType) func(options *IndexOptions) error { +// WithInsecure if true pulls and pushes the image to an insecure registry. +func WithInsecure() func(options *IndexOptions) error { return func(o *IndexOptions) error { - o.Format = format - return nil - } -} - -// IndexAddOptions - -// IndexPushOptions - -// If true, Deletes index from local filesystem after pushing to registry -func WithPurge(purge bool) func(options *IndexPushOptions) error { - return func(a *IndexPushOptions) error { - a.Purge = purge + o.Insecure = true return nil } } -// Push the Index with given format -func WithTags(tags ...string) func(options *IndexPushOptions) error { - return func(a *IndexPushOptions) error { - a.Tags = tags - return nil - } +type IndexPushOptions struct { + Purge bool + DestinationTags []string } -// Push index to Insecure Registry -func WithInsecure(insecure bool) func(options *IndexPushOptions) error { - return func(a *IndexPushOptions) error { - a.Insecure = insecure +// WithPurge if true deletes the index from the local filesystem after pushing +func WithPurge(purge bool) func(options *IndexOptions) error { + return func(a *IndexOptions) error { + a.Purge = purge return nil } } -// Push the Index with given format -func UsingFormat(format types.MediaType) func(options *IndexPushOptions) error { - return func(a *IndexPushOptions) error { - if !format.IsIndex() { - return fmt.Errorf("unsupported media type encountered in image: '%s'", format) - } - a.Format = format +// WithTags sets the destination tags for the index when pushed +func WithTags(tags ...string) func(options *IndexOptions) error { + return func(a *IndexOptions) error { + a.DestinationTags = tags return nil } } @@ -228,6 +203,5 @@ func GetTransport(insecure bool) http.RoundTripper { }, } } - return http.DefaultTransport } diff --git a/remote/new_test.go b/remote/new_test.go index e35915b1..50fc2de9 100644 --- a/remote/new_test.go +++ b/remote/new_test.go @@ -44,7 +44,7 @@ func testRemoteNew(t *testing.T, when spec.G, it spec.S) { it("should have expected indexOptions", func() { idx, err = remote.NewIndex( "busybox:1.36-musl", - imgutil.PullInsecure(), + imgutil.WithInsecure(), imgutil.WithKeychain(authn.DefaultKeychain), imgutil.WithXDGRuntimePath(xdgPath), ) @@ -58,7 +58,7 @@ func testRemoteNew(t *testing.T, when spec.G, it spec.S) { it("should return an error when invalid repoName is passed", func() { _, err = remote.NewIndex( "some/invalidImage", - imgutil.PullInsecure(), + imgutil.WithInsecure(), imgutil.WithKeychain(authn.DefaultKeychain), imgutil.WithXDGRuntimePath(xdgPath), ) @@ -67,7 +67,7 @@ func testRemoteNew(t *testing.T, when spec.G, it spec.S) { it("should return an error when index with the given repoName doesn't exists", func() { _, err = remote.NewIndex( "some/image", - imgutil.PullInsecure(), + imgutil.WithInsecure(), imgutil.WithKeychain(authn.DefaultKeychain), imgutil.WithXDGRuntimePath(xdgPath), ) @@ -76,7 +76,7 @@ func testRemoteNew(t *testing.T, when spec.G, it spec.S) { it("should return ImageIndex with expected output", func() { idx, err = remote.NewIndex( "busybox:1.36-musl", - imgutil.PullInsecure(), + imgutil.WithInsecure(), imgutil.WithKeychain(authn.DefaultKeychain), imgutil.WithXDGRuntimePath(xdgPath), ) @@ -93,7 +93,7 @@ func testRemoteNew(t *testing.T, when spec.G, it spec.S) { it("should able to call #ImageIndex", func() { idx, err = remote.NewIndex( "busybox:1.36-musl", - imgutil.PullInsecure(), + imgutil.WithInsecure(), imgutil.WithKeychain(authn.DefaultKeychain), imgutil.WithXDGRuntimePath(xdgPath), ) @@ -114,7 +114,7 @@ func testRemoteNew(t *testing.T, when spec.G, it spec.S) { it("should able to call #Image", func() { idx, err = remote.NewIndex( "busybox:1.36-musl", - imgutil.PullInsecure(), + imgutil.WithInsecure(), imgutil.WithKeychain(authn.DefaultKeychain), imgutil.WithXDGRuntimePath(xdgPath), )