CustomResources: in OpenAPI spec allow additionalProperties without properties

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

@@ -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

@@ -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

@@ -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

@@ -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

@@ -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 {