From a2f8482333ea99df923dd4795c4a95bd68f5baaa Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Tue, 30 Jan 2018 10:42:49 -0500
Subject: [PATCH] catch missing id attribute during interpolation

The id attribute can be missing during the destroy operation.
While the new destroy-time ordering of outputs and locals should prevent
resources from having their id attributes set to an empty string,
there's no reason to error out if we have the canonical ID field
available.

This still interrogates the attributes map first to retain any previous
behavior, but in the future we should settle on a single ID location.
---
 terraform/interpolate.go      | 15 ++++++++++++++
 terraform/interpolate_test.go | 38 +++++++++++++++++++++++++++++++++--
 2 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/terraform/interpolate.go b/terraform/interpolate.go
index 456e7e3a2..1509a65fe 100644
--- a/terraform/interpolate.go
+++ b/terraform/interpolate.go
@@ -518,6 +518,16 @@ func (i *Interpolater) computeResourceVariable(
 		return &v, err
 	}
 
+	// special case for the "id" field which is usually also an attribute
+	if v.Field == "id" && r.Primary.ID != "" {
+		// This is usually pulled from the attributes, but is sometimes missing
+		// during destroy. We can return the ID field in this case.
+		// FIXME: there should only be one ID to rule them all.
+		log.Printf("[WARN] resource %s missing 'id' attribute", v.ResourceId())
+		v, err := hil.InterfaceToVariable(r.Primary.ID)
+		return &v, err
+	}
+
 	// computed list or map attribute
 	_, isList = r.Primary.Attributes[v.Field+".#"]
 	_, isMap = r.Primary.Attributes[v.Field+".%"]
@@ -655,6 +665,11 @@ func (i *Interpolater) computeResourceMultiVariable(
 			continue
 		}
 
+		if v.Field == "id" && r.Primary.ID != "" {
+			log.Printf("[WARN] resource %s missing 'id' attribute", v.ResourceId())
+			values = append(values, r.Primary.ID)
+		}
+
 		// computed list or map attribute
 		_, isList := r.Primary.Attributes[v.Field+".#"]
 		_, isMap := r.Primary.Attributes[v.Field+".%"]
diff --git a/terraform/interpolate_test.go b/terraform/interpolate_test.go
index c497b43b8..10f23a278 100644
--- a/terraform/interpolate_test.go
+++ b/terraform/interpolate_test.go
@@ -129,6 +129,40 @@ func TestInterpolater_localVal(t *testing.T) {
 	})
 }
 
+func TestInterpolater_missingID(t *testing.T) {
+	lock := new(sync.RWMutex)
+	state := &State{
+		Modules: []*ModuleState{
+			&ModuleState{
+				Path: rootModulePath,
+				Resources: map[string]*ResourceState{
+					"aws_instance.web": &ResourceState{
+						Type: "aws_instance",
+						Primary: &InstanceState{
+							ID: "bar",
+						},
+					},
+				},
+			},
+		},
+	}
+
+	i := &Interpolater{
+		Module:    testModule(t, "interpolate-resource-variable"),
+		State:     state,
+		StateLock: lock,
+	}
+
+	scope := &InterpolationScope{
+		Path: rootModulePath,
+	}
+
+	testInterpolate(t, i, scope, "aws_instance.web.id", ast.Variable{
+		Value: "bar",
+		Type:  ast.TypeString,
+	})
+}
+
 func TestInterpolater_pathCwd(t *testing.T) {
 	i := &Interpolater{}
 	scope := &InterpolationScope{}
@@ -314,8 +348,8 @@ func TestInterpolater_resourceVariableMissingDuringInput(t *testing.T) {
 			&ModuleState{
 				Path:      rootModulePath,
 				Resources: map[string]*ResourceState{
-				// No resources at all yet, because we're still dealing
-				// with input and so the resources haven't been created.
+					// No resources at all yet, because we're still dealing
+					// with input and so the resources haven't been created.
 				},
 			},
 		},