Skip to content

K
k3s
Unverified Commit f3cad465 authored by Kubernetes Submit Queue's avatar Kubernetes Submit Queue Committed by GitHub
Browse files
Options

Merge pull request #62333 from sttts/sttts-crd-validation-string-maps

Automatic merge from submit-queue (batch tested with PRs 59027, 62333, 57661, 62086, 61584). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. CustomResources: in OpenAPI spec allow additionalProperties without properties This implements @ayushpateria's idea https://github.com/kubernetes/kubernetes/issues/59485#issuecomment-375726922. With this PR it becomes possible to specify `map[string]Interface{}` non-object types, e.g. `map[string]string` for selectors. On the other hands, "normal" objects use `properties`, mutually exclusively to `additionalProperties`. This way we avoid a conflict with Kubernetes API conventions that unknown objects fields are dropped. Fixes #59485 ```release-note Allow additionalProperties in CRD OpenAPI v3 specification for validation, mutually exclusive to properties. ```
parents 6816227d b9df44e3
Hide whitespace changes
Inline Side-by-side
Showing with 90 additions and 19 deletions
staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/fuzzer/fuzzer.go
View file @ f3cad465
......@@ -81,6 +81,7 @@ func Funcs(codecs runtimeserializer.CodecFactory) []interface{} {
},
func(obj *apiextensions.JSONSchemaPropsOrBool, c fuzz.Continue) {
if c.RandBool() {
obj.Allows = true
obj.Schema = &apiextensions.JSONSchemaProps{}
c.Fuzz(obj.Schema)
} else {
......
staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/marshal.go
View file @ f3cad465
......@@ -45,6 +45,7 @@ func (s *JSONSchemaPropsOrBool) UnmarshalJSON(data []byte) error {
if err := json.Unmarshal(data, &sch); err != nil {
return err
}
nw.Allows = true
nw.Schema = &sch
case len(data) == 4 && string(data) == "true":
nw.Allows = true
......
staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/marshal_test.go
View file @ f3cad465
......@@ -34,13 +34,13 @@ func TestJSONSchemaPropsOrBoolUnmarshalJSON(t *testing.T) {
}{
{`{}`, JSONSchemaPropsOrBoolHolder{}},
{`{"val1": {}}`, JSONSchemaPropsOrBoolHolder{JSPoB: JSONSchemaPropsOrBool{Schema: &JSONSchemaProps{}}}},
{`{"val1": {"type":"string"}}`, JSONSchemaPropsOrBoolHolder{JSPoB: JSONSchemaPropsOrBool{Schema: &JSONSchemaProps{Type: "string"}}}},
{`{"val1": {}}`, JSONSchemaPropsOrBoolHolder{JSPoB: JSONSchemaPropsOrBool{Allows: true, Schema: &JSONSchemaProps{}}}},
{`{"val1": {"type":"string"}}`, JSONSchemaPropsOrBoolHolder{JSPoB: JSONSchemaPropsOrBool{Allows: true, Schema: &JSONSchemaProps{Type: "string"}}}},
{`{"val1": false}`, JSONSchemaPropsOrBoolHolder{JSPoB: JSONSchemaPropsOrBool{}}},
{`{"val1": true}`, JSONSchemaPropsOrBoolHolder{JSPoB: JSONSchemaPropsOrBool{Allows: true}}},
{`{"val2": {}}`, JSONSchemaPropsOrBoolHolder{JSPoBOmitEmpty: &JSONSchemaPropsOrBool{Schema: &JSONSchemaProps{}}}},
{`{"val2": {"type":"string"}}`, JSONSchemaPropsOrBoolHolder{JSPoBOmitEmpty: &JSONSchemaPropsOrBool{Schema: &JSONSchemaProps{Type: "string"}}}},
{`{"val2": {}}`, JSONSchemaPropsOrBoolHolder{JSPoBOmitEmpty: &JSONSchemaPropsOrBool{Allows: true, Schema: &JSONSchemaProps{}}}},
{`{"val2": {"type":"string"}}`, JSONSchemaPropsOrBoolHolder{JSPoBOmitEmpty: &JSONSchemaPropsOrBool{Allows: true, Schema: &JSONSchemaProps{Type: "string"}}}},
{`{"val2": false}`, JSONSchemaPropsOrBoolHolder{JSPoBOmitEmpty: &JSONSchemaPropsOrBool{}}},
{`{"val2": true}`, JSONSchemaPropsOrBoolHolder{JSPoBOmitEmpty: &JSONSchemaPropsOrBool{Allows: true}}},
}
......
staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go
View file @ f3cad465
......@@ -240,14 +240,36 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch
allErrs = append(allErrs, field.Forbidden(fldPath.Child("uniqueItems"), "uniqueItems cannot be set to true since the runtime complexity becomes quadratic"))
}
// additionalProperties contradicts Kubernetes API convention to ignore unknown fields
// additionalProperties and properties are mutual exclusive because otherwise they
// contradict Kubernetes' API convention to ignore unknown fields.
//
// In other words:
// - properties are for structs,
// - additionalProperties are for map[string]interface{}
//
// Note: when patternProperties is added to OpenAPI some day, this will have to be
// restricted like additionalProperties.
if schema.AdditionalProperties != nil {
if schema.AdditionalProperties.Allows == false {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("additionalProperties"), "additionalProperties cannot be set to false"))
if len(schema.Properties) != 0 {
if schema.AdditionalProperties.Allows == false || schema.AdditionalProperties.Schema != nil {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("additionalProperties"), "additionalProperties and properties are mutual exclusive"))
}
}
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.AdditionalProperties.Schema, fldPath.Child("additionalProperties"), ssv)...)
}
if len(schema.Properties) != 0 {
for property, jsonSchema := range schema.Properties {
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("properties").Key(property), ssv)...)
}
}
if len(schema.PatternProperties) != 0 {
for property, jsonSchema := range schema.PatternProperties {
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("patternProperties").Key(property), ssv)...)
}
}
if schema.AdditionalItems != nil {
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.AdditionalItems.Schema, fldPath.Child("additionalItems"), ssv)...)
}
......@@ -272,18 +294,6 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch
}
}
if len(schema.Properties) != 0 {
for property, jsonSchema := range schema.Properties {
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("properties").Key(property), ssv)...)
}
}
if len(schema.PatternProperties) != 0 {
for property, jsonSchema := range schema.PatternProperties {
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("patternProperties").Key(property), ssv)...)
}
}
if len(schema.Definitions) != 0 {
for definition, jsonSchema := range schema.Definitions {
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("definitions").Key(definition), ssv)...)
......
staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go
View file @ f3cad465
......@@ -41,6 +41,9 @@ func unsupported(path ...string) validationMatch {
func immutable(path ...string) validationMatch {
return validationMatch{path: field.NewPath(path[0], path[1:]...), errorType: field.ErrorTypeInvalid}
}
func forbidden(path ...string) validationMatch {
return validationMatch{path: field.NewPath(path[0], path[1:]...), errorType: field.ErrorTypeForbidden}
}
func (v validationMatch) matches(err *field.Error) bool {
return err.Type == v.errorType && err.Field == v.path.String()
......@@ -160,6 +163,62 @@ func TestValidateCustomResourceDefinition(t *testing.T) {
invalid("status", "acceptedNames", "listKind"),
},
},
{
name: "additionalProperties and properties forbidden",
resource: &apiextensions.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"},
Spec: apiextensions.CustomResourceDefinitionSpec{
Group: "group.com",
Version: "version",
Scope: apiextensions.NamespaceScoped,
Names: apiextensions.CustomResourceDefinitionNames{
Plural: "plural",
Singular: "singular",
Kind: "Plural",
ListKind: "PluralList",
},
Validation: &apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Properties: map[string]apiextensions.JSONSchemaProps{
"foo": {},
},
AdditionalProperties: &apiextensions.JSONSchemaPropsOrBool{Allows: false},
},
},
},
},
errors: []validationMatch{
forbidden("spec", "validation", "openAPIV3Schema", "additionalProperties"),
},
},
{
name: "additionalProperties without properties allowed (map[string]string)",
resource: &apiextensions.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"},
Spec: apiextensions.CustomResourceDefinitionSpec{
Group: "group.com",
Version: "version",
Scope: apiextensions.NamespaceScoped,
Names: apiextensions.CustomResourceDefinitionNames{
Plural: "plural",
Singular: "singular",
Kind: "Plural",
ListKind: "PluralList",
},
Validation: &apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
AdditionalProperties: &apiextensions.JSONSchemaPropsOrBool{
Allows: true,
Schema: &apiextensions.JSONSchemaProps{
Type: "string",
},
},
},
},
},
},
errors: []validationMatch{},
},
}
for _, tc := range tests {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment