Skip to content

Commit 1c3cb2e

Browse files
author
Eric Stroczynski
authored
Merge pull request #118 from estroz/bugfix/crd-set-defaults
pkg/validation: set CRD defaults using the correct function
2 parents b512869 + 5e38e51 commit 1c3cb2e

File tree

5 files changed

+75
-21
lines changed

5 files changed

+75
-21
lines changed

‎pkg/validation/internal/crd.go‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ 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"
11+
v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
1212
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
1313
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation"
1414
"k8s.io/apimachinery/pkg/runtime"
@@ -37,8 +37,7 @@ func validateCRDs(objs ...interface{}) (results []errors.ManifestResult) {
3737

3838
func validateV1Beta1CRD(crd *v1beta1.CustomResourceDefinition) (result errors.ManifestResult) {
3939
internalCRD := &apiextensions.CustomResourceDefinition{}
40-
v1beta1.SetDefaults_CustomResourceDefinition(crd)
41-
v1beta1.SetDefaults_CustomResourceDefinitionSpec(&crd.Spec)
40+
v1beta1.SetObjectDefaults_CustomResourceDefinition(crd)
4241
err := scheme.Converter().Convert(crd, internalCRD, nil)
4342
if err != nil {
4443
result.Add(errors.ErrInvalidParse("error converting crd", err))
@@ -52,8 +51,7 @@ func validateV1Beta1CRD(crd *v1beta1.CustomResourceDefinition) (result errors.Ma
5251

5352
func validateV1CRD(crd *v1.CustomResourceDefinition) (result errors.ManifestResult) {
5453
internalCRD := &apiextensions.CustomResourceDefinition{}
55-
v1.SetDefaults_CustomResourceDefinition(crd)
56-
v1.SetDefaults_CustomResourceDefinitionSpec(&crd.Spec)
54+
v1.SetObjectDefaults_CustomResourceDefinition(crd)
5755
err := scheme.Converter().Convert(crd, internalCRD, nil)
5856
if err != nil {
5957
result.Add(errors.ErrInvalidParse("error converting crd", err))

‎pkg/validation/internal/crd_test.go‎

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -15,67 +15,78 @@ import (
1515
func TestValidateCRD(t *testing.T) {
1616
var table = []struct {
1717
description string
18-
directory string
18+
filePath string
1919
version string
2020
hasError bool
2121
errString string
2222
}{
2323
{
24-
description: "registryv1 bundle/valid crd",
25-
directory: "./testdata/v1beta1.crd.yaml",
24+
description: "registryv1 bundle/valid v1beta1 CRD",
25+
filePath: "./testdata/v1beta1.crd.yaml",
2626
version: "v1beta1",
2727
hasError: false,
2828
errString: "",
2929
},
3030
{
31-
description: "registryv1 bundle/invalid crd",
32-
directory: "./testdata/duplicateVersions.crd.yaml",
31+
description: "registryv1 bundle invalid v1beta1 CRD duplicate version",
32+
filePath: "./testdata/duplicateVersions.crd.yaml",
3333
version: "v1beta1",
3434
hasError: true,
3535
errString: "must contain unique version names",
3636
},
3737
{
38-
description: "registryv1 bundle/invalid crd",
39-
directory: "./testdata/v1.crd.yaml",
38+
description: "registryv1 bundle/valid v1 CRD",
39+
filePath: "./testdata/v1.crd.yaml",
4040
version: "v1",
4141
hasError: false,
4242
errString: "",
4343
},
4444
{
45-
description: "registryv1 bundle/invalid crd",
46-
directory: "./testdata/deprecatedVersion.crd.yaml",
45+
description: "registryv1 bundle invalid v1 CRD deprecated .spec.version field",
46+
filePath: "./testdata/deprecatedVersion.crd.yaml",
4747
version: "v1",
4848
hasError: true,
4949
errString: "must have exactly one version marked as storage version",
5050
},
51+
{
52+
description: "registryv1 bundle invalid CRD no conversionReviewVersions",
53+
filePath: "./testdata/noConversionReviewVersions.crd.yaml",
54+
version: "v1",
55+
hasError: true,
56+
errString: "spec.conversion.conversionReviewVersions: Required value",
57+
},
5158
}
52-
5359
for _, tt := range table {
54-
b, err := ioutil.ReadFile(tt.directory)
60+
b, err := ioutil.ReadFile(tt.filePath)
5561
if err != nil {
56-
t.Fatalf("Error reading CRD path %s: %v", tt.directory, err)
62+
t.Fatalf("Error reading CRD path %s: %v", tt.filePath, err)
5763
}
5864

5965
results := []errors.ManifestResult{}
6066
switch tt.version {
6167
case "v1":
6268
crd := &v1.CustomResourceDefinition{}
6369
if err = yaml.Unmarshal(b, crd); err != nil {
64-
t.Fatalf("Error unmarshalling CRD at path %s: %v", tt.directory, err)
70+
t.Fatalf("Error unmarshalling CRD at path %s: %v", tt.filePath, err)
6571
}
6672
results = CRDValidator.Validate(crd)
6773
default:
6874
crd := &v1beta1.CustomResourceDefinition{}
6975
if err = yaml.Unmarshal(b, crd); err != nil {
70-
t.Fatalf("Error unmarshalling CRD at path %s: %v", tt.directory, err)
76+
t.Fatalf("Error unmarshalling CRD at path %s: %v", tt.filePath, err)
7177
}
7278
results = CRDValidator.Validate(crd)
7379
}
7480

7581
if len(results) > 0 {
76-
require.Equal(t, results[0].HasError(), tt.hasError)
7782
if results[0].HasError() {
78-
require.Contains(t, results[0].Errors[0].Error(), tt.errString)
83+
if !tt.hasError {
84+
t.Errorf("%s: expected no errors, got: %+q", tt.description, results[0].Errors)
85+
} else {
86+
require.Contains(t, results[0].Errors[0].Error(), tt.errString, tt.description)
87+
}
88+
} else if tt.hasError {
89+
t.Errorf("%s: expected error %q, got none", tt.description, tt.errString)
7990
}
8091
}
8192
}

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
apiVersion: apiextensions.k8s.io/v1
2+
kind: CustomResourceDefinition
3+
metadata:
4+
name: etcdclusters.etcd.database.coreos.com
5+
spec:
6+
conversion:
7+
strategy: Webhook
8+
webhook:
9+
clientConfig:
10+
service:
11+
namespace: system
12+
name: webhook-service
13+
path: /convert
14+
group: etcd.database.coreos.com
15+
names:
16+
kind: EtcdCluster
17+
listKind: EtcdClusterList
18+
plural: etcdclusters
19+
shortNames:
20+
- etcdclus
21+
- etcd
22+
singular: etcdcluster
23+
scope: Namespaced
24+
versions:
25+
- name: v1beta2
26+
served: true
27+
storage: true

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,16 @@ kind: CustomResourceDefinition
33
metadata:
44
name: etcdclusters.etcd.database.coreos.com
55
spec:
6+
conversion:
7+
strategy: Webhook
8+
webhook:
9+
clientConfig:
10+
service:
11+
namespace: system
12+
name: webhook-service
13+
path: /convert
14+
conversionReviewVersions:
15+
- v1
616
group: etcd.database.coreos.com
717
names:
818
kind: EtcdCluster

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,13 @@ kind: CustomResourceDefinition
33
metadata:
44
name: etcdclusters.etcd.database.coreos.com
55
spec:
6+
conversion:
7+
strategy: Webhook
8+
webhookClientConfig:
9+
service:
10+
namespace: system
11+
name: webhook-service
12+
path: /convert
613
group: etcd.database.coreos.com
714
names:
815
kind: EtcdCluster
@@ -12,5 +19,6 @@ spec:
1219
- etcdclus
1320
- etcd
1421
singular: etcdcluster
22+
preserveUnknownFields: false
1523
scope: Namespaced
1624
version: v1beta2

0 commit comments

Comments
 (0)