Merge pull request #37886 from hashicorp/jbardin/state-deep-equal

Implement equality for state objects

.changes/v1.15/BUG FIXES-20251110-120921.yaml

@@ -0,0 +1,5 @@
+kind: BUG FIXES
+body: A refresh-only plan could result in a non-zero exit code with no changes
+time: 2025-11-10T12:09:21.029489-05:00
+custom:
+    Issue: "37406"

internal/states/checks.go

@@ -4,6 +4,8 @@
 package states
 
 import (
+	"slices"
+
 	"github.com/hashicorp/terraform/internal/addrs"
 	"github.com/hashicorp/terraform/internal/checks"
 )
@@ -169,6 +171,45 @@ func (r *CheckResults) DeepCopy() *CheckResults {
 	return ret
 }
 
+func (r *CheckResults) Equal(other *CheckResults) bool {
+	if r == other {
+		return true
+	}
+	if r == nil || other == nil {
+		return false
+	}
+
+	if r.ConfigResults.Len() != other.ConfigResults.Len() {
+		return false
+	}
+	for key, elem := range r.ConfigResults.Iter() {
+		otherElem := other.ConfigResults.Get(key)
+		if otherElem == nil {
+			return false
+		}
+		if elem.Status != otherElem.Status {
+			return false
+		}
+		if elem.ObjectResults.Len() != otherElem.ObjectResults.Len() {
+			return false
+		}
+		for key, res := range elem.ObjectResults.Iter() {
+			otherRes := otherElem.ObjectResults.Get(key)
+			if otherRes == nil {
+				return false
+			}
+			if res.Status != otherRes.Status {
+				return false
+			}
+			if !slices.Equal(res.FailureMessages, otherRes.FailureMessages) {
+				return false
+			}
+		}
+	}
+
+	return true
+}
+
 // ObjectAddrsKnown determines whether the set of objects recorded in this
 // aggregate is accurate (true) or if it's incomplete as a result of the
 // run being interrupted before instance expansion.

internal/states/instance_object_src.go

