fix: diffing config fields with dots, whitespaces, and double quotes (#…

…911)
pulumi · Jul 19, 2023 · 87740ca · 87740ca
1 parent 739b0cd
commit 87740ca
Show file tree

Hide file tree

Showing 3 changed files with 96 additions and 6 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,8 @@ Bug fixes:
   [#865](https://github.com/pulumi/pulumi-google-native/issues/865)
 - Fix non-string property value conversion in URLs
   [#890](https://github.com/pulumi/pulumi-google-native/pull/890)
+- Fix issues when diffing config property keys with special characters
+  [#911](https://github.com/pulumi/pulumi-google-native/pull/911)
 
 ## v0.30.0 (2023-04-14)
 Upstream breaking changes:

diff --git a/provider/pkg/provider/diff.go b/provider/pkg/provider/diff.go
@@ -113,26 +113,49 @@ type differ struct {
 
 func (d *differ) calculateObjectDiff(diff *resource.ObjectDiff, diffBase, replaceBase string) map[string]*rpc.PropertyDiff {
 	detailedDiff := map[string]*rpc.PropertyDiff{}
+
+	// asResourcePath converts a given base and a property key to a wellformed resource path key.
+	asResourcePath := func(base string, p resource.PropertyKey) string {
+		s := string(p)
+
+		// If the field name contains characters that needs escaping then the property key should be presented
+		// in square brackets and quotes.
+		// Example:
+		// If base is `config.labels.` and the field name is `app.kubernetes.io/name` then the resource key will
+		// be `config.labels["app.kubernetes.io/name"]`
+		if strings.ContainsAny(s, `". `) {
+			// Remove the trailing dot for the base
+			base = strings.TrimSuffix(base, ".")
+
+			// Escape double quotes
+			s = strings.ReplaceAll(s, `"`, `\"`)
+
+			s = fmt.Sprintf(`["%s"]`, s)
+		}
+
+		return base + s
+	}
+
 	for k, v := range diff.Updates {
-		key := diffBase + string(k)
-		replaceKey := replaceBase + string(k)
+		key := asResourcePath(diffBase, k)
+		replaceKey := asResourcePath(replaceBase, k)
 		subDiff := d.calculateValueDiff(&v, key, replaceKey)
 		for sk, sv := range subDiff {
 			detailedDiff[sk] = sv
 		}
 	}
 	for k := range diff.Adds {
-		diffKey := diffBase + string(k)
-		replaceKey := replaceBase + string(k)
+		diffKey := asResourcePath(diffBase, k)
+		replaceKey := asResourcePath(replaceBase, k)
 		kind := rpc.PropertyDiff_ADD
 		if d.replaceKeys.Has(replaceKey) {
 			kind = rpc.PropertyDiff_ADD_REPLACE
 		}
 		detailedDiff[diffKey] = &rpc.PropertyDiff{Kind: kind}
 	}
 	for k := range diff.Deletes {
-		diffKey := diffBase + string(k)
-		replaceKey := replaceBase + string(k)
+		diffKey := asResourcePath(diffBase, k)
+		replaceKey := asResourcePath(replaceBase, k)
 		kind := rpc.PropertyDiff_DELETE
 		if d.replaceKeys.Has(replaceKey) {
 			kind = rpc.PropertyDiff_DELETE_REPLACE

diff --git a/provider/pkg/provider/diff_test.go b/provider/pkg/provider/diff_test.go
@@ -19,6 +19,7 @@ import (
 
 	"github.com/pulumi/pulumi-google-native/provider/pkg/resources"
 	"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
+	rpc "github.com/pulumi/pulumi/sdk/v3/proto/go"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -81,3 +82,67 @@ func TestApplyDiff(t *testing.T) {
 	}
 	assert.Equal(t, expected, actual)
 }
+
+func TestCalculateObjectDiff(t *testing.T) {
+	diff := &resource.ObjectDiff{
+		Adds: resource.PropertyMap{
+			"p3":            {V: "newkey"},
+			"dots.addition": {V: "added"},
+		},
+		Deletes: resource.PropertyMap{
+			"p2":            {V: "iamdeleted"},
+			"dots.deletion": {V: "deleted"},
+		},
+		Updates: map[resource.PropertyKey]resource.ValueDiff{
+			"p1": {
+				Old: resource.PropertyValue{V: "oldvalue"},
+				New: resource.PropertyValue{V: "newvalue"},
+			},
+			"dots.update": {
+				Old: resource.PropertyValue{V: "oldvalue"},
+				New: resource.PropertyValue{V: "newvalue"},
+			},
+			"nested": {
+				Object: &resource.ObjectDiff{
+					Updates: map[resource.PropertyKey]resource.ValueDiff{
+						"deep": {
+							Object: &resource.ObjectDiff{
+								Updates: map[resource.PropertyKey]resource.ValueDiff{
+									"nested.update/field": {
+										Old: resource.PropertyValue{V: "oldvalue"},
+										New: resource.PropertyValue{V: "newvalue"},
+									},
+									"whitespaced field": {
+										Old: resource.PropertyValue{V: "oldvalue"},
+										New: resource.PropertyValue{V: "newvalue"},
+									},
+									`"quotedfield"`: {
+										Old: resource.PropertyValue{V: "oldvalue"},
+										New: resource.PropertyValue{V: "newvalue"},
+									},
+									"regular": {
+										Old: resource.PropertyValue{V: "oldvalue"},
+										New: resource.PropertyValue{V: "newvalue"},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+	d := differ{}
+	actual := d.calculateObjectDiff(diff, "", "")
+
+	assert.Equal(t, rpc.PropertyDiff_ADD, actual["p3"].Kind)
+	assert.Equal(t, rpc.PropertyDiff_ADD, actual[`["dots.addition"]`].Kind)
+	assert.Equal(t, rpc.PropertyDiff_DELETE, actual["p2"].Kind)
+	assert.Equal(t, rpc.PropertyDiff_DELETE, actual[`["dots.deletion"]`].Kind)
+	assert.Equal(t, rpc.PropertyDiff_UPDATE, actual["p1"].Kind)
+	assert.Equal(t, rpc.PropertyDiff_UPDATE, actual[`["dots.update"]`].Kind)
+	assert.Equal(t, rpc.PropertyDiff_UPDATE, actual[`nested.deep["nested.update/field"]`].Kind)
+	assert.Equal(t, rpc.PropertyDiff_UPDATE, actual[`nested.deep["whitespaced field"]`].Kind)
+	assert.Equal(t, rpc.PropertyDiff_UPDATE, actual[`nested.deep["\"quotedfield\""]`].Kind)
+	assert.Equal(t, rpc.PropertyDiff_UPDATE, actual[`nested.deep.regular`].Kind)
+}