From d60365af0229bd7f5257030d4f3d1a222f828c36 Mon Sep 17 00:00:00 2001 From: James Nugent Date: Wed, 22 Jun 2016 17:07:55 +0300 Subject: [PATCH] core: Correctly ensure that State() is a copy The previous mechanism for testing state threw away the mutation made on the state by calling State() twice - this commit corrects the test to match the comment. In addition, we replace the custom copying logic with the copystructure library to simplify the code. --- state/testing.go | 8 +++-- terraform/state.go | 85 +++++++++++----------------------------------- 2 files changed, 24 insertions(+), 69 deletions(-) diff --git a/state/testing.go b/state/testing.go index 357ea234a..207dda7d7 100644 --- a/state/testing.go +++ b/state/testing.go @@ -118,9 +118,11 @@ func TestState(t *testing.T, s interface{}) { t.Fatalf("bad: expected %d, got %d", serial, reader.State().Serial) } - // Check that State() returns a copy - reader.State().Serial++ - if reflect.DeepEqual(reader.State(), current) { + // Check that State() returns a copy by modifying the copy and comparing + // to the current state. + stateCopy := reader.State() + stateCopy.Serial++ + if reflect.DeepEqual(stateCopy, current) { t.Fatal("State() should return a copy") } } diff --git a/terraform/state.go b/terraform/state.go index 2e7708977..4ca020685 100644 --- a/terraform/state.go +++ b/terraform/state.go @@ -458,23 +458,12 @@ func (s *State) SameLineage(other *State) bool { // DeepCopy performs a deep copy of the state structure and returns // a new structure. func (s *State) DeepCopy() *State { - if s == nil { - return nil + copy, err := copystructure.Copy(s) + if err != nil { + panic(err) } - n := &State{ - Version: s.Version, - Lineage: s.Lineage, - TFVersion: s.TFVersion, - Serial: s.Serial, - Modules: make([]*ModuleState, 0, len(s.Modules)), - } - for _, mod := range s.Modules { - n.Modules = append(n.Modules, mod.deepcopy()) - } - if s.Remote != nil { - n.Remote = s.Remote.deepcopy() - } - return n + + return copy.(*State) } // IncrementSerialMaybe increments the serial number of this state @@ -1187,28 +1176,12 @@ func (r *ResourceState) init() { } func (r *ResourceState) deepcopy() *ResourceState { - if r == nil { - return nil + copy, err := copystructure.Copy(r) + if err != nil { + panic(err) } - n := &ResourceState{ - Type: r.Type, - Dependencies: nil, - Primary: r.Primary.DeepCopy(), - Provider: r.Provider, - } - if r.Dependencies != nil { - n.Dependencies = make([]string, len(r.Dependencies)) - copy(n.Dependencies, r.Dependencies) - } - if r.Deposed != nil { - n.Deposed = make([]*InstanceState, 0, len(r.Deposed)) - for _, inst := range r.Deposed { - n.Deposed = append(n.Deposed, inst.DeepCopy()) - } - } - - return n + return copy.(*ResourceState) } // prune is used to remove any instances that are no longer required @@ -1278,27 +1251,12 @@ func (i *InstanceState) init() { } func (i *InstanceState) DeepCopy() *InstanceState { - if i == nil { - return nil + copy, err := copystructure.Copy(i) + if err != nil { + panic(err) } - n := &InstanceState{ - ID: i.ID, - Ephemeral: *i.Ephemeral.DeepCopy(), - Tainted: i.Tainted, - } - if i.Attributes != nil { - n.Attributes = make(map[string]string, len(i.Attributes)) - for k, v := range i.Attributes { - n.Attributes[k] = v - } - } - if i.Meta != nil { - n.Meta = make(map[string]string, len(i.Meta)) - for k, v := range i.Meta { - n.Meta[k] = v - } - } - return n + + return copy.(*InstanceState) } func (s *InstanceState) Empty() bool { @@ -1446,17 +1404,12 @@ func (e *EphemeralState) init() { } func (e *EphemeralState) DeepCopy() *EphemeralState { - if e == nil { - return nil + copy, err := copystructure.Copy(e) + if err != nil { + panic(err) } - n := &EphemeralState{} - if e.ConnInfo != nil { - n.ConnInfo = make(map[string]string, len(e.ConnInfo)) - for k, v := range e.ConnInfo { - n.ConnInfo[k] = v - } - } - return n + + return copy.(*EphemeralState) } type jsonStateVersionIdentifier struct {