Wrapper suppresses exit code 2 even when it is an error

[hundt-corbalt]

In https://github.com/hashicorp/setup-terraform/pull/125 the wrapper was updated to consider an exit code of 2 to be a success, because of a quirk of the plan command. But fmt can also return exit code 2, and it is not considered a success in that case. An example is if you call terraform fmt -check -recursive <dir> and <dir> is not present. In that case a GitHub action running that command will succeed (if you use the wrapper) even though the command failed.

[chgans]

This is still an issue, see https://github.com/hashicorp/terraform/issues/32472 need to use terraform_wrapper: false as a workaround

[IngoStrauch2020]

I'm not happy that PR #125 got merged. In our case we want the step to fail when the plan command returns 2. The workaround to not use the wrapper would mean to not be able to access the outputs as easy as documented (surely ok in some but not all circumstances).

A shortened version of our workaround is (we also use "fmt" in addition)

      - name: Terraform Plan
        id: plan
        run: terraform plan -refresh=false -lock=false -no-color -detailed-exitcode

      - name: Check Terraform Plan
        if: ${{ steps.plan.outputs.exitcode == 2 || steps.plan.outputs.exitcode == 1 }}
        run: |
          echo "Terraform plan returned ${{ steps.plan.outputs.exitcode }}"
          echo "1 = Error, 2 = Succeeded with non-empty diff (changes present) "
          exit ${{ steps.plan.outputs.exitcode }}

To me this is a clear violation of the principle of least surprise: the second step uses the same exit code as the one before and someone not knowing about the special treatment of exit code 2 (it's not documented as far as I can see) has no way to understand what's going on.

I would prefer to keep the "only exit code 0 is success" default and make the special treatment of code 2 more explicit.

How about new optional settings for the action like for example

- uses: hashicorp/setup-terraform@v2
  with:
    plan_with_changes_is_success: <true|false>

or

- uses: hashicorp/setup-terraform@v2
  with:
    successful_exit_codes:
      - 0
      - 2

[audunsolemdal]

@IngoStrauch2020 could you elaborate a bit more on your workaround, after reading your post I tried getting something similar to work, but failed to do so. From looking further I cant really find a way to determine if the plan failed unless using terraform_wrapper

       (...)
      - name: Setup Terraform
        uses: hashicorp/setup-terraform@v2.0.3
        with:
          terraform_version: ${{ inputs.tfVersion }}
          terraform_wrapper: false

      - name: Terraform Plan without varFile specified
        id: nofile
        run: |
          terraform plan -parallelism=${{ inputs.tfParallelism }} -out=plan.tfplan -detailed-exitcode
        continue-on-error: true

      - name: Uncolored Terraform Plan for Bot
        if: github.event_name == 'pull_request'
        id: plan
        run: |
          terraform show -no-color plan.tfplan >${GITHUB_WORKSPACE}/plan.out
          terraform show -no-color -json plan.tfplan >${GITHUB_WORKSPACE}/plan.json

      (..) #action to write output from plan.out to github PR #

      - name: echo exitcode
        shell: bash
        run: |
          echo "uncolored: ${{steps.plan.outputs.exitcode}}"
          echo "nofile: ${{steps.nofile.outputs.exitcode}}"

      - name: Terraform Plan Status
        if: ${{ steps.nofile.outputs.exitcode == 1 }}
        run: exit 1

I do not wish to have the wrapper enabled as it creates extra output for my tfplan which I do not wish to show in my PR comments, Failing for me worked fine until TF 1.4 came along using steps.plan.outcome

[IngoStrauch2020]

${{steps.plan.outputs.exitcode}} is created by the wrapper which in your example is disabled.

[clintonb]

🤦🏾 I was just bitten by this issue. This should be better documented.

[Troesler-illuminate]

also ran into this issue... thankfully terraform init fails with the same error that terraform fmt -check=true does if the HCL is invalid, but for whitespace that won't be the case. This issue makes it really difficult to enforce formatting standards on your PR checks.

Can we get an example here at the very least of how to set the same stdout, stderr, and exitcode step output values since we're forced by this issue to disable the wrapper? I don't love having to choose between "easy to use" and "actually works".

[Troesler-illuminate]

also ran into this issue... thankfully terraform init fails with the same error that terraform fmt -check=true does if the HCL is invalid, but for whitespace that won't be the case. This issue makes it really difficult to enforce formatting standards on your PR checks.

Can we get an example here at the very least of how to set the same stdout, stderr, and exitcode step output values since we're forced by this issue to disable the wrapper? I don't love having to choose between "easy to use" and "actually works".

I found a relatively straightforward workaround that doesn't involve turning off the wrapper:

- name: Get Terraform
  id: get-terraform
  uses: hashicorp/setup-terraform@v3

- name: terraform fmt
  id: fmt
  shell: bash
  run: terraform fmt -check=true -recursive -no-color

- name: Fail if Needed
  if: ${{ steps.fmt.outputs.exitcode == 2 }}
  shell: bash
  run: exit 1

Still not ideal, but after Fail if Needed runs at least the values of steps.fmt.outputs are preserved (which is useful for accessing later to place a bot comment with the results).