minamijoyo / tfupdate

Update version constraints in your Terraform configurations
MIT License
539 stars 23 forks source link

tfupdate 0.7.0 behavior change #93

Closed chenrui333 closed 1 year ago

chenrui333 commented 1 year ago

Context

Running some regression test failure while trying to upgrade tfupdate to 0.7.0 release as error below:

failed to load module: dir = /private/tmp/tfupdate-test-20230704-72774-1kak03t/, err = Failed to read file: The configuration file "/private/tmp/tfupdate-test-20230704-72774-1kak03t/main.tf" could not be read.

How to reproduce

However, it works with tfupdate provider aws -v 5.6.2 .

minamijoyo commented 1 year ago

Hi @chenrui333, Thank you for your report. It looks like a regression in v0.7.0.

I have confirmed that the error occurs when an absolute path is specified.

[tfupdate@master|✔]$ go run main.go provider null -v 3.2.1 $(pwd)/test-fixtures/lock/simple/main.tf

On the other hand, a relative path seems to work fine.

[tfupdate@master|✔]$ go run main.go provider null -v 3.2.1 ./test-fixtures/lock/simple/main.tf

The error seems to come from this line, where I'm doing something a bit tricky, converting the filesystem between afero and tfconfig.

https://github.com/minamijoyo/tfupdate/blob/v0.7.0/tfupdate/context.go#L81-L84

I'll look into it tomorrow as it's already late in my time. Thanks!

chenrui333 commented 1 year ago

no rush at all. have a good night!

minamijoyo commented 1 year ago

After debugging, I found that the root cause of this problem is that afero.IOFS does not support an absolute path.

https://github.com/hashicorp/terraform-config-inspect/blob/f32df32a01cd687715ed165ac5d202009eba9b2f/tfconfig/load_hcl.go#L29 https://github.com/spf13/afero/blob/v1.9.5/iofs.go#L39 https://github.com/golang/go/blob/go1.20.5/src/io/fs/fs.go#L47

There are some possible options:

(1) Suggest a change to afero. (2) Use a native os filesystem without afero. (3) Detect required_providers without terraform-config-inspect. (4) Simply ignore the load error other than the lock subcommand because others actually have not depend on the result of terraform-config-inspect yet

For future extensibility, I think that (2) is probably the ideal solution, but for the time being I'll take the simpler (4) to fix the regression issue quickly.

minamijoyo commented 1 year ago

@chenrui333 Fixed it in v0.7.1 🚢

chenrui333 commented 1 year ago

Thanks!! Trying it now. 🚀

minamijoyo commented 1 year ago

FYI: I've already fixed this in (4), though, I found the Go's standard io/fs.FS does not support absolute paths. So option (1) is technically impossible. https://github.com/golang/go/issues/44279