Skip to content

Commit 80fd2f1

Browse files
dinhxuanvudinhxuanvu
authored
Merge pull request #12 from dinhxuanvu/fix-validate-crd
Refactor validate CRD to fix conversion error
2 parents 9066a6e + fab55ed commit 80fd2f1

File tree

6 files changed

+189
-11
lines changed

6 files changed

+189
-11
lines changed

‎pkg/validation/internal/crd.go‎

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,18 +8,18 @@ import (
88

99
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
1010
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/install"
11+
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
1112
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
1213
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation"
1314
"k8s.io/apimachinery/pkg/conversion"
1415
"k8s.io/apimachinery/pkg/runtime"
1516
"k8s.io/apimachinery/pkg/runtime/schema"
16-
"k8s.io/client-go/kubernetes/scheme"
1717
)
1818

19-
var Scheme = scheme.Scheme
19+
var scheme = runtime.NewScheme()
2020

2121
func init() {
22-
install.Install(Scheme)
22+
install.Install(scheme)
2323
}
2424

2525
var CRDValidator interfaces.Validator = interfaces.ValidatorFunc(validateCRDs)
@@ -28,31 +28,54 @@ func validateCRDs(objs ...interface{}) (results []errors.ManifestResult) {
2828
for _, obj := range objs {
2929
switch v := obj.(type) {
3030
case *v1beta1.CustomResourceDefinition:
31-
results = append(results, validateCRD(v))
31+
results = append(results, validateV1Beta1CRD(v))
32+
case *v1.CustomResourceDefinition:
33+
results = append(results, validateV1CRD(v))
3234
}
3335
}
3436
return results
3537
}
3638

