From 102c575f042db3d081f66ecc78dd872f82a329fd Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Fri, 6 Sep 2019 17:40:45 -0400
Subject: [PATCH] don't send an invalid config to a provisioner

The config diagnostics weren't checked before sending to the
provisioner, and may have contained invalid values.
---
 terraform/context_apply_test.go               | 66 +++++++++++++++++++
 terraform/eval_apply.go                       |  5 ++
 .../testdata/apply-provisioner-each/main.tf   |  7 ++
 3 files changed, 78 insertions(+)
 create mode 100644 terraform/testdata/apply-provisioner-each/main.tf

diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go
index 73dab280e..bd362668f 100644
--- a/terraform/context_apply_test.go
+++ b/terraform/context_apply_test.go
@@ -2018,6 +2018,72 @@ aws_instance.foo:
 	`)
 }
 
+// for_each values cannot be used in the provisioner during destroy.
+// There may be a way to handle this, but for now make sure we print an error
+// rather than crashing with an invalid config.
+func TestContext2Apply_provisionerDestroyForEach(t *testing.T) {
+	m := testModule(t, "apply-provisioner-each")
+	p := testProvider("aws")
+	pr := testProvisioner()
+	p.DiffFn = testDiffFn
+	p.ApplyFn = testApplyFn
+
+	s := &states.State{
+		Modules: map[string]*states.Module{
+			"": &states.Module{
+				Resources: map[string]*states.Resource{
+					"aws_instance.bar": &states.Resource{
+						Addr:     addrs.Resource{Mode: 77, Type: "aws_instance", Name: "bar"},
+						EachMode: states.EachMap,
+						Instances: map[addrs.InstanceKey]*states.ResourceInstance{
+							addrs.StringKey("a"): &states.ResourceInstance{
+								Current: &states.ResourceInstanceObjectSrc{
+									AttrsJSON: []byte(`{"foo":"bar","id":"foo"}`),
+								},
+							},
+							addrs.StringKey("b"): &states.ResourceInstance{
+								Current: &states.ResourceInstanceObjectSrc{
+									AttrsJSON: []byte(`{"foo":"bar","id":"foo"}`),
+								},
+							},
+						},
+						ProviderConfig: addrs.AbsProviderConfig{
+							Module:         addrs.ModuleInstance(nil),
+							ProviderConfig: addrs.ProviderConfig{Type: "aws", Alias: ""},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	ctx := testContext2(t, &ContextOpts{
+		Config: m,
+		ProviderResolver: providers.ResolverFixed(
+			map[string]providers.Factory{
+				"aws": testProviderFuncFixed(p),
+			},
+		),
+		Provisioners: map[string]ProvisionerFactory{
+			"shell": testProvisionerFuncFixed(pr),
+		},
+		State:   s,
+		Destroy: true,
+	})
+
+	if _, diags := ctx.Plan(); diags.HasErrors() {
+		t.Fatalf("plan errors: %s", diags.Err())
+	}
+
+	_, diags := ctx.Apply()
+	if diags == nil {
+		t.Fatal("should error")
+	}
+	if !strings.Contains(diags.Err().Error(), `Reference to "each" in context without for_each`) {
+		t.Fatal("unexpected error:", diags.Err())
+	}
+}
+
 func TestContext2Apply_cancelProvisioner(t *testing.T) {
 	m := testModule(t, "apply-cancel-provisioner")
 	p := testProvider("aws")
diff --git a/terraform/eval_apply.go b/terraform/eval_apply.go
index 7178f0030..7bf786654 100644
--- a/terraform/eval_apply.go
+++ b/terraform/eval_apply.go
@@ -557,6 +557,11 @@ func (n *EvalApplyProvisioners) apply(ctx EvalContext, provs []*configs.Provisio
 		config, _, configDiags := ctx.EvaluateBlock(prov.Config, schema, instanceAddr, keyData)
 		diags = diags.Append(configDiags)
 
+		// we can't apply the provisioner if the config has errors
+		if diags.HasErrors() {
+			return diags.Err()
+		}
+
 		// If the provisioner block contains a connection block of its own then
 		// it can override the base connection configuration, if any.
 		var localConn hcl.Body
diff --git a/terraform/testdata/apply-provisioner-each/main.tf b/terraform/testdata/apply-provisioner-each/main.tf
new file mode 100644
index 000000000..8d29a5c16
--- /dev/null
+++ b/terraform/testdata/apply-provisioner-each/main.tf
@@ -0,0 +1,7 @@
+resource "aws_instance" "bar" {
+    for_each = toset(["a"])
+    provisioner "shell" {
+      when = "destroy"
+      command = "echo ${each.value}"
+    }
+}