diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 1d1e98e54..3f7025dde 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -8,8 +8,8 @@ ### Bug Fixes -- [cli/engine] - Accuretly computes the fields changed when diffing with unhelpful providers. This - allows the replaceOnChanges feature to be respected for all providers. +- [cli/engine] - Accurately computes the fields changed when diffing with unhelpful providers. This + allows the `replaceOnChanges` feature to be respected for all providers. [#8488](https://github.com/pulumi/pulumi/pull/8488) - [codegen/go] - Respect default values in Pulumi object types. diff --git a/pkg/resource/deploy/step_generator.go b/pkg/resource/deploy/step_generator.go index a05421a29..1166c1ef4 100644 --- a/pkg/resource/deploy/step_generator.go +++ b/pkg/resource/deploy/step_generator.go @@ -1132,7 +1132,13 @@ func diffResource(urn resource.URN, id resource.ID, oldInputs, oldOutputs, return diff, err } if diff.Changes == plugin.DiffUnknown { - diff = plugin.NewDiffResultFromObjectDiff(oldInputs.Diff(newInputs), ignoreChanges...) + tmp := oldInputs.Diff(newInputs) + if tmp.AnyChanges() { + diff.Changes = plugin.DiffSome + diff.ChangedKeys = tmp.ChangedKeys() + } else { + diff.Changes = plugin.DiffNone + } } return diff, nil } diff --git a/pkg/resource/deploy/step_generator_test.go b/pkg/resource/deploy/step_generator_test.go index 34e4a8372..2c6aee3f0 100644 --- a/pkg/resource/deploy/step_generator_test.go +++ b/pkg/resource/deploy/step_generator_test.go @@ -3,6 +3,7 @@ package deploy import ( "testing" + "github.com/pulumi/pulumi/pkg/v3/resource/deploy/deploytest" "github.com/pulumi/pulumi/sdk/v3/go/common/resource" "github.com/pulumi/pulumi/sdk/v3/go/common/resource/plugin" "github.com/stretchr/testify/assert" @@ -194,3 +195,62 @@ func TestApplyReplaceOnChangesEmptyDetailedDiff(t *testing.T) { } } + +func TestEngineDiffResource(t *testing.T) { + cases := []struct { + name string + oldInputs, newInputs resource.PropertyMap + expected []resource.PropertyKey + }{ + { + name: "Empty diff", + oldInputs: resource.NewPropertyMapFromMap(map[string]interface{}{ + "val1": resource.NewPropertyValue(8), + "val2": resource.NewPropertyValue("hello"), + }), + newInputs: resource.NewPropertyMapFromMap(map[string]interface{}{ + "val1": resource.NewPropertyValue(8), + "val2": resource.NewPropertyValue("hello"), + }), + expected: nil, + }, + { + name: "All changes", + oldInputs: resource.NewPropertyMapFromMap(map[string]interface{}{ + "val0": resource.NewPropertyValue(3.14), + }), + newInputs: resource.NewPropertyMapFromMap(map[string]interface{}{ + "val1": resource.NewNumberProperty(42), + "val2": resource.NewPropertyValue("world"), + }), + expected: []resource.PropertyKey{"val0", "val1", "val2"}, + }, + { + name: "Some changes", + oldInputs: resource.NewPropertyMapFromMap(map[string]interface{}{ + "val1": resource.NewPropertyValue(42), + }), + newInputs: resource.NewPropertyMapFromMap(map[string]interface{}{ + "val1": resource.NewNumberProperty(42), + "val2": resource.NewPropertyValue("world"), + }), + expected: []resource.PropertyKey{"val2"}, + }, + } + urn := resource.URN("urn:pulumi:dev::website-and-lambda::aws:s3/bucket:Bucket::my-bucket") + id := resource.ID("someid") + var oldOutputs resource.PropertyMap + allowUnknowns := false + ignoreChanges := []string{} + provider := deploytest.Provider{} + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + diff, err := diffResource(urn, id, c.oldInputs, oldOutputs, c.newInputs, &provider, allowUnknowns, ignoreChanges) + t.Logf("diff.ChangedKeys = %v", diff.ChangedKeys) + t.Logf("diff.StableKeys = %v", diff.StableKeys) + t.Logf("diff.ReplaceKeys = %v", diff.ReplaceKeys) + assert.NoError(t, err) + assert.EqualValues(t, c.expected, diff.ChangedKeys) + }) + } +} diff --git a/sdk/go/common/resource/plugin/provider.go b/sdk/go/common/resource/plugin/provider.go index 72a481b91..5666e0c96 100644 --- a/sdk/go/common/resource/plugin/provider.go +++ b/sdk/go/common/resource/plugin/provider.go @@ -217,34 +217,6 @@ type DiffResult struct { DeleteBeforeReplace bool // if true, this resource must be deleted before recreating it. } -// Transforms a `ObjectDiff` into an equivalent `DiffResult`. Because ObjectDiff has no way of -// determining what should be changed vs replaced, all operations are added to the ChangedKeys set. -// -// Any key in ignoreChanged is treated as stable. -func NewDiffResultFromObjectDiff(obj *resource.ObjectDiff, ignoreChanged ...string) DiffResult { - result := DiffResult{} - if !obj.AnyChanges() { - result.Changes = DiffNone - return result - } - - ignoreSet := make(map[string]bool, len(ignoreChanged)) - for _, v := range ignoreChanged { - ignoreSet[v] = true - } - - result.Changes = DiffSome - for _, k := range obj.Keys() { - if !obj.Changed(k) || ignoreSet[string(k)] { - result.StableKeys = append(result.StableKeys, k) - } else { - result.ChangedKeys = append(result.ChangedKeys, k) - } - } - - return result -} - // Replace returns true if this diff represents a replacement. func (r DiffResult) Replace() bool { for _, v := range r.DetailedDiff { diff --git a/sdk/go/common/resource/properties_diff.go b/sdk/go/common/resource/properties_diff.go index c4a1dc369..2ae1196ce 100644 --- a/sdk/go/common/resource/properties_diff.go +++ b/sdk/go/common/resource/properties_diff.go @@ -80,6 +80,19 @@ func (diff *ObjectDiff) Keys() []PropertyKey { return ks } +// All keys where Changed(k) = true. +func (diff *ObjectDiff) ChangedKeys() []PropertyKey { + var ks []PropertyKey + if diff != nil { + for _, k := range diff.Keys() { + if diff.Changed(k) { + ks = append(ks, k) + } + } + } + return ks +} + // ValueDiff holds the results of diffing two property values. type ValueDiff struct { Old PropertyValue // the old value.