Reading automatically from ~/.tfswitch.toml is broken

[dixneuf19]

Hi,

I recently upgraded tfswitch and found out that my binaries stopped going into my custom location /home/dixneuf19/.local/bin/terraform.

I configured this behaviour a long ago with a ~/.tfswitch.toml

bin = "/home/dixneuf19/.local/bin/terraform"

Now with the latest 1.1.1 version, It does not read this file at all and save the binary into the default folder.

> tfswitch --log-level=DEBUG
14:46:26.050 INFO [versiontf.go:34,GetVersionFromVersionsTF] Reading version from Terraform module at "."  
14:46:26.052 DEBUG [list_versions.go:19,getTFList] Get list of terraform versions  
14:46:26.144 INFO [semver.go:14,GetSemver] Reading required version from constraint: "1.3.7"  
14:46:26.146 INFO [semver.go:42,SemVerParser] Matched version: "1.3.7"  
14:46:26.146 INFO [install.go:257,installableBinLocation] Installing terraform at "/home/dixneuf19/bin"  
14:46:26.146 INFO [install.go:307,InstallVersion] Switched terraform to version "1.3.7" 

I could simply add this path to PATH but I don't think this breaking change is wanted.

It seems to be linked to this PR https://github.com/warrensbox/terraform-switcher/pull/356/files#diff-70bb7ecbe02c1c0fb2f1ba6c3626d57287498e82043aca43fa54cb8649c12392 from @MatrixCrawler which modified quite a lot of things.

Now the code which check for the ~/.tfswitch.toml needs the -c arg to be executed, which is not the wanted behaviour.

Is this a know and undocumented regression, or a bug ?

Anyway for now I will add the ~/bin folder to my path as a workaround.

Anyway thanks for your work on this still very useful tool !

[MatrixCrawler]

The problem is, that the code only looks for a toml (or any other config file for that matter) if the -c param is defined.
This is abug if this differs from the old behaviour.
Thanks for poiting that out. We will have a look into that.

For the time being please use the -c param

[dixneuf19]

Using the -c param is not a solution either, since it stop reading the local Terraform version constraint of the current folder. Anyway, thanks for the response !

[MatrixCrawler]

Wait...

is there a version.tf file in the same folder @dixneuf19 ?

[yermulnik]

This is abug if this differs from the old behaviour.

Yes, this is defo a bug as this breaks old behavior.
TOML file has to be read always (as it has aux parameters in addition to TF version) with parameters from this file (TF version specifically) taking precedence over the rest of the means (as in reading TOML file should not prevent reading other files). TOML file can also exist in current dir (in addition to the one in user home) with the one from user home taking precedence.

I also think this is what's supposed and even what's "must-have": each item from the list of var sources is read if present and params from items with higher precedence overwrite params from lower precedence files. Why read them all and always: files may be present, but may not have TF version definition parameters set.

ps: is the docu site kinda broken? I encounter broken links from the README and also I see a truncated list of items here: https://tfswitch.warrensbox.com/usage/general/ 😕

[MatthewJohn]

I'd be happy to take a look into this, but just need confirmation of expected behavoir - is the order of preseidence:

• version specified in .tf files (-c dir, if provided, else current dir)
• TOML file in current dir (-c dir, if provided, else current dir)
• TOML file in home dir
  ?

[yermulnik]

@MatthewJohn Great 👍🏻 Please refer to https://github.com/warrensbox/terraform-switcher/blob/docs/Update_Site_and_README/www/docs/usage/general.md for the precedence order. Items 2-6 are what you mean: -c dir, if provided, else current dir.
This is probably where you start from.

@warrensbox Please weigh in if the below isn't fully correct:

TOML file has to be read always (as it has aux parameters in addition to TF version) with parameters from this file (TF version specifically) taking precedence over the rest of the means (as in reading TOML file should not prevent reading other files). TOML file can also exist in current dir (in addition to the one in user home) with the one from user home taking precedence.

I also think this is what's supposed and even what's "must-have": each item from the list of var sources is read if present and params from items with higher precedence overwrite params from lower precedence files. Why read them all and always: files may be present, but may not have TF version definition parameters set.

[warrensbox]

Yes @yermulnik is right. Having a toml file should let us use other config files itself. We had this in the old code. In the old code: https://github.com/warrensbox/terraform-switcher/blob/1.0.2/main.go#L102

If the toml file existed case fileExists(TOMLConfigFile) || fileExists(HomeTOMLConfigFile):
if would read the configuration from the toml file

	case fileExists(TOMLConfigFile) || fileExists(HomeTOMLConfigFile):
		version := ""
		binPath := *custBinPath
		if fileExists(TOMLConfigFile) { //read from toml from current directory
			version, binPath = getParamsTOML(binPath, *chDirPath)
		} else { // else read from toml from home directory
			version, binPath = getParamsTOML(binPath, homedir)
		}

AND also use the configuration in the other files if provided: ( Nested switch statement)

