Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"terraform fmt" doesn't format correctly in presence of here-docs #21434

Closed
multani opened this issue May 24, 2019 · 29 comments · Fixed by #22209
Closed

"terraform fmt" doesn't format correctly in presence of here-docs #21434

multani opened this issue May 24, 2019 · 29 comments · Fixed by #22209

Comments

@multani
Copy link
Contributor

multani commented May 24, 2019

Terraform Version

$ terraform -v
Terraform v0.12.0

Terraform Configuration Files

module "foo" {
foo = <<EOF
5
EOF
}

module "x" {
a = "b"
abcde = "456"
}

Expected Behavior

I expect terraform fmt to reformat and align the code like this:

module "foo" {
  foo = <<EOF
5
EOF
}

module "x" {
  a     = "b"
  abcde = "456"
}

Actual Behavior

Instead, the = signs are not aligned correctly:

module "foo" {
  foo = <<EOF
5
EOF
}

module "x" {
  a = "b"
  abcde = "456"
}

Steps to Reproduce

Create a Terraform file with the content above and run terraform fmt.

Additional Context

If I remove the here-doc variable, this formats the file as expected, and like Terraform < 0.12.0 used to do.

@donovan
Copy link

donovan commented May 27, 2019

Can confirm this is affecting our code base after migration to 0.12.0. All files with here-docs are not formatted after the here-doc when running fmt.

@chris-brace
Copy link

just wanted to mention that this appears to have been discussed before but the issue was closed with no resolution

#20563

@chris-brace
Copy link

In the mean time i've just put the heredocs at the bottom of the file as locals and referenced them above, because the formatter only breaks after the terminator

@nvtkaszpir
Copy link

would be nice to have any option to tell terraform fmt to ignore given lines/blocks

@ryanhartkopf
Copy link

I'm seeing a similar issue with heredoc formatting, except the formatter will indent all lines after EOF by 4 spaces.

Every subsequent heredoc will indent the text after it by another 4 spaces. This is leading to big ugly diffs after running terraform 0.12upgrade.

 resource "aws_sqs_queue" "this" {
-  count = "${local.create_this_sqs_queue == 1 ? length(var.queues) : 0}"
-
-  name                       = "ph-${var.ph_environment}-${var.queues[count.index]}"
-  max_message_size           = "${var.max_message_size}"
-  message_retention_seconds  = "${var.message_retention_seconds}"
-  receive_wait_time_seconds  = "${var.receive_wait_time_seconds}"
-  visibility_timeout_seconds = "${lookup(var.visibility_timeout_seconds, var.queues[count.index], var.visibility_timeout_seconds_default)}"
+  count = local.create_this_sqs_queue == 1 ? length(var.queues) : 0
+
+  name                      = "ph-${var.ph_environment}-${var.queues[count.index]}"
+  max_message_size          = var.max_message_size
+  message_retention_seconds = var.message_retention_seconds
+  receive_wait_time_seconds = var.receive_wait_time_seconds
+  visibility_timeout_seconds = lookup(
+    var.visibility_timeout_seconds,
+    var.queues[count.index],
+    var.visibility_timeout_seconds_default,
+  )
 
   redrive_policy = <<EOF
 {
   "deadLetterTargetArn": "${element(aws_sqs_queue.deadletter.*.arn, count.index)}",
-  "maxReceiveCount": ${lookup(var.max_receive_count, var.queues[count.index], var.max_receive_count_default)}
+  "maxReceiveCount": ${lookup(
+var.max_receive_count,
+var.queues[count.index],
+var.max_receive_count_default,
+)}
 }
 EOF
-}

-resource "aws_sqs_queue" "this_with_kms" {
-  count = "${local.create_this_sqs_queue == 0 ? length(var.queues) : 0}"
-
-  name                              = "ph-${var.ph_environment}-${var.queues[count.index]}"
-  max_message_size                  = "${var.max_message_size}"
-  message_retention_seconds         = "${var.message_retention_seconds}"
-  receive_wait_time_seconds         = "${var.receive_wait_time_seconds}"
-  visibility_timeout_seconds        = "${lookup(var.visibility_timeout_seconds, var.queues[count.index], var.visibility_timeout_seconds_default)}"
-  kms_master_key_id                 = "${var.kms_master_key_id}"
-  kms_data_key_reuse_period_seconds = "${var.kms_data_key_reuse_period_seconds}"
+    }
 
