Skip to content

Commit

Permalink
Make IsDNS1123Label return error strings
Browse files Browse the repository at this point in the history
  • Loading branch information
@thockin
thockin committed May 19, 2016
1 parent 9784dff commit 77eff06
Show file tree
Hide file tree
Showing 7 changed files with 109 additions and 64 deletions.
2 changes: 1 addition & 1 deletion cluster/addons/dns/kube2sky/kube2sky.go
View file
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
62 changes: 37 additions & 25 deletions pkg/api/validation/validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ func InclusiveRangeErrorMsg(lo, hi int) string {
}

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)
Expand Down Expand Up @@ -113,12 +112,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 +170,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 @@ -249,10 +252,7 @@ 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.
Expand Down Expand Up @@ -399,8 +399,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 +1358,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 +1551,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 +2028,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 +2902,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 +2939,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 @@ -3024,7 +3036,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
18 changes: 9 additions & 9 deletions pkg/api/validation/validation_test.go
View file
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 @@ -4695,7 +4695,7 @@ func TestValidateLimitRange(t *testing.T) {
},
"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 @@ -5019,7 +5019,7 @@ func TestValidateResourceQuota(t *testing.T) {
},
"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,7 +5613,7 @@ 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"}},
Expand Down
10 changes: 6 additions & 4 deletions pkg/apis/extensions/validation/validation.go
View file
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 @@ -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
18 changes: 8 additions & 10 deletions pkg/kubelet/kubelet.go
View file
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
Loading

0 comments on commit 77eff06

Please sign in to comment.