diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index 8553c82b75..b983c60a71 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -23,7 +23,6 @@ import ( "os" "path" "reflect" - "regexp" "strings" "github.com/golang/glog" @@ -1197,8 +1196,10 @@ func validateConfigMapKeySelector(s *api.ConfigMapKeySelector, fldPath *field.Pa } if len(s.Key) == 0 { allErrs = append(allErrs, field.Required(fldPath.Child("key"), "")) - } else if !IsSecretKey(s.Key) { - allErrs = append(allErrs, field.Invalid(fldPath.Child("key"), s.Key, fmt.Sprintf("must have at most %d characters and match regex %s", validation.DNS1123SubdomainMaxLength, SecretKeyFmt))) + } else { + for _, msg := range validation.IsConfigMapKey(s.Key) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("key"), s.Key, msg)) + } } return allErrs @@ -1212,8 +1213,10 @@ func validateSecretKeySelector(s *api.SecretKeySelector, fldPath *field.Path) fi } if len(s.Key) == 0 { allErrs = append(allErrs, field.Required(fldPath.Child("key"), "")) - } else if !IsSecretKey(s.Key) { - allErrs = append(allErrs, field.Invalid(fldPath.Child("key"), s.Key, fmt.Sprintf("must have at most %d characters and match regex %s", validation.DNS1123SubdomainMaxLength, SecretKeyFmt))) + } else { + for _, msg := range validation.IsConfigMapKey(s.Key) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("key"), s.Key, msg)) + } } return allErrs @@ -2617,16 +2620,6 @@ func ValidateServiceAccountUpdate(newServiceAccount, oldServiceAccount *api.Serv return allErrs } -const SecretKeyFmt string = "\\.?" + validation.DNS1123LabelFmt + "(\\." + validation.DNS1123LabelFmt + ")*" - -var secretKeyRegexp = regexp.MustCompile("^" + SecretKeyFmt + "$") - -// IsSecretKey tests for a string that conforms to the definition of a -// subdomain in DNS (RFC 1123), except that a leading dot is allowed -func IsSecretKey(value string) bool { - return len(value) <= validation.DNS1123SubdomainMaxLength && secretKeyRegexp.MatchString(value) -} - // ValidateSecret tests if required fields in the Secret are set. func ValidateSecret(secret *api.Secret) field.ErrorList { allErrs := ValidateObjectMeta(&secret.ObjectMeta, true, ValidateSecretName, field.NewPath("metadata")) @@ -2634,8 +2627,8 @@ func ValidateSecret(secret *api.Secret) field.ErrorList { dataPath := field.NewPath("data") totalSize := 0 for key, value := range secret.Data { - if !IsSecretKey(key) { - allErrs = append(allErrs, field.Invalid(dataPath.Key(key), key, fmt.Sprintf("must have at most %d characters and match regex %s", validation.DNS1123SubdomainMaxLength, SecretKeyFmt))) + for _, msg := range validation.IsConfigMapKey(key) { + allErrs = append(allErrs, field.Invalid(dataPath.Key(key), key, msg)) } totalSize += len(value) } @@ -2732,8 +2725,8 @@ func ValidateConfigMap(cfg *api.ConfigMap) field.ErrorList { totalSize := 0 for key, value := range cfg.Data { - if !IsSecretKey(key) { - allErrs = append(allErrs, field.Invalid(field.NewPath("data").Key(key), key, fmt.Sprintf("must have at most %d characters and match regex %s", validation.DNS1123SubdomainMaxLength, SecretKeyFmt))) + for _, msg := range validation.IsConfigMapKey(key) { + allErrs = append(allErrs, field.Invalid(field.NewPath("data").Key(key), key, msg)) } totalSize += len(value) } diff --git a/pkg/kubectl/configmap.go b/pkg/kubectl/configmap.go index fcd3fc6b8e..3e3a6c06cc 100644 --- a/pkg/kubectl/configmap.go +++ b/pkg/kubectl/configmap.go @@ -24,8 +24,8 @@ import ( "strings" "k8s.io/kubernetes/pkg/api" - "k8s.io/kubernetes/pkg/api/validation" "k8s.io/kubernetes/pkg/runtime" + "k8s.io/kubernetes/pkg/util/validation" ) // ConfigMapGeneratorV1 supports stable generation of a configMap. @@ -199,10 +199,9 @@ func addKeyFromFileToConfigMap(configMap *api.ConfigMap, keyName, filePath strin // addKeyFromLiteralToConfigMap adds the given key and data to the given config map, // returning an error if the key is not valid or if the key already exists. func addKeyFromLiteralToConfigMap(configMap *api.ConfigMap, keyName, data string) error { - // Note, the rules for ConfigMap keys are the exact same as the ones for SecretKeys - // to be consistent; validation.IsSecretKey is used here intentionally. - if !validation.IsSecretKey(keyName) { - return fmt.Errorf("%v is not a valid key name for a configMap", keyName) + // Note, the rules for ConfigMap keys are the exact same as the ones for SecretKeys. + if errs := validation.IsConfigMapKey(keyName); len(errs) != 0 { + return fmt.Errorf("%q is not a valid key name for a ConfigMap: %s", keyName, strings.Join(errs, ";")) } if _, entryExists := configMap.Data[keyName]; entryExists { return fmt.Errorf("cannot add key %s, another key by that name already exists: %v.", keyName, configMap.Data) diff --git a/pkg/kubectl/secret.go b/pkg/kubectl/secret.go index 85d264cbe1..cc3271197c 100644 --- a/pkg/kubectl/secret.go +++ b/pkg/kubectl/secret.go @@ -24,8 +24,8 @@ import ( "strings" "k8s.io/kubernetes/pkg/api" - "k8s.io/kubernetes/pkg/api/validation" "k8s.io/kubernetes/pkg/runtime" + "k8s.io/kubernetes/pkg/util/validation" ) // SecretGeneratorV1 supports stable generation of an opaque secret @@ -196,9 +196,10 @@ func addKeyFromFileToSecret(secret *api.Secret, keyName, filePath string) error } func addKeyFromLiteralToSecret(secret *api.Secret, keyName string, data []byte) error { - if !validation.IsSecretKey(keyName) { - return fmt.Errorf("%v is not a valid key name for a secret", keyName) + if errs := validation.IsConfigMapKey(keyName); len(errs) != 0 { + return fmt.Errorf("%q is not a valid key name for a Secret: %s", keyName, strings.Join(errs, ";")) } + if _, entryExists := secret.Data[keyName]; entryExists { return fmt.Errorf("cannot add key %s, another key by that name already exists: %v.", keyName, secret.Data) } diff --git a/pkg/util/validation/validation.go b/pkg/util/validation/validation.go index ee3dba3cae..d1e9f800fa 100644 --- a/pkg/util/validation/validation.go +++ b/pkg/util/validation/validation.go @@ -247,6 +247,23 @@ func IsHTTPHeaderName(value string) []string { return nil } +const configMapKeyFmt = "\\.?" + DNS1123SubdomainFmt + +var configMapKeyRegexp = regexp.MustCompile("^" + configMapKeyFmt + "$") + +// IsConfigMapKey tests for a string that conforms to the definition of a +// subdomain in DNS (RFC 1123), except that a leading dot is allowed +func IsConfigMapKey(value string) []string { + var errs []string + if len(value) > DNS1123SubdomainMaxLength { + errs = append(errs, MaxLenError(DNS1123SubdomainMaxLength)) + } + if !configMapKeyRegexp.MatchString(value) { + errs = append(errs, RegexError(configMapKeyFmt, "key.name")) + } + return errs +} + // MaxLenError returns a string explanation of a "string too long" validation // failure. func MaxLenError(length int) string { diff --git a/pkg/util/validation/validation_test.go b/pkg/util/validation/validation_test.go index fe4389cddb..b799469450 100644 --- a/pkg/util/validation/validation_test.go +++ b/pkg/util/validation/validation_test.go @@ -374,3 +374,31 @@ func TestIsValidPercent(t *testing.T) { } } } + +func TestIsConfigMapKey(t *testing.T) { + successCases := []string{ + "good", + "good-good", + "still.good", + "this.is.also.good", + ".so.is.this", + } + + for i := range successCases { + if errs := IsConfigMapKey(successCases[i]); len(errs) != 0 { + t.Errorf("[%d] expected success: %v", i, errs) + } + } + + failureCases := []string{ + "bad_bad", + "..bad", + "bad.", + } + + for i := range failureCases { + if errs := IsConfigMapKey(failureCases[i]); len(errs) == 0 { + t.Errorf("[%d] expected success", i) + } + } +}