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

Feature Request: terraform fmt applies sort to variables.tf and terraform.tfvars #12959

Closed
sysadmiral opened this issue Mar 22, 2017 · 35 comments

Comments

@sysadmiral
Copy link

It would be useful imho for terraform fmt to also do an alphabetical sort on variable definition files, namely variables.tf and terraform.tfvars.

@sysadmiral sysadmiral changed the title Feature Request: terraform fmt applies for to variables.tf and terraform.tfvars Feature Request: terraform fmt applies sort to variables.tf and terraform.tfvars Mar 22, 2017
@Gary-Armstrong
Copy link

Disagree.

@nateww
Copy link

nateww commented Mar 23, 2017

Disagree x 2

@spanktar
Copy link

We currently do this manually. While I agree with the dissenters, it would be excellent if this was an OPTION that could be enabled.

@dpgeekzero
Copy link

I'm curious, this sounds like a great feature. Why the disagreement?

@armenr
Copy link

armenr commented Oct 25, 2017

I'd buy it.

@armenr
Copy link

armenr commented Nov 21, 2017

I found a script somewhere on the webs that got me really close to what I needed, so I pretty much lifted that dude's code and made some minor changes - and now have a really, really, naive (and yeah, totally hacky) script to take care of automatically sorting terraform variables files.

See below, and please, if you have suggestions, I'd love for you to change the script and post back with what's more clever. I'll probably parameterize it so you pass the file you want sorted on the command line after calling the script. Tested on my own machine - works fine. Run at your own risk.

WARNING - If you've got ANY commented lines, descriptions inside the variable braces, or empty braces without any content initialized as variables, this will break your stuff! This only works when your variables look exactly like:

variable "testbed_vpc_name" {
  default = "testbed-vpc"
}
#!/bin/bash

# Get the number of lines in the document.
lines=$(cat variables.tf | wc -l)

# This is the starting range and end range. Each section is three lines.
x=1
y=3

until [ "$x" -gt "$lines" ]; do
    # Store the three lines to one line. 
    block=$(awk 'NR=="'"$x"'",NR=="'"$y"'"' variables.tf)
    # Echo each instance into my file. 
    # The $block variable is not double quotes so new lines are not honored. 
    echo $block >> tmp_vars.tf
    # Increment so it goes on to the next block.
    x=$((x+4)) 
    y=$((y+4)) 
done 

# Sort the output file in place by the second column.
sort -k2 tmp_vars.tf -o tmp_vars.tf
terraform fmt -write=true
mv tmp_vars.tf sorted_variables.tf

# Put it back into original formatting.
# while read i; do 
#     (echo "$i" | awk '{ print $1 " " $2 }'; echo "$i" | awk '{ print $3 }'; echo "$i" | awk '{ print $4 }'; echo "") >> final.txt
# done < output.txt

@apparentlymart
Copy link
Contributor

Hi all! Thanks for the discussion here.

This does seem like a reasonable helper-tool, though I'd be nervous about making it the default behavior for terraform fmt since it could make some pretty destructive changes given certain input, such as separating doc comments from what they are commenting, etc.

Sorting a hypothetical variables.tf file is not really straightforward because there's really nothing special about that file from Terraform's perspective... it's just another config file, which some users choose to use only for variable blocks. Some users call this vars.tf, and others mix variable blocks with output blocks in the same file, depending on what they are trying to achieve.

I could see us defining a "canonical sort" for a Terraform configuration file, such as:

  • The single terraform block, if present
  • The single atlas block, if present
  • variable blocks, alphabetized by name
  • provider blocks, alphabetized by name and alias
  • locals blocks, in some order (there isn't a well-defined sort for these, so not sure; maybe we'd just meld them all together into a single block and sort by key 🤷‍♂️)
  • data blocks, alphabetized by type and then name
  • module blocks, alphabetized by name
  • resource blocks, alphabetized by type and then name
  • output blocks, alphabetized by name

