Log the old and new attribute values of a detected diff

pkg/controller/external_tfpluginsdk.go

@@ -7,6 +7,9 @@ package controller
 import (
 	"context"
 	"fmt"
+	"maps"
+	"slices"
+	"strconv"
 	"strings"
 	"time"
 
@@ -534,7 +537,8 @@ func (n *terraformPluginSDKExternal) getResourceDataDiff(tr resource.Terraformed
 		instanceDiff.RawPlan = v
 	}
 	if instanceDiff != nil && !instanceDiff.Empty() {
-		n.logger.Debug("Diff detected", "instanceDiff", instanceDiff.GoString())
+		oldValues, newValues := instanceDiffValues(instanceDiff, n.config.TerraformResource.Schema)
+		n.logger.Debug("Diff detected", "old", oldValues, "new", newValues)
 		// Assumption: Source of truth when applying diffs, for instance on updates, is instanceDiff.Attributes.
 		// Setting instanceDiff.RawConfig has no effect on diff application.
 		instanceDiff.RawConfig = n.rawConfig
@@ -920,3 +924,111 @@ func (n *terraformPluginSDKExternal) fromInstanceStateToJSONMap(newState *tf.Ins
 	}
 	return stateValueMap, attrsAsCtyValue, nil
 }
+
+// instanceDiffValues returns the old and the planned attribute values of the
+// diff as two flat maps, so that the log output can be diffed. Values are
+// redacted when the attribute is sensitive in the schema, which also covers
+// the diffs the SDK does not flag itself: removed attributes and the elements
+// of sensitive lists and sets.
+func instanceDiffValues(d *tf.InstanceDiff, s map[string]*schema.Schema) (map[string]string, map[string]string) {
+	oldValues := make(map[string]string, len(d.Attributes))
+	newValues := make(map[string]string, len(d.Attributes))
+	labels := make(map[string]string)
+	for _, k := range slices.Sorted(maps.Keys(d.Attributes)) {
+		a := d.Attributes[k]
+		if a == nil {
+			continue
+		}
+		k, sensitive := sensitiveKey(s, k, labels)
+		if a.Sensitive || sensitive {
+			oldValues[k] = "<sensitive>"
+			if !a.NewRemoved {
+				newValues[k] = "<sensitive>"
+			}
+			continue
+		}
+		oldValues[k] = a.Old
+		switch {
+		case a.NewRemoved:
+		case a.NewComputed:
+			newValues[k] = "<computed>"
+		default:
+			newValues[k] = a.New
+		}
+	}
+	return oldValues, newValues
+}
+
+// sensitiveKey reports whether the flatmap attribute key k, or a collection
+// containing it, is marked sensitive in the schema, and returns k with the
+// element identifiers of sets that hold sensitive values replaced by opaque
+// labels, since the SDK derives those identifiers from a hash of the element.
+// labels keeps the label of each identifier across keys. The element count of
+// a collection is only sensitive when the collection itself is.
+func sensitiveKey(s map[string]*schema.Schema, k string, labels map[string]string) (string, bool) {
+	segments := strings.Split(k, ".")
+	sc, ok := s[segments[0]]
+	sensitive := false
+	for i := 1; ok; i++ {
+		sensitive = sensitive || sc.Sensitive
+		if i >= len(segments) || segments[i] == "#" || segments[i] == "%" {
+			break
+		}
+		if sc.Type == schema.TypeSet && hasSensitive(sc) {
+			segments[i] = setLabel(labels, strings.Join(segments[:i+1], "."))
+		}
+		sc, i, ok = elemSchema(sc, segments, i)
+	}
+	return strings.Join(segments, "."), sensitive
+}
+
+// setLabel returns the label of the set element at path, numbering the
+// elements of a set in the order they are first seen.
+func setLabel(labels map[string]string, path string) string {
+	if l, ok := labels[path]; ok {
+		return l
+	}
+	prefix := path[:strings.LastIndex(path, ".")+1]
+	n := 0
+	for p := range labels {
+		if strings.HasPrefix(p, prefix) && !strings.Contains(p[len(prefix):], ".") {
+			n++
+		}
+	}
+	labels[path] = "*" + strconv.Itoa(n)
+	return labels[path]
+}
+
+// elemSchema returns the schema addressed by the element identifier at
+// segments[i] and the index of the last segment it consumed.
+func elemSchema(sc *schema.Schema, segments []string, i int) (*schema.Schema, int, bool) {
+	switch e := sc.Elem.(type) {
+	case *schema.Resource:
+		if i+1 >= len(segments) {
+			return nil, i, false
+		}
+		field, ok := e.Schema[segments[i+1]]
+		return field, i + 1, ok
+	case *schema.Schema:
+		return e, i, true
+	default:
+		return nil, i, false
+	}
+}
+
+func hasSensitive(sc *schema.Schema) bool {
+	if sc.Sensitive {
+		return true
+	}
+	switch e := sc.Elem.(type) {
+	case *schema.Schema:
+		return hasSensitive(e)
+	case *schema.Resource:
+		for _, f := range e.Schema {
+			if hasSensitive(f) {
+				return true
+			}
+		}
+	}
+	return false
+}

pkg/controller/external_tfpluginsdk_test.go

@@ -602,3 +602,158 @@ func TestTerraformPluginSDKDelete(t *testing.T) {
 		})
 	}
 }
