diff --git a/Gopkg.lock b/Gopkg.lock index b89ef4c63..9ed0cbf47 100644 --- a/Gopkg.lock +++ b/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", diff --git a/pkg/commands/addmetadata.go b/pkg/commands/addmetadata.go index 56e57ba32..65336fcdf 100644 --- a/pkg/commands/addmetadata.go +++ b/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 } diff --git a/pkg/commands/addmetadata_test.go b/pkg/commands/addmetadata_test.go index e796ff04b..86d2a5bb1 100644 --- a/pkg/commands/addmetadata_test.go +++ b/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)) diff --git a/pkg/validate/validate.go b/pkg/validate/validate.go deleted file mode 100644 index 94ae5de11..000000000 --- a/pkg/validate/validate.go +++ /dev/null @@ -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 -} diff --git a/pkg/validate/validate_test.go b/pkg/validate/validate_test.go deleted file mode 100644 index ff84f82b2..000000000 --- a/pkg/validate/validate_test.go +++ /dev/null @@ -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) - } - } -}