-  redrive_policy = <<EOF
+    resource "aws_sqs_queue" "this_with_kms" {
+      count = local.create_this_sqs_queue == 0 ? length(var.queues) : 0
+
+      name                      = "ph-${var.ph_environment}-${var.queues[count.index]}"
+      max_message_size          = var.max_message_size
+      message_retention_seconds = var.message_retention_seconds
+      receive_wait_time_seconds = var.receive_wait_time_seconds
+      visibility_timeout_seconds = lookup(
+        var.visibility_timeout_seconds,
+        var.queues[count.index],
+        var.visibility_timeout_seconds_default,
+      )
+      kms_master_key_id                 = var.kms_master_key_id
+      kms_data_key_reuse_period_seconds = var.kms_data_key_reuse_period_seconds
+
+      redrive_policy = <<EOF
 {
   "deadLetterTargetArn": "${element(aws_sqs_queue.deadletter_with_kms.*.arn, count.index)}",
-  "maxReceiveCount": ${lookup(var.max_receive_count, var.queues[count.index], var.max_receive_count_default)}
+  "maxReceiveCount": ${lookup(
+var.max_receive_count,
+var.queues[count.index],
+var.max_receive_count_default,
+)}
 }
 EOF
-}

-resource "aws_sqs_queue" "deadletter" {
-  count = "${local.create_this_sqs_queue == 1 ? length(var.queues) : 0}"
-
-  name                       = "ph-${var.ph_environment}-${var.queues[count.index]}-deadletter"
-  max_message_size           = "${var.max_message_size}"
-  message_retention_seconds  = "${var.message_retention_seconds}"
-  receive_wait_time_seconds  = "${var.receive_wait_time_seconds}"
-  visibility_timeout_seconds = "${var.visibility_timeout_seconds_default}"
-}
+        }
+
+        resource "aws_sqs_queue" "deadletter" {
+          count = local.create_this_sqs_queue == 1 ? length(var.queues) : 0
+
+          name                       = "ph-${var.ph_environment}-${var.queues[count.index]}-deadletter"
+          max_message_size           = var.max_message_size
+          message_retention_seconds  = var.message_retention_seconds
+          receive_wait_time_seconds  = var.receive_wait_time_seconds
+          visibility_timeout_seconds = var.visibility_timeout_seconds_default
+        }

@luhn
Copy link

luhn commented Jun 4, 2019

Adding additional heredocs causes the formatting to change further. Eventually it will wrap back around to the correct formatting, which I think is hilarious.

It appears the pattern is:

  1. Correct formatting
  2. Equals signs are not aligned
  3. Indentation is ignored (both correct and incorrect number of spaces is left as-is)
  4. Both 2 and 3
  5. Back to correct
module "test1" {
  foo = <<EOF
5
EOF
}

module "test2" {
  a = "b"
  abcde = "456"
  foo = <<EOF
5
EOF
}

module "test3" {
a     = "b"
abcde = "456"
foo   = <<EOF
5
EOF
}

module "test4" {
a = "b"
abcde = "456"
foo = <<EOF
5
EOF
}

module "test5" {
  a     = "b"
  abcde = "456"
  foo   = <<EOF
5
EOF
}

module "test6" {
  a = "b"
  abcde = "456"
  foo = <<EOF
5
EOF
}

module "test7" {
a     = "b"
abcde = "456"
foo   = <<EOF
5
EOF
}

module "test8" {
  a = "b"
  abcde = "456"
  foo = <<EOF
5
EOF
}

module "test9" {
  a     = "b"
  abcde = "456"
  foo   = <<EOF
5
EOF
}

@manfredlift
Copy link

manfredlift commented Jun 10, 2019

