pkg/validation: set CRD defaults using the correct function

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))

pkg/validation/internal/crd_test.go

@@ -15,67 +15,78 @@ 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{}
 		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)
+				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)
 			}
 		}
 	}

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

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

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