From 928e60672f4eca77dc1d0734379d9f103c13ca69 Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Fri, 14 Apr 2017 10:53:08 -0400
Subject: [PATCH] context Refresh and Apply sometimes return nil

The documentation for Refresh indicates that it will always return a
valid state, but that wasn't true in the case of a graph builder error.
While this same concept wasn't documented for Apply, it was still
assumed in the terraform apply code.

Since the helper testing framework relies on the absence of a state to
determine if it can call Destroy, the Context can't can't start
returning a state in all cases. Document this, and use the State method
to fetch the correct state value after Apply.

Add a nil check to the WriteState function, so that writing a nil state
is a noop.

Make sure to init before sorting the state, to make sure we're not
attempting to sort nil values. This isn't technically needed with the
current code, but it's just safer in general.
---
 backend/local/backend_apply.go |  4 +++-
 terraform/context.go           | 15 ++++++++++++---
 terraform/state.go             | 21 +++++++++++++--------
 3 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/backend/local/backend_apply.go b/backend/local/backend_apply.go
index d7bf534a1..8fec2019e 100644
--- a/backend/local/backend_apply.go
+++ b/backend/local/backend_apply.go
@@ -102,7 +102,9 @@ func (b *Local) opApply(
 	doneCh := make(chan struct{})
 	go func() {
 		defer close(doneCh)
-		applyState, applyErr = tfCtx.Apply()
+		_, applyErr = tfCtx.Apply()
+		// we always want the state, even if apply failed
+		applyState = tfCtx.State()
 
 		/*
 			// Record any shadow errors for later
diff --git a/terraform/context.go b/terraform/context.go
index 15528beed..306128edf 100644
--- a/terraform/context.go
+++ b/terraform/context.go
@@ -453,8 +453,17 @@ func (c *Context) Input(mode InputMode) error {
 // Apply applies the changes represented by this context and returns
 // the resulting state.
 //
-// In addition to returning the resulting state, this context is updated
-// with the latest state.
+// Even in the case an error is returned, the state may be returned and will
+// potentially be partially updated.  In addition to returning the resulting
+// state, this context is updated with the latest state.
+//
+// If the state is required after an error, the caller should call
+// Context.State, rather than rely on the return value.
+//
+// TODO: Apply and Refresh should either always return a state, or rely on the
+//       State() method. Currently the helper/resource testing framework relies
+//       on the absence of a returned state to determine if Destroy can be
+//       called, so that will need to be refactored before this can be changed.
 func (c *Context) Apply() (*State, error) {
 	defer c.acquireRun("apply")()
 
@@ -580,7 +589,7 @@ func (c *Context) Plan() (*Plan, error) {
 // to their latest state. This will update the state that this context
 // works with, along with returning it.
 //
-// Even in the case an error is returned, the state will be returned and
+// Even in the case an error is returned, the state may be returned and
 // will potentially be partially updated.
 func (c *Context) Refresh() (*State, error) {
 	defer c.acquireRun("refresh")()
diff --git a/terraform/state.go b/terraform/state.go
index 78725e88d..84d4c2669 100644
--- a/terraform/state.go
+++ b/terraform/state.go
@@ -1960,12 +1960,12 @@ func ReadStateV2(jsonBytes []byte) (*State, error) {
 		}
 	}
 
-	// Sort it
-	state.sort()
-
 	// catch any unitialized fields in the state
 	state.init()
 
+	// Sort it
+	state.sort()
+
 	return state, nil
 }
 
@@ -1995,12 +1995,12 @@ func ReadStateV3(jsonBytes []byte) (*State, error) {
 		}
 	}
 
-	// Sort it
-	state.sort()
-
 	// catch any unitialized fields in the state
 	state.init()
 
+	// Sort it
+	state.sort()
+
 	// Now we write the state back out to detect any changes in normaliztion.
 	// If our state is now written out differently, bump the serial number to
 	// prevent conflicts.
@@ -2020,12 +2020,17 @@ func ReadStateV3(jsonBytes []byte) (*State, error) {
 
 // WriteState writes a state somewhere in a binary format.
 func WriteState(d *State, dst io.Writer) error {
-	// Make sure it is sorted
-	d.sort()
+	// writing a nil state is a noop.
+	if d == nil {
+		return nil
+	}
 
 	// make sure we have no uninitialized fields
 	d.init()
 
+	// Make sure it is sorted
+	d.sort()
+
 	// Ensure the version is set
 	d.Version = StateVersion