terraform icon indicating copy to clipboard operation
terraform copied to clipboard

fmt `check` should not override `write=true` option

Open haroldo-bonette opened this issue 1 year ago • 2 comments

Terraform Version

Terraform v1.6.6
on linux_amd64

Use Cases

the goal is to run terraform fmt -check -write=true and preserve the -write=true option so in our PR check we can at the same time validate terraform format and if -check returns an error we are able to create a second PR with the changes done by -write=true, this way we dont have to run terraform fmt twice

Attempted Solutions

the -write=true is overwritten by -check

  - name: Terraform fmt
    id: fmt
    run: terraform fmt -check -write=true

  - name: Create Pull Request
    if: ${{ always() && (steps.fmt.outcome == 'success' || steps.fmt.outcome == 'failure') }}
    uses: peter-evans/create-pull-request@v5
    with:
      commit-message: terraform fmt
      title: Reformat terraform files
      body: Update Terraform files to canonical format using `terraform fmt`
      branch: automated-terraform-fmt
      base: ${{ github.event.pull_request.head.ref }}
      labels: terraform, fmt
      token: ${{ secrets.GITHUB_TOKEN }}

  - name: Post Format Comment
    if: ${{ always() && (steps.fmt.outcome == 'success' || steps.fmt.outcome == 'failure') }}
    uses: GetTerminus/terraform-pr-commenter@v3
    with:
      commenter_type: fmt
      commenter_input: ${{ format('{0}{1}', steps.fmt.outputs.stdout, steps.fmt.outputs.stderr) }}
      commenter_exitcode: ${{ steps.fmt.outputs.exitcode }}

Proposal

The proposal is that -write=false is only the default value for -check option but once -write=true is set by user it wont be overwritten by terraform fmt

References

No response

haroldo-bonette avatar Dec 29 '23 08:12 haroldo-bonette

Thanks for the proposal. It's unlikely we're going to change the documented behaviour of these options in combination, as to do so could introduce compatibility issues with existing scripts. In particular, it's not clear what the exit codes of terraform fmt -check -write=true would represent.

Your PR check use case makes sense to me, but it doesn't look like it requires the check and the reformat to happen in one command. Is there a reason you can't run terraform fmt -check in the "Terraform fmt" step, and if that fails, run terraform fmt in the "Create Pull Request" step?

kmoe avatar Jan 02 '24 13:01 kmoe

@kmoe thanks for the quick reply. the workaround I have implemented is indeed run 2 separated steps, but this is a bit inefficient and the option to respect -write=true would be much cleaner.

I believe the issues mentioned by you could be easily managed, the compatibility issue is very unlikely to happen as -write=false is already the default and would not change, with regards to exit codes it would be just a matter to handle the errors accordingly.

thanks

haroldo-bonette avatar Jan 02 '24 15:01 haroldo-bonette