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

@@ -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 {

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

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)

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,

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)
+	}
+}