From c6b3a2f477c9ed051ace8ae1c479424e4a5c066d Mon Sep 17 00:00:00 2001 From: Jiahui Feng Date: Mon, 23 Oct 2023 15:30:17 -0700 Subject: [PATCH 1/2] allow empty object to be CEL value. --- .../k8s.io/apiserver/pkg/cel/common/values.go | 26 +++++++++++-------- .../apiserver/pkg/cel/openapi/values_test.go | 9 +++++++ 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/cel/common/values.go b/staging/src/k8s.io/apiserver/pkg/cel/common/values.go index d9034a80fb2..c8279f01371 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/common/values.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/common/values.go @@ -84,18 +84,22 @@ func UnstructuredToVal(unstructured interface{}, schema Schema) ref.Val { }, } } - // A object with x-kubernetes-preserve-unknown-fields but no properties or additionalProperties is treated - // as an empty object. - if schema.IsXPreserveUnknownFields() { - return &unstructuredMap{ - value: m, - schema: schema, - propSchema: func(key string) (Schema, bool) { - return nil, false - }, - } + + // properties and additionalProperties are mutual exclusive, but nothing prevents the situation + // where both are missing. + // An object that (1) has no properties (2) has no additionalProperties or additionalProperties == false + // is treated as an empty object. + // An object that has additionalProperties == true is treated as an unstructured map. + // An object that has x-kubernetes-preserve-unknown-field extension set is treated as an unstructured map. + // Empty object vs unstructured map is differentiated by unstructuredMap implementation with the set schema. + // The resulting result remains the same. + return &unstructuredMap{ + value: m, + schema: schema, + propSchema: func(key string) (Schema, bool) { + return nil, false + }, } - return types.NewErr("invalid object type, expected either Properties or AdditionalProperties with Allows=true and non-empty Schema") } if schema.Type() == "array" { diff --git a/staging/src/k8s.io/apiserver/pkg/cel/openapi/values_test.go b/staging/src/k8s.io/apiserver/pkg/cel/openapi/values_test.go index 0607327f8bb..6914cce465f 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/openapi/values_test.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/openapi/values_test.go @@ -97,6 +97,9 @@ var ( Type: []string{"object"}, AdditionalProperties: &spec.SchemaOrBool{Schema: stringSchema}, }} + emptyObjectSchema = &spec.Schema{ + SchemaProps: spec.SchemaProps{Type: []string{"object"}}, + } ) func TestEquality(t *testing.T) { @@ -331,6 +334,12 @@ func TestEquality(t *testing.T) { rhs: UnstructuredToVal([]interface{}{"a", "b", "c"}, atomicListSchema), equal: false, }, + { + name: "empty objects are equal", + lhs: UnstructuredToVal(map[string]interface{}{}, emptyObjectSchema), + rhs: UnstructuredToVal(map[string]interface{}{}, emptyObjectSchema), + equal: true, + }, { name: "identical objects are equal", lhs: UnstructuredToVal(map[string]interface{}{"field1": "a", "field2": "b"}, objectSchema), From a6f5246565bdd8239c87a158807341e367baa8ae Mon Sep 17 00:00:00 2001 From: Jiahui Feng Date: Mon, 23 Oct 2023 16:19:32 -0700 Subject: [PATCH 2/2] integration test for empty & unstructured. --- .../crd_validation_expressions_test.go | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/test/integration/apiserver/crd_validation_expressions_test.go b/test/integration/apiserver/crd_validation_expressions_test.go index 9d1848e0969..586fda300f9 100644 --- a/test/integration/apiserver/crd_validation_expressions_test.go +++ b/test/integration/apiserver/crd_validation_expressions_test.go @@ -301,6 +301,13 @@ func TestCustomResourceValidators(t *testing.T) { t.Error("Unexpected error creating custom resource but metadata validation rule") } }) + t.Run("CRD creation MUST pass for an CRD with empty field", func(t *testing.T) { + structuralWithValidators := crdWithSchema(t, "WithEmptyObject", structuralSchemaWithEmptyObject) + _, err := fixtures.CreateNewV1CustomResourceDefinition(structuralWithValidators, apiExtensionClient, dynamicClient) + if err != nil { + t.Errorf("unexpected error creating CRD with empty field: %v", err) + } + }) t.Run("CR creation MUST fail if a x-kubernetes-validations rule exceeds the runtime cost limit", func(t *testing.T) { structuralWithValidators := crdWithSchema(t, "RuntimeCostLimit", structuralSchemaWithCostLimit) crd, err := fixtures.CreateNewV1CustomResourceDefinition(structuralWithValidators, apiExtensionClient, dynamicClient) @@ -1101,3 +1108,26 @@ var structuralSchemaWithCostLimit = []byte(` } } }`) + +var structuralSchemaWithEmptyObject = []byte(` +{ + "openAPIV3Schema": { + "description": "weird CRD with empty spec, unstructured status. designed to fit test fixtures.", + "type": "object", + "x-kubernetes-validations": [ + { + "rule": "[has(self.spec), has(self.status)].exists_one(x, x)" + } + ], + "properties": { + "spec": { + "type": "object" + }, + "status": { + "type": "object", + "additionalProperties": true + } + } + } +} +`)