Refactor validate CRD to fix conversion error

pkg/validation/internal/crd.go

@@ -8,18 +8,18 @@ import (
 
 	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
 	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/install"
+	"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/conversion"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
-	"k8s.io/client-go/kubernetes/scheme"
 )
 
-var Scheme = scheme.Scheme
+var scheme = runtime.NewScheme()
 
 func init() {
-	install.Install(Scheme)
+	install.Install(scheme)
 }
 
 var CRDValidator interfaces.Validator = interfaces.ValidatorFunc(validateCRDs)
@@ -28,31 +28,54 @@ func validateCRDs(objs ...interface{}) (results []errors.ManifestResult) {
 	for _, obj := range objs {
 		switch v := obj.(type) {
 		case *v1beta1.CustomResourceDefinition:
-			results = append(results, validateCRD(v))
+			results = append(results, validateV1Beta1CRD(v))
+		case *v1.CustomResourceDefinition:
+			results = append(results, validateV1CRD(v))
 		}
 	}
 	return results
 }
 
-func validateCRD(crd runtime.Object) (result errors.ManifestResult) {
-	unversionedCRD := apiextensions.CustomResourceDefinition{}
-	err := Scheme.Converter().Convert(&crd, &unversionedCRD, conversion.SourceToDest, nil)
+func validateV1Beta1CRD(crd *v1beta1.CustomResourceDefinition) (result errors.ManifestResult) {
+	internalCRD := &apiextensions.CustomResourceDefinition{}
+	v1beta1.SetDefaults_CustomResourceDefinition(crd)
+	v1beta1.SetDefaults_CustomResourceDefinitionSpec(&crd.Spec)
+	err := scheme.Converter().Convert(crd, internalCRD, conversion.SourceToDest, nil)
 	if err != nil {
-		result.Add(errors.ErrInvalidParse("error converting versioned crd to unversioned crd", err))
+		result.Add(errors.ErrInvalidParse("error converting crd", err))
 		return result
 	}
+
 	gv := crd.GetObjectKind().GroupVersionKind().GroupVersion()
-	result = validateCRDUnversioned(&unversionedCRD, gv)
-	result.Name = unversionedCRD.GetName()
+	result = validateInternalCRD(internalCRD, gv)
 	return result
 }
 
-func validateCRDUnversioned(crd *apiextensions.CustomResourceDefinition, gv schema.GroupVersion) (result errors.ManifestResult) {
+func validateV1CRD(crd *v1.CustomResourceDefinition) (result errors.ManifestResult) {
+	internalCRD := &apiextensions.CustomResourceDefinition{}
+	v1.SetDefaults_CustomResourceDefinition(crd)
+	v1.SetDefaults_CustomResourceDefinitionSpec(&crd.Spec)
+	err := scheme.Converter().Convert(crd, internalCRD, conversion.SourceToDest, nil)
+	if err != nil {
+		result.Add(errors.ErrInvalidParse("error converting crd", err))
+		return result
+	}
+
+	gv := crd.GetObjectKind().GroupVersionKind().GroupVersion()
+	result = validateInternalCRD(internalCRD, gv)
+	return result
+}
+
+func validateInternalCRD(crd *apiextensions.CustomResourceDefinition, gv schema.GroupVersion) (result errors.ManifestResult) {
 	errList := validation.ValidateCustomResourceDefinition(crd, gv)
 	for _, err := range errList {
 		if !strings.Contains(err.Field, "openAPIV3Schema") && !strings.Contains(err.Field, "status") {
 			result.Add(errors.NewError(errors.ErrorType(err.Type), err.Error(), err.Field, err.BadValue))
 		}
 	}
+
+	if result.HasError() {
+		result.Name = crd.GetName()
+	}
 	return result
 }

pkg/validation/internal/crd_test.go

@@ -0,0 +1,82 @@
+package internal
+
+import (
+	"io/ioutil"
+	"testing"
+
+	"github.com/operator-framework/api/pkg/validation/errors"
+	"github.com/stretchr/testify/require"
+	v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
+
+	"github.com/ghodss/yaml"
+)
+
+func TestValidateCRD(t *testing.T) {
+	var table = []struct {
+		description string
+		directory   string
+		version     string
+		hasError    bool
+		errString   string
+	}{
+		{
+			description: "registryv1 bundle/valid crd",
+			directory:   "./testdata/v1beta1.crd.yaml",
+			version:     "v1beta1",
+			hasError:    false,
+			errString:   "",
+		},
+		{
+			description: "registryv1 bundle/invalid crd",
+			directory:   "./testdata/duplicateVersions.crd.yaml",
+			version:     "v1beta1",
+			hasError:    true,
+			errString:   "must contain unique version names",
+		},
+		{
+			description: "registryv1 bundle/invalid crd",
+			directory:   "./testdata/v1.crd.yaml",
+			version:     "v1",
+			hasError:    false,
+			errString:   "",
+		},
+		{
+			description: "registryv1 bundle/invalid crd",
+			directory:   "./testdata/deprecatedVersion.crd.yaml",
+			version:     "v1",
+			hasError:    true,
+			errString:   "must have exactly one version marked as storage version",
+		},
+	}
+
+	for _, tt := range table {
+		b, err := ioutil.ReadFile(tt.directory)
+		if err != nil {
+			t.Fatalf("Error reading CRD path %s: %v", tt.directory, err)
+		}
+
+		results := []errors.ManifestResult{}
+		switch tt.version {
+		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)
+			}
+			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)
+			}
+			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)
+			}
+		}
+	}
+}

pkg/validation/internal/testdata/deprecatedVersion.crd.yaml

@@ -0,0 +1,16 @@
+apiVersion: apiextensions.k8s.io/v1
+kind: CustomResourceDefinition
+metadata:
+  name: etcdclusters.etcd.database.coreos.com
+spec:
+  group: etcd.database.coreos.com
+  names:
+    kind: EtcdCluster
+    listKind: EtcdClusterList
+    plural: etcdclusters
+    shortNames:
+    - etcdclus
+    - etcd
+    singular: etcdcluster
+  scope: Namespaced
+  version: v1beta2

pkg/validation/internal/testdata/duplicateVersions.crd.yaml

@@ -0,0 +1,22 @@
+apiVersion: apiextensions.k8s.io/v1beta1
+kind: CustomResourceDefinition
+metadata:
+  name: etcdclusters.etcd.database.coreos.com
+spec:
+  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
+  - name: v1beta2
+    served: true
+    storage: false

pkg/validation/internal/testdata/v1.crd.yaml

@@ -0,0 +1,19 @@
+apiVersion: apiextensions.k8s.io/v1
+kind: CustomResourceDefinition
+metadata:
+  name: etcdclusters.etcd.database.coreos.com
+spec:
+  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

pkg/validation/internal/testdata/v1beta1.crd.yaml

@@ -0,0 +1,16 @@
+apiVersion: apiextensions.k8s.io/v1beta1
+kind: CustomResourceDefinition
+metadata:
+  name: etcdclusters.etcd.database.coreos.com
+spec:
+  group: etcd.database.coreos.com
+  names:
+    kind: EtcdCluster
+    listKind: EtcdClusterList
+    plural: etcdclusters
+    shortNames:
+    - etcdclus
+    - etcd
+    singular: etcdcluster
+  scope: Namespaced
+  version: v1beta2