Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Failed to load modules from outside of the module's directory #1508

Closed
wata727 opened this issue Sep 10, 2022 · 4 comments · Fixed by #1612
Closed

Failed to load modules from outside of the module's directory #1508

wata727 opened this issue Sep 10, 2022 · 4 comments · Fixed by #1612
Labels

Comments

@wata727
Copy link
Member

wata727 commented Sep 10, 2022

Introduction

Enabling module inspection and inspecting from outside the module's directory will cause the module to fail to load.

$ tflint --module ./work
Failed to load configurations; work/main.tf:1,1-18: `instance` module is not found. Did you run `terraform init`?; :

Error: `instance` module is not found. Did you run `terraform init`?

  on work/main.tf line 1, in module "instance":
   1: module "instance" {
$ cd work; tflint --module
# No errors

See also #1435

Expected Behavior

It loads successfully with no errors.

$ tflint --module ./work
# No errors

Actual behavior

It failed to load.

Step to Reproduce

$ mkdir work
$ cat <<EOS > work/main.tf
module "instance" {
  source = "./module"
}
EOS
$ mkdir work/module
$ cat <<EOS > work/module/main.tf
resource "aws_instance" "main" {
  instance_type = "tx.micro"
}
EOS
$ tree work
work
├── main.tf
└── module
    └── main.tf

1 directory, 2 files
$ cd work
$ terraform get
- instance in module
$ cd ../
$ tflint --module ./work

Additional Context

$ tflint -v
TFLint version 0.40.0
+ ruleset.azurerm (0.18.0)
$ terraform -v
Terraform v1.2.6
on linux_amd64
@wata727 wata727 added the bug label Sep 10, 2022
@bendrucker
Copy link
Member

#1315 (-chdir) would prevent this edge case. I can make some time to work on that soon. The ability to pass multiple modules and partial modules (single files) has been responsible for a lot of bugs and unexpected behavior.

@wata727
Copy link
Member Author

wata727 commented Sep 23, 2022

Thank you for the comment, but I'm wondering if the -chdir should be implemented.
Previously, TFLint used Terraform's native loader, so the current directory was important for module loading. I think the -chdir should be implemented in this design.

However, the current directory is not necessarily important for module loading, as there is now room for improving the module loader to TFLint's use case. I think the use case where -chdir is needed is often to inspect multiple modules. In that case, I believe the most appropriate solution is a recursive inspection rather than -chdir.

I think extending the module loader to allow modules to be loaded without depending on the current directory would be a good solution to this issue.

@bendrucker
Copy link
Member

Sure, we can discuss further on that issue. Terraform did this for the sake of simplicity, because having various commands/code paths need to consider the working directory led to inconsistency/bugs:

hashicorp/terraform#26087

Previously, cd foo/ && terraform init had different behavior from terraform init foo/, when it came to the location of the .terraform directory. The switch to -chdir eliminated that distinction.

Assuming the original example were actually working, presumably the full script would be:

terraform -chdir=./work init
tflint --module ./work

Setting partial workarounds aside (#1502), if you want to use module inspection, you have to run init. This is why recursive module discovery/inspection has always struck me as convenient in theory but probably a design mistake in practice.

@wata727
Copy link
Member Author

wata727 commented Sep 24, 2022

Setting partial workarounds aside (#1502), if you want to use module inspection, you have to run init. This is why recursive module discovery/inspection has always struck me as convenient in theory but probably a design mistake in practice.

Good point. As mentioned in #1355 (comment), remote module inspection is a challenge in implementing recursive inspection. I agree that recursive inspection should not be provided when conforming to Terraform semantics.

However, in reality, there are many use cases for inspecting multiple modules, and I now believe that deliberately "distorting" the design for that purpose is not a bad option. Terragrunt is a wrapper designed to solve this design limitation of Terraform, and its design is worth looking at.

On the other hand, a design that accepts a directory as an argument may not be good as it is inconsistent with Terraform's CLI design. Apart from recursive inspection, given the use case of running terraform init and tflint --module only on modules that have changed when inspecting remote modules, providing -chdir might not be a bad idea.

@wata727 wata727 mentioned this issue Dec 3, 2022
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.

2 participants