Skip to content

Commit e2fd72c

Browse files
stttssttts
committed
apiextensions: do not check for pruned defaults under metadata
1 parent d06827a commit e2fd72c

File tree

5 files changed

+371
-43
lines changed

5 files changed

+371
-43
lines changed

‎staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go‎

Lines changed: 74 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -566,6 +566,11 @@ func ValidateCustomResourceColumnDefinition(col *apiextensions.CustomResourceCol
566566
// specStandardValidator applies validations for different OpenAPI specification versions.
567567
type specStandardValidator interface {
568568
validate(spec *apiextensions.JSONSchemaProps, fldPath *field.Path) field.ErrorList
569+
withForbiddenDefaults(reason string) specStandardValidator
570+
571+
// insideResourceMeta returns true when validating either TypeMeta or ObjectMeta, from an embedded resource or on the top-level.
572+
insideResourceMeta() bool
573+
withInsideResourceMeta() specStandardValidator
569574
}
570575

571576
// ValidateCustomResourceDefinitionValidation statically validates
@@ -612,7 +617,7 @@ func ValidateCustomResourceDefinitionValidation(customResourceValidation *apiext
612617
openAPIV3Schema := &specStandardValidatorV3{
613618
allowDefaults: allowDefaults,
614619
}
615-
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema, fldPath.Child("openAPIV3Schema"), openAPIV3Schema)...)
620+
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema, fldPath.Child("openAPIV3Schema"), openAPIV3Schema, true)...)
616621

617622
if mustBeStructural {
618623
if ss, err := structuralschema.NewStructural(schema); err != nil {
@@ -636,7 +641,7 @@ func ValidateCustomResourceDefinitionValidation(customResourceValidation *apiext
636641
}
637642

638643
// ValidateCustomResourceDefinitionOpenAPISchema statically validates
639-
func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSchemaProps, fldPath *field.Path, ssv specStandardValidator) field.ErrorList {
644+
func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSchemaProps, fldPath *field.Path, ssv specStandardValidator, isRoot bool) field.ErrorList {
640645
allErrs := field.ErrorList{}
641646

642647
if schema == nil {
@@ -664,53 +669,68 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch
664669
allErrs = append(allErrs, field.Forbidden(fldPath.Child("additionalProperties"), "additionalProperties and properties are mutual exclusive"))
665670
}
666671
}
667-
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.AdditionalProperties.Schema, fldPath.Child("additionalProperties"), ssv)...)
672+
// Note: we forbid additionalProperties at resource root, both embedded and top-level.
673+
// But further inside, additionalProperites is possible, e.g. for labels or annotations.
674+
subSsv := ssv
675+
if ssv.insideResourceMeta() {
676+
// we have to forbid defaults inside additionalProperties because pruning without actual value is ambiguous
677+
subSsv = ssv.withForbiddenDefaults("inside additionalProperties applying to object metadata")
678+
}
679+
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.AdditionalProperties.Schema, fldPath.Child("additionalProperties"), subSsv, false)...)
668680
}
669681

670682
if len(schema.Properties) != 0 {
671683
for property, jsonSchema := range schema.Properties {
672-
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("properties").Key(property), ssv)...)
684+
subSsv := ssv
685+
if (isRoot || schema.XEmbeddedResource) && property == "metadata" {
686+
// we recurse into the schema that applies to ObjectMeta.
687+
subSsv = ssv.withInsideResourceMeta()
688+
if isRoot {
689+
subSsv = subSsv.withForbiddenDefaults(fmt.Sprintf("in top-level %s", property))
690+
}
691+
}
692+
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("properties").Key(property), subSsv, false)...)
673693
}
674694
}
675695

676-
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.Not, fldPath.Child("not"), ssv)...)
696+
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.Not, fldPath.Child("not"), ssv, false)...)
677697

678698
if len(schema.AllOf) != 0 {
679699
for i, jsonSchema := range schema.AllOf {
680-
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("allOf").Index(i), ssv)...)
700+
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("allOf").Index(i), ssv, false)...)
681701
}
682702
}
683703

684704
if len(schema.OneOf) != 0 {
685705
for i, jsonSchema := range schema.OneOf {
686-
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("oneOf").Index(i), ssv)...)
706+
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("oneOf").Index(i), ssv, false)...)
687707
}
688708
}
689709

690710
if len(schema.AnyOf) != 0 {
691711
for i, jsonSchema := range schema.AnyOf {
692-
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("anyOf").Index(i), ssv)...)
712+
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("anyOf").Index(i), ssv, false)...)
693713
}
694714
}
695715

