Merge pull request #22597 from pselle/for-each-wholly-known

Ensure for_each values wholly known for sets

terraform/context_apply_test.go

@@ -2562,6 +2562,41 @@ func TestContext2Apply_provisionerInterpCount(t *testing.T) {
 	}
 }
 
+func TestContext2Apply_foreachVariable(t *testing.T) {
+	m := testModule(t, "plan-for-each-unknown-value")
+	p := testProvider("aws")
+	p.ApplyFn = testApplyFn
+	p.DiffFn = testDiffFn
+	ctx := testContext2(t, &ContextOpts{
+		Config: m,
+		ProviderResolver: providers.ResolverFixed(
+			map[string]providers.Factory{
+				"aws": testProviderFuncFixed(p),
+			},
+		),
+		Variables: InputValues{
+			"foo": &InputValue{
+				Value: cty.StringVal("hello"),
+			},
+		},
+	})
+
+	if _, diags := ctx.Plan(); diags.HasErrors() {
+		t.Fatalf("plan errors: %s", diags.Err())
+	}
+
+	state, diags := ctx.Apply()
+	if diags.HasErrors() {
+		t.Fatalf("diags: %s", diags.Err())
+	}
+
+	actual := strings.TrimSpace(state.String())
+	expected := strings.TrimSpace(testTerraformApplyForEachVariableStr)
+	if actual != expected {
+		t.Fatalf("wrong result\n\ngot:\n%s\n\nwant:\n%s", actual, expected)
+	}
+}
+
 func TestContext2Apply_moduleBasic(t *testing.T) {
 	m := testModule(t, "apply-module")
 	p := testProvider("aws")

terraform/context_plan_test.go

@@ -3385,7 +3385,35 @@ func TestContext2Plan_forEach(t *testing.T) {
 			t.Fatal(err)
 		}
 	}
+}
+
+func TestContext2Plan_forEachUnknownValue(t *testing.T) {
+	// This module has a variable defined, but it is not provided
+	// in the context below and we expect the plan to error, but not panic
+	m := testModule(t, "plan-for-each-unknown-value")
+	p := testProvider("aws")
+	p.DiffFn = testDiffFn
+	ctx := testContext2(t, &ContextOpts{
+		Config: m,
+		ProviderResolver: providers.ResolverFixed(
+			map[string]providers.Factory{
+				"aws": testProviderFuncFixed(p),
+			},
+		),
+	})
 
+	_, diags := ctx.Plan()
+	if !diags.HasErrors() {
+		// Should get this error:
+		// Invalid for_each argument: The "for_each" value depends on resource attributes that cannot be determined until apply...
+		t.Fatal("succeeded; want errors")
+	}
+
+	gotErrStr := diags.Err().Error()
+	wantErrStr := "Invalid for_each argument"
+	if !strings.Contains(gotErrStr, wantErrStr) {
+		t.Fatalf("missing expected error\ngot: %s\n\nwant: error containing %q", gotErrStr, wantErrStr)
+	}
 }
 
 func TestContext2Plan_destroy(t *testing.T) {

terraform/eval_for_each.go

@@ -19,7 +19,7 @@ func evaluateResourceForEachExpression(expr hcl.Expression, ctx EvalContext) (fo
 		// Attach a diag as we do with count, with the same downsides
 		diags = diags.Append(&hcl.Diagnostic{
 			Severity: hcl.DiagError,
-			Summary:  "Invalid forEach argument",
+			Summary:  "Invalid for_each argument",
 			Detail:   `The "for_each" value depends on resource attributes that cannot be determined until apply, so Terraform cannot predict how many instances will be created. To work around this, use the -target argument to first apply only the resources that the for_each depends on.`,
 		})
 	}
@@ -74,6 +74,14 @@ func evaluateResourceForEachExpressionKnown(expr hcl.Expression, ctx EvalContext
 			})
 			return nil, true, diags
 		}
+
+		// A set may contain unknown values that must be
+		// discovered by checking with IsWhollyKnown (which iterates through the
+		// structure), while for maps in cty, keys can never be unknown or null,
+		// thus the earlier IsKnown check suffices for maps
+		if !forEachVal.IsWhollyKnown() {
+			return map[string]cty.Value{}, false, diags
+		}
 	}
 
 	// If the map is empty ({}), return an empty map, because cty will return nil when representing {} AsValueMap

terraform/node_data_refresh.go

@@ -39,6 +39,7 @@ func (n *NodeRefreshableDataResource) DynamicExpand(ctx EvalContext) (*Graph, er
 	}
 
 	forEachMap, forEachKnown, forEachDiags := evaluateResourceForEachExpressionKnown(n.Config.ForEach, ctx)
+	diags = diags.Append(forEachDiags)
 	if forEachDiags.HasErrors() {
 		return nil, diags.Err()
 	}

terraform/terraform_test.go

@@ -538,7 +538,40 @@ aws_instance.foo.1:
   ID = foo
   provider = provider.aws
 `
+const testTerraformApplyForEachVariableStr = `
+aws_instance.foo["b15c6d616d6143248c575900dff57325eb1de498"]:
+  ID = foo
+  provider = provider.aws
+  foo = foo
+  type = aws_instance
+aws_instance.foo["c3de47d34b0a9f13918dd705c141d579dd6555fd"]:
+  ID = foo
+  provider = provider.aws
+  foo = foo
+  type = aws_instance
+aws_instance.foo["e30a7edcc42a846684f2a4eea5f3cd261d33c46d"]:
+  ID = foo
+  provider = provider.aws
+  foo = foo
+  type = aws_instance
+aws_instance.one["a"]:
+  ID = foo
+  provider = provider.aws
+aws_instance.one["b"]:
+  ID = foo
+  provider = provider.aws
+aws_instance.two["a"]:
+  ID = foo
+  provider = provider.aws
 
+  Dependencies:
+    aws_instance.one
+aws_instance.two["b"]:
+  ID = foo
+  provider = provider.aws
+
+  Dependencies:
+    aws_instance.one`
 const testTerraformApplyMinimalStr = `
 aws_instance.bar:
   ID = foo

terraform/testdata/plan-for-each-unknown-value/main.tf

@@ -0,0 +1,20 @@
+# expressions with variable reference
+variable "foo" {
+  type = string
+}
+
+resource "aws_instance" "foo" {
+  for_each = toset(
+       [for i in range(0,3) : sha1("${i}${var.foo}")]
+    )
+  foo = "foo"
+}
+
+# referencing another resource, which means it has some unknown values in it
+resource "aws_instance" "one" {
+  for_each = toset(["a", "b"])
+}
+
+resource "aws_instance" "two" {
+  for_each = aws_instance.one
+}