use apimachinery for annotation/label validation

Author: khrisrichardson

Gopkg.lock

@@ -22,8 +22,10 @@ import (
 
 	"github.com/kubernetes-sigs/kustomize/pkg/constants"
 	"github.com/kubernetes-sigs/kustomize/pkg/fs"
-	"github.com/kubernetes-sigs/kustomize/pkg/validate"
 	"github.com/spf13/cobra"
+	"k8s.io/apimachinery/pkg/api/validation"
+	v1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 )
 
 // KindOfAdd is the kind of metadata being added: label or annotation
@@ -46,12 +48,17 @@ func (k KindOfAdd) String() string {
 }
 
 type addMetadataOptions struct {
-	metadata map[string]string
+	metadata            map[string]string
+	validateAnnotations func(map[string]string) field.ErrorList
+	validateLabels      func(map[string]string) field.ErrorList
 }
 
 // newCmdAddAnnotation adds one or more commonAnnotations to the kustomization file.
 func newCmdAddAnnotation(fsys fs.FileSystem) *cobra.Command {
 	var o addMetadataOptions
+	o.validateAnnotations = func(x map[string]string) field.ErrorList {
+		return validation.ValidateAnnotations(x, field.NewPath("field"))
+	}
 
 	cmd := &cobra.Command{
 		Use:   "annotation",
@@ -72,6 +79,9 @@ func newCmdAddAnnotation(fsys fs.FileSystem) *cobra.Command {
 // newCmdAddLabel adds one or more commonLabels to the kustomization file.
 func newCmdAddLabel(fsys fs.FileSystem) *cobra.Command {
 	var o addMetadataOptions
+	o.validateLabels = func(x map[string]string) field.ErrorList {
+		return v1validation.ValidateLabels(x, field.NewPath("field"))
+	}
 
 	cmd := &cobra.Command{
 		Use:   "label",
@@ -100,23 +110,30 @@ func (o *addMetadataOptions) ValidateAndParse(args []string, k KindOfAdd) error
 	}
 	inputs := strings.Split(args[0], ",")
 	for _, input := range inputs {
+		metadata := make(map[string]string)
+		//parse annotation keys and values into metadata
+		kv := strings.Split(input, ":")
+		if len(kv[0]) < 1 {
+			return fmt.Errorf("invalid %s format: %s", k, input)
+		}
+		if len(kv) > 1 {
+			metadata[kv[0]] = kv[1]
+		} else {
+			metadata[kv[0]] = ""
+		}
 		switch k {
 		case label:
-			valid, err := validate.IsValidLabel(input)
-			if !valid {
-				return err
+			if errs := o.validateLabels(metadata); len(errs) != 0 {
+				return fmt.Errorf("invalid %s format: %s", k, input)
 			}
 		case annotation:
-			valid, err := validate.IsValidAnnotation(input)
-			if !valid {
-				return err
+			if errs := o.validateAnnotations(metadata); len(errs) != 0 {
+				return fmt.Errorf("invalid %s format: %s", k, input)
 			}
 		default:
 			return fmt.Errorf("unknown metadata kind %s", k)
 		}
-		//parse annotation keys and values into metadata
-		kv := strings.Split(input, ":")
-		o.metadata[kv[0]] = kv[1]
+		o.metadata[kv[0]] = metadata[kv[0]]
 	}
 	return nil
 }

pkg/commands/addmetadata.go

@@ -17,80 +17,12 @@ limitations under the License.
 package commands
 
 import (
-	"reflect"
 	"testing"
 
 	"github.com/kubernetes-sigs/kustomize/pkg/constants"
 	"github.com/kubernetes-sigs/kustomize/pkg/fs"
 )
 
-func TestParseValidateInput(t *testing.T) {
-	var testcases = []struct {
-		input        string
-		valid        bool
-		name         string
-		expectedData map[string]string
-		kind         KindOfAdd
-	}{
-		{
-			input: "otters:cute",
-			valid: true,
-			name:  "Adds single input",
-			expectedData: map[string]string{
-				"otters": "cute",
-			},
-			kind: label,
-		},
-		{
-			input: "owls:great,unicorns:magical",
-			valid: true,
-			name:  "Adds two items",
-			expectedData: map[string]string{
-				"owls":     "great",
-				"unicorns": "magical",
-			},
-			kind: label,
-		},
-		{
-			input: "123:45",
-			valid: true,
-			name:  "Numeric input is allowed",
-			expectedData: map[string]string{
-				"123": "45",
-			},
-			kind: annotation,
-		},
-		{
-			input:        " ",
-			valid:        false,
-			name:         "Empty space input",
-			expectedData: nil,
-			kind:         annotation,
-		},
-	}
-	var o addMetadataOptions
-	for _, tc := range testcases {
-		args := []string{tc.input}
-		err := o.ValidateAndParse(args, tc.kind)
-		if err != nil && tc.valid {
-			t.Errorf("for test case %s, unexpected cmd error: %v", tc.name, err)
-		}
-		if err == nil && !tc.valid {
-			t.Errorf("unexpected error: expected invalid format error for test case %v", tc.name)
-		}
-		//o.metadata should be the same as expectedData
-		if tc.valid {
-			if !reflect.DeepEqual(o.metadata, tc.expectedData) {
-				t.Errorf("unexpected error: for test case %s, unexpected data was added", tc.name)
-			}
-		} else {
-			if len(o.metadata) != 0 {
-				t.Errorf("unexpected error: for test case %s, expected no data to be added", tc.name)
-			}
-		}
-	}
-}
-
 func TestRunAddAnnotation(t *testing.T) {
 	fakeFS := fs.MakeFakeFS()
 	fakeFS.WriteFile(constants.KustomizationFileName, []byte(kustomizationContent))
@@ -125,6 +57,44 @@ func TestAddAnnotationNoArgs(t *testing.T) {
 		t.Errorf("incorrect error: %v", err.Error())
 	}
 }
+
+func TestAddAnnotationInvalidFormat(t *testing.T) {
+	fakeFS := fs.MakeFakeFS()
+	cmd := newCmdAddAnnotation(fakeFS)
+	args := []string{"exclamation!:point"}
+	err := cmd.RunE(cmd, args)
+	if err == nil {
+		t.Errorf("expected an error but error is %v", err)
+	}
+	if err != nil && err.Error() != "invalid annotation format: exclamation!:point" {
+		t.Errorf("incorrect error: %v", err.Error())
+	}
+}
+
+func TestAddAnnotationNoKey(t *testing.T) {
+	fakeFS := fs.MakeFakeFS()
+	cmd := newCmdAddAnnotation(fakeFS)
+	args := []string{":nokey"}
+	err := cmd.RunE(cmd, args)
+	if err == nil {
+		t.Errorf("expected an error but error is %v", err)
+	}
+	if err != nil && err.Error() != "invalid annotation format: :nokey" {
+		t.Errorf("incorrect error: %v", err.Error())
+	}
+}
+
+func TestAddAnnotationNoValue(t *testing.T) {
+	fakeFS := fs.MakeFakeFS()
+	fakeFS.WriteFile(constants.KustomizationFileName, []byte(kustomizationContent))
+	cmd := newCmdAddAnnotation(fakeFS)
+	args := []string{"no:,value"}
+	err := cmd.RunE(cmd, args)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err.Error())
+	}
+}
+
 func TestAddAnnotationMultipleArgs(t *testing.T) {
 	fakeFS := fs.MakeFakeFS()
 	fakeFS.WriteFile(constants.KustomizationFileName, []byte(kustomizationContent))
@@ -164,7 +134,6 @@ func TestRunAddLabel(t *testing.T) {
 
 func TestAddLabelNoArgs(t *testing.T) {
 	fakeFS := fs.MakeFakeFS()
-
 	cmd := newCmdAddLabel(fakeFS)
 	err := cmd.Execute()
 	if err == nil {
@@ -175,6 +144,43 @@ func TestAddLabelNoArgs(t *testing.T) {
 	}
 }
 
+func TestAddLabelInvalidFormat(t *testing.T) {
+	fakeFS := fs.MakeFakeFS()
+	cmd := newCmdAddLabel(fakeFS)
+	args := []string{"exclamation!:point"}
+	err := cmd.RunE(cmd, args)
+	if err == nil {
+		t.Errorf("expected an error but error is: %v", err)
+	}
+	if err != nil && err.Error() != "invalid label format: exclamation!:point" {
+		t.Errorf("incorrect error: %v", err.Error())
+	}
+}
+
+func TestAddLabelNoKey(t *testing.T) {
+	fakeFS := fs.MakeFakeFS()
+	cmd := newCmdAddLabel(fakeFS)
+	args := []string{":nokey"}
+	err := cmd.RunE(cmd, args)
+	if err == nil {
+		t.Errorf("expected an error but error is: %v", err)
+	}
+	if err != nil && err.Error() != "invalid label format: :nokey" {
+		t.Errorf("incorrect error: %v", err.Error())
+	}
+}
+
+func TestAddLabelNoValue(t *testing.T) {
+	fakeFS := fs.MakeFakeFS()
+	fakeFS.WriteFile(constants.KustomizationFileName, []byte(kustomizationContent))
+	cmd := newCmdAddLabel(fakeFS)
+	args := []string{"no,value:"}
+	err := cmd.RunE(cmd, args)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err.Error())
+	}
+}
+
 func TestAddLabelMultipleArgs(t *testing.T) {
 	fakeFS := fs.MakeFakeFS()
 	fakeFS.WriteFile(constants.KustomizationFileName, []byte(kustomizationContent))