Ensure better state normalization

Fix checksum issue with remote state

If we read a state file with "null" objects in a module and they become
initialized to an empty map the state file may be written out with empty
objects rather than "null", changing the checksum. If we can detect
this, increment the serial number to prevent a conflict in atlas.

Our fakeAtlas test server now needs to decode the state directly rather
than using the ReadState function, so as to be able to read the state
unaltered.

The terraform.State data structures have initialization spread out
throughout the package. More thoroughly initialize State during
ReadState, and add a call to init() during WriteState as another
normalization safeguard.

Expose State.init through an exported Init() method, so that a new State
can be completely realized outside of the terraform package.
Additionally, the internal init now completely walks all internal state
structures ensuring that all maps and slices are initialized.  While it
was mentioned before that the `init()` methods are problematic with too
many call sites, expanding this out better exposes the entry points that
will need to be refactored later for improved concurrency handling.

The State structures had a mix of `omitempty` fields. Remove omitempty
for all maps and slices as part of this normalization process. Make
Lineage mandatory, which is now explicitly set in some tests.

command/apply_test.go

@@ -1181,6 +1181,7 @@ func TestApply_backup(t *testing.T) {
 			},
 		},
 	}
+	originalState.Init()
 
 	statePath := testStateFile(t, originalState)
 	backupPath := testTempFile(t)

command/command_test.go

