apiextensions: normalize CR validation to return multiple errors

staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/validator.go

@@ -43,73 +43,36 @@ type customResourceValidator struct {
 }
 
 func (a customResourceValidator) Validate(ctx context.Context, obj runtime.Object, scale *apiextensions.CustomResourceSubresourceScale) field.ErrorList {
+	u, ok := obj.(*unstructured.Unstructured)
+	if !ok {
+		return field.ErrorList{field.Invalid(field.NewPath(""), u, fmt.Sprintf("has type %T. Must be a pointer to an Unstructured type", u))}
+	}
 	accessor, err := meta.Accessor(obj)
 	if err != nil {
 		return field.ErrorList{field.Invalid(field.NewPath("metadata"), nil, err.Error())}
 	}
-	typeAccessor, err := meta.TypeAccessor(obj)
+
-	if err != nil {
+	if errs := a.ValidateTypeMeta(ctx, u); len(errs) > 0 {
-		return field.ErrorList{field.Invalid(field.NewPath("kind"), nil, err.Error())}
+		return errs
-	}
-	if typeAccessor.GetKind() != a.kind.Kind {
-		return field.ErrorList{field.Invalid(field.NewPath("kind"), typeAccessor.GetKind(), fmt.Sprintf("must be %v", a.kind.Kind))}
-	}
-	if typeAccessor.GetAPIVersion() != a.kind.Group+"/"+a.kind.Version {
-		return field.ErrorList{field.Invalid(field.NewPath("apiVersion"), typeAccessor.GetAPIVersion(), fmt.Sprintf("must be %v", a.kind.Group+"/"+a.kind.Version))}
 	}
 
-	customResourceObject, ok := obj.(*unstructured.Unstructured)
+	var allErrs field.ErrorList
-	// this will never happen.
+
-	if !ok {
+	allErrs = append(allErrs, validation.ValidateObjectMetaAccessor(accessor, a.namespaceScoped, validation.NameIsDNSSubdomain, field.NewPath("metadata"))...)
-		return field.ErrorList{field.Invalid(field.NewPath(""), customResourceObject, fmt.Sprintf("has type %T. Must be a pointer to an Unstructured type", customResourceObject))}
+	if err = apiservervalidation.ValidateCustomResource(u.UnstructuredContent(), a.schemaValidator); err != nil {
+		allErrs = append(allErrs, field.Invalid(field.NewPath(""), u.UnstructuredContent(), err.Error()))
 	}
-	customResource := customResourceObject.UnstructuredContent()
+	allErrs = append(allErrs, a.ValidateScaleSpec(ctx, u, scale)...)
+	allErrs = append(allErrs, a.ValidateScaleStatus(ctx, u, scale)...)
 
-	if err = apiservervalidation.ValidateCustomResource(customResource, a.schemaValidator); err != nil {
+	return allErrs
-		return field.ErrorList{field.Invalid(field.NewPath(""), customResource, err.Error())}
-	}
-
-	if scale != nil {
-		// validate specReplicas
-		specReplicasPath := strings.TrimPrefix(scale.SpecReplicasPath, ".") // ignore leading period
-		specReplicas, _, err := unstructured.NestedInt64(customResource, strings.Split(specReplicasPath, ".")...)
-		if err != nil {
-			return field.ErrorList{field.Invalid(field.NewPath(scale.SpecReplicasPath), specReplicas, err.Error())}
-		}
-		if specReplicas < 0 {
-			return field.ErrorList{field.Invalid(field.NewPath(scale.SpecReplicasPath), specReplicas, "should be a non-negative integer")}
-		}
-		if specReplicas > math.MaxInt32 {
-			return field.ErrorList{field.Invalid(field.NewPath(scale.SpecReplicasPath), specReplicas, fmt.Sprintf("should be less than or equal to %v", math.MaxInt32))}
-		}
-
-		// validate statusReplicas
-		statusReplicasPath := strings.TrimPrefix(scale.StatusReplicasPath, ".") // ignore leading period
-		statusReplicas, _, err := unstructured.NestedInt64(customResource, strings.Split(statusReplicasPath, ".")...)
-		if err != nil {
-			return field.ErrorList{field.Invalid(field.NewPath(scale.StatusReplicasPath), statusReplicas, err.Error())}
-		}
-		if statusReplicas < 0 {
-			return field.ErrorList{field.Invalid(field.NewPath(scale.StatusReplicasPath), statusReplicas, "should be a non-negative integer")}
-		}
-		if statusReplicas > math.MaxInt32 {
-			return field.ErrorList{field.Invalid(field.NewPath(scale.StatusReplicasPath), statusReplicas, fmt.Sprintf("should be less than or equal to %v", math.MaxInt32))}
-		}
-
-		// validate labelSelector
-		if scale.LabelSelectorPath != nil {
-			labelSelectorPath := strings.TrimPrefix(*scale.LabelSelectorPath, ".") // ignore leading period
-			labelSelector, _, err := unstructured.NestedString(customResource, strings.Split(labelSelectorPath, ".")...)
-			if err != nil {
-				return field.ErrorList{field.Invalid(field.NewPath(*scale.LabelSelectorPath), labelSelector, err.Error())}
-			}
-		}
-	}
-
-	return validation.ValidateObjectMetaAccessor(accessor, a.namespaceScoped, validation.NameIsDNSSubdomain, field.NewPath("metadata"))
 }
 
 func (a customResourceValidator) ValidateUpdate(ctx context.Context, obj, old runtime.Object, scale *apiextensions.CustomResourceSubresourceScale) field.ErrorList {
+	u, ok := obj.(*unstructured.Unstructured)
+	if !ok {
+		return field.ErrorList{field.Invalid(field.NewPath(""), u, fmt.Sprintf("has type %T. Must be a pointer to an Unstructured type", u))}
+	}
 	objAccessor, err := meta.Accessor(obj)
 	if err != nil {
 		return field.ErrorList{field.Invalid(field.NewPath("metadata"), nil, err.Error())}
@@ -118,69 +81,28 @@ func (a customResourceValidator) ValidateUpdate(ctx context.Context, obj, old ru
 	if err != nil {
 		return field.ErrorList{field.Invalid(field.NewPath("metadata"), nil, err.Error())}
 	}
-	typeAccessor, err := meta.TypeAccessor(obj)
+
-	if err != nil {
+	if errs := a.ValidateTypeMeta(ctx, u); len(errs) > 0 {
-		return field.ErrorList{field.Invalid(field.NewPath("kind"), nil, err.Error())}
+		return errs
-	}
-	if typeAccessor.GetKind() != a.kind.Kind {
-		return field.ErrorList{field.Invalid(field.NewPath("kind"), typeAccessor.GetKind(), fmt.Sprintf("must be %v", a.kind.Kind))}
-	}
-	if typeAccessor.GetAPIVersion() != a.kind.Group+"/"+a.kind.Version {
-		return field.ErrorList{field.Invalid(field.NewPath("apiVersion"), typeAccessor.GetAPIVersion(), fmt.Sprintf("must be %v", a.kind.Group+"/"+a.kind.Version))}
 	}
 
-	customResourceObject, ok := obj.(*unstructured.Unstructured)
+	var allErrs field.ErrorList
-	// this will never happen.
+
-	if !ok {
+	allErrs = append(allErrs, validation.ValidateObjectMetaAccessorUpdate(objAccessor, oldAccessor, field.NewPath("metadata"))...)
-		return field.ErrorList{field.Invalid(field.NewPath(""), customResourceObject, fmt.Sprintf("has type %T. Must be a pointer to an Unstructured type", customResourceObject))}
+	if err = apiservervalidation.ValidateCustomResource(u.UnstructuredContent(), a.schemaValidator); err != nil {
+		allErrs = append(allErrs, field.Invalid(field.NewPath(""), u.UnstructuredContent(), err.Error()))
 	}
-	customResource := customResourceObject.UnstructuredContent()
+	allErrs = append(allErrs, a.ValidateScaleSpec(ctx, u, scale)...)
+	allErrs = append(allErrs, a.ValidateScaleStatus(ctx, u, scale)...)
 
-	if err = apiservervalidation.ValidateCustomResource(customResource, a.schemaValidator); err != nil {
+	return allErrs
-		return field.ErrorList{field.Invalid(field.NewPath(""), customResource, err.Error())}
-	}
-
-	if scale != nil {
-		// validate specReplicas
-		specReplicasPath := strings.TrimPrefix(scale.SpecReplicasPath, ".") // ignore leading period
-		specReplicas, _, err := unstructured.NestedInt64(customResource, strings.Split(specReplicasPath, ".")...)
-		if err != nil {
-			return field.ErrorList{field.Invalid(field.NewPath(scale.SpecReplicasPath), specReplicas, err.Error())}
-		}
-		if specReplicas < 0 {
-			return field.ErrorList{field.Invalid(field.NewPath(scale.SpecReplicasPath), specReplicas, "should be a non-negative integer")}
-		}
-		if specReplicas > math.MaxInt32 {
-			return field.ErrorList{field.Invalid(field.NewPath(scale.SpecReplicasPath), specReplicas, fmt.Sprintf("should be less than or equal to %v", math.MaxInt32))}
-		}
-
-		// validate statusReplicas
-		statusReplicasPath := strings.TrimPrefix(scale.StatusReplicasPath, ".") // ignore leading period
-		statusReplicas, _, err := unstructured.NestedInt64(customResource, strings.Split(statusReplicasPath, ".")...)
-		if err != nil {
-			return field.ErrorList{field.Invalid(field.NewPath(scale.StatusReplicasPath), statusReplicas, err.Error())}
-		}
-		if statusReplicas < 0 {
-			return field.ErrorList{field.Invalid(field.NewPath(scale.StatusReplicasPath), statusReplicas, "should be a non-negative integer")}
-		}
-		if statusReplicas > math.MaxInt32 {
-			return field.ErrorList{field.Invalid(field.NewPath(scale.StatusReplicasPath), statusReplicas, fmt.Sprintf("should be less than or equal to %v", math.MaxInt32))}
-		}
-
-		// validate labelSelector
-		if scale.LabelSelectorPath != nil {
-			labelSelectorPath := strings.TrimPrefix(*scale.LabelSelectorPath, ".") // ignore leading period
-			labelSelector, _, err := unstructured.NestedString(customResource, strings.Split(labelSelectorPath, ".")...)
-			if err != nil {
-				return field.ErrorList{field.Invalid(field.NewPath(*scale.LabelSelectorPath), labelSelector, err.Error())}
-			}
-		}
-	}
-
-	return validation.ValidateObjectMetaAccessorUpdate(objAccessor, oldAccessor, field.NewPath("metadata"))
 }
 
 func (a customResourceValidator) ValidateStatusUpdate(ctx context.Context, obj, old runtime.Object, scale *apiextensions.CustomResourceSubresourceScale) field.ErrorList {
+	u, ok := obj.(*unstructured.Unstructured)
+	if !ok {
+		return field.ErrorList{field.Invalid(field.NewPath(""), u, fmt.Sprintf("has type %T. Must be a pointer to an Unstructured type", u))}
+	}
 	objAccessor, err := meta.Accessor(obj)
 	if err != nil {
 		return field.ErrorList{field.Invalid(field.NewPath("metadata"), nil, err.Error())}
@@ -189,53 +111,85 @@ func (a customResourceValidator) ValidateStatusUpdate(ctx context.Context, obj,
 	if err != nil {
 		return field.ErrorList{field.Invalid(field.NewPath("metadata"), nil, err.Error())}
 	}
+
+	if errs := a.ValidateTypeMeta(ctx, u); len(errs) > 0 {
+		return errs
+	}
+
+	var allErrs field.ErrorList
+
+	allErrs = append(allErrs, validation.ValidateObjectMetaAccessorUpdate(objAccessor, oldAccessor, field.NewPath("metadata"))...)
+	if err = apiservervalidation.ValidateCustomResource(u.UnstructuredContent(), a.schemaValidator); err != nil {
+		allErrs = append(allErrs, field.Invalid(field.NewPath(""), u.UnstructuredContent(), err.Error()))
+	}
+	allErrs = append(allErrs, a.ValidateScaleStatus(ctx, u, scale)...)
+
+	return allErrs
+}
+
+func (a customResourceValidator) ValidateTypeMeta(ctx context.Context, obj *unstructured.Unstructured) field.ErrorList {
 	typeAccessor, err := meta.TypeAccessor(obj)
 	if err != nil {
 		return field.ErrorList{field.Invalid(field.NewPath("kind"), nil, err.Error())}
 	}
+
+	var allErrs field.ErrorList
 	if typeAccessor.GetKind() != a.kind.Kind {
-		return field.ErrorList{field.Invalid(field.NewPath("kind"), typeAccessor.GetKind(), fmt.Sprintf("must be %v", a.kind.Kind))}
+		allErrs = append(allErrs, field.Invalid(field.NewPath("kind"), typeAccessor.GetKind(), fmt.Sprintf("must be %v", a.kind.Kind)))
 	}
 	if typeAccessor.GetAPIVersion() != a.kind.Group+"/"+a.kind.Version {
-		return field.ErrorList{field.Invalid(field.NewPath("apiVersion"), typeAccessor.GetAPIVersion(), fmt.Sprintf("must be %v", a.kind.Group+"/"+a.kind.Version))}
+		allErrs = append(allErrs, field.Invalid(field.NewPath("apiVersion"), typeAccessor.GetAPIVersion(), fmt.Sprintf("must be %v", a.kind.Group+"/"+a.kind.Version)))
 	}
-
+	return allErrs
-	customResourceObject, ok := obj.(*unstructured.Unstructured)
+}
-	// this will never happen.
+
-	if !ok {
+func (a customResourceValidator) ValidateScaleSpec(ctx context.Context, obj *unstructured.Unstructured, scale *apiextensions.CustomResourceSubresourceScale) field.ErrorList {
-		return field.ErrorList{field.Invalid(field.NewPath(""), customResourceObject, fmt.Sprintf("has type %T. Must be a pointer to an Unstructured type", customResourceObject))}
+	if scale == nil {
-	}
+		return nil
-	customResource := customResourceObject.UnstructuredContent()
+	}
-
+
-	// validate only the status
+	var allErrs field.ErrorList
-	customResourceStatus := customResource["status"]
+
-	if err = apiservervalidation.ValidateCustomResource(customResourceStatus, a.statusSchemaValidator); err != nil {
+	// validate specReplicas
-		return field.ErrorList{field.Invalid(field.NewPath("status"), customResourceStatus, err.Error())}
+	specReplicasPath := strings.TrimPrefix(scale.SpecReplicasPath, ".") // ignore leading period
-	}
+	specReplicas, _, err := unstructured.NestedInt64(obj.UnstructuredContent(), strings.Split(specReplicasPath, ".")...)
-
+	if err != nil {
-	if scale != nil {
+		allErrs = append(allErrs, field.Invalid(field.NewPath(scale.SpecReplicasPath), specReplicas, err.Error()))
-		// validate statusReplicas
+	} else if specReplicas < 0 {
-		statusReplicasPath := strings.TrimPrefix(scale.StatusReplicasPath, ".") // ignore leading period
+		allErrs = append(allErrs, field.Invalid(field.NewPath(scale.SpecReplicasPath), specReplicas, "should be a non-negative integer"))
-		statusReplicas, _, err := unstructured.NestedInt64(customResource, strings.Split(statusReplicasPath, ".")...)
+	} else if specReplicas > math.MaxInt32 {
-		if err != nil {
+		allErrs = append(allErrs, field.Invalid(field.NewPath(scale.SpecReplicasPath), specReplicas, fmt.Sprintf("should be less than or equal to %v", math.MaxInt32)))
-			return field.ErrorList{field.Invalid(field.NewPath(scale.StatusReplicasPath), statusReplicas, err.Error())}
+	}
-		}
+
-		if statusReplicas < 0 {
+	return allErrs
-			return field.ErrorList{field.Invalid(field.NewPath(scale.StatusReplicasPath), statusReplicas, "should be a non-negative integer")}
+}
-		}
+
-		if statusReplicas > math.MaxInt32 {
+func (a customResourceValidator) ValidateScaleStatus(ctx context.Context, obj *unstructured.Unstructured, scale *apiextensions.CustomResourceSubresourceScale) field.ErrorList {
-			return field.ErrorList{field.Invalid(field.NewPath(scale.StatusReplicasPath), statusReplicas, fmt.Sprintf("should be less than or equal to %v", math.MaxInt32))}
+	if scale == nil {
-		}
+		return nil
-
+	}
-		// validate labelSelector
+
-		if scale.LabelSelectorPath != nil {
+	var allErrs field.ErrorList
-			labelSelectorPath := strings.TrimPrefix(*scale.LabelSelectorPath, ".") // ignore leading period
+
-			labelSelector, _, err := unstructured.NestedString(customResource, strings.Split(labelSelectorPath, ".")...)
+	// validate statusReplicas
-			if err != nil {
+	statusReplicasPath := strings.TrimPrefix(scale.StatusReplicasPath, ".") // ignore leading period
-				return field.ErrorList{field.Invalid(field.NewPath(*scale.LabelSelectorPath), labelSelector, err.Error())}
+	statusReplicas, _, err := unstructured.NestedInt64(obj.UnstructuredContent(), strings.Split(statusReplicasPath, ".")...)
-			}
+	if err != nil {
-		}
+		allErrs = append(allErrs, field.Invalid(field.NewPath(scale.StatusReplicasPath), statusReplicas, err.Error()))
-	}
+	} else if statusReplicas < 0 {
-
+		allErrs = append(allErrs, field.Invalid(field.NewPath(scale.StatusReplicasPath), statusReplicas, "should be a non-negative integer"))
-	return validation.ValidateObjectMetaAccessorUpdate(objAccessor, oldAccessor, field.NewPath("metadata"))
+	} else if statusReplicas > math.MaxInt32 {
+		allErrs = append(allErrs, field.Invalid(field.NewPath(scale.StatusReplicasPath), statusReplicas, fmt.Sprintf("should be less than or equal to %v", math.MaxInt32)))
+	}
+
+	// validate labelSelector
+	if scale.LabelSelectorPath != nil {
+		labelSelectorPath := strings.TrimPrefix(*scale.LabelSelectorPath, ".") // ignore leading period
+		labelSelector, _, err := unstructured.NestedString(obj.UnstructuredContent(), strings.Split(labelSelectorPath, ".")...)
+		if err != nil {
+			allErrs = append(allErrs, field.Invalid(field.NewPath(*scale.LabelSelectorPath), labelSelector, err.Error()))
+		}
+	}
+
+	return allErrs
 }