Same issue here. In presence of here-docs equal signs are not aligned. This causes our CI/CD pipeline to fail when running terraform fmt -check -diff when the format of the file has been manually fixed.

As a workaround, putting all here-docs at the end of the file in locals works for me.

@mschuchard
Copy link
Contributor

mschuchard commented Jun 11, 2019

Can confirm this issue is still present in 0.12.2 and that 0.11.14 works correctly.

@rporres
Copy link

rporres commented Jul 3, 2019

This still happens in 0.12.3

@camerondavison
Copy link

this bug has been open a for a while, hope it is not too hard to fix. I thought that with 3 patch versions it would be safe to try to upgrade, but got bit by this. after running terraform 0.12upgrade the formatting in my files are all messed up.

@querry43
Copy link

querry43 commented Jul 9, 2019

@camerondavison, I was able to work around this by using locals which probably made the code more readable anyhow. Maybe that will help?

@luhn
Copy link

luhn commented Jul 9, 2019

@querry43, can you clarify how you worked around it with locals?

Edit: Answered up here #21434 (comment)

@camerondavison
Copy link

Not sure that makes sense for me. Im using terraform to provision datadog monitoring and it would be best to have the multiline message in the monitor rather than having to find the local for the message for every monitor.

@multani
Copy link
Contributor Author

multani commented Jul 9, 2019

There's apparently a fix in hashicorp/hcl2#107, this would need to get merged first and then backported into Terraform.

@querry43
Copy link

querry43 commented Jul 9, 2019

@querry43, can you clarify how you worked around it with locals?

Edit: Answered up here #21434 (comment)

@luhn , it is a fairly specific case. The HEREDOC was breaking because of a ${function call} in it. Pulling that out into a local appeared to make the problem go away.

locals {
  kibana_host = replace(aws_elasticsearch_domain.elasticsearch_domain.kibana_endpoint, "/\\/.*/", "")
}

...

  environment = <<EOF
[
  { "name" : "ADMIN_USER", "value" : "${data.aws_ssm_parameter.elasticsearch_admin_user.value}" },
  { "name" : "ADMIN_PASSWORD", "value" : "${data.aws_ssm_parameter.elasticsearch_admin_password.value}" },
  { "name" : "RESOLVER_IP", "value" : "169.254.169.253" },
  { "name" : "SERVICE_URL", "value" : "http://${local.kibana_host}" }
]
EOF

@luhn
Copy link

luhn commented Jul 9, 2019

The HEREDOC was breaking because of a ${function call} in it.

