diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index ec11f35c7c5..348915e8432 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" @@ -66,11 +65,13 @@ func ValidateHasLabel(meta api.ObjectMeta, fldPath *field.Path, key, expectedVal allErrs := field.ErrorList{} actualValue, found := meta.Labels[key] if !found { - allErrs = append(allErrs, field.Required(fldPath.Child("labels"), key+"="+expectedValue)) + allErrs = append(allErrs, field.Required(fldPath.Child("labels").Key(key), + fmt.Sprintf("must be '%s'", expectedValue))) return allErrs } if actualValue != expectedValue { - allErrs = append(allErrs, field.Invalid(fldPath.Child("labels"), meta.Labels, "expected "+key+"="+expectedValue)) + allErrs = append(allErrs, field.Invalid(fldPath.Child("labels").Key(key), meta.Labels, + fmt.Sprintf("must be '%s'", expectedValue))) } return allErrs } @@ -91,6 +92,14 @@ func ValidateAnnotations(annotations map[string]string, fldPath *field.Path) fie return allErrs } +func ValidateDNS1123Label(value string, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + for _, msg := range validation.IsDNS1123Label(value) { + allErrs = append(allErrs, field.Invalid(fldPath, value, msg)) + } + return allErrs +} + func ValidatePodSpecificAnnotations(annotations map[string]string, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} if annotations[api.AffinityAnnotationKey] != "" { @@ -101,16 +110,12 @@ func ValidatePodSpecificAnnotations(annotations map[string]string, fldPath *fiel allErrs = append(allErrs, ValidateTolerationsInPodAnnotations(annotations, fldPath)...) } + // TODO: remove these after we EOL the annotations. if hostname, exists := annotations[utilpod.PodHostnameAnnotation]; exists { - for _, msg := range validation.IsDNS1123Label(hostname) { - allErrs = append(allErrs, field.Invalid(fldPath, utilpod.PodHostnameAnnotation, msg)) - } + allErrs = append(allErrs, ValidateDNS1123Label(hostname, fldPath.Key(utilpod.PodHostnameAnnotation))...) } - if subdomain, exists := annotations[utilpod.PodSubdomainAnnotation]; exists { - for _, msg := range validation.IsDNS1123Label(subdomain) { - allErrs = append(allErrs, field.Invalid(fldPath, utilpod.PodSubdomainAnnotation, msg)) - } + allErrs = append(allErrs, ValidateDNS1123Label(subdomain, fldPath.Key(utilpod.PodSubdomainAnnotation))...) } allErrs = append(allErrs, ValidateSeccompPodAnnotations(annotations, fldPath)...) @@ -120,6 +125,7 @@ func ValidatePodSpecificAnnotations(annotations map[string]string, fldPath *fiel func ValidateEndpointsSpecificAnnotations(annotations map[string]string, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} + // TODO: remove this after we EOL the annotation. hostnamesMap, exists := annotations[endpoints.PodHostnamesAnnotation] if exists && !isValidHostnamesMap(hostnamesMap) { allErrs = append(allErrs, field.Invalid(fldPath, endpoints.PodHostnamesAnnotation, @@ -396,15 +402,15 @@ func validateVolumes(volumes []api.Volume, fldPath *field.Path) (sets.String, fi allNames := sets.String{} for i, vol := range volumes { idxPath := fldPath.Index(i) + namePath := idxPath.Child("name") el := validateVolumeSource(&vol.VolumeSource, idxPath) if len(vol.Name) == 0 { - el = append(el, field.Required(idxPath.Child("name"), "")) - } else if msgs := validation.IsDNS1123Label(vol.Name); len(msgs) != 0 { - for i := range msgs { - el = append(el, field.Invalid(idxPath.Child("name"), vol.Name, msgs[i])) - } - } else if allNames.Has(vol.Name) { - el = append(el, field.Duplicate(idxPath.Child("name"), vol.Name)) + el = append(el, field.Required(namePath, "")) + } else { + el = append(el, ValidateDNS1123Label(vol.Name, namePath)...) + } + if allNames.Has(vol.Name) { + el = append(el, field.Duplicate(namePath, vol.Name)) } if len(el) == 0 { allNames.Insert(vol.Name) @@ -1202,8 +1208,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 @@ -1217,8 +1225,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 @@ -1448,14 +1458,14 @@ func validateContainers(containers []api.Container, volumes sets.String, fldPath allNames := sets.String{} for i, ctr := range containers { idxPath := fldPath.Index(i) + namePath := idxPath.Child("name") if len(ctr.Name) == 0 { - allErrs = append(allErrs, field.Required(idxPath.Child("name"), "")) - } else if msgs := validation.IsDNS1123Label(ctr.Name); len(msgs) != 0 { - for i := range msgs { - allErrs = append(allErrs, field.Invalid(idxPath.Child("name"), ctr.Name, msgs[i])) - } - } else if allNames.Has(ctr.Name) { - allErrs = append(allErrs, field.Duplicate(idxPath.Child("name"), ctr.Name)) + allErrs = append(allErrs, field.Required(namePath, "")) + } else { + allErrs = append(allErrs, ValidateDNS1123Label(ctr.Name, namePath)...) + } + if allNames.Has(ctr.Name) { + allErrs = append(allErrs, field.Duplicate(namePath, ctr.Name)) } else { allNames.Insert(ctr.Name) } @@ -1644,15 +1654,11 @@ func ValidatePodSpec(spec *api.PodSpec, fldPath *field.Path) field.ErrorList { } if len(spec.Hostname) > 0 { - for _, msg := range validation.IsDNS1123Label(spec.Hostname) { - allErrs = append(allErrs, field.Invalid(fldPath.Child("hostname"), spec.Hostname, msg)) - } + allErrs = append(allErrs, ValidateDNS1123Label(spec.Hostname, fldPath.Child("hostname"))...) } if len(spec.Subdomain) > 0 { - for _, msg := range validation.IsDNS1123Label(spec.Subdomain) { - allErrs = append(allErrs, field.Invalid(fldPath.Child("subdomain"), spec.Subdomain, msg)) - } + allErrs = append(allErrs, ValidateDNS1123Label(spec.Subdomain, fldPath.Child("subdomain"))...) } return allErrs @@ -2167,11 +2173,8 @@ func validateServicePort(sp *api.ServicePort, requireName, isHeadlessService boo if requireName && len(sp.Name) == 0 { allErrs = append(allErrs, field.Required(fldPath.Child("name"), "")) } else if len(sp.Name) != 0 { - if msgs := validation.IsDNS1123Label(sp.Name); len(msgs) != 0 { - for i := range msgs { - allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), sp.Name, msgs[i])) - } - } else if allNames.Has(sp.Name) { + allErrs = append(allErrs, ValidateDNS1123Label(sp.Name, fldPath.Child("name"))...) + if allNames.Has(sp.Name) { allErrs = append(allErrs, field.Duplicate(fldPath.Child("name"), sp.Name)) } else { allNames.Insert(sp.Name) @@ -2622,16 +2625,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")) @@ -2639,8 +2632,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) } @@ -2737,8 +2730,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) } @@ -3046,9 +3039,7 @@ func validateEndpointAddress(address *api.EndpointAddress, fldPath *field.Path) allErrs = append(allErrs, field.Invalid(fldPath.Child("ip"), address.IP, msg)) } if len(address.Hostname) > 0 { - for _, msg := range validation.IsDNS1123Label(address.Hostname) { - allErrs = append(allErrs, field.Invalid(fldPath.Child("hostname"), address.Hostname, msg)) - } + allErrs = append(allErrs, ValidateDNS1123Label(address.Hostname, fldPath.Child("hostname"))...) } if len(allErrs) > 0 { return allErrs @@ -3088,9 +3079,7 @@ func validateEndpointPort(port *api.EndpointPort, requireName bool, fldPath *fie if requireName && len(port.Name) == 0 { allErrs = append(allErrs, field.Required(fldPath.Child("name"), "")) } else if len(port.Name) != 0 { - for _, msg := range validation.IsDNS1123Label(port.Name) { - allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), port.Name, msg)) - } + allErrs = append(allErrs, ValidateDNS1123Label(port.Name, fldPath.Child("name"))...) } for _, msg := range validation.IsValidPortNum(int(port.Port)) { allErrs = append(allErrs, field.Invalid(fldPath.Child("port"), port.Port, msg)) @@ -3174,6 +3163,7 @@ func ValidateLoadBalancerStatus(status *api.LoadBalancerStatus, fldPath *field.P return allErrs } +// TODO: remove this after we EOL the annotation that carries it. func isValidHostnamesMap(serializedPodHostNames string) bool { if len(serializedPodHostNames) == 0 { return false diff --git a/pkg/kubectl/configmap.go b/pkg/kubectl/configmap.go index fcd3fc6b8eb..3e3a6c06ccb 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 85d264cbe11..cc3271197cd 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 ee3dba3caed..6e6a0270ca4 100644 --- a/pkg/util/validation/validation.go +++ b/pkg/util/validation/validation.go @@ -85,10 +85,10 @@ func IsValidLabelValue(value string) []string { return errs } -const DNS1123LabelFmt string = "[a-z0-9]([-a-z0-9]*[a-z0-9])?" +const dns1123LabelFmt string = "[a-z0-9]([-a-z0-9]*[a-z0-9])?" const DNS1123LabelMaxLength int = 63 -var dns1123LabelRegexp = regexp.MustCompile("^" + DNS1123LabelFmt + "$") +var dns1123LabelRegexp = regexp.MustCompile("^" + dns1123LabelFmt + "$") // IsDNS1123Label tests for a string that conforms to the definition of a label in // DNS (RFC 1123). @@ -98,15 +98,15 @@ func IsDNS1123Label(value string) []string { errs = append(errs, MaxLenError(DNS1123LabelMaxLength)) } if !dns1123LabelRegexp.MatchString(value) { - errs = append(errs, RegexError(DNS1123LabelFmt, "my-name", "123-abc")) + errs = append(errs, RegexError(dns1123LabelFmt, "my-name", "123-abc")) } return errs } -const DNS1123SubdomainFmt string = DNS1123LabelFmt + "(\\." + DNS1123LabelFmt + ")*" +const dns1123SubdomainFmt string = dns1123LabelFmt + "(\\." + dns1123LabelFmt + ")*" const DNS1123SubdomainMaxLength int = 253 -var dns1123SubdomainRegexp = regexp.MustCompile("^" + DNS1123SubdomainFmt + "$") +var dns1123SubdomainRegexp = regexp.MustCompile("^" + dns1123SubdomainFmt + "$") // IsDNS1123Subdomain tests for a string that conforms to the definition of a // subdomain in DNS (RFC 1123). @@ -116,15 +116,15 @@ func IsDNS1123Subdomain(value string) []string { errs = append(errs, MaxLenError(DNS1123SubdomainMaxLength)) } if !dns1123SubdomainRegexp.MatchString(value) { - errs = append(errs, RegexError(DNS1123SubdomainFmt, "example.com")) + errs = append(errs, RegexError(dns1123SubdomainFmt, "example.com")) } return errs } -const DNS952LabelFmt string = "[a-z]([-a-z0-9]*[a-z0-9])?" +const dns952LabelFmt string = "[a-z]([-a-z0-9]*[a-z0-9])?" const DNS952LabelMaxLength int = 24 -var dns952LabelRegexp = regexp.MustCompile("^" + DNS952LabelFmt + "$") +var dns952LabelRegexp = regexp.MustCompile("^" + dns952LabelFmt + "$") // IsDNS952Label tests for a string that conforms to the definition of a label in // DNS (RFC 952). @@ -134,20 +134,20 @@ func IsDNS952Label(value string) []string { errs = append(errs, MaxLenError(DNS952LabelMaxLength)) } if !dns952LabelRegexp.MatchString(value) { - errs = append(errs, RegexError(DNS952LabelFmt, "my-name", "abc-123")) + errs = append(errs, RegexError(dns952LabelFmt, "my-name", "abc-123")) } return errs } -const CIdentifierFmt string = "[A-Za-z_][A-Za-z0-9_]*" +const cIdentifierFmt string = "[A-Za-z_][A-Za-z0-9_]*" -var cIdentifierRegexp = regexp.MustCompile("^" + CIdentifierFmt + "$") +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) []string { if !cIdentifierRegexp.MatchString(value) { - return []string{RegexError(CIdentifierFmt, "my_name", "MY_NAME", "MyName")} + return []string{RegexError(cIdentifierFmt, "my_name", "MY_NAME", "MyName")} } return nil } @@ -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 fe4389cddbc..b799469450f 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) + } + } +}