maxmustermann/pulumi
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Fix rendering of diffs for resource without DetailedDiffs (#7500)

Browse source 
The diff rendering logic tests whether the DetailedDiff is nil to determine whether to use it for rendering or defer to older older supported approaches to computing diffs.

The new logic added in https://github.com/pulumi/pulumi/pull/7226 could lead to replacing a nil DetailedDiff with an empty DetailedDiff, whcih would make the rendering logic believe that a DetailedDiff was present but empty, instead of missing entirely.

This change ensures that we maintain nil-ness of the DetailedDiff when we transform it for handling of replaceOnChanges.

Also adds tests for this and other cases on applyReplaceOnChanges.

Fixes #7486.
This commit is contained in:
Luke Hoban 2021-07-13 07:39:48 -07:00 committed by GitHub
parent 343bc4c778
commit c3f5177f22
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 109 additions and 20 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
CHANGELOG.md
View file

@ -16,6 +16,9 @@ CHANGELOG
- [sdk/python] - Fix an issue where dependency keys were incorrectly translates to camelcase
[#7443](https://github.com/pulumi/pulumi/pull/7443)
- [cli] - Fix rendering of diffs for resource without DetailedDiffs
[#7500](https://github.com/pulumi/pulumi/pull/7500)
## 3.6.0 (2021-06-30)
### Improvements

45
pkg/resource/deploy/step_generator.go
View file

@ -527,7 +527,7 @@ func (sg *stepGenerator) generateStepsFromDiff(
hasInitErrors := len(old.InitErrors) > 0
// Update the diff to apply any replaceOnChanges annotations and to include initErrors in the diff.
diff, err = sg.applyReplaceOnChanges(diff, goal.ReplaceOnChanges, hasInitErrors)
diff, err = applyReplaceOnChanges(diff, goal.ReplaceOnChanges, hasInitErrors)
if err != nil {
return nil, result.FromError(err)
}
@ -1196,7 +1196,7 @@ const initErrorSpecialKey = "#initerror"
// applyReplaceOnChanges adjusts a DiffResult returned from a provider to apply the ReplaceOnChange
// settings in the desired state and init errors from the previous state.
func (sg *stepGenerator) applyReplaceOnChanges(diff plugin.DiffResult,
func applyReplaceOnChanges(diff plugin.DiffResult,
replaceOnChanges []string, hasInitErrors bool) (plugin.DiffResult, error) {
// No further work is necessary for DiffNone unless init errors are present.
@ -1214,23 +1214,26 @@ func (sg *stepGenerator) applyReplaceOnChanges(diff plugin.DiffResult,
}
// Calculate the new DetailedDiff
modifiedDiff := map[string]plugin.PropertyDiff{}
for p, v := range diff.DetailedDiff {
diffPath, err := resource.ParsePropertyPath(p)
if err != nil {
return diff, err
}
changeToReplace := false
for _, replaceOnChangePath := range replaceOnChangePaths {
if replaceOnChangePath.Contains(diffPath) {
changeToReplace = true
break
var modifiedDiff map[string]plugin.PropertyDiff
if diff.DetailedDiff != nil {
modifiedDiff = map[string]plugin.PropertyDiff{}
for p, v := range diff.DetailedDiff {
diffPath, err := resource.ParsePropertyPath(p)
if err != nil {
return diff, err
}
changeToReplace := false
for _, replaceOnChangePath := range replaceOnChangePaths {
if replaceOnChangePath.Contains(diffPath) {
changeToReplace = true
break
}
}
if changeToReplace {
v = v.ToReplace()
}
modifiedDiff[p] = v
}
if changeToReplace {
v = v.ToReplace()
}
modifiedDiff[p] = v
}
// Calculate the new ReplaceKeys
@ -1264,9 +1267,11 @@ func (sg *stepGenerator) applyReplaceOnChanges(diff plugin.DiffResult,
}
if replaceOnChangePath.Contains(initErrPath) {
modifiedReplaceKeys = append(modifiedReplaceKeys, initErrorSpecialKey)
modifiedDiff[initErrorSpecialKey] = plugin.PropertyDiff{
Kind: plugin.DiffUpdateReplace,
InputDiff: false,
if modifiedDiff != nil {
modifiedDiff[initErrorSpecialKey] = plugin.PropertyDiff{
Kind: plugin.DiffUpdateReplace,
InputDiff: false,
}
}
// If an init error is present on a path that causes replacement, then trigger a replacement.
modifiedChanges = plugin.DiffSome

81
pkg/resource/deploy/step_generator_test.go
View file

@ -4,6 +4,7 @@ import (
"testing"
"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
"github.com/pulumi/pulumi/sdk/v3/go/common/resource/plugin"
"github.com/stretchr/testify/assert"
)
@ -113,3 +114,83 @@ func TestIgnoreChanges(t *testing.T) {
})
}
}
func TestApplyReplaceOnChangesEmptyDetailedDiff(t *testing.T) {
cases := []struct {
name string
diff plugin.DiffResult
replaceOnChanges []string
hasInitErrors bool
expected plugin.DiffResult
}{
{
name: "Empty diff and replaceOnChanges",
diff: plugin.DiffResult{},
replaceOnChanges: []string{},
hasInitErrors: false,
expected: plugin.DiffResult{},
},
{
name: "DiffSome and empty replaceOnChanges",
diff: plugin.DiffResult{Changes: plugin.DiffSome, ChangedKeys: []resource.PropertyKey{"a"}},
replaceOnChanges: []string{},
hasInitErrors: false,
expected: plugin.DiffResult{Changes: plugin.DiffSome, ChangedKeys: []resource.PropertyKey{"a"}},
},
{
name: "DiffSome and non-empty replaceOnChanges",
diff: plugin.DiffResult{Changes: plugin.DiffSome, ChangedKeys: []resource.PropertyKey{"a"}},
replaceOnChanges: []string{"a"},
hasInitErrors: false,
expected: plugin.DiffResult{
Changes: plugin.DiffSome,
ChangedKeys: []resource.PropertyKey{"a"},
ReplaceKeys: []resource.PropertyKey{"a"},
},
},
{
name: "Empty diff and replaceOnChanges w/ init errors",
diff: plugin.DiffResult{},
replaceOnChanges: []string{},
hasInitErrors: true,
expected: plugin.DiffResult{},
},
{
name: "DiffSome and empty replaceOnChanges w/ init errors",
diff: plugin.DiffResult{Changes: plugin.DiffSome, ChangedKeys: []resource.PropertyKey{"a"}},
replaceOnChanges: []string{},
hasInitErrors: true,
expected: plugin.DiffResult{Changes: plugin.DiffSome, ChangedKeys: []resource.PropertyKey{"a"}},
},
{
name: "DiffSome and non-empty replaceOnChanges w/ init errors",
diff: plugin.DiffResult{Changes: plugin.DiffSome, ChangedKeys: []resource.PropertyKey{"a"}},
replaceOnChanges: []string{"a"},
hasInitErrors: true,
expected: plugin.DiffResult{
Changes: plugin.DiffSome,
ChangedKeys: []resource.PropertyKey{"a"},
ReplaceKeys: []resource.PropertyKey{"a"},
},
},
{
name: "Empty diff and non-empty replaceOnChanges w/ init errors",
diff: plugin.DiffResult{},
replaceOnChanges: []string{"*"},
hasInitErrors: true,
expected: plugin.DiffResult{
Changes: plugin.DiffSome,
ReplaceKeys: []resource.PropertyKey{"#initerror"},
},
},
}
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
newdiff, err := applyReplaceOnChanges(c.diff, c.replaceOnChanges, c.hasInitErrors)
assert.NoError(t, err)
assert.Equal(t, c.expected, newdiff)
})
}
}