warrensbox / terraform-switcher

A command line tool to switch between different versions of terraform (install with homebrew and more)
https://tfswitch.warrensbox.com
MIT License
1.33k stars 133 forks source link

Version: v1.1.1 no longer reads terraform required_version anymore #441

Closed d33psky closed 3 weeks ago

d33psky commented 1 month ago

head -2 versions.tf

terraform {
  required_version = "1.6.3"

tfswitch 1.1.1 now shows questions in a github action ci/cd pipeline:

Use the arrow keys to navigate: ↓ ↑ → ←
? Select Terraform version:
  ▸ 1.5.7 *recent
    1.6.3 *recent
    1.1.9 *recent
    1.8.4
↓   1.8.3

'old' version 0.13 does what I expect it to :

~/bin/tfswitch-0.13.1308
Reading required version from terraform file
Reading required version from constraint: 1.6.3
Matched version: 1.6.3
Installing terraform at /home/hans/bin
Switched terraform to version "1.6.3"
yermulnik commented 1 month ago

@d33psky Thanks for raising this. We're aware and we're working towards resolution and will cut a new release that fixes this and reverts regular behavior. At the moment you may workaround this issue by ensuring there's no .tfswitch.toml, .tfswitchrc or .terraform-version files around (in the $HOME dir or in the dir with this Terraform code). Given you may be using .tfswitch.toml to define bin parameter value, you'd then need to temporary switch to use -b (--bin=) command line flag. Apologies for the inconveniences and hope we'll cut new tfswitch release with a fix soon.

d33psky commented 1 month ago

Hi yermulnik, glad to hear it's a known issue that gets attention :)

I've verified that there's no .tfswitch.toml, .tfswitchrc or .terraform-version files in either my ~/ nor in the dir with this Terraform code. So that is no workaround sadly.

yermulnik commented 1 month ago

I've verified that there's no .tfswitch.toml, .tfswitchrc or .terraform-version files in either my ~/ nor in the dir with this Terraform code. So that is no workaround sadly.

Hmm, that's weird as there's no issue with the similar on my end:

> tfswitch --version
Version: v1.1.1

> fgrep required_version *.tf
main.tf:  required_version = "1.6.3"

> tfswitch
21:20:10.496 INFO Reading version from Terraform module at "."
21:20:10.681 INFO Reading required version from constraint: "1.6.3"
21:20:10.685 INFO Matched version: "1.6.3"
21:20:10.686 INFO Installing terraform at "/home/giermulnik/bin"
21:20:10.686 INFO Switched terraform to version "1.6.3"

Would you mind sharing console output as above so that I can see logging strings?

d33psky commented 1 month ago

Sure, here goes :

tfswitch --version
Version: v1.1.1

fgrep required_version *.tf
versions.tf:  required_version = "1.6.3"

tfswitch
Use the arrow keys to navigate: ↓ ↑ → ←
? Select Terraform version:
  ▸ 1.5.7 *recent
    1.6.3 *recent
    1.1.9 *recent
    1.8.4
↓   1.8.3

Nothing new to work with I'm afraid

d33psky commented 1 month ago

adding --log-level=DEBUG prints only 1 extra line before the 'Use the arrow keys to' stuff :

20:34:09.441 DEBUG [list_versions.go:19,getTFList] Get list of terraform versions

Running tfswitch in strace shows it at least looks at the versions.tf file :

openat(AT_FDCWD, "/home/hans-sw/develop/tf/tf-deploy-aws-eks/versions.tf", O_RDONLY|O_CLOEXEC) = 3^M
fcntl(3, F_GETFL)                       = 0x8000 (flags O_RDONLY|O_LARGEFILE)^M
fcntl(3, F_SETFL, O_RDONLY|O_NONBLOCK|O_LARGEFILE) = 0^M
epoll_ctl(4, EPOLL_CTL_ADD, 3, {events=EPOLLIN|EPOLLOUT|EPOLLRDHUP|EPOLLET, data={u32=4057989132, u64=8322654864217735180}}) = -1 EPERM (Operation not permitted)^M
fcntl(3, F_GETFL)                       = 0x8800 (flags O_RDONLY|O_NONBLOCK|O_LARGEFILE)^M
fcntl(3, F_SETFL, O_RDONLY|O_LARGEFILE) = 0^M
fstat(3, {st_mode=S_IFREG|0664, st_size=561, ...}) = 0^M
read(3, "terraform {\n  required_version ="..., 562) = 561^M
read(3, "", 1)                          = 0^M
close(3)                                = 0^M

btw, this is on Ubuntu 22.04.4 LTS on my local and in a github action in case you're wondering.

yermulnik commented 1 month ago

To be honest I have no idea why it's misbehaving this way at your end 😲 That strace bit of output looks all fine (though is of no use). The OS shouldn't be a matter for this use case I guess. Nevertheless I'm on the same Ubuntu version.

Last guess I have: is there by any chance TF_VERSION env var defined? Maybe even with an empty value.... 🤔

d33psky commented 1 month ago

env | grep TF_ resulted in nothing and $?=1 What can I try next ?

yermulnik commented 1 month ago

What can I try next ?

Well, looks like wait for a next release when we will provide an correction to the broken logic.

@MatrixCrawler Maybe you may have an idea of what could be preventing tfswitch from picking up TF version for @d33psky given an existing logic in parameters parsing? Thanks.

yermulnik commented 1 month ago

@MatthewJohn Maybe you also as you took the challenge to streamline parameters parsing thing. Thanks in advance.

warrensbox commented 4 weeks ago

1.1.1 seems to be working

warrenv@mass ~/WarrensBox/go/src/github.com/warrensbox/tfswitch/test-data/integration-tests/test_versiontf --> tfswitch
13:30:13.896 INFO Reading version from Terraform module at "."
13:30:14.062 INFO Reading required version from constraint: "1.6.3"
13:30:14.063 INFO Matched version: "1.6.3"
13:30:14.075 INFO Installing terraform at "/Users/warrenv/bin"
13:30:14.254 INFO Downloading "https://releases.hashicorp.com/terraform/1.6.3/terraform_1.6.3_darwin_arm64.zip"
13:30:14.254 INFO Downloading to "/Users/warrenv/.terraform.versions/terraform_1.6.3_darwin_arm64.zip"
13:30:15.310 INFO 24617887 bytes downloaded
13:30:15.322 INFO Downloading "https://releases.hashicorp.com/terraform/1.6.3/terraform_1.6.3_SHA256SUMS"
13:30:15.322 INFO Downloading to "/Users/warrenv/.terraform.versions/terraform_1.6.3_SHA256SUMS"
13:30:15.330 INFO 1378 bytes downloaded
13:30:15.330 INFO Downloading "https://releases.hashicorp.com/terraform/1.6.3/terraform_1.6.3_SHA256SUMS.72D7468F.sig"
13:30:15.330 INFO Downloading to "/Users/warrenv/.terraform.versions/terraform_1.6.3_SHA256SUMS.72D7468F.sig"
13:30:15.359 INFO 566 bytes downloaded
13:30:15.359 INFO Verifying signature of checksum file...
13:30:15.361 INFO Checksum file signature verification successful.
13:30:15.375 INFO Deleting "/Users/warrenv/.terraform.versions/terraform_1.6.3_SHA256SUMS"
13:30:15.375 INFO Deleting "/Users/warrenv/.terraform.versions/terraform_1.6.3_SHA256SUMS.72D7468F.sig"
13:30:15.849 INFO Switched terraform to version "1.6.3"

Look out for the next release

MatthewJohn commented 4 weeks ago

Yeh, I think it's worth getting an early build out for @d33psky - it will either fix the issue or should hopefully give us better insight into what's happening

d33psky commented 4 weeks ago

both required_version = "~> 1.6.3" and required_version = "<=1.6.3" result in


? Select Terraform version:
  ▸ 1.5.7 *recent
    1.6.3 *recent
    1.1.9 *recent
    1.8.4
↓   1.8.3```
d33psky commented 4 weeks ago

Found a way to reproduce the problem in a minimal setup :

Have a file versions.tf with

terraform {
  required_version = "1.6.3"
}

and a file eks.tf with

module "eks" {

in an otherwise empty directory. This triggers the 'Select Terraform version;' bug.

If I put a hash in front of module the error does not trigger.

MatthewJohn commented 4 weeks ago

Ah, so does that module have an upper requirement of a particular version? That's odd.. we use the HCL libraries for determining the constraint. I would have thought it would error if the root module's constraint doesn't fit with module's requirements! I will take a look and reproduce this evening - I think I remember seeing something about this

yermulnik commented 3 weeks ago

@d33psky Agree with what Matt says: please share TF version constraint implied by ecs module.

d33psky commented 3 weeks ago

There is no eks terraform module version mentioned in my minimal setup. It is literally that single line.

The original code has

module "eks" {
  source  = "terraform-aws-modules/eks/aws"
  version = "~>20.8.2"

The 20.x module series added this requirement about terraform :

Minimum supported Terraform version increased to v1.3 to support Terraform state moved blocks as well as other advanced features

( from https://github.com/terraform-aws-modules/terraform-aws-eks/blob/master/docs/UPGRADE-20.0.md )

yermulnik commented 3 weeks ago

When the module version is not specified, the latest is implied, hence required_version = ">= 1.3.2": https://github.com/terraform-aws-modules/terraform-aws-eks/blob/master/versions.tf#L1C1-L2C32 Though I still can't reproduce the issue even having the below code in my test setup 🤷🏻

module "eks" {
  source  = "terraform-aws-modules/eks/aws"
}

terraform {
  required_version = "1.6.3"
}
d33psky commented 3 weeks ago

Let me try to rephrase, it is not 'module eks' causing problems. If I replace the line with module "blabla" I get the same error.

yermulnik commented 3 weeks ago

Matt's guess was that locally defined version may be clashing with the one from the EKS module. Though it appeared to be not the matter 🤷🏻 Maybe Matt will be able to reproduce this when he gets to it. To help him (and me) establish conditions that may help reproduce the issue, could you please share the output of ls -la in the dir with the problematic code and cat *.tf (with sensitive data obfuscated if any). Thanks.

MatthewJohn commented 3 weeks ago

Found a way to reproduce the problem in a minimal setup :

Have a file versions.tf with

terraform {
  required_version = "1.6.3"
}

and a file eks.tf with

module "eks" {

in an otherwise empty directory. This triggers the 'Select Terraform version;' bug.

If I put a hash in front of module the error does not trigger.

@d33psky , I'm sorry, so, there are absolutely no module calls in the minimal example that reproduce the issue? As from the message above, I thought you were saying that it does have both (in two different files)?

Maybe Matt will be able to reproduce this when he gets to it. To help him (and me) establish conditions that may help reproduce the issue, could you please share the output of ls -la in the dir with the problematic code and cat *.tf (with sensitive data obfuscated if any). Thanks.

This would be greatly appreciated :)

TBC (below), just trying to work something out

I have a theory, actually - (assuming this is the case, until you can confirm the above).. if there are conflicting version requirements, then the perhaps there a case where Terraform fails to provide the constraint and we don't get a constraint from terraform and tfswitch continues without it

d33psky commented 3 weeks ago

I already did, but here it is again for clarity :

ls -a1
.
..
eks.tf
versions.tf

and

wc -l *
 1 eks.tf
 3 versions.tf
 4 total

and

cat *
module "blabla" {
terraform {
  required_version = "1.6.3"
}
MatthewJohn commented 3 weeks ago

@d33psky , so the contents of the eks.tf file is literally:

module "blabla" {

?

MatthewJohn commented 3 weeks ago

If so, yes, I can reproduce - in this case, the Terraform is invalid, so tfswitch falls back to asking for input

d33psky commented 3 weeks ago

Ah ! that's interesting. So the minimal test setup I made is too minimal. But this also means that this tfswitch interactive behaviour now depends on having valid terraform code.

MatthewJohn commented 3 weeks ago

Ah ! that's interesting. So the minimal test setup I made is too minimal. But this also means that this tfswitch interactive behaviour now depends on having valid terraform code.

Yes, we use the HCL libraries to "read" the terraform code to be able to extract the version - if the terraform is invalid, it will fail to read it. You can use the flag:

 -d, --default=value
                    Default to this version in case no other versions could be
                    detected. Ex: tfswitch --default 1.2.4

meaning that if tfswitch can't obtain a version from anywhere else, it will fall back to this version and will never be interactive, if this helps?

Having said this, as far as I'm aware, it should be failing if it can't read the terraform (assuming that there is terraform in the directory), so I'll look into this

d33psky commented 3 weeks ago

Found the issue cause: a ${} not in "" . Shows up nicely in terraform test of course. Yeah that's not tfswitch' task, when there is an error in the tf code tfswitch should not start asking interactive questions.

MatthewJohn commented 3 weeks ago

Ah, no, before we "actually" attempt to get the version constraints, we check if there "is terraform" (https://github.com/warrensbox/terraform-switcher/blob/master/lib/param_parsing/versiontf.go#L66), during which we ignore any errors whilst loading the terraform and don't attempt to load it "properly". So yes, if there's terraform with incorrect syntax, we silently ignore it. I will bring this up in #429

d33psky commented 3 weeks ago

Thanks. I hope this "when there is an error in the tf code tfswitch should not start asking interactive questions" gets handled in that #429 then :)

MatthewJohn commented 3 weeks ago

I already did, but here it is again for clarity :

Apologies, you had.. I shouldn't but did miss this - so many notifications to catchup on (and emails from github are very hard (at least for me) to read :D

Assigning the issue so I can ensure it's checked in #429