Skip to content

Commit f51b02f

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 f51b02f

File tree

3 files changed

+98
-0
lines changed

3 files changed

+98
-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+
// A change in a description is always 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: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -904,3 +904,90 @@ 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: "this is a field",
977+
},
978+
New: &apiextensionsv1.JSONSchemaProps{
979+
ID: "bar",
980+
Description: "this is a field",
981+
},
982+
},
983+
err: nil,
984+
handled: false,
985+
},
986+
} {
987+
t.Run(tc.name, func(t *testing.T) {
988+
handled, err := Description(tc.diff)
989+
require.Equal(t, tc.err, err)
990+
require.Equal(t, tc.handled, handled)
991+
})
992+
}
993+
}

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)