Skip to content

Commit a82e95b

Browse files
azychazych
committed
refactor: extract ServedVersionValidator to its own file
1 parent 46c0846 commit a82e95b

File tree

4 files changed

+265
-248
lines changed

4 files changed

+265
-248
lines changed

‎internal/operator-controller/rukpak/preflights/crdupgradesafety/checks.go‎

Lines changed: 0 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -3,80 +3,13 @@ package crdupgradesafety
33
import (
44
"bytes"
55
"cmp"
6-
"errors"
76
"fmt"
8-
"maps"
97
"reflect"
10-
"slices"
118

129
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
1310
"k8s.io/apimachinery/pkg/util/sets"
14-
versionhelper "k8s.io/apimachinery/pkg/version"
1511
)
1612

17-
type ServedVersionValidator struct {
18-
Validations []ChangeValidation
19-
}
20-
21-
func (c *ServedVersionValidator) Validate(old, new apiextensionsv1.CustomResourceDefinition) error {
22-
// If conversion webhook is specified, pass check
23-
if new.Spec.Conversion != nil && new.Spec.Conversion.Strategy == apiextensionsv1.WebhookConverter {
24-
return nil
25-
}
26-
27-
errs := []error{}
28-
servedVersions := []apiextensionsv1.CustomResourceDefinitionVersion{}
29-
for _, version := range new.Spec.Versions {
30-
if version.Served {
31-
servedVersions = append(servedVersions, version)
32-
}
33-
}
34-
35-
slices.SortFunc(servedVersions, func(a, b apiextensionsv1.CustomResourceDefinitionVersion) int {
36-
return versionhelper.CompareKubeAwareVersionStrings(a.Name, b.Name)
37-
})
38-
39-
for i, oldVersion := range servedVersions[:len(servedVersions)-1] {
40-
for _, newVersion := range servedVersions[i+1:] {
41-
flatOld := FlattenSchema(oldVersion.Schema.OpenAPIV3Schema)
42-
flatNew := FlattenSchema(newVersion.Schema.OpenAPIV3Schema)
43-
diffs, err := CalculateFlatSchemaDiff(flatOld, flatNew)
44-
if err != nil {
45-
errs = append(errs, fmt.Errorf("calculating schema diff between CRD versions %q and %q", oldVersion.Name, newVersion.Name))
46-
continue
47-
}
48-
49-
for _, field := range slices.Sorted(maps.Keys(diffs)) {
50-
diff := diffs[field]
51-
52-
handled := false
53-
for _, validation := range c.Validations {
54-
ok, err := validation(diff)
55-
if err != nil {
56-
errs = append(errs, fmt.Errorf("version upgrade %q to %q, field %q: %w", oldVersion.Name, newVersion.Name, field, err))
57-
}
58-
if ok {
59-
handled = true
60-
break
61-
}
62-
}
63-
64-
if !handled {
65-
errs = append(errs, fmt.Errorf("version %q, field %q has unknown change, refusing to determine that change is safe", oldVersion.Name, field))
66-
}
67-
}
68-
}
69-
}
70-
if len(errs) > 0 {
71-
return errors.Join(errs...)
72-
}
73-
return nil
74-
}
75-
76-
func (c *ServedVersionValidator) Name() string {
77-
return "ServedVersionValidator"
78-
}
79-
8013
type resetFunc func(diff FieldDiff) FieldDiff
8114

