Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit a43bcd5

Browse files
committedMar 19, 2025··
UPSTREAM: <carry>: fix crdupgradesafety diff when items schema differ (#1863)
* crdupgradesafety: original copies from kapp Signed-off-by: Joe Lanford <[email protected]> * crdupgradesafety: drop kapp dependency Signed-off-by: Joe Lanford <[email protected]> * crdupgradesafety: remove unused code Signed-off-by: Joe Lanford <[email protected]> * crdupgradesafety: fixup apiextensionsv1 import aliases Signed-off-by: Joe Lanford <[email protected]> * crdupgradesafety: fix to ignore diffs in child items Signed-off-by: Joe Lanford <[email protected]> --------- Signed-off-by: Joe Lanford <[email protected]>
1 parent 481e275 commit a43bcd5

File tree

10 files changed

+1063
-397
lines changed

10 files changed

+1063
-397
lines changed
 

‎go.mod

+1-11
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ module github.com/operator-framework/operator-controller
33
go 1.22.5
44

55
require (
6-
carvel.dev/kapp v0.63.3
76
github.com/BurntSushi/toml v1.4.0
87
github.com/Masterminds/semver/v3 v3.3.0
98
github.com/blang/semver/v4 v4.0.0
@@ -16,6 +15,7 @@ require (
1615
github.com/onsi/ginkgo/v2 v2.21.0
1716
github.com/onsi/gomega v1.35.1
1817
github.com/opencontainers/go-digest v1.0.0
18+
github.com/openshift/crd-schema-checker v0.0.0-20240404194209-35a9033b1d11
1919
github.com/operator-framework/api v0.27.0
2020
github.com/operator-framework/catalogd v1.0.0-rc1
2121
github.com/operator-framework/helm-operator-plugins v0.7.0
@@ -38,7 +38,6 @@ require (
3838
)
3939

4040
require (
41-
carvel.dev/vendir v0.40.0 // indirect
4241
dario.cat/mergo v1.0.1 // indirect
4342
github.com/AdaLogics/go-fuzz-headers v0.0.0-20230811130428-ced1acdcaa24 // indirect
4443
github.com/Azure/go-ansiterm v0.0.0-20230124172434-306776ec8161 // indirect
@@ -69,9 +68,6 @@ require (
6968
github.com/containers/libtrust v0.0.0-20230121012942-c1716e8a8d01 // indirect
7069
github.com/containers/ocicrypt v1.2.0 // indirect
7170
github.com/containers/storage v1.55.0 // indirect
72-
github.com/cppforlife/cobrautil v0.0.0-20221130162803-acdfead391ef // indirect
73-
github.com/cppforlife/color v1.9.1-0.20200716202919-6706ac40b835 // indirect
74-
github.com/cppforlife/go-cli-ui v0.0.0-20220425131040-94f26b16bc14 // indirect
7571
github.com/cyberphone/json-canonicalization v0.0.0-20231217050601-ba74d44ecf5f // indirect
7672
github.com/cyphar/filepath-securejoin v0.3.6 // indirect
7773
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
@@ -128,7 +124,6 @@ require (
128124
github.com/h2non/go-is-svg v0.0.0-20160927212452-35e8c4b0612c // indirect
129125
github.com/hashicorp/errwrap v1.1.0 // indirect
130126
github.com/hashicorp/go-multierror v1.1.1 // indirect
131-
github.com/hashicorp/go-version v1.6.0 // indirect
132127
github.com/huandu/xstrings v1.5.0 // indirect
133128
github.com/imdario/mergo v0.3.16 // indirect
134129
github.com/inconshreveable/mousetrap v1.1.0 // indirect
@@ -137,8 +132,6 @@ require (
137132
github.com/joelanford/ignore v0.1.1 // indirect
138133
github.com/josharian/intern v1.0.0 // indirect
139134
github.com/json-iterator/go v1.1.12 // indirect
140-
github.com/k14s/starlark-go v0.0.0-20200720175618-3a5c849cc368 // indirect
141-
github.com/k14s/ytt v0.36.0 // indirect
142135
github.com/klauspost/compress v1.17.11 // indirect
143136
github.com/klauspost/pgzip v1.2.6 // indirect
144137
github.com/lann/builder v0.0.0-20180802200727-47ae307949d0 // indirect
@@ -172,7 +165,6 @@ require (
172165
github.com/oklog/ulid v1.3.1 // indirect
173166
github.com/opencontainers/image-spec v1.1.0 // indirect
174167
github.com/opencontainers/runtime-spec v1.2.0 // indirect
175-
github.com/openshift/crd-schema-checker v0.0.0-20240404194209-35a9033b1d11 // indirect
176168
github.com/operator-framework/operator-lib v0.15.0 // indirect
177169
github.com/otiai10/copy v1.14.0 // indirect
178170
github.com/peterbourgon/diskv v2.0.1+incompatible // indirect
@@ -202,8 +194,6 @@ require (
202194
github.com/ulikunitz/xz v0.5.12 // indirect
203195
github.com/vbatts/tar-split v0.11.5 // indirect
204196
github.com/vbauerster/mpb/v8 v8.7.5 // indirect
205-
github.com/vito/go-interact v1.0.1 // indirect
206-
github.com/vmware-tanzu/carvel-kapp-controller v0.51.0 // indirect
207197
github.com/x448/float16 v0.8.4 // indirect
208198
github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb // indirect
209199
github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect

‎go.sum

-277
Large diffs are not rendered by default.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
// Originally copied from https://github.com/carvel-dev/kapp/tree/d7fc2e15439331aa3a379485bb124e91a0829d2e
2+
// Attribution:
3+
// Copyright 2024 The Carvel Authors.
4+
// SPDX-License-Identifier: Apache-2.0
5+
6+
package crdupgradesafety
7+
8+
import (
9+
"errors"
10+
"fmt"
11+
"reflect"
12+
13+
"github.com/openshift/crd-schema-checker/pkg/manifestcomparators"
14+
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
15+
"k8s.io/apimachinery/pkg/util/validation/field"
16+
)
17+
18+
// ChangeValidation is a function that accepts a FieldDiff
19+
// as a parameter and should return:
20+
// - a boolean representation of whether or not the change
21+
// - an error if the change would be unsafe
22+
// has been fully handled (i.e no additional changes exist)
23+
type ChangeValidation func(diff FieldDiff) (bool, error)
24+
25+
// ChangeValidator is a Validation implementation focused on
26+
// handling updates to existing fields in a CRD
27+
type ChangeValidator struct {
28+
// Validations is a slice of ChangeValidations
29+
// to run against each changed field
30+
Validations []ChangeValidation
31+
}
32+
33+
func (cv *ChangeValidator) Name() string {
34+
return "ChangeValidator"
35+
}
36+
37+
// Validate will compare each version in the provided existing and new CRDs.
38+
// Since the ChangeValidator is tailored to handling updates to existing fields in
39+
// each version of a CRD. As such the following is assumed:
40+
// - Validating the removal of versions during an update is handled outside of this
41+
// validator. If a version in the existing version of the CRD does not exist in the new
42+
// version that version of the CRD is skipped in this validator.
43+
// - Removal of existing fields is unsafe. Regardless of whether or not this is handled
44+
// by a validator outside this one, if a field is present in a version provided by the existing CRD
45+
// but not present in the same version provided by the new CRD this validation will fail.
46+
//
47+
// Additionally, any changes that are not validated and handled by the known ChangeValidations
48+
// are deemed as unsafe and returns an error.
49+
func (cv *ChangeValidator) Validate(old, new apiextensionsv1.CustomResourceDefinition) error {
50+
errs := []error{}
51+
for _, version := range old.Spec.Versions {
52+
newVersion := manifestcomparators.GetVersionByName(&new, version.Name)
53+
if newVersion == nil {
54+
// if the new version doesn't exist skip this version
55+
continue
56+
}
57+
flatOld := FlattenSchema(version.Schema.OpenAPIV3Schema)
58+
flatNew := FlattenSchema(newVersion.Schema.OpenAPIV3Schema)
59+
60+
diffs, err := CalculateFlatSchemaDiff(flatOld, flatNew)
61+
if err != nil {
62+
errs = append(errs, fmt.Errorf("calculating schema diff for CRD version %q", version.Name))
63+
continue
64+
}
65+
66+
for field, diff := range diffs {
67+
handled := false
68+
for _, validation := range cv.Validations {
69+
ok, err := validation(diff)
70+
if err != nil {
71+
errs = append(errs, fmt.Errorf("version %q, field %q: %w", version.Name, field, err))
72+
}
73+
if ok {
74+
handled = true
75+
break
76+
}
77+
}
78+
79+
if !handled {
80+
errs = append(errs, fmt.Errorf("version %q, field %q has unknown change, refusing to determine that change is safe", version.Name, field))
81+
}
82+
}
83+
}
84+
85+
if len(errs) > 0 {
86+
return errors.Join(errs...)
87+
}
88+
return nil
89+
}
90+
91+
type FieldDiff struct {
92+
Old *apiextensionsv1.JSONSchemaProps
93+
New *apiextensionsv1.JSONSchemaProps
94+
}
95+
96+
// FlatSchema is a flat representation of a CRD schema.
97+
type FlatSchema map[string]*apiextensionsv1.JSONSchemaProps
98+
99+
// FlattenSchema takes in a CRD version OpenAPIV3Schema and returns
100+
// a flattened representation of it. For example, a CRD with a schema of:
101+
// ```yaml
102+
//
103+
// ...
104+
// spec:
105+
// type: object
106+
// properties:
107+
// foo:
108+
// type: string
109+
// bar:
110+
// type: string
111+
// ...
112+
//
113+
// ```
114+
// would be represented as:
115+
//
116+
// map[string]*apiextensionsv1.JSONSchemaProps{
117+
// "^": {},
118+
// "^.spec": {},
119+
// "^.spec.foo": {},
120+
// "^.spec.bar": {},
121+
// }
122+
//
123+
// where "^" represents the "root" schema
124+
func FlattenSchema(schema *apiextensionsv1.JSONSchemaProps) FlatSchema {
125+
fieldMap := map[string]*apiextensionsv1.JSONSchemaProps{}
126+
127+
manifestcomparators.SchemaHas(schema,
128+
field.NewPath("^"),
129+
field.NewPath("^"),
130+
nil,
131+
func(s *apiextensionsv1.JSONSchemaProps, _, simpleLocation *field.Path, _ []*apiextensionsv1.JSONSchemaProps) bool {
132+
fieldMap[simpleLocation.String()] = s.DeepCopy()
133+
return false
134+
})
135+
136+
return fieldMap
137+
}
138+
139+
// CalculateFlatSchemaDiff finds fields in a FlatSchema that are different
140+
// and returns a mapping of field --> old and new field schemas. If a field
141+
// exists in the old FlatSchema but not the new an empty diff mapping and an error is returned.
142+
func CalculateFlatSchemaDiff(o, n FlatSchema) (map[string]FieldDiff, error) {
143+
diffMap := map[string]FieldDiff{}
144+
for field, schema := range o {
145+
if _, ok := n[field]; !ok {
146+
return diffMap, fmt.Errorf("field %q in existing not found in new", field)
147+
}
148+
newSchema := n[field]
149+
150+
// Copy the schemas and remove any child properties for comparison.
151+
// In theory this will focus in on detecting changes for only the
152+
// field we are looking at and ignore changes in the children fields.
153+
// Since we are iterating through the map that should have all fields
154+
// we should still detect changes in the children fields.
155+
oldCopy := schema.DeepCopy()
156+
newCopy := newSchema.DeepCopy()
157+
oldCopy.Properties, oldCopy.Items = nil, nil
158+
newCopy.Properties, newCopy.Items = nil, nil
159+
if !reflect.DeepEqual(oldCopy, newCopy) {
160+
diffMap[field] = FieldDiff{
161+
Old: oldCopy,
162+
New: newCopy,
163+
}
164+
}
165+
}
166+
return diffMap, nil
167+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,327 @@
1+
// Originally copied from https://github.com/carvel-dev/kapp/tree/d7fc2e15439331aa3a379485bb124e91a0829d2e
2+
// Attribution:
3+
// Copyright 2024 The Carvel Authors.
4+
// SPDX-License-Identifier: Apache-2.0
5+
6+
package crdupgradesafety_test
7+
8+
import (
9+
"errors"
10+
"testing"
11+
12+
"github.com/stretchr/testify/assert"
13+
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
14+
15+
"github.com/operator-framework/operator-controller/internal/rukpak/preflights/crdupgradesafety"
16+
)
17+
18+
func TestCalculateFlatSchemaDiff(t *testing.T) {
19+
for _, tc := range []struct {
20+
name string
21+
old crdupgradesafety.FlatSchema
22+
new crdupgradesafety.FlatSchema
23+
expectedDiff map[string]crdupgradesafety.FieldDiff
24+
shouldError bool
25+
}{
26+
{
27+
name: "no diff in schemas, empty diff, no error",
28+
old: crdupgradesafety.FlatSchema{
29+
"foo": &apiextensionsv1.JSONSchemaProps{},
30+
},
31+
new: crdupgradesafety.FlatSchema{
32+
"foo": &apiextensionsv1.JSONSchemaProps{},
33+
},
34+
expectedDiff: map[string]crdupgradesafety.FieldDiff{},
35+
},
36+
{
37+
name: "diff in schemas, diff returned, no error",
38+
old: crdupgradesafety.FlatSchema{
39+
"foo": &apiextensionsv1.JSONSchemaProps{},
40+
},
41+
new: crdupgradesafety.FlatSchema{
42+
"foo": &apiextensionsv1.JSONSchemaProps{
43+
ID: "bar",
44+
},
45+
},
46+
expectedDiff: map[string]crdupgradesafety.FieldDiff{
47+
"foo": {
48+
Old: &apiextensionsv1.JSONSchemaProps{},
49+
New: &apiextensionsv1.JSONSchemaProps{ID: "bar"},
50+
},
51+
},
52+
},
53+
{
54+
name: "diff in child properties only, no diff returned, no error",
55+
old: crdupgradesafety.FlatSchema{
56+
"foo": &apiextensionsv1.JSONSchemaProps{
57+
Properties: map[string]apiextensionsv1.JSONSchemaProps{
58+
"bar": {ID: "bar"},
59+
},
60+
},
61+
},
62+
new: crdupgradesafety.FlatSchema{
63+
"foo": &apiextensionsv1.JSONSchemaProps{
64+
Properties: map[string]apiextensionsv1.JSONSchemaProps{
65+
"bar": {ID: "baz"},
66+
},
67+
},
68+
},
69+
expectedDiff: map[string]crdupgradesafety.FieldDiff{},
70+
},
71+
{
72+
name: "diff in child items only, no diff returned, no error",
73+
old: crdupgradesafety.FlatSchema{
74+
"foo": &apiextensionsv1.JSONSchemaProps{
75+
Items: &apiextensionsv1.JSONSchemaPropsOrArray{Schema: &apiextensionsv1.JSONSchemaProps{ID: "bar"}},
76+
},
77+
},
78+
new: crdupgradesafety.FlatSchema{
79+
"foo": &apiextensionsv1.JSONSchemaProps{
80+
Items: &apiextensionsv1.JSONSchemaPropsOrArray{Schema: &apiextensionsv1.JSONSchemaProps{ID: "baz"}},
81+
},
82+
},
83+
expectedDiff: map[string]crdupgradesafety.FieldDiff{},
84+
},
85+
{
86+
name: "field exists in old but not new, no diff returned, error",
87+
old: crdupgradesafety.FlatSchema{
88+
"foo": &apiextensionsv1.JSONSchemaProps{},
89+
},
90+
new: crdupgradesafety.FlatSchema{
91+
"bar": &apiextensionsv1.JSONSchemaProps{},
92+
},
93+
expectedDiff: map[string]crdupgradesafety.FieldDiff{},
94+
shouldError: true,
95+
},
96+
} {
97+
t.Run(tc.name, func(t *testing.T) {
98+
diff, err := crdupgradesafety.CalculateFlatSchemaDiff(tc.old, tc.new)
99+
assert.Equal(t, tc.shouldError, err != nil, "should error? - %v", tc.shouldError)
100+
assert.Equal(t, tc.expectedDiff, diff)
101+
})
102+
}
103+
}
104+
105+
func TestFlattenSchema(t *testing.T) {
106+
schema := &apiextensionsv1.JSONSchemaProps{
107+
Properties: map[string]apiextensionsv1.JSONSchemaProps{
108+
"foo": {
109+
Properties: map[string]apiextensionsv1.JSONSchemaProps{
110+
"bar": {},
111+
},
112+
},
113+
"baz": {},
114+
},
115+
}
116+
117+
foo := schema.Properties["foo"]
118+
foobar := schema.Properties["foo"].Properties["bar"]
119+
baz := schema.Properties["baz"]
120+
expected := crdupgradesafety.FlatSchema{
121+
"^": schema,
122+
"^.foo": &foo,
123+
"^.foo.bar": &foobar,
124+
"^.baz": &baz,
125+
}
126+
127+
actual := crdupgradesafety.FlattenSchema(schema)
128+
129+
assert.Equal(t, expected, actual)
130+
}
131+
132+
func TestChangeValidator(t *testing.T) {
133+
for _, tc := range []struct {
134+
name string
135+
changeValidator *crdupgradesafety.ChangeValidator
136+
old apiextensionsv1.CustomResourceDefinition
137+
new apiextensionsv1.CustomResourceDefinition
138+
shouldError bool
139+
}{
140+
{
141+
name: "no changes, no error",
142+
changeValidator: &crdupgradesafety.ChangeValidator{
143+
Validations: []crdupgradesafety.ChangeValidation{
144+
func(_ crdupgradesafety.FieldDiff) (bool, error) {
145+
return false, errors.New("should not run")
146+
},
147+
},
148+
},
149+
old: apiextensionsv1.CustomResourceDefinition{
150+
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
151+
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
152+
{
153+
Name: "v1alpha1",
154+
Schema: &apiextensionsv1.CustomResourceValidation{
155+
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{},
156+
},
157+
},
158+
},
159+
},
160+
},
161+
new: apiextensionsv1.CustomResourceDefinition{
162+
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
163+
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
164+
{
165+
Name: "v1alpha1",
166+
Schema: &apiextensionsv1.CustomResourceValidation{
167+
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{},
168+
},
169+
},
170+
},
171+
},
172+
},
173+
},
174+
{
175+
name: "changes, validation successful, change is fully handled, no error",
176+
changeValidator: &crdupgradesafety.ChangeValidator{
177+
Validations: []crdupgradesafety.ChangeValidation{
178+
func(_ crdupgradesafety.FieldDiff) (bool, error) {
179+
return true, nil
180+
},
181+
},
182+
},
183+
old: apiextensionsv1.CustomResourceDefinition{
184+
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
185+
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
186+
{
187+
Name: "v1alpha1",
188+
Schema: &apiextensionsv1.CustomResourceValidation{
189+
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{},
190+
},
191+
},
192+
},
193+
},
194+
},
195+
new: apiextensionsv1.CustomResourceDefinition{
196+
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
197+
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
198+
{
199+
Name: "v1alpha1",
200+
Schema: &apiextensionsv1.CustomResourceValidation{
201+
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
202+
ID: "foo",
203+
},
204+
},
205+
},
206+
},
207+
},
208+
},
209+
},
210+
{
211+
name: "changes, validation successful, change not fully handled, error",
212+
changeValidator: &crdupgradesafety.ChangeValidator{
213+
Validations: []crdupgradesafety.ChangeValidation{
214+
func(_ crdupgradesafety.FieldDiff) (bool, error) {
215+
return false, nil
216+
},
217+
},
218+
},
219+
old: apiextensionsv1.CustomResourceDefinition{
220+
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
221+
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
222+
{
223+
Name: "v1alpha1",
224+
Schema: &apiextensionsv1.CustomResourceValidation{
225+
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{},
226+
},
227+
},
228+
},
229+
},
230+
},
231+
new: apiextensionsv1.CustomResourceDefinition{
232+
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
233+
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
234+
{
235+
Name: "v1alpha1",
236+
Schema: &apiextensionsv1.CustomResourceValidation{
237+
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
238+
ID: "foo",
239+
},
240+
},
241+
},
242+
},
243+
},
244+
},
245+
shouldError: true,
246+
},
247+
{
248+
name: "changes, validation failed, change fully handled, error",
249+
changeValidator: &crdupgradesafety.ChangeValidator{
250+
Validations: []crdupgradesafety.ChangeValidation{
251+
func(_ crdupgradesafety.FieldDiff) (bool, error) {
252+
return true, errors.New("fail")
253+
},
254+
},
255+
},
256+
old: apiextensionsv1.CustomResourceDefinition{
257+
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
258+
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
259+
{
260+
Name: "v1alpha1",
261+
Schema: &apiextensionsv1.CustomResourceValidation{
262+
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{},
263+
},
264+
},
265+
},
266+
},
267+
},
268+
new: apiextensionsv1.CustomResourceDefinition{
269+
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
270+
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
271+
{
272+
Name: "v1alpha1",
273+
Schema: &apiextensionsv1.CustomResourceValidation{
274+
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
275+
ID: "foo",
276+
},
277+
},
278+
},
279+
},
280+
},
281+
},
282+
shouldError: true,
283+
},
284+
{
285+
name: "changes, validation failed, change not fully handled, error",
286+
changeValidator: &crdupgradesafety.ChangeValidator{
287+
Validations: []crdupgradesafety.ChangeValidation{
288+
func(_ crdupgradesafety.FieldDiff) (bool, error) {
289+
return false, errors.New("fail")
290+
},
291+
},
292+
},
293+
old: apiextensionsv1.CustomResourceDefinition{
294+
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
295+
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
296+
{
297+
Name: "v1alpha1",
298+
Schema: &apiextensionsv1.CustomResourceValidation{
299+
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{},
300+
},
301+
},
302+
},
303+
},
304+
},
305+
new: apiextensionsv1.CustomResourceDefinition{
306+
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
307+
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
308+
{
309+
Name: "v1alpha1",
310+
Schema: &apiextensionsv1.CustomResourceValidation{
311+
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
312+
ID: "foo",
313+
},
314+
},
315+
},
316+
},
317+
},
318+
},
319+
shouldError: true,
320+
},
321+
} {
322+
t.Run(tc.name, func(t *testing.T) {
323+
err := tc.changeValidator.Validate(tc.old, tc.new)
324+
assert.Equal(t, tc.shouldError, err != nil, "should error? - %v", tc.shouldError)
325+
})
326+
}
327+
}