@@ -131,7 +131,7 @@ func testReadPlan(t *testing.T, path string) *terraform.Plan {
 
 // testState returns a test State structure that we use for a lot of tests.
 func testState() *terraform.State {
-	return &terraform.State{
+	state := &terraform.State{
 		Version: 2,
 		Modules: []*terraform.ModuleState{
 			&terraform.ModuleState{
@@ -148,6 +148,8 @@ func testState() *terraform.State {
 			},
 		},
 	}
+	state.Init()
+	return state
 }
 
 func testStateFile(t *testing.T, s *terraform.State) string {

command/refresh_test.go

@@ -173,7 +173,7 @@ func TestRefresh_defaultState(t *testing.T) {
 	}
 
 	p.RefreshFn = nil
-	p.RefreshReturn = &terraform.InstanceState{ID: "yes"}
+	p.RefreshReturn = newInstanceState("yes")
 
 	args := []string{
 		testFixturePath("refresh"),
@@ -200,7 +200,8 @@ func TestRefresh_defaultState(t *testing.T) {
 	actual := newState.RootModule().Resources["test_instance.foo"].Primary
 	expected := p.RefreshReturn
 	if !reflect.DeepEqual(actual, expected) {
-		t.Fatalf("bad: %#v", actual)
+		t.Logf("expected:\n%#v", expected)
+		t.Fatalf("bad:\n%#v", actual)
 	}
 
 	f, err = os.Open(statePath + DefaultBackupExtension)
@@ -347,7 +348,7 @@ func TestRefresh_outPath(t *testing.T) {
 	}
 
 	p.RefreshFn = nil
-	p.RefreshReturn = &terraform.InstanceState{ID: "yes"}
+	p.RefreshReturn = newInstanceState("yes")
 
 	args := []string{
 		"-state", statePath,
@@ -577,7 +578,7 @@ func TestRefresh_backup(t *testing.T) {
 	}
 
 	p.RefreshFn = nil
-	p.RefreshReturn = &terraform.InstanceState{ID: "yes"}
+	p.RefreshReturn = newInstanceState("yes")
 
 	args := []string{
 		"-state", statePath,
@@ -662,7 +663,7 @@ func TestRefresh_disableBackup(t *testing.T) {
 	}
 
 	p.RefreshFn = nil
-	p.RefreshReturn = &terraform.InstanceState{ID: "yes"}
+	p.RefreshReturn = newInstanceState("yes")
 
 	args := []string{
 		"-state", statePath,
@@ -742,6 +743,20 @@ func TestRefresh_displaysOutputs(t *testing.T) {
 	}
 }
 
+// When creating an InstaneState for direct comparison to one contained in
+// terraform.State, all fields must be inintialized (duplicating the
+// InstanceState.init() method)
+func newInstanceState(id string) *terraform.InstanceState {
+	return &terraform.InstanceState{
+		ID:         id,
+		Attributes: make(map[string]string),
+		Ephemeral: terraform.EphemeralState{
+			ConnInfo: make(map[string]string),
+		},
+		Meta: make(map[string]string),
+	}
+}
+
 const refreshVarFile = `
 foo = "bar"
 `

state/remote/atlas_test.go

@@ -5,6 +5,7 @@ import (
 	"crypto/md5"
 	"crypto/tls"
 	"crypto/x509"
+	"encoding/json"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
@@ -161,6 +162,9 @@ func TestAtlasClient_LegitimateConflict(t *testing.T) {
 		t.Fatalf("err: %s", err)
 	}
 
+	var buf bytes.Buffer
+	terraform.WriteState(state, &buf)
+
 	// Changing the state but not the serial. Should generate a conflict.
 	state.RootModule().Outputs["drift"] = &terraform.OutputState{
 		Type:      "string",
@@ -244,7 +248,10 @@ func (f *fakeAtlas) Server() *httptest.Server {
 }
 
 func (f *fakeAtlas) CurrentState() *terraform.State {
-	currentState, err := terraform.ReadState(bytes.NewReader(f.state))
+	// we read the state manually here, because terraform amy alter state
+	// during read
+	currentState := &terraform.State{}
+	err := json.Unmarshal(f.state, currentState)
 	if err != nil {
 		f.t.Fatalf("err: %s", err)
 	}
@@ -288,10 +295,15 @@ func (f *fakeAtlas) handler(resp http.ResponseWriter, req *http.Request) {
 		var buf bytes.Buffer
 		buf.ReadFrom(req.Body)
 		sum := md5.Sum(buf.Bytes())
-		state, err := terraform.ReadState(&buf)
+
+		// we read the state manually here, because terraform amy alter state
+		// during read
+		state := &terraform.State{}
+		err := json.Unmarshal(buf.Bytes(), state)
 		if err != nil {
 			f.t.Fatalf("err: %s", err)
 		}
+
 		conflict := f.CurrentSerial() == state.Serial && f.CurrentSum() != sum
 		conflict = conflict || f.alwaysConflict
 		if conflict {
@@ -351,7 +363,8 @@ var testStateModuleOrderChange = []byte(
 var testStateSimple = []byte(
 	`{
     "version": 3,
-    "serial": 1,
+    "serial": 2,
+    "lineage": "c00ad9ac-9b35-42fe-846e-b06f0ef877e9",
     "modules": [
         {
             "path": [
@@ -364,7 +377,8 @@ var testStateSimple = []byte(
                     "value": "bar"
                 }
             },
-            "resources": null
+            "resources": {},
+            "depends_on": []
         }
     ]
 }

state/testing.go

@@ -1,7 +1,6 @@
 package state
 
 import (
-	"bytes"
 	"reflect"
 	"testing"
 
@@ -29,12 +28,12 @@ func TestState(t *testing.T, s interface{}) {
 
 	// Check that the initial state is correct
 	if state := reader.State(); !current.Equal(state) {
-		t.Fatalf("not initial: %#v\n\n%#v", state, current)
+		t.Fatalf("not initial:\n%#v\n\n%#v", state, current)
 	}
 
 	// Write a new state and verify that we have it
 	if ws, ok := s.(StateWriter); ok {
-		current.Modules = append(current.Modules, &terraform.ModuleState{
+		current.AddModuleState(&terraform.ModuleState{
 			Path: []string{"root"},
 			Outputs: map[string]*terraform.OutputState{
 				"bar": &terraform.OutputState{
@@ -50,7 +49,7 @@ func TestState(t *testing.T, s interface{}) {
 		}
 
 		if actual := reader.State(); !actual.Equal(current) {
-			t.Fatalf("bad: %#v\n\n%#v", actual, current)
+			t.Fatalf("bad:\n%#v\n\n%#v", actual, current)
 		}
 	}
 
@@ -146,7 +145,7 @@ func TestStateInitial() *terraform.State {
 		},
 	}
 
-	var scratch bytes.Buffer
-	terraform.WriteState(initial, &scratch)
+	initial.Init()
+
 	return initial
 }