8215
func isHandled(diff FieldDiff, reset resetFunc) bool {

‎internal/operator-controller/rukpak/preflights/crdupgradesafety/checks_test.go‎

Lines changed: 0 additions & 181 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,8 @@ package crdupgradesafety
22

33
import (
44
"errors"
5-
"fmt"
65
"testing"
76

8-
"github.com/stretchr/testify/assert"
97
"github.com/stretchr/testify/require"
108
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
119
"k8s.io/utils/ptr"
@@ -906,182 +904,3 @@ func TestType(t *testing.T) {
906904
})
907905
}
908906
}
909-
910-
func TestServedVersionValidator(t *testing.T) {
911-
validationErr1 := errors.New(`version "v1alpha1", field "^" has unknown change, refusing to determine that change is safe`)
912-
validationErr2 := errors.New(`version upgrade "v1alpha1" to "v1alpha2", field "^": fail`)
913-
914-
for _, tc := range []struct {
915-
name string
916-
servedVersionValidator *ServedVersionValidator
917-
new apiextensionsv1.CustomResourceDefinition
918-
expectedError error
919-
}{
920-
{
921-
name: "no changes, no error",
922-
servedVersionValidator: &ServedVersionValidator{
923-
Validations: []ChangeValidation{
924-
func(_ FieldDiff) (bool, error) {
925-
return false, errors.New("should not run")
926-
},
927-
},
928-
},
929-
new: apiextensionsv1.CustomResourceDefinition{
930-
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
931-
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
932-
{
933-
Name: "v1alpha1",
934-
Served: true,
935-
Schema: &apiextensionsv1.CustomResourceValidation{
936-
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{},
937-
},
938-
},
939-
},
940-
},
941-
},
942-
},
943-
{
944-
name: "changes, validation successful, change is fully handled, no error",
945-
servedVersionValidator: &ServedVersionValidator{
946-
Validations: []ChangeValidation{
947-
func(_ FieldDiff) (bool, error) {
948-
return true, nil
949-
},
950-
},
951-
},
952-
new: apiextensionsv1.CustomResourceDefinition{
953-
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
954-
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
955-
{
956-
Name: "v1alpha1",
957-
Served: true,
958-
Schema: &apiextensionsv1.CustomResourceValidation{
959-
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{},
960-
},
961-
},
962-
{
963-
Name: "v1alpha2",
964-
Served: true,
965-
Schema: &apiextensionsv1.CustomResourceValidation{
966-
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
967-
ID: "foo",
968-
},
969-
},
970-
},
971-
},
972-
},
973-
},
974-
},
975-
{
976-
name: "changes, validation successful, change not fully handled, error",
977-
servedVersionValidator: &ServedVersionValidator{
978-
Validations: []ChangeValidation{
979-
func(_ FieldDiff) (bool, error) {
980-
return false, nil
981-
},
982-
},
983-
},
984-
new: apiextensionsv1.CustomResourceDefinition{
985-
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
986-
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
987-
{
988-
Name: "v1alpha1",
989-
Served: true,
990-
Schema: &apiextensionsv1.CustomResourceValidation{
991-
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{},
992-
},
993-
},
994-
{
995-
Name: "v1alpha2",
996-
Served: true,
997-
Schema: &apiextensionsv1.CustomResourceValidation{
998-
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
999-
ID: "foo",
1000-
},
1001-
},
1002-
},
1003-
},
1004-
},
1005-
},
1006-
expectedError: validationErr1,
1007-
},
1008-
{
1009-
name: "changes, validation failed, change fully handled, error",
1010-
servedVersionValidator: &ServedVersionValidator{
1011-
Validations: []ChangeValidation{
1012-
func(_ FieldDiff) (bool, error) {
1013-
return true, errors.New("fail")
1014-
},
1015-
},
1016-
},
1017-
new: apiextensionsv1.CustomResourceDefinition{
1018-
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
1019-
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
1020-
{
1021-
Name: "v1alpha1",
1022-
Served: true,
1023-
Schema: &apiextensionsv1.CustomResourceValidation{
1024-
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{},
1025-
},
1026-
},
1027-
{
1028-
Name: "v1alpha2",
1029-
Served: true,
1030-
Schema: &apiextensionsv1.CustomResourceValidation{
1031-
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
1032-
ID: "foo",
1033-
},
1034-
},
1035-
},
1036-
},
1037-
},
1038-
},
1039-
expectedError: validationErr2,
1040-
},
1041-
{
1042-
name: "changes, validation failed, change not fully handled, ordered error",
1043-
servedVersionValidator: &ServedVersionValidator{
1044-
Validations: []ChangeValidation{
1045-
func(_ FieldDiff) (bool, error) {
1046-
return false, errors.New("fail")
1047-
},
1048-
func(_ FieldDiff) (bool, error) {
1049-
return false, errors.New("error")
1050-
},
1051-
},
1052-
},
1053-
new: apiextensionsv1.CustomResourceDefinition{
1054-
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
1055-
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
1056-
{
1057-
Name: "v1alpha1",
1058-
Served: true,
1059-
Schema: &apiextensionsv1.CustomResourceValidation{
1060-
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{},
1061-
},
1062-
},
1063-
{
1064-
Name: "v1alpha2",
1065-
Served: true,
1066-
Schema: &apiextensionsv1.CustomResourceValidation{
1067-
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
1068-
ID: "foo",
1069-
},
1070-
},
1071-
},
1072-
},
1073-
},
1074-
},
1075-
expectedError: fmt.Errorf("%w\n%s\n%w", validationErr2, `version upgrade "v1alpha1" to "v1alpha2", field "^": error`, validationErr1),
1076-
},
1077-
} {
1078-
t.Run(tc.name, func(t *testing.T) {
1079-
err := tc.servedVersionValidator.Validate(apiextensionsv1.CustomResourceDefinition{}, tc.new)
1080-
if tc.expectedError != nil {
1081-
assert.EqualError(t, err, tc.expectedError.Error())
1082-
} else {
1083-
assert.NoError(t, err)
1084-
}
1085-
})
1086-
}
1087-
}

