Non-quoting requirement for tags in TF 0.12.0-alpha2 breaks many of our configs

[ashafer01]

Terraform Version

Terraform v0.12.0-alpha2

Terraform Configuration Files

resource "aws_instance" "microservice" {
  # ...
  tags {
    # ...
    "ops:chef_role"   = "${local.chef_role}"
}

Expected Behavior

Terraform should work even with quoted tag names.

Actual Behavior

Almost all terraform operations fail (including very basic things like terraform providers) with the following error:

Error: Invalid argument name               
                                                                      
  on modules/microservice/ec2.tf line 86:                             
  86:     "ops:chef_role"   = "${local.chef_role}"                 
                                                                    
Argument names must not be quoted.

Steps to Reproduce

1. Include a config with any AWS tags that require quoting
2. Run any terraform command (such as init or plan or providers)

[apparentlymart]

Hi @ashafer01! Sorry for this confusing error.

Unfortunately due to the order of operations here Terraform is giving a misleading error. In prior versions of Terraform the parser was very liberal in how it dealt with nested blocks vs. attributes with map values, allowing certain forms like you have used here to work. However, this flexibility at the parser layer was not true all the way through the language interpreter, which was the root cause of a variety of strange bugs.

As a consequence, the new parser in v0.12 is stricter about this distinction, requiring you to use an equals sign for map attributes and to omit the equals sign for nested blocks. Since tags is a map attribute, it must now be written this way:

resource "aws_instance" "microservice" {
  # ...

  tags = {
    "ops:chef_role"   = "${local.chef_role}"
  }
}

Normally there is a helpful error message to point this out and tell you that tags is an attribute, but unfortunately Terraform isn't getting that far because the parser is detecting tags { } as a nested block and then failing to parse it due to the quoted attribute name, as you saw. This means you see the syntax error rather than the helpful semantic analysis error. 😖

The configuration upgrade tool that will be included in the final release will be able to fix this usage automatically by adding in that equals sign. However, we should look for a way to return a better error for the situation you found here so that we don't create the misleading impression that tags containing non-identifier characters are now invalid; that improved error message and something in the upgrade guide is what we'll use this issue to represent.

Thanks for pointing this out, and sorry for the misleading message.

[ashafer01]

Thanks so much for the detailed response, @apparentlymart . This makes a lot of sense - somehow I never realized tags was not supposed to be a nested block; it definitely makes a lot more sense as a map attribute. And a big 👍 to improving the error output and docs around this.

Will get our tags usage corrected and report any other issues we find. We have an extensive terraform configuration with several complex modules, and everyone is very excited for TF 12!

[apparentlymart]

We've not had a chance yet to dig in and try to improve this error message due to focus being elsewhere, but I tested this situation with the partially-implemented upgrade tool in v0.12.0-alpha4 and verified that it does at least fix the problem by automatically adding that equals sign:

--- quoted-tags-confusion.tf.orig	2018-12-17 17:18:00.897643148 -0800
+++ quoted-tags-confusion.tf	2018-12-17 17:18:08.129682999 -0800
@@ -1,5 +1,6 @@
 resource "aws_instance" "microservice" {
-  tags {
+  tags = {
     "ops:chef_role" = "foo"
   }
 }

I'm going to leave this open to see if we can still improve that error message, but with the configuration upgrade tool able to deal with it we might end up cutting this to reduce scope and get the final release out sooner. We'll improve the error message if there's a reasonable way to do it without adding a lot of extra complexity, though.

[ebekker]

Would it be useful to implement "fallback" behavior when parsing map attributes (normally with the equals sign) to allow omitting the equals sign. For example the parser would be strict about a missing an equals sign for nested blocks, but for map attributes, it would be allow with or without the equals sign.

Presumably a resource cannot have both a map attribute and a nested block with the same name (correct?), so there should not be any conflict in distinguishing one from the other. I find the ability to define map attribute values without the equal sign more elegant and more conducive to promoting that DSL feel.

[apparentlymart]

This distinction is important both from an implementation standpoint and from a user experience standpoint.

Implementation-wise it is important because it allows the parser to know that the keys that follow are to be interpreted as expressions rather than literal names. This is a consequence of merging HCL and HIL together into a single language, where before dynamic keys required some ugly function calls as seen in #2042. The new HCL parser uses the equals sign as a signal to parse what follows as an arbitrary expression rather than as a block.

The user experience reason is, of course, more subjective: we think it's important that objects and maps have this subtle visual distinction so that it's easy to understand whether the key/value pairs inside are fixed arguments defined by the schema or arbitrary keys defined by the user. The poor separation between these concepts has historically caused lots of user confusion as to why certain permutations worked while others did not. I understand that existing users have developed their own tastes that exploit the high amount of flexibility HCL 1 allowed, but we've made the conscious decision for Terraform 0.12 to be more consistent and prescriptive about style to improve readability as people move from one Terraform codebase to another over time.

The configuration upgrade tool's purpose is to help introduce that new consistency into existing codebases without a lot of manual work. That is the compromise we've chosen to take here to get this improved consistency while minimizing upgrade friction.

[ebekker]

Ah, that makes sense, I didn't think about the case where you could have a expression on the right that simply evaluates to a whole value of type map. In light of that, it does make sense to keep the two use cases strictly distinct.

[apparentlymart]

Hi again, all.

Unfortunately it doesn't seem like it'll be practical to introduce a specialized error message there because the parser is part of HCL itself rather than of Terraform, and HCL doesn't know anything about Terraform's upgrade process.

Instead, we're going to rely on the upgrade tool to deal with this situation. This is one of the few sharp corners where the new parser can't parse the old syntax at all, and so it was initially creating a chicken/egg problem for terraform init that was discussed over in #19724. This is now fixed in master by making terraform init detect this situation and run in a special mode where it'll install the required providers and instruct the user to run terraform 0.12upgrade to complete the required changes.

While this is not an ideal situation by any means, this particular problem is a collision of a few different tricky problems and so the upgrade tool is, it seems, our best tool to deal with it while keeping the special cases self-contained. We'll note this situation in the upgrade guide that will accompany the final 0.12.0 release.

Thanks again for reporting this, @ashafer01! I'm going to close this out now just because the development work for what I described above was already merged.

[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.