Skip to content

Commit 4cc480f

Browse files
DirectXMan12DirectXMan12
committed
Collapse Common CRD Version Info
The Kube API server requires that information common to all versions of a CRD be collapsed into the top-level fields in v1beta1, since not doing so would create ambiguity when round-tripping through the upcoming v1 version (which doesn't have top-level fields). This makes sure we collapse things.
1 parent 6f93faf commit 4cc480f

File tree

3 files changed

+300
-1
lines changed

3 files changed

+300
-1
lines changed

‎Gopkg.lock

Lines changed: 3 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/crd/crd_spec_test.go

Lines changed: 212 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,212 @@
1+
/*
2+
Copyright 2019 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package crd_test
18+
19+
import (
20+
. "github.com/onsi/ginkgo"
21+
. "github.com/onsi/gomega"
22+
apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
23+
24+
"sigs.k8s.io/controller-tools/pkg/crd"
25+
)
26+
27+
var _ = Describe("CRD Generation", func() {
28+
Describe("Utilities", func() {
29+
Describe("MergeIdenticalVersionInfo", func() {
30+
It("should replace per-version schemata with a top-level schema if all are identical", func() {
31+
spec := &apiext.CustomResourceDefinition{
32+
Spec: apiext.CustomResourceDefinitionSpec{
33+
Versions: []apiext.CustomResourceDefinitionVersion{
34+
{
35+
Name: "v1",
36+
Schema: &apiext.CustomResourceValidation{
37+
OpenAPIV3Schema: &apiext.JSONSchemaProps{
38+
Required: []string{"foo"},
39+
Type: "object",
40+
Properties: map[string]apiext.JSONSchemaProps{"foo": apiext.JSONSchemaProps{Type: "string"}},
41+
},
42+
},
43+
},
44+
{
45+
Name: "v2",
46+
Storage: true,
47+
Schema: &apiext.CustomResourceValidation{
48+
OpenAPIV3Schema: &apiext.JSONSchemaProps{
49+
Required: []string{"foo"},
50+
Type: "object",
51+
Properties: map[string]apiext.JSONSchemaProps{"foo": apiext.JSONSchemaProps{Type: "string"}},
52+
},
53+
},
54+
},
55+
},
56+
},
57+
}
58+
crd.MergeIdenticalVersionInfo(spec)
59+
Expect(spec.Spec.Validation).To(Equal(&apiext.CustomResourceValidation{
60+
OpenAPIV3Schema: &apiext.JSONSchemaProps{
61+
Required: []string{"foo"},
62+
Type: "object",
63+
Properties: map[string]apiext.JSONSchemaProps{"foo": apiext.JSONSchemaProps{Type: "string"}},
64+
},
65+
}))
66+
Expect(spec.Spec.Versions).To(Equal([]apiext.CustomResourceDefinitionVersion{
67+
{Name: "v1"}, {Name: "v2", Storage: true},
68+
}))
69+
})
70+
71+
It("shouldn't merge different schemata", func() {
72+
spec := &apiext.CustomResourceDefinition{
73+
Spec: apiext.CustomResourceDefinitionSpec{
74+
Versions: []apiext.CustomResourceDefinitionVersion{
75+
{
76+
Name: "v1",
77+
Schema: &apiext.CustomResourceValidation{
78+
OpenAPIV3Schema: &apiext.JSONSchemaProps{
79+
Type: "object",
80+
Properties: map[string]apiext.JSONSchemaProps{"foo": apiext.JSONSchemaProps{Type: "string"}},
81+
},
82+
},
83+
},
84+
{
85+
Name: "v2",
86+
Storage: true,
87+
Schema: &apiext.CustomResourceValidation{
88+
OpenAPIV3Schema: &apiext.JSONSchemaProps{
89+
Required: []string{"foo"},
90+
Type: "object",
91+
Properties: map[string]apiext.JSONSchemaProps{"foo": apiext.JSONSchemaProps{Type: "string"}},
92+
},
93+
},
94+
},
95+
},
96+
},
97+
}
98+
orig := spec.DeepCopy()
99+
crd.MergeIdenticalVersionInfo(spec)
100+
Expect(spec).To(Equal(orig))
101+
})
102+
103+
It("should replace per-version subresources with top-level subresources if all are identical", func() {
104+
spec := &apiext.CustomResourceDefinition{
105+
Spec: apiext.CustomResourceDefinitionSpec{
106+
Versions: []apiext.CustomResourceDefinitionVersion{
107+
{
108+
Name: "v1",
109+
Subresources: &apiext.CustomResourceSubresources{
110+
Status: &apiext.CustomResourceSubresourceStatus{},
111+
},
112+
},
113+
{
114+
Name: "v2",
115+
Storage: true,
116+
Subresources: &apiext.CustomResourceSubresources{
117+
Status: &apiext.CustomResourceSubresourceStatus{},
118+
},
119+
},
120+
},
121+
},
122+
}
123+
124+
crd.MergeIdenticalVersionInfo(spec)
125+
Expect(spec.Spec.Subresources).To(Equal(&apiext.CustomResourceSubresources{
126+
Status: &apiext.CustomResourceSubresourceStatus{},
127+
}))
128+
Expect(spec.Spec.Versions).To(Equal([]apiext.CustomResourceDefinitionVersion{
129+
{Name: "v1"}, {Name: "v2", Storage: true},
130+
}))
131+
})
132+
133+
It("shouldn't merge different subresources", func() {
134+
spec := &apiext.CustomResourceDefinition{
135+
Spec: apiext.CustomResourceDefinitionSpec{
136+
Versions: []apiext.CustomResourceDefinitionVersion{
137+
{
138+
Name: "v1",
139+
Subresources: &apiext.CustomResourceSubresources{
140+
Status: &apiext.CustomResourceSubresourceStatus{},
141+
},
142+
},
143+
{
144+
Name: "v2",
145+
Storage: true,
146+
},
147+
},
148+
},
149+
}
150+
orig := spec.DeepCopy()
151+
crd.MergeIdenticalVersionInfo(spec)
152+
Expect(spec).To(Equal(orig))
153+
})
154+
155+
It("should replace per-version printer columns with top-level printer columns if all are identical", func() {
156+
spec := &apiext.CustomResourceDefinition{
157+
Spec: apiext.CustomResourceDefinitionSpec{
158+
Versions: []apiext.CustomResourceDefinitionVersion{
159+
{
160+
Name: "v1",
161+
AdditionalPrinterColumns: []apiext.CustomResourceColumnDefinition{
162+
{Name: "Cheddar", JSONPath: ".spec.cheddar"},
163+
{Name: "Parmesan", JSONPath: ".status.parmesan"},
164+
},
165+
},
166+
{
167+
Name: "v2",
168+
Storage: true,
169+
AdditionalPrinterColumns: []apiext.CustomResourceColumnDefinition{
170+
{Name: "Cheddar", JSONPath: ".spec.cheddar"},
171+
{Name: "Parmesan", JSONPath: ".status.parmesan"},
172+
},
173+
},
174+
},
175+
},
176+
}
177+
178+
crd.MergeIdenticalVersionInfo(spec)
179+
Expect(spec.Spec.AdditionalPrinterColumns).To(Equal([]apiext.CustomResourceColumnDefinition{
180+
{Name: "Cheddar", JSONPath: ".spec.cheddar"},
181+
{Name: "Parmesan", JSONPath: ".status.parmesan"},
182+
}))
183+
Expect(spec.Spec.Versions).To(Equal([]apiext.CustomResourceDefinitionVersion{
184+
{Name: "v1"}, {Name: "v2", Storage: true},
185+
}))
186+
})
187+
188+
It("shouldn't merge different printer columns", func() {
189+
spec := &apiext.CustomResourceDefinition{
190+
Spec: apiext.CustomResourceDefinitionSpec{
191+
Versions: []apiext.CustomResourceDefinitionVersion{
192+
{
193+
Name: "v1",
194+
AdditionalPrinterColumns: []apiext.CustomResourceColumnDefinition{
195+
{Name: "Cheddar", JSONPath: ".spec.cheddar"},
196+
{Name: "Parmesan", JSONPath: ".status.parmesan"},
197+
},
198+
},
199+
{
200+
Name: "v2",
201+
Storage: true,
202+
},
203+
},
204+
},
205+
}
206+
orig := spec.DeepCopy()
207+
crd.MergeIdenticalVersionInfo(spec)
208+
Expect(spec).To(Equal(orig))
209+
})
210+
})
211+
})
212+
})

‎pkg/crd/spec.go

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"github.com/gobuffalo/flect"
2323

2424
apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
25+
"k8s.io/apimachinery/pkg/api/equality"
2526
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2627
"k8s.io/apimachinery/pkg/runtime/schema"
2728

@@ -36,6 +37,86 @@ type SpecMarker interface {
3637
ApplyToCRD(crd *apiext.CustomResourceDefinitionSpec, version string) error
3738
}
3839

40+
// mergeIdenticalSubresources checks to see if subresources are identical across
41+
// all versions, and if so, merges them into a top-level version.
42+
//
43+
// This assumes you're not using trivial versions.
44+
func mergeIdenticalSubresources(crd *apiext.CustomResourceDefinition) {
45+
subres := crd.Spec.Versions[0].Subresources
46+
for _, ver := range crd.Spec.Versions {
47+
if ver.Subresources == nil || !equality.Semantic.DeepEqual(subres, ver.Subresources) {
48+
// either all nil, or not identical
49+
return
50+
}
51+
}
52+
53+
// things are identical if we've gotten this far, so move the subresources up
54+
// and discard the identical per-version ones
55+
crd.Spec.Subresources = subres
56+
for i := range crd.Spec.Versions {
57+
crd.Spec.Versions[i].Subresources = nil
58+
}
59+
}
60+
61+
// mergeIdenticalSchemata checks to see if schemata are identical across
62+
// all versions, and if so, merges them into a top-level version.
63+
//
64+
// This assumes you're not using trivial versions.
65+
func mergeIdenticalSchemata(crd *apiext.CustomResourceDefinition) {
66+
schema := crd.Spec.Versions[0].Schema
67+
for _, ver := range crd.Spec.Versions {
68+
if ver.Schema == nil || !equality.Semantic.DeepEqual(schema, ver.Schema) {
69+
// either all nil, or not identical
70+
return
71+
}
72+
}
73+
74+
// things are identical if we've gotten this far, so move the schemata up
75+
// to a single schema and discard the identical per-version ones
76+
crd.Spec.Validation = schema
77+
for i := range crd.Spec.Versions {
78+
crd.Spec.Versions[i].Schema = nil
79+
}
80+
}
81+
82+
// mergeIdenticalPrinterColumns checks to see if schemata are identical across
83+
// all versions, and if so, merges them into a top-level version.
84+
//
85+
// This assumes you're not using trivial versions.
86+
func mergeIdenticalPrinterColumns(crd *apiext.CustomResourceDefinition) {
87+
cols := crd.Spec.Versions[0].AdditionalPrinterColumns
88+
for _, ver := range crd.Spec.Versions {
89+
if len(ver.AdditionalPrinterColumns) == 0 || !equality.Semantic.DeepEqual(cols, ver.AdditionalPrinterColumns) {
90+
// either all nil, or not identical
91+
return
92+
}
93+
}
94+
95+
// things are identical if we've gotten this far, so move the printer columns up
96+
// and discard the identical per-version ones
97+
crd.Spec.AdditionalPrinterColumns = cols
98+
for i := range crd.Spec.Versions {
99+
crd.Spec.Versions[i].AdditionalPrinterColumns = nil
100+
}
101+
}
102+
103+
// MergeIdenticalVersionInfo makes sure that components of the Versions field that are identical
104+
// across all versions get merged into the top-level fields in v1beta1.
105+
//
106+
// This is required by the Kubernetes API server validation.
107+
//
108+
// The reason is that a v1beta1 -> v1 -> v1beta1 conversion cycle would need to
109+
// round-trip identically, v1 doesn't have top-level subresources, and without
110+
// this restriction it would be ambiguous how a v1-with-identical-subresources
111+
// converts into a v1beta1).
112+
func MergeIdenticalVersionInfo(crd *apiext.CustomResourceDefinition) {
113+
if len(crd.Spec.Versions) > 1 {
114+
mergeIdenticalSubresources(crd)
115+
mergeIdenticalSchemata(crd)
116+
mergeIdenticalPrinterColumns(crd)
117+
}
118+
}
119+
39120
// NeedCRDFor requests the full CRD for the given group-kind. It requires
40121
// that the packages containing the Go structs for that CRD have already
41122
// been loaded with NeedPackage.
@@ -144,5 +225,9 @@ func (p *Parser) NeedCRDFor(groupKind schema.GroupKind) {
144225
crd.Status.Conditions = []apiext.CustomResourceDefinitionCondition{}
145226
crd.Status.StoredVersions = []string{}
146227

228+
// make sure we merge identical per-version parts, to avoid validation errors
229+
// (see the reasoning near the top of the file).
230+
MergeIdenticalVersionInfo(&crd)
231+
147232
p.CustomResourceDefinitions[groupKind] = crd
148233
}

0 commit comments

Comments
 (0)