Skip to content

Commit

Permalink
Merge pull request #22597 from pselle/for-each-wholly-known
Browse files Browse the repository at this point in the history
Ensure for_each values wholly known for sets
  • Loading branch information
Pam Selle authored Sep 3, 2019
2 parents c4e508f + 37e8147 commit 555ed96
Show file tree
Hide file tree
Showing 6 changed files with 126 additions and 1 deletion.
35 changes: 35 additions & 0 deletions terraform/context_apply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
28 changes: 28 additions & 0 deletions terraform/context_plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
10 changes: 9 additions & 1 deletion terraform/eval_for_each.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.`,
})
}
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions terraform/node_data_refresh.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down
33 changes: 33 additions & 0 deletions terraform/terraform_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 20 additions & 0 deletions terraform/testdata/plan-for-each-unknown-value/main.tf
Original file line number Diff line number Diff line change
@@ -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
}

0 comments on commit 555ed96

Please sign in to comment.