This would allow us to then have a tool that applies this sort to arbitrary config files, as long as the input complies with certain expectations, such as doc comments appearing immediately before whatever they belong to and having no other "unattached" comments at the top-level of the file.

(Much of the above ordering is arbitrary, just for illustration purposes; let's not bike-shed the specific ordering for the moment.)

The ordering of .tfvars files is easier because their top-level is all just user-supplied keys, which we could sort by name with the same constraints about comments.

Such a tool won't be a priority for us right now because we're in the middle of integrating a revamped config language parser and so this would end up needing to get rewritten against the new parser anyway, but once that's done we could think about how best to design a tool like this, how/whether it should integrate with terraform fmt etc.

In the mean time, having third-party tools to do this for subsets of valid Terraform configurations seems reasonable, with the caveat that they will probably need revision once the new configuration language parser is integrated since it includes new syntax elements that the current parser would choke on.

@Gary-Armstrong
Copy link

This is indeed a very good discussion. I will remain in the camp that organizes variables and other resources in more logical groupings by function rather than a lexical sorting. I would rather dev time be spent on making the fmt subcommand more flexible, since I don't like how it does some things but others are great.

It might meet certain requests if terraform state show is given additional display options. Not 100% related to sorting source code, but maybe 65% related in terms of finding things.

Ultimately I will also appreciate a greater amount of time spent on core and critical issues.

@robinbowes
Copy link

Here's a slightly less dirty hack than the previously-presented bash script.

#!/usr/bin/env python
# sort terraform variables

import sys
import re

# this regex matches terraform variable definitions
# we capture the variable name so we can sort on it
pattern = r'(variable ")([^"]+)(" {[^{]+})'


def process(content):
    # sort the content (a list of tuples) on the second item of the tuple
    # (which is the variable name)
    matches = sorted(re.findall(pattern, content), key=lambda x: x[1])

    # iterate over the sorted list and output them
    for match in matches:
        print ''.join(map(str, match))

        # don't print the newline on the last item
        if match != matches[-1]:
            print


# check if we're reading from stdin
if not sys.stdin.isatty():
    stdin = sys.stdin.read()
    if stdin:
        process(stdin)

# process any filenames on the command line
for filename in sys.argv[1:]:
    with open(filename) as f:
        process(f.read())

@armenr
Copy link

armenr commented Nov 27, 2017

@robinbowes - Well done. Thank you for this. Hell of a lot cleaner than what I'd ham-fistedly mashed together.

@pysysops
Copy link

pysysops commented Jan 14, 2019

This is old and closed but still found by Google searching. For anyone who stumbles on this, the Python code above works but will destroy any map variables you have. Change the pattern regex to this:

pattern = r'(variable ")([\w\d]+)(" {\n[\w\W]+?\n})'

Example:
https://www.regexpal.com/?fam=107028

Sure, there's probably a nicer / more optimal way but it works...

@robinbowes
Copy link

robinbowes commented Jan 14, 2019

Actually, this is probably the least hacky way to sort variables:

 json2hcl < <(jq . < <(hcltool variables.tf))

Or, if you prefer pipes:

hcltool variables.tf | jq . | json2hcl

@pysysops
Copy link

Nice @robinbowes

@smiller171
Copy link

I would love to see this as a non-default option in fmt, as well as the ability so sort parameters within resource blocks.

@tzaitsev
Copy link

tzaitsev commented Jul 26, 2019

Actually, this is probably the least hacky way to sort variables:

 json2hcl < <(jq . < <(hcltool variables.tf))

Or, if you prefer pipes:

hcltool variables.tf | jq . | json2hcl

At least with the versions of the libraries I tried this technique modifies and quotes the 'variable' so each one reads as "variable" in a .tf @robinbowes

@smiller171
Copy link

hcl2json doesn't support hcl2, so the fun workarounds are dead for me :(

@sblack4
Copy link

sblack4 commented May 18, 2020

I updated @robinbowes script for python3, https://gist.github.com/sblack4/34d74f6a4a6df65eb8d6e563a5135111

> python sort_terraform_variables.py variables.tf > sorted_variables.tf
> mv sorted_variables.tf variables.tf

@robinbowes
Copy link

terraform-config-inspect might also be worth a look.

@sblack4
Copy link

sblack4 commented May 18, 2020

terraform-config-inspect might also be worth a look.

It's cool and probably would be useful if I was handy in go but I need a quick fix. I ran it and here's an example output:

>  terraform-config-inspect                                                                                                                                                                                                                

# Module `.`

## Input Variables
* `name` (required): Moniker to apply to all resources in the module
* `tags` (required): User-Defined tags

## Output Values
* `tags_module`: Tags Module in it's entirety

## Child Modules
* `tags` from `rhythmictech/tags/terraform` (`1.0.0`)

@robinbowes
Copy link

Try it with the --json switch.

@ChristophShyper
Copy link

I would love to see this as a non-default option in fmt, as well as the ability so sort parameters within resource blocks.

Well, sorting parameters isn't good in my opinion. Some resources have a lot of them and I, personally, prefer to write them in the same order as described in documentation. So it's quicker to find any.

@robinbowes
Copy link

I would love to see this as a non-default option in fmt, as well as the ability so sort parameters within resource blocks.

Well, sorting parameters isn't good in my opinion. Some resources have a lot of them and I, personally, prefer to write them in the same order as described in documentation. So it's quicker to find any.

"in my opinion", "I, personally, prefer" - the discussion here is about adding an option so those who do want sorting can have it if they choose. You are free to not use the option.

@smiller171
Copy link

Agree with Robin. As long as you aren't changing default behavior adding these features hurts no one that doesn't want to use them. It's just a matter of engineering time/priorities to write the code, and I very much understand those constraints.

@ChristophShyper
Copy link

Totally agree as long as it's not a default behavior :)

@yermulnik
Copy link

I came up achieving the goal of sorting variables.tf with a simple awk script (which for most of the cases eliminates the need of external (non-base) deps like python, hcltool, et alia):
https://gist.github.com/yermulnik/7e0cf991962680d406692e1db1b551e6
Since this was more of an exercise out of curiosity, it was tested with GNU Awk 5.0.1, API: 2.0 (GNU MPFR 4.0.2, GNU MP 6.2.0) only, which I've got installed by default as /bin/awk on my Ubuntu 20.04.3 LTS.
Works for files with variable definitions only.
Usage: cat variables.tf | awk -f tf_vars_sort.awk | tee sorted_variables.tf
Hope this helps someone 😉

@yermulnik
Copy link

yermulnik commented Oct 23, 2021

JFYI: I've just extended the script to handle also some other types of TF resources (not only variable's!) — please let me know if you find anything which would better be improved. Thanks.

