From ce86a659db3744a7e6690423f7314627bd3ee003 Mon Sep 17 00:00:00 2001 From: Bryce Palmer Date: Thu, 7 Nov 2024 14:36:17 -0500 Subject: [PATCH] :sparkles: reduce use of carvel validators (#1269) * reduce use of carvel validators by implementing our own. This gives us more freedom in maing changes and fixing bugs in the actual validations instead of having to push all fixes up to the carvel/kapp project. Signed-off-by: everettraven * generic min/max verification functions Signed-off-by: everettraven --------- Signed-off-by: everettraven --- .../preflights/crdupgradesafety/checks.go | 237 +++++ .../crdupgradesafety/checks_test.go | 907 ++++++++++++++++++ .../crdupgradesafety/crdupgradesafety.go | 23 +- .../crdupgradesafety/crdupgradesafety_test.go | 46 +- 4 files changed, 1179 insertions(+), 34 deletions(-) create mode 100644 internal/rukpak/preflights/crdupgradesafety/checks_test.go diff --git a/internal/rukpak/preflights/crdupgradesafety/checks.go b/internal/rukpak/preflights/crdupgradesafety/checks.go index cc7be4a66..b795b11de 100644 --- a/internal/rukpak/preflights/crdupgradesafety/checks.go +++ b/internal/rukpak/preflights/crdupgradesafety/checks.go @@ -1,12 +1,16 @@ package crdupgradesafety import ( + "bytes" + "cmp" "errors" "fmt" + "reflect" "slices" kappcus "carvel.dev/kapp/pkg/kapp/crdupgradesafety" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apimachinery/pkg/util/sets" versionhelper "k8s.io/apimachinery/pkg/version" ) @@ -70,3 +74,236 @@ func (c *ServedVersionValidator) Validate(old, new apiextensionsv1.CustomResourc func (c *ServedVersionValidator) Name() string { return "ServedVersionValidator" } + +type resetFunc func(diff kappcus.FieldDiff) kappcus.FieldDiff + +func isHandled(diff kappcus.FieldDiff, reset resetFunc) bool { + diff = reset(diff) + return reflect.DeepEqual(diff.Old, diff.New) +} + +func Enum(diff kappcus.FieldDiff) (bool, error) { + reset := func(diff kappcus.FieldDiff) kappcus.FieldDiff { + diff.Old.Enum = []apiextensionsv1.JSON{} + diff.New.Enum = []apiextensionsv1.JSON{} + return diff + } + + oldEnums := sets.New[string]() + for _, json := range diff.Old.Enum { + oldEnums.Insert(string(json.Raw)) + } + + newEnums := sets.New[string]() + for _, json := range diff.New.Enum { + newEnums.Insert(string(json.Raw)) + } + diffEnums := oldEnums.Difference(newEnums) + var err error + + switch { + case oldEnums.Len() == 0 && newEnums.Len() > 0: + err = fmt.Errorf("enum constraints %v added when there were no restrictions previously", newEnums.UnsortedList()) + case diffEnums.Len() > 0: + err = fmt.Errorf("enums %v removed from the set of previously allowed values", diffEnums.UnsortedList()) + } + + return isHandled(diff, reset), err +} + +func Required(diff kappcus.FieldDiff) (bool, error) { + reset := func(diff kappcus.FieldDiff) kappcus.FieldDiff { + diff.Old.Required = []string{} + diff.New.Required = []string{} + return diff + } + + oldRequired := sets.New(diff.Old.Required...) + newRequired := sets.New(diff.New.Required...) + diffRequired := newRequired.Difference(oldRequired) + var err error + + if diffRequired.Len() > 0 { + err = fmt.Errorf("new required fields %v added", diffRequired.UnsortedList()) + } + + return isHandled(diff, reset), err +} + +func maxVerification[T cmp.Ordered](older *T, newer *T) error { + var err error + switch { + case older == nil && newer != nil: + err = fmt.Errorf("constraint %v added when there were no restrictions previously", *newer) + case older != nil && newer != nil && *newer < *older: + err = fmt.Errorf("constraint decreased from %v to %v", *older, *newer) + } + return err +} + +func Maximum(diff kappcus.FieldDiff) (bool, error) { + reset := func(diff kappcus.FieldDiff) kappcus.FieldDiff { + diff.Old.Maximum = nil + diff.New.Maximum = nil + return diff + } + + err := maxVerification(diff.Old.Maximum, diff.New.Maximum) + if err != nil { + err = fmt.Errorf("maximum: %s", err.Error()) + } + + return isHandled(diff, reset), err +} + +func MaxItems(diff kappcus.FieldDiff) (bool, error) { + reset := func(diff kappcus.FieldDiff) kappcus.FieldDiff { + diff.Old.MaxItems = nil + diff.New.MaxItems = nil + return diff + } + + err := maxVerification(diff.Old.MaxItems, diff.New.MaxItems) + if err != nil { + err = fmt.Errorf("maxItems: %s", err.Error()) + } + + return isHandled(diff, reset), err +} + +func MaxLength(diff kappcus.FieldDiff) (bool, error) { + reset := func(diff kappcus.FieldDiff) kappcus.FieldDiff { + diff.Old.MaxLength = nil + diff.New.MaxLength = nil + return diff + } + + err := maxVerification(diff.Old.MaxLength, diff.New.MaxLength) + if err != nil { + err = fmt.Errorf("maxLength: %s", err.Error()) + } + + return isHandled(diff, reset), err +} + +func MaxProperties(diff kappcus.FieldDiff) (bool, error) { + reset := func(diff kappcus.FieldDiff) kappcus.FieldDiff { + diff.Old.MaxProperties = nil + diff.New.MaxProperties = nil + return diff + } + + err := maxVerification(diff.Old.MaxProperties, diff.New.MaxProperties) + if err != nil { + err = fmt.Errorf("maxProperties: %s", err.Error()) + } + + return isHandled(diff, reset), err +} + +func minVerification[T cmp.Ordered](older *T, newer *T) error { + var err error + switch { + case older == nil && newer != nil: + err = fmt.Errorf("constraint %v added when there were no restrictions previously", *newer) + case older != nil && newer != nil && *newer > *older: + err = fmt.Errorf("constraint increased from %v to %v", *older, *newer) + } + return err +} + +func Minimum(diff kappcus.FieldDiff) (bool, error) { + reset := func(diff kappcus.FieldDiff) kappcus.FieldDiff { + diff.Old.Minimum = nil + diff.New.Minimum = nil + return diff + } + + err := minVerification(diff.Old.Minimum, diff.New.Minimum) + if err != nil { + err = fmt.Errorf("minimum: %s", err.Error()) + } + + return isHandled(diff, reset), err +} + +func MinItems(diff kappcus.FieldDiff) (bool, error) { + reset := func(diff kappcus.FieldDiff) kappcus.FieldDiff { + diff.Old.MinItems = nil + diff.New.MinItems = nil + return diff + } + + err := minVerification(diff.Old.MinItems, diff.New.MinItems) + if err != nil { + err = fmt.Errorf("minItems: %s", err.Error()) + } + + return isHandled(diff, reset), err +} + +func MinLength(diff kappcus.FieldDiff) (bool, error) { + reset := func(diff kappcus.FieldDiff) kappcus.FieldDiff { + diff.Old.MinLength = nil + diff.New.MinLength = nil + return diff + } + + err := minVerification(diff.Old.MinLength, diff.New.MinLength) + if err != nil { + err = fmt.Errorf("minLength: %s", err.Error()) + } + + return isHandled(diff, reset), err +} + +func MinProperties(diff kappcus.FieldDiff) (bool, error) { + reset := func(diff kappcus.FieldDiff) kappcus.FieldDiff { + diff.Old.MinProperties = nil + diff.New.MinProperties = nil + return diff + } + + err := minVerification(diff.Old.MinProperties, diff.New.MinProperties) + if err != nil { + err = fmt.Errorf("minProperties: %s", err.Error()) + } + + return isHandled(diff, reset), err +} + +func Default(diff kappcus.FieldDiff) (bool, error) { + reset := func(diff kappcus.FieldDiff) kappcus.FieldDiff { + diff.Old.Default = nil + diff.New.Default = nil + return diff + } + + var err error + + switch { + case diff.Old.Default == nil && diff.New.Default != nil: + err = fmt.Errorf("default value %q added when there was no default previously", string(diff.New.Default.Raw)) + case diff.Old.Default != nil && diff.New.Default == nil: + err = fmt.Errorf("default value %q removed", string(diff.Old.Default.Raw)) + case diff.Old.Default != nil && diff.New.Default != nil && !bytes.Equal(diff.Old.Default.Raw, diff.New.Default.Raw): + err = fmt.Errorf("default value changed from %q to %q", string(diff.Old.Default.Raw), string(diff.New.Default.Raw)) + } + + return isHandled(diff, reset), err +} + +func Type(diff kappcus.FieldDiff) (bool, error) { + reset := func(diff kappcus.FieldDiff) kappcus.FieldDiff { + diff.Old.Type = "" + diff.New.Type = "" + return diff + } + + var err error + if diff.Old.Type != diff.New.Type { + err = fmt.Errorf("type changed from %q to %q", diff.Old.Type, diff.New.Type) + } + + return isHandled(diff, reset), err +} diff --git a/internal/rukpak/preflights/crdupgradesafety/checks_test.go b/internal/rukpak/preflights/crdupgradesafety/checks_test.go new file mode 100644 index 000000000..6544006ce --- /dev/null +++ b/internal/rukpak/preflights/crdupgradesafety/checks_test.go @@ -0,0 +1,907 @@ +package crdupgradesafety + +import ( + "errors" + "testing" + + kappcus "carvel.dev/kapp/pkg/kapp/crdupgradesafety" + "github.com/stretchr/testify/require" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/utils/ptr" +) + +type testcase struct { + name string + diff kappcus.FieldDiff + err error + handled bool +} + +func TestEnum(t *testing.T) { + for _, tc := range []testcase{ + { + name: "no diff, no error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + Enum: []apiextensionsv1.JSON{ + { + Raw: []byte("foo"), + }, + }, + }, + New: &apiextensionsv1.JSONSchemaProps{ + Enum: []apiextensionsv1.JSON{ + { + Raw: []byte("foo"), + }, + }, + }, + }, + err: nil, + handled: true, + }, + { + name: "new enum constraint, error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + Enum: []apiextensionsv1.JSON{}, + }, + New: &apiextensionsv1.JSONSchemaProps{ + Enum: []apiextensionsv1.JSON{ + { + Raw: []byte("foo"), + }, + }, + }, + }, + err: errors.New("enum constraints [foo] added when there were no restrictions previously"), + handled: true, + }, + { + name: "remove enum value, error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + Enum: []apiextensionsv1.JSON{ + { + Raw: []byte("foo"), + }, + { + Raw: []byte("bar"), + }, + }, + }, + New: &apiextensionsv1.JSONSchemaProps{ + Enum: []apiextensionsv1.JSON{ + { + Raw: []byte("bar"), + }, + }, + }, + }, + err: errors.New("enums [foo] removed from the set of previously allowed values"), + handled: true, + }, + { + name: "different field changed, no error, not handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + ID: "foo", + }, + New: &apiextensionsv1.JSONSchemaProps{ + ID: "bar", + }, + }, + err: nil, + handled: false, + }, + { + name: "different field changed with enum, no error, not handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + ID: "foo", + Enum: []apiextensionsv1.JSON{ + { + Raw: []byte("foo"), + }, + }, + }, + New: &apiextensionsv1.JSONSchemaProps{ + ID: "bar", + Enum: []apiextensionsv1.JSON{ + { + Raw: []byte("foo"), + }, + }, + }, + }, + err: nil, + handled: false, + }, + } { + t.Run(tc.name, func(t *testing.T) { + handled, err := Enum(tc.diff) + require.Equal(t, tc.err, err) + require.Equal(t, tc.handled, handled) + }) + } +} + +func TestRequired(t *testing.T) { + for _, tc := range []testcase{ + { + name: "no diff, no error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + Required: []string{ + "foo", + }, + }, + New: &apiextensionsv1.JSONSchemaProps{ + Required: []string{ + "foo", + }, + }, + }, + err: nil, + handled: true, + }, + { + name: "new required field, error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{}, + New: &apiextensionsv1.JSONSchemaProps{ + Required: []string{ + "foo", + }, + }, + }, + err: errors.New("new required fields [foo] added"), + handled: true, + }, + { + name: "different field changed, no error, not handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + ID: "foo", + }, + New: &apiextensionsv1.JSONSchemaProps{ + ID: "bar", + }, + }, + err: nil, + handled: false, + }, + } { + t.Run(tc.name, func(t *testing.T) { + handled, err := Required(tc.diff) + require.Equal(t, tc.err, err) + require.Equal(t, tc.handled, handled) + }) + } +} + +func TestMaximum(t *testing.T) { + for _, tc := range []testcase{ + { + name: "no diff, no error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + Maximum: ptr.To(10.0), + }, + New: &apiextensionsv1.JSONSchemaProps{ + Maximum: ptr.To(10.0), + }, + }, + err: nil, + handled: true, + }, + { + name: "new maximum constraint, error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{}, + New: &apiextensionsv1.JSONSchemaProps{ + Maximum: ptr.To(10.0), + }, + }, + err: errors.New("maximum: constraint 10 added when there were no restrictions previously"), + handled: true, + }, + { + name: "maximum constraint decreased, error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + Maximum: ptr.To(20.0), + }, + New: &apiextensionsv1.JSONSchemaProps{ + Maximum: ptr.To(10.0), + }, + }, + err: errors.New("maximum: constraint decreased from 20 to 10"), + handled: true, + }, + { + name: "maximum constraint increased, no error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + Maximum: ptr.To(20.0), + }, + New: &apiextensionsv1.JSONSchemaProps{ + Maximum: ptr.To(30.0), + }, + }, + err: nil, + handled: true, + }, + { + name: "different field changed, no error, not handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + ID: "foo", + }, + New: &apiextensionsv1.JSONSchemaProps{ + ID: "bar", + }, + }, + err: nil, + handled: false, + }, + } { + t.Run(tc.name, func(t *testing.T) { + handled, err := Maximum(tc.diff) + require.Equal(t, tc.err, err) + require.Equal(t, tc.handled, handled) + }) + } +} + +func TestMaxItems(t *testing.T) { + for _, tc := range []testcase{ + { + name: "no diff, no error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + MaxItems: ptr.To(int64(10)), + }, + New: &apiextensionsv1.JSONSchemaProps{ + MaxItems: ptr.To(int64(10)), + }, + }, + err: nil, + handled: true, + }, + { + name: "new maxItems constraint, error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{}, + New: &apiextensionsv1.JSONSchemaProps{ + MaxItems: ptr.To(int64(10)), + }, + }, + err: errors.New("maxItems: constraint 10 added when there were no restrictions previously"), + handled: true, + }, + { + name: "maxItems constraint decreased, error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + MaxItems: ptr.To(int64(20)), + }, + New: &apiextensionsv1.JSONSchemaProps{ + MaxItems: ptr.To(int64(10)), + }, + }, + err: errors.New("maxItems: constraint decreased from 20 to 10"), + handled: true, + }, + { + name: "maxitems constraint increased, no error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + MaxItems: ptr.To(int64(10)), + }, + New: &apiextensionsv1.JSONSchemaProps{ + MaxItems: ptr.To(int64(20)), + }, + }, + err: nil, + handled: true, + }, + { + name: "different field changed, no error, not handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + ID: "foo", + }, + New: &apiextensionsv1.JSONSchemaProps{ + ID: "bar", + }, + }, + err: nil, + handled: false, + }, + } { + t.Run(tc.name, func(t *testing.T) { + handled, err := MaxItems(tc.diff) + require.Equal(t, tc.err, err) + require.Equal(t, tc.handled, handled) + }) + } +} + +func TestMaxLength(t *testing.T) { + for _, tc := range []testcase{ + { + name: "no diff, no error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + MaxLength: ptr.To(int64(10)), + }, + New: &apiextensionsv1.JSONSchemaProps{ + MaxLength: ptr.To(int64(10)), + }, + }, + err: nil, + handled: true, + }, + { + name: "new maxLength constraint, error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{}, + New: &apiextensionsv1.JSONSchemaProps{ + MaxLength: ptr.To(int64(10)), + }, + }, + err: errors.New("maxLength: constraint 10 added when there were no restrictions previously"), + handled: true, + }, + { + name: "maxLength constraint decreased, error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + MaxLength: ptr.To(int64(20)), + }, + New: &apiextensionsv1.JSONSchemaProps{ + MaxLength: ptr.To(int64(10)), + }, + }, + err: errors.New("maxLength: constraint decreased from 20 to 10"), + handled: true, + }, + { + name: "maxLength constraint increased, no error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + MaxLength: ptr.To(int64(10)), + }, + New: &apiextensionsv1.JSONSchemaProps{ + MaxLength: ptr.To(int64(20)), + }, + }, + err: nil, + handled: true, + }, + { + name: "different field changed, no error, not handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + ID: "foo", + }, + New: &apiextensionsv1.JSONSchemaProps{ + ID: "bar", + }, + }, + err: nil, + handled: false, + }, + } { + t.Run(tc.name, func(t *testing.T) { + handled, err := MaxLength(tc.diff) + require.Equal(t, tc.err, err) + require.Equal(t, tc.handled, handled) + }) + } +} + +func TestMaxProperties(t *testing.T) { + for _, tc := range []testcase{ + { + name: "no diff, no error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + MaxProperties: ptr.To(int64(10)), + }, + New: &apiextensionsv1.JSONSchemaProps{ + MaxProperties: ptr.To(int64(10)), + }, + }, + err: nil, + handled: true, + }, + { + name: "new maxProperties constraint, error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{}, + New: &apiextensionsv1.JSONSchemaProps{ + MaxProperties: ptr.To(int64(10)), + }, + }, + err: errors.New("maxProperties: constraint 10 added when there were no restrictions previously"), + handled: true, + }, + { + name: "maxProperties constraint decreased, error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + MaxProperties: ptr.To(int64(20)), + }, + New: &apiextensionsv1.JSONSchemaProps{ + MaxProperties: ptr.To(int64(10)), + }, + }, + err: errors.New("maxProperties: constraint decreased from 20 to 10"), + handled: true, + }, + { + name: "maxProperties constraint increased, no error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + MaxProperties: ptr.To(int64(10)), + }, + New: &apiextensionsv1.JSONSchemaProps{ + MaxProperties: ptr.To(int64(20)), + }, + }, + err: nil, + handled: true, + }, + { + name: "different field changed, no error, not handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + ID: "foo", + }, + New: &apiextensionsv1.JSONSchemaProps{ + ID: "bar", + }, + }, + err: nil, + handled: false, + }, + } { + t.Run(tc.name, func(t *testing.T) { + handled, err := MaxProperties(tc.diff) + require.Equal(t, tc.err, err) + require.Equal(t, tc.handled, handled) + }) + } +} + +func TestMinItems(t *testing.T) { + for _, tc := range []testcase{ + { + name: "no diff, no error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + MinItems: ptr.To(int64(10)), + }, + New: &apiextensionsv1.JSONSchemaProps{ + MinItems: ptr.To(int64(10)), + }, + }, + err: nil, + handled: true, + }, + { + name: "new minItems constraint, error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{}, + New: &apiextensionsv1.JSONSchemaProps{ + MinItems: ptr.To(int64(10)), + }, + }, + err: errors.New("minItems: constraint 10 added when there were no restrictions previously"), + handled: true, + }, + { + name: "minItems constraint decreased, no error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + MinItems: ptr.To(int64(20)), + }, + New: &apiextensionsv1.JSONSchemaProps{ + MinItems: ptr.To(int64(10)), + }, + }, + err: nil, + handled: true, + }, + { + name: "minItems constraint increased, error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + MinItems: ptr.To(int64(10)), + }, + New: &apiextensionsv1.JSONSchemaProps{ + MinItems: ptr.To(int64(20)), + }, + }, + err: errors.New("minItems: constraint increased from 10 to 20"), + handled: true, + }, + { + name: "different field changed, no error, not handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + ID: "foo", + }, + New: &apiextensionsv1.JSONSchemaProps{ + ID: "bar", + }, + }, + err: nil, + handled: false, + }, + } { + t.Run(tc.name, func(t *testing.T) { + handled, err := MinItems(tc.diff) + require.Equal(t, tc.err, err) + require.Equal(t, tc.handled, handled) + }) + } +} + +func TestMinimum(t *testing.T) { + for _, tc := range []testcase{ + { + name: "no diff, no error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + Minimum: ptr.To(10.0), + }, + New: &apiextensionsv1.JSONSchemaProps{ + Minimum: ptr.To(10.0), + }, + }, + err: nil, + handled: true, + }, + { + name: "new minimum constraint, error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{}, + New: &apiextensionsv1.JSONSchemaProps{ + Minimum: ptr.To(10.0), + }, + }, + err: errors.New("minimum: constraint 10 added when there were no restrictions previously"), + handled: true, + }, + { + name: "minLength constraint decreased, no error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + Minimum: ptr.To(20.0), + }, + New: &apiextensionsv1.JSONSchemaProps{ + Minimum: ptr.To(10.0), + }, + }, + err: nil, + handled: true, + }, + { + name: "minLength constraint increased, error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + Minimum: ptr.To(10.0), + }, + New: &apiextensionsv1.JSONSchemaProps{ + Minimum: ptr.To(20.0), + }, + }, + err: errors.New("minimum: constraint increased from 10 to 20"), + handled: true, + }, + { + name: "different field changed, no error, not handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + ID: "foo", + }, + New: &apiextensionsv1.JSONSchemaProps{ + ID: "bar", + }, + }, + err: nil, + handled: false, + }, + } { + t.Run(tc.name, func(t *testing.T) { + handled, err := Minimum(tc.diff) + require.Equal(t, tc.err, err) + require.Equal(t, tc.handled, handled) + }) + } +} + +func TestMinLength(t *testing.T) { + for _, tc := range []testcase{ + { + name: "no diff, no error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + MinLength: ptr.To(int64(10)), + }, + New: &apiextensionsv1.JSONSchemaProps{ + MinLength: ptr.To(int64(10)), + }, + }, + err: nil, + handled: true, + }, + { + name: "new minLength constraint, error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{}, + New: &apiextensionsv1.JSONSchemaProps{ + MinLength: ptr.To(int64(10)), + }, + }, + err: errors.New("minLength: constraint 10 added when there were no restrictions previously"), + handled: true, + }, + { + name: "minLength constraint decreased, no error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + MinLength: ptr.To(int64(20)), + }, + New: &apiextensionsv1.JSONSchemaProps{ + MinLength: ptr.To(int64(10)), + }, + }, + err: nil, + handled: true, + }, + { + name: "minLength constraint increased, error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + MinLength: ptr.To(int64(10)), + }, + New: &apiextensionsv1.JSONSchemaProps{ + MinLength: ptr.To(int64(20)), + }, + }, + err: errors.New("minLength: constraint increased from 10 to 20"), + handled: true, + }, + { + name: "different field changed, no error, not handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + ID: "foo", + }, + New: &apiextensionsv1.JSONSchemaProps{ + ID: "bar", + }, + }, + err: nil, + handled: false, + }, + } { + t.Run(tc.name, func(t *testing.T) { + handled, err := MinLength(tc.diff) + require.Equal(t, tc.err, err) + require.Equal(t, tc.handled, handled) + }) + } +} + +func TestMinProperties(t *testing.T) { + for _, tc := range []testcase{ + { + name: "no diff, no error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + MinProperties: ptr.To(int64(10)), + }, + New: &apiextensionsv1.JSONSchemaProps{ + MinProperties: ptr.To(int64(10)), + }, + }, + err: nil, + handled: true, + }, + { + name: "new minProperties constraint, error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{}, + New: &apiextensionsv1.JSONSchemaProps{ + MinProperties: ptr.To(int64(10)), + }, + }, + err: errors.New("minProperties: constraint 10 added when there were no restrictions previously"), + handled: true, + }, + { + name: "minProperties constraint decreased, no error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + MinProperties: ptr.To(int64(20)), + }, + New: &apiextensionsv1.JSONSchemaProps{ + MinProperties: ptr.To(int64(10)), + }, + }, + err: nil, + handled: true, + }, + { + name: "minProperties constraint increased, error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + MinProperties: ptr.To(int64(10)), + }, + New: &apiextensionsv1.JSONSchemaProps{ + MinProperties: ptr.To(int64(20)), + }, + }, + err: errors.New("minProperties: constraint increased from 10 to 20"), + handled: true, + }, + { + name: "different field changed, no error, not handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + ID: "foo", + }, + New: &apiextensionsv1.JSONSchemaProps{ + ID: "bar", + }, + }, + err: nil, + handled: false, + }, + } { + t.Run(tc.name, func(t *testing.T) { + handled, err := MinProperties(tc.diff) + require.Equal(t, tc.err, err) + require.Equal(t, tc.handled, handled) + }) + } +} + +func TestDefault(t *testing.T) { + for _, tc := range []testcase{ + { + name: "no diff, no error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + Default: &apiextensionsv1.JSON{ + Raw: []byte("foo"), + }, + }, + New: &apiextensionsv1.JSONSchemaProps{ + Default: &apiextensionsv1.JSON{ + Raw: []byte("foo"), + }, + }, + }, + err: nil, + handled: true, + }, + { + name: "new default value, error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{}, + New: &apiextensionsv1.JSONSchemaProps{ + Default: &apiextensionsv1.JSON{ + Raw: []byte("foo"), + }, + }, + }, + err: errors.New("default value \"foo\" added when there was no default previously"), + handled: true, + }, + { + name: "default value removed, error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + Default: &apiextensionsv1.JSON{ + Raw: []byte("foo"), + }, + }, + New: &apiextensionsv1.JSONSchemaProps{}, + }, + err: errors.New("default value \"foo\" removed"), + handled: true, + }, + { + name: "default value changed, error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + Default: &apiextensionsv1.JSON{ + Raw: []byte("foo"), + }, + }, + New: &apiextensionsv1.JSONSchemaProps{ + Default: &apiextensionsv1.JSON{ + Raw: []byte("bar"), + }, + }, + }, + err: errors.New("default value changed from \"foo\" to \"bar\""), + handled: true, + }, + { + name: "different field changed, no error, not handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + ID: "foo", + }, + New: &apiextensionsv1.JSONSchemaProps{ + ID: "bar", + }, + }, + err: nil, + handled: false, + }, + } { + t.Run(tc.name, func(t *testing.T) { + handled, err := Default(tc.diff) + require.Equal(t, tc.err, err) + require.Equal(t, tc.handled, handled) + }) + } +} + +func TestType(t *testing.T) { + for _, tc := range []testcase{ + { + name: "no diff, no error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + Type: "string", + }, + New: &apiextensionsv1.JSONSchemaProps{ + Type: "string", + }, + }, + err: nil, + handled: true, + }, + { + name: "type changed, error, handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + Type: "string", + }, + New: &apiextensionsv1.JSONSchemaProps{ + Type: "integer", + }, + }, + err: errors.New("type changed from \"string\" to \"integer\""), + handled: true, + }, + { + name: "different field changed, no error, not handled", + diff: kappcus.FieldDiff{ + Old: &apiextensionsv1.JSONSchemaProps{ + ID: "foo", + }, + New: &apiextensionsv1.JSONSchemaProps{ + ID: "bar", + }, + }, + err: nil, + handled: false, + }, + } { + t.Run(tc.name, func(t *testing.T) { + handled, err := Type(tc.diff) + require.Equal(t, tc.err, err) + require.Equal(t, tc.handled, handled) + }) + } +} diff --git a/internal/rukpak/preflights/crdupgradesafety/crdupgradesafety.go b/internal/rukpak/preflights/crdupgradesafety/crdupgradesafety.go index 3f91c8c2b..7cdd905f6 100644 --- a/internal/rukpak/preflights/crdupgradesafety/crdupgradesafety.go +++ b/internal/rukpak/preflights/crdupgradesafety/crdupgradesafety.go @@ -32,17 +32,18 @@ type Preflight struct { func NewPreflight(crdCli apiextensionsv1client.CustomResourceDefinitionInterface, opts ...Option) *Preflight { changeValidations := []kappcus.ChangeValidation{ - kappcus.EnumChangeValidation, - kappcus.RequiredFieldChangeValidation, - kappcus.MaximumChangeValidation, - kappcus.MaximumItemsChangeValidation, - kappcus.MaximumLengthChangeValidation, - kappcus.MaximumPropertiesChangeValidation, - kappcus.MinimumChangeValidation, - kappcus.MinimumItemsChangeValidation, - kappcus.MinimumLengthChangeValidation, - kappcus.MinimumPropertiesChangeValidation, - kappcus.DefaultValueChangeValidation, + Enum, + Required, + Maximum, + MaxItems, + MaxLength, + MaxProperties, + Minimum, + MinItems, + MinLength, + MinProperties, + Default, + Type, } p := &Preflight{ crdClient: crdCli, diff --git a/internal/rukpak/preflights/crdupgradesafety/crdupgradesafety_test.go b/internal/rukpak/preflights/crdupgradesafety/crdupgradesafety_test.go index 39e0a0fe9..98b2289bd 100644 --- a/internal/rukpak/preflights/crdupgradesafety/crdupgradesafety_test.go +++ b/internal/rukpak/preflights/crdupgradesafety/crdupgradesafety_test.go @@ -166,17 +166,17 @@ func TestInstall(t *testing.T) { wantErrMsgs: []string{ `"NoScopeChange"`, `"NoStoredVersionRemoved"`, - `enums added`, - `new required fields added`, - `maximum constraint added when one did not exist previously`, - `maximum items constraint added`, - `maximum length constraint added`, - `maximum properties constraint added`, - `minimum constraint added when one did not exist previously`, - `minimum items constraint added`, - `minimum length constraint added`, - `minimum properties constraint added`, - `new value added as default`, + `enum constraints`, + `new required fields`, + `maximum: constraint`, + `maxItems: constraint`, + `maxLength: constraint`, + `maxProperties: constraint`, + `minimum: constraint`, + `minItems: constraint`, + `minLength: constraint`, + `minProperties: constraint`, + `default value`, }, }, { @@ -303,17 +303,17 @@ func TestUpgrade(t *testing.T) { wantErrMsgs: []string{ `"NoScopeChange"`, `"NoStoredVersionRemoved"`, - `enums added`, - `new required fields added`, - `maximum constraint added when one did not exist previously`, - `maximum items constraint added`, - `maximum length constraint added`, - `maximum properties constraint added`, - `minimum constraint added when one did not exist previously`, - `minimum items constraint added`, - `minimum length constraint added`, - `minimum properties constraint added`, - `new value added as default`, + `enum constraints`, + `new required fields`, + `maximum: constraint`, + `maxItems: constraint`, + `maxLength: constraint`, + `maxProperties: constraint`, + `minimum: constraint`, + `minItems: constraint`, + `minLength: constraint`, + `minProperties: constraint`, + `default value`, }, }, { @@ -345,7 +345,7 @@ func TestUpgrade(t *testing.T) { Manifest: getManifestString(t, "crd-conversion-no-webhook.json"), }, wantErrMsgs: []string{ - `"ServedVersionValidator" validation failed: version upgrade "v1" to "v2", field "^.spec.foobarbaz": enum values removed`, + `"ServedVersionValidator" validation failed: version upgrade "v1" to "v2", field "^.spec.foobarbaz": enums`, }, }, }