37-
func validateCRD(crd runtime.Object) (result errors.ManifestResult) {
38-
unversionedCRD := apiextensions.CustomResourceDefinition{}
39-
err := Scheme.Converter().Convert(&crd, &unversionedCRD, conversion.SourceToDest, nil)
39+
func validateV1Beta1CRD(crd *v1beta1.CustomResourceDefinition) (result errors.ManifestResult) {
40+
internalCRD := &apiextensions.CustomResourceDefinition{}
41+
v1beta1.SetDefaults_CustomResourceDefinition(crd)
42+
v1beta1.SetDefaults_CustomResourceDefinitionSpec(&crd.Spec)
43+
err := scheme.Converter().Convert(crd, internalCRD, conversion.SourceToDest, nil)
4044
if err != nil {
41-
result.Add(errors.ErrInvalidParse("error converting versioned crd to unversioned crd", err))
45+
result.Add(errors.ErrInvalidParse("error converting crd", err))
4246
return result
4347
}
48+
4449
gv := crd.GetObjectKind().GroupVersionKind().GroupVersion()
45-
result = validateCRDUnversioned(&unversionedCRD, gv)
46-
result.Name = unversionedCRD.GetName()
50+
result = validateInternalCRD(internalCRD, gv)
4751
return result
4852
}
4953

50-
func validateCRDUnversioned(crd *apiextensions.CustomResourceDefinition, gv schema.GroupVersion) (result errors.ManifestResult) {
54+
func validateV1CRD(crd *v1.CustomResourceDefinition) (result errors.ManifestResult) {
55+
internalCRD := &apiextensions.CustomResourceDefinition{}
56+
v1.SetDefaults_CustomResourceDefinition(crd)
57+
v1.SetDefaults_CustomResourceDefinitionSpec(&crd.Spec)
58+
err := scheme.Converter().Convert(crd, internalCRD, conversion.SourceToDest, nil)
59+
if err != nil {
60+
result.Add(errors.ErrInvalidParse("error converting crd", err))
61+
return result
62+
}
63+
64+
gv := crd.GetObjectKind().GroupVersionKind().GroupVersion()
65+
result = validateInternalCRD(internalCRD, gv)
66+
return result
67+
}
68+
69+
func validateInternalCRD(crd *apiextensions.CustomResourceDefinition, gv schema.GroupVersion) (result errors.ManifestResult) {
5170
errList := validation.ValidateCustomResourceDefinition(crd, gv)
5271
for _, err := range errList {
5372
if !strings.Contains(err.Field, "openAPIV3Schema") && !strings.Contains(err.Field, "status") {
5473
result.Add(errors.NewError(errors.ErrorType(err.Type), err.Error(), err.Field, err.BadValue))
5574
}
5675
}
76+
77+
if result.HasError() {
78+
result.Name = crd.GetName()
79+
}
5780
return result
5881
}

‎pkg/validation/internal/crd_test.go‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
package internal
2+
3+
import (
4+
"io/ioutil"
5+
"testing"
6+
7+
"github.com/operator-framework/api/pkg/validation/errors"
8+
"github.com/stretchr/testify/require"
9+
v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
10+
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
11+
12+
"github.com/ghodss/yaml"
13+
)
14+
15+
func TestValidateCRD(t *testing.T) {
16+
var table = []struct {
17+
description string
18+
directory string
19+
version string
20+
hasError bool
21+
errString string
22+
}{
23+
{
24+
description: "registryv1 bundle/valid crd",
25+
directory: "./testdata/v1beta1.crd.yaml",
26+
version: "v1beta1",
27+
hasError: false,
28+
errString: "",
29+
},
30+
{
31+
description: "registryv1 bundle/invalid crd",
32+
directory: "./testdata/duplicateVersions.crd.yaml",
33+
version: "v1beta1",
34+
hasError: true,
35+
errString: "must contain unique version names",
36+
},
37+
{
38+
description: "registryv1 bundle/invalid crd",
39+
directory: "./testdata/v1.crd.yaml",
40+
version: "v1",
41+
hasError: false,
42+
errString: "",
43+
},
44+
{
45+
description: "registryv1 bundle/invalid crd",
46+
directory: "./testdata/deprecatedVersion.crd.yaml",
47+
version: "v1",
48+
hasError: true,
49+
errString: "must have exactly one version marked as storage version",
50+
},
51+
}
52+
53+
for _, tt := range table {
54+
b, err := ioutil.ReadFile(tt.directory)
55+
if err != nil {
56+
t.Fatalf("Error reading CRD path %s: %v", tt.directory, err)
57+
}
58+
59+
results := []errors.ManifestResult{}
60+
switch tt.version {
61+
case "v1":
62+
crd := &v1.CustomResourceDefinition{}
63+
if err = yaml.Unmarshal(b, crd); err != nil {
64+
t.Fatalf("Error unmarshalling CRD at path %s: %v", tt.directory, err)
65+
}
66+
results = CRDValidator.Validate(crd)
67+
default:
68+
crd := &v1beta1.CustomResourceDefinition{}
69+
if err = yaml.Unmarshal(b, crd); err != nil {
70+
t.Fatalf("Error unmarshalling CRD at path %s: %v", tt.directory, err)
71+
}
72+
results = CRDValidator.Validate(crd)
73+
}
74+
75+
if len(results) > 0 {
76+
require.Equal(t, results[0].HasError(), tt.hasError)
77+
if results[0].HasError() {
78+
require.Contains(t, results[0].Errors[0].Error(), tt.errString)
79+
}
80+
}
81+
}
82+
}

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
apiVersion: apiextensions.k8s.io/v1
2+
kind: CustomResourceDefinition
3+
metadata:
4+
name: etcdclusters.etcd.database.coreos.com
5+
spec:
6+
group: etcd.database.coreos.com
7+
names:
8+
kind: EtcdCluster
9+
listKind: EtcdClusterList
10+
plural: etcdclusters
11+
shortNames:
12+
- etcdclus
13+
- etcd
14+
singular: etcdcluster
15+
scope: Namespaced
16+
version: v1beta2

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
apiVersion: apiextensions.k8s.io/v1beta1
2+
kind: CustomResourceDefinition
3+
metadata:
4+
name: etcdclusters.etcd.database.coreos.com
5+
spec:
6+
group: etcd.database.coreos.com
7+
names:
8+
kind: EtcdCluster
9+
listKind: EtcdClusterList
10+
plural: etcdclusters
11+
shortNames:
12+
- etcdclus
13+
- etcd
14+
singular: etcdcluster
15+
scope: Namespaced
16+
versions:
17+
- name: v1beta2
18+
served: true
19+
storage: true
20+
- name: v1beta2
21+
served: true
22+
storage: false

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
apiVersion: apiextensions.k8s.io/v1
2+
kind: CustomResourceDefinition
3+
metadata:
4+
name: etcdclusters.etcd.database.coreos.com
5+
spec:
6+
group: etcd.database.coreos.com
7+
names:
8+
kind: EtcdCluster
9+
listKind: EtcdClusterList
10+
plural: etcdclusters
11+
shortNames:
12+
- etcdclus
13+
- etcd
14+
singular: etcdcluster
15+
scope: Namespaced
16+
versions:
17+
- name: v1beta2
18+
served: true
19+
storage: true

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
apiVersion: apiextensions.k8s.io/v1beta1
2+
kind: CustomResourceDefinition
3+
metadata:
4+
name: etcdclusters.etcd.database.coreos.com
5+
spec:
6+
group: etcd.database.coreos.com
7+
names:
8+
kind: EtcdCluster
9+
listKind: EtcdClusterList
10+
plural: etcdclusters
11+
shortNames:
12+
- etcdclus
13+
- etcd
14+
singular: etcdcluster
15+
scope: Namespaced
16+
version: v1beta2

0 commit comments

Comments
 (0)