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), 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 + } + } + } +} +`)