@Office-Manager
Copy link

@yermulnik Excellent script and did exactly what I was looking for
Only improvement I would possibly suggest is to make the alphabetical order agnostic to case sensitivity as currently items with a capital letter starting will appear first before the lower-case

@yermulnik
Copy link

@Office-Manager Yeah, I see, I didn't think of such a use case. And it sounds reasonable. Thanks for the suggestion. I made a tiny update to the script (and published it) to ignore case for string operations in the block where sorting is done. Seemingly should not break something else 🤞

@gtmtech
Copy link

gtmtech commented Nov 4, 2022

I agree with @apparentlymart - nothing can make super clear decisions on reformatting because it cant possibly know how to keep comments with the right variable/resource/module. Does a comment outside a variable/resource/module relate to the block above or the block below? or neither? or both?

We just use a pre-commit githook to tell us if the sort order is not what we want, and then require the developer to do the work of fixing it up.

The sort order we use for every type of block except locals is:

  • count
  • for_each
  • source
  • everything else (alphabetised)

locals should not be sorted, because the order of locals attributes can affect functionality within a single locals block.

We also then sort all resources / modules / datasources by class (data/resource/module) and then alphabetically by name.

Anyway - TL;DR - githooks throwing out the commit worked better for us than auto-reformatting for above reasons.

