Skip to content

Commit

Permalink
CRD v1: require schema
Browse files Browse the repository at this point in the history
  • Loading branch information
@liggitt
liggitt committed Aug 16, 2019
1 parent 15097a5 commit fec2d37
Show file tree
Hide file tree
Showing 3 changed files with 253 additions and 2 deletions.
43 changes: 42 additions & 1 deletion staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ func ValidateCustomResourceDefinition(obj *apiextensions.CustomResourceDefinitio
allowDefaults: allowDefaults(requestGV),
requireRecognizedConversionReviewVersion: true,
requireImmutableNames: false,
requireOpenAPISchema: requireOpenAPISchema(requestGV, nil),
}

allErrs := genericvalidation.ValidateObjectMeta(&obj.ObjectMeta, false, nameValidationFn, field.NewPath("metadata"))
Expand All @@ -84,6 +85,8 @@ type validationOptions struct {
requireRecognizedConversionReviewVersion bool
// requireImmutableNames disables changing spec.names
requireImmutableNames bool
// requireOpenAPISchema requires an openapi V3 schema be specified
requireOpenAPISchema bool
}

// ValidateCustomResourceDefinitionUpdate statically validates
Expand All @@ -92,6 +95,7 @@ func ValidateCustomResourceDefinitionUpdate(obj, oldObj *apiextensions.CustomRes
allowDefaults: allowDefaults(requestGV) || specHasDefaults(&oldObj.Spec),
requireRecognizedConversionReviewVersion: oldObj.Spec.Conversion == nil || hasValidConversionReviewVersionOrEmpty(oldObj.Spec.Conversion.ConversionReviewVersions),
requireImmutableNames: apiextensions.IsCRDConditionTrue(oldObj, apiextensions.Established),
requireOpenAPISchema: requireOpenAPISchema(requestGV, &oldObj.Spec),
}

