Merge pull request #15850 from mqliang/refactorValidation

Auto commit by PR queue bot
2015-10-25 05:42:49 -07:00 · 2015-10-25 05:42:49 -07:00 · fea0a7d753
parent 6b330a8db5 ce384e62ed
commit fea0a7d753
3 changed files with 51 additions and 68 deletions
--- a/pkg/api/validation/validation.go
+++ b/pkg/api/validation/validation.go
@ -42,12 +42,12 @@ import (
 // fields by default.
 var RepairMalformedUpdates bool = true

-const cIdentifierErrorMsg string = `must be a C identifier (matching regex ` + validation.CIdentifierFmt + `): e.g. "my_name" or "MyName"`
 const isNegativeErrorMsg string = `must be non-negative`
-const isNotIntegerErrorMsg string = `must be an integer`
 const fieldImmutableErrorMsg string = `field is immutable`
+const cIdentifierErrorMsg string = `must be a C identifier (matching regex ` + validation.CIdentifierFmt + `): e.g. "my_name" or "MyName"`
+const isNotIntegerErrorMsg string = `must be an integer`

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

@ -56,9 +56,9 @@ var qualifiedNameErrorMsg string = fmt.Sprintf(`must be a qualified name (at mos
 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 = intervalErrorMsg(0, 255)
-var portRangeErrorMsg string = intervalErrorMsg(0, 65536)
-var portNameErrorMsg string = fmt.Sprintf(`must be an IANA_SVC_NAME (at most 15 characters, matching regex %s, it must contain at least one letter [a-z], and hyphens cannot be adjacent to other hyphens): e.g. "http"`, validation.IdentifierNoHyphensBeginEndFmt)
+var pdPartitionErrorMsg string = IntervalErrorMsg(0, 255)
+var PortRangeErrorMsg string = IntervalErrorMsg(0, 65536)
+var PortNameErrorMsg string = fmt.Sprintf(`must be an IANA_SVC_NAME (at most 15 characters, matching regex %s, it must contain at least one letter [a-z], and hyphens cannot be adjacent to other hyphens): e.g. "http"`, validation.IdentifierNoHyphensBeginEndFmt)

 const totalAnnotationSizeLimitB int = 256 * (1 << 10) // 256 kB

@ -235,6 +235,14 @@ func ValidatePositiveQuantity(value resource.Quantity, fieldName string) errs.Va
 	return allErrs
 }

+func ValidateImmutableField(old, new interface{}, fieldName string) errs.ValidationErrorList {
+	allErrs := errs.ValidationErrorList{}
+	if !api.Semantic.DeepEqual(old, new) {
+		allErrs = append(allErrs, errs.NewFieldInvalid(fieldName, new, fieldImmutableErrorMsg))
+	}
+	return allErrs
+}
+
 // ValidateObjectMeta validates an object's metadata on creation. It expects that name generation has already
 // been performed.
 // TODO: Remove calls to this method scattered in validations of specific resources, e.g., ValidatePodUpdate.
@ -315,18 +323,10 @@ func ValidateObjectMetaUpdate(new, old *api.ObjectMeta) errs.ValidationErrorList
 		allErrs = append(allErrs, errs.NewFieldInvalid("resourceVersion", new.ResourceVersion, "resourceVersion must be specified for an update"))
 	}

-	if old.Name != new.Name {
-		allErrs = append(allErrs, errs.NewFieldInvalid("name", new.Name, fieldImmutableErrorMsg))
-	}
-	if old.Namespace != new.Namespace {
-		allErrs = append(allErrs, errs.NewFieldInvalid("namespace", new.Namespace, fieldImmutableErrorMsg))
-	}
-	if old.UID != new.UID {
-		allErrs = append(allErrs, errs.NewFieldInvalid("uid", new.UID, fieldImmutableErrorMsg))
-	}
-	if old.CreationTimestamp != new.CreationTimestamp {
-		allErrs = append(allErrs, errs.NewFieldInvalid("creationTimestamp", new.CreationTimestamp, fieldImmutableErrorMsg))
-	}
+	allErrs = append(allErrs, ValidateImmutableField(old.Name, new.Name, "name")...)
+	allErrs = append(allErrs, ValidateImmutableField(old.Namespace, new.Namespace, "namespace")...)
+	allErrs = append(allErrs, ValidateImmutableField(old.UID, new.UID, "uid")...)
+	allErrs = append(allErrs, ValidateImmutableField(old.CreationTimestamp, new.CreationTimestamp, "creationTimestamp")...)

 	allErrs = append(allErrs, ValidateLabels(new.Labels, "labels")...)
 	allErrs = append(allErrs, ValidateAnnotations(new.Annotations, "annotations")...)
@ -771,7 +771,7 @@ func validatePorts(ports []api.ContainerPort) errs.ValidationErrorList {
 		pErrs := errs.ValidationErrorList{}
 		if len(port.Name) > 0 {
 			if !validation.IsValidPortName(port.Name) {
-				pErrs = append(pErrs, errs.NewFieldInvalid("name", port.Name, portNameErrorMsg))
+				pErrs = append(pErrs, errs.NewFieldInvalid("name", port.Name, PortNameErrorMsg))
 			} else if allNames.Has(port.Name) {
 				pErrs = append(pErrs, errs.NewFieldDuplicate("name", port.Name))
 			} else {
@ -779,12 +779,12 @@ func validatePorts(ports []api.ContainerPort) errs.ValidationErrorList {
 			}
 		}
 		if port.ContainerPort == 0 {
-			pErrs = append(pErrs, errs.NewFieldInvalid("containerPort", port.ContainerPort, portRangeErrorMsg))
+			pErrs = append(pErrs, errs.NewFieldInvalid("containerPort", port.ContainerPort, PortRangeErrorMsg))
 		} else if !validation.IsValidPortNum(port.ContainerPort) {
-			pErrs = append(pErrs, errs.NewFieldInvalid("containerPort", port.ContainerPort, portRangeErrorMsg))
+			pErrs = append(pErrs, errs.NewFieldInvalid("containerPort", port.ContainerPort, PortRangeErrorMsg))
 		}
 		if port.HostPort != 0 && !validation.IsValidPortNum(port.HostPort) {
-			pErrs = append(pErrs, errs.NewFieldInvalid("hostPort", port.HostPort, portRangeErrorMsg))
+			pErrs = append(pErrs, errs.NewFieldInvalid("hostPort", port.HostPort, PortRangeErrorMsg))
 		}
 		if len(port.Protocol) == 0 {
 			pErrs = append(pErrs, errs.NewFieldRequired("protocol"))
@ -934,9 +934,9 @@ func validateHTTPGetAction(http *api.HTTPGetAction) errs.ValidationErrorList {
 		allErrors = append(allErrors, errs.NewFieldRequired("path"))
 	}
 	if http.Port.Kind == util.IntstrInt && !validation.IsValidPortNum(http.Port.IntVal) {
-		allErrors = append(allErrors, errs.NewFieldInvalid("port", http.Port, portRangeErrorMsg))
+		allErrors = append(allErrors, errs.NewFieldInvalid("port", http.Port, PortRangeErrorMsg))
 	} else if http.Port.Kind == util.IntstrString && !validation.IsValidPortName(http.Port.StrVal) {
-		allErrors = append(allErrors, errs.NewFieldInvalid("port", http.Port.StrVal, portNameErrorMsg))
+		allErrors = append(allErrors, errs.NewFieldInvalid("port", http.Port.StrVal, PortNameErrorMsg))
 	}
 	supportedSchemes := sets.NewString(string(api.URISchemeHTTP), string(api.URISchemeHTTPS))
 	if !supportedSchemes.Has(string(http.Scheme)) {
@ -948,9 +948,9 @@ func validateHTTPGetAction(http *api.HTTPGetAction) errs.ValidationErrorList {
 func validateTCPSocketAction(tcp *api.TCPSocketAction) errs.ValidationErrorList {
 	allErrors := errs.ValidationErrorList{}
 	if tcp.Port.Kind == util.IntstrInt && !validation.IsValidPortNum(tcp.Port.IntVal) {
-		allErrors = append(allErrors, errs.NewFieldInvalid("port", tcp.Port, portRangeErrorMsg))
+		allErrors = append(allErrors, errs.NewFieldInvalid("port", tcp.Port, PortRangeErrorMsg))
 	} else if tcp.Port.Kind == util.IntstrString && !validation.IsValidPortName(tcp.Port.StrVal) {
-		allErrors = append(allErrors, errs.NewFieldInvalid("port", tcp.Port.StrVal, portNameErrorMsg))
+		allErrors = append(allErrors, errs.NewFieldInvalid("port", tcp.Port.StrVal, PortNameErrorMsg))
 	}
 	return allErrors
 }
@ -1320,7 +1320,7 @@ func validateServicePort(sp *api.ServicePort, requireName bool, allNames *sets.S
 	}

 	if !validation.IsValidPortNum(sp.Port) {
-		allErrs = append(allErrs, errs.NewFieldInvalid("port", sp.Port, portRangeErrorMsg))
+		allErrs = append(allErrs, errs.NewFieldInvalid("port", sp.Port, PortRangeErrorMsg))
 	}

 	if len(sp.Protocol) == 0 {
@ -1330,10 +1330,10 @@ func validateServicePort(sp *api.ServicePort, requireName bool, allNames *sets.S
 	}

 	if sp.TargetPort.Kind == util.IntstrInt && !validation.IsValidPortNum(sp.TargetPort.IntVal) {
-		allErrs = append(allErrs, errs.NewFieldInvalid("targetPort", sp.TargetPort, portRangeErrorMsg))
+		allErrs = append(allErrs, errs.NewFieldInvalid("targetPort", sp.TargetPort, PortRangeErrorMsg))
 	}
 	if sp.TargetPort.Kind == util.IntstrString && !validation.IsValidPortName(sp.TargetPort.StrVal) {
-		allErrs = append(allErrs, errs.NewFieldInvalid("targetPort", sp.TargetPort, portNameErrorMsg))
+		allErrs = append(allErrs, errs.NewFieldInvalid("targetPort", sp.TargetPort, PortNameErrorMsg))
 	}

 	return allErrs
@ -1344,8 +1344,8 @@ func ValidateServiceUpdate(oldService, service *api.Service) errs.ValidationErro
 	allErrs := errs.ValidationErrorList{}
 	allErrs = append(allErrs, ValidateObjectMetaUpdate(&service.ObjectMeta, &oldService.ObjectMeta).Prefix("metadata")...)

-	if api.IsServiceIPSet(oldService) && service.Spec.ClusterIP != oldService.Spec.ClusterIP {
-		allErrs = append(allErrs, errs.NewFieldInvalid("spec.clusterIP", service.Spec.ClusterIP, fieldImmutableErrorMsg))
+	if api.IsServiceIPSet(oldService) {
+		allErrs = append(allErrs, ValidateImmutableField(oldService.Spec.ClusterIP, service.Spec.ClusterIP, "spec.clusterIP")...)
 	}

 	allErrs = append(allErrs, ValidateService(service)...)
@ -1710,9 +1710,8 @@ func ValidateSecretUpdate(oldSecret, newSecret *api.Secret) errs.ValidationError
 	if len(newSecret.Type) == 0 {
 		newSecret.Type = oldSecret.Type
 	}
-	if newSecret.Type != oldSecret.Type {
-		allErrs = append(allErrs, errs.NewFieldInvalid("type", newSecret.Type, fieldImmutableErrorMsg))
-	}
+
+	allErrs = append(allErrs, ValidateImmutableField(oldSecret.Type, newSecret.Type, "type")...)

 	allErrs = append(allErrs, ValidateSecret(newSecret)...)
 	return allErrs
@ -1966,7 +1965,7 @@ func validateEndpointPort(port *api.EndpointPort, requireName bool) errs.Validat
 		}
 	}
 	if !validation.IsValidPortNum(port.Port) {
-		allErrs = append(allErrs, errs.NewFieldInvalid("port", port.Port, portRangeErrorMsg))
+		allErrs = append(allErrs, errs.NewFieldInvalid("port", port.Port, PortRangeErrorMsg))
 	}
 	if len(port.Protocol) == 0 {
 		allErrs = append(allErrs, errs.NewFieldRequired("protocol"))
--- a/pkg/api/validation/validation_test.go
+++ b/pkg/api/validation/validation_test.go
@ -607,16 +607,16 @@ func TestValidatePorts(t *testing.T) {
 		F string
 		D string
 	}{
-		"name > 15 characters":                     {[]api.ContainerPort{{Name: strings.Repeat("a", 16), ContainerPort: 80, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].name", portNameErrorMsg},
-		"name not a IANA svc name ":                {[]api.ContainerPort{{Name: "a.b.c", ContainerPort: 80, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].name", portNameErrorMsg},
-		"name not a IANA svc name (i.e. a number)": {[]api.ContainerPort{{Name: "80", ContainerPort: 80, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].name", portNameErrorMsg},
+		"name > 15 characters":                     {[]api.ContainerPort{{Name: strings.Repeat("a", 16), ContainerPort: 80, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].name", PortNameErrorMsg},
+		"name not a IANA svc name ":                {[]api.ContainerPort{{Name: "a.b.c", ContainerPort: 80, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].name", PortNameErrorMsg},
+		"name not a IANA svc name (i.e. a number)": {[]api.ContainerPort{{Name: "80", ContainerPort: 80, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].name", PortNameErrorMsg},
 		"name not unique": {[]api.ContainerPort{
 			{Name: "abc", ContainerPort: 80, Protocol: "TCP"},
 			{Name: "abc", ContainerPort: 81, Protocol: "TCP"},
 		}, errors.ValidationErrorTypeDuplicate, "[1].name", ""},
-		"zero container port":    {[]api.ContainerPort{{ContainerPort: 0, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].containerPort", portRangeErrorMsg},
-		"invalid container port": {[]api.ContainerPort{{ContainerPort: 65536, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].containerPort", portRangeErrorMsg},
-		"invalid host port":      {[]api.ContainerPort{{ContainerPort: 80, HostPort: 65536, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].hostPort", portRangeErrorMsg},
+		"zero container port":    {[]api.ContainerPort{{ContainerPort: 0, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].containerPort", PortRangeErrorMsg},
+		"invalid container port": {[]api.ContainerPort{{ContainerPort: 65536, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].containerPort", PortRangeErrorMsg},
+		"invalid host port":      {[]api.ContainerPort{{ContainerPort: 80, HostPort: 65536, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].hostPort", PortRangeErrorMsg},
 		"invalid protocol case":  {[]api.ContainerPort{{ContainerPort: 80, Protocol: "tcp"}}, errors.ValidationErrorTypeNotSupported, "[0].protocol", "supported values: TCP, UDP"},
 		"invalid protocol":       {[]api.ContainerPort{{ContainerPort: 80, Protocol: "ICMP"}}, errors.ValidationErrorTypeNotSupported, "[0].protocol", "supported values: TCP, UDP"},
 		"protocol required":      {[]api.ContainerPort{{Name: "abc", ContainerPort: 80}}, errors.ValidationErrorTypeRequired, "[0].protocol", ""},
@ -3831,7 +3831,7 @@ func TestValidateEndpoints(t *testing.T) {
 				},
 			},
 			errorType:   "FieldValueInvalid",
-			errorDetail: portRangeErrorMsg,
+			errorDetail: PortRangeErrorMsg,
 		},
 		"Invalid protocol": {
 			endpoints: api.Endpoints{
@ -3869,7 +3869,7 @@ func TestValidateEndpoints(t *testing.T) {
 				},
 			},
 			errorType:   "FieldValueInvalid",
-			errorDetail: portRangeErrorMsg,
+			errorDetail: PortRangeErrorMsg,
 		},
 		"Port missing protocol": {
 			endpoints: api.Endpoints{
--- a/pkg/apis/extensions/validation/validation.go
+++ b/pkg/apis/extensions/validation/validation.go
@ -34,16 +34,6 @@ import (
 	utilvalidation "k8s.io/kubernetes/pkg/util/validation"
 )

-const isNegativeErrorMsg string = `must be non-negative`
-
-// TODO: Expose from apivalidation instead of duplicating.
-func intervalErrorMsg(lo, hi int) string {
-	return fmt.Sprintf(`must be greater than %d and less than %d`, lo, hi)
-}
-
-var portRangeErrorMsg string = intervalErrorMsg(0, 65536)
-var portNameErrorMsg string = fmt.Sprintf(`must be an IANA_SVC_NAME (at most 15 characters, matching regex %s, it must contain at least one letter [a-z], and hyphens cannot be adjacent to other hyphens): e.g. "http"`, validation.IdentifierNoHyphensBeginEndFmt)
-
 // ValidateHorizontalPodAutoscaler can be used to check whether the given autoscaler name is valid.
 // Prefix indicates this name will be used as part of generation, in which case trailing dashes are allowed.
 func ValidateHorizontalPodAutoscalerName(name string, prefix bool) (bool, string) {
@ -318,11 +308,11 @@ func ValidateJob(job *extensions.Job) errs.ValidationErrorList {
 func ValidateJobSpec(spec *extensions.JobSpec) errs.ValidationErrorList {
 	allErrs := errs.ValidationErrorList{}

-	if spec.Parallelism != nil && *spec.Parallelism < 0 {
-		allErrs = append(allErrs, errs.NewFieldInvalid("parallelism", spec.Parallelism, isNegativeErrorMsg))
+	if spec.Parallelism != nil {
+		allErrs = append(allErrs, apivalidation.ValidatePositiveField(int64(*spec.Parallelism), "parallelism")...)
 	}
-	if spec.Completions != nil && *spec.Completions < 0 {
-		allErrs = append(allErrs, errs.NewFieldInvalid("completions", spec.Completions, isNegativeErrorMsg))
+	if spec.Completions != nil {
+		allErrs = append(allErrs, apivalidation.ValidatePositiveField(int64(*spec.Completions), "completions")...)
 	}
 	if spec.Selector == nil {
 		allErrs = append(allErrs, errs.NewFieldRequired("selector"))
@ -371,15 +361,9 @@ func ValidateJobUpdateStatus(oldJob, job *extensions.Job) errs.ValidationErrorLi
 func ValidateJobSpecUpdate(oldSpec, spec extensions.JobSpec) errs.ValidationErrorList {
 	allErrs := errs.ValidationErrorList{}
 	allErrs = append(allErrs, ValidateJobSpec(&spec)...)
-	if !api.Semantic.DeepEqual(oldSpec.Completions, spec.Completions) {
-		allErrs = append(allErrs, errs.NewFieldInvalid("completions", spec.Completions, "field is immutable"))
-	}
-	if !api.Semantic.DeepEqual(oldSpec.Selector, spec.Selector) {
-		allErrs = append(allErrs, errs.NewFieldInvalid("selector", spec.Selector, "field is immutable"))
-	}
-	if !api.Semantic.DeepEqual(oldSpec.Template, spec.Template) {
-		allErrs = append(allErrs, errs.NewFieldInvalid("template", "[omitted]", "field is immutable"))
-	}
+	allErrs = append(allErrs, apivalidation.ValidateImmutableField(oldSpec.Completions, spec.Completions, "completions")...)
+	allErrs = append(allErrs, apivalidation.ValidateImmutableField(oldSpec.Selector, spec.Selector, "selector")...)
+	allErrs = append(allErrs, apivalidation.ValidateImmutableField(oldSpec.Template, spec.Template, "template")...)
 	return allErrs
 }

@ -506,10 +490,10 @@ func validateIngressBackend(backend *extensions.IngressBackend) errs.ValidationE
 			allErrs = append(allErrs, errs.NewFieldInvalid("servicePort", backend.ServicePort.StrVal, apivalidation.DNS1123LabelErrorMsg))
 		}
 		if !utilvalidation.IsValidPortName(backend.ServicePort.StrVal) {
-			allErrs = append(allErrs, errs.NewFieldInvalid("servicePort", backend.ServicePort.StrVal, portNameErrorMsg))
+			allErrs = append(allErrs, errs.NewFieldInvalid("servicePort", backend.ServicePort.StrVal, apivalidation.PortNameErrorMsg))
 		}
 	} else if !utilvalidation.IsValidPortNum(backend.ServicePort.IntVal) {
-		allErrs = append(allErrs, errs.NewFieldInvalid("servicePort", backend.ServicePort, portRangeErrorMsg))
+		allErrs = append(allErrs, errs.NewFieldInvalid("servicePort", backend.ServicePort, apivalidation.PortRangeErrorMsg))
 	}
 	return allErrs
 }