From ac2219ba6ee17928d0deef700e0cb07caedf4f2c Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 4 Jun 2019 10:32:12 -0400 Subject: [PATCH] don't lose Private state data during copy Fix the scope of the private data copy in DeepCopy. Make sure Dependencies matches nil vs empty so that Equal compares correctly between copied states --- command/apply_test.go | 3 +- command/command_test.go | 10 +++++-- command/refresh_test.go | 6 ++-- states/state_deepcopy.go | 13 +++++---- states/state_test.go | 59 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 80 insertions(+), 11 deletions(-) diff --git a/command/apply_test.go b/command/apply_test.go index c27d184d89..4d86449eac 100644 --- a/command/apply_test.go +++ b/command/apply_test.go @@ -15,6 +15,7 @@ import ( "time" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/mitchellh/cli" "github.com/zclconf/go-cty/cty" @@ -1389,7 +1390,7 @@ func TestApply_backup(t *testing.T) { actual := backupState.RootModule().Resources["test_instance.foo"] expected := originalState.RootModule().Resources["test_instance.foo"] - if !cmp.Equal(actual, expected) { + if !cmp.Equal(actual, expected, cmpopts.EquateEmpty()) { t.Fatalf( "wrong aws_instance.foo state\n%s", cmp.Diff(expected, actual, cmp.Transformer("bytesAsString", func(b []byte) string { diff --git a/command/command_test.go b/command/command_test.go index 8753028bc3..d701c9a3b1 100644 --- a/command/command_test.go +++ b/command/command_test.go @@ -7,8 +7,6 @@ import ( "encoding/json" "flag" "fmt" - "github.com/hashicorp/terraform/internal/initwd" - "github.com/hashicorp/terraform/registry" "io" "io/ioutil" "log" @@ -21,6 +19,9 @@ import ( "syscall" "testing" + "github.com/hashicorp/terraform/internal/initwd" + "github.com/hashicorp/terraform/registry" + "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs" "github.com/hashicorp/terraform/configs/configload" @@ -266,7 +267,10 @@ func testState() *states.State { Type: "test", }.Absolute(addrs.RootModuleInstance), ) - }) + // DeepCopy is used here to ensure our synthetic state matches exactly + // with a state that will have been copied during the command + // operation, and all fields have been copied correctly. + }).DeepCopy() } // writeStateForTesting is a helper that writes the given naked state to the diff --git a/command/refresh_test.go b/command/refresh_test.go index 603781466c..5781c82d1c 100644 --- a/command/refresh_test.go +++ b/command/refresh_test.go @@ -12,6 +12,8 @@ import ( "testing" "github.com/davecgh/go-spew/spew" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/mitchellh/cli" "github.com/zclconf/go-cty/cty" @@ -557,8 +559,8 @@ func TestRefresh_backup(t *testing.T) { } newState := testStateRead(t, statePath) - if !reflect.DeepEqual(newState, state) { - t.Fatalf("bad: %#v", newState) + if !cmp.Equal(newState, state, cmpopts.EquateEmpty()) { + t.Fatalf("got:\n%s\nexpected:\n%s\n", newState, state) } newState = testStateRead(t, outPath) diff --git a/states/state_deepcopy.go b/states/state_deepcopy.go index ea717d00ec..8664f3bea2 100644 --- a/states/state_deepcopy.go +++ b/states/state_deepcopy.go @@ -147,7 +147,7 @@ func (obj *ResourceInstanceObjectSrc) DeepCopy() *ResourceInstanceObjectSrc { var private []byte if obj.Private != nil { - private := make([]byte, len(obj.Private)) + private = make([]byte, len(obj.Private)) copy(private, obj.Private) } @@ -181,14 +181,17 @@ func (obj *ResourceInstanceObject) DeepCopy() *ResourceInstanceObject { var private []byte if obj.Private != nil { - private := make([]byte, len(obj.Private)) + private = make([]byte, len(obj.Private)) copy(private, obj.Private) } - // Some addrs.Referencable implementations are technically mutable, but + // Some addrs.Referenceable implementations are technically mutable, but // we treat them as immutable by convention and so we don't deep-copy here. - dependencies := make([]addrs.Referenceable, len(obj.Dependencies)) - copy(dependencies, obj.Dependencies) + var dependencies []addrs.Referenceable + if obj.Dependencies != nil { + dependencies = make([]addrs.Referenceable, len(obj.Dependencies)) + copy(dependencies, obj.Dependencies) + } return &ResourceInstanceObject{ Value: obj.Value, diff --git a/states/state_test.go b/states/state_test.go index 8f941d1797..618cfaafb8 100644 --- a/states/state_test.go +++ b/states/state_test.go @@ -115,3 +115,62 @@ func TestState(t *testing.T) { t.Error(problem) } } + +func TestStateDeepCopy(t *testing.T) { + state := NewState() + + rootModule := state.RootModule() + if rootModule == nil { + t.Errorf("root module is nil; want valid object") + } + + rootModule.SetLocalValue("foo", cty.StringVal("foo value")) + rootModule.SetOutputValue("bar", cty.StringVal("bar value"), false) + rootModule.SetOutputValue("secret", cty.StringVal("secret value"), true) + rootModule.SetResourceInstanceCurrent( + addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "test_thing", + Name: "baz", + }.Instance(addrs.IntKey(0)), + &ResourceInstanceObjectSrc{ + Status: ObjectReady, + SchemaVersion: 1, + AttrsJSON: []byte(`{"woozles":"confuzles"}`), + Private: []byte("private data"), + Dependencies: []addrs.Referenceable{}, + }, + addrs.ProviderConfig{ + Type: "test", + }.Absolute(addrs.RootModuleInstance), + ) + rootModule.SetResourceInstanceCurrent( + addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "test_thing", + Name: "bar", + }.Instance(addrs.IntKey(0)), + &ResourceInstanceObjectSrc{ + Status: ObjectReady, + SchemaVersion: 1, + AttrsJSON: []byte(`{"woozles":"confuzles"}`), + Private: []byte("private data"), + Dependencies: []addrs.Referenceable{addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "test_thing", + Name: "baz", + }}, + }, + addrs.ProviderConfig{ + Type: "test", + }.Absolute(addrs.RootModuleInstance), + ) + + childModule := state.EnsureModule(addrs.RootModuleInstance.Child("child", addrs.NoKey)) + childModule.SetOutputValue("pizza", cty.StringVal("hawaiian"), false) + + stateCopy := state.DeepCopy() + if !state.Equal(stateCopy) { + t.Fatalf("\nexpected:\n%q\ngot:\n%q\n", state, stateCopy) + } +}