You must be talking about a different issue. The issue in question occurs regardless of the contents of the heredoc. (OP's example just has "5" as the contents of the heredoc.)

@asgoel
Copy link

asgoel commented Jul 17, 2019

Any update on this one? It's quite annoying to have to put all HEREDOCs at the bottom as locals.

@OJFord
Copy link
Contributor

OJFord commented Jul 23, 2019

Fixed upstream: hashicorp/hcl2#107

pselle pushed a commit to pselle/terraform that referenced this issue Jul 23, 2019
@pselle
Copy link
Contributor

pselle commented Jul 24, 2019

I tried upgrading HCL to incorporate the ☝️ mentioned fix, but it doesn't fix the issue described here, and it looks as though some further investigation is needed.

@OJFord
Copy link
Contributor

OJFord commented Jul 24, 2019

Damn, thanks for trying, @pselle!

I feel compelled to mention, in receipt of so much ❤️ , that the mentioned fix is due to @nozaq, and I claim no credit!

One other thing to note in addition to the 'locals at the bottom' workaround, this prompted me to discover data.aws_iam_policy_document, which is a jolly nice way to get rid of most of my use of terraform heredocs anyway!

I also replaced some command heredocs with join(" && ", []) which I think reads quite well. To be honest, I wish such resources/provisioners would admit lists for such commands anyway.

@ibrahima
Copy link

Oh nice, I did not know about aws_iam_policy_document, that would indeed help with most of my heredocs.

@nozaq
Copy link
Contributor

nozaq commented Jul 24, 2019

@pselle @OJFord

Hi, just notified of this thread!

I've tried cases described here with the current master which already has my PR merged.
Belows are results of go run ./cmd/hclfmt/main.go in hcl2 repo and they seem correct.

Is there specific example working incorrectly? I'm happy to dig into it further :)

Case 1 (Original Post)

Input

module "foo" {
foo = <<EOF
5
EOF
}

module "x" {
a = "b"
abcde = "456"
}

Output

module "foo" {                                                                                                                  
  foo = <<EOF
5               
EOF      
}              
             
module "x" {
  a = "b"
  abcde = "456"
}

Case 2(@luhn's post)

Input

module "test1" {
  foo = <<EOF
5
EOF
}

module "test2" {
  a = "b"
  abcde = "456"
  foo = <<EOF
5
EOF
}

module "test3" {
a     = "b"
abcde = "456"
foo   = <<EOF
5
EOF
}

module "test4" {
a = "b"
abcde = "456"
foo = <<EOF
5
EOF
}

module "test5" {
  a     = "b"
  abcde = "456"
  foo   = <<EOF
5
EOF
}

module "test6" {
  a = "b"
  abcde = "456"
  foo = <<EOF
5
EOF
}

module "test7" {
a     = "b"
abcde = "456"
foo   = <<EOF
5
EOF
}

module "test8" {
  a = "b"
  abcde = "456"
  foo = <<EOF
5
EOF
}

module "test9" {
  a     = "b"
  abcde = "456"
  foo   = <<EOF
5
EOF
}

Output

module "test1" {
  foo = <<EOF
5
EOF
}

module "test2" {
  a = "b"
  abcde = "456"
  foo = <<EOF
5
EOF
}

module "test3" {
  a     = "b"
  abcde = "456"
  foo   = <<EOF
5
EOF
}

module "test4" {
  a = "b"
  abcde = "456"
  foo = <<EOF
5
EOF
}

module "test5" {
  a     = "b"
  abcde = "456"
  foo   = <<EOF
5
EOF
}

module "test6" {
  a = "b"
  abcde = "456"
  foo = <<EOF
5
EOF
}

module "test7" {
  a     = "b"
  abcde = "456"
  foo   = <<EOF
5
EOF
}

module "test8" {
  a = "b"
  abcde = "456"
  foo = <<EOF
5
EOF
}

module "test9" {
  a     = "b"
  abcde = "456"
  foo   = <<EOF
5
EOF
}

@luhn
Copy link

luhn commented Jul 24, 2019

Thanks for your work, @nozaq!

It does look like the indentation problems are fixed with your PR, but the equals sign alignment remains an issue.

@nozaq
Copy link
Contributor

nozaq commented Jul 24, 2019

@luhn Thanks for the clarification!
Understood, I'll look into this as well :)

apparentlymart pushed a commit to hashicorp/hcl2 that referenced this issue Jul 25, 2019
 Waiting for TokenCHeredoc never ends since scanTokens() does not
 produce
 TokenNewlines inside heredocs.

 Related Issue: hashicorp/terraform#21434
@nozaq
Copy link
Contributor

nozaq commented Jul 25, 2019

The fix was merged into upstream now :)

#21434

@shantanugadgil
Copy link

shantanugadgil commented Jul 26, 2019

This is going to make a lot of folks (including me) very very happy. 🕺

I've had to write special scripts to fmt only specific files from my sources since this bug had "crept" in.

Would you be willing to consider releasing a point release just for this instead of the regular cadence?

@pselle
Copy link
Contributor

pselle commented Jul 26, 2019

@shantanugadgil We are not planning to release outside of the our flow, this will go out with the next Terraform release.

@pselle
Copy link
Contributor

pselle commented Aug 20, 2019

This was released in 0.12.6: https://github.com/hashicorp/terraform/blob/master/CHANGELOG.md#0126-july-31-2019

@ghost
Copy link

ghost commented Aug 25, 2019

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.

@ghost ghost locked and limited conversation to collaborators Aug 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.