‎internal/rukpak/preflights/crdupgradesafety/checks.go

+30-31
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,13 @@ import (
88
"reflect"
99
"slices"
1010

11-
kappcus "carvel.dev/kapp/pkg/kapp/crdupgradesafety"
1211
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
1312
"k8s.io/apimachinery/pkg/util/sets"
1413
versionhelper "k8s.io/apimachinery/pkg/version"
1514
)
1615

1716
type ServedVersionValidator struct {
18-
Validations []kappcus.ChangeValidation
17+
Validations []ChangeValidation
1918
}
2019

2120
func (c *ServedVersionValidator) Validate(old, new apiextensionsv1.CustomResourceDefinition) error {
@@ -38,9 +37,9 @@ func (c *ServedVersionValidator) Validate(old, new apiextensionsv1.CustomResourc
3837

3938
for i, oldVersion := range servedVersions[:len(servedVersions)-1] {
4039
for _, newVersion := range servedVersions[i+1:] {
41-
flatOld := kappcus.FlattenSchema(oldVersion.Schema.OpenAPIV3Schema)
42-
flatNew := kappcus.FlattenSchema(newVersion.Schema.OpenAPIV3Schema)
43-
diffs, err := kappcus.CalculateFlatSchemaDiff(flatOld, flatNew)
40+
flatOld := FlattenSchema(oldVersion.Schema.OpenAPIV3Schema)
41+
flatNew := FlattenSchema(newVersion.Schema.OpenAPIV3Schema)
42+
diffs, err := CalculateFlatSchemaDiff(flatOld, flatNew)
4443
if err != nil {
4544
errs = append(errs, fmt.Errorf("calculating schema diff between CRD versions %q and %q", oldVersion.Name, newVersion.Name))
4645
continue
@@ -75,15 +74,15 @@ func (c *ServedVersionValidator) Name() string {
7574
return "ServedVersionValidator"
7675
}
7776

78-
type resetFunc func(diff kappcus.FieldDiff) kappcus.FieldDiff
77+
type resetFunc func(diff FieldDiff) FieldDiff
7978

80-
func isHandled(diff kappcus.FieldDiff, reset resetFunc) bool {
79+
func isHandled(diff FieldDiff, reset resetFunc) bool {
8180
diff = reset(diff)
8281
return reflect.DeepEqual(diff.Old, diff.New)
8382
}
8483

85-
func Enum(diff kappcus.FieldDiff) (bool, error) {
86-
reset := func(diff kappcus.FieldDiff) kappcus.FieldDiff {
84+
func Enum(diff FieldDiff) (bool, error) {
85+
reset := func(diff FieldDiff) FieldDiff {
8786
diff.Old.Enum = []apiextensionsv1.JSON{}
8887
diff.New.Enum = []apiextensionsv1.JSON{}
8988
return diff
@@ -111,8 +110,8 @@ func Enum(diff kappcus.FieldDiff) (bool, error) {
111110
return isHandled(diff, reset), err
112111
}
113112

114-
func Required(diff kappcus.FieldDiff) (bool, error) {
115-
reset := func(diff kappcus.FieldDiff) kappcus.FieldDiff {
113+
func Required(diff FieldDiff) (bool, error) {
114+
reset := func(diff FieldDiff) FieldDiff {
116115
diff.Old.Required = []string{}
117116
diff.New.Required = []string{}
118117
return diff
@@ -141,8 +140,8 @@ func maxVerification[T cmp.Ordered](older *T, newer *T) error {
141140
return err
142141
}
143142

144-
func Maximum(diff kappcus.FieldDiff) (bool, error) {
145-
reset := func(diff kappcus.FieldDiff) kappcus.FieldDiff {
143+
func Maximum(diff FieldDiff) (bool, error) {
144+
reset := func(diff FieldDiff) FieldDiff {
146145
diff.Old.Maximum = nil
147146
diff.New.Maximum = nil
148147
return diff
@@ -156,8 +155,8 @@ func Maximum(diff kappcus.FieldDiff) (bool, error) {
156155
return isHandled(diff, reset), err
157156
}
158157

159-
func MaxItems(diff kappcus.FieldDiff) (bool, error) {
160-
reset := func(diff kappcus.FieldDiff) kappcus.FieldDiff {
158+
func MaxItems(diff FieldDiff) (bool, error) {
159+
reset := func(diff FieldDiff) FieldDiff {
161160
diff.Old.MaxItems = nil
162161
diff.New.MaxItems = nil
163162
return diff
@@ -171,8 +170,8 @@ func MaxItems(diff kappcus.FieldDiff) (bool, error) {
171170
return isHandled(diff, reset), err
172171
}
173172

174-
func MaxLength(diff kappcus.FieldDiff) (bool, error) {
175-
reset := func(diff kappcus.FieldDiff) kappcus.FieldDiff {
173+
func MaxLength(diff FieldDiff) (bool, error) {
174+
reset := func(diff FieldDiff) FieldDiff {
176175
diff.Old.MaxLength = nil
177176
diff.New.MaxLength = nil
178177
return diff
@@ -186,8 +185,8 @@ func MaxLength(diff kappcus.FieldDiff) (bool, error) {
186185
return isHandled(diff, reset), err
187186
}
188187

189-
func MaxProperties(diff kappcus.FieldDiff) (bool, error) {
190-
reset := func(diff kappcus.FieldDiff) kappcus.FieldDiff {
188+
func MaxProperties(diff FieldDiff) (bool, error) {
189+
reset := func(diff FieldDiff) FieldDiff {
191190
diff.Old.MaxProperties = nil
192191
diff.New.MaxProperties = nil
193192
return diff
@@ -212,8 +211,8 @@ func minVerification[T cmp.Ordered](older *T, newer *T) error {
212211
return err
213212
}
214213

215-
func Minimum(diff kappcus.FieldDiff) (bool, error) {
216-
reset := func(diff kappcus.FieldDiff) kappcus.FieldDiff {
214+
func Minimum(diff FieldDiff) (bool, error) {
215+
reset := func(diff FieldDiff) FieldDiff {
217216
diff.Old.Minimum = nil
218217
diff.New.Minimum = nil
219218
return diff
@@ -227,8 +226,8 @@ func Minimum(diff kappcus.FieldDiff) (bool, error) {
227226
return isHandled(diff, reset), err
228227
}
229228

230-
func MinItems(diff kappcus.FieldDiff) (bool, error) {
231-
reset := func(diff kappcus.FieldDiff) kappcus.FieldDiff {
229+
func MinItems(diff FieldDiff) (bool, error) {
230+
reset := func(diff FieldDiff) FieldDiff {
232231
diff.Old.MinItems = nil
233232
diff.New.MinItems = nil
234233
return diff
@@ -242,8 +241,8 @@ func MinItems(diff kappcus.FieldDiff) (bool, error) {
242241
return isHandled(diff, reset), err
243242
}
244243

245-
func MinLength(diff kappcus.FieldDiff) (bool, error) {
246-
reset := func(diff kappcus.FieldDiff) kappcus.FieldDiff {
244+
func MinLength(diff FieldDiff) (bool, error) {
245+
reset := func(diff FieldDiff) FieldDiff {
247246
diff.Old.MinLength = nil
248247
diff.New.MinLength = nil
249248
return diff
@@ -257,8 +256,8 @@ func MinLength(diff kappcus.FieldDiff) (bool, error) {
257256
return isHandled(diff, reset), err
258257
}
259258

260-
func MinProperties(diff kappcus.FieldDiff) (bool, error) {
261-
reset := func(diff kappcus.FieldDiff) kappcus.FieldDiff {
259+
func MinProperties(diff FieldDiff) (bool, error) {
260+
reset := func(diff FieldDiff) FieldDiff {
262261
diff.Old.MinProperties = nil
263262
diff.New.MinProperties = nil
264263
return diff
@@ -272,8 +271,8 @@ func MinProperties(diff kappcus.FieldDiff) (bool, error) {
272271
return isHandled(diff, reset), err
273272
}
274273

275-
func Default(diff kappcus.FieldDiff) (bool, error) {
276-
reset := func(diff kappcus.FieldDiff) kappcus.FieldDiff {
274+
func Default(diff FieldDiff) (bool, error) {
275+
reset := func(diff FieldDiff) FieldDiff {
277276
diff.Old.Default = nil
278277
diff.New.Default = nil
279278
return diff
@@ -293,8 +292,8 @@ func Default(diff kappcus.FieldDiff) (bool, error) {
293292
return isHandled(diff, reset), err
294293
}
295294

296-
func Type(diff kappcus.FieldDiff) (bool, error) {
297-
reset := func(diff kappcus.FieldDiff) kappcus.FieldDiff {
295+
func Type(diff FieldDiff) (bool, error) {
296+
reset := func(diff FieldDiff) FieldDiff {
298297
diff.Old.Type = ""
299298
diff.New.Type = ""
300299
return diff

‎internal/rukpak/preflights/crdupgradesafety/checks_test.go

+57-58
Large diffs are not rendered by default.

‎internal/rukpak/preflights/crdupgradesafety/crdupgradesafety.go

+9-10
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
"fmt"
77
"strings"
88

9-
kappcus "carvel.dev/kapp/pkg/kapp/crdupgradesafety"
109
"helm.sh/helm/v3/pkg/release"
1110
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
1211
apiextensionsv1client "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1"
@@ -19,19 +18,19 @@ import (
1918

2019
type Option func(p *Preflight)
2120

22-
func WithValidator(v *kappcus.Validator) Option {
21+
func WithValidator(v *Validator) Option {
2322
return func(p *Preflight) {
2423
p.validator = v
2524
}
2625
}
2726

2827
type Preflight struct {
2928
crdClient apiextensionsv1client.CustomResourceDefinitionInterface
30-
validator *kappcus.Validator
29+
validator *Validator
3130
}
3231

3332
func NewPreflight(crdCli apiextensionsv1client.CustomResourceDefinitionInterface, opts ...Option) *Preflight {
34-
changeValidations := []kappcus.ChangeValidation{
33+
changeValidations := []ChangeValidation{
3534
Enum,
3635
Required,
3736
Maximum,
@@ -48,13 +47,13 @@ func NewPreflight(crdCli apiextensionsv1client.CustomResourceDefinitionInterface
4847
p := &Preflight{
4948
crdClient: crdCli,
5049
// create a default validator. Can be overridden via the options
51-
validator: &kappcus.Validator{
52-
Validations: []kappcus.Validation{
53-
kappcus.NewValidationFunc("NoScopeChange", kappcus.NoScopeChange),
54-
kappcus.NewValidationFunc("NoStoredVersionRemoved", kappcus.NoStoredVersionRemoved),
55-
kappcus.NewValidationFunc("NoExistingFieldRemoved", kappcus.NoExistingFieldRemoved),
50+
validator: &Validator{
51+
Validations: []Validation{
52+
NewValidationFunc("NoScopeChange", NoScopeChange),
53+
NewValidationFunc("NoStoredVersionRemoved", NoStoredVersionRemoved),
54+
NewValidationFunc("NoExistingFieldRemoved", NoExistingFieldRemoved),
5655
&ServedVersionValidator{Validations: changeValidations},
57-
&kappcus.ChangeValidator{Validations: changeValidations},
56+
&ChangeValidator{Validations: changeValidations},
5857
},
5958
},
6059
}

‎internal/rukpak/preflights/crdupgradesafety/crdupgradesafety_test.go

+9-10
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"strings"
88
"testing"
99

10-
kappcus "carvel.dev/kapp/pkg/kapp/crdupgradesafety"
1110
"github.com/stretchr/testify/require"
1211
"helm.sh/helm/v3/pkg/release"
1312
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
@@ -31,7 +30,7 @@ func (c *MockCRDGetter) Get(ctx context.Context, name string, options metav1.Get
3130
return c.oldCrd, c.getErr
3231
}
3332

34-
func newMockPreflight(crd *apiextensionsv1.CustomResourceDefinition, err error, customValidator *kappcus.Validator) *crdupgradesafety.Preflight {
33+
func newMockPreflight(crd *apiextensionsv1.CustomResourceDefinition, err error, customValidator *crdupgradesafety.Validator) *crdupgradesafety.Preflight {
3534
var preflightOpts []crdupgradesafety.Option
3635
if customValidator != nil {
3736
preflightOpts = append(preflightOpts, crdupgradesafety.WithValidator(customValidator))
@@ -76,7 +75,7 @@ func TestInstall(t *testing.T) {
7675
tests := []struct {
7776
name string
7877
oldCrdPath string
79-
validator *kappcus.Validator
78+
validator *crdupgradesafety.Validator
8079
release *release.Release
8180
wantErrMsgs []string
8281
wantCrdGetErr error
@@ -137,9 +136,9 @@ func TestInstall(t *testing.T) {
137136
Name: "test-release",
138137
Manifest: getManifestString(t, "old-crd.json"),
139138
},
140-
validator: &kappcus.Validator{
141-
Validations: []kappcus.Validation{
142-
kappcus.NewValidationFunc("test", func(old, new apiextensionsv1.CustomResourceDefinition) error {
139+
validator: &crdupgradesafety.Validator{
140+
Validations: []crdupgradesafety.Validation{
141+
crdupgradesafety.NewValidationFunc("test", func(old, new apiextensionsv1.CustomResourceDefinition) error {
143142
return fmt.Errorf("custom validation error!!")
144143
}),
145144
},
@@ -213,7 +212,7 @@ func TestUpgrade(t *testing.T) {
213212
tests := []struct {
214213
name string
215214
oldCrdPath string
216-
validator *kappcus.Validator
215+
validator *crdupgradesafety.Validator
217216
release *release.Release
218217
wantErrMsgs []string
219218
wantCrdGetErr error
@@ -274,9 +273,9 @@ func TestUpgrade(t *testing.T) {
274273
Name: "test-release",
275274
Manifest: getManifestString(t, "old-crd.json"),
276275
},
277-
validator: &kappcus.Validator{
278-
Validations: []kappcus.Validation{
279-
kappcus.NewValidationFunc("test", func(old, new apiextensionsv1.CustomResourceDefinition) error {
276+
validator: &crdupgradesafety.Validator{
277+
Validations: []crdupgradesafety.Validation{
278+
crdupgradesafety.NewValidationFunc("test", func(old, new apiextensionsv1.CustomResourceDefinition) error {
280279
return fmt.Errorf("custom validation error!!")
281280
}),
282281
},
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
// Originally copied from https://github.com/carvel-dev/kapp/tree/d7fc2e15439331aa3a379485bb124e91a0829d2e
2+
// Attribution:
3+
// Copyright 2024 The Carvel Authors.
4+
// SPDX-License-Identifier: Apache-2.0
5+
6+
package crdupgradesafety
7+
8+
import (
9+
"errors"
10+
"fmt"
11+
"strings"
12+
13+
"github.com/openshift/crd-schema-checker/pkg/manifestcomparators"
14+
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
15+
"k8s.io/apimachinery/pkg/util/sets"
16+
)
17+
18+
// Validation is a representation of a validation to run
19+
// against a CRD being upgraded
20+
type Validation interface {
21+
// Validate contains the actual validation logic. An error being
22+
// returned means validation has failed
23+
Validate(old, new apiextensionsv1.CustomResourceDefinition) error
24+
// Name returns a human-readable name for the validation
25+
Name() string
26+
}
27+
28+
// ValidateFunc is a function to validate a CustomResourceDefinition
29+
// for safe upgrades. It accepts the old and new CRDs and returns an
30+
// error if performing an upgrade from old -> new is unsafe.
31+
type ValidateFunc func(old, new apiextensionsv1.CustomResourceDefinition) error
32+
33+
// ValidationFunc is a helper to wrap a ValidateFunc
34+
// as an implementation of the Validation interface
35+
type ValidationFunc struct {
36+
name string
37+
validateFunc ValidateFunc
38+
}
39+
40+
func NewValidationFunc(name string, vfunc ValidateFunc) Validation {
41+
return &ValidationFunc{
42+
name: name,
43+
validateFunc: vfunc,
44+
}
45+
}
46+
47+
func (vf *ValidationFunc) Name() string {
48+
return vf.name
49+
}
50+
51+
func (vf *ValidationFunc) Validate(old, new apiextensionsv1.CustomResourceDefinition) error {
52+
return vf.validateFunc(old, new)
53+
}
54+
55+
type Validator struct {
56+
Validations []Validation
57+
}
58+
59+
func (v *Validator) Validate(old, new apiextensionsv1.CustomResourceDefinition) error {
60+
validateErrs := []error{}
61+
for _, validation := range v.Validations {
62+
if err := validation.Validate(old, new); err != nil {
63+
formattedErr := fmt.Errorf("CustomResourceDefinition %s failed upgrade safety validation. %q validation failed: %w",
64+
new.Name, validation.Name(), err)
65+
66+
validateErrs = append(validateErrs, formattedErr)
67+
}
68+
}
69+
if len(validateErrs) > 0 {
70+
return errors.Join(validateErrs...)
71+
}
72+
return nil
73+
}
74+
75+
func NoScopeChange(old, new apiextensionsv1.CustomResourceDefinition) error {
76+
if old.Spec.Scope != new.Spec.Scope {
77+
return fmt.Errorf("scope changed from %q to %q", old.Spec.Scope, new.Spec.Scope)
78+
}
79+
return nil
80+
}
81+
82+
func NoStoredVersionRemoved(old, new apiextensionsv1.CustomResourceDefinition) error {
83+
newVersions := sets.New[string]()
84+
for _, version := range new.Spec.Versions {
85+
if !newVersions.Has(version.Name) {
86+
newVersions.Insert(version.Name)
87+
}
88+
}
89+
90+
for _, storedVersion := range old.Status.StoredVersions {
91+
if !newVersions.Has(storedVersion) {
92+
return fmt.Errorf("stored version %q removed", storedVersion)
93+
}
94+
}
95+
96+
return nil
97+
}
98+
99+
func NoExistingFieldRemoved(old, new apiextensionsv1.CustomResourceDefinition) error {
100+
reg := manifestcomparators.NewRegistry()
101+
err := reg.AddComparator(manifestcomparators.NoFieldRemoval())
102+
if err != nil {
103+
return err
104+
}
105+
106+
results, errs := reg.Compare(&old, &new)
107+
if len(errs) > 0 {
108+
return errors.Join(errs...)
109+
}
110+
111+
errSet := []error{}
112+
113+
for _, result := range results {
114+
if len(result.Errors) > 0 {
115+
errSet = append(errSet, errors.New(strings.Join(result.Errors, "\n")))
116+
}
117+
}
118+
if len(errSet) > 0 {
119+
return errors.Join(errSet...)
120+
}
121+
122+
return nil
123+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,340 @@
1+
// Originally copied from https://github.com/carvel-dev/kapp/tree/d7fc2e15439331aa3a379485bb124e91a0829d2e
2+
// Attribution:
3+
// Copyright 2024 The Carvel Authors.
4+
// SPDX-License-Identifier: Apache-2.0
5+
6+
package crdupgradesafety
7+
8+
import (
9+
"errors"
10+
"testing"
11+
12+
"github.com/stretchr/testify/assert"
13+
"github.com/stretchr/testify/require"
14+
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
15+
)
16+
17+
func TestValidator(t *testing.T) {
18+
for _, tc := range []struct {
19+
name string
20+
validations []Validation
21+
shouldErr bool
22+
}{
23+
{
24+
name: "no validators, no error",
25+
validations: []Validation{},
26+
},
27+
{
28+
name: "passing validator, no error",
29+
validations: []Validation{
30+
NewValidationFunc("pass", func(_, _ apiextensionsv1.CustomResourceDefinition) error {
31+
return nil
32+
}),
33+
},
34+
},
35+
{
36+
name: "failing validator, error",
37+
validations: []Validation{
38+
NewValidationFunc("fail", func(_, _ apiextensionsv1.CustomResourceDefinition) error {
39+
return errors.New("boom")
40+
}),
41+
},
42+
shouldErr: true,
43+
},
44+
{
45+
name: "passing+failing validator, error",
46+
validations: []Validation{
47+
NewValidationFunc("pass", func(_, _ apiextensionsv1.CustomResourceDefinition) error {
48+
return nil
49+
}),
50+
NewValidationFunc("fail", func(_, _ apiextensionsv1.CustomResourceDefinition) error {
51+
return errors.New("boom")
52+
}),
53+
},
54+
shouldErr: true,
55+
},
56+
} {
57+
t.Run(tc.name, func(t *testing.T) {
58+
v := Validator{
59+
Validations: tc.validations,
60+
}
61+
var o, n apiextensionsv1.CustomResourceDefinition
62+
63+
err := v.Validate(o, n)
64+
require.Equal(t, tc.shouldErr, err != nil)
65+
})
66+
}
67+
}
68+
69+
func TestNoScopeChange(t *testing.T) {
70+
for _, tc := range []struct {
71+
name string
72+
old apiextensionsv1.CustomResourceDefinition
73+
new apiextensionsv1.CustomResourceDefinition
74+
shouldError bool
75+
}{
76+
{
77+
name: "no scope change, no error",
78+
old: apiextensionsv1.CustomResourceDefinition{
79+
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
80+
Scope: apiextensionsv1.ClusterScoped,
81+
},
82+
},
83+
new: apiextensionsv1.CustomResourceDefinition{
84+
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
85+
Scope: apiextensionsv1.ClusterScoped,
86+
},
87+
},
88+
},
89+
{
90+
name: "scope change, error",
91+
old: apiextensionsv1.CustomResourceDefinition{
92+
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
93+
Scope: apiextensionsv1.ClusterScoped,
94+
},
95+
},
96+
new: apiextensionsv1.CustomResourceDefinition{
97+
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
98+
Scope: apiextensionsv1.NamespaceScoped,
99+
},
100+
},
101+
shouldError: true,
102+
},
103+
} {
104+
t.Run(tc.name, func(t *testing.T) {
105+
err := NoScopeChange(tc.old, tc.new)
106+
require.Equal(t, tc.shouldError, err != nil)
107+
})
108+
}
109+
}
110+
111+
func TestNoStoredVersionRemoved(t *testing.T) {
112+
for _, tc := range []struct {
113+
name string
114+
old apiextensionsv1.CustomResourceDefinition
115+
new apiextensionsv1.CustomResourceDefinition
116+
shouldError bool
117+
}{
118+
{
119+
name: "no stored versions, no error",
120+
new: apiextensionsv1.CustomResourceDefinition{
121+
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
122+
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
123+
{
124+
Name: "v1alpha1",
125+
},
126+
},
127+
},
128+
},
129+
old: apiextensionsv1.CustomResourceDefinition{},
130+
},
131+
{
132+
name: "stored versions, no stored version removed, no error",
133+
new: apiextensionsv1.CustomResourceDefinition{
134+
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
135+
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
136+
{
137+
Name: "v1alpha1",
138+
},
139+
{
140+
Name: "v1alpha2",
141+
},
142+
},
143+
},
144+
},
145+
old: apiextensionsv1.CustomResourceDefinition{
146+
Status: apiextensionsv1.CustomResourceDefinitionStatus{
147+
StoredVersions: []string{
148+
"v1alpha1",
149+
},
150+
},
151+
},
152+
},
153+
{
154+
name: "stored versions, stored version removed, error",
155+
new: apiextensionsv1.CustomResourceDefinition{
156+
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
157+
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
158+
{
159+
Name: "v1alpha2",
160+
},
161+
},
162+
},
163+
},
164+
old: apiextensionsv1.CustomResourceDefinition{
165+
Status: apiextensionsv1.CustomResourceDefinitionStatus{
166+
StoredVersions: []string{
167+
"v1alpha1",
168+
},
169+
},
170+
},
171+
shouldError: true,
172+
},
173+
} {
174+
t.Run(tc.name, func(t *testing.T) {
175+
err := NoStoredVersionRemoved(tc.old, tc.new)
176+
require.Equal(t, tc.shouldError, err != nil)
177+
})
178+
}
179+
}
180+
181+
func TestNoExistingFieldRemoved(t *testing.T) {
182+
for _, tc := range []struct {
183+
name string
184+
new apiextensionsv1.CustomResourceDefinition
185+
old apiextensionsv1.CustomResourceDefinition
186+
shouldError bool
187+
}{
188+
{
189+
name: "no existing field removed, no error",
190+
old: apiextensionsv1.CustomResourceDefinition{
191+
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
192+
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
193+
{
194+
Name: "v1alpha1",
195+
Schema: &apiextensionsv1.CustomResourceValidation{
196+
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
197+
Type: "object",
198+
Properties: map[string]apiextensionsv1.JSONSchemaProps{
199+
"fieldOne": {
200+
Type: "string",
201+
},
202+
},
203+
},
204+
},
205+
},
206+
},
207+
},
208+
},
209+
new: apiextensionsv1.CustomResourceDefinition{
210+
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
211+
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
212+
{
213+
Name: "v1alpha1",
214+
Schema: &apiextensionsv1.CustomResourceValidation{
215+
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
216+
Type: "object",
217+
Properties: map[string]apiextensionsv1.JSONSchemaProps{
218+
"fieldOne": {
219+
Type: "string",
220+
},
221+
},
222+
},
223+
},
224+
},
225+
},
226+
},
227+
},
228+
},
229+
{
230+
name: "existing field removed, error",
231+
old: apiextensionsv1.CustomResourceDefinition{
232+
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
233+
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
234+
{
235+
Name: "v1alpha1",
236+
Schema: &apiextensionsv1.CustomResourceValidation{
237+
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
238+
Type: "object",
239+
Properties: map[string]apiextensionsv1.JSONSchemaProps{
240+
"fieldOne": {
241+
Type: "string",
242+
},
243+
"fieldTwo": {
244+
Type: "string",
245+
},
246+
},
247+
},
248+
},
249+
},
250+
},
251+
},
252+
},
253+
new: apiextensionsv1.CustomResourceDefinition{
254+
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
255+
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
256+
{
257+
Name: "v1alpha1",
258+
Schema: &apiextensionsv1.CustomResourceValidation{
259+
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
260+
Type: "object",
261+
Properties: map[string]apiextensionsv1.JSONSchemaProps{
262+
"fieldOne": {
263+
Type: "string",
264+
},
265+
},
266+
},
267+
},
268+
},
269+
},
270+
},
271+
},
272+
shouldError: true,
273+
},
274+
{
275+
name: "new version is added with the field removed, no error",
276+
old: apiextensionsv1.CustomResourceDefinition{
277+
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
278+
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
279+
{
280+
Name: "v1alpha1",
281+
Schema: &apiextensionsv1.CustomResourceValidation{
282+
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
283+
Type: "object",
284+
Properties: map[string]apiextensionsv1.JSONSchemaProps{
285+
"fieldOne": {
286+
Type: "string",
287+
},
288+
"fieldTwo": {
289+
Type: "string",
290+
},
291+
},
292+
},
293+
},
294+
},
295+
},
296+
},
297+
},
298+
new: apiextensionsv1.CustomResourceDefinition{
299+
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
300+
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
301+
{
302+
Name: "v1alpha1",
303+
Schema: &apiextensionsv1.CustomResourceValidation{
304+
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
305+
Type: "object",
306+
Properties: map[string]apiextensionsv1.JSONSchemaProps{
307+
"fieldOne": {
308+
Type: "string",
309+
},
310+
"fieldTwo": {
311+
Type: "string",
312+
},
313+
},
314+
},
315+
},
316+
},
317+
{
318+
Name: "v1alpha2",
319+
Schema: &apiextensionsv1.CustomResourceValidation{
320+
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
321+
Type: "object",
322+
Properties: map[string]apiextensionsv1.JSONSchemaProps{
323+
"fieldOne": {
324+
Type: "string",
325+
},
326+
},
327+
},
328+
},
329+
},
330+
},
331+
},
332+
},
333+
},
334+
} {
335+
t.Run(tc.name, func(t *testing.T) {
336+
err := NoExistingFieldRemoved(tc.old, tc.new)
337+
assert.Equal(t, tc.shouldError, err != nil)
338+
})
339+
}
340+
}

0 commit comments

Comments
 (0)
Please sign in to comment.