From 62a99050e15f9053082dceb380ac158fdd3f0e06 Mon Sep 17 00:00:00 2001 From: Joe Searcy Date: Sun, 6 Aug 2023 18:06:29 -0400 Subject: [PATCH 01/12] Add flag to handle additional tags Signed-off-by: Joe Searcy --- pkg/imgpkg/cmd/tag_flags.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 pkg/imgpkg/cmd/tag_flags.go diff --git a/pkg/imgpkg/cmd/tag_flags.go b/pkg/imgpkg/cmd/tag_flags.go new file mode 100644 index 00000000..9c33a108 --- /dev/null +++ b/pkg/imgpkg/cmd/tag_flags.go @@ -0,0 +1,18 @@ +// Copyright 2020 VMware, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package cmd + +import ( + "github.com/spf13/cobra" +) + +// TagFlags is a struct that holds the additional tags for an OCI artifact +type TagFlags struct { + Tags []string +} + +// Set sets additional tags for an OCI artifact +func (t *TagFlags) Set(cmd *cobra.Command) { + cmd.Flags().StringSliceVar(&t.Tags, "additional-tags", []string{}, "Set additional tags on image") +} From 41cfb536044e82723795fec4eac0a998c03d2d11 Mon Sep 17 00:00:00 2001 From: Joe Searcy Date: Sun, 6 Aug 2023 18:07:08 -0400 Subject: [PATCH 02/12] Add logic to handle additional tags Signed-off-by: Joe Searcy --- pkg/imgpkg/cmd/push.go | 144 +++++++++++++++++++++++++++++++++-------- 1 file changed, 118 insertions(+), 26 deletions(-) diff --git a/pkg/imgpkg/cmd/push.go b/pkg/imgpkg/cmd/push.go index 7ad0c0d3..13b975f0 100644 --- a/pkg/imgpkg/cmd/push.go +++ b/pkg/imgpkg/cmd/push.go @@ -5,6 +5,7 @@ package cmd import ( "fmt" + "strings" "github.com/cppforlife/go-cli-ui/ui" regname "github.com/google/go-containerregistry/pkg/name" @@ -25,6 +26,7 @@ type PushOptions struct { FileFlags FileFlags RegistryFlags RegistryFlags LabelFlags LabelFlags + TagFlags TagFlags } func NewPushOptions(ui ui.UI) *PushOptions { @@ -49,6 +51,7 @@ func NewPushCmd(o *PushOptions) *cobra.Command { o.FileFlags.Set(cmd) o.RegistryFlags.Set(cmd) o.LabelFlags.Set(cmd) + o.TagFlags.Set(cmd) return cmd } @@ -92,54 +95,69 @@ func (po *PushOptions) Run() error { panic("Unreachable code") } - po.ui.BeginLinef("Pushed '%s'", imageURL) + po.ui.BeginLinef("Pushed: \n%s\n", imageURL) return nil } func (po *PushOptions) pushBundle(registry registry.Registry) (string, error) { - uploadRef, err := regname.NewTag(po.BundleFlags.Bundle, regname.WeakValidation) - if err != nil { - return "", fmt.Errorf("Parsing '%s': %s", po.BundleFlags.Bundle, err) - } - logger := util.NewUILevelLogger(util.LogWarn, util.NewLogger(po.ui)) - imageURL, err := bundle.NewContents(po.FileFlags.Files, po.FileFlags.ExcludedFilePaths, po.FileFlags.PreservePermissions).Push(uploadRef, po.LabelFlags.Labels, registry, logger) + imageRefs := []string{} + + baseBundleName, err := po.stripTag() if err != nil { return "", err } - if po.LockOutputFlags.LockFilePath != "" { - bundleLock := lockconfig.BundleLock{ - LockVersion: lockconfig.LockVersion{ - APIVersion: lockconfig.BundleLockAPIVersion, - Kind: lockconfig.BundleLockKind, - }, - Bundle: lockconfig.BundleRef{ - Image: imageURL, - Tag: uploadRef.TagStr(), - }, + // Loop through all tags specified by the user and push the related bundle+tag + for _, tag := range po.TagFlags.Tags { + uploadRef, err := regname.NewTag(baseBundleName+":"+tag, regname.WeakValidation) + if err != nil { + return "", fmt.Errorf("Parsing '%s': %s", tag, err) } - err := bundleLock.WriteToPath(po.LockOutputFlags.LockFilePath) + logger := util.NewUILevelLogger(util.LogWarn, util.NewLogger(po.ui)) + imageURL, err := bundle.NewContents(po.FileFlags.Files, po.FileFlags.ExcludedFilePaths, po.FileFlags.PreservePermissions).Push(uploadRef, po.LabelFlags.Labels, registry, logger) if err != nil { return "", err } + + if po.LockOutputFlags.LockFilePath != "" { + bundleLock := lockconfig.BundleLock{ + LockVersion: lockconfig.LockVersion{ + APIVersion: lockconfig.BundleLockAPIVersion, + Kind: lockconfig.BundleLockKind, + }, + Bundle: lockconfig.BundleRef{ + Image: imageURL, + Tag: uploadRef.TagStr(), + }, + } + + err := bundleLock.WriteToPath(po.LockOutputFlags.LockFilePath) + if err != nil { + return "", err + } + } + + if !strings.Contains(strings.Join(imageRefs, ","), imageURL) { + imageRefs = append(imageRefs, imageURL) + } } - return imageURL, nil + po.ui.BeginLinef("Tags: %s\n", strings.Join(po.TagFlags.Tags, ", ")) + + return strings.Join(imageRefs, "\n"), nil } func (po *PushOptions) pushImage(registry registry.Registry) (string, error) { + + imageRefs := []string{} + if po.LockOutputFlags.LockFilePath != "" { return "", fmt.Errorf("Lock output is not compatible with image, use bundle for lock output") } - uploadRef, err := regname.NewTag(po.ImageFlags.Image, regname.WeakValidation) - if err != nil { - return "", fmt.Errorf("Parsing '%s': %s", po.ImageFlags.Image, err) - } - isBundle, err := bundle.NewContents(po.FileFlags.Files, po.FileFlags.ExcludedFilePaths, po.FileFlags.PreservePermissions).PresentsAsBundle() if err != nil { return "", err @@ -148,8 +166,33 @@ func (po *PushOptions) pushImage(registry registry.Registry) (string, error) { return "", fmt.Errorf("Images cannot be pushed with '.imgpkg' directories, consider using --bundle (-b) option") } - logger := util.NewUILevelLogger(util.LogWarn, util.NewLogger(po.ui)) - return plainimage.NewContents(po.FileFlags.Files, po.FileFlags.ExcludedFilePaths, po.FileFlags.PreservePermissions).Push(uploadRef, po.LabelFlags.Labels, registry, logger) + baseImageName, err := po.stripTag() + if err != nil { + return "", err + } + + // Loop through all tags specified by the user and push the related image+tag + for _, tag := range po.TagFlags.Tags { + + uploadRef, err := regname.NewTag(baseImageName+":"+tag, regname.WeakValidation) + if err != nil { + return "", fmt.Errorf("Parsing '%s': %s", tag, err) + } + + logger := util.NewUILevelLogger(util.LogWarn, util.NewLogger(po.ui)) + imageURL, err := plainimage.NewContents(po.FileFlags.Files, po.FileFlags.ExcludedFilePaths, po.FileFlags.PreservePermissions).Push(uploadRef, po.LabelFlags.Labels, registry, logger) + if err != nil { + return "", err + } + + if !strings.Contains(strings.Join(imageRefs, ","), imageURL) { + imageRefs = append(imageRefs, imageURL) + } + } + + po.ui.BeginLinef("Tags: %s\n", strings.Join(po.TagFlags.Tags, ", ")) + + return strings.Join(imageRefs, "\n"), nil } // validateFlags checks if the provided flags are valid @@ -165,3 +208,52 @@ func (po *PushOptions) validateFlags() error { return nil } + +func (po *PushOptions) stripTag() (string, error) { + + object := "" + isBundle := po.BundleFlags.Bundle != "" + isImage := po.ImageFlags.Image != "" + + switch { + + case isBundle: + object = po.BundleFlags.Bundle + + case isImage: + object = po.ImageFlags.Image + + default: + panic("Unreachable code") + } + + objectRef, err := regname.NewTag(object, regname.WeakValidation) + if err != nil { + fmt.Println("FAILING BEFORE TAG STRIP") + fmt.Printf("TEST - BUNDLE NAME: %s\n", po.BundleFlags.Bundle) + fmt.Printf("TEST - REGISTRY: %s\n", objectRef.RegistryStr()) + fmt.Printf("TEST - REPOSITORY: %s\n", objectRef.RepositoryStr()) + fmt.Printf("TEST - NAME: %s\n", objectRef.Name()) + fmt.Printf("TEST - TAG: %s\n", objectRef.TagStr()) + return "", fmt.Errorf("Parsing '%s': %s", object, err) + } + + embeddedTag := objectRef.TagStr() + + fmt.Printf("TEST - REGISTRY: %s\n", objectRef.RegistryStr()) + fmt.Printf("TEST - REPOSITORY: %s\n", objectRef.RepositoryStr()) + fmt.Printf("TEST - NAME: %s\n", objectRef.Name()) + fmt.Printf("TEST - TAG: %s\n", objectRef.TagStr()) + + if embeddedTag != "" { + po.TagFlags.Tags = append(po.TagFlags.Tags, embeddedTag) + } + + baseObjectName := strings.TrimSuffix(objectRef.Name(), ":"+objectRef.TagStr()) + + if baseObjectName == "" { + return "", fmt.Errorf("'%s' is not a valid image reference", object) + } + + return baseObjectName, nil +} From 6b4a4724af9e7801f7c6b74d6346ff369f5fef0c Mon Sep 17 00:00:00 2001 From: Joe Searcy Date: Sun, 6 Aug 2023 18:07:28 -0400 Subject: [PATCH 03/12] Add tests to cover additional tags logic Signed-off-by: Joe Searcy --- pkg/imgpkg/cmd/push_test.go | 104 ++++++++++++++++++++++++++++++++++++ 1 file changed, 104 insertions(+) diff --git a/pkg/imgpkg/cmd/push_test.go b/pkg/imgpkg/cmd/push_test.go index df743b8c..0899eda1 100644 --- a/pkg/imgpkg/cmd/push_test.go +++ b/pkg/imgpkg/cmd/push_test.go @@ -326,6 +326,110 @@ func TestLabels(t *testing.T) { } } +func TestTags(t *testing.T) { + testCases := []struct { + name string + opType string + expectedError string + expectedTags []string + tagInput string + inlineTag string + }{ + { + name: "bundle with one inline tag", + opType: "bundle", + expectedError: "", + tagInput: "", + expectedTags: []string{"v1.0.1"}, + inlineTag: "v1.0.1", + }, + { + name: "bundle with one tag via flag", + opType: "bundle", + expectedError: "", + tagInput: "v1.0.1", + expectedTags: []string{"v1.0.1", "latest"}, + inlineTag: "", + }, + { + name: "bundle with inline tag and tag via flag", + opType: "bundle", + expectedError: "", + tagInput: "v1.0.2", + expectedTags: []string{"v1.0.1", "v1.0.2"}, + inlineTag: "v1.0.1", + }, + { + name: "bundle with multiple tags via flag", + opType: "bundle", + expectedError: "", + tagInput: "v1.0.1,v1.0.2", + expectedTags: []string{"v1.0.1", "v1.0.2", "latest"}, + inlineTag: "", + }, + { + name: "image with one inline tag", + opType: "image", + expectedError: "", + tagInput: "", + expectedTags: []string{"v1.0.1"}, + inlineTag: "v1.0.1", + }, + { + name: "image with one tag via flag", + opType: "image", + expectedError: "", + tagInput: "v1.0.1", + expectedTags: []string{"v1.0.1", "latest"}, + inlineTag: "", + }, + } + + for _, tc := range testCases { + f := func(t *testing.T) { + env := helpers.BuildEnv(t) + targetImage := env.Image + imgpkg := helpers.Imgpkg{T: t, ImgpkgPath: env.ImgpkgPath} + defer env.Cleanup() + + opTypeFlag := "-b" + pushDir := env.BundleFactory.CreateBundleDir(helpers.BundleYAML, helpers.ImagesYAML) + + if tc.opType == "image" { + opTypeFlag = "-i" + pushDir = env.Assets.CreateAndCopySimpleApp("image-to-push") + } + + if tc.inlineTag != "" { + targetImage = env.Image + ":" + tc.inlineTag + } + + if tc.tagInput == "" { + imgpkg.Run([]string{"push", opTypeFlag, targetImage, "-f", pushDir}) + } else { + imgpkg.Run([]string{"push", opTypeFlag, targetImage, "--additional-tags", tc.tagInput, "-f", pushDir}) + } + + // Loop through expected tags and validate they exist on the image + for _, tag := range tc.expectedTags { + ref, _ := name.NewTag(env.Image+":"+tag, name.WeakValidation) + image, err := remote.Image(ref, remote.WithAuthFromKeychain(authn.DefaultKeychain)) + require.NoError(t, err) + + tagList := imgpkg.Run([]string{"tag", "ls", "-i", env.Image + ":" + tag}) + + _, err = image.ConfigFile() + require.NoError(t, err) + + require.Contains(t, tagList, tag, "Expected tags provided via flags to match tags discovered for image") + + } + } + + t.Run(tc.name, f) + } +} + func Cleanup(dirs ...string) { for _, dir := range dirs { os.RemoveAll(dir) From db21b84605871e06b3a228471aab308061f3e55e Mon Sep 17 00:00:00 2001 From: Joe Searcy Date: Tue, 12 Sep 2023 21:24:15 -0400 Subject: [PATCH 04/12] Add MultiTagPush() method for Image contents Signed-off-by: Joe Searcy --- pkg/imgpkg/plainimage/contents.go | 50 +++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/pkg/imgpkg/plainimage/contents.go b/pkg/imgpkg/plainimage/contents.go index fa767f22..f44e09b4 100644 --- a/pkg/imgpkg/plainimage/contents.go +++ b/pkg/imgpkg/plainimage/contents.go @@ -74,6 +74,56 @@ func (i Contents) Push(uploadRef regname.Tag, labels map[string]string, writer I return fmt.Sprintf("%s@%s", uploadRef.Context(), digest), nil } +// MultiTagPush pushes the OCI Image to the registry with multiple tags +func (i Contents) MultiTagPush(uploadRefs []regname.Tag, labels map[string]string, writer ImagesWriter, logger Logger) (string, error) { + + var digest regv1.Hash + var primaryUploadRef regname.Tag + + err := i.validate() + if err != nil { + return "", err + } + + tarImg := ctlimg.NewTarImage(i.paths, i.excludedPaths, logger, i.preservePermissions) + primaryUploadRef = uploadRefs[0] + + for _, uploadRef := range uploadRefs { + + img, err := tarImg.AsFileImage(labels) + if err != nil { + return "", err + } + + defer img.Remove() + + err = writer.WriteImage(uploadRef, img, nil) + + if err != nil { + return "", fmt.Errorf("Writing '%s': %s", uploadRef.Name(), err) + } + + digest, err = img.Digest() + if err != nil { + return "", err + } + + uploadTagRef, err := util.BuildDefaultUploadTagRef(img, uploadRef.Repository) + if err != nil { + return "", fmt.Errorf("Building default upload tag image ref: %s", err) + } + + err = writer.WriteTag(uploadTagRef, img) + if err != nil { + return "", fmt.Errorf("Writing Tag '%s': %s", uploadRef.Name(), err) + } + + } + + return fmt.Sprintf("%s@%s", primaryUploadRef.Context(), digest), nil + +} + func (i Contents) validate() error { return i.checkRepeatedPaths() } From 3775cc1a271b33fa8922247992bb16a95d945002 Mon Sep 17 00:00:00 2001 From: Joe Searcy Date: Tue, 12 Sep 2023 21:24:59 -0400 Subject: [PATCH 05/12] Add MultiTagPush() method for Bundle contents Signed-off-by: Joe Searcy --- pkg/imgpkg/bundle/contents.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/pkg/imgpkg/bundle/contents.go b/pkg/imgpkg/bundle/contents.go index 90b9a311..d89e14e0 100644 --- a/pkg/imgpkg/bundle/contents.go +++ b/pkg/imgpkg/bundle/contents.go @@ -57,6 +57,22 @@ func (b Contents) Push(uploadRef regname.Tag, labels map[string]string, registry return plainimage.NewContents(b.paths, b.excludedPaths, b.preservePermissions).Push(uploadRef, labels, registry, logger) } +// MultiTagPush pushes the OCI Bundle to the registry with multiple tags +func (b Contents) MultiTagPush(uploadRefs []regname.Tag, labels map[string]string, registry ImagesMetadataWriter, logger Logger) (string, error) { + + err := b.validate() + if err != nil { + return "", err + } + + if labels == nil { + labels = map[string]string{} + } + labels[BundleConfigLabel] = "true" + + return plainimage.NewContents(b.paths, b.excludedPaths, b.preservePermissions).MultiTagPush(uploadRefs, labels, registry, logger) +} + // PresentsAsBundle checks if the provided folders have the needed structure to be a bundle func (b Contents) PresentsAsBundle() (bool, error) { imgpkgDirs, err := b.findImgpkgDirs() From 77c163fcbe2e939aa703ac6c0d970e0aaa2f15d7 Mon Sep 17 00:00:00 2001 From: Joe Searcy Date: Tue, 12 Sep 2023 21:27:00 -0400 Subject: [PATCH 06/12] Add OtherTags field to BundleRef/BundleLock API Signed-off-by: Joe Searcy --- pkg/imgpkg/lockconfig/bundle_lock.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/imgpkg/lockconfig/bundle_lock.go b/pkg/imgpkg/lockconfig/bundle_lock.go index 39b39317..081c3cf9 100644 --- a/pkg/imgpkg/lockconfig/bundle_lock.go +++ b/pkg/imgpkg/lockconfig/bundle_lock.go @@ -22,8 +22,9 @@ type BundleLock struct { } type BundleRef struct { - Image string `json:"image,omitempty"` // This generated yaml, but due to lib we need to use `json` - Tag string `json:"tag,omitempty"` // This generated yaml, but due to lib we need to use `json` + Image string `json:"image,omitempty"` // This generated yaml, but due to lib we need to use `json` + Tag string `json:"tag,omitempty"` // This generated yaml, but due to lib we need to use `json` + OtherTags string `json:"otherTags,omitempty"` } func NewBundleLockFromPath(path string) (BundleLock, error) { From 04aea19052b5e4989bee55132e512eeed8a773b1 Mon Sep 17 00:00:00 2001 From: Joe Searcy Date: Tue, 12 Sep 2023 21:27:35 -0400 Subject: [PATCH 07/12] Updates to support MultiTag Image/Bundle push Signed-off-by: Joe Searcy --- pkg/imgpkg/cmd/push.go | 129 ++++++++++++++++++++++++++--------------- 1 file changed, 81 insertions(+), 48 deletions(-) diff --git a/pkg/imgpkg/cmd/push.go b/pkg/imgpkg/cmd/push.go index 13b975f0..f7328f78 100644 --- a/pkg/imgpkg/cmd/push.go +++ b/pkg/imgpkg/cmd/push.go @@ -95,64 +95,95 @@ func (po *PushOptions) Run() error { panic("Unreachable code") } - po.ui.BeginLinef("Pushed: \n%s\n", imageURL) + po.ui.BeginLinef("\nPushed: \n%s\n", imageURL) return nil } func (po *PushOptions) pushBundle(registry registry.Registry) (string, error) { + imageURL := "" imageRefs := []string{} + uploadRefs := []regname.Tag{} - baseBundleName, err := po.stripTag() + baseImageName, err := po.stripTag() if err != nil { return "", err } - // Loop through all tags specified by the user and push the related bundle+tag + baseRef, err := regname.NewTag(po.BundleFlags.Bundle, regname.WeakValidation) + if err != nil { + return "", fmt.Errorf("Parsing '%s': %s", po.BundleFlags.Bundle, err) + } + + // Append the base image_tag to the list of refs to upload + uploadRefs = append(uploadRefs, baseRef) + + // TODO(phenixblue): Cleanup when done testing + fmt.Printf("\nAdding base ref to list of tags: %s\n", baseRef) + + // Loop through all tags specified by the user and push the related image+tag for _, tag := range po.TagFlags.Tags { - uploadRef, err := regname.NewTag(baseBundleName+":"+tag, regname.WeakValidation) + + uploadRef, err := regname.NewTag(baseImageName+":"+tag, regname.WeakValidation) if err != nil { return "", fmt.Errorf("Parsing '%s': %s", tag, err) } - logger := util.NewUILevelLogger(util.LogWarn, util.NewLogger(po.ui)) - imageURL, err := bundle.NewContents(po.FileFlags.Files, po.FileFlags.ExcludedFilePaths, po.FileFlags.PreservePermissions).Push(uploadRef, po.LabelFlags.Labels, registry, logger) + uploadRefs = append(uploadRefs, uploadRef) + // TODO(phenixblue): Cleanup when done testing + fmt.Printf("\nAdding non-base ref to list of tags: %s\n", uploadRef) + + } + + logger := util.NewUILevelLogger(util.LogWarn, util.NewLogger(po.ui)) + + if len(po.TagFlags.Tags) > 1 { + imageURL, err = bundle.NewContents(po.FileFlags.Files, po.FileFlags.ExcludedFilePaths, po.FileFlags.PreservePermissions).MultiTagPush(uploadRefs, po.LabelFlags.Labels, registry, logger) + if err != nil { + return "", err + } + + } else { + imageURL, err = bundle.NewContents(po.FileFlags.Files, po.FileFlags.ExcludedFilePaths, po.FileFlags.PreservePermissions).Push(uploadRefs[0], po.LabelFlags.Labels, registry, logger) if err != nil { return "", err } + } - if po.LockOutputFlags.LockFilePath != "" { - bundleLock := lockconfig.BundleLock{ - LockVersion: lockconfig.LockVersion{ - APIVersion: lockconfig.BundleLockAPIVersion, - Kind: lockconfig.BundleLockKind, - }, - Bundle: lockconfig.BundleRef{ - Image: imageURL, - Tag: uploadRef.TagStr(), - }, - } - - err := bundleLock.WriteToPath(po.LockOutputFlags.LockFilePath) - if err != nil { - return "", err - } + if po.LockOutputFlags.LockFilePath != "" { + bundleLock := lockconfig.BundleLock{ + LockVersion: lockconfig.LockVersion{ + APIVersion: lockconfig.BundleLockAPIVersion, + Kind: lockconfig.BundleLockKind, + }, + Bundle: lockconfig.BundleRef{ + Image: imageURL, + Tag: uploadRefs[0].TagStr(), + OtherTags: strings.Join(po.TagFlags.Tags, ","), + }, } - if !strings.Contains(strings.Join(imageRefs, ","), imageURL) { - imageRefs = append(imageRefs, imageURL) + err := bundleLock.WriteToPath(po.LockOutputFlags.LockFilePath) + if err != nil { + return "", err } } - po.ui.BeginLinef("Tags: %s\n", strings.Join(po.TagFlags.Tags, ", ")) + if !strings.Contains(strings.Join(imageRefs, ","), imageURL) { + imageRefs = append(imageRefs, imageURL) + } + + po.ui.BeginLinef("\nTags: %s, %s\n", baseRef.TagStr(), strings.Join(po.TagFlags.Tags, ", ")) return strings.Join(imageRefs, "\n"), nil } func (po *PushOptions) pushImage(registry registry.Registry) (string, error) { + imageURL := "" imageRefs := []string{} + uploadRefs := []regname.Tag{} if po.LockOutputFlags.LockFilePath != "" { return "", fmt.Errorf("Lock output is not compatible with image, use bundle for lock output") @@ -171,6 +202,14 @@ func (po *PushOptions) pushImage(registry registry.Registry) (string, error) { return "", err } + baseRef, err := regname.NewTag(po.ImageFlags.Image, regname.WeakValidation) + if err != nil { + return "", fmt.Errorf("Parsing '%s': %s", po.BundleFlags.Bundle, err) + } + + // Append the base image_tag to the list of refs to upload + uploadRefs = append(uploadRefs, baseRef) + // Loop through all tags specified by the user and push the related image+tag for _, tag := range po.TagFlags.Tags { @@ -179,18 +218,29 @@ func (po *PushOptions) pushImage(registry registry.Registry) (string, error) { return "", fmt.Errorf("Parsing '%s': %s", tag, err) } - logger := util.NewUILevelLogger(util.LogWarn, util.NewLogger(po.ui)) - imageURL, err := plainimage.NewContents(po.FileFlags.Files, po.FileFlags.ExcludedFilePaths, po.FileFlags.PreservePermissions).Push(uploadRef, po.LabelFlags.Labels, registry, logger) + uploadRefs = append(uploadRefs, uploadRef) + + } + + logger := util.NewUILevelLogger(util.LogWarn, util.NewLogger(po.ui)) + + if len(po.TagFlags.Tags) > 1 { + imageURL, err = plainimage.NewContents(po.FileFlags.Files, po.FileFlags.ExcludedFilePaths, po.FileFlags.PreservePermissions).MultiTagPush(uploadRefs, po.LabelFlags.Labels, registry, logger) if err != nil { return "", err } - - if !strings.Contains(strings.Join(imageRefs, ","), imageURL) { - imageRefs = append(imageRefs, imageURL) + } else { + imageURL, err = plainimage.NewContents(po.FileFlags.Files, po.FileFlags.ExcludedFilePaths, po.FileFlags.PreservePermissions).Push(uploadRefs[0], po.LabelFlags.Labels, registry, logger) + if err != nil { + return "", err } } - po.ui.BeginLinef("Tags: %s\n", strings.Join(po.TagFlags.Tags, ", ")) + if !strings.Contains(strings.Join(imageRefs, ","), imageURL) { + imageRefs = append(imageRefs, imageURL) + } + + po.ui.BeginLinef("\nTags: %s, %s\n", baseRef.TagStr(), strings.Join(po.TagFlags.Tags, ", ")) return strings.Join(imageRefs, "\n"), nil } @@ -229,26 +279,9 @@ func (po *PushOptions) stripTag() (string, error) { objectRef, err := regname.NewTag(object, regname.WeakValidation) if err != nil { - fmt.Println("FAILING BEFORE TAG STRIP") - fmt.Printf("TEST - BUNDLE NAME: %s\n", po.BundleFlags.Bundle) - fmt.Printf("TEST - REGISTRY: %s\n", objectRef.RegistryStr()) - fmt.Printf("TEST - REPOSITORY: %s\n", objectRef.RepositoryStr()) - fmt.Printf("TEST - NAME: %s\n", objectRef.Name()) - fmt.Printf("TEST - TAG: %s\n", objectRef.TagStr()) return "", fmt.Errorf("Parsing '%s': %s", object, err) } - embeddedTag := objectRef.TagStr() - - fmt.Printf("TEST - REGISTRY: %s\n", objectRef.RegistryStr()) - fmt.Printf("TEST - REPOSITORY: %s\n", objectRef.RepositoryStr()) - fmt.Printf("TEST - NAME: %s\n", objectRef.Name()) - fmt.Printf("TEST - TAG: %s\n", objectRef.TagStr()) - - if embeddedTag != "" { - po.TagFlags.Tags = append(po.TagFlags.Tags, embeddedTag) - } - baseObjectName := strings.TrimSuffix(objectRef.Name(), ":"+objectRef.TagStr()) if baseObjectName == "" { From b864d5939f98f4d82c5e35ddbfbe49a4e385f435 Mon Sep 17 00:00:00 2001 From: Joe Searcy Date: Tue, 12 Sep 2023 21:28:16 -0400 Subject: [PATCH 08/12] Update multi-tag push tests Signed-off-by: Joe Searcy --- pkg/imgpkg/cmd/push_test.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/pkg/imgpkg/cmd/push_test.go b/pkg/imgpkg/cmd/push_test.go index 0899eda1..fa51011c 100644 --- a/pkg/imgpkg/cmd/push_test.go +++ b/pkg/imgpkg/cmd/push_test.go @@ -355,8 +355,8 @@ func TestTags(t *testing.T) { name: "bundle with inline tag and tag via flag", opType: "bundle", expectedError: "", - tagInput: "v1.0.2", - expectedTags: []string{"v1.0.1", "v1.0.2"}, + tagInput: "v1.2.0-alpha,latest", + expectedTags: []string{"v1.0.1", "v1.2.0-alpha", "latest"}, inlineTag: "v1.0.1", }, { @@ -383,6 +383,14 @@ func TestTags(t *testing.T) { expectedTags: []string{"v1.0.1", "latest"}, inlineTag: "", }, + { + name: "image with inline tag and tags via flag", + opType: "image", + expectedError: "", + tagInput: "latest,stable", + expectedTags: []string{"v1.0.1", "latest"}, + inlineTag: "v1.0.1", + }, } for _, tc := range testCases { From e416d943d9b0a6aa074d1f158addf73d2e6fa12f Mon Sep 17 00:00:00 2001 From: Joe Searcy Date: Wed, 13 Sep 2023 17:03:19 -0400 Subject: [PATCH 09/12] Cleanup errant newlines and remove testing comments Signed-off-by: Joe Searcy --- pkg/imgpkg/bundle/contents.go | 1 - pkg/imgpkg/cmd/push.go | 13 ------------- 2 files changed, 14 deletions(-) diff --git a/pkg/imgpkg/bundle/contents.go b/pkg/imgpkg/bundle/contents.go index d89e14e0..9e9fa50d 100644 --- a/pkg/imgpkg/bundle/contents.go +++ b/pkg/imgpkg/bundle/contents.go @@ -59,7 +59,6 @@ func (b Contents) Push(uploadRef regname.Tag, labels map[string]string, registry // MultiTagPush pushes the OCI Bundle to the registry with multiple tags func (b Contents) MultiTagPush(uploadRefs []regname.Tag, labels map[string]string, registry ImagesMetadataWriter, logger Logger) (string, error) { - err := b.validate() if err != nil { return "", err diff --git a/pkg/imgpkg/cmd/push.go b/pkg/imgpkg/cmd/push.go index f7328f78..bb95e517 100644 --- a/pkg/imgpkg/cmd/push.go +++ b/pkg/imgpkg/cmd/push.go @@ -101,7 +101,6 @@ func (po *PushOptions) Run() error { } func (po *PushOptions) pushBundle(registry registry.Registry) (string, error) { - imageURL := "" imageRefs := []string{} uploadRefs := []regname.Tag{} @@ -119,21 +118,14 @@ func (po *PushOptions) pushBundle(registry registry.Registry) (string, error) { // Append the base image_tag to the list of refs to upload uploadRefs = append(uploadRefs, baseRef) - // TODO(phenixblue): Cleanup when done testing - fmt.Printf("\nAdding base ref to list of tags: %s\n", baseRef) - // Loop through all tags specified by the user and push the related image+tag for _, tag := range po.TagFlags.Tags { - uploadRef, err := regname.NewTag(baseImageName+":"+tag, regname.WeakValidation) if err != nil { return "", fmt.Errorf("Parsing '%s': %s", tag, err) } uploadRefs = append(uploadRefs, uploadRef) - // TODO(phenixblue): Cleanup when done testing - fmt.Printf("\nAdding non-base ref to list of tags: %s\n", uploadRef) - } logger := util.NewUILevelLogger(util.LogWarn, util.NewLogger(po.ui)) @@ -180,7 +172,6 @@ func (po *PushOptions) pushBundle(registry registry.Registry) (string, error) { } func (po *PushOptions) pushImage(registry registry.Registry) (string, error) { - imageURL := "" imageRefs := []string{} uploadRefs := []regname.Tag{} @@ -212,14 +203,12 @@ func (po *PushOptions) pushImage(registry registry.Registry) (string, error) { // Loop through all tags specified by the user and push the related image+tag for _, tag := range po.TagFlags.Tags { - uploadRef, err := regname.NewTag(baseImageName+":"+tag, regname.WeakValidation) if err != nil { return "", fmt.Errorf("Parsing '%s': %s", tag, err) } uploadRefs = append(uploadRefs, uploadRef) - } logger := util.NewUILevelLogger(util.LogWarn, util.NewLogger(po.ui)) @@ -260,13 +249,11 @@ func (po *PushOptions) validateFlags() error { } func (po *PushOptions) stripTag() (string, error) { - object := "" isBundle := po.BundleFlags.Bundle != "" isImage := po.ImageFlags.Image != "" switch { - case isBundle: object = po.BundleFlags.Bundle From bb2a783900980517b0eb3c6cdee5850801b281bb Mon Sep 17 00:00:00 2001 From: Joe Searcy Date: Wed, 13 Sep 2023 17:08:00 -0400 Subject: [PATCH 10/12] Add comment to stripTag() method Signed-off-by: Joe Searcy --- pkg/imgpkg/cmd/push.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/imgpkg/cmd/push.go b/pkg/imgpkg/cmd/push.go index bb95e517..8e2d62bf 100644 --- a/pkg/imgpkg/cmd/push.go +++ b/pkg/imgpkg/cmd/push.go @@ -248,6 +248,7 @@ func (po *PushOptions) validateFlags() error { } +// stripTag removes the tag from the provided image or bundle reference func (po *PushOptions) stripTag() (string, error) { object := "" isBundle := po.BundleFlags.Bundle != "" From 67eac8b2f67ee73a0045889d18915c8cc0cfabce Mon Sep 17 00:00:00 2001 From: Joe Searcy Date: Wed, 13 Sep 2023 17:11:37 -0400 Subject: [PATCH 11/12] Address comments from PR review Signed-off-by: Joe Searcy --- pkg/imgpkg/cmd/push.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pkg/imgpkg/cmd/push.go b/pkg/imgpkg/cmd/push.go index 8e2d62bf..cf530ff3 100644 --- a/pkg/imgpkg/cmd/push.go +++ b/pkg/imgpkg/cmd/push.go @@ -103,7 +103,6 @@ func (po *PushOptions) Run() error { func (po *PushOptions) pushBundle(registry registry.Registry) (string, error) { imageURL := "" imageRefs := []string{} - uploadRefs := []regname.Tag{} baseImageName, err := po.stripTag() if err != nil { @@ -116,7 +115,7 @@ func (po *PushOptions) pushBundle(registry registry.Registry) (string, error) { } // Append the base image_tag to the list of refs to upload - uploadRefs = append(uploadRefs, baseRef) + uploadRefs := []regname.Tag{baseRef} // Loop through all tags specified by the user and push the related image+tag for _, tag := range po.TagFlags.Tags { @@ -174,7 +173,6 @@ func (po *PushOptions) pushBundle(registry registry.Registry) (string, error) { func (po *PushOptions) pushImage(registry registry.Registry) (string, error) { imageURL := "" imageRefs := []string{} - uploadRefs := []regname.Tag{} if po.LockOutputFlags.LockFilePath != "" { return "", fmt.Errorf("Lock output is not compatible with image, use bundle for lock output") @@ -199,7 +197,7 @@ func (po *PushOptions) pushImage(registry registry.Registry) (string, error) { } // Append the base image_tag to the list of refs to upload - uploadRefs = append(uploadRefs, baseRef) + uploadRefs := []regname.Tag{baseRef} // Loop through all tags specified by the user and push the related image+tag for _, tag := range po.TagFlags.Tags { From 07b03b396e4e736b996cb6bb7b410cc50e29b01a Mon Sep 17 00:00:00 2001 From: Joe Searcy Date: Wed, 13 Sep 2023 18:37:17 -0400 Subject: [PATCH 12/12] Merge Push() and MultiPush() methods based on PR feedback Signed-off-by: Joe Searcy --- pkg/imgpkg/bundle/contents.go | 21 ++---------- pkg/imgpkg/bundle/contents_test.go | 4 +-- pkg/imgpkg/bundle/locations_configs.go | 2 +- pkg/imgpkg/cmd/push.go | 27 ++++------------ pkg/imgpkg/plainimage/contents.go | 44 ++------------------------ 5 files changed, 14 insertions(+), 84 deletions(-) diff --git a/pkg/imgpkg/bundle/contents.go b/pkg/imgpkg/bundle/contents.go index 9e9fa50d..66a59a3a 100644 --- a/pkg/imgpkg/bundle/contents.go +++ b/pkg/imgpkg/bundle/contents.go @@ -42,8 +42,8 @@ func NewContents(paths []string, excludedPaths []string, preservePermissions boo return Contents{paths: paths, excludedPaths: excludedPaths, preservePermissions: preservePermissions} } -// Push the contents of the bundle to the registry as an OCI Image -func (b Contents) Push(uploadRef regname.Tag, labels map[string]string, registry ImagesMetadataWriter, logger Logger) (string, error) { +// Push the contents of the bundle to the registry as an OCI Image with one or more tags +func (b Contents) Push(uploadRefs []regname.Tag, labels map[string]string, registry ImagesMetadataWriter, logger Logger) (string, error) { err := b.validate() if err != nil { return "", err @@ -54,22 +54,7 @@ func (b Contents) Push(uploadRef regname.Tag, labels map[string]string, registry } labels[BundleConfigLabel] = "true" - return plainimage.NewContents(b.paths, b.excludedPaths, b.preservePermissions).Push(uploadRef, labels, registry, logger) -} - -// MultiTagPush pushes the OCI Bundle to the registry with multiple tags -func (b Contents) MultiTagPush(uploadRefs []regname.Tag, labels map[string]string, registry ImagesMetadataWriter, logger Logger) (string, error) { - err := b.validate() - if err != nil { - return "", err - } - - if labels == nil { - labels = map[string]string{} - } - labels[BundleConfigLabel] = "true" - - return plainimage.NewContents(b.paths, b.excludedPaths, b.preservePermissions).MultiTagPush(uploadRefs, labels, registry, logger) + return plainimage.NewContents(b.paths, b.excludedPaths, b.preservePermissions).Push(uploadRefs, labels, registry, logger) } // PresentsAsBundle checks if the provided folders have the needed structure to be a bundle diff --git a/pkg/imgpkg/bundle/contents_test.go b/pkg/imgpkg/bundle/contents_test.go index 8e31d2ab..8edb05cc 100644 --- a/pkg/imgpkg/bundle/contents_test.go +++ b/pkg/imgpkg/bundle/contents_test.go @@ -44,7 +44,7 @@ images: t.Fatalf("failed to read tag: %s", err) } - _, err = subject.Push(imgTag, map[string]string{}, fakeRegistry, util.NewNoopLevelLogger()) + _, err = subject.Push([]name.Tag{imgTag}, map[string]string{}, fakeRegistry, util.NewNoopLevelLogger()) if err != nil { t.Fatalf("not expecting push to fail: %s", err) } @@ -78,7 +78,7 @@ images: t.Fatalf("failed to read tag: %s", err) } - _, err = subject.Push(imgTag, map[string]string{}, fakeRegistry, util.NewNoopLevelLogger()) + _, err = subject.Push([]name.Tag{imgTag}, map[string]string{}, fakeRegistry, util.NewNoopLevelLogger()) if err != nil { t.Fatalf("not expecting push to fail: %s", err) } diff --git a/pkg/imgpkg/bundle/locations_configs.go b/pkg/imgpkg/bundle/locations_configs.go index 19da3515..a5338036 100644 --- a/pkg/imgpkg/bundle/locations_configs.go +++ b/pkg/imgpkg/bundle/locations_configs.go @@ -133,7 +133,7 @@ func (r LocationsConfigs) Save(reg ImagesMetadataWriter, bundleRef name.Digest, r.ui.Tracef("Pushing image\n") - _, err = plainimage.NewContents([]string{tmpDir}, nil, false).Push(locRef, nil, reg.CloneWithLogger(util.NewNoopProgressBar()), logger) + _, err = plainimage.NewContents([]string{tmpDir}, nil, false).Push([]name.Tag{locRef}, nil, reg.CloneWithLogger(util.NewNoopProgressBar()), logger) if err != nil { // Immutable tag errors within registries are not standardized. // Assume word "immutable" would be present in most cases. diff --git a/pkg/imgpkg/cmd/push.go b/pkg/imgpkg/cmd/push.go index cf530ff3..a931ef6d 100644 --- a/pkg/imgpkg/cmd/push.go +++ b/pkg/imgpkg/cmd/push.go @@ -129,17 +129,9 @@ func (po *PushOptions) pushBundle(registry registry.Registry) (string, error) { logger := util.NewUILevelLogger(util.LogWarn, util.NewLogger(po.ui)) - if len(po.TagFlags.Tags) > 1 { - imageURL, err = bundle.NewContents(po.FileFlags.Files, po.FileFlags.ExcludedFilePaths, po.FileFlags.PreservePermissions).MultiTagPush(uploadRefs, po.LabelFlags.Labels, registry, logger) - if err != nil { - return "", err - } - - } else { - imageURL, err = bundle.NewContents(po.FileFlags.Files, po.FileFlags.ExcludedFilePaths, po.FileFlags.PreservePermissions).Push(uploadRefs[0], po.LabelFlags.Labels, registry, logger) - if err != nil { - return "", err - } + imageURL, err = bundle.NewContents(po.FileFlags.Files, po.FileFlags.ExcludedFilePaths, po.FileFlags.PreservePermissions).Push(uploadRefs, po.LabelFlags.Labels, registry, logger) + if err != nil { + return "", err } if po.LockOutputFlags.LockFilePath != "" { @@ -211,16 +203,9 @@ func (po *PushOptions) pushImage(registry registry.Registry) (string, error) { logger := util.NewUILevelLogger(util.LogWarn, util.NewLogger(po.ui)) - if len(po.TagFlags.Tags) > 1 { - imageURL, err = plainimage.NewContents(po.FileFlags.Files, po.FileFlags.ExcludedFilePaths, po.FileFlags.PreservePermissions).MultiTagPush(uploadRefs, po.LabelFlags.Labels, registry, logger) - if err != nil { - return "", err - } - } else { - imageURL, err = plainimage.NewContents(po.FileFlags.Files, po.FileFlags.ExcludedFilePaths, po.FileFlags.PreservePermissions).Push(uploadRefs[0], po.LabelFlags.Labels, registry, logger) - if err != nil { - return "", err - } + imageURL, err = plainimage.NewContents(po.FileFlags.Files, po.FileFlags.ExcludedFilePaths, po.FileFlags.PreservePermissions).Push(uploadRefs, po.LabelFlags.Labels, registry, logger) + if err != nil { + return "", err } if !strings.Contains(strings.Join(imageRefs, ","), imageURL) { diff --git a/pkg/imgpkg/plainimage/contents.go b/pkg/imgpkg/plainimage/contents.go index f44e09b4..6b1612a5 100644 --- a/pkg/imgpkg/plainimage/contents.go +++ b/pkg/imgpkg/plainimage/contents.go @@ -34,48 +34,8 @@ func NewContents(paths []string, excludedPaths []string, preservePermissions boo return Contents{paths: paths, excludedPaths: excludedPaths, preservePermissions: preservePermissions} } -// Push the OCI Image to the registry -func (i Contents) Push(uploadRef regname.Tag, labels map[string]string, writer ImagesWriter, logger Logger) (string, error) { - err := i.validate() - if err != nil { - return "", err - } - - tarImg := ctlimg.NewTarImage(i.paths, i.excludedPaths, logger, i.preservePermissions) - - img, err := tarImg.AsFileImage(labels) - if err != nil { - return "", err - } - - defer img.Remove() - - err = writer.WriteImage(uploadRef, img, nil) - - if err != nil { - return "", fmt.Errorf("Writing '%s': %s", uploadRef.Name(), err) - } - - digest, err := img.Digest() - if err != nil { - return "", err - } - - uploadTagRef, err := util.BuildDefaultUploadTagRef(img, uploadRef.Repository) - if err != nil { - return "", fmt.Errorf("Building default upload tag image ref: %s", err) - } - - err = writer.WriteTag(uploadTagRef, img) - if err != nil { - return "", fmt.Errorf("Writing Tag '%s': %s", uploadRef.Name(), err) - } - - return fmt.Sprintf("%s@%s", uploadRef.Context(), digest), nil -} - -// MultiTagPush pushes the OCI Image to the registry with multiple tags -func (i Contents) MultiTagPush(uploadRefs []regname.Tag, labels map[string]string, writer ImagesWriter, logger Logger) (string, error) { +// Push pushes the OCI Image to the registry with multiple one or more tags +func (i Contents) Push(uploadRefs []regname.Tag, labels map[string]string, writer ImagesWriter, logger Logger) (string, error) { var digest regv1.Hash var primaryUploadRef regname.Tag