From fec2d3717a99bdf1fed5942570518cc6c4f7bfe4 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Thu, 8 Aug 2019 21:46:12 -0400 Subject: [PATCH] CRD v1: require schema --- .../apiextensions/validation/validation.go | 43 +++- .../validation/validation_test.go | 207 ++++++++++++++++++ test/integration/etcd/data.go | 5 +- 3 files changed, 253 insertions(+), 2 deletions(-) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go index e2606024b90..812526c8c96 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go @@ -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")) @@ -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 @@ -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")) @@ -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") @@ -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) { 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 23957f473cd..41d5f567a3c 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 @@ -1320,6 +1320,7 @@ func TestValidateCustomResourceDefinition(t *testing.T) { StoredVersions: []string{"version"}, }, }, + requestGV: apiextensionsv1beta1.SchemeGroupVersion, errors: []validationMatch{ required("spec", "versions[1]", "schema", "openAPIV3Schema"), }, @@ -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"), @@ -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{ diff --git a/test/integration/etcd/data.go b/test/integration/etcd/data.go index 14a5502c77a..6544ca3e8c3 100644 --- a/test/integration/etcd/data.go +++ b/test/integration/etcd/data.go @@ -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"), },