Skip to content

Commit

Permalink
Address review suggestions
Browse files Browse the repository at this point in the history
Signed-off-by: Rashmi Gottipati <chowdary.grashmi@gmail.com>
  • Loading branch information
rashmigottipati committed May 2, 2024
1 parent 258492f commit 2373d47
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 4 deletions.
13 changes: 11 additions & 2 deletions pkg/kapp/crdupgradesafety/change_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package crdupgradesafety

import (
"bytes"
"errors"
"fmt"
"reflect"
Expand Down Expand Up @@ -253,9 +254,17 @@ func MinimumPropertiesChangeValidation(diff FieldDiff) (bool, error) {
// - An error if either of the above criteria are not met
func DefaultValueChangeValidation(diff FieldDiff) (bool, error) {
handled := func() bool {
tmpOldDefault := diff.Old.Default
tmpNewDefault := diff.New.Default

diff.Old.Default = &v1.JSON{}
diff.New.Default = &v1.JSON{}
return reflect.DeepEqual(diff.Old, diff.New)
result := reflect.DeepEqual(diff.Old, diff.New)

// resetting diff.Old.Default and diff.New.Default to original values
diff.Old.Default = tmpOldDefault
diff.New.Default = tmpNewDefault
return result
}

switch {
Expand All @@ -266,7 +275,7 @@ func DefaultValueChangeValidation(diff FieldDiff) (bool, error) {
return handled(), fmt.Errorf("default value has been removed when previously a default value existed: %+v", diff.Old.Default.Raw)

case diff.Old.Default != nil && diff.New.Default != nil:
if !reflect.DeepEqual(diff.Old.Default.Raw, diff.New.Default.Raw) {
if !bytes.Equal(diff.Old.Default.Raw, diff.New.Default.Raw) {
return handled(), fmt.Errorf("default value has been changed from %+v to %+v", diff.Old.Default.Raw, diff.New.Default.Raw)
}
fallthrough
Expand Down
2 changes: 0 additions & 2 deletions pkg/kapp/crdupgradesafety/change_validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -954,8 +954,6 @@ func TestDefaultChangeValidation(t *testing.T) {
handled, err := crdupgradesafety.DefaultValueChangeValidation(tc.diff)
assert.Equal(t, tc.shouldError, err != nil, "should error? - %v", tc.shouldError)
assert.Equal(t, tc.shouldHandle, handled, "should be handled? - %v", tc.shouldHandle)
assert.Empty(t, tc.diff.Old.Default)
assert.Empty(t, tc.diff.New.Default)
})
}
}

0 comments on commit 2373d47

Please sign in to comment.