Skip to content

Conversation

@ijin
Copy link
Contributor

@ijin ijin commented Apr 23, 2025

Description

This PR updates tfcmt to assign labels to pull requests even when the --output flag is used, as long as --disable-label is not specified.

Previously, specifying --output caused tfcmt to skip label assignment. With this update, label assignment is decoupled from comment posting. Users who wish to disable labeling can still do so explicitly with --disable-label.

Changes

  • Label updates are now performed even when --output is specified (unless --disable-label is set).
  • This behavior is consistent across both plan and apply workflows.
  • No changes to the default behavior for users who do not use --output.

Check List

@ijin
Copy link
Contributor Author

ijin commented Apr 25, 2025

@suzuki-shunsuke Could you take a look at this too?

@suzuki-shunsuke suzuki-shunsuke added this to the v4.14.6 milestone Apr 27, 2025
@suzuki-shunsuke
Copy link
Owner

Thank you!

@suzuki-shunsuke suzuki-shunsuke merged commit 6101068 into suzuki-shunsuke:main Apr 27, 2025
11 checks passed
@github-project-automation github-project-automation bot moved this to Done in main Apr 27, 2025
@ijin
Copy link
Contributor Author

ijin commented Apr 28, 2025

Oh wow, thanks for the refactoring!

@suzuki-shunsuke
Copy link
Owner

@bigwheel
Copy link

bigwheel commented May 1, 2025

FYI: Today, following error happens and I guess this PR is cause.

image

For followers, post error messages.

tfcmt -var "target:terraform/base/development" --config "$GITHUB_WORKSPACE/.github/tfcmt-for-slack-post.yml" --output apply.json apply -- cat apply_output.txt
  shell: /usr/bin/bash -e {0}
  env:
    AWS_DEFAULT_REGION: ap-northeast-1
    AWS_REGION: ap-northeast-1
    AWS_ACCESS_KEY_ID: ***
    AWS_SECRET_ACCESS_KEY: ***
    AWS_SESSION_TOKEN: ***
    TERRAFORM_CLI_PATH: /home/runner/work/_temp/90eb5c27-d924-4632-be95-8948ba9a4e80
time="2025-05-01T03:25:38Z" level=error msg="tfcmt failed" error="github token is missing"
Error: Process completed with exit code 1.

@suzuki-shunsuke
Copy link
Owner

@bigwheel
Ah, GITHUB_TOKEN wasn't required if --output was set.
But this pull request requires GITHUB_TOKEN to update labels.
You can solve the error by setting GITHUB_TOKEN.
But this is an unexpected change. Sorry, I missed.

@suzuki-shunsuke
Copy link
Owner

suzuki-shunsuke commented May 1, 2025

Or --disable-label keeps the original behaviour.

@suzuki-shunsuke
Copy link
Owner

Hmm. I'm wondering if --output should update labels by default.
Perhaps I'll revert this change.
I don't use --output option, so I'm not sure how users use this option and what behaviour they expect.

@bigwheel
Copy link

bigwheel commented May 1, 2025

@suzuki-shunsuke I agree this PR and I fixed above error by

tfcmt -var "target:${{ matrix.working_directory }}" --config "$GITHUB_WORKSPACE/.github/tfcmt-for-slack-post.yml" --output apply.json apply -- cat apply_output.txt

to

tfcmt -var "target:${{ matrix.working_directory }}" --config "$GITHUB_WORKSPACE/.github/tfcmt-for-slack-post.yml" --output apply.json apply --disable-label -- cat apply_output.txt

I think no need to revert.
But, it is very kind if there is version update guide to fix this (here?).

@suzuki-shunsuke
Copy link
Owner

Oh? This change affects apply command, right?
apply command shouldn't update labels.
I'll look into it later.

@bigwheel
Copy link

bigwheel commented May 1, 2025

@suzuki-shunsuke
Sorry for invalid info.

#1758 (comment)
This solution is not correct. You are right, apply mode doesn't accept '--disable-label'.

image

Finally, I append GITHUB_TOKEN.

      - name: tfcmt 2回目 (Slack投稿用に一時ファイルへ出力)
        run: tfcmt -var "target:${{ matrix.working_directory }}" --config "$GITHUB_WORKSPACE/.github/tfcmt-for-slack-post.yml" --output apply.json apply -- cat apply_output.txt
        env:
          # https://github.com/suzuki-shunsuke/tfcmt/pull/1758
          # で入ったらしきバグのworkaroundとしてやむなくgithub tokenを渡す。解決したら削除すること
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

This worked correctly.

image

Problem cause

I guess the cause is move of this.

https://github.com/suzuki-shunsuke/tfcmt/pull/1758/files#diff-243ebed2765f75e6a54f57167212fefb08c3b2a85967ad2acbc0eb78919019c1L134-L152

@suzuki-shunsuke
Copy link
Owner

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

Successfully merging this pull request may close these issues.

3 participants