Try to simplify index options by putting them all on one struct

Signed-off-by: Natalie Arellano <narellano@vmware.com>

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
 	}
 

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
 }

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:

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))

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,19 +131,21 @@ 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
 		return nil
 	}
 }
 
-// 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
 }

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),
 			)