From 72a066a992867bb79b83386f944b9a8d237c4b6e Mon Sep 17 00:00:00 2001 From: Marek Grabowski Date: Thu, 5 Feb 2015 01:36:27 +0100 Subject: [PATCH] Add more information to Validator error messages. --- pkg/api/validation/validation.go | 82 ++++++---- pkg/api/validation/validation_test.go | 225 ++++++++++++++------------ pkg/util/validation.go | 28 ++-- 3 files changed, 181 insertions(+), 154 deletions(-) diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index af47e3e0ad..78427fa2f2 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -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 @@ -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 @@ -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. @@ -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 @@ -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 { @@ -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)) } @@ -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 } @@ -271,8 +285,8 @@ 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 { @@ -280,12 +294,12 @@ func validatePorts(ports []api.Port) errs.ValidationErrorList { } } 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)) @@ -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)...) } @@ -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 { @@ -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)) @@ -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 { @@ -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 @@ -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")) @@ -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")) } } @@ -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 @@ -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))...) } @@ -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 } diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index 3e680f0f90..d89e4a06bf 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -94,6 +94,11 @@ func TestValidateLabels(t *testing.T) { errs := ValidateLabels(errorCases[i], "field") if len(errs) != 1 { t.Errorf("case[%d] expected failure", i) + } else { + detail := errs[0].(*errors.ValidationError).Detail + if detail != qualifiedNameErrorMsg { + t.Errorf("error detail %s should be equal %s", detail, qualifiedNameErrorMsg) + } } } } @@ -131,6 +136,11 @@ func TestValidateAnnotations(t *testing.T) { errs := ValidateAnnotations(errorCases[i], "field") if len(errs) != 1 { t.Errorf("case[%d] expected failure", i) + } else { + detail := errs[0].(*errors.ValidationError).Detail + if detail != qualifiedNameErrorMsg { + t.Errorf("error detail %s should be equal %s", detail, qualifiedNameErrorMsg) + } } } } @@ -175,6 +185,10 @@ func TestValidateVolumes(t *testing.T) { if errs[i].(*errors.ValidationError).Field != v.F { t.Errorf("%s: expected errors to have field %s: %v", k, v.F, errs[i]) } + detail := errs[i].(*errors.ValidationError).Detail + if detail != "" && detail != dnsLabelErrorMsg { + t.Errorf("%s: expected error detail either empty or %s, got %s", k, dnsLabelErrorMsg, detail) + } } } } @@ -203,18 +217,19 @@ func TestValidatePorts(t *testing.T) { P []api.Port T errors.ValidationErrorType F string + D string }{ - "name > 63 characters": {[]api.Port{{Name: strings.Repeat("a", 64), ContainerPort: 80, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].name"}, - "name not a DNS label": {[]api.Port{{Name: "a.b.c", ContainerPort: 80, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].name"}, + "name > 63 characters": {[]api.Port{{Name: strings.Repeat("a", 64), ContainerPort: 80, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].name", dnsLabelErrorMsg}, + "name not a DNS label": {[]api.Port{{Name: "a.b.c", ContainerPort: 80, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].name", dnsLabelErrorMsg}, "name not unique": {[]api.Port{ {Name: "abc", ContainerPort: 80, Protocol: "TCP"}, {Name: "abc", ContainerPort: 81, Protocol: "TCP"}, - }, errors.ValidationErrorTypeDuplicate, "[1].name"}, - "zero container port": {[]api.Port{{ContainerPort: 0, Protocol: "TCP"}}, errors.ValidationErrorTypeRequired, "[0].containerPort"}, - "invalid container port": {[]api.Port{{ContainerPort: 65536, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].containerPort"}, - "invalid host port": {[]api.Port{{ContainerPort: 80, HostPort: 65536, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].hostPort"}, - "invalid protocol": {[]api.Port{{ContainerPort: 80, Protocol: "ICMP"}}, errors.ValidationErrorTypeNotSupported, "[0].protocol"}, - "protocol required": {[]api.Port{{Name: "abc", ContainerPort: 80}}, errors.ValidationErrorTypeRequired, "[0].protocol"}, + }, errors.ValidationErrorTypeDuplicate, "[1].name", ""}, + "zero container port": {[]api.Port{{ContainerPort: 0, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].containerPort", portRangeErrorMsg}, + "invalid container port": {[]api.Port{{ContainerPort: 65536, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].containerPort", portRangeErrorMsg}, + "invalid host port": {[]api.Port{{ContainerPort: 80, HostPort: 65536, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].hostPort", portRangeErrorMsg}, + "invalid protocol": {[]api.Port{{ContainerPort: 80, Protocol: "ICMP"}}, errors.ValidationErrorTypeNotSupported, "[0].protocol", ""}, + "protocol required": {[]api.Port{{Name: "abc", ContainerPort: 80}}, errors.ValidationErrorTypeRequired, "[0].protocol", ""}, } for k, v := range errorCases { errs := validatePorts(v.P) @@ -228,6 +243,10 @@ func TestValidatePorts(t *testing.T) { if errs[i].(*errors.ValidationError).Field != v.F { t.Errorf("%s: expected errors to have field %s: %v", k, v.F, errs[i]) } + detail := errs[i].(*errors.ValidationError).Detail + if detail != v.D { + t.Errorf("%s: expected error detail either empty or %s, got %s", k, v.D, detail) + } } } } @@ -250,6 +269,13 @@ func TestValidateEnv(t *testing.T) { for k, v := range errorCases { if errs := validateEnv(v); len(errs) == 0 { t.Errorf("expected failure for %s", k) + } else { + for i := range errs { + detail := errs[i].(*errors.ValidationError).Detail + if detail != "" && detail != cIdentifierErrorMsg { + t.Errorf("%s: expected error detail either empty or %s, got %s", k, cIdentifierErrorMsg, detail) + } + } } } } @@ -1076,7 +1102,7 @@ func TestValidateService(t *testing.T) { Protocol: "TCP", }, }, - // Should fail because protocol is missing. + // Should fail because the session affinity is missing. numErrs: 1, }, { @@ -1397,6 +1423,9 @@ func TestValidateService(t *testing.T) { errs := ValidateService(&svc) if len(errs) != 0 { t.Errorf("Unexpected non-zero error list: %#v", errs) + for i := range errs { + t.Errorf("Found error: %s", errs[i].Error()) + } } } @@ -2168,38 +2197,46 @@ func TestValidateResourceNames(t *testing.T) { {"kubernetes.io", false}, {"kubernetes.io/will/not/work/", false}, } - for _, item := range table { - err := validateResourceName(item.input) + for k, item := range table { + err := validateResourceName(item.input, "sth") if len(err) != 0 && item.success { t.Errorf("expected no failure for input %q", item.input) } else if len(err) == 0 && !item.success { t.Errorf("expected failure for input %q", item.input) + for i := range err { + detail := err[i].(*errors.ValidationError).Detail + if detail != "" && detail != qualifiedNameErrorMsg { + t.Errorf("%s: expected error detail either empty or %s, got %s", k, qualifiedNameErrorMsg, detail) + } + } } } } func TestValidateLimitRange(t *testing.T) { + spec := api.LimitRangeSpec{ + Limits: []api.LimitRangeItem{ + { + Type: api.LimitTypePod, + Max: api.ResourceList{ + api.ResourceCPU: resource.MustParse("100"), + api.ResourceMemory: resource.MustParse("10000"), + }, + Min: api.ResourceList{ + api.ResourceCPU: resource.MustParse("0"), + api.ResourceMemory: resource.MustParse("100"), + }, + }, + }, + } + successCases := []api.LimitRange{ { ObjectMeta: api.ObjectMeta{ Name: "abc", Namespace: "foo", }, - Spec: api.LimitRangeSpec{ - Limits: []api.LimitRangeItem{ - { - Type: api.LimitTypePod, - Max: api.ResourceList{ - api.ResourceCPU: resource.MustParse("100"), - api.ResourceMemory: resource.MustParse("10000"), - }, - Min: api.ResourceList{ - api.ResourceCPU: resource.MustParse("0"), - api.ResourceMemory: resource.MustParse("100"), - }, - }, - }, - }, + Spec: spec, }, } @@ -2209,82 +2246,65 @@ func TestValidateLimitRange(t *testing.T) { } } - errorCases := map[string]api.LimitRange{ + errorCases := map[string]struct { + R api.LimitRange + D string + }{ "zero-length Name": { - ObjectMeta: api.ObjectMeta{ - Name: "", - Namespace: "foo", - }, - Spec: api.LimitRangeSpec{ - Limits: []api.LimitRangeItem{ - { - Type: api.LimitTypePod, - Max: api.ResourceList{ - api.ResourceCPU: resource.MustParse("100"), - api.ResourceMemory: resource.MustParse("10000"), - }, - Min: api.ResourceList{ - api.ResourceCPU: resource.MustParse("0"), - api.ResourceMemory: resource.MustParse("100"), - }, - }, - }, - }, + api.LimitRange{ObjectMeta: api.ObjectMeta{Name: "", Namespace: "foo"}, Spec: spec}, + "", }, "zero-length-namespace": { - ObjectMeta: api.ObjectMeta{ - Name: "abc", - Namespace: "", - }, - Spec: api.LimitRangeSpec{ - Limits: []api.LimitRangeItem{ - { - Type: api.LimitTypePod, - Max: api.ResourceList{ - api.ResourceCPU: resource.MustParse("100"), - api.ResourceMemory: resource.MustParse("10000"), - }, - Min: api.ResourceList{ - api.ResourceCPU: resource.MustParse("0"), - api.ResourceMemory: resource.MustParse("100"), - }, - }, - }, - }, + api.LimitRange{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: ""}, Spec: spec}, + "", + }, + "invalid Name": { + api.LimitRange{ObjectMeta: api.ObjectMeta{Name: "^Invalid", Namespace: "foo"}, Spec: spec}, + dnsSubdomainErrorMsg, + }, + "invalid Namespace": { + api.LimitRange{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: "^Invalid"}, Spec: spec}, + dnsSubdomainErrorMsg, }, } for k, v := range errorCases { - errs := ValidateLimitRange(&v) + errs := ValidateLimitRange(&v.R) if len(errs) == 0 { t.Errorf("expected failure for %s", k) } for i := range errs { field := errs[i].(*errors.ValidationError).Field + detail := errs[i].(*errors.ValidationError).Detail if field != "name" && field != "namespace" { t.Errorf("%s: missing prefix for: %v", k, errs[i]) } + if detail != v.D { + t.Errorf("%s: expected error detail either empty or %s, got %s", k, v.D, detail) + } } } } func TestValidateResourceQuota(t *testing.T) { + spec := api.ResourceQuotaSpec{ + Hard: api.ResourceList{ + api.ResourceCPU: resource.MustParse("100"), + api.ResourceMemory: resource.MustParse("10000"), + api.ResourcePods: resource.MustParse("10"), + api.ResourceServices: resource.MustParse("10"), + api.ResourceReplicationControllers: resource.MustParse("10"), + api.ResourceQuotas: resource.MustParse("10"), + }, + } + successCases := []api.ResourceQuota{ { ObjectMeta: api.ObjectMeta{ Name: "abc", Namespace: "foo", }, - Spec: api.ResourceQuotaSpec{ - Hard: api.ResourceList{ - api.ResourceCPU: resource.MustParse("100"), - api.ResourceMemory: resource.MustParse("10000"), - api.ResourcePods: resource.MustParse("10"), - api.ResourceServices: resource.MustParse("10"), - api.ResourceReplicationControllers: resource.MustParse("10"), - api.ResourceQuotas: resource.MustParse("10"), - }, - }, + Spec: spec, }, } @@ -2294,51 +2314,42 @@ func TestValidateResourceQuota(t *testing.T) { } } - errorCases := map[string]api.ResourceQuota{ + errorCases := map[string]struct { + R api.ResourceQuota + D string + }{ "zero-length Name": { - ObjectMeta: api.ObjectMeta{ - Name: "", - Namespace: "foo", - }, - Spec: api.ResourceQuotaSpec{ - Hard: api.ResourceList{ - api.ResourceCPU: resource.MustParse("100"), - api.ResourceMemory: resource.MustParse("10000"), - api.ResourcePods: resource.MustParse("10"), - api.ResourceServices: resource.MustParse("10"), - api.ResourceReplicationControllers: resource.MustParse("10"), - api.ResourceQuotas: resource.MustParse("10"), - }, - }, + api.ResourceQuota{ObjectMeta: api.ObjectMeta{Name: "", Namespace: "foo"}, Spec: spec}, + "", }, - "zero-length-namespace": { - ObjectMeta: api.ObjectMeta{ - Name: "abc", - Namespace: "", - }, - Spec: api.ResourceQuotaSpec{ - Hard: api.ResourceList{ - api.ResourceCPU: resource.MustParse("100"), - api.ResourceMemory: resource.MustParse("10000"), - api.ResourcePods: resource.MustParse("10"), - api.ResourceServices: resource.MustParse("10"), - api.ResourceReplicationControllers: resource.MustParse("10"), - api.ResourceQuotas: resource.MustParse("10"), - }, - }, + "zero-length Namespace": { + api.ResourceQuota{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: ""}, Spec: spec}, + "", + }, + "invalid Name": { + api.ResourceQuota{ObjectMeta: api.ObjectMeta{Name: "^Invalid", Namespace: "foo"}, Spec: spec}, + dnsSubdomainErrorMsg, + }, + "invalid Namespace": { + api.ResourceQuota{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: "^Invalid"}, Spec: spec}, + dnsSubdomainErrorMsg, }, } for k, v := range errorCases { - errs := ValidateResourceQuota(&v) + errs := ValidateResourceQuota(&v.R) if len(errs) == 0 { t.Errorf("expected failure for %s", k) } for i := range errs { field := errs[i].(*errors.ValidationError).Field + detail := errs[i].(*errors.ValidationError).Detail if field != "name" && field != "namespace" { t.Errorf("%s: missing prefix for: %v", k, errs[i]) } + if detail != v.D { + t.Errorf("%s: expected error detail either empty or %s, got %s", k, v.D, detail) + } } } } diff --git a/pkg/util/validation.go b/pkg/util/validation.go index 7bf1d70895..5b1143e3c8 100644 --- a/pkg/util/validation.go +++ b/pkg/util/validation.go @@ -33,45 +33,45 @@ func IsDNSSubdomain(value string) bool { return IsDNS1123Subdomain(value) } -const dns1123LabelFmt string = "[a-z0-9]([-a-z0-9]*[a-z0-9])?" +const DNS1123LabelFmt string = "[a-z0-9]([-a-z0-9]*[a-z0-9])?" -var dns1123LabelRegexp = regexp.MustCompile("^" + dns1123LabelFmt + "$") +var dns1123LabelRegexp = regexp.MustCompile("^" + DNS1123LabelFmt + "$") -const dns1123LabelMaxLength int = 63 +const DNS1123LabelMaxLength int = 63 // IsDNS1123Label tests for a string that conforms to the definition of a label in // DNS (RFC 1123). func IsDNS1123Label(value string) bool { - return len(value) <= dns1123LabelMaxLength && dns1123LabelRegexp.MatchString(value) + return len(value) <= DNS1123LabelMaxLength && dns1123LabelRegexp.MatchString(value) } -const dns1123SubdomainFmt string = dns1123LabelFmt + "(\\." + dns1123LabelFmt + ")*" +const DNS1123SubdomainFmt string = DNS1123LabelFmt + "(\\." + DNS1123LabelFmt + ")*" -var dns1123SubdomainRegexp = regexp.MustCompile("^" + dns1123SubdomainFmt + "$") +var dns1123SubdomainRegexp = regexp.MustCompile("^" + DNS1123SubdomainFmt + "$") -const dns1123SubdomainMaxLength int = 253 +const DNS1123SubdomainMaxLength int = 253 // IsDNS1123Subdomain tests for a string that conforms to the definition of a // subdomain in DNS (RFC 1123). func IsDNS1123Subdomain(value string) bool { - return len(value) <= dns1123SubdomainMaxLength && dns1123SubdomainRegexp.MatchString(value) + return len(value) <= DNS1123SubdomainMaxLength && dns1123SubdomainRegexp.MatchString(value) } -const dns952LabelFmt string = "[a-z]([-a-z0-9]*[a-z0-9])?" +const DNS952LabelFmt string = "[a-z]([-a-z0-9]*[a-z0-9])?" -var dns952LabelRegexp = regexp.MustCompile("^" + dns952LabelFmt + "$") +var dns952LabelRegexp = regexp.MustCompile("^" + DNS952LabelFmt + "$") -const dns952LabelMaxLength int = 24 +const DNS952LabelMaxLength int = 24 // IsDNS952Label tests for a string that conforms to the definition of a label in // DNS (RFC 952). func IsDNS952Label(value string) bool { - return len(value) <= dns952LabelMaxLength && dns952LabelRegexp.MatchString(value) + return len(value) <= DNS952LabelMaxLength && dns952LabelRegexp.MatchString(value) } -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.