"list of string is required" for list expressions with redundant extra brackets

[apparentlymart]

Terraform Version

Terraform v0.12.0-alpha1

Terraform Configuration Files

Prior to Terraform v0.9.6 it was necessary to wrap any interpolation expression that returns a list in an extra level of list brackets due to an inherited limitation from earlier versions of Terraform that lacked first-class list support:

    vpc_security_group_ids = ["${aws_security_group.example.*.id}"]

This requirement was lifted in Terraform v0.9.6 but this form with the extra level of brackets was still supported for compatibility by ignoring wrapping lists with only one element that is also a list.

To allow the language to now properly support lists of lists, this compatibility fallback is being removed for v0.12, where the above would now be a type mismatch error. Instead, we can now write the expression alone, without either the extra redundant brackets or the string interpolation syntax:

    vpc_security_group_ids = aws_security_group.example.*.id

This problem is unfortunately exacerbated because there's another bug in the SDK as of 0.11 (and, I think, much earlier) where in some cases the SDK fails to handle an unknown list, returning the confusing error "must be a list". The backward-compatibility syntax for lists has been employed as a workaround for that bug, so there are existing configurations in the wild that use the brackets in versions 0.10 and later for this reason.

Expected Behavior

When encountering an expression that is a list of lists where only a single list was expected, produce a helpful error message explaining that this behavior has changed in v0.12 and suggesting to remove those extra brackets to migrate.

(The configuration upgrade tool coming with v0.12 final will make this change automatically, but not everyone will choose to use it.)

Actual Behavior

In v0.12-alpha1 there is no special-case handling of this error and so only a generic type mismatch error is produced: "list of string is required".

[apparentlymart]

Here's a simple test configuration to verify this against:

resource "aws_instance" "foo" {
  ami           = "ami-abc123"
  instance_type = "t2.micro"

  vpc_security_group_ids = ["${aws_security_group.example.*.id}"]
}

resource "aws_security_group" "example" {
  name = "testing blahblah"
}

We've not yet updated the error message as of 0.12.0-alpha4, so this is still returning the generic error given in the original issue comment:

Error: Incorrect attribute value type

  on redundant-list-brackets.tf line 5, in resource "aws_instance" "foo":
   5:   vpc_security_group_ids = ["${aws_security_group.example.*.id}"]

Inappropriate value for attribute "vpc_security_group_ids": set of string
required.

The work-in-progress snapshot of the upgrade tool in v0.12.0-alpha4 also failed to fix this up properly:

--- redundant-list-brackets.tf.orig	2018-12-17 18:51:15.265843439 -0800
+++ redundant-list-brackets.tf	2018-12-17 18:51:25.093833195 -0800
@@ -2,9 +2,10 @@
   ami           = "ami-abc123"
   instance_type = "t2.micro"
 
-  vpc_security_group_ids = ["${aws_security_group.example.*.id}"]
+  vpc_security_group_ids = [aws_security_group.example.*.id]
 }
 
 resource "aws_security_group" "example" {
   name = "testing blahblah"
 }

I think this is a false positive in the detection logic: because vpc_security_group_ids is typed as "set of string" rather than "list of string", the fixup rule isn't being activated. We need to ensure that this mapping rule applies also to sets, for consistency with our usual behavior of automatically converting lists into sets for arguments.

[apparentlymart]

Over in #20477 I verified that the upgrade tool now handles this properly, producing the following diff:

--- redundant-list-brackets.tf.orig	2019-02-22 16:54:44.606843923 -0800
+++ redundant-list-brackets.tf	2019-02-22 16:54:51.574961368 -0800
@@ -2,7 +2,7 @@
   ami           = "ami-abc123"
   instance_type = "t2.micro"
 
-  vpc_security_group_ids = ["${aws_security_group.example.*.id}"]
+  vpc_security_group_ids = aws_security_group.example.*.id
 }
 
 resource "aws_security_group" "example" {

Unfortunately the news for the error message isn't so good: this type-check happens in the guts of the HCL decoder and the Terraform-level code doesn't get any opportunity to insert custom behavior into it, and so without some significant refactoring in HCL (which would be too risky to do at this late stage), a custom error message would not be possible.

The other option I considered here was to make the error message describe what type it found along with what type was expected, but that doesn't produce a helpful result for the usual reason: HCL has various automatic type conversions that are intended to allow users to not worry too much about exact types most of the time, and as a consequence it gets away with this situation where the result of a tuple constructor is being assigned where a set is expected. Printing out the actual type along with the expected type therefore makes this (along with various other similar situations) more confusing than otherwise:

Inappropriate value for attribute "vpc_security_group_ids": set of string required, but got tuple.

A user reading this would be reasonable to understand it as needing some explicit conversion from tuple to set, when really it's the element types that are incorrect here. While this particular legacy situation in Terraform happens to interact poorly with this HCL behavior, the upgrade tool fixes it for existing configurations, and so it should become less common over time and thus not worth making the HCL error messages more confusing in other cases.

With all of that said, I think we've done the best we can here for the moment. We may revisit this in a v0.12.0 patch release if real-world experience suggests that the more significant HCL2-level refactor is warranted to let Terraform insert a custom error message here, but for initial release we'll rely on the upgrade tool and guide.

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.