Merge pull request #30226 from hashicorp/nf/dec21-derandomize-dependencies

Sort dependencies when encoding `ResourceInstanceObject`

internal/states/instance_object.go

@@ -1,6 +1,8 @@
 package states
 
 import (
+	"sort"
+
 	"github.com/zclconf/go-cty/cty"
 	ctyjson "github.com/zclconf/go-cty/cty/json"
 
@@ -108,6 +110,13 @@ func (o *ResourceInstanceObject) Encode(ty cty.Type, schemaVersion uint64) (*Res
 		return nil, err
 	}
 
+	// Dependencies are collected and merged in an unordered format (using map
+	// keys as a set), then later changed to a slice (in random ordering) to be
+	// stored in state as an array. To avoid pointless thrashing of state in
+	// refresh-only runs, we can either override comparison of dependency lists
+	// (more desirable, but tricky for Reasons) or just sort when encoding.
+	sort.Slice(o.Dependencies, func(i, j int) bool { return o.Dependencies[i].String() < o.Dependencies[j].String() })
+
 	return &ResourceInstanceObjectSrc{
 		SchemaVersion:       schemaVersion,
 		AttrsJSON:           src,

internal/states/instance_object_test.go

@@ -0,0 +1,50 @@
+package states
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/hashicorp/terraform/internal/addrs"
+	"github.com/zclconf/go-cty/cty"
+)
+
+func TestResourceInstanceObject_encode(t *testing.T) {
+	value := cty.ObjectVal(map[string]cty.Value{
+		"foo": cty.True,
+	})
+	// The in-memory order of resource dependencies is random, since they're an
+	// unordered set.
+	depsOne := []addrs.ConfigResource{
+		addrs.RootModule.Resource(addrs.ManagedResourceMode, "test", "honk"),
+		addrs.RootModule.Child("child").Resource(addrs.ManagedResourceMode, "test", "flub"),
+		addrs.RootModule.Resource(addrs.ManagedResourceMode, "test", "boop"),
+	}
+	depsTwo := []addrs.ConfigResource{
+		addrs.RootModule.Child("child").Resource(addrs.ManagedResourceMode, "test", "flub"),
+		addrs.RootModule.Resource(addrs.ManagedResourceMode, "test", "boop"),
+		addrs.RootModule.Resource(addrs.ManagedResourceMode, "test", "honk"),
+	}
+	rioOne := &ResourceInstanceObject{
+		Value:        value,
+		Status:       ObjectPlanned,
+		Dependencies: depsOne,
+	}
+	rioTwo := &ResourceInstanceObject{
+		Value:        value,
+		Status:       ObjectPlanned,
+		Dependencies: depsTwo,
+	}
+	riosOne, err := rioOne.Encode(value.Type(), 0)
+	if err != nil {
+		t.Fatalf("unexpected error: %s", err)
+	}
+	riosTwo, err := rioTwo.Encode(value.Type(), 0)
+	if err != nil {
+		t.Fatalf("unexpected error: %s", err)
+	}
+	// However, identical sets of dependencies should always be written to state
+	// in an identical order, so we don't do meaningless state updates on refresh.
+	if diff := cmp.Diff(riosOne.Dependencies, riosTwo.Dependencies); diff != "" {
+		t.Errorf("identical dependencies got encoded in different orders:\n%s", diff)
+	}
+}