Merge pull request #552 from phenixblue/add-labels-for-bundles-and-im…

…ages

Add support for OCI labels on image/bundle push

pkg/imgpkg/bundle/contents.go

@@ -43,13 +43,17 @@ func NewContents(paths []string, excludedPaths []string, preservePermissions boo
 }
 
 // Push the contents of the bundle to the registry as an OCI Image
-func (b Contents) Push(uploadRef regname.Tag, registry ImagesMetadataWriter, logger Logger) (string, error) {
+func (b Contents) Push(uploadRef regname.Tag, labels map[string]string, registry ImagesMetadataWriter, logger Logger) (string, error) {
 	err := b.validate()
 	if err != nil {
 		return "", err
 	}
 
-	labels := map[string]string{BundleConfigLabel: "true"}
+	if labels == nil {
+		labels = map[string]string{}
+	}
+	labels[BundleConfigLabel] = "true"
+
 	return plainimage.NewContents(b.paths, b.excludedPaths, b.preservePermissions).Push(uploadRef, labels, registry, logger)
 }
 

pkg/imgpkg/bundle/contents_test.go

@@ -44,7 +44,7 @@ images:
 			t.Fatalf("failed to read tag: %s", err)
 		}
 
-		_, err = subject.Push(imgTag, fakeRegistry, util.NewNoopLevelLogger())
+		_, err = subject.Push(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, fakeRegistry, util.NewNoopLevelLogger())
+		_, err = subject.Push(imgTag, map[string]string{}, fakeRegistry, util.NewNoopLevelLogger())
 		if err != nil {
 			t.Fatalf("not expecting push to fail: %s", err)
 		}

pkg/imgpkg/cmd/label_flags.go

@@ -0,0 +1,18 @@
+// Copyright 2020 VMware, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+package cmd
+
+import (
+	"github.com/spf13/cobra"
+)
+
+// LabelFlags is a struct that holds the labels for an OCI artifact
+type LabelFlags struct {
+	Labels map[string]string
+}
+
+// Set sets the labels for an OCI artifact
+func (l *LabelFlags) Set(cmd *cobra.Command) {
+	cmd.Flags().StringToStringVarP(&l.Labels, "labels", "l", map[string]string{}, "Set labels on image")
+}

pkg/imgpkg/cmd/push.go

@@ -24,6 +24,7 @@ type PushOptions struct {
 	LockOutputFlags LockOutputFlags
 	FileFlags       FileFlags
 	RegistryFlags   RegistryFlags
+	LabelFlags      LabelFlags
 }
 
 func NewPushOptions(ui ui.UI) *PushOptions {
@@ -47,6 +48,8 @@ func NewPushCmd(o *PushOptions) *cobra.Command {
 	o.LockOutputFlags.SetOnPush(cmd)
 	o.FileFlags.Set(cmd)
 	o.RegistryFlags.Set(cmd)
+	o.LabelFlags.Set(cmd)
+
 	return cmd
 }
 
@@ -56,6 +59,11 @@ func (po *PushOptions) Run() error {
 		return err
 	}
 
+	err = po.validateFlags()
+	if err != nil {
+		return err
+	}
+
 	var imageURL string
 
 	isBundle := po.BundleFlags.Bundle != ""
@@ -96,7 +104,7 @@ func (po *PushOptions) pushBundle(registry registry.Registry) (string, error) {
 	}
 
 	logger := util.NewUILevelLogger(util.LogWarn, util.NewLogger(po.ui))
-	imageURL, err := bundle.NewContents(po.FileFlags.Files, po.FileFlags.ExcludedFilePaths, po.FileFlags.PreservePermissions).Push(uploadRef, registry, logger)
+	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
 	}
@@ -141,5 +149,19 @@ func (po *PushOptions) pushImage(registry registry.Registry) (string, error) {
 	}
 
 	logger := util.NewUILevelLogger(util.LogWarn, util.NewLogger(po.ui))
