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

Add more information to Validator error messages. #4174

Merged
merged 1 commit into from
Feb 9, 2015
Merged

Add more information to Validator error messages. #4174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
82 changes: 49 additions & 33 deletions pkg/api/validation/validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,26 @@ import (
"github.com/golang/glog"
)

const qualifiedNameErrorMsg string = "must match regex [" + util.DNS1123SubdomainFmt + " / ] " + util.DNS1123LabelFmt
const cIdentifierErrorMsg string = "must match regex " + util.CIdentifierFmt
const isNegativeErrorMsg string = "value must not be negative"

func intervalErrorMsg(lo, hi int) string {
return fmt.Sprintf("must be greater than %d and less than %d", lo, hi)
}

var dnsSubdomainErrorMsg string = fmt.Sprintf("must have at most %d characters and match regex %s", util.DNS1123SubdomainMaxLength, util.DNS1123SubdomainFmt)
var dnsLabelErrorMsg string = fmt.Sprintf("must have at most %d characters and match regex %s", util.DNS1123LabelMaxLength, util.DNS1123LabelFmt)
var dns952LabelErrorMsg string = fmt.Sprintf("must have at most %d characters and match regex %s", util.DNS952LabelMaxLength, util.DNS952LabelFmt)
var pdPartitionErrorMsg string = intervalErrorMsg(0, 255)
var portRangeErrorMsg string = intervalErrorMsg(0, 65536)

