terraform fmt error message do not include source code snippets

[ozbillwang]

Terraform Version

$ terraform12 --version
Terraform v0.12.0-beta1

$ terraform --version
Terraform v0.11.13

Terraform Configuration Files

$ cat backend.tf
terraform {backend "s3" {}}

Debug Output

$ terraform12 fmt

Error: Argument definition required

  on backend.tf line 1:
  (source code not available)

A single-line block definition can contain only a single argument. If you
meant to define argument "backend", use an equals sign to assign it a value.
To define a nested block, place it on a line of its own within its parent
block.

$ terraform fmt
backend.tf

$ cat backend.tf
terraform {
  backend "s3" {}
}

Crash Output

terraform 0.12 beta1 doesn't work with tf file which is in one line such as terraform {backend "s3" {}}, but terraform version 0.11.x and previous can.

Expected Behavior

Same as previous version.

Actual Behavior

Failed with error, show above.

Steps to Reproduce

show above.

Additional Context

References

[ozbillwang]

Second, seems 0.12upgrade can't work with mix versions.

If I have run 0.12upgrade one time, remove versions.tf, and add some codes with old format, if I run 0.12upgrade again, it failed on new format.

Third, seems all sourced terraform modules need be 0.12 ready as well.

Warning: Deprecated ignore_changes wildcard

  on .terraform/modules/xxx/iam.tf line 14, in resource "aws_iam_policy" "publisher_service_policy":
  14:     ignore_changes = ["*"]

