diff --git a/pkg/validation/internal/crd.go b/pkg/validation/internal/crd.go index 8f0e4fd01..79dae726e 100644 --- a/pkg/validation/internal/crd.go +++ b/pkg/validation/internal/crd.go @@ -8,7 +8,7 @@ import ( "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/install" - "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation" "k8s.io/apimachinery/pkg/runtime" @@ -37,8 +37,7 @@ func validateCRDs(objs ...interface{}) (results []errors.ManifestResult) { func validateV1Beta1CRD(crd *v1beta1.CustomResourceDefinition) (result errors.ManifestResult) { internalCRD := &apiextensions.CustomResourceDefinition{} - v1beta1.SetDefaults_CustomResourceDefinition(crd) - v1beta1.SetDefaults_CustomResourceDefinitionSpec(&crd.Spec) + v1beta1.SetObjectDefaults_CustomResourceDefinition(crd) err := scheme.Converter().Convert(crd, internalCRD, nil) if err != nil { result.Add(errors.ErrInvalidParse("error converting crd", err)) @@ -52,8 +51,7 @@ func validateV1Beta1CRD(crd *v1beta1.CustomResourceDefinition) (result errors.Ma func validateV1CRD(crd *v1.CustomResourceDefinition) (result errors.ManifestResult) { internalCRD := &apiextensions.CustomResourceDefinition{} - v1.SetDefaults_CustomResourceDefinition(crd) - v1.SetDefaults_CustomResourceDefinitionSpec(&crd.Spec) + v1.SetObjectDefaults_CustomResourceDefinition(crd) err := scheme.Converter().Convert(crd, internalCRD, nil) if err != nil { result.Add(errors.ErrInvalidParse("error converting crd", err)) diff --git a/pkg/validation/internal/crd_test.go b/pkg/validation/internal/crd_test.go index a9ce874be..73c948edc 100644 --- a/pkg/validation/internal/crd_test.go +++ b/pkg/validation/internal/crd_test.go @@ -15,45 +15,51 @@ import ( func TestValidateCRD(t *testing.T) { var table = []struct { description string - directory string + filePath string version string hasError bool errString string }{ { - description: "registryv1 bundle/valid crd", - directory: "./testdata/v1beta1.crd.yaml", + description: "registryv1 bundle/valid v1beta1 CRD", + filePath: "./testdata/v1beta1.crd.yaml", version: "v1beta1", hasError: false, errString: "", }, { - description: "registryv1 bundle/invalid crd", - directory: "./testdata/duplicateVersions.crd.yaml", + description: "registryv1 bundle invalid v1beta1 CRD duplicate version", + filePath: "./testdata/duplicateVersions.crd.yaml", version: "v1beta1", hasError: true, errString: "must contain unique version names", }, { - description: "registryv1 bundle/invalid crd", - directory: "./testdata/v1.crd.yaml", + description: "registryv1 bundle/valid v1 CRD", + filePath: "./testdata/v1.crd.yaml", version: "v1", hasError: false, errString: "", }, { - description: "registryv1 bundle/invalid crd", - directory: "./testdata/deprecatedVersion.crd.yaml", + description: "registryv1 bundle invalid v1 CRD deprecated .spec.version field", + filePath: "./testdata/deprecatedVersion.crd.yaml", version: "v1", hasError: true, errString: "must have exactly one version marked as storage version", }, + { + description: "registryv1 bundle invalid CRD no conversionReviewVersions", + filePath: "./testdata/noConversionReviewVersions.crd.yaml", + version: "v1", + hasError: true, + errString: "spec.conversion.conversionReviewVersions: Required value", + }, } - for _, tt := range table { - b, err := ioutil.ReadFile(tt.directory) + b, err := ioutil.ReadFile(tt.filePath) if err != nil { - t.Fatalf("Error reading CRD path %s: %v", tt.directory, err) + t.Fatalf("Error reading CRD path %s: %v", tt.filePath, err) } results := []errors.ManifestResult{} @@ -61,21 +67,26 @@ func TestValidateCRD(t *testing.T) { case "v1": crd := &v1.CustomResourceDefinition{} if err = yaml.Unmarshal(b, crd); err != nil { - t.Fatalf("Error unmarshalling CRD at path %s: %v", tt.directory, err) + t.Fatalf("Error unmarshalling CRD at path %s: %v", tt.filePath, err) } results = CRDValidator.Validate(crd) default: crd := &v1beta1.CustomResourceDefinition{} if err = yaml.Unmarshal(b, crd); err != nil { - t.Fatalf("Error unmarshalling CRD at path %s: %v", tt.directory, err) + t.Fatalf("Error unmarshalling CRD at path %s: %v", tt.filePath, err) } results = CRDValidator.Validate(crd) } if len(results) > 0 { - require.Equal(t, results[0].HasError(), tt.hasError) if results[0].HasError() { - require.Contains(t, results[0].Errors[0].Error(), tt.errString) + if !tt.hasError { + t.Errorf("%s: expected no errors, got: %+q", tt.description, results[0].Errors) + } else { + require.Contains(t, results[0].Errors[0].Error(), tt.errString, tt.description) + } + } else if tt.hasError { + t.Errorf("%s: expected error %q, got none", tt.description, tt.errString) } } } diff --git a/pkg/validation/internal/testdata/noConversionReviewVersions.crd.yaml b/pkg/validation/internal/testdata/noConversionReviewVersions.crd.yaml new file mode 100644 index 000000000..be0f36777 --- /dev/null +++ b/pkg/validation/internal/testdata/noConversionReviewVersions.crd.yaml @@ -0,0 +1,27 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: etcdclusters.etcd.database.coreos.com +spec: + conversion: + strategy: Webhook + webhook: + clientConfig: + service: + namespace: system + name: webhook-service + path: /convert + group: etcd.database.coreos.com + names: + kind: EtcdCluster + listKind: EtcdClusterList + plural: etcdclusters + shortNames: + - etcdclus + - etcd + singular: etcdcluster + scope: Namespaced + versions: + - name: v1beta2 + served: true + storage: true diff --git a/pkg/validation/internal/testdata/v1.crd.yaml b/pkg/validation/internal/testdata/v1.crd.yaml index 4fef5f3ad..91a5209f8 100644 --- a/pkg/validation/internal/testdata/v1.crd.yaml +++ b/pkg/validation/internal/testdata/v1.crd.yaml @@ -3,6 +3,16 @@ kind: CustomResourceDefinition metadata: name: etcdclusters.etcd.database.coreos.com spec: + conversion: + strategy: Webhook + webhook: + clientConfig: + service: + namespace: system + name: webhook-service + path: /convert + conversionReviewVersions: + - v1 group: etcd.database.coreos.com names: kind: EtcdCluster diff --git a/pkg/validation/internal/testdata/v1beta1.crd.yaml b/pkg/validation/internal/testdata/v1beta1.crd.yaml index 01111e5c5..98c65812a 100644 --- a/pkg/validation/internal/testdata/v1beta1.crd.yaml +++ b/pkg/validation/internal/testdata/v1beta1.crd.yaml @@ -3,6 +3,13 @@ kind: CustomResourceDefinition metadata: name: etcdclusters.etcd.database.coreos.com spec: + conversion: + strategy: Webhook + webhookClientConfig: + service: + namespace: system + name: webhook-service + path: /convert group: etcd.database.coreos.com names: kind: EtcdCluster @@ -12,5 +19,6 @@ spec: - etcdclus - etcd singular: etcdcluster + preserveUnknownFields: false scope: Namespaced version: v1beta2