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

Wrapper suppresses exit code 2 even when it is an error #328

Open
hundt-corbalt opened this issue Jun 7, 2023 · 5 comments
Open

Wrapper suppresses exit code 2 even when it is an error #328

hundt-corbalt opened this issue Jun 7, 2023 · 5 comments

Comments

@hundt-corbalt
Copy link

In #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
Copy link

chgans commented Jun 15, 2023

This is still an issue, see hashicorp/terraform#32472
need to use terraform_wrapper: false as a workaround

@IngoStrauch2020
Copy link

IngoStrauch2020 commented Jun 20, 2023

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

audunsolemdal commented Jul 7, 2023

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

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

@clintonb
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants