Merge pull request #121337 from alexzielenski/apiserver/apiextensions/cleanup-validator

apiextensions-apiserver: simplify validation codepath

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

@@ -805,7 +805,7 @@ func (r *crdHandler) getOrCreateServingInfoFor(uid types.UID, name string) (*crd
 				kind,
 				validator,
 				statusValidator,
-				structuralSchemas,
+				structuralSchemas[v.Name],
 				statusSpec,
 				scaleSpec,
 			),

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

@@ -18,6 +18,7 @@ package customresource
 
 import (
 	"context"
+	"fmt"
 
 	"sigs.k8s.io/structured-merge-diff/v4/fieldpath"
 
@@ -84,36 +85,31 @@ func (a statusStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.O
 
 // ValidateUpdate is the default update validation for an end user updating status.
 func (a statusStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList {
-	var errs field.ErrorList
-	errs = append(errs, a.customResourceStrategy.validator.ValidateStatusUpdate(ctx, obj, old, a.scale)...)
-
 	uNew, ok := obj.(*unstructured.Unstructured)
 	if !ok {
-		return errs
+		return field.ErrorList{field.Invalid(field.NewPath(""), obj, fmt.Sprintf("has type %T. Must be a pointer to an Unstructured type", obj))}
 	}
 	uOld, ok := old.(*unstructured.Unstructured)
-	var oldObject map[string]interface{}
 	if !ok {
-		oldObject = nil
+		return field.ErrorList{field.Invalid(field.NewPath(""), old, fmt.Sprintf("has type %T. Must be a pointer to an Unstructured type", old))}
-	} else {
-		oldObject = uOld.Object
 	}
 
-	v := obj.GetObjectKind().GroupVersionKind().Version
+	var errs field.ErrorList
+	errs = append(errs, a.customResourceStrategy.validator.ValidateStatusUpdate(ctx, uNew, uOld, a.scale)...)
 
 	// ratcheting validation of x-kubernetes-list-type value map and set
-	if newErrs := structurallisttype.ValidateListSetsAndMaps(nil, a.structuralSchemas[v], uNew.Object); len(newErrs) > 0 {
+	if newErrs := structurallisttype.ValidateListSetsAndMaps(nil, a.structuralSchema, uNew.Object); len(newErrs) > 0 {
-		if oldErrs := structurallisttype.ValidateListSetsAndMaps(nil, a.structuralSchemas[v], oldObject); len(oldErrs) == 0 {
+		if oldErrs := structurallisttype.ValidateListSetsAndMaps(nil, a.structuralSchema, uOld.Object); len(oldErrs) == 0 {
 			errs = append(errs, newErrs...)
 		}
 	}
 
 	// validate x-kubernetes-validations rules
-	if celValidator, ok := a.customResourceStrategy.celValidators[v]; ok {
+	if celValidator := a.customResourceStrategy.celValidator; celValidator != nil {
 		if has, err := hasBlockingErr(errs); has {
 			errs = append(errs, err)
 		} else {
-			err, _ := celValidator.Validate(ctx, nil, a.customResourceStrategy.structuralSchemas[v], uNew.Object, oldObject, celconfig.RuntimeCELCostBudget)
+			err, _ := celValidator.Validate(ctx, nil, a.customResourceStrategy.structuralSchema, uNew.Object, uOld.Object, celconfig.RuntimeCELCostBudget)
 			errs = append(errs, err...)
 		}
 	}

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

@@ -198,9 +198,7 @@ func TestStatusStrategyValidateUpdate(t *testing.T) {
 	}
 	strategy.customResourceStrategy.validator.kind = kind
 	ss, _ := structuralschema.NewStructural(crd.Spec.Versions[0].Schema.OpenAPIV3Schema)
-	strategy.structuralSchemas = map[string]*structuralschema.Structural{
+	strategy.structuralSchema = ss
-		crd.Spec.Versions[0].Name: ss,
-	}
 
 	ctx := context.TODO()
 

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

@@ -18,6 +18,7 @@ package customresource
 
 import (
 	"context"
+	"fmt"
 
 	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
 	structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
@@ -44,29 +45,25 @@ import (
 	"sigs.k8s.io/structured-merge-diff/v4/fieldpath"
 )
 
-// customResourceStrategy implements behavior for CustomResources.
+// customResourceStrategy implements behavior for CustomResources for a single
+// version
 type customResourceStrategy struct {
 	runtime.ObjectTyper
 	names.NameGenerator
 
 	namespaceScoped  bool
 	validator        customResourceValidator
-	structuralSchemas map[string]*structuralschema.Structural
+	structuralSchema *structuralschema.Structural
-	celValidators     map[string]*cel.Validator
+	celValidator     *cel.Validator
 	status           *apiextensions.CustomResourceSubresourceStatus
 	scale            *apiextensions.CustomResourceSubresourceScale
 	kind             schema.GroupVersionKind
 }
 
-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 {
+func NewStrategy(typer runtime.ObjectTyper, namespaceScoped bool, kind schema.GroupVersionKind, schemaValidator, statusSchemaValidator validation.SchemaValidator, structuralSchema *structuralschema.Structural, status *apiextensions.CustomResourceSubresourceStatus, scale *apiextensions.CustomResourceSubresourceScale) customResourceStrategy {
-	celValidators := map[string]*cel.Validator{}
+	var celValidator *cel.Validator
 	if utilfeature.DefaultFeatureGate.Enabled(features.CustomResourceValidationExpressions) {
-		for name, s := range structuralSchemas {
+		celValidator = cel.NewValidator(structuralSchema, true, celconfig.PerCallLimit) // CEL programs are compiled and cached here
-			v := cel.NewValidator(s, true, celconfig.PerCallLimit) // CEL programs are compiled and cached here
-			if v != nil {
-				celValidators[name] = v
-			}
-		}
 	}
 
 	return customResourceStrategy{
@@ -81,8 +78,8 @@ func NewStrategy(typer runtime.ObjectTyper, namespaceScoped bool, kind schema.Gr
 			schemaValidator:       schemaValidator,
 			statusSchemaValidator: statusSchemaValidator,
 		},
-		structuralSchemas: structuralSchemas,
+		structuralSchema: structuralSchema,
-		celValidators:     celValidators,
+		celValidator:     celValidator,
 		kind:             kind,
 	}
 }
@@ -163,27 +160,29 @@ func copyNonMetadata(original map[string]interface{}) map[string]interface{} {
 
 // Validate validates a new CustomResource.
 func (a customResourceStrategy) Validate(ctx context.Context, obj runtime.Object) 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", obj))}
+	}
+
 	var errs field.ErrorList
-	errs = append(errs, a.validator.Validate(ctx, obj, a.scale)...)
+	errs = append(errs, a.validator.Validate(ctx, u, a.scale)...)
 
 	// validate embedded resources
-	if u, ok := obj.(*unstructured.Unstructured); ok {
+	errs = append(errs, schemaobjectmeta.Validate(nil, u.Object, a.structuralSchema, false)...)
-		v := obj.GetObjectKind().GroupVersionKind().Version
-		errs = append(errs, schemaobjectmeta.Validate(nil, u.Object, a.structuralSchemas[v], false)...)
 
 	// validate x-kubernetes-list-type "map" and "set" invariant
-		errs = append(errs, structurallisttype.ValidateListSetsAndMaps(nil, a.structuralSchemas[v], u.Object)...)
+	errs = append(errs, structurallisttype.ValidateListSetsAndMaps(nil, a.structuralSchema, u.Object)...)
 
 	// validate x-kubernetes-validations rules
-		if celValidator, ok := a.celValidators[v]; ok {
+	if celValidator := a.celValidator; celValidator != nil {
 		if has, err := hasBlockingErr(errs); has {
 			errs = append(errs, err)
 		} else {
-				err, _ := celValidator.Validate(ctx, nil, a.structuralSchemas[v], u.Object, nil, celconfig.RuntimeCELCostBudget)
+			err, _ := celValidator.Validate(ctx, nil, a.structuralSchema, u.Object, nil, celconfig.RuntimeCELCostBudget)
 			errs = append(errs, err...)
 		}
 	}
-	}
 
 	return errs
 }
@@ -234,33 +233,32 @@ func (customResourceStrategy) AllowUnconditionalUpdate() bool {
 
 // ValidateUpdate is the default update validation for an end user updating status.
 func (a customResourceStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList {
-	var errs field.ErrorList
-	errs = append(errs, a.validator.ValidateUpdate(ctx, obj, old, a.scale)...)
-
 	uNew, ok := obj.(*unstructured.Unstructured)
 	if !ok {
-		return errs
+		return field.ErrorList{field.Invalid(field.NewPath(""), obj, fmt.Sprintf("has type %T. Must be a pointer to an Unstructured type", obj))}
 	}
 	uOld, ok := old.(*unstructured.Unstructured)
 	if !ok {
-		return errs
+		return field.ErrorList{field.Invalid(field.NewPath(""), old, fmt.Sprintf("has type %T. Must be a pointer to an Unstructured type", old))}
 	}
 
+	var errs field.ErrorList
+	errs = append(errs, a.validator.ValidateUpdate(ctx, uNew, uOld, a.scale)...)
+
 	// Checks the embedded objects. We don't make a difference between update and create for those.
-	v := obj.GetObjectKind().GroupVersionKind().Version
+	errs = append(errs, schemaobjectmeta.Validate(nil, uNew.Object, a.structuralSchema, false)...)
-	errs = append(errs, schemaobjectmeta.Validate(nil, uNew.Object, a.structuralSchemas[v], false)...)
 
 	// ratcheting validation of x-kubernetes-list-type value map and set
-	if oldErrs := structurallisttype.ValidateListSetsAndMaps(nil, a.structuralSchemas[v], uOld.Object); len(oldErrs) == 0 {
+	if oldErrs := structurallisttype.ValidateListSetsAndMaps(nil, a.structuralSchema, uOld.Object); len(oldErrs) == 0 {
-		errs = append(errs, structurallisttype.ValidateListSetsAndMaps(nil, a.structuralSchemas[v], uNew.Object)...)
+		errs = append(errs, structurallisttype.ValidateListSetsAndMaps(nil, a.structuralSchema, uNew.Object)...)
 	}
 
 	// validate x-kubernetes-validations rules
-	if celValidator, ok := a.celValidators[v]; ok {
+	if celValidator := a.celValidator; celValidator != nil {
 		if has, err := hasBlockingErr(errs); has {
 			errs = append(errs, err)
 		} else {
-			err, _ := celValidator.Validate(ctx, nil, a.structuralSchemas[v], uNew.Object, uOld.Object, celconfig.RuntimeCELCostBudget)
+			err, _ := celValidator.Validate(ctx, nil, a.structuralSchema, uNew.Object, uOld.Object, celconfig.RuntimeCELCostBudget)
 			errs = append(errs, err...)
 		}
 	}

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

@@ -27,7 +27,6 @@ import (
 	"k8s.io/apimachinery/pkg/api/validation"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/sets"
 	apimachineryvalidation "k8s.io/apimachinery/pkg/util/validation"
@@ -44,58 +43,32 @@ type customResourceValidator struct {
 	statusSchemaValidator apiextensionsvalidation.SchemaValidator
 }
 
-func (a customResourceValidator) Validate(ctx context.Context, obj runtime.Object, scale *apiextensions.CustomResourceSubresourceScale) field.ErrorList {
+func (a customResourceValidator) Validate(ctx context.Context, obj *unstructured.Unstructured, scale *apiextensions.CustomResourceSubresourceScale) field.ErrorList {
-	u, ok := obj.(*unstructured.Unstructured)
+	if errs := a.ValidateTypeMeta(ctx, obj); len(errs) > 0 {
-	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())}
-	}
-
-	if errs := a.ValidateTypeMeta(ctx, u); len(errs) > 0 {
 		return errs
 	}
 
 	var allErrs field.ErrorList
 
-	allErrs = append(allErrs, validation.ValidateObjectMetaAccessor(accessor, a.namespaceScoped, validation.NameIsDNSSubdomain, field.NewPath("metadata"))...)
+	allErrs = append(allErrs, validation.ValidateObjectMetaAccessor(obj, a.namespaceScoped, validation.NameIsDNSSubdomain, field.NewPath("metadata"))...)
-	allErrs = append(allErrs, apiextensionsvalidation.ValidateCustomResource(nil, u.UnstructuredContent(), a.schemaValidator)...)
+	allErrs = append(allErrs, apiextensionsvalidation.ValidateCustomResource(nil, obj.UnstructuredContent(), a.schemaValidator)...)
-	allErrs = append(allErrs, a.ValidateScaleSpec(ctx, u, scale)...)
+	allErrs = append(allErrs, a.ValidateScaleSpec(ctx, obj, scale)...)
-	allErrs = append(allErrs, a.ValidateScaleStatus(ctx, u, scale)...)
+	allErrs = append(allErrs, a.ValidateScaleStatus(ctx, obj, scale)...)
 
 	return allErrs
 }
 
-func (a customResourceValidator) ValidateUpdate(ctx context.Context, obj, old runtime.Object, scale *apiextensions.CustomResourceSubresourceScale) field.ErrorList {
+func (a customResourceValidator) ValidateUpdate(ctx context.Context, obj, old *unstructured.Unstructured, scale *apiextensions.CustomResourceSubresourceScale) field.ErrorList {
-	u, ok := obj.(*unstructured.Unstructured)
+	if errs := a.ValidateTypeMeta(ctx, obj); len(errs) > 0 {
-	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())}
-	}
-	oldAccessor, err := meta.Accessor(old)
-	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"))...)
+	allErrs = append(allErrs, validation.ValidateObjectMetaAccessorUpdate(obj, old, field.NewPath("metadata"))...)
-	allErrs = append(allErrs, apiextensionsvalidation.ValidateCustomResourceUpdate(nil, u.UnstructuredContent(), oldU.UnstructuredContent(), a.schemaValidator)...)
+	allErrs = append(allErrs, apiextensionsvalidation.ValidateCustomResourceUpdate(nil, obj.UnstructuredContent(), old.UnstructuredContent(), a.schemaValidator)...)
-	allErrs = append(allErrs, a.ValidateScaleSpec(ctx, u, scale)...)
+	allErrs = append(allErrs, a.ValidateScaleSpec(ctx, obj, scale)...)
-	allErrs = append(allErrs, a.ValidateScaleStatus(ctx, u, scale)...)
+	allErrs = append(allErrs, a.ValidateScaleStatus(ctx, obj, scale)...)
 
 	return allErrs
 }
@@ -119,35 +92,18 @@ func validateKubeFinalizerName(stringValue string, fldPath *field.Path) []string
 	return allWarnings
 }
 
-func (a customResourceValidator) ValidateStatusUpdate(ctx context.Context, obj, old runtime.Object, scale *apiextensions.CustomResourceSubresourceScale) field.ErrorList {
+func (a customResourceValidator) ValidateStatusUpdate(ctx context.Context, obj, old *unstructured.Unstructured, scale *apiextensions.CustomResourceSubresourceScale) field.ErrorList {
-	u, ok := obj.(*unstructured.Unstructured)
+	if errs := a.ValidateTypeMeta(ctx, obj); len(errs) > 0 {
-	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())}
-	}
-	oldAccessor, err := meta.Accessor(old)
-	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"))...)
+	allErrs = append(allErrs, validation.ValidateObjectMetaAccessorUpdate(obj, old, field.NewPath("metadata"))...)
-	if status, hasStatus := u.UnstructuredContent()["status"]; hasStatus {
+	if status, hasStatus := obj.UnstructuredContent()["status"]; hasStatus {
-		allErrs = append(allErrs, apiextensionsvalidation.ValidateCustomResourceUpdate(nil, status, oldU.UnstructuredContent()["status"], a.statusSchemaValidator)...)
+		allErrs = append(allErrs, apiextensionsvalidation.ValidateCustomResourceUpdate(nil, status, old.UnstructuredContent()["status"], a.statusSchemaValidator)...)
 	}
-	allErrs = append(allErrs, a.ValidateScaleStatus(ctx, u, scale)...)
+	allErrs = append(allErrs, a.ValidateScaleStatus(ctx, obj, scale)...)
 
 	return allErrs
 }