// ValidateLabels validates that a set of labels are correctly defined.
func ValidateLabels(labels map[string]string, field string) errs.ValidationErrorList {
allErrs := errs.ValidationErrorList{}
for k := range labels {
if !util.IsQualifiedName(k) {
allErrs = append(allErrs, errs.NewFieldInvalid(field, k, ""))
allErrs = append(allErrs, errs.NewFieldInvalid(field, k, qualifiedNameErrorMsg))
}
}
return allErrs
Expand All @@ -46,7 +60,7 @@ func ValidateAnnotations(annotations map[string]string, field string) errs.Valid
allErrs := errs.ValidationErrorList{}
for k := range annotations {
if !util.IsQualifiedName(strings.ToLower(k)) {
allErrs = append(allErrs, errs.NewFieldInvalid(field, k, ""))
allErrs = append(allErrs, errs.NewFieldInvalid(field, k, qualifiedNameErrorMsg))
}
}
return allErrs
Expand Down Expand Up @@ -103,7 +117,7 @@ func nameIsDNSSubdomain(name string, prefix bool) (bool, string) {
if util.IsDNSSubdomain(name) {
return true, ""
}
return false, "name must be lowercase letters and numbers, with inline dashes or periods"
return false, dnsSubdomainErrorMsg
}

// nameIsDNS952Label is a ValidateNameFunc for names that must be a DNS 952 label.
Expand All @@ -114,7 +128,7 @@ func nameIsDNS952Label(name string, prefix bool) (bool, string) {
if util.IsDNS952Label(name) {
return true, ""
}
return false, "name must be lowercase letters, numbers, and dashes"
return false, dns952LabelErrorMsg
}

// ValidateObjectMeta validates an object's metadata on creation. It expects that name generation has already
Expand All @@ -141,7 +155,7 @@ func ValidateObjectMeta(meta *api.ObjectMeta, requiresNamespace bool, nameFn Val
if len(meta.Namespace) == 0 {
allErrs = append(allErrs, errs.NewFieldRequired("namespace", meta.Namespace))
} else if !util.IsDNSSubdomain(meta.Namespace) {
allErrs = append(allErrs, errs.NewFieldInvalid("namespace", meta.Namespace, ""))
allErrs = append(allErrs, errs.NewFieldInvalid("namespace", meta.Namespace, dnsSubdomainErrorMsg))
}
} else {
if len(meta.Namespace) != 0 {
Expand Down Expand Up @@ -194,7 +208,7 @@ func validateVolumes(volumes []api.Volume) (util.StringSet, errs.ValidationError
if len(vol.Name) == 0 {
el = append(el, errs.NewFieldRequired("name", vol.Name))
} else if !util.IsDNSLabel(vol.Name) {
el = append(el, errs.NewFieldInvalid("name", vol.Name, ""))
el = append(el, errs.NewFieldInvalid("name", vol.Name, dnsLabelErrorMsg))
} else if allNames.Has(vol.Name) {
el = append(el, errs.NewFieldDuplicate("name", vol.Name))
}
Expand Down Expand Up @@ -257,7 +271,7 @@ func validateGCEPersistentDisk(PD *api.GCEPersistentDisk) errs.ValidationErrorLi
allErrs = append(allErrs, errs.NewFieldRequired("fsType", PD.FSType))
}
if PD.Partition < 0 || PD.Partition > 255 {
allErrs = append(allErrs, errs.NewFieldInvalid("partition", PD.Partition, ""))
allErrs = append(allErrs, errs.NewFieldInvalid("partition", PD.Partition, pdPartitionErrorMsg))
}
return allErrs
}
Expand All @@ -271,21 +285,21 @@ func validatePorts(ports []api.Port) errs.ValidationErrorList {
for i, port := range ports {
pErrs := errs.ValidationErrorList{}
if len(port.Name) > 0 {
if len(port.Name) > 63 || !util.IsDNSLabel(port.Name) {
pErrs = append(pErrs, errs.NewFieldInvalid("name", port.Name, ""))
if len(port.Name) > util.DNS1123LabelMaxLength || !util.IsDNSLabel(port.Name) {
pErrs = append(pErrs, errs.NewFieldInvalid("name", port.Name, dnsLabelErrorMsg))
} else if allNames.Has(port.Name) {
pErrs = append(pErrs, errs.NewFieldDuplicate("name", port.Name))
} else {
allNames.Insert(port.Name)
}
}
if port.ContainerPort == 0 {
pErrs = append(pErrs, errs.NewFieldRequired("containerPort", port.ContainerPort))
pErrs = append(pErrs, errs.NewFieldInvalid("containerPort", port.ContainerPort, portRangeErrorMsg))
} else if !util.IsValidPortNum(port.ContainerPort) {
pErrs = append(pErrs, errs.NewFieldInvalid("containerPort", port.ContainerPort, ""))
pErrs = append(pErrs, errs.NewFieldInvalid("containerPort", port.ContainerPort, portRangeErrorMsg))
}
if port.HostPort != 0 && !util.IsValidPortNum(port.HostPort) {
pErrs = append(pErrs, errs.NewFieldInvalid("hostPort", port.HostPort, ""))
pErrs = append(pErrs, errs.NewFieldInvalid("hostPort", port.HostPort, portRangeErrorMsg))
}
if len(port.Protocol) == 0 {
pErrs = append(pErrs, errs.NewFieldRequired("protocol", port.Protocol))
Expand All @@ -306,7 +320,7 @@ func validateEnv(vars []api.EnvVar) errs.ValidationErrorList {
vErrs = append(vErrs, errs.NewFieldRequired("name", ev.Name))
}
if !util.IsCIdentifier(ev.Name) {
vErrs = append(vErrs, errs.NewFieldInvalid("name", ev.Name, ""))
vErrs = append(vErrs, errs.NewFieldInvalid("name", ev.Name, cIdentifierErrorMsg))
}
allErrs = append(allErrs, vErrs.PrefixIndex(i)...)
}
Expand Down Expand Up @@ -430,7 +444,7 @@ func validateContainers(containers []api.Container, volumes util.StringSet) errs
if len(ctr.Name) == 0 {
cErrs = append(cErrs, errs.NewFieldRequired("name", ctr.Name))
} else if !util.IsDNSLabel(ctr.Name) {
cErrs = append(cErrs, errs.NewFieldInvalid("name", ctr.Name, ""))
cErrs = append(cErrs, errs.NewFieldInvalid("name", ctr.Name, dnsLabelErrorMsg))
} else if allNames.Has(ctr.Name) {
cErrs = append(cErrs, errs.NewFieldDuplicate("name", ctr.Name))
} else if ctr.Privileged && !capabilities.AllowPrivileged {
Expand Down Expand Up @@ -574,7 +588,7 @@ func ValidateService(service *api.Service) errs.ValidationErrorList {
allErrs = append(allErrs, ValidateObjectMeta(&service.ObjectMeta, true, ValidateServiceName).Prefix("metadata")...)

if !util.IsValidPortNum(service.Spec.Port) {
allErrs = append(allErrs, errs.NewFieldInvalid("spec.port", service.Spec.Port, ""))
allErrs = append(allErrs, errs.NewFieldInvalid("spec.port", service.Spec.Port, portRangeErrorMsg))
}
if len(service.Spec.Protocol) == 0 {
allErrs = append(allErrs, errs.NewFieldRequired("spec.protocol", service.Spec.Protocol))
Expand Down Expand Up @@ -635,7 +649,7 @@ func ValidateReplicationControllerSpec(spec *api.ReplicationControllerSpec) errs
allErrs = append(allErrs, errs.NewFieldRequired("selector", spec.Selector))
}
if spec.Replicas < 0 {
allErrs = append(allErrs, errs.NewFieldInvalid("replicas", spec.Replicas, ""))
allErrs = append(allErrs, errs.NewFieldInvalid("replicas", spec.Replicas, isNegativeErrorMsg))
}

if spec.Template == nil {
Expand Down Expand Up @@ -694,7 +708,7 @@ func ValidateBoundPod(pod *api.BoundPod) errs.ValidationErrorList {
if len(pod.Namespace) == 0 {
allErrs = append(allErrs, errs.NewFieldRequired("namespace", pod.Namespace))
} else if !util.IsDNSSubdomain(pod.Namespace) {
allErrs = append(allErrs, errs.NewFieldInvalid("namespace", pod.Namespace, ""))
allErrs = append(allErrs, errs.NewFieldInvalid("namespace", pod.Namespace, dnsSubdomainErrorMsg))
}
allErrs = append(allErrs, ValidatePodSpec(&pod.Spec).Prefix("spec")...)
return allErrs
Expand Down Expand Up @@ -726,6 +740,7 @@ func ValidateMinionUpdate(oldMinion *api.Node, minion *api.Node) errs.Validation
// Clear status
oldMinion.Status = minion.Status

// TODO: Add a 'real' ValidationError type for this error and provide print actual diffs.
if !api.Semantic.DeepEqual(oldMinion, minion) {
glog.V(4).Infof("Update failed validation %#v vs %#v", oldMinion, minion)
allErrs = append(allErrs, fmt.Errorf("update contains more than labels or capacity changes"))
Expand All @@ -737,14 +752,15 @@ func ValidateMinionUpdate(oldMinion *api.Node, minion *api.Node) errs.Validation

// Validate compute resource typename.
// Refer to docs/resources.md for more details.
func validateResourceName(str string) errs.ValidationErrorList {
if !util.IsQualifiedName(str) {
return errs.ValidationErrorList{fmt.Errorf("invalid compute resource typename format %q", str)}
func validateResourceName(value string, field string) errs.ValidationErrorList {
allErrs := errs.ValidationErrorList{}
if !util.IsQualifiedName(value) {
return append(allErrs, errs.NewFieldInvalid(field, value, "resource typename: "+qualifiedNameErrorMsg))
}

if len(strings.Split(str, "/")) == 1 {
if !api.IsStandardResourceName(str) {
return errs.ValidationErrorList{fmt.Errorf("invalid compute resource typename. %q is neither a standard resource type nor is fully qualified", str)}
if len(strings.Split(value, "/")) == 1 {
if !api.IsStandardResourceName(value) {
return append(allErrs, errs.NewFieldInvalid(field, value, "is neither a standard resource type nor is fully qualified"))
}
}

Expand All @@ -757,21 +773,21 @@ func ValidateLimitRange(limitRange *api.LimitRange) errs.ValidationErrorList {
if len(limitRange.Name) == 0 {
allErrs = append(allErrs, errs.NewFieldRequired("name", limitRange.Name))
} else if !util.IsDNSSubdomain(limitRange.Name) {
allErrs = append(allErrs, errs.NewFieldInvalid("name", limitRange.Name, ""))
allErrs = append(allErrs, errs.NewFieldInvalid("name", limitRange.Name, dnsSubdomainErrorMsg))
}
if len(limitRange.Namespace) == 0 {
allErrs = append(allErrs, errs.NewFieldRequired("namespace", limitRange.Namespace))
} else if !util.IsDNSSubdomain(limitRange.Namespace) {
allErrs = append(allErrs, errs.NewFieldInvalid("namespace", limitRange.Namespace, ""))
allErrs = append(allErrs, errs.NewFieldInvalid("namespace", limitRange.Namespace, dnsSubdomainErrorMsg))
}
// ensure resource names are properly qualified per docs/resources.md
for i := range limitRange.Spec.Limits {
limit := limitRange.Spec.Limits[i]
for k := range limit.Max {
allErrs = append(allErrs, validateResourceName(string(k))...)
allErrs = append(allErrs, validateResourceName(string(k), fmt.Sprintf("spec.limits[%d].max[%s]", i, k))...)
}
for k := range limit.Min {
allErrs = append(allErrs, validateResourceName(string(k))...)
allErrs = append(allErrs, validateResourceName(string(k), fmt.Sprintf("spec.limits[%d].min[%s]", i, k))...)
}
}
return allErrs
Expand All @@ -789,7 +805,7 @@ func validateResourceRequirements(container *api.Container) errs.ValidationError
allErrs := errs.ValidationErrorList{}
for resourceName, quantity := range container.Resources.Limits {
// Validate resource name.
errs := validateResourceName(resourceName.String())
errs := validateResourceName(resourceName.String(), fmt.Sprintf("resources.limits[%s]", resourceName))
if api.IsStandardResourceName(resourceName.String()) {
errs = append(errs, validateBasicResource(quantity).Prefix(fmt.Sprintf("Resource %s: ", resourceName))...)
}
Expand All @@ -805,21 +821,21 @@ func ValidateResourceQuota(resourceQuota *api.ResourceQuota) errs.ValidationErro
if len(resourceQuota.Name) == 0 {
allErrs = append(allErrs, errs.NewFieldRequired("name", resourceQuota.Name))
} else if !util.IsDNSSubdomain(resourceQuota.Name) {
allErrs = append(allErrs, errs.NewFieldInvalid("name", resourceQuota.Name, ""))
allErrs = append(allErrs, errs.NewFieldInvalid("name", resourceQuota.Name, dnsSubdomainErrorMsg))
}
if len(resourceQuota.Namespace) == 0 {
allErrs = append(allErrs, errs.NewFieldRequired("namespace", resourceQuota.Namespace))
} else if !util.IsDNSSubdomain(resourceQuota.Namespace) {
allErrs = append(allErrs, errs.NewFieldInvalid("namespace", resourceQuota.Namespace, ""))
allErrs = append(allErrs, errs.NewFieldInvalid("namespace", resourceQuota.Namespace, dnsSubdomainErrorMsg))
}
for k := range resourceQuota.Spec.Hard {
allErrs = append(allErrs, validateResourceName(string(k))...)
allErrs = append(allErrs, validateResourceName(string(k), string(resourceQuota.TypeMeta.Kind))...)
}
for k := range resourceQuota.Status.Hard {
allErrs = append(allErrs, validateResourceName(string(k))...)
allErrs = append(allErrs, validateResourceName(string(k), string(resourceQuota.TypeMeta.Kind))...)
}
for k := range resourceQuota.Status.Used {
allErrs = append(allErrs, validateResourceName(string(k))...)
allErrs = append(allErrs, validateResourceName(string(k), string(resourceQuota.TypeMeta.Kind))...)
}
return allErrs
}