Skip to content

Commit

Permalink
address review comments
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 3, 2024
1 parent 2a1a52e commit 0acdf73
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 39 deletions.
43 changes: 4 additions & 39 deletions pkg/kapp/crdupgradesafety/change_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,17 +252,9 @@ func MinimumPropertiesChangeValidation(diff FieldDiff) (bool, error) {
// - An error if either of the above criteria are not met
func MaximumChangeValidation(diff FieldDiff) (bool, error) {
handled := func() bool {
tmpOldMaximum := diff.Old.Maximum
tmpNewMaximum := diff.New.Maximum

diff.Old.Maximum = nil
diff.New.Maximum = nil
result := reflect.DeepEqual(diff.Old, diff.New)

// reset diff.Old.Maximum and diff.New.Maximum to original values
diff.Old.Maximum = tmpOldMaximum
diff.New.Maximum = tmpNewMaximum
return result
return reflect.DeepEqual(diff.Old, diff.New)
}

switch {
Expand Down Expand Up @@ -292,18 +284,9 @@ func MaximumChangeValidation(diff FieldDiff) (bool, error) {
// - An error if either of the above criteria are not met
func MaximumLengthChangeValidation(diff FieldDiff) (bool, error) {
handled := func() bool {
tmpOldMaxLength := diff.Old.MaxLength
tmpNewMaxLength := diff.New.MaxLength

diff.Old.MaxLength = nil
diff.New.MaxLength = nil
result := reflect.DeepEqual(diff.Old, diff.New)

// reset diff.Old.MaxLength and diff.New.MaxLength to original values
diff.Old.MaxLength = tmpOldMaxLength
diff.New.MaxLength = tmpNewMaxLength

return result
return reflect.DeepEqual(diff.Old, diff.New)
}

switch {
Expand Down Expand Up @@ -333,18 +316,9 @@ func MaximumLengthChangeValidation(diff FieldDiff) (bool, error) {
// - An error if either of the above criteria are not met
func MaximumItemsChangeValidation(diff FieldDiff) (bool, error) {
handled := func() bool {
tmpOldMaxItems := diff.Old.MaxItems
tmpNewMaxItems := diff.New.MaxItems

diff.Old.MaxItems = nil
diff.New.MaxItems = nil
result := reflect.DeepEqual(diff.Old, diff.New)

// reset diff.Old.MaxItems and diff.New.MaxItems to original values
diff.Old.MaxItems = tmpOldMaxItems
diff.New.MaxItems = tmpNewMaxItems

return result
return reflect.DeepEqual(diff.Old, diff.New)
}

switch {
Expand Down Expand Up @@ -374,18 +348,9 @@ func MaximumItemsChangeValidation(diff FieldDiff) (bool, error) {
// - An error if either of the above criteria are not met
func MaximumPropertiesChangeValidation(diff FieldDiff) (bool, error) {
handled := func() bool {
tmpOldMaxProperties := diff.Old.MaxProperties
tmpNewMaxProperties := diff.New.MaxProperties

diff.Old.MaxProperties = nil
diff.New.MaxProperties = nil
result := reflect.DeepEqual(diff.Old, diff.New)

// reset diff.Old.MaxProperties and diff.New.MaxProperties to original values
diff.Old.MaxProperties = tmpOldMaxProperties
diff.New.MaxProperties = tmpNewMaxProperties

return result
return reflect.DeepEqual(diff.Old, diff.New)
}

switch {
Expand Down
8 changes: 8 additions & 0 deletions pkg/kapp/crdupgradesafety/change_validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -949,6 +949,8 @@ func TestMaximumChangeValidation(t *testing.T) {
handled, err := crdupgradesafety.MaximumChangeValidation(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.Maximum)
assert.Empty(t, tc.diff.New.Maximum)
})
}
}
Expand Down Expand Up @@ -1036,6 +1038,8 @@ func TestMaximumLengthChangeValidation(t *testing.T) {
handled, err := crdupgradesafety.MaximumLengthChangeValidation(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.MaxLength)
assert.Empty(t, tc.diff.New.MaxLength)
})
}
}
Expand Down Expand Up @@ -1123,6 +1127,8 @@ func TestMaximumItemsChangeValidation(t *testing.T) {
handled, err := crdupgradesafety.MaximumItemsChangeValidation(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.MaxItems)
assert.Empty(t, tc.diff.New.MaxItems)
})
}
}
Expand Down Expand Up @@ -1210,6 +1216,8 @@ func TestMaximumPropertiesChangeValidation(t *testing.T) {
handled, err := crdupgradesafety.MaximumPropertiesChangeValidation(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.MaxProperties)
assert.Empty(t, tc.diff.New.MaxProperties)
})
}
}

0 comments on commit 0acdf73

Please sign in to comment.