diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go index 92442f89d0c..983d14fc7d9 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go @@ -784,21 +784,23 @@ func (v *specStandardValidatorV3) validate(schema *apiextensions.JSONSchemaProps // ignore errors here locally. They will show up for the root of the schema. clone := runtime.DeepCopyJSONValue(interface{}(*schema.Default)) - if !v.isInsideResourceMeta || s.XEmbeddedResource { - pruning.Prune(clone, s, s.XEmbeddedResource) + if !v.isInsideResourceMeta { // If we are under metadata, there are implicitly specified fields like kind, apiVersion, metadata, labels. // We cannot prune as they are pruned as well. This allows more defaults than we would like to. // TODO: be precise about pruning under metadata - } - // TODO: coerce correctly if we are not at the object root, but somewhere below. - if err := schemaobjectmeta.Coerce(fldPath, clone, s, s.XEmbeddedResource, false); err != nil { - allErrs = append(allErrs, err) - } - if !reflect.DeepEqual(clone, interface{}(*schema.Default)) { - allErrs = append(allErrs, field.Invalid(fldPath.Child("default"), schema.Default, "must not have unknown fields")) - } else if s.XEmbeddedResource { - // validate an embedded resource - schemaobjectmeta.Validate(fldPath, interface{}(*schema.Default), nil, true) + pruning.Prune(clone, s, s.XEmbeddedResource) + + // TODO: coerce correctly if we are not at the object root, but somewhere below. + if err := schemaobjectmeta.Coerce(fldPath, clone, s, s.XEmbeddedResource, false); err != nil { + allErrs = append(allErrs, err) + } + + if !reflect.DeepEqual(clone, interface{}(*schema.Default)) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("default"), schema.Default, "must not have unknown fields")) + } else if s.XEmbeddedResource { + // validate an embedded resource + schemaobjectmeta.Validate(fldPath, interface{}(*schema.Default), nil, true) + } } // validate the default value with user the provided schema. diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta/algorithm.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta/algorithm.go index fd3ff23d703..4cd32c1293b 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta/algorithm.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta/algorithm.go @@ -23,16 +23,18 @@ import ( ) // Coerce checks types of embedded ObjectMeta and TypeMeta and prunes unknown fields inside the former. -// It does coerce ObjectMeta and TypeMeta at the root if includeRoot is true. +// It does coerce ObjectMeta and TypeMeta at the root if isResourceRoot is true. // If dropInvalidFields is true, fields of wrong type will be dropped. -func Coerce(pth *field.Path, obj interface{}, s *structuralschema.Structural, includeRoot, dropInvalidFields bool) *field.Error { - if includeRoot { +func Coerce(pth *field.Path, obj interface{}, s *structuralschema.Structural, isResourceRoot, dropInvalidFields bool) *field.Error { + if isResourceRoot { if s == nil { s = &structuralschema.Structural{} } - clone := *s - clone.XEmbeddedResource = true - s = &clone + if !s.XEmbeddedResource { + clone := *s + clone.XEmbeddedResource = true + s = &clone + } } c := coercer{dropInvalidFields: dropInvalidFields} return c.coerce(pth, obj, s) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta/validation.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta/validation.go index 5e788109f90..195150ae236 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta/validation.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta/validation.go @@ -28,15 +28,17 @@ import ( ) // Validate validates embedded ObjectMeta and TypeMeta. -// It also validate those at the root if includeRoot is true. -func Validate(pth *field.Path, obj interface{}, s *structuralschema.Structural, includeRoot bool) field.ErrorList { - if includeRoot { +// It also validate those at the root if isResourceRoot is true. +func Validate(pth *field.Path, obj interface{}, s *structuralschema.Structural, isResourceRoot bool) field.ErrorList { + if isResourceRoot { if s == nil { s = &structuralschema.Structural{} } - clone := *s - clone.XEmbeddedResource = true - s = &clone + if !s.XEmbeddedResource { + clone := *s + clone.XEmbeddedResource = true + s = &clone + } } return validate(pth, obj, s) } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta/validation_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta/validation_test.go index 3ac225ff804..bed35140be1 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta/validation_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta/validation_test.go @@ -139,6 +139,20 @@ func TestValidate(t *testing.T) { required("nested", "embedded", "apiVersion"), required("nested", "embedded", "kind"), }}, + {name: "items", object: ` +{ + "items": [{}] +}`, errors: []validationMatch{ + required("items[0]", "apiVersion"), + required("items[0]", "kind"), + }}, + {name: "additionalProperties", object: ` +{ + "additionalProperties": {"foo":{}} +}`, errors: []validationMatch{ + required("additionalProperties[foo]", "apiVersion"), + required("additionalProperties[foo]", "kind"), + }}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -151,6 +165,20 @@ func TestValidate(t *testing.T) { "embedded": {Extensions: structuralschema.Extensions{XEmbeddedResource: true}}, }, }, + "items": { + Items: &structuralschema.Structural{ + Extensions: structuralschema.Extensions{XEmbeddedResource: true}, + }, + }, + "additionalProperties": { + Generic: structuralschema.Generic{ + AdditionalProperties: &structuralschema.StructuralOrBool{ + Structural: &structuralschema.Structural{ + Extensions: structuralschema.Extensions{XEmbeddedResource: true}, + }, + }, + }, + }, }, } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/pruning/algorithm.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/pruning/algorithm.go index 164105c9408..5a6a8c7c8c4 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/pruning/algorithm.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/pruning/algorithm.go @@ -21,15 +21,17 @@ import ( ) // Prune removes object fields in obj which are not specified in s. It skips TypeMeta and ObjectMeta fields -// if XEmbeddedResource is set to true, or for the root if root=true. -func Prune(obj interface{}, s *structuralschema.Structural, root bool) { - if root { +// if XEmbeddedResource is set to true, or for the root if isResourceRoot=true. +func Prune(obj interface{}, s *structuralschema.Structural, isResourceRoot bool) { + if isResourceRoot { if s == nil { s = &structuralschema.Structural{} } - clone := *s - clone.XEmbeddedResource = true - s = &clone + if !s.XEmbeddedResource { + clone := *s + clone.XEmbeddedResource = true + s = &clone + } } prune(obj, s) } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/pruning/algorithm_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/pruning/algorithm_test.go index 755028cce49..4ddf9301144 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/pruning/algorithm_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/pruning/algorithm_test.go @@ -29,11 +29,11 @@ import ( func TestPrune(t *testing.T) { tests := []struct { - name string - json string - dontPruneMetaAtRoot bool - schema *structuralschema.Structural - expected string + name string + json string + isResourceRoot bool + schema *structuralschema.Structural + expected string }{ {name: "empty", json: "null", expected: "null"}, {name: "scalar", json: "4", schema: &structuralschema.Structural{}, expected: "4"}, @@ -85,7 +85,13 @@ func TestPrune(t *testing.T) { "pruning": {"unspecified": "bar"}, "preserving": {"unspecified": "bar"} }, - "preservingAdditionalProperties": { + "preservingAdditionalPropertiesNotInheritingXPreserveUnknownFields": { + "foo": { + "specified": {"unspecified":"bar"}, + "unspecified": "bar" + } + }, + "preservingAdditionalPropertiesKeyPruneValues": { "foo": { "specified": {"unspecified":"bar"}, "unspecified": "bar" @@ -123,7 +129,8 @@ func TestPrune(t *testing.T) { }, }, }, - "preservingAdditionalProperties": { + "preservingAdditionalPropertiesNotInheritingXPreserveUnknownFields": { + // this x-kubernetes-preserve-unknown-fields is not inherited by the schema inside of additionalProperties Extensions: structuralschema.Extensions{XPreserveUnknownFields: true}, Generic: structuralschema.Generic{ Type: "object", @@ -137,6 +144,19 @@ func TestPrune(t *testing.T) { }, }, }, + "preservingAdditionalPropertiesKeyPruneValues": { + Generic: structuralschema.Generic{ + Type: "object", + AdditionalProperties: &structuralschema.StructuralOrBool{ + Structural: &structuralschema.Structural{ + Generic: structuralschema.Generic{Type: "object"}, + Properties: map[string]structuralschema.Structural{ + "specified": {Generic: structuralschema.Generic{Type: "object"}}, + }, + }, + }, + }, + }, }, }, expected: ` { @@ -154,7 +174,12 @@ func TestPrune(t *testing.T) { "pruning": {}, "preserving": {"unspecified": "bar"} }, - "preservingAdditionalProperties": { + "preservingAdditionalPropertiesNotInheritingXPreserveUnknownFields": { + "foo": { + "specified": {} + } + }, + "preservingAdditionalPropertiesKeyPruneValues": { "foo": { "specified": {} } @@ -397,7 +422,7 @@ func TestPrune(t *testing.T) { } } } -`, dontPruneMetaAtRoot: true, schema: &structuralschema.Structural{ +`, isResourceRoot: true, schema: &structuralschema.Structural{ Generic: structuralschema.Generic{Type: "object"}, Properties: map[string]structuralschema.Structural{ "pruned": { @@ -508,7 +533,7 @@ func TestPrune(t *testing.T) { t.Fatal(err) } - Prune(in, tt.schema, tt.dontPruneMetaAtRoot) + Prune(in, tt.schema, tt.isResourceRoot) if !reflect.DeepEqual(in, expected) { var buf bytes.Buffer enc := json.NewEncoder(&buf)