allErrs := genericvalidation.ValidateObjectMetaUpdate(&obj.ObjectMeta, &oldObj.ObjectMeta, field.NewPath("metadata"))
Expand Down Expand Up @@ -161,10 +165,23 @@ func validateCustomResourceDefinitionSpec(spec *apiextensions.CustomResourceDefi

allErrs = append(allErrs, validateEnumStrings(fldPath.Child("scope"), string(spec.Scope), []string{string(apiextensions.ClusterScoped), string(apiextensions.NamespaceScoped)}, true)...)

// enabling pruning requires structural schemas
mustBeStructural := false
if spec.PreserveUnknownFields == nil || *spec.PreserveUnknownFields == false {
mustBeStructural = true
// check that either a global schema or versioned schemas are set
}

if opts.requireOpenAPISchema {
// check that either a global schema or versioned schemas are set in all versions
if spec.Validation == nil || spec.Validation.OpenAPIV3Schema == nil {
for i, v := range spec.Versions {
if v.Schema == nil || v.Schema.OpenAPIV3Schema == nil {
allErrs = append(allErrs, field.Required(fldPath.Child("versions").Index(i).Child("schema").Child("openAPIV3Schema"), "schemas are required"))
}
}
}
} else if spec.PreserveUnknownFields == nil || *spec.PreserveUnknownFields == false {
// check that either a global schema or versioned schemas are set in served versions
if spec.Validation == nil || spec.Validation.OpenAPIV3Schema == nil {
for i, v := range spec.Versions {
schemaPath := fldPath.Child("versions").Index(i).Child("schema", "openAPIV3Schema")
Expand Down Expand Up @@ -946,6 +963,30 @@ func allowedAtRootSchema(field string) bool {
return false
}

// requireOpenAPISchema returns true if the request group version requires a schema
func requireOpenAPISchema(requestGV schema.GroupVersion, oldCRDSpec *apiextensions.CustomResourceDefinitionSpec) bool {
if requestGV == v1beta1.SchemeGroupVersion {
// for backwards compatibility
return false
}
if oldCRDSpec != nil && !allVersionsSpecifyOpenAPISchema(oldCRDSpec) {
// don't tighten validation on existing persisted data
return false
}
return true
}
func allVersionsSpecifyOpenAPISchema(spec *apiextensions.CustomResourceDefinitionSpec) bool {
if spec.Validation != nil && spec.Validation.OpenAPIV3Schema != nil {
return true
}
for _, v := range spec.Versions {
if v.Schema == nil || v.Schema.OpenAPIV3Schema == nil {
return false
}
}
return true
}

// allowDefaults returns true if the defaulting feature is enabled and the request group version allows adding defaults
func allowDefaults(requestGV schema.GroupVersion) bool {
if !utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CustomResourceDefaulting) {
Expand Down
207 changes: 207 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 @@ -1320,6 +1320,7 @@ func TestValidateCustomResourceDefinition(t *testing.T) {
StoredVersions: []string{"version"},
},
},
requestGV: apiextensionsv1beta1.SchemeGroupVersion,
errors: []validationMatch{
required("spec", "versions[1]", "schema", "openAPIV3Schema"),
},
Expand Down Expand Up @@ -1360,6 +1361,49 @@ func TestValidateCustomResourceDefinition(t *testing.T) {
StoredVersions: []string{"version"},
},
},
requestGV: apiextensionsv1beta1.SchemeGroupVersion,
errors: []validationMatch{
required("spec", "versions[0]", "schema", "openAPIV3Schema"),
required("spec", "versions[1]", "schema", "openAPIV3Schema"),
},
},
{
name: "no schema via v1",
resource: &apiextensions.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"},
Spec: apiextensions.CustomResourceDefinitionSpec{
Group: "group.com",
Version: "version",
Versions: []apiextensions.CustomResourceDefinitionVersion{
{
Name: "version",
Served: true,
Storage: true,
Schema: nil,
},
{
Name: "version2",
Served: true,
Storage: false,
Schema: &apiextensions.CustomResourceValidation{
OpenAPIV3Schema: nil,
},
},
},
Scope: apiextensions.NamespaceScoped,
Names: apiextensions.CustomResourceDefinitionNames{
Plural: "plural",
Singular: "singular",
Kind: "Plural",
ListKind: "PluralList",
},
PreserveUnknownFields: pointer.BoolPtr(false),
},
Status: apiextensions.CustomResourceDefinitionStatus{
StoredVersions: []string{"version"},
},
},
requestGV: apiextensionsv1.SchemeGroupVersion,
errors: []validationMatch{
required("spec", "versions[0]", "schema", "openAPIV3Schema"),
required("spec", "versions[1]", "schema", "openAPIV3Schema"),
Expand Down Expand Up @@ -3489,6 +3533,169 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) {
requestGV: apiextensionsv1.SchemeGroupVersion,
errors: []validationMatch{},
},
{
name: "schema not required via v1beta1",
old: &apiextensions.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "1"},
Spec: apiextensions.CustomResourceDefinitionSpec{
Group: "group.com",
Version: "version",
Versions: []apiextensions.CustomResourceDefinitionVersion{{Name: "version", Served: true, Storage: true}},
Scope: apiextensions.NamespaceScoped,
Names: apiextensions.CustomResourceDefinitionNames{Plural: "plural", Singular: "singular", Kind: "Plural", ListKind: "PluralList"},
PreserveUnknownFields: pointer.BoolPtr(true),
},
Status: apiextensions.CustomResourceDefinitionStatus{StoredVersions: []string{"version"}},
},
resource: &apiextensions.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "1"},
Spec: apiextensions.CustomResourceDefinitionSpec{
Group: "group.com",
Version: "version",
Versions: []apiextensions.CustomResourceDefinitionVersion{{Name: "version", Served: true, Storage: true}},
Scope: apiextensions.NamespaceScoped,
Names: apiextensions.CustomResourceDefinitionNames{Plural: "plural", Singular: "singular", Kind: "Plural", ListKind: "PluralList"},
PreserveUnknownFields: pointer.BoolPtr(true),
},
Status: apiextensions.CustomResourceDefinitionStatus{StoredVersions: []string{"version"}},
},
requestGV: apiextensionsv1beta1.SchemeGroupVersion,
errors: []validationMatch{},
},
{
name: "schema not required via v1 if old object is missing schema",
old: &apiextensions.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "1"},
Spec: apiextensions.CustomResourceDefinitionSpec{
Group: "group.com",
Version: "version",
Versions: []apiextensions.CustomResourceDefinitionVersion{{Name: "version", Served: true, Storage: true}},
Scope: apiextensions.NamespaceScoped,
Names: apiextensions.CustomResourceDefinitionNames{Plural: "plural", Singular: "singular", Kind: "Plural", ListKind: "PluralList"},
PreserveUnknownFields: pointer.BoolPtr(true),
},
Status: apiextensions.CustomResourceDefinitionStatus{StoredVersions: []string{"version"}},
},
resource: &apiextensions.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "1"},
Spec: apiextensions.CustomResourceDefinitionSpec{
Group: "group.com",
Version: "version",
Versions: []apiextensions.CustomResourceDefinitionVersion{{Name: "version", Served: true, Storage: true}},
Scope: apiextensions.NamespaceScoped,
Names: apiextensions.CustomResourceDefinitionNames{Plural: "plural", Singular: "singular", Kind: "Plural", ListKind: "PluralList"},
PreserveUnknownFields: pointer.BoolPtr(true),
},
Status: apiextensions.CustomResourceDefinitionStatus{StoredVersions: []string{"version"}},
},
requestGV: apiextensionsv1.SchemeGroupVersion,
errors: []validationMatch{},
},
{
name: "schema not required via v1 if old object is missing schema for some versions",
old: &apiextensions.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "1"},
Spec: apiextensions.CustomResourceDefinitionSpec{
Group: "group.com",
Version: "version",
Versions: []apiextensions.CustomResourceDefinitionVersion{
{Name: "version", Served: true, Storage: true, Schema: &apiextensions.CustomResourceValidation{OpenAPIV3Schema: &apiextensions.JSONSchemaProps{Type: "object"}}},
{Name: "version2", Served: true, Storage: false},
},
Scope: apiextensions.NamespaceScoped,
Names: apiextensions.CustomResourceDefinitionNames{Plural: "plural", Singular: "singular", Kind: "Plural", ListKind: "PluralList"},
PreserveUnknownFields: pointer.BoolPtr(true),
},
Status: apiextensions.CustomResourceDefinitionStatus{StoredVersions: []string{"version"}},
},
resource: &apiextensions.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "1"},
Spec: apiextensions.CustomResourceDefinitionSpec{
Group: "group.com",
Version: "version",
Versions: []apiextensions.CustomResourceDefinitionVersion{
{Name: "version", Served: true, Storage: true},
{Name: "version2", Served: true, Storage: false},
},
Scope: apiextensions.NamespaceScoped,
Names: apiextensions.CustomResourceDefinitionNames{Plural: "plural", Singular: "singular", Kind: "Plural", ListKind: "PluralList"},
PreserveUnknownFields: pointer.BoolPtr(true),
},
Status: apiextensions.CustomResourceDefinitionStatus{StoredVersions: []string{"version"}},
},
requestGV: apiextensionsv1.SchemeGroupVersion,
errors: []validationMatch{},
},
{
name: "schema required via v1 if old object has top-level schema",
old: &apiextensions.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "1"},
Spec: apiextensions.CustomResourceDefinitionSpec{
Group: "group.com",
Version: "version",
Versions: []apiextensions.CustomResourceDefinitionVersion{{Name: "version", Served: true, Storage: true}},
Validation: &apiextensions.CustomResourceValidation{OpenAPIV3Schema: &apiextensions.JSONSchemaProps{Type: "object"}},
Scope: apiextensions.NamespaceScoped,
Names: apiextensions.CustomResourceDefinitionNames{Plural: "plural", Singular: "singular", Kind: "Plural", ListKind: "PluralList"},
PreserveUnknownFields: pointer.BoolPtr(true),
},
Status: apiextensions.CustomResourceDefinitionStatus{StoredVersions: []string{"version"}},
},
resource: &apiextensions.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "1"},
Spec: apiextensions.CustomResourceDefinitionSpec{
Group: "group.com",
Version: "version",
Versions: []apiextensions.CustomResourceDefinitionVersion{{Name: "version", Served: true, Storage: true}},
Scope: apiextensions.NamespaceScoped,
Names: apiextensions.CustomResourceDefinitionNames{Plural: "plural", Singular: "singular", Kind: "Plural", ListKind: "PluralList"},
PreserveUnknownFields: pointer.BoolPtr(true),
},
Status: apiextensions.CustomResourceDefinitionStatus{StoredVersions: []string{"version"}},
},
requestGV: apiextensionsv1.SchemeGroupVersion,
errors: []validationMatch{
required("spec.versions[0].schema.openAPIV3Schema"),
},
},
{
name: "schema required via v1 if all versions of old object have schema",
old: &apiextensions.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "1"},
Spec: apiextensions.CustomResourceDefinitionSpec{
Group: "group.com",
Version: "version",
Versions: []apiextensions.CustomResourceDefinitionVersion{
{Name: "version", Served: true, Storage: true, Schema: &apiextensions.CustomResourceValidation{OpenAPIV3Schema: &apiextensions.JSONSchemaProps{Type: "object", Description: "1"}}},
{Name: "version2", Served: true, Storage: false, Schema: &apiextensions.CustomResourceValidation{OpenAPIV3Schema: &apiextensions.JSONSchemaProps{Type: "object", Description: "2"}}},
},
Scope: apiextensions.NamespaceScoped,
Names: apiextensions.CustomResourceDefinitionNames{Plural: "plural", Singular: "singular", Kind: "Plural", ListKind: "PluralList"},
PreserveUnknownFields: pointer.BoolPtr(true),
},
Status: apiextensions.CustomResourceDefinitionStatus{StoredVersions: []string{"version"}},
},
resource: &apiextensions.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "1"},
Spec: apiextensions.CustomResourceDefinitionSpec{
Group: "group.com",
Version: "version",
Versions: []apiextensions.CustomResourceDefinitionVersion{
{Name: "version", Served: true, Storage: true},
{Name: "version2", Served: true, Storage: false},
},
Scope: apiextensions.NamespaceScoped,
Names: apiextensions.CustomResourceDefinitionNames{Plural: "plural", Singular: "singular", Kind: "Plural", ListKind: "PluralList"},
PreserveUnknownFields: pointer.BoolPtr(true),
},
Status: apiextensions.CustomResourceDefinitionStatus{StoredVersions: []string{"version"}},
},
requestGV: apiextensionsv1.SchemeGroupVersion,
errors: []validationMatch{
required("spec.versions[0].schema.openAPIV3Schema"),
required("spec.versions[1].schema.openAPIV3Schema"),
},
},
{
name: "setting defaults with enabled feature gate",
old: &apiextensions.CustomResourceDefinition{
Expand Down
5 changes: 4 additions & 1 deletion test/integration/etcd/data.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,10 @@ func GetEtcdStorageDataForNamespace(namespace string) map[schema.GroupVersionRes

// k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1
gvr("apiextensions.k8s.io", "v1", "customresourcedefinitions"): {
Stub: `{"metadata": {"name": "openshiftwebconsoleconfigs.webconsole2.operator.openshift.io"},"spec": {"scope": "Cluster","group": "webconsole2.operator.openshift.io","versions": [{"name":"v1alpha1","storage":true,"served":true}],"names": {"kind": "OpenShiftWebConsoleConfig","plural": "openshiftwebconsoleconfigs","singular": "openshiftwebconsoleconfig"}}}`,
Stub: `{"metadata": {"name": "openshiftwebconsoleconfigs.webconsole2.operator.openshift.io"},"spec": {` +
`"scope": "Cluster","group": "webconsole2.operator.openshift.io",` +
`"versions": [{"name":"v1alpha1","storage":true,"served":true,"schema":{"openAPIV3Schema":{"type":"object"}}}],` +
`"names": {"kind": "OpenShiftWebConsoleConfig","plural": "openshiftwebconsoleconfigs","singular": "openshiftwebconsoleconfig"}}}`,
ExpectedEtcdPath: "/registry/apiextensions.k8s.io/customresourcedefinitions/openshiftwebconsoleconfigs.webconsole2.operator.openshift.io",
ExpectedGVK: gvkP("apiextensions.k8s.io", "v1beta1", "CustomResourceDefinition"),
},
Expand Down

0 comments on commit fec2d37

Please sign in to comment.