switch {
		/* GIVEN A TOML FILE, */
		/* show all terraform version including betas and RCs*/
		case *listAllFlag:
			listAll := true //set list all true - all versions including beta and rc will be displayed
			installOption(listAll, &binPath, mirrorURL)
		/* latest pre-release implicit version. Ex: tfswitch --latest-pre 0.13 downloads 0.13.0-rc1 (latest) */
		case *latestPre != "":
			preRelease := true
			installLatestImplicitVersion(*latestPre, &binPath, mirrorURL, preRelease)
		/* latest implicit version. Ex: tfswitch --latest 0.13 downloads 0.13.5 (latest) */
		case *latestStable != "":
			preRelease := false
			installLatestImplicitVersion(*latestStable, &binPath, mirrorURL, preRelease)
		/* latest stable version */
		case *latestFlag:
			installLatestVersion(&binPath, mirrorURL)
		/* version provided on command line as arg */
		case len(args) == 1:
			installVersion(args[0], &binPath, mirrorURL)
		/* provide an tfswitchrc file (IN ADDITION TO A TOML FILE) */
		case fileExists(RCFile) && len(args) == 0:
			readingFileMsg(rcFilename)
			tfversion := retrieveFileContents(RCFile)
			installVersion(tfversion, &binPath, mirrorURL)
		/* if .terraform-version file found (IN ADDITION TO A TOML FILE) */
		case fileExists(TFVersionFile) && len(args) == 0:
			readingFileMsg(tfvFilename)
			tfversion := retrieveFileContents(TFVersionFile)
			installVersion(tfversion, &binPath, mirrorURL)
		/* if versions.tf file found (IN ADDITION TO A TOML FILE) */
		case checkTFModuleFileExist(*chDirPath) && len(args) == 0:
			installTFProvidedModule(*chDirPath, &binPath, mirrorURL)
		/* if Terraform Version environment variable is set */
		case checkTFEnvExist() && len(args) == 0 && version == "":
			tfversion := os.Getenv("TF_VERSION")
			fmt.Printf("Terraform version environment variable: %s\n", tfversion)
			installVersion(tfversion, &binPath, mirrorURL)
		/* if terragrunt.hcl file found (IN ADDITION TO A TOML FILE) */
		case fileExists(TGHACLFile) && checkVersionDefinedHCL(&TGHACLFile) && len(args) == 0:
			installTGHclFile(&TGHACLFile, &binPath, mirrorURL)
		// if no arg is provided - but toml file is provided
		case version != "":
			installVersion(version, &binPath, mirrorURL)
		default:
			listAll := false //set list all false - only official release will be displayed
			installOption(listAll, &binPath, mirrorURL)
		}

The toml file has the location of the bin path (which is the most important bit). You can also specify the version in the toml file, but it can be overridden later (see code above).

As an example, the user does not want to keep passing in the location of the bin path everytime they run tfswitch, so they put the bin path in the toml file in the home directory (which is possible) => fileExists(HomeTOMLConfigFile). Now, when they run tfswitch, they can run it with any configuration (version.tf, terragrunt.hcl, or just dropdown) since tfswitch knows the location of the bin path.

[warrensbox]

@MatthewJohn Great 👍🏻 Please refer to https://github.com/warrensbox/terraform-switcher/blob/docs/Update_Site_and_README/www/docs/usage/general.md for the precedence order. Items 2-6 are what you mean: -c dir, if provided, else current dir. This is probably where you start from.

@warrensbox Please weigh in if the below isn't fully correct:

TOML file has to be read always (as it has aux parameters in addition to TF version) with parameters from this file (TF version specifically) taking precedence over the rest of the means (as in reading TOML file should not prevent reading other files). TOML file can also exist in current dir (in addition to the one in user home) with the one from user home taking precedence.
I also think this is what's supposed and even what's "must-have": each item from the list of var sources is read if present and params from items with higher precedence overwrite params from lower precedence files. Why read them all and always: files may be present, but may not have TF version definition parameters set.

Technically, in the previous behavior, the BIN path in the toml file had the highest precedent, only the bin path or the version can be overridden later.
https://github.com/warrensbox/terraform-switcher/blob/docs/Update_Site_and_README/www/docs/usage/general.md

[warrensbox]

Feel free to add your suggestions here:

#420

[MatthewJohn]

I'd be happy to take a look into this, but just need confirmation of expected behavoir - is the order of preseidence:

@MatrixCrawler I assume you're currently working on this (based on assignee)? If not, happy to help out :)

[warrensbox]

I'd be happy to take a look into this, but just need confirmation of expected behavoir - is the order of preseidence:

@MatrixCrawler I assume you're currently working on this (based on assignee)? If not, happy to help out :)

@yermulnik was working on re-documenting the precedence.

[yermulnik]

I'd be happy to take a look into this, but just need confirmation of expected behavoir - is the order of preseidence:

@MatrixCrawler I assume you're currently working on this (based on assignee)? If not, happy to help out :)

@yermulnik was working on re-documenting the precedence.

Please see #419 (comment)
I'd be glad to push documentation change to a PR with the fix into precedence logic though.

[warrensbox]

I'd be happy to take a look into this, but just need confirmation of expected behavoir - is the order of preseidence:

@MatrixCrawler I assume you're currently working on this (based on assignee)? If not, happy to help out :)

@MatrixCrawler do you need help with this?
@MatthewJohn or I can help out as well

[MatthewJohn]

Hey @dixneuf19 ,

Apologies for the wait, just to keep you up to date - we've had some discussions on this (#420) and I'd hope that #429 should certainly fix your issues :) Just waiting for some reviews on it :)

Thanks
Matt

[aryklein]

He folks, any update on this bug?

[yermulnik]

Once we get #429 merged and get new release cut, this will be hopefully fixed.
Apologies for the inconveniences and that it is taking longer than we anticipated and expected.

As a temp solution you may want to downgrade to previous version of tfswitch that doesn't not have this bug. Please refer to https://tfswitch.warrensbox.com/Installation/ for instructions on manually "switching" tfswitch version.

[aryklein]

No rush, thanks so much! I just downgraded and works fine.
Thanks for your work on this project!