‎internal/operator-controller/rukpak/preflights/crdupgradesafety/shared_version_validator.go‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
package crdupgradesafety
2+
3+
import (
4+
"errors"
5+
"fmt"
6+
"maps"
7+
"slices"
8+
9+
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
10+
versionhelper "k8s.io/apimachinery/pkg/version"
11+
)
12+
13+
type ServedVersionValidator struct {
14+
Validations []ChangeValidation
15+
}
16+
17+
func (c *ServedVersionValidator) Validate(old, new apiextensionsv1.CustomResourceDefinition) error {
18+
// If conversion webhook is specified, pass check
19+
if new.Spec.Conversion != nil && new.Spec.Conversion.Strategy == apiextensionsv1.WebhookConverter {
20+
return nil
21+
}
22+
23+
errs := []error{}
24+
servedVersions := []apiextensionsv1.CustomResourceDefinitionVersion{}
25+
for _, version := range new.Spec.Versions {
26+
if version.Served {
27+
servedVersions = append(servedVersions, version)
28+
}
29+
}
30+
31+
slices.SortFunc(servedVersions, func(a, b apiextensionsv1.CustomResourceDefinitionVersion) int {
32+
return versionhelper.CompareKubeAwareVersionStrings(a.Name, b.Name)
33+
})
34+
35+
for i, oldVersion := range servedVersions[:len(servedVersions)-1] {
36+
for _, newVersion := range servedVersions[i+1:] {
37+
flatOld := FlattenSchema(oldVersion.Schema.OpenAPIV3Schema)
38+
flatNew := FlattenSchema(newVersion.Schema.OpenAPIV3Schema)
39+
diffs, err := CalculateFlatSchemaDiff(flatOld, flatNew)
40+
if err != nil {
41+
errs = append(errs, fmt.Errorf("calculating schema diff between CRD versions %q and %q", oldVersion.Name, newVersion.Name))
42+
continue
43+
}
44+
45+
for _, field := range slices.Sorted(maps.Keys(diffs)) {
46+
diff := diffs[field]
47+
48+
handled := false
49+
for _, validation := range c.Validations {
50+
ok, err := validation(diff)
51+
if err != nil {
52+
errs = append(errs, fmt.Errorf("version upgrade %q to %q, field %q: %w", oldVersion.Name, newVersion.Name, field, err))
53+
}
54+
if ok {
55+
handled = true
56+
break
57+
}
58+
}
59+
60+
if !handled {
61+
errs = append(errs, fmt.Errorf("version %q, field %q has unknown change, refusing to determine that change is safe", oldVersion.Name, field))
62+
}
63+
}
64+
}
65+
}
66+
if len(errs) > 0 {
67+
return errors.Join(errs...)
68+
}
69+
return nil
70+
}
71+
72+
func (c *ServedVersionValidator) Name() string {
73+
return "ServedVersionValidator"
74+
}

0 commit comments

Comments
 (0)