@wyardley
Copy link

wyardley commented Jul 27, 2023

Auto sort for lists and object keys would be really helpful IMO (for sure optional, not default behavior).

We have a lot of patterns where we use lists / objects to create module resources, and with those, for us, it's generally tidiest to request that those be sorted alphanumerically.

@apparentlymart
Copy link
Contributor

Hi all,

The goal of terraform fmt is to provide a small set of non-customizable and localized style rules to help modules written by different teams appear more stylistically similar. It focuses only on small syntax details because that typically avoids changing the intended meaning of broader decisions about how things are arranged.

This issue is asking for something that is outside the intended scope of that command, in a few different ways:

  • It's proposed as an option rather than an always-on behavior. terraform fmt's design goal is to provide just one definition of "idiomatic style", without any options.
  • It proposes making changes with a much broader scope than anything else terraform fmt has done so far, potentially changing the relative positions of some blocks and arguments, which were potentially ordered in a particular way intentionally by the author. Terraform cannot infer that broader meaning in order to preserve it.
  • It would cause the command to propose changes that are not justified purely by the idiomatic style implied by the examples in the Terraform documentation, which has typically been our design guide for other rules: fmt makes your configuration in a sense "look like the documentation", following the same lightweight style rules that the examples follow.

For that reason, I don't think this is something we will add to terraform fmt, and the exact rules proposed seem subjective enough that we would not want to implement them in an official capacity even outside the scope of that command.

Instead, this seems like a good opportunity for one or more third-party tools that can propose a more opinionated style that is considerably heavier than what terraform fmt is intended to achieve. You would then opt in to a particular tool's opinions about style by installing and using that tool, and thus the Terraform team would not be in the business of arbitrating between many subjective competing ideas about what is "the best order" for declaring certain items, and teams with different needs and preferences could select different tools.

The HCL API that terraform fmt uses is importable by other codebases and so it should already be possible to implement additional tools. If you want to implement something that the current hclwrite API doesn't support then the HCL maintainers (which overlaps with the Terraform team) would consider feature requests to help support third-party style-enforcing tools. If you use hclwrite then the whitespace layout details from terraform fmt will be applied automatically -- that's how terraform fmt achieves them -- and so a separate tool should be able to cooperate with terraform fmt in the same pipeline without the two arguing with one another about where spaces ought to go, avoiding the need for a separate tool to also reimplement all of what terraform fmt does.

Given all of the above, I'm going to close this issue. The Terraform team will not offer more opinionated formatting options built in, but we'd welcome folks in the community implementing additional tools if they have particular preferences beyond what terraform fmt is intended to achieve, and if it seems likely that others would have similar preferences.

Thanks for the discussion here! Although it didn't lead to a change, it has helped to clarify what is and isn't in scope for terraform fmt and given some examples of some broader style rules that external tools could potentially implement.

@apparentlymart apparentlymart closed this as not planned Won't fix, can't repro, duplicate, stale Jul 27, 2023
@ryanpeach
Copy link

Does anyone know the name of such a tool?

@dfredell
Copy link

Does anyone know the name of such a tool?

I migrated our users.auto.tfvars to users.auto.tfvars.json (via https://github.com/tmccombs/hcl2json) and run it through jq sort in CI.

jq '.users |= sort_by(.name)' users.auto.tfvars.json > users.auto.tfvars.json.sorted
mv users.auto.tfvars.json.sorted users.auto.tfvars.json

@gtirloni
Copy link

gtirloni commented Nov 4, 2023

This is indeed a very good discussion. I will remain in the camp that organizes variables and other resources in more logical groupings by function rather than a lexical sorting

I prefer to add some meaningful prefix to variables so the sorting would match the logical grouping at the same time.

Copy link
Contributor

github-actions bot commented Dec 7, 2023

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests