use apimachinery for annotation/label validation

Gopkg.lock

@@ -427,9 +427,14 @@
   digest = "1:bcb2285bb525712de7903a5d254c2789df65c8b58d2cfac5a26d950ad94c2079"
   name = "k8s.io/apimachinery"
   packages = [
+    "pkg/api/equality",
+    "pkg/api/meta",
     "pkg/api/resource",
+    "pkg/api/validation",
     "pkg/apis/meta/v1",
     "pkg/apis/meta/v1/unstructured",
+    "pkg/apis/meta/v1/validation",
+    "pkg/apis/meta/v1beta1",
     "pkg/conversion",
     "pkg/conversion/queryparams",
     "pkg/fields",
@@ -493,14 +498,17 @@
     "github.com/pkg/errors",
     "github.com/spf13/cobra",
     "k8s.io/api/core/v1",
+    "k8s.io/apimachinery/pkg/api/validation",
     "k8s.io/apimachinery/pkg/apis/meta/v1",
     "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured",
+    "k8s.io/apimachinery/pkg/apis/meta/v1/validation",
     "k8s.io/apimachinery/pkg/runtime",
     "k8s.io/apimachinery/pkg/runtime/schema",
     "k8s.io/apimachinery/pkg/util/mergepatch",
     "k8s.io/apimachinery/pkg/util/sets",
     "k8s.io/apimachinery/pkg/util/strategicpatch",
     "k8s.io/apimachinery/pkg/util/validation",
+    "k8s.io/apimachinery/pkg/util/validation/field",
     "k8s.io/apimachinery/pkg/util/yaml",
     "k8s.io/client-go/kubernetes/scheme",
     "k8s.io/kube-openapi/pkg/common",

pkg/commands/addmetadata.go

@@ -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_test.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))

pkg/validate/validate.go

@@ -1,35 +0,0 @@
-package validate
-
-import (
-	"fmt"
-	"regexp"
-)
-
-// TODO: these are rudimentary placeholder validation functions and need
-// additional work to truly match expected syntax rules.
-
-// IsValidLabel checks whether a label key/value pair has correct syntax and
-// character set
-func IsValidLabel(keyval string) (bool, error) {
-	ok, err := regexp.MatchString(`\A([a-zA-Z0-9_.-]+):([a-zA-Z0-9_.-]+)\z`, keyval)
-	if err != nil {
-		return false, err
-	}
-	if !ok {
-		return false, fmt.Errorf("invalid label format: %s", keyval)
-	}
-	return true, nil
-}
-
-// IsValidAnnotation checks whether an annotation key/value pair has correct
-// syntax and character set
-func IsValidAnnotation(keyval string) (bool, error) {
-	ok, err := regexp.MatchString(`\A([a-zA-Z0-9_.-]+):([a-zA-Z0-9_.-]+)\z`, keyval)
-	if err != nil {
-		return false, err
-	}
-	if !ok {
-		return false, fmt.Errorf("invalid annotation format: %s", keyval)
-	}
-	return true, nil
-}

pkg/validate/validate_test.go

@@ -1,103 +0,0 @@
-package validate
-
-import "testing"
-
-func TestIsValidLabel(t *testing.T) {
-	testcases := []struct {
-		input, name string
-		valid       bool
-	}{
-		{
-			input: "otters:cute",
-			valid: true,
-			name:  "Valid input format",
-		},
-		{
-			input: "dogs,cats",
-			valid: false,
-			name:  "Does not contain colon",
-		},
-		{
-			input: ":noKey",
-			valid: false,
-			name:  "Missing key",
-		},
-		{
-			input: "noValue:",
-			valid: false,
-			name:  "Missing value",
-		},
-		{
-			input: "exclamation!:point",
-			valid: false,
-			name:  "Non-alphanumeric input",
-		},
-		{
-			input: "123:45",
-			valid: true,
-			name:  "Numeric input is allowed",
-		},
-	}
-	for _, tc := range testcases {
-		ok, err := IsValidLabel(tc.input)
-		if tc.valid && err != nil {
-			t.Errorf("unexpected error: for test case %s, expected no error but got: %s", tc.name, err.Error())
-		}
-		if ok && !tc.valid {
-			t.Errorf("for test case %s, expected invalid label format error", tc.name)
-		}
-		if !ok && tc.valid {
-			t.Errorf("unexpected error: for test case %s, expected test to pass", tc.name)
-		}
-	}
-}
-
-func TestIsValidAnnotation(t *testing.T) {
-	testcases := []struct {
-		input, name string
-		valid       bool
-	}{
-		{
-			input: "owls:adorable",
-			valid: true,
-			name:  "Valid input format",
-		},
-		{
-			input: "cake,cookies",
-			valid: false,
-			name:  "Does not contain colon",
-		},
-		{
-			input: ":noKey",
-			valid: false,
-			name:  "Missing key",
-		},
-		{
-			input: "noValue:",
-			valid: false,
-			name:  "Missing value",
-		},
-		{
-			input: "exclamation!:point",
-			valid: false,
-			name:  "Input has a bang!",
-		},
-		{
-			input: "987:65",
-			valid: true,
-			name:  "Numeric input is valid",
-		},
-	}
-	for _, tc := range testcases {
-		ok, err := IsValidAnnotation(tc.input)
-		if tc.valid && err != nil {
-			t.Errorf("unexpected error: for test case %s, expected no error but got: %s", tc.name, err.Error())
-		}
-		if ok && !tc.valid {
-			t.Errorf("for test case %s, expected invalid annotation format error", tc.name)
-		}
-		if !ok && tc.valid {
-			t.Errorf("unexpected error: for test case %s, expected test to pass", tc.name)
-		}
-	}
-}