diff --git a/.changes/v1.15/BUG FIXES-20251110-120921.yaml b/.changes/v1.15/BUG FIXES-20251110-120921.yaml new file mode 100644 index 000000000000..cc5ae0920bb8 --- /dev/null +++ b/.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" diff --git a/internal/states/checks.go b/internal/states/checks.go index f5718e49f2a9..9d8fa9c6218b 100644 --- a/internal/states/checks.go +++ b/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. diff --git a/internal/states/instance_object_src.go b/internal/states/instance_object_src.go index 63b7963c2249..fd7d445759b2 100644 --- a/internal/states/instance_object_src.go +++ b/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 +} diff --git a/internal/states/output_value.go b/internal/states/output_value.go index 98a3606c2283..4da33f82dc80 100644 --- a/internal/states/output_value.go +++ b/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) +} diff --git a/internal/states/resource.go b/internal/states/resource.go index 25eed13c5ee5..70e290c69b9b 100644 --- a/internal/states/resource.go +++ b/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 +} diff --git a/internal/states/state_equal.go b/internal/states/state_equal.go index f771a714b2d0..61e343ff30f1 100644 --- a/internal/states/state_equal.go +++ b/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 + } + + 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 + } -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, 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 } } diff --git a/internal/states/state_test.go b/internal/states/state_test.go index 0ce3f74c806d..4d6915978a84 100644 --- a/internal/states/state_test.go +++ b/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) {