From 189d4a5159e8f725c09b1bed2d75c03f11561842 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Sat, 19 Dec 2015 22:52:48 -0800 Subject: [PATCH] Make CIdentifier return error strings --- pkg/api/validation/validation.go | 7 ++++--- pkg/api/validation/validation_test.go | 2 +- pkg/kubectl/run.go | 7 +++++-- pkg/util/validation/validation.go | 7 +++++-- pkg/util/validation/validation_test.go | 6 +++--- 5 files changed, 18 insertions(+), 11 deletions(-) diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index 5b9b739f6b..6b6b75a90b 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -51,7 +51,6 @@ var RepairMalformedUpdates bool = true const isNegativeErrorMsg string = `must be greater than or equal to 0` const isInvalidQuotaResource string = `must be a standard resource for quota` const fieldImmutableErrorMsg string = `field is immutable` -const cIdentifierErrorMsg string = `must be a C identifier (matching regex ` + validation.CIdentifierFmt + `): e.g. "my_name" or "MyName"` const isNotIntegerErrorMsg string = `must be an integer` func InclusiveRangeErrorMsg(lo, hi int) string { @@ -1087,8 +1086,10 @@ func validateEnv(vars []api.EnvVar, fldPath *field.Path) field.ErrorList { idxPath := fldPath.Index(i) if len(ev.Name) == 0 { allErrs = append(allErrs, field.Required(idxPath.Child("name"), "")) - } else if !validation.IsCIdentifier(ev.Name) { - allErrs = append(allErrs, field.Invalid(idxPath.Child("name"), ev.Name, cIdentifierErrorMsg)) + } else { + for _, msg := range validation.IsCIdentifier(ev.Name) { + allErrs = append(allErrs, field.Invalid(idxPath.Child("name"), ev.Name, msg)) + } } allErrs = append(allErrs, validateEnvVarValueFrom(ev, idxPath.Child("valueFrom"))...) } diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index 2781a1bdb4..25c2f93187 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -1155,7 +1155,7 @@ func TestValidateEnv(t *testing.T) { { name: "name not a C identifier", envs: []api.EnvVar{{Name: "a.b.c"}}, - expectedError: `[0].name: Invalid value: "a.b.c": must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName"`, + expectedError: `[0].name: Invalid value: "a.b.c": must match the regex`, }, { name: "value and valueFrom specified", diff --git a/pkg/kubectl/run.go b/pkg/kubectl/run.go index 1ce9f890ef..0b89b7814f 100644 --- a/pkg/kubectl/run.go +++ b/pkg/kubectl/run.go @@ -835,7 +835,10 @@ func parseEnvs(envArray []string) ([]api.EnvVar, error) { } name := env[:pos] value := env[pos+1:] - if len(name) == 0 || !validation.IsCIdentifier(name) || len(value) == 0 { + if len(name) == 0 || len(value) == 0 { + return nil, fmt.Errorf("invalid env: %v", env) + } + if len(validation.IsCIdentifier(name)) != 0 { return nil, fmt.Errorf("invalid env: %v", env) } envVar := api.EnvVar{Name: name, Value: value} @@ -853,7 +856,7 @@ func parseV1Envs(envArray []string) ([]v1.EnvVar, error) { } name := env[:pos] value := env[pos+1:] - if len(name) == 0 || !validation.IsCIdentifier(name) || len(value) == 0 { + if len(name) == 0 || len(validation.IsCIdentifier(name)) != 0 || len(value) == 0 { return nil, fmt.Errorf("invalid env: %v", env) } envVar := v1.EnvVar{Name: name, Value: value} diff --git a/pkg/util/validation/validation.go b/pkg/util/validation/validation.go index 135c5e9082..bdb99b1e32 100644 --- a/pkg/util/validation/validation.go +++ b/pkg/util/validation/validation.go @@ -145,8 +145,11 @@ var cIdentifierRegexp = regexp.MustCompile("^" + CIdentifierFmt + "$") // IsCIdentifier tests for a string that conforms the definition of an identifier // in C. This checks the format, but not the length. -func IsCIdentifier(value string) bool { - return cIdentifierRegexp.MatchString(value) +func IsCIdentifier(value string) []string { + if !cIdentifierRegexp.MatchString(value) { + return []string{RegexError(CIdentifierFmt, "my_name", "MY_NAME", "MyName")} + } + return nil } // IsValidPortNum tests that the argument is a valid, non-zero port number. diff --git a/pkg/util/validation/validation_test.go b/pkg/util/validation/validation_test.go index 6af22f5b1d..6b24835123 100644 --- a/pkg/util/validation/validation_test.go +++ b/pkg/util/validation/validation_test.go @@ -119,8 +119,8 @@ func TestIsCIdentifier(t *testing.T) { "A", "AB", "AbC", "A1", "_A", "A_", "A_B", "A_1", "A__1__2__B", "__123_ABC", } for _, val := range goodValues { - if !IsCIdentifier(val) { - t.Errorf("expected true for '%s'", val) + if msgs := IsCIdentifier(val); len(msgs) != 0 { + t.Errorf("expected true for '%s': %v", val, msgs) } } @@ -132,7 +132,7 @@ func TestIsCIdentifier(t *testing.T) { "#a#", } for _, val := range badValues { - if IsCIdentifier(val) { + if msgs := IsCIdentifier(val); len(msgs) == 0 { t.Errorf("expected false for '%s'", val) } }