From 31a1c00e49ce299cc531c0e0fe9edc69150eb9d2 Mon Sep 17 00:00:00 2001 From: Alexander Zielenski Date: Wed, 18 Oct 2023 15:08:43 -0700 Subject: [PATCH] cleanup: move unstructured check earlier in status update --- .../customresource/status_strategy.go | 18 +++++------ .../pkg/registry/customresource/validator.go | 30 ++++--------------- 2 files changed, 14 insertions(+), 34 deletions(-) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/status_strategy.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/status_strategy.go index a3b363c798c..ceaf56629b0 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/status_strategy.go +++ b/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,24 +85,21 @@ 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 - } else { - oldObject = uOld.Object + 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.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.structuralSchema, uNew.Object); len(newErrs) > 0 { - if oldErrs := structurallisttype.ValidateListSetsAndMaps(nil, a.structuralSchema, oldObject); len(oldErrs) == 0 { + if oldErrs := structurallisttype.ValidateListSetsAndMaps(nil, a.structuralSchema, uOld.Object); len(oldErrs) == 0 { errs = append(errs, newErrs...) } } @@ -111,7 +109,7 @@ func (a statusStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Obj if has, err := hasBlockingErr(errs); has { errs = append(errs, err) } else { - err, _ := celValidator.Validate(ctx, nil, a.customResourceStrategy.structuralSchema, uNew.Object, oldObject, celconfig.RuntimeCELCostBudget) + err, _ := celValidator.Validate(ctx, nil, a.customResourceStrategy.structuralSchema, uNew.Object, uOld.Object, celconfig.RuntimeCELCostBudget) errs = append(errs, err...) } } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/validator.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/validator.go index 82163fa8221..a1a15108d6a 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/validator.go +++ b/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" @@ -93,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 { - 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))} - } - 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 { +func (a customResourceValidator) ValidateStatusUpdate(ctx context.Context, obj, old *unstructured.Unstructured, scale *apiextensions.CustomResourceSubresourceScale) field.ErrorList { + if errs := a.ValidateTypeMeta(ctx, obj); len(errs) > 0 { return errs } var allErrs field.ErrorList - allErrs = append(allErrs, validation.ValidateObjectMetaAccessorUpdate(objAccessor, oldAccessor, field.NewPath("metadata"))...) - if status, hasStatus := u.UnstructuredContent()["status"]; hasStatus { - allErrs = append(allErrs, apiextensionsvalidation.ValidateCustomResourceUpdate(nil, status, oldU.UnstructuredContent()["status"], a.statusSchemaValidator)...) + allErrs = append(allErrs, validation.ValidateObjectMetaAccessorUpdate(obj, old, field.NewPath("metadata"))...) + if status, hasStatus := obj.UnstructuredContent()["status"]; hasStatus { + 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 }