configs/configupgrade: Detect and fix element(...) usage with sets

Although sets do not have indexed elements, in Terraform 0.11 and earlier
element(...) would work with sets because we'd automatically convert them
to lists on entry to HIL -- with an arbitrary-but-consistent ordering --
and this return an arbitrary-but-consistent element from the list.

The element(...) function in Terraform 0.12 does not allow this because it
is not safe in general, but there was an existing pattern relying on this
in Terraform 0.11 configs which this upgrade rule is intended to preserve:

    resource "example" "example" {
      count = "${length(any_set_attribute)}"

      foo = "${element(any_set_attribute, count.index}"
    }

The above works because the exact indices assigned in the conversion are
irrelevant: we're just asking Terraform to create one resource for each
distinct element in the set.

This upgrade rule therefore inserts an explicit conversion to list if it
is able to successfully provide that the given expression will return a
set type:

    foo = "${element(tolist(any_set_attribute), count.index}"

This makes the conversion explicit, allowing users to decide if it is
safe and rework the configuration if not. Since our static type analysis
functionality focuses mainly on resource type attributes, in practice this
rule will only apply when the given expression is a statically-checkable
resource reference. Since sets are an SDK-only concept in Terraform 0.11
and earlier anyway, in practice that works out just right: it's not
possible for sets to appear anywhere else in older versions anyway.
This commit is contained in:
Martin Atkins 2019-02-20 16:28:15 -08:00
parent 4ab701620e
commit fa0d6484df
5 changed files with 35 additions and 0 deletions

View File

@ -0,0 +1,7 @@
resource "test_instance" "a" {
subnet_ids = ["boop"] # this attribute takes a set of strings
}
output "b" {
value = "${element(test_instance.a.subnet_ids, 0)}"
}

View File

@ -0,0 +1,7 @@
resource "test_instance" "a" {
subnet_ids = ["boop"] # this attribute takes a set of strings
}
output "b" {
value = element(tolist(test_instance.a.subnet_ids), 0)
}

View File

@ -0,0 +1,3 @@
terraform {
required_version = ">= 0.12"
}

View File

@ -398,6 +398,23 @@ Value:
buf.WriteByte(']')
break Value
}
case "element":
// We cannot replace element with index syntax safely in general
// because users may be relying on its special modulo wraparound
// behavior that the index syntax doesn't do. However, if it seems
// like the user is trying to use element with a set, we'll insert
// an explicit conversion to list to mimic the implicit conversion
// that we used to do as an unintended side-effect of how functions
// work in HIL.
if len(argExprs) > 0 {
argTy := an.InferExpressionType(argExprs[0], nil)
if argTy.IsSetType() {
newExpr := []byte(`tolist(`)
newExpr = append(newExpr, argExprs[0]...)
newExpr = append(newExpr, ')')
argExprs[0] = newExpr
}
}
// HIL used some undocumented special functions to implement certain
// operations, but since those were actually callable in real expressions

View File

@ -195,6 +195,7 @@ var testProviders = map[string]providers.Factory{
"image": {Type: cty.String, Optional: true},
"tags": {Type: cty.Map(cty.String), Optional: true},
"security_groups": {Type: cty.List(cty.String), Optional: true},
"subnet_ids": {Type: cty.Set(cty.String), Optional: true},
},
BlockTypes: map[string]*configschema.NestedBlock{
"network": {