diff --git a/pkg/commands/addmetadata.go b/pkg/commands/addmetadata.go index 65336fcdf..9dcec9b12 100644 --- a/pkg/commands/addmetadata.go +++ b/pkg/commands/addmetadata.go @@ -22,21 +22,19 @@ import ( "github.com/kubernetes-sigs/kustomize/pkg/constants" "github.com/kubernetes-sigs/kustomize/pkg/fs" + "github.com/kubernetes-sigs/kustomize/pkg/validators" "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 -type KindOfAdd int +// kindOfAdd is the kind of metadata being added: label or annotation +type kindOfAdd int const ( - annotation KindOfAdd = iota + annotation kindOfAdd = iota label ) -func (k KindOfAdd) String() string { +func (k kindOfAdd) String() string { kinds := [...]string{ "annotation", "label", @@ -48,98 +46,96 @@ func (k KindOfAdd) String() string { } type addMetadataOptions struct { - metadata map[string]string - validateAnnotations func(map[string]string) field.ErrorList - validateLabels func(map[string]string) field.ErrorList + metadata map[string]string + mapValidator validators.MapValidatorFunc + kind kindOfAdd } // newCmdAddAnnotation adds one or more commonAnnotations to the kustomization file. -func newCmdAddAnnotation(fsys fs.FileSystem) *cobra.Command { +func newCmdAddAnnotation(fSys fs.FileSystem, v validators.MapValidatorFunc) *cobra.Command { var o addMetadataOptions - o.validateAnnotations = func(x map[string]string) field.ErrorList { - return validation.ValidateAnnotations(x, field.NewPath("field")) - } + o.kind = annotation + o.mapValidator = v cmd := &cobra.Command{ Use: "annotation", - Short: "Adds one or more commonAnnotations to the kustomization.yaml in current directory", + Short: "Adds one or more commonAnnotations to " + constants.KustomizationFileName, Example: ` add annotation {annotationKey1:annotationValue1},{annotationKey2:annotationValue2}`, RunE: func(cmd *cobra.Command, args []string) error { - err := o.ValidateAndParse(args, annotation) + err := o.ValidateAndParse(args) if err != nil { return err } - return o.RunAddAnnotation(fsys, annotation) + return o.RunAddAnnotation(fSys) }, } return cmd } // newCmdAddLabel adds one or more commonLabels to the kustomization file. -func newCmdAddLabel(fsys fs.FileSystem) *cobra.Command { +func newCmdAddLabel(fSys fs.FileSystem, v validators.MapValidatorFunc) *cobra.Command { var o addMetadataOptions - o.validateLabels = func(x map[string]string) field.ErrorList { - return v1validation.ValidateLabels(x, field.NewPath("field")) - } + o.kind = label + o.mapValidator = v cmd := &cobra.Command{ Use: "label", - Short: "Adds one or more commonLabels to the kustomization.yaml in current directory", + Short: "Adds one or more commonLabels to " + constants.KustomizationFileName, Example: ` add label {labelKey1:labelValue1},{labelKey2:labelValue2}`, RunE: func(cmd *cobra.Command, args []string) error { - err := o.ValidateAndParse(args, label) + err := o.ValidateAndParse(args) if err != nil { return err } - return o.RunAddLabel(fsys, label) + return o.RunAddLabel(fSys) }, } return cmd } // ValidateAndParse validates addLabel and addAnnotation commands and parses them into o.metadata -func (o *addMetadataOptions) ValidateAndParse(args []string, k KindOfAdd) error { - o.metadata = make(map[string]string) +func (o *addMetadataOptions) ValidateAndParse(args []string) error { if len(args) < 1 { - return fmt.Errorf("must specify %s", k) + return fmt.Errorf("must specify %s", o.kind) } if len(args) > 1 { - return fmt.Errorf("%ss must be comma-separated, with no spaces. See help text for example", k) + return fmt.Errorf("%ss must be comma-separated, with no spaces", o.kind) } - 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: - if errs := o.validateLabels(metadata); len(errs) != 0 { - return fmt.Errorf("invalid %s format: %s", k, input) - } - case annotation: - 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) - } - o.metadata[kv[0]] = metadata[kv[0]] + m, err := o.convertToMap(args[0]) + if err != nil { + return err } + if err = o.mapValidator(m); err != nil { + return err + } + o.metadata = m return nil } +func (o *addMetadataOptions) convertToMap(arg string) (map[string]string, error) { + result := make(map[string]string) + inputs := strings.Split(arg, ",") + for _, input := range inputs { + kv := strings.Split(input, ":") + if len(kv[0]) < 1 { + return nil, makeError(o.kind, input, "empty key") + } + if len(kv) > 2 { + return nil, makeError(o.kind, input, "too many colons") + } + if len(kv) > 1 { + result[kv[0]] = kv[1] + } else { + result[kv[0]] = "" + } + } + return result, nil +} + // RunAddAnnotation runs addAnnotation command, doing the real work. -func (o *addMetadataOptions) RunAddAnnotation(fsys fs.FileSystem, k KindOfAdd) error { +func (o *addMetadataOptions) RunAddAnnotation(fsys fs.FileSystem) error { mf, err := newKustomizationFile(constants.KustomizationFileName, fsys) if err != nil { return err @@ -154,18 +150,16 @@ func (o *addMetadataOptions) RunAddAnnotation(fsys fs.FileSystem, k KindOfAdd) e } for key, value := range o.metadata { - if k == annotation { - if _, ok := m.CommonAnnotations[key]; ok { - return fmt.Errorf("%s %s already in kustomization file", k, key) - } - m.CommonAnnotations[key] = value + if _, ok := m.CommonAnnotations[key]; ok { + return fmt.Errorf("%s %s already in kustomization file", o.kind, key) } + m.CommonAnnotations[key] = value } return mf.write(m) } // RunAddLabel runs addLabel command, doing the real work. -func (o *addMetadataOptions) RunAddLabel(fsys fs.FileSystem, k KindOfAdd) error { +func (o *addMetadataOptions) RunAddLabel(fsys fs.FileSystem) error { mf, err := newKustomizationFile(constants.KustomizationFileName, fsys) if err != nil { return err @@ -181,9 +175,13 @@ func (o *addMetadataOptions) RunAddLabel(fsys fs.FileSystem, k KindOfAdd) error for key, value := range o.metadata { if _, ok := m.CommonLabels[key]; ok { - return fmt.Errorf("%s %s already in kustomization file", k, key) + return fmt.Errorf("%s %s already in kustomization file", o.kind, key) } m.CommonLabels[key] = value } return mf.write(m) } + +func makeError(k kindOfAdd, input string, message string) error { + return fmt.Errorf("invalid %s: %s (%s)", k, input, message) +} diff --git a/pkg/commands/addmetadata_test.go b/pkg/commands/addmetadata_test.go index 86d2a5bb1..3099bed77 100644 --- a/pkg/commands/addmetadata_test.go +++ b/pkg/commands/addmetadata_test.go @@ -21,6 +21,7 @@ import ( "github.com/kubernetes-sigs/kustomize/pkg/constants" "github.com/kubernetes-sigs/kustomize/pkg/fs" + "github.com/kubernetes-sigs/kustomize/pkg/validators" ) func TestRunAddAnnotation(t *testing.T) { @@ -29,18 +30,18 @@ func TestRunAddAnnotation(t *testing.T) { var o addMetadataOptions o.metadata = map[string]string{"owls": "cute", "otters": "adorable"} - err := o.RunAddAnnotation(fakeFS, annotation) + err := o.RunAddAnnotation(fakeFS) if err != nil { t.Errorf("unexpected error: could not write to kustomization file") } // adding the same test input should not work - err = o.RunAddAnnotation(fakeFS, annotation) + err = o.RunAddAnnotation(fakeFS) if err == nil { t.Errorf("expected already in kustomization file error") } // adding new annotations should work o.metadata = map[string]string{"new": "annotation"} - err = o.RunAddAnnotation(fakeFS, annotation) + err = o.RunAddAnnotation(fakeFS) if err != nil { t.Errorf("unexpected error: could not write to kustomization file") } @@ -48,38 +49,72 @@ func TestRunAddAnnotation(t *testing.T) { func TestAddAnnotationNoArgs(t *testing.T) { fakeFS := fs.MakeFakeFS() - cmd := newCmdAddAnnotation(fakeFS) + v := validators.MakeHappyMapValidator(t) + cmd := newCmdAddAnnotation(fakeFS, v.Validator) err := cmd.Execute() + v.VerifyNoCall() if err == nil { - t.Errorf("expected an error but error is %v", err) + t.Errorf("expected an error") } - if err != nil && err.Error() != "must specify annotation" { + if err.Error() != "must specify annotation" { t.Errorf("incorrect error: %v", err.Error()) } } func TestAddAnnotationInvalidFormat(t *testing.T) { fakeFS := fs.MakeFakeFS() - cmd := newCmdAddAnnotation(fakeFS) - args := []string{"exclamation!:point"} + v := validators.MakeSadMapValidator(t) + cmd := newCmdAddAnnotation(fakeFS, v.Validator) + args := []string{"whatever:whatever"} err := cmd.RunE(cmd, args) + v.VerifyCall() if err == nil { - t.Errorf("expected an error but error is %v", err) + t.Errorf("expected an error") } - if err != nil && err.Error() != "invalid annotation format: exclamation!:point" { + if err.Error() != validators.SAD { t.Errorf("incorrect error: %v", err.Error()) } } +func TestAddAnnotationManyArgs(t *testing.T) { + fakeFS := fs.MakeFakeFS() + fakeFS.WriteFile(constants.KustomizationFileName, []byte(kustomizationContent)) + v := validators.MakeHappyMapValidator(t) + cmd := newCmdAddAnnotation(fakeFS, v.Validator) + args := []string{"k1:v1,k2:v2,k3:v3,k4:v5"} + err := cmd.RunE(cmd, args) + v.VerifyCall() + if err != nil { + t.Errorf("unexpected error: %v", err.Error()) + } +} + func TestAddAnnotationNoKey(t *testing.T) { fakeFS := fs.MakeFakeFS() - cmd := newCmdAddAnnotation(fakeFS) + v := validators.MakeHappyMapValidator(t) + cmd := newCmdAddAnnotation(fakeFS, v.Validator) args := []string{":nokey"} err := cmd.RunE(cmd, args) + v.VerifyNoCall() if err == nil { - t.Errorf("expected an error but error is %v", err) + t.Errorf("expected an error") } - if err != nil && err.Error() != "invalid annotation format: :nokey" { + if err.Error() != "invalid annotation: :nokey (empty key)" { + t.Errorf("incorrect error: %v", err.Error()) + } +} + +func TestAddAnnotationTooManyColons(t *testing.T) { + fakeFS := fs.MakeFakeFS() + v := validators.MakeHappyMapValidator(t) + cmd := newCmdAddAnnotation(fakeFS, v.Validator) + args := []string{"key:v1:v2"} + err := cmd.RunE(cmd, args) + v.VerifyNoCall() + if err == nil { + t.Errorf("expected an error") + } + if err.Error() != "invalid annotation: key:v1:v2 (too many colons)" { t.Errorf("incorrect error: %v", err.Error()) } } @@ -87,9 +122,11 @@ func TestAddAnnotationNoKey(t *testing.T) { func TestAddAnnotationNoValue(t *testing.T) { fakeFS := fs.MakeFakeFS() fakeFS.WriteFile(constants.KustomizationFileName, []byte(kustomizationContent)) - cmd := newCmdAddAnnotation(fakeFS) + v := validators.MakeHappyMapValidator(t) + cmd := newCmdAddAnnotation(fakeFS, v.Validator) args := []string{"no:,value"} err := cmd.RunE(cmd, args) + v.VerifyCall() if err != nil { t.Errorf("unexpected error: %v", err.Error()) } @@ -98,13 +135,15 @@ func TestAddAnnotationNoValue(t *testing.T) { func TestAddAnnotationMultipleArgs(t *testing.T) { fakeFS := fs.MakeFakeFS() fakeFS.WriteFile(constants.KustomizationFileName, []byte(kustomizationContent)) - cmd := newCmdAddAnnotation(fakeFS) + v := validators.MakeHappyMapValidator(t) + cmd := newCmdAddAnnotation(fakeFS, v.Validator) args := []string{"this:annotation", "has:spaces"} err := cmd.RunE(cmd, args) + v.VerifyNoCall() if err == nil { - t.Errorf("expected an error but error is %v", err) + t.Errorf("expected an error") } - if err != nil && err.Error() != "annotations must be comma-separated, with no spaces. See help text for example" { + if err.Error() != "annotations must be comma-separated, with no spaces" { t.Errorf("incorrect error: %v", err.Error()) } } @@ -115,18 +154,18 @@ func TestRunAddLabel(t *testing.T) { var o addMetadataOptions o.metadata = map[string]string{"owls": "cute", "otters": "adorable"} - err := o.RunAddLabel(fakeFS, label) + err := o.RunAddLabel(fakeFS) if err != nil { t.Errorf("unexpected error: could not write to kustomization file") } // adding the same test input should not work - err = o.RunAddLabel(fakeFS, label) + err = o.RunAddLabel(fakeFS) if err == nil { t.Errorf("expected already in kustomization file error") } // adding new labels should work o.metadata = map[string]string{"new": "label"} - err = o.RunAddLabel(fakeFS, label) + err = o.RunAddLabel(fakeFS) if err != nil { t.Errorf("unexpected error: could not write to kustomization file") } @@ -134,38 +173,59 @@ func TestRunAddLabel(t *testing.T) { func TestAddLabelNoArgs(t *testing.T) { fakeFS := fs.MakeFakeFS() - cmd := newCmdAddLabel(fakeFS) + v := validators.MakeHappyMapValidator(t) + cmd := newCmdAddLabel(fakeFS, v.Validator) err := cmd.Execute() + v.VerifyNoCall() if err == nil { - t.Errorf("expected an error but error is: %v", err) + t.Errorf("expected an error") } - if err != nil && err.Error() != "must specify label" { + if err.Error() != "must specify label" { t.Errorf("incorrect error: %v", err.Error()) } } func TestAddLabelInvalidFormat(t *testing.T) { fakeFS := fs.MakeFakeFS() - cmd := newCmdAddLabel(fakeFS) + v := validators.MakeSadMapValidator(t) + cmd := newCmdAddLabel(fakeFS, v.Validator) args := []string{"exclamation!:point"} err := cmd.RunE(cmd, args) + v.VerifyCall() if err == nil { - t.Errorf("expected an error but error is: %v", err) + t.Errorf("expected an error") } - if err != nil && err.Error() != "invalid label format: exclamation!:point" { + if err.Error() != validators.SAD { t.Errorf("incorrect error: %v", err.Error()) } } func TestAddLabelNoKey(t *testing.T) { fakeFS := fs.MakeFakeFS() - cmd := newCmdAddLabel(fakeFS) + v := validators.MakeHappyMapValidator(t) + cmd := newCmdAddLabel(fakeFS, v.Validator) args := []string{":nokey"} err := cmd.RunE(cmd, args) + v.VerifyNoCall() if err == nil { - t.Errorf("expected an error but error is: %v", err) + t.Errorf("expected an error") } - if err != nil && err.Error() != "invalid label format: :nokey" { + if err.Error() != "invalid label: :nokey (empty key)" { + t.Errorf("incorrect error: %v", err.Error()) + } +} + +func TestAddLabelTooManyColons(t *testing.T) { + fakeFS := fs.MakeFakeFS() + v := validators.MakeHappyMapValidator(t) + cmd := newCmdAddLabel(fakeFS, v.Validator) + args := []string{"key:v1:v2"} + err := cmd.RunE(cmd, args) + v.VerifyNoCall() + if err == nil { + t.Errorf("expected an error") + } + if err.Error() != "invalid label: key:v1:v2 (too many colons)" { t.Errorf("incorrect error: %v", err.Error()) } } @@ -173,9 +233,11 @@ func TestAddLabelNoKey(t *testing.T) { func TestAddLabelNoValue(t *testing.T) { fakeFS := fs.MakeFakeFS() fakeFS.WriteFile(constants.KustomizationFileName, []byte(kustomizationContent)) - cmd := newCmdAddLabel(fakeFS) + v := validators.MakeHappyMapValidator(t) + cmd := newCmdAddLabel(fakeFS, v.Validator) args := []string{"no,value:"} err := cmd.RunE(cmd, args) + v.VerifyCall() if err != nil { t.Errorf("unexpected error: %v", err.Error()) } @@ -184,13 +246,15 @@ func TestAddLabelNoValue(t *testing.T) { func TestAddLabelMultipleArgs(t *testing.T) { fakeFS := fs.MakeFakeFS() fakeFS.WriteFile(constants.KustomizationFileName, []byte(kustomizationContent)) - cmd := newCmdAddLabel(fakeFS) + v := validators.MakeHappyMapValidator(t) + cmd := newCmdAddLabel(fakeFS, v.Validator) args := []string{"this:input", "has:spaces"} err := cmd.RunE(cmd, args) + v.VerifyNoCall() if err == nil { - t.Errorf("expected an error but error is: %v", err) + t.Errorf("expected an error") } - if err != nil && err.Error() != "labels must be comma-separated, with no spaces. See help text for example" { + if err.Error() != "labels must be comma-separated, with no spaces" { t.Errorf("incorrect error: %v", err.Error()) } } diff --git a/pkg/commands/commands.go b/pkg/commands/commands.go index d75d34c38..f2b3e891d 100644 --- a/pkg/commands/commands.go +++ b/pkg/commands/commands.go @@ -22,6 +22,7 @@ import ( "os" "github.com/kubernetes-sigs/kustomize/pkg/fs" + "github.com/kubernetes-sigs/kustomize/pkg/validators" "github.com/spf13/cobra" ) @@ -109,8 +110,8 @@ func newCmdAdd(fsys fs.FileSystem) *cobra.Command { newCmdAddPatch(fsys), newCmdAddConfigMap(fsys), newCmdAddBase(fsys), - newCmdAddLabel(fsys), - newCmdAddAnnotation(fsys), + newCmdAddLabel(fsys, validators.MakeLabelValidator()), + newCmdAddAnnotation(fsys, validators.MakeAnnotationValidator()), ) return c } diff --git a/pkg/validators/validators.go b/pkg/validators/validators.go new file mode 100644 index 000000000..ca577b2f7 --- /dev/null +++ b/pkg/validators/validators.go @@ -0,0 +1,79 @@ +package validators + +import ( + "errors" + apivalidation "k8s.io/apimachinery/pkg/api/validation" + v1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation" + "k8s.io/apimachinery/pkg/util/validation/field" + "testing" +) + +// MapValidatorFunc returns an error if a map contains errors. +type MapValidatorFunc func(map[string]string) error + +// MakeAnnotationValidator returns a MapValidatorFunc using apimachinery. +func MakeAnnotationValidator() MapValidatorFunc { + return func(x map[string]string) error { + errs := apivalidation.ValidateAnnotations(x, field.NewPath("field")) + if errs != nil { + return errors.New(errs.ToAggregate().Error()) + } + return nil + } +} + +// MakeLabelValidator returns a MapValidatorFunc using apimachinery. +func MakeLabelValidator() MapValidatorFunc { + return func(x map[string]string) error { + errs := v1validation.ValidateLabels(x, field.NewPath("field")) + if errs != nil { + return errors.New(errs.ToAggregate().Error()) + } + return nil + } +} + +// FakeValidator can be used in tests. +type FakeValidator struct { + happy bool + called bool + t *testing.T +} + +// SAD is an error string. +const SAD = "i'm not happy Bob, NOT HAPPY" + +// MakeHappyMapValidator makes a FakeValidator that always passes. +func MakeHappyMapValidator(t *testing.T) *FakeValidator { + return &FakeValidator{happy: true, t: t} +} + +// MakeSadMapValidator makes a FakeValidator that always fails. +func MakeSadMapValidator(t *testing.T) *FakeValidator { + return &FakeValidator{happy: false, t: t} +} + +// Validator replaces apimachinery validation in tests. +// Can be set to fail or succeed to test error handling. +// Can confirm if run or not run by surrounding code. +func (v *FakeValidator) Validator(_ map[string]string) error { + v.called = true + if v.happy { + return nil + } + return errors.New(SAD) +} + +// VerifyCall returns true if Validator was used. +func (v *FakeValidator) VerifyCall() { + if !v.called { + v.t.Errorf("should have called Validator") + } +} + +// VerifyNoCall returns true if Validator was not used. +func (v *FakeValidator) VerifyNoCall() { + if v.called { + v.t.Errorf("should not have called Validator") + } +}