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 074c4b73c96..efd9b2dff55 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 @@ -22,6 +22,7 @@ import ( "sigs.k8s.io/structured-merge-diff/v4/fieldpath" "k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel" + structurallisttype "k8s.io/apiextensions-apiserver/pkg/apiserver/schema/listtype" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation/field" @@ -91,18 +92,28 @@ func (a statusStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Obj return errs } uOld, ok := old.(*unstructured.Unstructured) + var oldObject map[string]interface{} if !ok { - uOld = nil // as a safety precaution, continue with validation if uOld self cannot be cast + oldObject = nil + } else { + oldObject = uOld.Object } v := obj.GetObjectKind().GroupVersionKind().Version + // 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 oldErrs := structurallisttype.ValidateListSetsAndMaps(nil, a.structuralSchemas[v], oldObject); len(oldErrs) == 0 { + errs = append(errs, newErrs...) + } + } + // validate x-kubernetes-validations rules if celValidator, ok := a.customResourceStrategy.celValidators[v]; ok { if has, err := hasBlockingErr(errs); has { errs = append(errs, err) } else { - err, _ := celValidator.Validate(ctx, nil, a.customResourceStrategy.structuralSchemas[v], uNew.Object, uOld.Object, cel.RuntimeCELCostBudget) + err, _ := celValidator.Validate(ctx, nil, a.customResourceStrategy.structuralSchemas[v], uNew.Object, oldObject, cel.RuntimeCELCostBudget) errs = append(errs, err...) } } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/status_strategy_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/status_strategy_test.go index 8a651d12bb0..a91e8f0587d 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/status_strategy_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/status_strategy_test.go @@ -21,7 +21,12 @@ import ( "reflect" "testing" + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/yaml" ) func TestPrepareForUpdate(t *testing.T) { @@ -136,3 +141,108 @@ func TestPrepareForUpdate(t *testing.T) { } } } + +const listTypeResourceSchema = ` +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: foos.test +spec: + group: test + names: + kind: Foo + listKind: FooList + plural: foos + singular: foo + scope: Cluster + versions: + - name: v1 + schema: + openAPIV3Schema: + type: object + properties: + numArray: + type: array + x-kubernetes-list-type: set + items: + type: object + served: true + storage: true + - name: v2 + schema: + openAPIV3Schema: + type: object + properties: + numArray2: + type: array +` + +func TestStatusStrategyValidateUpdate(t *testing.T) { + crdV1 := &apiextensionsv1.CustomResourceDefinition{} + err := yaml.Unmarshal([]byte(listTypeResourceSchema), &crdV1) + if err != nil { + t.Fatalf("unexpected decoding error: %v", err) + } + t.Logf("crd v1 details: %v", crdV1) + crd := &apiextensions.CustomResourceDefinition{} + if err = apiextensionsv1.Convert_v1_CustomResourceDefinition_To_apiextensions_CustomResourceDefinition(crdV1, crd, nil); err != nil { + t.Fatalf("unexpected convert error: %v", err) + } + t.Logf("crd details: %v", crd) + + strategy := statusStrategy{} + kind := schema.GroupVersionKind{ + Version: crd.Spec.Versions[0].Name, + Kind: crd.Spec.Names.Kind, + Group: crd.Spec.Group, + } + strategy.customResourceStrategy.validator.kind = kind + ss, _ := structuralschema.NewStructural(crd.Spec.Versions[0].Schema.OpenAPIV3Schema) + strategy.structuralSchemas = map[string]*structuralschema.Structural{ + crd.Spec.Versions[0].Name: ss, + } + + ctx := context.TODO() + + tcs := []struct { + name string + old *unstructured.Unstructured + obj *unstructured.Unstructured + isValid bool + }{ + { + name: "bothValid", + old: &unstructured.Unstructured{Object: map[string]interface{}{"apiVersion": "test/v1", "kind": "Foo", "numArray": []interface{}{1, 2}}}, + obj: &unstructured.Unstructured{Object: map[string]interface{}{"apiVersion": "test/v1", "kind": "Foo", "numArray": []interface{}{1, 3}, "metadata": map[string]interface{}{"resourceVersion": "1"}}}, + isValid: true, + }, + { + name: "change to invalid", + old: &unstructured.Unstructured{Object: map[string]interface{}{"apiVersion": "test/v1", "kind": "Foo", "spec": "old", "numArray": []interface{}{1, 2}}}, + obj: &unstructured.Unstructured{Object: map[string]interface{}{"apiVersion": "test/v1", "kind": "Foo", "spec": "new", "numArray": []interface{}{1, 1}, "metadata": map[string]interface{}{"resourceVersion": "1"}}}, + isValid: false, + }, + { + name: "change to valid", + old: &unstructured.Unstructured{Object: map[string]interface{}{"apiVersion": "test/v1", "kind": "Foo", "spec": "new", "numArray": []interface{}{1, 1}}}, + obj: &unstructured.Unstructured{Object: map[string]interface{}{"apiVersion": "test/v1", "kind": "Foo", "spec": "old", "numArray": []interface{}{1, 2}, "metadata": map[string]interface{}{"resourceVersion": "1"}}}, + isValid: true, + }, + { + name: "keeps invalid", + old: &unstructured.Unstructured{Object: map[string]interface{}{"apiVersion": "test/v1", "kind": "Foo", "spec": "new", "numArray": []interface{}{1, 1}}}, + obj: &unstructured.Unstructured{Object: map[string]interface{}{"apiVersion": "test/v1", "kind": "Foo", "spec": "old", "numArray": []interface{}{1, 1}, "metadata": map[string]interface{}{"resourceVersion": "1"}}}, + isValid: true, + }, + } + + for _, tc := range tcs { + errs := strategy.ValidateUpdate(ctx, tc.obj, tc.old) + if tc.isValid && len(errs) > 0 { + t.Errorf("%v: unexpected error: %v", tc.name, errs) + } + if !tc.isValid && len(errs) == 0 { + t.Errorf("%v: unexpected non-error", tc.name) + } + } +}