696716
if len(schema.Definitions) != 0 {
697717
for definition, jsonSchema := range schema.Definitions {
698-
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("definitions").Key(definition), ssv)...)
718+
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("definitions").Key(definition), ssv, false)...)
699719
}
700720
}
701721

702722
if schema.Items != nil {
703-
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.Items.Schema, fldPath.Child("items"), ssv)...)
723+
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.Items.Schema, fldPath.Child("items"), ssv, false)...)
704724
if len(schema.Items.JSONSchemas) != 0 {
705725
for i, jsonSchema := range schema.Items.JSONSchemas {
706-
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("items").Index(i), ssv)...)
726+
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("items").Index(i), ssv, false)...)
707727
}
708728
}
709729
}
710730

711731
if schema.Dependencies != nil {
712732
for dependency, jsonSchemaPropsOrStringArray := range schema.Dependencies {
713-
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(jsonSchemaPropsOrStringArray.Schema, fldPath.Child("dependencies").Key(dependency), ssv)...)
733+
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(jsonSchemaPropsOrStringArray.Schema, fldPath.Child("dependencies").Key(dependency), ssv, false)...)
714734
}
715735
}
716736

@@ -722,7 +742,26 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch
722742
}
723743

724744
type specStandardValidatorV3 struct {
725-
allowDefaults bool
745+
allowDefaults bool
746+
disallowDefaultsReason string
747+
isInsideResourceMeta bool
748+
}
749+
750+
func (v *specStandardValidatorV3) withForbiddenDefaults(reason string) specStandardValidator {
751+
clone := *v
752+
clone.disallowDefaultsReason = reason
753+
clone.allowDefaults = false
754+
return &clone
755+
}
756+
757+
func (v *specStandardValidatorV3) withInsideResourceMeta() specStandardValidator {
758+
clone := *v
759+
clone.isInsideResourceMeta = true
760+
return &clone
761+
}
762+
763+
func (v *specStandardValidatorV3) insideResourceMeta() bool {
764+
return v.isInsideResourceMeta
726765
}
727766