Error: Unsupported block type

  on .terraform/modules/xxx/alarms.tf line 67, in resource "aws_cloudwatch_metric_alarm" "lambda_error":
  67:   dimensions {

Blocks of type "dimensions" are not expected here. Did you mean to define
argument "dimensions"? If so, use the equals sign to assign it a value.

Can we have features to support both versions, 0.11x and 0.12.x in final version? With current way, terraform 0.12 is a totally break change, we'd better to rename it to a new name, more than use the exist name. or new MAJOR version

https://semver.org/

MAJOR version when you make incompatible API changes,

MINOR version when you add functionality in a backwards-compatible manner, and PATCHversion when you make backwards-compatible bug fixes. Additional labels for pre-release and build metadata are available as extensions to the MAJOR.MINOR.PATCH format.

[ozbillwang]

Because terraform 0.12 is break change, I am worried about our CICD pipeline.

I do find a lot of terraform repo running currently without limit terraform version in provider aws, such as ~> 0.11, it will be problem if new terraform 0.12 is released formally.

It will change the tfstate file to up version and we need manual job to roll it back.

[mildwonkey]

Hi @ozbillwang !

Thank you for opening this issue. You've brought up a few (very reasonable) concerns and I will try to address them all here.

The first concern, and subject of this issue, is terraform fmt behavior in 0.12. I've confirmed that this issue is still present in the master branch, which has several updates since the beta was released, so I will flag this as a bug.

---

It is indeed important that you specify terraform and terraform provider versions to avoid unexpected upgrades. Your example version constraint, ~> 0.11, is a pessimistic constraint operator which is the same as >= 0.11, < 0.12, so (at least in this example) you do not need to worry about unexpected upgrades to terraform 0.12 when it is released.

I understand your concerns that terraform isn't following semantic versioning. There is a well-written comment on a similar issue that covers our reasoning much better than I can express myself. The very short version is that we take the idea of a 1.0 release very seriously and we're not there yet.

You are correct in your observation that 0.12upgrade can't work with mixed versions. You will need to update all of your terraform configuration files at once, or split them up into separate directories so you can upgrade them individually before putting them back into one directory. The reason for this is that an early step of 0.12upgrade is to load and validate all the configuration with the HCL (terraform 0.11) parser. It cannot parse HCL2/ 0.12 configuration. It will likewise not be possible to support mixed 0.11 and 0.12 syntax.

As a workaround, you can use module versioning to pin 0.11-compatible modules to 0.11-compatible terraform configuration files, and 0.12 modules to 0.12-compatible terraform configuration files. (From your comments I see you are already familiar with this best practice, so pardon me for repeating the obvious)

We have a preliminary 0.12 upgrade guide that also addresses many of these points.

I hope that you find the enhancements in terraform 0.12 worth the hassle, and I do understand your concerns and frustration. Thank you for opening this issue, and thank you for your help evaluating the terraform 0.12 beta release.

[apparentlymart]

r.e. the terraform fmt report:

The requirement that the one-line block syntax can only be used with a single attribute is an intentional change in Terraform v0.12, which the 0.12upgrade tool should fix automatically by rewriting the input like this:

terraform {
  backend "s3" {}
}

In the Terraform v0.12 language, whitespace is slightly more significant than it was before, both in order to support the "first class expressions" syntax (which would otherwise have introduced some ambiguity into the language) and to place only one statement on each line for more readability. The only cases where a block can be written all on one line are when it is entirely empty or when it contains only a single attribute definition:

variable "foo" {}
variable "bar" { type = string }

Placing a block inside another block on the same line is a syntax error in the new version of the language, so the terraform fmt command is rejecting it at parse time, before it begins formatting. This is unlikely to change prior to the v0.12.0 release, though we may consider adding a more relaxed parsing mode for terraform fmt in a future version that would ignore that sort of whitespace-related error and thus give the formatter a chance to fix it automatically. In the immediate term though, it is a bug that the error message from terraform fmt doesn't include a snippet of the source code of the file in question.

(At the moment the formatter does not fix newline-related layout. The v0.12.0 formatter is a lot more conservative than it was in prior versions; we decided to focus only on simple indentation and spacing adjustments for the first release because the v0.11 formatter was far too complicated and tended to make certain complex configurations worse, and so we wanted to start with simple rules and carefully introduce more complex ones as we learn from real-world usage.)

[mildwonkey]

Apologies - I just checked and terraform 0.12upgrade does indeed catch and fix this.

[syl20bnr]

@apparentlymart

As Terraform is configuration variables heavy by nature we were used to declare all variables on single lines with proper alignment so it is both condensed and readable:

variable "bar" { type = "list" default = [] }

Now terraform 0.12 forces us to make exceptions for some variables with default values and we loose in readability and formatting consistency.

Do you think it would be possible to add to the grammar some delimitation character to be able to still have single lines ?

variable "bar" { type = list(string); default = [] }

[apparentlymart]

Hi @syl20bnr,

I understand that you had developed a local style of putting the type and default on the same line. The lax parsing in Terraform 0.11 allowed for some different styles to develop, and so the change to a more restrictive model was inevitably going to require changes for some users.

We do not have plans to introduce more variants for declaring blocks. Formatting decisions are of course subjective, but we think it's important to have one consistent way to format Terraform configurations rather than every configuration having its own different way to do it. Language design is all tradeoffs, and this is one that I know not everyone will agree with, but the general rule for the new language syntax is: one piece of information per line. Since the variable type and the default are two separate pieces of information, they get one line each so that it's easier to scan vertically and see them both.

[syl20bnr]

I understand the design choice. I hope you'll be in a position at some point where you can offer this flexibility as this is very clean to have single line variables in Terraform, visually but also practically where grep like command results are cleaner.

I don't like long lines but was happy to make the trade offf in the case of Terraform.

For reference here is an example with both formatting. Some people will like more the vertical option although it has only the advantage to have shorter lines. In my opinion the vertical way only make access to the information and readability harder as it is harder to parse by human eyes.

Terraform 11:

# Variables for gcp/google/network_services/public_https_to_http_load_balancer module

# mandatory ------------------------------------------------------------------
variable "env_long"              { }
variable "env_short"             { }
variable "firewall_rules_prefix" { }
variable "group"                 { }
variable "health_checks_cidrs"   { type = "list" }
variable "instance_group"        { }
variable "network_link"          { }
variable "project"               { }
variable "ssl_cert_link"         { }
variable "xpn_host_project"      { }
variable "zone"                  { }

# optional -------------------------------------------------------------------
variable "backend_service_timeout_sec"      { default = 60 }
variable "health_check_healthy_threshold"   { default = 1 }
variable "health_check_interval_sec"        { default = 2 }
variable "health_check_port"                { default = 80 }
variable "health_check_timeout_sec"         { default = 2 }
variable "health_check_unhealthy_threshold" { default = 3 }
variable "health_request_path"              { default = "/" }
variable "iap_config"                       {type = "list" default = []}
variable "number"                           { default = "1" }
variable "role"                             { default = "def" }

Terraform 12:

# Variables for gcp/google/network_services/public_https_to_http_load_balancer module

# mandatory ------------------------------------------------------------------
variable "env_long" {
}

variable "env_short" {
}

variable "firewall_rules_prefix" {
}

variable "group" {
}

variable "health_checks_cidrs" {
  type = list(string)
}

variable "instance_group" {
}

variable "network_link" {
}

variable "project" {
}

variable "ssl_cert_link" {
}

variable "xpn_host_project" {
}

variable "zone" {
}

# optional -------------------------------------------------------------------
variable "backend_service_timeout_sec" {
  default = 60
}

variable "health_check_healthy_threshold" {
  default = 1
}

variable "health_check_interval_sec" {
  default = 2
}

variable "health_check_port" {
  default = 80
}

variable "health_check_timeout_sec" {
  default = 2
}

variable "health_check_unhealthy_threshold" {
  default = 3
}

variable "health_request_path" {
  default = "/"
}

variable "iap_config" {
  type    = list(string)
  default = []
}

variable "number" {
  default = "1"
}

variable "role" {
  default = "def"
}

[apparentlymart]

Terraform does allow empty blocks and single-argument blocks to be given on one line, as a compromise to allow a more compact form for simple blocks. In that example, variable "iap_config" is the only one that Terraform 0.12 will require to be spread over multiple lines, because it has two arguments inside it:

# mandatory ------------------------------------------------------------------
variable "env_long" {}
variable "env_short" {}
variable "firewall_rules_prefix" {}
variable "group" {}
variable "health_checks_cidrs" { type = list(string) }
variable "instance_group" {}
variable "network_link" {}
variable "project" {}
variable "ssl_cert_link" {}
variable "xpn_host_project" {}
variable "zone" {}

# optional -------------------------------------------------------------------
variable "backend_service_timeout_sec" { default = 60 }
variable "health_check_healthy_threshold" { default = 1 }
variable "health_check_interval_sec" { default = 2 }
variable "health_check_port" { default = 80 }
variable "health_check_timeout_sec" { default = 2 }
variable "health_check_unhealthy_threshold" { default = 3 }
variable "health_request_path" { default = "/" }
variable "iap_config" {
  type    = list(string)
  default = []
}
variable "number" { default = "1" }
variable "role" { default = "def" }

The terraform 0.12upgrade command is forced to be more opinionated than terraform fmt is because converting between what is internally two entirely separate language implementations is lossy. terraform fmt will not itself insert any newlines into these blocks in 0.12, so if you wish you can manually revert them to being single-line (with the exception of variable "iap_config") and leave them that way. terraform fmt in 0.12 is primarily concerned with horizontal spacing for alignment and indentation.

[syl20bnr]

@apparentlymart thank you for the reply.

with the exception of variable "iap_config"

This is precisely my issue here as stated in this comment: https://github.com/hashicorp/terraform/issues/20910#issuecomment-495639420

I hope that you will be able to provide this flexibility at some point for variable with an explicit type and default values because it is very valuable to be able to have everything on the same line. As I stated terraform is variables heavy and be able to quickly search for information is important.

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