+
+func TestInstanceDiffValues(t *testing.T) {
+	type args struct {
+		diff   *tf.InstanceDiff
+		schema map[string]*schema.Schema
+	}
+	type want struct {
+		oldValues map[string]string
+		newValues map[string]string
+	}
+	cases := map[string]struct {
+		args
+		want
+	}{
+		"Changed": {
+			args: args{
+				diff: &tf.InstanceDiff{Attributes: map[string]*tf.ResourceAttrDiff{"name": {Old: "a", New: "b"}}},
+			},
+			want: want{
+				oldValues: map[string]string{"name": "a"},
+				newValues: map[string]string{"name": "b"},
+			},
+		},
+		"Added": {
+			args: args{
+				diff: &tf.InstanceDiff{Attributes: map[string]*tf.ResourceAttrDiff{"description": {Old: "", New: "added"}}},
+			},
+			want: want{
+				oldValues: map[string]string{"description": ""},
+				newValues: map[string]string{"description": "added"},
+			},
+		},
+		"Computed": {
+			args: args{
+				diff: &tf.InstanceDiff{Attributes: map[string]*tf.ResourceAttrDiff{"count": {Old: "1", NewComputed: true}}},
+			},
+			want: want{
+				oldValues: map[string]string{"count": "1"},
+				newValues: map[string]string{"count": "<computed>"},
+			},
+		},
+		"Removed": {
+			args: args{
+				diff: &tf.InstanceDiff{Attributes: map[string]*tf.ResourceAttrDiff{"tags.env": {Old: "dev", NewRemoved: true}}},
+			},
+			want: want{
+				oldValues: map[string]string{"tags.env": "dev"},
+				newValues: map[string]string{},
+			},
+		},
+		"Nil": {
+			args: args{
+				diff: &tf.InstanceDiff{Attributes: map[string]*tf.ResourceAttrDiff{"name": nil}},
+			},
+			want: want{
+				oldValues: map[string]string{},
+				newValues: map[string]string{},
+			},
+		},
+		"SensitiveFlag": {
+			args: args{
+				diff: &tf.InstanceDiff{Attributes: map[string]*tf.ResourceAttrDiff{"password": {Old: "x", New: "y", Sensitive: true}}},
+			},
+			want: want{
+				oldValues: map[string]string{"password": "<sensitive>"},
+				newValues: map[string]string{"password": "<sensitive>"},
+			},
+		},
+		"SensitiveRemoved": {
+			args: args{
+				diff:   &tf.InstanceDiff{Attributes: map[string]*tf.ResourceAttrDiff{"password": {Old: "x", NewRemoved: true}}},
+				schema: map[string]*schema.Schema{"password": {Type: schema.TypeString, Sensitive: true}},
+			},
+			want: want{
+				oldValues: map[string]string{"password": "<sensitive>"},
+				newValues: map[string]string{},
+			},
+		},
+		"SensitiveList": {
+			args: args{
+				diff: &tf.InstanceDiff{Attributes: map[string]*tf.ResourceAttrDiff{
+					"keys.#": {Old: "1", New: "2"},
+					"keys.0": {Old: "k1", New: "k1"},
+					"keys.1": {Old: "", New: "k2"},
+				}},
+				schema: map[string]*schema.Schema{"keys": {Type: schema.TypeList, Sensitive: true, Elem: &schema.Schema{Type: schema.TypeString}}},
+			},
+			want: want{
+				oldValues: map[string]string{"keys.#": "<sensitive>", "keys.0": "<sensitive>", "keys.1": "<sensitive>"},
+				newValues: map[string]string{"keys.#": "<sensitive>", "keys.0": "<sensitive>", "keys.1": "<sensitive>"},
+			},
+		},
+		"SensitiveSetElements": {
+			args: args{
+				diff: &tf.InstanceDiff{Attributes: map[string]*tf.ResourceAttrDiff{
+					"keys.#":    {Old: "1", New: "1"},
+					"keys.1234": {Old: "k1", NewRemoved: true},
+					"keys.5678": {Old: "", New: "k2"},
+				}},
+				schema: map[string]*schema.Schema{"keys": {Type: schema.TypeSet, Elem: &schema.Schema{Type: schema.TypeString, Sensitive: true}}},
+			},
+			want: want{
+				oldValues: map[string]string{"keys.#": "1", "keys.*0": "<sensitive>", "keys.*1": "<sensitive>"},
+				newValues: map[string]string{"keys.#": "1", "keys.*1": "<sensitive>"},
+			},
+		},
+		"SensitiveSetBlock": {
+			args: args{
+				diff: &tf.InstanceDiff{Attributes: map[string]*tf.ResourceAttrDiff{
+					"block.#":           {Old: "1", New: "1"},
+					"block.1234.name":   {Old: "a", NewRemoved: true},
+					"block.1234.secret": {Old: "x", NewRemoved: true},
+					"block.5678.name":   {Old: "", New: "b"},
+					"block.5678.secret": {Old: "", New: "y"},
+				}},
+				schema: map[string]*schema.Schema{"block": {Type: schema.TypeSet, Elem: &schema.Resource{Schema: map[string]*schema.Schema{
+					"name":   {Type: schema.TypeString},
+					"secret": {Type: schema.TypeString, Sensitive: true},
+				}}}},
+			},
+			want: want{
+				oldValues: map[string]string{"block.#": "1", "block.*0.name": "a", "block.*0.secret": "<sensitive>", "block.*1.name": "", "block.*1.secret": "<sensitive>"},
+				newValues: map[string]string{"block.#": "1", "block.*1.name": "b", "block.*1.secret": "<sensitive>"},
+			},
+		},
+		"SensitiveBlockField": {
+			args: args{
+				diff: &tf.InstanceDiff{Attributes: map[string]*tf.ResourceAttrDiff{
+					"block.#":        {Old: "1", New: "1"},
+					"block.0.name":   {Old: "a", New: "b"},
+					"block.0.secret": {Old: "x", New: "y"},
+				}},
+				schema: map[string]*schema.Schema{"block": {Type: schema.TypeList, Elem: &schema.Resource{Schema: map[string]*schema.Schema{
+					"name":   {Type: schema.TypeString},
+					"secret": {Type: schema.TypeString, Sensitive: true},
+				}}}},
+			},
+			want: want{
+				oldValues: map[string]string{"block.#": "1", "block.0.name": "a", "block.0.secret": "<sensitive>"},
+				newValues: map[string]string{"block.#": "1", "block.0.name": "b", "block.0.secret": "<sensitive>"},
+			},
+		},
+	}
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			oldValues, newValues := instanceDiffValues(tc.args.diff, tc.args.schema)
+			if diff := cmp.Diff(tc.want.oldValues, oldValues); diff != "" {
+				t.Errorf("\n%s\ninstanceDiffValues(...): -want old, +got old:\n", diff)
+			}
+			if diff := cmp.Diff(tc.want.newValues, newValues); diff != "" {
+				t.Errorf("\n%s\ninstanceDiffValues(...): -want new, +got new:\n", diff)
+			}
+		})
+	}
+}