Skip to content

Commit

Permalink
apiextensions: restrict spec of x-kubernetes-embedded-resource fields
Browse files Browse the repository at this point in the history
  • Loading branch information
@liggitt @sttts
liggitt authored and sttts committed Jun 10, 2019
1 parent e69f44e commit 3f7c629
Show file tree
Hide file tree
Showing 2 changed files with 166 additions and 0 deletions.
149 changes: 149 additions & 0 deletions ...g/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
17 changes: 17 additions & 0 deletions staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand All @@ -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"]
Expand Down

0 comments on commit 3f7c629

Please sign in to comment.