terraform icon indicating copy to clipboard operation
terraform copied to clipboard

stripmarkers and indented heredoc

Open bentterp opened this issue 4 years ago • 9 comments

Terraform Version

0.12.18

Terraform Configuration Files

locals {
  hosts = [
    {private_ip: "10.0.1.4", name: "server1"},
    {private_ip: "10.0.2.4", name: "server2"},
    {private_ip: "10.0.3.4", name: "server3"}
  ]
}

output "missing_strip" {
  value = <<-EOT
    %{ for server in local.hosts }
    ${server.private_ip} ${server.name}
    %{ endfor ~}
    EOT
}

output "missing_indent" {
  value = <<-EOT
    %{ for server in local.hosts ~}
    ${server.private_ip} ${server.name}
    %{ endfor }
    EOT
}

Debug Output

Crash Output

Expected Behavior

According to the documentation, it should not matter if the strip marker is placed in the for directive or the endfor directive.

Actual Behavior

When the strip marker is put in the for directive, the indentation of the heredoc is not removed as expected but the newline is removed. When the strip marker is put in the endfor directive, the indentation of the heredoc is removed as expected, but not the newline.

Steps to Reproduce

terraform apply

Additional Context

References

bentterp avatar Dec 18 '19 07:12 bentterp

Having the strip marker in both places doesn't work, either

bentterp avatar Dec 18 '19 07:12 bentterp

Thanks for reporting this, @bentterp.

Indeed, it does seem like the strip markers and the flush-parsing mode are interacting poorly here. This seems to be a bug upstream in HCL, and seems to be caused by the strip marker changing the resulting tokens during parsing in a way that the flush mode processing can't handle. Specifically, flush mode expects to find a newline before each "indentation", but I think the strip marker is stripping away that newline during parsing so that flush mode then doesn't understand the following indent as being an indent.

I think the root problem here is that the strip marker handling is a little flawed: it's stripping leading whitespace from the next literal token, but that's not sufficient in this case because the whitespace is actually split over two tokens, like this:

  • TokenTemplateSeqEnd (the closing ~})
  • TokenStringLit "\n"` (the newline)
  • TokenStringLit (the four spaces of indent)
  • TokenTemplateInterp (the opening ${)

The strip marker removes the newline, but it doesn't remove the four spaces of indent because they are in a separate token.

If I'm right about this cause, I think the fix would be for the template parser to have an additional case in its handling of right strip markers where if the next token it encounters is entirely whitespace (that is, if the string is empty after removing all of the whitespace from the front) then to leave the "left strip" flag true so that it can continue stripping whitespace on subsequent tokens until it finds either a non-literal or a literal containing at least one non-whitespace character.

We'll need to fix this with some care because although the current behavior is not following the HCL spec I expect there are some folks relying on the current buggy implementation nonetheless, and so changing it will be impactful to those users. For Terraform in particular, heredoc templates are commonly used to populate arguments like user_data on aws_instance where changes would require replacement. However, there is some pressure the other way too: the longer this bug remains in the Go HCL implementation the more other applications will end up including it and thus the harder it will be to change.

apparentlymart avatar Jun 30 '20 16:06 apparentlymart

I expect there are some folks relying on the current bugging implementation nonetheless, and so changing it will be impactful to those users

What about introducing a new strict indented heredoc variant, e.g. <<=TOKEN or <<~TOKEN to avoid breaking existing configurations and let users opt-in with the new syntax?

pdecat avatar Jun 30 '20 17:06 pdecat

There are indentation-related inconsistencies even with regular heredocs. The following output different values even though as I read it they should produce the same result:

output "hello_heredoc" {
  value = <<EOF
Hello,
%{~ if "" == "" ~}
 world
%{~ endif ~}
!
EOF
}

output "hello_literal" {
  value = "Hello,\n%{~ if "" == "" ~}\n world\n%{~ endif ~}\n!"
}

Output:

$ terraform apply

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

Outputs:

hello_heredoc = Hello, world!

hello_literal = Hello,world!

kbolino avatar Jul 24 '20 16:07 kbolino

I'm having the same issue here: a normal heredoc and stripmarkers are behaving differently now than they used to. Here's an example from Terraform: Up & Running:

variable "names" {
  description = "Names to render"
  type        = list(string)
  default     = ["neo", "trinity", "morpheus"]
}

output "for_directive_strip_marker" {
  value = <<EOF
%{~ for name in var.names }
  ${name}
%{~ endfor }
EOF
}

With TF 0.12 or so, it used to output:

neo
trinity
morpheus

Now, with TF 1.1, the output has extra leading and trailing whitespace:


neo
trinity
morpheus

I had automated tests for this that used to pass and are failing now... And I can't find any way to fix it. It seems like HEREDOC doesn't let you put text on the same line as the HEREDOC marker, but the stripmarker no longer gets rid of those extra newlines.

brikis98 avatar Feb 25 '22 10:02 brikis98

Hi,

I can confirm the issue is still present on Terraform v1.1.2:

locals {
    mylist = [
        "item1",
        "item2",
        "item3"
    ]
}

output "template" {
    value = <<-EOF
        %{for item in local.mylist ~}
        ${item}
        %{endfor~}
    EOF
}

Results show that the indentation is not stripped as expected:

template = <<EOT
        item1
        item2
        item3

EOT

I tried to combine this with trimspace but it did not work.

I finally gave up and just use the following syntax:

locals {
    mylist = [
        "item1",
        "item2",
        "item3"
    ]
}

output "template" {
    value = <<EOF
%{for item in local.mylist ~}
${item}
%{endfor~}
EOF
}

Results:

template = <<EOT
item1
item2
item3

EOT

Not really a workaround because it doesn't makes the code more readable but I couldn't find a better solution.

src386 avatar Mar 10 '22 11:03 src386

@apparentlymart is there a reason why this still isn't fixed?

stevehipwell avatar Oct 17 '22 16:10 stevehipwell

The current behavior is protected by the v1.x compatibility promises and so we cannot change the existing behavior without introducing new syntax to opt into it.

Therefore this requires further design work. Nobody on the team at HashiCorp is currently working on this.

apparentlymart avatar Oct 17 '22 21:10 apparentlymart

@apparentlymart this was reported and a known issue well before the Terraform v1 compatibility promise was even a potential deliverable. This defect does not follow the HCL language spec, does not follow the Terraform documented behaviour and the actual behaviour is not documented or easily discoverable. What this defect actually means is; 1 that it's impossible to template inside an indented heredoc string, and 2 that templating in an un-indented heredoc string will result in additional whitespace no matter what.

Could this (and the boolean operator short-circuit issue) not be fixed by adding a new opt-in field in the terraform block?

If you're not going to fix this then it needs to be documented with big warning signs as it regularly causes significant time wasting as someone tries to get the documented behaviour to happen.

stevehipwell avatar Oct 18 '22 11:10 stevehipwell

Facing the same issue when using <<-EOF for a terraform template to initialize configs in ECS. Everything works as expected until it enters the for loop, then the whitespaces/newlines stripping is not applied.

danlsgiga avatar Oct 31 '22 17:10 danlsgiga