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

stripmarkers and indented heredoc #23710

Open
bentterp opened this issue Dec 18, 2019 · 11 comments
Open

stripmarkers and indented heredoc #23710

bentterp opened this issue Dec 18, 2019 · 11 comments
Labels
bug config confirmed a Terraform Core team member has reproduced this issue documentation

Comments

@bentterp
Copy link

bentterp commented Dec 18, 2019

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
Copy link
Author

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

@apparentlymart
Copy link
Contributor

apparentlymart commented Jun 30, 2020

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 apparentlymart added the confirmed a Terraform Core team member has reproduced this issue label Jun 30, 2020
@pdecat
Copy link
Contributor

pdecat commented Jun 30, 2020

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?

@kbolino
Copy link

kbolino commented Jul 24, 2020

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!

@brikis98
Copy link
Contributor

brikis98 commented Feb 25, 2022

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.

@src386
Copy link

src386 commented Mar 10, 2022

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.

innovate-invent pushed a commit to brinkmanlab/galaxy-container that referenced this issue Sep 23, 2022
innovate-invent pushed a commit to brinkmanlab/galaxy-container that referenced this issue Sep 23, 2022
@stevehipwell
Copy link

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

@apparentlymart
Copy link
Contributor

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.

@stevehipwell
Copy link

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

@danlsgiga
Copy link

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.

@kbolino
Copy link

kbolino commented Oct 29, 2023

I think it would be better to change some userdata scripts and cause some EC2 instances to have deltas in a plan, than to keep a confusingly broken templating system as a core "compatibility guarantee" for the language. If people have set up their IaC to auto-approve all plans and also haven't set prevent_destroy on affected resources, then they should be anticipating and ready to handle changes like this (and if not, well, how far are you going to go to hold the hand of people doing the wrong thing on multiple levels?).

It's also quite possible that, like with the Go loopvar behavior change, the vast majority of userdata scripts (and similar) that would be changed should change because they're already broken (not doing what was intended) but the faults are being silently ignored. After all, whitespace is significant in a lot of languages, including shell and YAML.

Moreover, the actual text of the compatibility promise seems to contradict leaving this unfixed:

"If the actual behavior of a feature differs from what we explicitly documented as the feature's behavior, we will usually treat that as a bug and change the feature to match the documentation, although we will avoid making such changes if they seem likely to cause broad compatibility problems. We cannot promise to always remain "bug-compatible" with previous releases, but we will consider such fixes carefully to minimize their impact."

As it stands, the only place the current behavior seems to be documented "officially" is this issue. The docs for Strings and Templates don't mention any caveats about indented heredocs and templates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug config confirmed a Terraform Core team member has reproduced this issue documentation
Projects
None yet
Development

No branches or pull requests

10 participants