Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Validation: Make validation func return error strings #21240

Merged
merged 4 commits into from
Jul 7, 2016
Merged

Validation: Make validation func return error strings #21240

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
766f44f
Move IsSecretKey into util/validation
thockin Apr 29, 2016
493b3fa
Unexport DNS regexps, lengths are still used
thockin Apr 29, 2016
09f1c97
Clean up ValidateHasLabel()
thockin Apr 29, 2016
2f3863f
Wrap DNS util/validation with api/validation
thockin Apr 29, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
108 changes: 49 additions & 59 deletions pkg/api/validation/validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"os"
"path"
"reflect"
"regexp"
"strings"

"github.com/golang/glog"
Expand Down Expand Up @@ -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
}
Expand All @@ -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] != "" {
Expand All @@ -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)...)
Expand All @@ -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,
Expand Down Expand Up @@ -391,15 +397,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)
Expand Down Expand Up @@ -1197,8 +1203,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
Expand All @@ -1212,8 +1220,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
Expand Down Expand Up @@ -1443,14 +1453,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)
}
Expand Down Expand Up @@ -1639,15 +1649,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
Expand Down Expand Up @@ -2162,11 +2168,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)
Expand Down Expand Up @@ -2617,25 +2620,15 @@ 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"))

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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -3041,9 +3034,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
Expand Down Expand Up @@ -3083,9 +3074,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))
Expand Down Expand Up @@ -3169,6 +3158,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
Expand Down
9 changes: 4 additions & 5 deletions pkg/kubectl/configmap.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down
7 changes: 4 additions & 3 deletions pkg/kubectl/secret.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Expand Down
41 changes: 29 additions & 12 deletions pkg/util/validation/validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand All @@ -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).
Expand All @@ -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).
Expand All @@ -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
}
Expand Down Expand Up @@ -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 {
Expand Down