728767
// validate validates against OpenAPI Schema v3.
@@ -741,23 +780,37 @@ func (v *specStandardValidatorV3) validate(schema *apiextensions.JSONSchemaProps
741780
if v.allowDefaults {
742781
if s, err := structuralschema.NewStructural(schema); err == nil {
743782
// ignore errors here locally. They will show up for the root of the schema.
744-
pruned := runtime.DeepCopyJSONValue(interface{}(*schema.Default))
745-
pruning.Prune(pruned, s, false)
746-
if err := schemaobjectmeta.Coerce(fldPath, pruned, s, false, false); err != nil {
783+
784+
clone := runtime.DeepCopyJSONValue(interface{}(*schema.Default))
785+
if !v.isInsideResourceMeta || s.XEmbeddedResource {
786+
pruning.Prune(clone, s, s.XEmbeddedResource)
787+
// If we are under metadata, there are implicitly specified fields like kind, apiVersion, metadata, labels.
788+
// We cannot prune as they are pruned as well. This allows more defaults than we would like to.
789+
// TODO: be precise about pruning under metadata
790+
}
791+
// TODO: coerce correctly if we are not at the object root, but somewhere below.
792+
if err := schemaobjectmeta.Coerce(fldPath, clone, s, s.XEmbeddedResource, false); err != nil {
747793
allErrs = append(allErrs, err)
748794
}
749-
if !reflect.DeepEqual(pruned, *schema.Default) {
750-
allErrs = append(allErrs, field.Invalid(fldPath.Child("default"), schema.Default, "must not have unspecified fields"))
795+
if !reflect.DeepEqual(clone, interface{}(*schema.Default)) {
796+
allErrs = append(allErrs, field.Invalid(fldPath.Child("default"), schema.Default, "must not have unknown fields"))
797+
} else if s.XEmbeddedResource {
798+
// validate an embedded resource
799+
schemaobjectmeta.Validate(fldPath, interface{}(*schema.Default), nil, true)
751800
}
752801

753-
// validate the default value. Only validating and pruned defaults are allowed.
802+
// validate the default value with user the provided schema.
754803
validator := govalidate.NewSchemaValidator(s.ToGoOpenAPI(), nil, "", strfmt.Default)
755-
if err := apiservervalidation.ValidateCustomResource(pruned, validator); err != nil {
804+
if err := apiservervalidation.ValidateCustomResource(interface{}(*schema.Default), validator); err != nil {
756805
allErrs = append(allErrs, field.Invalid(fldPath.Child("default"), schema.Default, fmt.Sprintf("must validate: %v", err)))
757806
}
758807
}
759808
} else {
760-
allErrs = append(allErrs, field.Forbidden(fldPath.Child("default"), "must not be set"))
809+
detail := "must not be set"
810+
if len(v.disallowDefaultsReason) > 0 {
811+
detail += " " + v.disallowDefaultsReason
812+
}
813+
allErrs = append(allErrs, field.Forbidden(fldPath.Child("default"), detail))
761814
}
762815
}
763816

‎staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go‎

Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1956,6 +1956,213 @@ func TestValidateCustomResourceDefinition(t *testing.T) {
19561956
},
19571957
enabledFeatures: []featuregate.Feature{features.CustomResourceDefaulting},
19581958
},
1959+
{
1960+
name: "metadata defaults",
1961+
resource: &apiextensions.CustomResourceDefinition{
1962+
ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"},
1963+
Spec: apiextensions.CustomResourceDefinitionSpec{
1964+
Group: "group.com",
1965+
Version: "v1",
1966+
Versions: []apiextensions.CustomResourceDefinitionVersion{
1967+
{
1968+
Name: "v1",
1969+
Served: true,
1970+
Storage: true,
1971+
Schema: &apiextensions.CustomResourceValidation{
1972+
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
1973+
Type: "object",
1974+
Properties: map[string]apiextensions.JSONSchemaProps{
1975+
"metadata": {
1976+
Type: "object",
1977+
// forbidden: no default for top-level metadata
1978+
Default: jsonPtr(map[string]interface{}{
1979+
"name": "foo",
1980+
}),
1981+
},
1982+
"embedded": {
1983+
Type: "object",
1984+
XEmbeddedResource: true,
1985+
Properties: map[string]apiextensions.JSONSchemaProps{
1986+
"metadata": {
1987+
Type: "object",
1988+
Default: jsonPtr(map[string]interface{}{
1989+
"name": "foo",
1990+
// TODO: forbid unknown field under metadata
1991+
"unknown": int64(42),
1992+
}),
1993+
},
1994+
},
1995+
},
1996+
},
1997+
},
1998+
},
1999+
},
2000+
{
2001+
Name: "v2",
2002+
Served: true,
2003+
Storage: false,
2004+
Schema: &apiextensions.CustomResourceValidation{
2005+
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
2006+
Type: "object",
2007+
Properties: map[string]apiextensions.JSONSchemaProps{
2008+
"metadata": {
2009+
Type: "object",
2010+
Properties: map[string]apiextensions.JSONSchemaProps{
2011+
"name": {
2012+
Type: "string",
2013+
// forbidden: no default in top-level metadata
2014+
Default: jsonPtr("foo"),
2015+
},
2016+
},
2017+
},
2018+
"embedded": {
2019+
Type: "object",
2020+
XEmbeddedResource: true,
2021+
Properties: map[string]apiextensions.JSONSchemaProps{
2022+
"apiVersion": {
2023+
Type: "string",
2024+
Default: jsonPtr("v1"),
2025+
},
2026+
"kind": {
2027+
Type: "string",
2028+
Default: jsonPtr("Pod"),
2029+
},
2030+
"metadata": {
2031+
Type: "object",
2032+
Properties: map[string]apiextensions.JSONSchemaProps{
2033+
"name": {
2034+
Type: "string",
2035+
Default: jsonPtr("foo"),
2036+
},
2037+
},
2038+
},
2039+
},
2040+
},
2041+
},
2042+
},
2043+
},
2044+
},
2045+
{
2046+
Name: "v3",
2047+
Served: true,
2048+
Storage: false,
2049+
Schema: &apiextensions.CustomResourceValidation{
2050+
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
2051+
Type: "object",
2052+
Properties: map[string]apiextensions.JSONSchemaProps{
2053+
"embedded": {
2054+
Type: "object",
2055+
XEmbeddedResource: true,
2056+
Properties: map[string]apiextensions.JSONSchemaProps{
2057+
"apiVersion": {
2058+
Type: "string",
2059+
Default: jsonPtr("v1"),
2060+
},
2061+
"kind": {
2062+
Type: "string",
2063+
// TODO: forbid non-validating nested values in metadata
2064+
Default: jsonPtr("%"),
2065+
},
2066+
"metadata": {
2067+
Type: "object",
2068+
Default: jsonPtr(map[string]interface{}{
2069+
"labels": map[string]interface{}{
2070+
// TODO: forbid non-validating nested field in meta
2071+
"bar": "x y",
2072+
},
2073+
}),
2074+
},
2075+
},
2076+
},
2077+
},
2078+
},
2079+
},
2080+
},
2081+
{
2082+
Name: "v4",
2083+
Served: true,
2084+
Storage: false,
2085+
Schema: &apiextensions.CustomResourceValidation{
2086+
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
2087+
Type: "object",
2088+
Properties: map[string]apiextensions.JSONSchemaProps{
2089+
"embedded": {
2090+
Type: "object",
2091+
XEmbeddedResource: true,
2092+
Properties: map[string]apiextensions.JSONSchemaProps{
2093+
"metadata": {
2094+
Type: "object",
2095+
Properties: map[string]apiextensions.JSONSchemaProps{
2096+
"name": {
2097+
Type: "string",
2098+
// TODO: forbid wrongly typed nested fields in metadata
2099+
Default: jsonPtr("%"),
2100+
},
2101+
"labels": {
2102+
Type: "object",
2103+
Properties: map[string]apiextensions.JSONSchemaProps{
2104+
"bar": {
2105+
Type: "string",
2106+
// TODO: forbid non-validating nested fields in metadata
2107+
Default: jsonPtr("x y"),
2108+
},
2109+
},
2110+
},
2111+
"annotations": {
2112+
Type: "object",
2113+
AdditionalProperties: &apiextensions.JSONSchemaPropsOrBool{
2114+
Schema: &apiextensions.JSONSchemaProps{
2115+
Type: "string",
2116+
// forbidden: no default under additionalProperties inside of metadata
2117+
Default: jsonPtr("abc"),
2118+
},
2119+
},
2120+
},
2121+
},
2122+
},
2123+
},
2124+
},
2125+
},
2126+
},
2127+
},
2128+
},
2129+
},
2130+
Scope: apiextensions.NamespaceScoped,
2131+
Names: apiextensions.CustomResourceDefinitionNames{
2132+
Plural: "plural",
2133+
Singular: "singular",
2134+
Kind: "Plural",
2135+
ListKind: "PluralList",
2136+
},
2137+
PreserveUnknownFields: pointer.BoolPtr(false),
2138+
},
2139+
Status: apiextensions.CustomResourceDefinitionStatus{
2140+
StoredVersions: []string{"v1"},
2141+
},
2142+
},
2143+
errors: []validationMatch{
2144+
// Forbidden: must not be set in top-level metadata
2145+
forbidden("spec", "versions[0]", "schema", "openAPIV3Schema", "properties[metadata]", "default"),
2146+
// Invalid value: map[string]interface {}{"name":"foo", "unknown":42}: must not have unknown fields
2147+
// TODO: invalid("spec", "versions[0]", "schema", "openAPIV3Schema", "properties[embedded]", "properties[metadata]", "default"),
2148+
2149+
// Forbidden: must not be set in top-level metadata
2150+
forbidden("spec", "versions[1]", "schema", "openAPIV3Schema", "properties[metadata]", "properties[name]", "default"),
2151+
2152+
// Invalid value: "x y"
2153+
// TODO: invalid("spec", "versions[2]", "schema", "openAPIV3Schema", "properties[embedded]", "properties[metadata]", "default"),
2154+
// Invalid value: "%": kind: Invalid value: "%"
2155+
// TODO: invalid("spec", "versions[2]", "schema", "openAPIV3Schema", "properties[embedded]", "properties[kind]", "default"),
2156+
2157+
// Invalid value: "%"
2158+
// TODO: invalid("spec", "versions[3]", "schema", "openAPIV3Schema", "properties[embedded]", "properties[metadata]", "properties[labels]", "properties[bar]", "default"),
2159+
// Invalid value: "x y"
2160+
// TODO: invalid("spec", "versions[3]", "schema", "openAPIV3Schema", "properties[embedded]", "properties[metadata]", "properties[name]", "default"),
2161+
// Forbidden: must not be set inside additionalProperties applying to object metadata
2162+
forbidden("spec", "versions[3]", "schema", "openAPIV3Schema", "properties[embedded]", "properties[metadata]", "properties[annotations]", "additionalProperties", "default"),
2163+
},
2164+
enabledFeatures: []featuregate.Feature{features.CustomResourceDefaulting},
2165+
},
19592166
{
19602167
name: "contradicting meta field types",
19612168
resource: &apiextensions.CustomResourceDefinition{

0 commit comments

Comments
 (0)