tfutils / tfenv

Terraform version manager
MIT License
4.48k stars 454 forks source link

always logging to STDOUT breaks tools that use terraform output #310

Open drew88 opened 2 years ago

drew88 commented 2 years ago

Can we get an option to completely disable any output to STDOUT? and/or do all logging to STDERR? some terraform commands (e.g. terraform output) are intended to write output to STDOUT, and if tfenv inadvertently adds STDOUT to the output stream, the expected data will be corrupted if a .terraform-version file indicates a not-previously-installed terraform version is needed.

OJFord commented 2 years ago

I don't think it should be an option, all logs should go to stderr. That's one for @Zordrak/bashlog though - it only puts error-level logs to stderr.

You can work around it (/it is an option) today with either:

to only show error logs (which already go to stderr) or:

to enable file instead of stdio logging, but then with stderr chosen as the file.

drew88 commented 2 years ago

@OJFord Neither of the above options actually suppress all of the STDOUT messages. Here's a run of terraform --version when the required version is not currently installed and BASHLOG_FILE=1 BASHLOG_FILE_PATH=/dev/stderr DEBUG=1:

$ BASHLOG_FILE=1 BASHLOG_FILE_PATH=/dev/stderr DEBUG=1 terraform --version 2>/dev/null
version '0.12.31' is not installed (set by /Users/ddickson/git/rmbx-infra-live/.terraform-version). Installing now as TFENV_AUTO_INSTALL==true
Installing Terraform v0.12.31
Downloading release tarball from https://releases.hashicorp.com/terraform/0.12.31/terraform_0.12.31_darwin_amd64.zip
Downloading SHA hash file from https://releases.hashicorp.com/terraform/0.12.31/terraform_0.12.31_SHA256SUMS
No keybase install found, skipping OpenPGP signature verification
Archive:  tfenv_download.x8QMkX/terraform_0.12.31_darwin_amd64.zip
  inflating: /usr/local/Cellar/tfenv/2.2.2/versions/0.12.31/terraform
Installation of terraform v0.12.31 successful. To make this your default version, run 'tfenv use 0.12.31'
Terraform v0.12.31

Your version of Terraform is out of date! The latest version
is 1.0.11. You can update by downloading from https://www.terraform.io/downloads.html

it looks like info and warn messages go to STDOUT no matter what: https://github.com/tfutils/tfenv/blob/e920c309e5af2a6ad1d7c09699b6e7ca2476549f/lib/bashlog.sh#L138

OJFord commented 2 years ago

Hm you're right, I can reproduce that.

it looks like info and warn messages go to STDOUT no matter what

Yes that's what I meant by only error-level logs go to stderr, and why I thought DEBUG=3 or lower would work. (Edit: re-reading my previous comment, maybe it's not clear, by 'should go to stderr' I mean that's my opinion of how it should work - not that (and it is not the case that) that's how it is currently intended to work.)

drew88 commented 2 years ago

@Zordrak/bashlog hasn't been changed in 3 years. Is it still being maintained?

OJFord commented 2 years ago

I would guess it's just been working as expected - @Zordrak is a collaborator with recent commits in tfenv, so I assume so where(/when/if) deemed necessary.

Zordrak commented 2 years ago

Zordrak/bashlog is just a reference, I don't honestly expect people to depend on it. The bashlog implementation inside tfenv is managed independently and updated as necessary for tfenv.

Zordrak commented 2 years ago

The problem seems to be one of shim invocation versus direct invcocation. It's correct for tfenv to output to STDOUT when called as tfenv, but when called as a substitute for terraform, yes you're right it would seem inappropriate for it to write to STDOUT.

OJFord commented 2 years ago

I recognise it's something of a subjective/style guide sort of thing, but in my opinion tfenv logs should go to stderr too; only stdout in the cases of tfenv --help, tfenv list desired output, etc.

Currently tfenv list writes to stdout:

  1.1.7
  1.0.0
No default set. Set with 'tfenv use <version>'

The final line, which is informational logging, not 'output', should in my opinion be sent to stderr.

Toy example use case:

$tfenv list | xargs -I@ echo 'Terraform v@ is installed'
Terraform v1.1.7 is installed
Terraform v1.0.0 is installed
Terraform vNo default set. Set with tfenv use <version> is installed

(Of course, the workaround is to pipe through head -n -1 or similar, but IMO that shouldn't be necessary, and it makes log messages somewhat part of the API.)