@@ -4,7 +4,9 @@
 package states
 
 import (
+	"bytes"
 	"fmt"
+	"reflect"
 
 	"github.com/zclconf/go-cty/cty"
 	ctyjson "github.com/zclconf/go-cty/cty/json"
@@ -168,3 +170,48 @@ func (os *ResourceInstanceObjectSrc) CompleteIdentityUpgrade(newAttrs cty.Value,
 	new.IdentitySchemaVersion = uint64(schema.IdentityVersion)
 	return new, nil
 }
+
+// Equal compares two ResourceInstanceObjectSrc objects for equality, skipping
+// any internal fields which are not stored to the final serialized state.
+func (os *ResourceInstanceObjectSrc) Equal(other *ResourceInstanceObjectSrc) bool {
+	if os == other {
+		return true
+	}
+	if os == nil || other == nil {
+		return false
+	}
+
+	if os.SchemaVersion != other.SchemaVersion {
+		return false
+	}
+	if os.IdentitySchemaVersion != other.IdentitySchemaVersion {
+		return false
+	}
+	if os.Status != other.Status {
+		return false
+	}
+	if os.CreateBeforeDestroy != other.CreateBeforeDestroy {
+		return false
+	}
+
+	if !bytes.Equal(os.AttrsJSON, other.AttrsJSON) {
+		return false
+	}
+	if !bytes.Equal(os.IdentityJSON, other.IdentityJSON) {
+		return false
+	}
+	if !bytes.Equal(os.Private, other.Private) {
+		return false
+	}
+
+	// Compare legacy AttrsFlat maps. We shouldn't see this ever being used, but
+	// deal with in just in case until we remove it entirely. These are all
+	// simple maps of strings, so DeepEqual is perfectly fine here.
+	if !reflect.DeepEqual(os.AttrsFlat, other.AttrsFlat) {
+		return false
+	}
+
+	// We skip fields that have no functional impact on resource state.
+
+	return true
+}

internal/states/output_value.go

@@ -17,3 +17,19 @@ type OutputValue struct {
 	Value     cty.Value
 	Sensitive bool
 }
+
+func (o *OutputValue) Equal(other *OutputValue) bool {
+	if o == other {
+		return true
+	}
+	if o == nil || other == nil {
+		return false
+	}
+	if !o.Addr.Equal(other.Addr) {
+		return false
+	}
+	if o.Sensitive != other.Sensitive {
+		return false
+	}
+	return o.Value.RawEquals(other.Value)
+}

internal/states/resource.go

@@ -177,3 +177,64 @@ func NewDeposedKey() DeposedKey {
 func ParseDeposedKey(raw string) (DeposedKey, error) {
 	return addrs.ParseDeposedKey(raw)
 }
+
+// Equal compares Resource objects for equality, comparing all the current and
+// deposed instances.
+func (rs *Resource) Equal(other *Resource) bool {
+	if rs == other {
+		return true
+	}
+	if rs == nil || other == nil {
+		return false
+	}
+
+	if !rs.Addr.Equal(other.Addr) {
+		return false
+	}
+	if !rs.ProviderConfig.Equal(other.ProviderConfig) {
+		return false
+	}
+
+	if len(rs.Instances) != len(other.Instances) {
+		return false
+	}
+	for k, inst := range rs.Instances {
+		otherInst, ok := other.Instances[k]
+		if !ok {
+			return false
+		}
+		if !inst.Equal(otherInst) {
+			return false
+		}
+	}
+
+	return true
+}
+
+func (i *ResourceInstance) Equal(other *ResourceInstance) bool {
+	if i == other {
+		return true
+	}
+	if i == nil || other == nil {
+		return false
+	}
+
+	if !i.Current.Equal(other.Current) {
+		return false
+	}
+
+	if len(i.Deposed) != len(other.Deposed) {
+		return false
+	}
+	for k, dep := range i.Deposed {
+		otherObj, ok := other.Deposed[k]
+		if !ok {
+			return false
+		}
+		if !dep.Equal(otherObj) {
+			return false
+		}
+	}
+
+	return true
+}

internal/states/state_equal.go

@@ -3,12 +3,6 @@
 
 package states
 
-import (
-	"reflect"
-
-	"github.com/hashicorp/terraform/internal/addrs"
-)
-
 // Equal returns true if the receiver is functionally equivalent to other,
 // including any ephemeral portions of the state that would not be included
 // if the state were saved to files.
@@ -16,10 +10,22 @@ import (
 // To test only the persistent portions of two states for equality, instead
 // use statefile.StatesMarshalEqual.
 func (s *State) Equal(other *State) bool {
-	// For the moment this is sufficient, but we may need to do something
-	// more elaborate in future if we have any portions of state that require
-	// more sophisticated comparisons.
-	return reflect.DeepEqual(s, other)
+	if s == other {
+		return true
+	}
+	if s == nil || other == nil {
+		return false
+	}
+
+	if !s.RootOutputValuesEqual(other) {
+		return false
+	}
+
+	if !s.CheckResults.Equal(other.CheckResults) {
+		return false
+	}
+
+	return s.ManagedResourcesEqual(other)
 }
 
 // ManagedResourcesEqual returns true if all of the managed resources tracked
@@ -33,9 +39,14 @@ func (s *State) ManagedResourcesEqual(other *State) bool {
 	// First, some accommodations for situations where one of the objects is
 	// nil, for robustness since we sometimes use a nil state to represent
 	// a prior state being entirely absent.
-	if s == nil && other == nil {
+	if s == other {
+		// covers both states being nil, or both states being the exact same
+		// object.
 		return true
 	}
+
+	// Managed resources are technically equal if one state is nil while the
+	// other has no resources.
 	if s == nil {
 		return !other.HasManagedResourceInstanceObjects()
 	}
@@ -45,29 +56,37 @@ func (s *State) ManagedResourcesEqual(other *State) bool {
 
 	// If we get here then both states are non-nil.
 
-	// sameManagedResources tests that its second argument has all the
-	// resources that the first one does, so we'll call it twice with the
-	// arguments inverted to ensure that we'll also catch situations where
-	// the second has resources that the first does not.
-	return sameManagedResources(s, other) && sameManagedResources(other, s)
-}
+	if len(s.Modules) != len(other.Modules) {
+		return false
+	}
 
-func sameManagedResources(s1, s2 *State) bool {
-	for _, ms := range s1.Modules {
-		for _, rs := range ms.Resources {
-			addr := rs.Addr
-			if addr.Resource.Mode != addrs.ManagedResourceMode {
-				continue
+	for key, sMod := range s.Modules {
+		otherMod, ok := other.Modules[key]
+		if !ok {
+			return false
+		}
+		// Something else is wrong if the addresses don't match, but they are
+		// definitely not equal
+		if !sMod.Addr.Equal(otherMod.Addr) {
+			return false
+		}
+
+		if len(sMod.Resources) != len(otherMod.Resources) {
+			return false
+		}
+
+		for key, sRes := range sMod.Resources {
+			otherRes, ok := otherMod.Resources[key]
+			if !ok {
+				return false
 			}
-			otherRS := s2.Resource(addr)
-			if !reflect.DeepEqual(rs, otherRS) {
+			if !sRes.Equal(otherRes) {
 				return false
 			}
 		}
 	}
 
 	return true
-
 }
 
 // RootOutputValuesEqual returns true if the root output values tracked in the
@@ -77,25 +96,14 @@ func (s *State) RootOutputValuesEqual(s2 *State) bool {
 	if s == nil && s2 == nil {
 		return true
 	}
-	if s == nil {
-		return !s2.HasRootOutputValues()
-	}
-	if s2 == nil {
-		return !s.HasRootOutputValues()
-	}
 
-	return sameRootOutputValues(s, s2)
-}
-
-// sameRootOutputValues returns true if the two states have the same root output values.
-func sameRootOutputValues(s1, s2 *State) bool {
-	if len(s1.RootOutputValues) != len(s2.RootOutputValues) {
+	if len(s.RootOutputValues) != len(s2.RootOutputValues) {
 		return false
 	}
 
-	for k, v1 := range s1.RootOutputValues {
+	for k, v1 := range s2.RootOutputValues {
 		v2, ok := s2.RootOutputValues[k]
-		if !ok || !reflect.DeepEqual(v1, v2) {
+		if !ok || !v1.Equal(v2) {
 			return false
 		}
 	}

internal/states/state_test.go

@@ -203,6 +203,13 @@ func TestStateDeepCopy(t *testing.T) {
 			Private:             []byte("private data"),
 			Dependencies:        []addrs.ConfigResource{},
 			CreateBeforeDestroy: true,
+
+			// these may or may not be copied, but should not affect equality of
+			// the resources.
+			decodeValueCache: cty.ObjectVal(map[string]cty.Value{
+				"woozles": cty.StringVal("confuzles"),
+			}),
+			decodeIdentityCache: cty.DynamicVal,
 		},
 		addrs.AbsProviderConfig{
 			Provider: addrs.NewDefaultProvider("test"),
@@ -242,11 +249,34 @@ func TestStateDeepCopy(t *testing.T) {
 	)
 
 	state.EnsureModule(addrs.RootModuleInstance.Child("child", addrs.NoKey))
-
 	stateCopy := state.DeepCopy()
 	if !state.Equal(stateCopy) {
 		t.Fatalf("\nexpected:\n%q\ngot:\n%q\n", state, stateCopy)
 	}
+
+	// this is implied by the above, but has previously used a different
+	// codepath for comparison.
+	if !state.ManagedResourcesEqual(stateCopy) {
+		t.Fatalf("\nexpected managed resources to be equal:\n%q\ngot:\n%q\n", state, stateCopy)
+	}
+
+	// remove the cached values and ensure equality still holds
+	for _, mod := range stateCopy.Modules {
+		for _, res := range mod.Resources {
+			for _, inst := range res.Instances {
+				inst.Current.decodeValueCache = cty.NilVal
+				inst.Current.decodeIdentityCache = cty.NilVal
+			}
+		}
+	}
+
+	if !state.Equal(stateCopy) {
+		t.Fatalf("\nexpected:\n%q\ngot:\n%q\n", state, stateCopy)
+	}
+
+	if !state.ManagedResourcesEqual(stateCopy) {
+		t.Fatalf("\nexpected managed resources to be equal:\n%q\ngot:\n%q\n", state, stateCopy)
+	}
 }
 
 func TestStateHasResourceInstanceObjects(t *testing.T) {