refactor: add ValidateCustomResourceUpdate to support future validators for CRD Updates

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go

@@ -79,8 +79,6 @@ import (
 	"k8s.io/klog/v2"
 	"k8s.io/kube-openapi/pkg/spec3"
 	"k8s.io/kube-openapi/pkg/validation/spec"
-	"k8s.io/kube-openapi/pkg/validation/strfmt"
-	"k8s.io/kube-openapi/pkg/validation/validate"
 )
 
 // crdHandler serves the `/apis` endpoint.
@@ -740,8 +738,9 @@ func (r *crdHandler) getOrCreateServingInfoFor(uid types.UID, name string) (*crd
 			return nil, fmt.Errorf("the server could not properly serve the CR schema")
 		}
 		var internalSchemaProps *apiextensionsinternal.JSONSchemaProps
+		var internalValidationSchema *apiextensionsinternal.CustomResourceValidation
 		if validationSchema != nil {
-			internalValidationSchema := &apiextensionsinternal.CustomResourceValidation{}
+			internalValidationSchema = &apiextensionsinternal.CustomResourceValidation{}
 			if err := apiextensionsv1.Convert_v1_CustomResourceValidation_To_apiextensions_CustomResourceValidation(validationSchema, internalValidationSchema, nil); err != nil {
 				return nil, fmt.Errorf("failed to convert CRD validation to internal version: %v", err)
 			}
@@ -753,7 +752,7 @@ func (r *crdHandler) getOrCreateServingInfoFor(uid types.UID, name string) (*crd
 		}
 
 		var statusSpec *apiextensionsinternal.CustomResourceSubresourceStatus
-		var statusValidator *validate.SchemaValidator
+		var statusValidator apiservervalidation.SchemaValidator
 		subresources, err := apiextensionshelpers.GetSubresourcesForVersion(crd, v.Name)
 		if err != nil {
 			utilruntime.HandleError(err)
@@ -768,11 +767,10 @@ func (r *crdHandler) getOrCreateServingInfoFor(uid types.UID, name string) (*crd
 			// for the status subresource, validate only against the status schema
 			if internalValidationSchema != nil && internalValidationSchema.OpenAPIV3Schema != nil && internalValidationSchema.OpenAPIV3Schema.Properties != nil {
 				if statusSchema, ok := internalValidationSchema.OpenAPIV3Schema.Properties["status"]; ok {
-					openapiSchema := &spec.Schema{}
+					statusValidator, _, err = apiservervalidation.NewSchemaValidator(&statusSchema)
-					if err := apiservervalidation.ConvertJSONSchemaPropsWithPostProcess(&statusSchema, openapiSchema, apiservervalidation.StripUnsupportedFormatsPostProcess); err != nil {
+					if err != nil {
 						return nil, err
 					}
-					statusValidator = validate.NewSchemaValidator(openapiSchema, nil, "", strfmt.Default)
 				}
 			}
 		}

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation.go

@@ -29,8 +29,34 @@ import (
 	"k8s.io/kube-openapi/pkg/validation/validate"
 )
 
+type SchemaValidator interface {
+	SchemaCreateValidator
+	ValidateUpdate(new, old interface{}) *validate.Result
+}
+
+type SchemaCreateValidator interface {
+	Validate(value interface{}) *validate.Result
+}
+
+// basicSchemaValidator wraps a kube-openapi SchemaCreateValidator to
+// support ValidateUpdate. It implements ValidateUpdate by simply validating
+// the new value via kube-openapi, ignoring the old value.
+type basicSchemaValidator struct {
+	*validate.SchemaValidator
+}
+
+func (s basicSchemaValidator) ValidateUpdate(new, old interface{}) *validate.Result {
+	return s.Validate(new)
+}
+
 // NewSchemaValidator creates an openapi schema validator for the given CRD validation.
-func NewSchemaValidator(customResourceValidation *apiextensions.JSONSchemaProps) (*validate.SchemaValidator, *spec.Schema, error) {
+//
+// If feature `CRDValidationRatcheting` is disabled, this returns validator which
+// validates all `Update`s and `Create`s as a `Create` - without considering old value.
+//
+// If feature `CRDValidationRatcheting` is enabled - the validator returned
+// will support ratcheting unchanged correlatable fields across an update.
+func NewSchemaValidator(customResourceValidation *apiextensions.JSONSchemaProps) (SchemaValidator, *spec.Schema, error) {
 	// Convert CRD schema to openapi schema
 	openapiSchema := &spec.Schema{}
 	if customResourceValidation != nil {
@@ -39,12 +65,35 @@ func NewSchemaValidator(customResourceValidation *apiextensions.JSONSchemaProps)
 			return nil, nil, err
 		}
 	}
-	return validate.NewSchemaValidator(openapiSchema, nil, "", strfmt.Default), openapiSchema, nil
+
+	return basicSchemaValidator{validate.NewSchemaValidator(openapiSchema, nil, "", strfmt.Default)}, openapiSchema, nil
+}
+
+// ValidateCustomResourceUpdate validates the transition of Custom Resource from
+// `old` to `new` against the schema in the CustomResourceDefinition.
+// Both customResource and old represent a JSON data structures.
+//
+// If feature `CRDValidationRatcheting` is disabled, this behaves identically to
+// ValidateCustomResource(customResource).
+func ValidateCustomResourceUpdate(fldPath *field.Path, customResource, old interface{}, validator SchemaValidator) field.ErrorList {
+	// Additional feature gate check for sanity
+	if !utilfeature.DefaultFeatureGate.Enabled(features.CRDValidationRatcheting) {
+		return ValidateCustomResource(nil, customResource, validator)
+	} else if validator == nil {
+		return nil
+	}
+
+	result := validator.ValidateUpdate(customResource, old)
+	if result.IsValid() {
+		return nil
+	}
+
+	return kubeOpenAPIResultToFieldErrors(fldPath, result)
 }
 
 // ValidateCustomResource validates the Custom Resource against the schema in the CustomResourceDefinition.
 // CustomResource is a JSON data structure.
-func ValidateCustomResource(fldPath *field.Path, customResource interface{}, validator *validate.SchemaValidator) field.ErrorList {
+func ValidateCustomResource(fldPath *field.Path, customResource interface{}, validator SchemaCreateValidator) field.ErrorList {
 	if validator == nil {
 		return nil
 	}
@@ -53,6 +102,11 @@ func ValidateCustomResource(fldPath *field.Path, customResource interface{}, val
 	if result.IsValid() {
 		return nil
 	}
+
+	return kubeOpenAPIResultToFieldErrors(fldPath, result)
+}
+
+func kubeOpenAPIResultToFieldErrors(fldPath *field.Path, result *validate.Result) field.ErrorList {
 	var allErrs field.ErrorList
 	for _, err := range result.Errors {
 		switch err := err.(type) {

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

@@ -19,13 +19,12 @@ package customresource
 import (
 	"context"
 
-	"k8s.io/kube-openapi/pkg/validation/validate"
-
 	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
 	structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
 	"k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel"
 	structurallisttype "k8s.io/apiextensions-apiserver/pkg/apiserver/schema/listtype"
 	schemaobjectmeta "k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta"
+	"k8s.io/apiextensions-apiserver/pkg/apiserver/validation"
 	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -58,7 +57,7 @@ type customResourceStrategy struct {
 	kind              schema.GroupVersionKind
 }
 
-func NewStrategy(typer runtime.ObjectTyper, namespaceScoped bool, kind schema.GroupVersionKind, schemaValidator, statusSchemaValidator *validate.SchemaValidator, structuralSchemas map[string]*structuralschema.Structural, status *apiextensions.CustomResourceSubresourceStatus, scale *apiextensions.CustomResourceSubresourceScale) customResourceStrategy {
+func NewStrategy(typer runtime.ObjectTyper, namespaceScoped bool, kind schema.GroupVersionKind, schemaValidator, statusSchemaValidator validation.SchemaValidator, structuralSchemas map[string]*structuralschema.Structural, status *apiextensions.CustomResourceSubresourceStatus, scale *apiextensions.CustomResourceSubresourceScale) customResourceStrategy {
 	celValidators := map[string]*cel.Validator{}
 	if utilfeature.DefaultFeatureGate.Enabled(features.CustomResourceValidationExpressions) {
 		for name, s := range structuralSchemas {

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

@@ -28,7 +28,6 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/validation/field"
-	"k8s.io/kube-openapi/pkg/validation/validate"
 
 	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
 	apiextensionsvalidation "k8s.io/apiextensions-apiserver/pkg/apiserver/validation"
@@ -37,8 +36,8 @@ import (
 type customResourceValidator struct {
 	namespaceScoped       bool
 	kind                  schema.GroupVersionKind
-	schemaValidator       *validate.SchemaValidator
+	schemaValidator       apiextensionsvalidation.SchemaValidator
-	statusSchemaValidator *validate.SchemaValidator
+	statusSchemaValidator apiextensionsvalidation.SchemaValidator
 }
 
 func (a customResourceValidator) Validate(ctx context.Context, obj runtime.Object, scale *apiextensions.CustomResourceSubresourceScale) field.ErrorList {
@@ -70,6 +69,10 @@ func (a customResourceValidator) ValidateUpdate(ctx context.Context, obj, old ru
 	if !ok {
 		return field.ErrorList{field.Invalid(field.NewPath(""), u, fmt.Sprintf("has type %T. Must be a pointer to an Unstructured type", u))}
 	}
+	oldU, ok := old.(*unstructured.Unstructured)
+	if !ok {
+		return field.ErrorList{field.Invalid(field.NewPath(""), old, 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())}
@@ -86,7 +89,7 @@ func (a customResourceValidator) ValidateUpdate(ctx context.Context, obj, old ru
 	var allErrs field.ErrorList
 
 	allErrs = append(allErrs, validation.ValidateObjectMetaAccessorUpdate(objAccessor, oldAccessor, field.NewPath("metadata"))...)
-	allErrs = append(allErrs, apiextensionsvalidation.ValidateCustomResource(nil, u.UnstructuredContent(), a.schemaValidator)...)
+	allErrs = append(allErrs, apiextensionsvalidation.ValidateCustomResourceUpdate(nil, u.UnstructuredContent(), oldU.UnstructuredContent(), a.schemaValidator)...)
 	allErrs = append(allErrs, a.ValidateScaleSpec(ctx, u, scale)...)
 	allErrs = append(allErrs, a.ValidateScaleStatus(ctx, u, scale)...)
 
@@ -103,6 +106,10 @@ func (a customResourceValidator) ValidateStatusUpdate(ctx context.Context, obj,
 	if !ok {
 		return field.ErrorList{field.Invalid(field.NewPath(""), u, fmt.Sprintf("has type %T. Must be a pointer to an Unstructured type", u))}
 	}
+	oldU, ok := old.(*unstructured.Unstructured)
+	if !ok {
+		return field.ErrorList{field.Invalid(field.NewPath(""), old, 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())}
@@ -119,7 +126,7 @@ func (a customResourceValidator) ValidateStatusUpdate(ctx context.Context, obj,
 	var allErrs field.ErrorList
 
 	allErrs = append(allErrs, validation.ValidateObjectMetaAccessorUpdate(objAccessor, oldAccessor, field.NewPath("metadata"))...)
-	allErrs = append(allErrs, apiextensionsvalidation.ValidateCustomResource(nil, u.UnstructuredContent(), a.schemaValidator)...)
+	allErrs = append(allErrs, apiextensionsvalidation.ValidateCustomResourceUpdate(nil, u.UnstructuredContent(), oldU, a.schemaValidator)...)
 	allErrs = append(allErrs, a.ValidateScaleStatus(ctx, u, scale)...)
 
 	return allErrs