-	return plainimage.NewContents(po.FileFlags.Files, po.FileFlags.ExcludedFilePaths, po.FileFlags.PreservePermissions).Push(uploadRef, nil, registry, logger)
+	return plainimage.NewContents(po.FileFlags.Files, po.FileFlags.ExcludedFilePaths, po.FileFlags.PreservePermissions).Push(uploadRef, po.LabelFlags.Labels, registry, logger)
+}
+
+// validateFlags checks if the provided flags are valid
+func (po *PushOptions) validateFlags() error {
+
+	// Verify the user did NOT specify a reserved OCI label
+	_, present := po.LabelFlags.Labels[bundle.BundleConfigLabel]
+
+	if present {
+		return fmt.Errorf("label '%s' is reserved and cannot be overriden. Please use a different key", bundle.BundleConfigLabel)
+	}
+
+	return nil
+
 }

pkg/imgpkg/cmd/push_test.go

@@ -11,8 +11,12 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/google/go-containerregistry/pkg/authn"
+	"github.com/google/go-containerregistry/pkg/name"
+	"github.com/google/go-containerregistry/pkg/v1/remote"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"github.com/vmware-tanzu/carvel-imgpkg/test/helpers"
 )
 
 const emptyImagesYaml = `apiVersion: imgpkg.carvel.dev/v1alpha1
@@ -235,6 +239,93 @@ func TestImageAndBundleLockError(t *testing.T) {
 	}
 }
 
+func TestLabels(t *testing.T) {
+	testCases := []struct {
+		name           string
+		opType         string
+		expectedError  string
+		expectedLabels map[string]string
+		labelInput     string
+	}{
+		{
+			name:           "bundle with multiple labels",
+			opType:         "bundle",
+			expectedError:  "",
+			labelInput:     "foo=bar,bar=baz",
+			expectedLabels: map[string]string{"dev.carvel.imgpkg.bundle": "true", "foo": "bar", "bar": "baz"},
+		},
+		{
+			name:           "image with multiple labels",
+			opType:         "image",
+			expectedError:  "",
+			labelInput:     "foo=bar,bar=baz",
+			expectedLabels: map[string]string{"foo": "bar", "bar": "baz"},
+		},
+		{
+			name:           "bundle with \".\" in label key",
+			opType:         "bundle",
+			expectedError:  "",
+			labelInput:     "foo.bar=baz",
+			expectedLabels: map[string]string{"dev.carvel.imgpkg.bundle": "true", "foo.bar": "baz"},
+		},
+		{
+			name:           "bundle with long label key (> 64 chars)",
+			opType:         "bundle",
+			expectedError:  "",
+			labelInput:     "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa=baz",
+			expectedLabels: map[string]string{"dev.carvel.imgpkg.bundle": "true", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa": "baz"},
+		},
+		{
+			name:           "bundle with long label value (> 256 chars)",
+			opType:         "bundle",
+			expectedError:  "",
+			labelInput:     "foo=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+			expectedLabels: map[string]string{"dev.carvel.imgpkg.bundle": "true", "foo": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"},
+		},
+		{
+			name:           "bundle with spaces in label value",
+			opType:         "bundle",
+			expectedError:  "",
+			labelInput:     "foo.bar=baz bar",
+			expectedLabels: map[string]string{"dev.carvel.imgpkg.bundle": "true", "foo.bar": "baz bar"},
+		},
+	}
+
+	for _, tc := range testCases {
+		f := func(t *testing.T) {
+			env := helpers.BuildEnv(t)
+			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.labelInput == "" {
+				imgpkg.Run([]string{"push", opTypeFlag, env.Image, "-f", pushDir})
+			} else {
+				imgpkg.Run([]string{"push", opTypeFlag, env.Image, "-l", tc.labelInput, "-f", pushDir})
+			}
+
+			ref, _ := name.NewTag(env.Image, name.WeakValidation)
+			image, err := remote.Image(ref, remote.WithAuthFromKeychain(authn.DefaultKeychain))
+			require.NoError(t, err)
+
+			config, err := image.ConfigFile()
+			require.NoError(t, err)
+
+			require.Equal(t, tc.expectedLabels, config.Config.Labels, "Expected labels provided via flags to match labels discovered on image")
+
+		}
+
+		t.Run(tc.name, f)
+	}
+}
+
 func Cleanup(dirs ...string) {
 	for _, dir := range dirs {
 		os.RemoveAll(dir)