hashicorp / terraform

Terraform enables you to safely and predictably create, change, and improve infrastructure. It is a source-available tool that codifies APIs into declarative configuration files that can be shared amongst team members, treated as code, edited, reviewed, and versioned.
https://www.terraform.io/
Other
42.41k stars 9.5k forks source link

indent function should not add spaces after final newline #29508

Open tmshn opened 3 years ago

tmshn commented 3 years ago

Currently indent function adds whitespaces after every newline character \n.

For example

aaa⏎
bbb⏎
ccc⏎

would be

aaa⏎
__bbb⏎
__ccc⏎
__

However, newline character is the end of the line, not the beginning (ref: posix document). In that sense, adding something after the final newline character means adding extra thing after the final line, which we haven't expected.

And in fact, most of multi-line string ends with newline character, including the one read using file() function or written using here-doc literal <<EOS ... EOS in terraform configuration.

The ideal output is

aaa⏎
__bbb⏎
__ccc⏎

Current Terraform Version

$ terraform -version
Terraform v1.0.5
on linux_amd64

Use-cases

Render multi-line string inside of terraform. My current use case is filter expression for Google Cloud logging sink, but not limited to that.

Attempted Solutions

Instead of doing " ${indent(2, var.filter1)}", I do " ${indent(2, chomp(var.filter))}\n"

Proposal

Just not add spaces after final newline.

References

apparentlymart commented 3 years ago

Hi @tmshn! Thanks for reporting this.

I agree with your argument here that the behavior of this function is strange. The current implementation is very simple: it's just inserting a set of spaces after each newline, and so if you have two consecutive newlines (whether at the end of the string or not) then they will have spaces added between them, as you've seen.

With that said, we cannot change the function now because it's covered by the Terraform v1.0 Compatibility Promises: changing the result of this function for existing modules could change their behavior and, depending on how the author was using indent, potentially cause providers to propose to replace an important object.

This is a frustrating contradiction because the fact that we've had this function implemented in this way for so many years and only now someone observed this strange behavior suggests that this isn't a widely-used function, and thus it's hard to justify adding more complexity in order to support both the old behavior and the new behavior, but also we don't have access to any real data on how this function is used and thus we are constrained by compatibility promises not to change it.

Given that, honestly my initial instinct is to leave it as-is and perhaps document your solution of using chomp on the input, because while not ideal that answer strikes a compromise of not adding additional complexity for an assumed-rarely-used function and retaining compatibility with existing modules which we can't see that might be using it.

tmshn commented 3 years ago

@apparentlymart

I haven't know that this function is so old...

I truly understand the importance of compatibility promise, so fixed just the docs without changing its behavior.

https://github.com/hashicorp/terraform/pull/29525

crw commented 6 months ago

Thank you for your continued interest in this issue.

Terraform version 1.8 launches with support of provider-defined functions. It is now possible to implement your own functions! We would love to see this implemented as a provider-defined function.

Please see the provider-defined functions documentation to learn how to implement functions in your providers. If you are new to provider development, learn how to create a new provider with the Terraform Plugin Framework. If you have any questions, please visit the Terraform Plugin Development category in our official forum.

We hope this feature unblocks future function development and provides more flexibility for the Terraform community. Thank you for your continued support of Terraform!