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.

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

Make DNS validators return error strings #24801

Merged
merged 3 commits into from
May 19, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion cluster/addons/dns/kube2sky/kube2sky.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ func getHostname(address *kapi.EndpointAddress, podHostnames map[string]endpoint
if len(address.Hostname) > 0 {
return address.Hostname, true
}
if hostRecord, exists := podHostnames[address.IP]; exists && validation.IsDNS1123Label(hostRecord.HostName) {
if hostRecord, exists := podHostnames[address.IP]; exists && len(validation.IsDNS1123Label(hostRecord.HostName)) == 0 {
return hostRecord.HostName, true
}
return "", false
Expand Down
4 changes: 2 additions & 2 deletions pkg/api/validation/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ func ValidateEvent(event *api.Event) field.ErrorList {
event.Namespace != event.InvolvedObject.Namespace {
allErrs = append(allErrs, field.Invalid(field.NewPath("involvedObject", "namespace"), event.InvolvedObject.Namespace, "does not match involvedObject"))
}
if !validation.IsDNS1123Subdomain(event.Namespace) {
allErrs = append(allErrs, field.Invalid(field.NewPath("namespace"), event.Namespace, ""))
for _, msg := range validation.IsDNS1123Subdomain(event.Namespace) {
allErrs = append(allErrs, field.Invalid(field.NewPath("namespace"), event.Namespace, msg))
}
return allErrs
}
76 changes: 40 additions & 36 deletions pkg/api/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,6 @@ func InclusiveRangeErrorMsg(lo, hi int) string {
return fmt.Sprintf(`must be between %d and %d, inclusive`, lo, hi)
}

var DNSSubdomainErrorMsg string = fmt.Sprintf(`must be a DNS subdomain (at most %d characters, matching regex %s): e.g. "example.com"`, validation.DNS1123SubdomainMaxLength, validation.DNS1123SubdomainFmt)
var DNS1123LabelErrorMsg string = fmt.Sprintf(`must be a DNS label (at most %d characters, matching regex %s): e.g. "my-name"`, validation.DNS1123LabelMaxLength, validation.DNS1123LabelFmt)
var DNS952LabelErrorMsg string = fmt.Sprintf(`must be a DNS 952 label (at most %d characters, matching regex %s): e.g. "my-name"`, validation.DNS952LabelMaxLength, validation.DNS952LabelFmt)
var pdPartitionErrorMsg string = InclusiveRangeErrorMsg(1, 255)
var PortRangeErrorMsg string = InclusiveRangeErrorMsg(1, 65535)
var IdRangeErrorMsg string = InclusiveRangeErrorMsg(0, math.MaxInt32)
Expand Down Expand Up @@ -113,12 +110,16 @@ func ValidatePodSpecificAnnotations(annotations map[string]string, fldPath *fiel
allErrs = append(allErrs, ValidateTolerationsInPodAnnotations(annotations, fldPath)...)
}

if hostname, exists := annotations[utilpod.PodHostnameAnnotation]; exists && !validation.IsDNS1123Label(hostname) {
allErrs = append(allErrs, field.Invalid(fldPath, utilpod.PodHostnameAnnotation, DNS1123LabelErrorMsg))
if hostname, exists := annotations[utilpod.PodHostnameAnnotation]; exists {
for _, msg := range validation.IsDNS1123Label(hostname) {
allErrs = append(allErrs, field.Invalid(fldPath, utilpod.PodHostnameAnnotation, msg))
}
}

if subdomain, exists := annotations[utilpod.PodSubdomainAnnotation]; exists && !validation.IsDNS1123Label(subdomain) {
allErrs = append(allErrs, field.Invalid(fldPath, utilpod.PodSubdomainAnnotation, DNS1123LabelErrorMsg))
if subdomain, exists := annotations[utilpod.PodSubdomainAnnotation]; exists {
for _, msg := range validation.IsDNS1123Label(subdomain) {
allErrs = append(allErrs, field.Invalid(fldPath, utilpod.PodSubdomainAnnotation, msg))
}
}

return allErrs
Expand Down Expand Up @@ -167,7 +168,7 @@ func ValidateOwnerReferences(ownerReferences []api.OwnerReference, fldPath *fiel

// ValidateNameFunc validates that the provided name is valid for a given resource type.
// Not all resources have the same validation rules for names. Prefix is true
// if the name will have a value appended to it. If the names is not valid,
// if the name will have a value appended to it. If the name is not valid,
// this returns a list of descriptions of individual characteristics of the
// value that were not valid. Otherwise this returns an empty list or nil.
type ValidateNameFunc func(name string, prefix bool) []string
Expand Down Expand Up @@ -238,32 +239,23 @@ func NameIsDNSSubdomain(name string, prefix bool) []string {
if prefix {
name = maskTrailingDash(name)
}
if validation.IsDNS1123Subdomain(name) {
return nil
}
return []string{DNSSubdomainErrorMsg}
return validation.IsDNS1123Subdomain(name)
}

// NameIsDNSLabel is a ValidateNameFunc for names that must be a DNS 1123 label.
func NameIsDNSLabel(name string, prefix bool) []string {
if prefix {
name = maskTrailingDash(name)
}
if validation.IsDNS1123Label(name) {
return nil
}
return []string{DNS1123LabelErrorMsg}
return validation.IsDNS1123Label(name)
}

// NameIsDNS952Label is a ValidateNameFunc for names that must be a DNS 952 label.
func NameIsDNS952Label(name string, prefix bool) []string {
if prefix {
name = maskTrailingDash(name)
}
if validation.IsDNS952Label(name) {
return nil
}
return []string{DNS952LabelErrorMsg}
return validation.IsDNS952Label(name)
}

// Validates that given value is not negative.
Expand Down Expand Up @@ -399,8 +391,10 @@ func validateVolumes(volumes []api.Volume, fldPath *field.Path) (sets.String, fi
el := validateVolumeSource(&vol.VolumeSource, idxPath)
if len(vol.Name) == 0 {
el = append(el, field.Required(idxPath.Child("name"), ""))
} else if !validation.IsDNS1123Label(vol.Name) {
el = append(el, field.Invalid(idxPath.Child("name"), vol.Name, DNS1123LabelErrorMsg))
} 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))
}
Expand Down Expand Up @@ -1356,8 +1350,10 @@ func validateContainers(containers []api.Container, volumes sets.String, fldPath
idxPath := fldPath.Index(i)
if len(ctr.Name) == 0 {
allErrs = append(allErrs, field.Required(idxPath.Child("name"), ""))
} else if !validation.IsDNS1123Label(ctr.Name) {
allErrs = append(allErrs, field.Invalid(idxPath.Child("name"), ctr.Name, DNS1123LabelErrorMsg))
} 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))
} else {
Expand Down Expand Up @@ -1547,12 +1543,16 @@ func ValidatePodSpec(spec *api.PodSpec, fldPath *field.Path) field.ErrorList {
}
}

if len(spec.Hostname) > 0 && !validation.IsDNS1123Label(spec.Hostname) {
allErrs = append(allErrs, field.Invalid(fldPath.Child("hostname"), spec.Hostname, DNS1123LabelErrorMsg))
if len(spec.Hostname) > 0 {
for _, msg := range validation.IsDNS1123Label(spec.Hostname) {
allErrs = append(allErrs, field.Invalid(fldPath.Child("hostname"), spec.Hostname, msg))
}
}

if len(spec.Subdomain) > 0 && !validation.IsDNS1123Label(spec.Subdomain) {
allErrs = append(allErrs, field.Invalid(fldPath.Child("subdomain"), spec.Subdomain, DNS1123LabelErrorMsg))
if len(spec.Subdomain) > 0 {
for _, msg := range validation.IsDNS1123Label(spec.Subdomain) {
allErrs = append(allErrs, field.Invalid(fldPath.Child("subdomain"), spec.Subdomain, msg))
}
}

return allErrs
Expand Down Expand Up @@ -2020,8 +2020,10 @@ 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 !validation.IsDNS1123Label(sp.Name) {
allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), sp.Name, DNS1123LabelErrorMsg))
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, field.Duplicate(fldPath.Child("name"), sp.Name))
} else {
Expand Down Expand Up @@ -2892,8 +2894,10 @@ func validateEndpointAddress(address *api.EndpointAddress, fldPath *field.Path)
if !validation.IsValidIP(address.IP) {
allErrs = append(allErrs, field.Invalid(fldPath.Child("ip"), address.IP, "must be a valid IP address"))
}
if len(address.Hostname) > 0 && !validation.IsDNS1123Label(address.Hostname) {
allErrs = append(allErrs, field.Invalid(fldPath.Child("hostname"), address.Hostname, DNS1123LabelErrorMsg))
if len(address.Hostname) > 0 {
for _, msg := range validation.IsDNS1123Label(address.Hostname) {
allErrs = append(allErrs, field.Invalid(fldPath.Child("hostname"), address.Hostname, msg))
}
}
if len(allErrs) > 0 {
return allErrs
Expand Down Expand Up @@ -2927,8 +2931,8 @@ 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 {
if !validation.IsDNS1123Label(port.Name) {
allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), port.Name, DNS1123LabelErrorMsg))
for _, msg := range validation.IsDNS1123Label(port.Name) {
allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), port.Name, msg))
}
}
if !validation.IsValidPortNum(int(port.Port)) {
Expand Down Expand Up @@ -3002,7 +3006,7 @@ func ValidateLoadBalancerStatus(status *api.LoadBalancerStatus, fldPath *field.P
}
}
if len(ingress.Hostname) > 0 {
for _, msg := range NameIsDNSSubdomain(ingress.Hostname, false) {
for _, msg := range validation.IsDNS1123Subdomain(ingress.Hostname) {
allErrs = append(allErrs, field.Invalid(idxPath.Child("hostname"), ingress.Hostname, msg))
}
if isIP := (net.ParseIP(ingress.Hostname) != nil); isIP {
Expand All @@ -3024,7 +3028,7 @@ func isValidHostnamesMap(serializedPodHostNames string) bool {
}

for ip, hostRecord := range podHostNames {
if !validation.IsDNS1123Label(hostRecord.HostName) {
if len(validation.IsDNS1123Label(hostRecord.HostName)) != 0 {
return false
}
if net.ParseIP(ip) == nil {
Expand Down
24 changes: 12 additions & 12 deletions pkg/api/validation/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,10 @@ func TestValidateObjectMetaNamespaces(t *testing.T) {
return nil
},
field.NewPath("field"))
if len(errs) != 1 {
if len(errs) != 2 {
t.Fatalf("unexpected errors: %v", errs)
}
if !strings.Contains(errs[0].Error(), "Invalid value") {
if !strings.Contains(errs[0].Error(), "Invalid value") || !strings.Contains(errs[1].Error(), "Invalid value") {
t.Errorf("unexpected error message: %v", errs)
}
}
Expand Down Expand Up @@ -748,12 +748,12 @@ func TestValidateVolumes(t *testing.T) {
"name > 63 characters": {
[]api.Volume{{Name: strings.Repeat("a", 64), VolumeSource: emptyVS}},
field.ErrorTypeInvalid,
"name", "must be a DNS label",
"name", "must be no more than",
},
"name not a DNS label": {
[]api.Volume{{Name: "a.b.c", VolumeSource: emptyVS}},
field.ErrorTypeInvalid,
"name", "must be a DNS label",
"name", "must match the regex",
},
"name not unique": {
[]api.Volume{{Name: "abc", VolumeSource: emptyVS}, {Name: "abc", VolumeSource: emptyVS}},
Expand Down Expand Up @@ -4691,11 +4691,11 @@ func TestValidateLimitRange(t *testing.T) {
},
"invalid-name": {
api.LimitRange{ObjectMeta: api.ObjectMeta{Name: "^Invalid", Namespace: "foo"}, Spec: api.LimitRangeSpec{}},
DNSSubdomainErrorMsg,
"must match the regex",
},
"invalid-namespace": {
api.LimitRange{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: "^Invalid"}, Spec: api.LimitRangeSpec{}},
DNS1123LabelErrorMsg,
"must match the regex",
},
"duplicate-limit-type": {
api.LimitRange{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: "foo"}, Spec: api.LimitRangeSpec{
Expand Down Expand Up @@ -4839,7 +4839,7 @@ func TestValidateLimitRange(t *testing.T) {
}
for i := range errs {
detail := errs[i].Detail
if detail != v.D {
if !strings.Contains(detail, v.D) {
t.Errorf("[%s]: expected error detail either empty or %q, got %q", k, v.D, detail)
}
}
Expand Down Expand Up @@ -5015,11 +5015,11 @@ func TestValidateResourceQuota(t *testing.T) {
},
"invalid Name": {
api.ResourceQuota{ObjectMeta: api.ObjectMeta{Name: "^Invalid", Namespace: "foo"}, Spec: spec},
DNSSubdomainErrorMsg,
"must match the regex",
},
"invalid Namespace": {
api.ResourceQuota{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: "^Invalid"}, Spec: spec},
DNS1123LabelErrorMsg,
"must match the regex",
},
"negative-limits": {
api.ResourceQuota{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: "foo"}, Spec: negativeSpec},
Expand Down Expand Up @@ -5052,7 +5052,7 @@ func TestValidateResourceQuota(t *testing.T) {
t.Errorf("expected failure for %s", k)
}
for i := range errs {
if errs[i].Detail != v.D {
if !strings.Contains(errs[i].Detail, v.D) {
t.Errorf("[%s]: expected error detail either empty or %s, got %s", k, v.D, errs[i].Detail)
}
}
Expand Down Expand Up @@ -5613,12 +5613,12 @@ func TestValidateEndpoints(t *testing.T) {
"invalid namespace": {
endpoints: api.Endpoints{ObjectMeta: api.ObjectMeta{Name: "mysvc", Namespace: "no@#invalid.;chars\"allowed"}},
errorType: "FieldValueInvalid",
errorDetail: DNS1123LabelErrorMsg,
errorDetail: "must match the regex",
},
"invalid name": {
endpoints: api.Endpoints{ObjectMeta: api.ObjectMeta{Name: "-_Invliad^&Characters", Namespace: "namespace"}},
errorType: "FieldValueInvalid",
errorDetail: DNSSubdomainErrorMsg,
errorDetail: "must match the regex",
},
"empty addresses": {
endpoints: api.Endpoints{
Expand Down
12 changes: 7 additions & 5 deletions pkg/apis/extensions/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,10 @@ func ValidateThirdPartyResource(obj *extensions.ThirdPartyResource) field.ErrorL
version := &obj.Versions[ix]
if len(version.Name) == 0 {
allErrs = append(allErrs, field.Invalid(field.NewPath("versions").Index(ix).Child("name"), version, "must not be empty"))
} else if !validation.IsDNS1123Label(version.Name) {
allErrs = append(allErrs, field.Invalid(field.NewPath("versions").Index(ix).Child("name"), version, apivalidation.DNS1123LabelErrorMsg))
} else {
for _, msg := range validation.IsDNS1123Label(version.Name) {
allErrs = append(allErrs, field.Invalid(field.NewPath("versions").Index(ix).Child("name"), version, msg))
}
}
if versions.Has(version.Name) {
allErrs = append(allErrs, field.Duplicate(field.NewPath("versions").Index(ix).Child("name"), version))
Expand Down Expand Up @@ -351,7 +353,7 @@ func validateIngressRules(IngressRules []extensions.IngressRule, fldPath *field.
if len(ih.Host) > 0 {
// TODO: Ports and ips are allowed in the host part of a url
// according to RFC 3986, consider allowing them.
for _, msg := range apivalidation.NameIsDNSSubdomain(ih.Host, false) {
for _, msg := range validation.IsDNS1123Subdomain(ih.Host) {
allErrs = append(allErrs, field.Invalid(fldPath.Index(i).Child("host"), ih.Host, msg))
}
if isIP := (net.ParseIP(ih.Host) != nil); isIP {
Expand Down Expand Up @@ -413,8 +415,8 @@ func validateIngressBackend(backend *extensions.IngressBackend, fldPath *field.P
}
}
if backend.ServicePort.Type == intstr.String {
if !validation.IsDNS1123Label(backend.ServicePort.StrVal) {
allErrs = append(allErrs, field.Invalid(fldPath.Child("servicePort"), backend.ServicePort.StrVal, apivalidation.DNS1123LabelErrorMsg))
for _, msg := range validation.IsDNS1123Label(backend.ServicePort.StrVal) {
allErrs = append(allErrs, field.Invalid(fldPath.Child("servicePort"), backend.ServicePort.StrVal, msg))
}
if !validation.IsValidPortName(backend.ServicePort.StrVal) {
allErrs = append(allErrs, field.Invalid(fldPath.Child("servicePort"), backend.ServicePort.StrVal, apivalidation.PortNameErrorMsg))
Expand Down
6 changes: 4 additions & 2 deletions pkg/client/unversioned/clientcmd/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,8 +260,10 @@ func validateContext(contextName string, context clientcmdapi.Context, config cl
validationErrors = append(validationErrors, fmt.Errorf("cluster %q was not found for context %q", context.Cluster, contextName))
}

if (len(context.Namespace) != 0) && !validation.IsDNS952Label(context.Namespace) {
validationErrors = append(validationErrors, fmt.Errorf("namespace %q for context %q does not conform to the kubernetes DNS952 rules", context.Namespace, contextName))
if len(context.Namespace) != 0 {
if len(validation.IsDNS1123Label(context.Namespace)) != 0 {
validationErrors = append(validationErrors, fmt.Errorf("namespace %q for context %q does not conform to the kubernetes DNS_LABEL rules", context.Namespace, contextName))
}
}

return validationErrors
Expand Down
18 changes: 8 additions & 10 deletions pkg/kubelet/kubelet.go
Original file line number Diff line number Diff line change
Expand Up @@ -1312,14 +1312,13 @@ func (kl *Kubelet) GeneratePodHostNameAndDomain(pod *api.Pod) (string, string, e
}
hostname := pod.Name
if len(pod.Spec.Hostname) > 0 {
if utilvalidation.IsDNS1123Label(pod.Spec.Hostname) {
hostname = pod.Spec.Hostname
} else {
return "", "", fmt.Errorf("Pod Hostname %q is not a valid DNS label.", pod.Spec.Hostname)
if msgs := utilvalidation.IsDNS1123Label(pod.Spec.Hostname); len(msgs) != 0 {
return "", "", fmt.Errorf("Pod Hostname %q is not a valid DNS label: %s", pod.Spec.Hostname, strings.Join(msgs, ";"))
}
hostname = pod.Spec.Hostname
} else {
hostnameCandidate := podAnnotations[utilpod.PodHostnameAnnotation]
if utilvalidation.IsDNS1123Label(hostnameCandidate) {
if len(utilvalidation.IsDNS1123Label(hostnameCandidate)) == 0 {
// use hostname annotation, if specified.
hostname = hostnameCandidate
}
Expand All @@ -1331,14 +1330,13 @@ func (kl *Kubelet) GeneratePodHostNameAndDomain(pod *api.Pod) (string, string, e

hostDomain := ""
if len(pod.Spec.Subdomain) > 0 {
if utilvalidation.IsDNS1123Label(pod.Spec.Subdomain) {
hostDomain = fmt.Sprintf("%s.%s.svc.%s", pod.Spec.Subdomain, pod.Namespace, clusterDomain)
} else {
return "", "", fmt.Errorf("Pod Subdomain %q is not a valid DNS label.", pod.Spec.Subdomain)
if msgs := utilvalidation.IsDNS1123Label(pod.Spec.Subdomain); len(msgs) != 0 {
return "", "", fmt.Errorf("Pod Subdomain %q is not a valid DNS label: %s", pod.Spec.Subdomain, strings.Join(msgs, ";"))
}
hostDomain = fmt.Sprintf("%s.%s.svc.%s", pod.Spec.Subdomain, pod.Namespace, clusterDomain)
} else {
subdomainCandidate := pod.Annotations[utilpod.PodSubdomainAnnotation]
if utilvalidation.IsDNS1123Label(subdomainCandidate) {
if len(utilvalidation.IsDNS1123Label(subdomainCandidate)) == 0 {
hostDomain = fmt.Sprintf("%s.%s.svc.%s", subdomainCandidate, pod.Namespace, clusterDomain)
}
}
Expand Down