Skip to content

Commit eebe373

Browse files
committed
fix(crd-upgrade-safety): Safely handle changes to description fields
Motivation: When attempting to upgrade argocd-operator from v0.5.0 to v0.7.0, the upgrade process fails during the preflight CRD safety validation. The validation correctly detects that the `argocds.argoproj.io` CRD has been modified between the two versions. The specific error reported is: ``` CustomResourceDefinition argocds.argoproj.io failed upgrade safety validation. "ChangeValidator" validation failed: version "v1alpha1", field "^.status.applicationController" has unknown change, refusing to determine that change is safe ``` However, changes between the CRD versions in this instance are limited to non-functional updates in the description fields of various properties (e.g., status.applicationController).`ChangeValidator` lacks a specific rule to classify a description-only update as safe, which blocks legitimate and otherwise safe operator upgrades. Solution: This PR enhances the CRD upgrade safety validation logic to correctly handle changes to description fields by introducing a new `ChangeValidation` check for `Description`, and registering the check by adding it to the default list of `ChangeValidations` used by `ChangeValidator`. Result: Non-functional updates to documentation fields are now deemed safe(which resolves the upgrade failure for argocd-operator from v0.5.0 to v0.7.0)
1 parent b152c7b commit eebe373

File tree

3 files changed

+113
-0
lines changed

3 files changed

+113
-0
lines changed

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,3 +242,13 @@ func Type(diff FieldDiff) (bool, error) {
242242

243243
return isHandled(diff, reset), err
244244
}
245+
246+
// Description changes are considered safe and non-breaking.
247+
func Description(diff FieldDiff) (bool, error) {
248+
reset := func(diff FieldDiff) FieldDiff {
249+
diff.Old.Description = ""
250+
diff.New.Description = ""
251+
return diff
252+
}
253+
return isHandled(diff, reset), nil
254+
}

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

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -904,3 +904,105 @@ func TestType(t *testing.T) {
904904
})
905905
}
906906
}
907+
908+
func TestDescription(t *testing.T) {
909+
for _, tc := range []testcase{
910+
{
911+
name: "no diff, no error, handled",
912+
diff: FieldDiff{
913+
Old: &apiextensionsv1.JSONSchemaProps{
914+
Description: "some field",
915+
},
916+
New: &apiextensionsv1.JSONSchemaProps{
917+
Description: "some field",
918+
},
919+
},
920+
err: nil,
921+
handled: true,
922+
},
923+
{
924+
name: "description changed, no error, handled",
925+
diff: FieldDiff{
926+
Old: &apiextensionsv1.JSONSchemaProps{
927+
Description: "old description",
928+
},
929+
New: &apiextensionsv1.JSONSchemaProps{
930+
Description: "new description",
931+
},
932+
},
933+
err: nil,
934+
handled: true,
935+
},
936+
{
937+
name: "description added, no error, handled",
938+
diff: FieldDiff{
939+
Old: &apiextensionsv1.JSONSchemaProps{},
940+
New: &apiextensionsv1.JSONSchemaProps{
941+
Description: "a new description was added",
942+
},
943+
},
944+
err: nil,
945+
handled: true,
946+
},
947+
{
948+
name: "description removed, no error, handled",
949+
diff: FieldDiff{
950+
Old: &apiextensionsv1.JSONSchemaProps{
951+
Description: "this description will be removed",
952+
},
953+
New: &apiextensionsv1.JSONSchemaProps{},
954+
},
955+
err: nil,
956+
handled: true,
957+
},
958+
{
959+
name: "different field changed, no error, not handled",
960+
diff: FieldDiff{
961+
Old: &apiextensionsv1.JSONSchemaProps{
962+
ID: "foo",
963+
},
964+
New: &apiextensionsv1.JSONSchemaProps{
965+
ID: "bar",
966+
},
967+
},
968+
err: nil,
969+
handled: false,
970+
},
971+
{
972+
name: "different field changed with description, no error, not handled",
973+
diff: FieldDiff{
974+
Old: &apiextensionsv1.JSONSchemaProps{
975+
ID: "foo",
976+
Description: "description",
977+
},
978+
New: &apiextensionsv1.JSONSchemaProps{
979+
ID: "bar",
980+
Description: "description",
981+
},
982+
},
983+
err: nil,
984+
handled: false,
985+
},
986+
{
987+
name: "description and ID changed, no error, not handled",
988+
diff: FieldDiff{
989+
Old: &apiextensionsv1.JSONSchemaProps{
990+
ID: "foo",
991+
Description: "old description",
992+
},
993+
New: &apiextensionsv1.JSONSchemaProps{
994+
ID: "bar",
995+
Description: "new description",
996+
},
997+
},
998+
err: nil,
999+
handled: false,
1000+
},
1001+
} {
1002+
t.Run(tc.name, func(t *testing.T) {
1003+
handled, err := Description(tc.diff)
1004+
require.Equal(t, tc.err, err)
1005+
require.Equal(t, tc.handled, handled)
1006+
})
1007+
}
1008+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ type Preflight struct {
3131

3232
func NewPreflight(crdCli apiextensionsv1client.CustomResourceDefinitionInterface, opts ...Option) *Preflight {
3333
changeValidations := []ChangeValidation{
34+
Description,
3435
Enum,
3536
Required,
3637
Maximum,

0 commit comments

Comments
 (0)