Env var should be the highest order of precedence, not lowest
Closed this issue · 5 comments
From the readme:
## Order of precedence
| Order | Method |
| --- | ----------- |
| 1 | .tfswitch.toml |
| 2 | .tfswitchrc |
| 3 | .terraform-version |
| 4 | Environment variable |
With 1 being the highest precedence and 4 the lowest
*(If you disagree with this order of precedence, please open an issue)*I don't really have an opinion on the order of the files, but I do think it's unusual to have an environment variable be the lowest.
Say you want to override it one time:
$TF_VERSION=1.2.3 tfswitch
vs.
$TF_VERSION=1.2.3 mv .tfswitch.toml{,.bak} && tfswitch; mv .tfswitch.toml{.bak,}
(or worse if the other files might exist too!)
As the caller of the command you're in control of its environment, but not necessarily the existence of files or their contents.
I'm not (yet) a user though (I use tfenv, came across tfswitch for the first time just now. I like that it's Go, but I think I prefer the terraform-hijacking model of tfenv to having to explicitly switch) so take this with a pinch of salt, just some first impression feedback.
Also, speaking about orders - I'd rather add in this table the mention of using the version from the TF project itself (versions.tf part), and I'd put it above the .terraform-version, leaving the .terraform-version at the very bottom. The reason is mostly due to tfenv supports many "extensions" to version syntax on its own - like latest-allowed, min-required, latest:1.2.3 etc., which trigger an "Args must be a valid terraform version" error in tfswitch
@OJFord While what you're saying about env var precedence makes sense to me, I must admit that we're a bit short on time and effort at the moment, hence would you be able to create PR to elevate env var precedence? Also since this is sort of an essential change in habitual behavior of tfswitch, this would also require an explicit mention in README and might probably be postponed to a next minor or major release.
@jukie @warrensbox What do you think?
@ikorchynskyi I'm not sure about your suggestion on precedence. I guess everything "Terraform-native" should have lowest precedence to allow users to customize Terraform version per their use cases.
The reason of putting tfenv related config to the end is that tfenv has its own extensions which are not supported - like latest, latest-allowed, min-required, latest:${REGEX} etc. Thus, having the .terraform-version in that format would anyway prevent tfswitch from working properly.
But that may be expected in that case, so take this suggestion with a grain of salt.
Well, what you suggest won't fix the incompatibility issue (which is the gist of what you suggest from my point of view), but will just hide it deeper, imho.
What's the reason to use tfswitch for the project which has tfenv as its recommended and only Terraform version management tool?
Basic support for tfenv's version file was added back in 2019 (#60) and has never meant to be a full compatibility feature I guess (or maybe tfenv had no this number of customization options at that time 🤔). Hence the only reasonable suggestion about it I'd think of is to just ignore it with a corresponding diagnostic message instead of failing tfswitch execution when it is not parsable by tfswitch. Does this make sense? If yes and if this is something that you'd prefer to resolve, would you be up to create a PR? (We're quite short on time and effort at this moment and any assistance is much appreciated).
JFYI: I've came across this issue again and what I think w.r.t. env vars precedence is that at the moment tfswitch looks to be following what Terraform declares — https://developer.hashicorp.com/terraform/language/values/variables#variable-definition-precedence
So the resolution for this use case would probably be to use command line option instead of the env var to switch to the exact version of Terraform by overriding configuration files.
I'm closing this issue, but please feel free to provide more justifications or to open new issue if you think it makes sense.
Thank you.