diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go index cef63542e3e..bd79af557a4 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go @@ -1796,6 +1796,155 @@ func TestValidateCustomResourceDefinition(t *testing.T) { }, enabledFeatures: []featuregate.Feature{features.CustomResourceDefaulting}, }, + { + name: "contradicting meta field types", + resource: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"}, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Group: "group.com", + Version: "version", + Versions: singleVersionList, + Scope: apiextensions.NamespaceScoped, + Names: apiextensions.CustomResourceDefinitionNames{ + Plural: "plural", + Singular: "singular", + Kind: "Plural", + ListKind: "PluralList", + }, + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "apiVersion": {Type: "number"}, + "kind": {Type: "number"}, + "metadata": { + Type: "number", + Properties: map[string]apiextensions.JSONSchemaProps{ + "name": { + Type: "string", + Pattern: "abc", + }, + "generateName": { + Type: "string", + Pattern: "abc", + }, + "generation": { + Type: "integer", + }, + }, + }, + "valid": { + Type: "object", + XEmbeddedResource: true, + Properties: map[string]apiextensions.JSONSchemaProps{ + "apiVersion": {Type: "string"}, + "kind": {Type: "string"}, + "metadata": { + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "name": { + Type: "string", + Pattern: "abc", + }, + "generateName": { + Type: "string", + Pattern: "abc", + }, + "generation": { + Type: "integer", + Minimum: float64Ptr(42.0), // does not make sense, but is allowed for nested ObjectMeta + }, + }, + }, + }, + }, + "invalid": { + Type: "object", + XEmbeddedResource: true, + Properties: map[string]apiextensions.JSONSchemaProps{ + "apiVersion": {Type: "number"}, + "kind": {Type: "number"}, + "metadata": { + Type: "number", + Properties: map[string]apiextensions.JSONSchemaProps{ + "name": { + Type: "string", + Pattern: "abc", + }, + "generateName": { + Type: "string", + Pattern: "abc", + }, + "generation": { + Type: "integer", + Minimum: float64Ptr(42.0), // does not make sense, but is allowed for nested ObjectMeta + }, + }, + }, + }, + }, + "nested": { + Type: "object", + XEmbeddedResource: true, + Properties: map[string]apiextensions.JSONSchemaProps{ + "invalid": { + Type: "object", + XEmbeddedResource: true, + Properties: map[string]apiextensions.JSONSchemaProps{ + "apiVersion": {Type: "number"}, + "kind": {Type: "number"}, + "metadata": { + Type: "number", + Properties: map[string]apiextensions.JSONSchemaProps{ + "name": { + Type: "string", + Pattern: "abc", + }, + "generateName": { + Type: "string", + Pattern: "abc", + }, + "generation": { + Type: "integer", + Minimum: float64Ptr(42.0), // does not make sense, but is allowed for nested ObjectMeta + }, + }, + }, + }, + }, + }, + }, + "noEmbeddedObject": { + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "apiVersion": {Type: "number"}, + "kind": {Type: "number"}, + "metadata": {Type: "number"}, + }, + }, + }, + }, + }, + PreserveUnknownFields: pointer.BoolPtr(false), + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"version"}, + }, + }, + errors: []validationMatch{ + forbidden("spec", "validation", "openAPIV3Schema", "properties[metadata]"), + invalid("spec", "validation", "openAPIV3Schema", "properties[apiVersion]", "type"), + invalid("spec", "validation", "openAPIV3Schema", "properties[kind]", "type"), + invalid("spec", "validation", "openAPIV3Schema", "properties[metadata]", "type"), + invalid("spec", "validation", "openAPIV3Schema", "properties[invalid]", "properties[apiVersion]", "type"), + invalid("spec", "validation", "openAPIV3Schema", "properties[invalid]", "properties[kind]", "type"), + invalid("spec", "validation", "openAPIV3Schema", "properties[invalid]", "properties[metadata]", "type"), + invalid("spec", "validation", "openAPIV3Schema", "properties[nested]", "properties[invalid]", "properties[apiVersion]", "type"), + invalid("spec", "validation", "openAPIV3Schema", "properties[nested]", "properties[invalid]", "properties[kind]", "type"), + invalid("spec", "validation", "openAPIV3Schema", "properties[nested]", "properties[invalid]", "properties[metadata]", "type"), + }, + enabledFeatures: []featuregate.Feature{features.CustomResourceDefaulting}, + }, } for _, tc := range tests { diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/validation.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/validation.go index c5f5f0b8261..abdcf886589 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/validation.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/validation.go @@ -108,6 +108,8 @@ func validateStructuralInvariants(s *Structural, lvl level, fldPath *field.Path) allErrs = append(allErrs, validateValueValidation(s.ValueValidation, skipAnyOf, skipFirstAllOfAnyOf, lvl, fldPath)...) + checkMetadata := (lvl == rootLevel) || s.XEmbeddedResource + if s.XEmbeddedResource && s.Type != "object" { if len(s.Type) == 0 { allErrs = append(allErrs, field.Required(fldPath.Child("type"), "must be object if x-kubernetes-embedded-resource is true")) @@ -130,6 +132,21 @@ func validateStructuralInvariants(s *Structural, lvl level, fldPath *field.Path) } // restrict metadata schemas to name and generateName only + if kind, found := s.Properties["kind"]; found && checkMetadata { + if kind.Type != "string" { + allErrs = append(allErrs, field.Invalid(fldPath.Child("properties").Key("kind").Child("type"), kind.Type, "must be string")) + } + } + if apiVersion, found := s.Properties["apiVersion"]; found && checkMetadata { + if apiVersion.Type != "string" { + allErrs = append(allErrs, field.Invalid(fldPath.Child("properties").Key("apiVersion").Child("type"), apiVersion.Type, "must be string")) + } + } + if metadata, found := s.Properties["metadata"]; found && checkMetadata { + if metadata.Type != "object" { + allErrs = append(allErrs, field.Invalid(fldPath.Child("properties").Key("metadata").Child("type"), metadata.Type, "must be object")) + } + } if metadata, found := s.Properties["metadata"]; found && lvl == rootLevel { // metadata is a shallow copy. We can mutate it. _, foundName := metadata.Properties["name"]