suzuki-shunsuke / tfcmt

Fork of mercari/tfnotify. tfcmt enhances tfnotify in many ways, including Terraform >= v0.15 support and advanced formatting options
https://suzuki-shunsuke.github.io/tfcmt/
Other
406 stars 45 forks source link

Exit code 0 even if fails to post comment #1187

Closed doriandekoning closed 4 months ago

doriandekoning commented 6 months ago

tfcmt version

$ tfcmt -v
tfcmt -var "target:dir" --config tfcmt.yml plan -- terraform plan

Environment

Overview

Background: When running tfcmt from our github actions we ocasionally hit github rate limits on the amount of comments we can post. Tfcmt then nicely logs an error about hitting the rate limit but still returns exit code 0. This can be quite dangerous if used in combination with --skip-no-changes because when the rate limit is hit no comment is posted and the user might wrongly assume there are no planned changes.

Question: Is it intentional that when an error occurs posting a comment tfcmt exits with 0 as exit code? For example, when no github token is provided a non-zero exit code is provided but if an invalid token is provided tfcmt exits with 0.

If this is intentional it would be very useful to add an option that when set would make tfcmt fail if an error occurs.

I'd be happy to open a PR if it would be welcome!

How to reproduce

tfcmt.yaml

any

Terraform Configuration

any

Executed command and output

$ tfcmt --config tfcmt.yml plan -- terraform plan; echo "Exit code $?"
...
post a comment: POST https://api.github.com/repos/[snip]/comments: 401 Bad credentials []
Exit code 0

Debug output

$ 

Expected behaviour

Tfcmt exits with a non zero exit code if it fails to post a comment.

Actual behaviour

Tfcmt fails with a zero exit code when it fails to post a comment.

Important Factoids

No response

Note

No response

suzuki-shunsuke commented 6 months ago

Is it intentional that when an error occurs posting a comment tfcmt exits with 0 as exit code?

Yes.

This can be quite dangerous if used in combination with --skip-no-changes because when the rate limit is hit no comment is posted and the user might wrongly assume there are not planned changes.

I see. It makes sense.

I'm torn between two options.

  1. Change the exit code to 1 if it fails to post a comment
  2. Add a command line option to change the exit code
suzuki-shunsuke commented 6 months ago

Thank you for your feedback.

doriandekoning commented 6 months ago

Thanks for the quick reply! Both options would work for us, the first option would be what I'd intuitively expect to happen but it might be a breaking change for some users. I'd be happy to implement and open a pr for either of the options if you like

doriandekoning commented 5 months ago

Any updates on this @suzuki-shunsuke? I'd be happy to implement one of the options you mention but I would need to know which one you prefer :)

suzuki-shunsuke commented 5 months ago

Sorry for late reply. I prefer the first option.

Change the exit code to 1 if it fails to post a comment

suzuki-shunsuke commented 4 months ago

v4.9.1 is out 🎉 https://github.com/suzuki-shunsuke/tfcmt/releases/tag/v4.9.1