Skip to content

Commit

Permalink
Fix a panic during diff
Browse files Browse the repository at this point in the history
  • Loading branch information
blampe committed May 1, 2024
1 parent 3ebc7ba commit b886cfb
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 8 deletions.
17 changes: 14 additions & 3 deletions provider/pkg/provider/diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@ import (
"k8s.io/apimachinery/pkg/runtime/schema"
)

type object = map[string]any
type list = []any
type expected = map[string]*pulumirpc.PropertyDiff
type (
object = map[string]any
list = []any
expected = map[string]*pulumirpc.PropertyDiff
)

func TestPatchToDiff(t *testing.T) {
var (
Expand Down Expand Up @@ -232,6 +234,15 @@ func TestPatchToDiff(t *testing.T) {
"spec.containers[0]": U,
},
},
{
name: "Removing a field that was nil should not panic (#1970).",
group: "tekton.dev", version: "v1beta1", kind: "Pipeline",
old: object{"taskSpec": object{"spec": nil, "steps": list{object{"name": "something"}}}},
new: object{"taskSpec": object{"steps": list{object{"name": "something-else"}}}},
expected: expected{
"taskSpec.steps[0].name": U,
},
},
}

for _, tt := range tests {
Expand Down
10 changes: 5 additions & 5 deletions provider/pkg/provider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -3017,12 +3017,12 @@ type patchConverter struct {
diff map[string]*pulumirpc.PropertyDiff
}

// addPatchValueToDiff adds the given patched value to the detailed diff. Either the patched value or the old value
// must not be nil.
// addPatchValueToDiff adds the given patched value to the detailed diff.
//
// The particular difference that is recorded depends on the old and new values:
// - If the patched value is nil, the property is recorded as deleted
// - If the old value is nil, the property is recorded as added
// - If the old and patched values are both nil, no diff is recorded.
// - If the types of the old and new values differ, the property is recorded as updated
// - If both values are maps, the maps are recursively compared on a per-property basis and added to the diff
// - If both values are arrays, the arrays are recursively compared on a per-element basis and added to the diff
Expand All @@ -3035,9 +3035,9 @@ type patchConverter struct {
func (pc *patchConverter) addPatchValueToDiff(
path []any, v, old, newInput, oldInput any, inArray bool,
) error {
contract.Assertf(v != nil || old != nil || oldInput != nil || newInput != nil,
"path: %+v | v: %+v | old: %+v | oldInput: %+v | newInput: %+v",
path, v, old, oldInput, newInput)
if v == nil && old == nil && oldInput == nil && newInput == nil {
return nil
}

// If there is no new input, then the only possible diff here is a delete. All other diffs must be diffs between
// old and new properties that are populated by the server. If there is also no old input, then there is no diff
Expand Down

0 comments on commit b886cfb

Please sign in to comment.