Add --chdir option

[wata727]

Fixes #1315
Fixes #1508
See also hashicorp/terraform#26087

This PR adds --chdir option for inspection. This is intended to match Terraform's behavior regarding other directories.

You can inspect against a Terraform configuration in another directory like this:

$ tflint --chdir=environment/production

Similar behavior could be reproduced by passing the directory as an argument, but --chdir differs in the following points:

• Load a module manifest file under the directory passed by --chdir, not the current directory.
  • This resolves Failed to load modules from outside of the module's directory #1508.
  • It means terraform -chdir=dir get; tflint --chdir=dir --module works fine.
• file("./config.tmpl") is resolved against the directory passed by --chdir, not the current directory.
  • This is the same behavior as Terraform and is preferable in many cases.

For the above reasons, we recommend that users using directory arguments switch to the --chdir option. The directory argument still works fine at this point, but is deprecated and may be removed in a future version. To avoid confusion, using --chdir option and a directory argument together is an error.

The --chdir behavior is the same as Terraform's behavior, but there are some TFLint-specific considerations:

• Config file (.tflint.hcl) is processed before acting on the --chdir option.
• Files specified with relative paths like --var-file and varfile on config files are resolved against the original working directory.

UPDATED: The above exceptions were eventually removed. --chdir is basically equivalent to cd dir; tflint.

However, unlike Terraform, --chdir cannot be used with tflint --init or tflint --langserver. This is because I believe that --chdir is necessary only in limited cases.

In the future, this behavior is intended to be used for recursive inspection. Recursive inspection is equivalent to running --chdir for each module and returning the results.

ToDo

• Add tests
• Add E2E tests
• Add documentation
• Define behavior when --chdir and directory argument are passed
• Return an error if --chdir is passed outside of inspection

[wata727]

@bendrucker Could you give me your opinion on the behavior?

[bendrucker]

Will do, I'll give this a thorough review tomorrow

[bendrucker]

I'd expect this behavior to work exactly like terraform -chdir, meaning the chdir call happens before anything else, and is equivalent to cd dir/ && tflint. I'm assuming the difference comes down to the intent to use the internals in the future for recursive inspection. But I think that implementation detail should be hidden and the flag should match Terraform's behavior.

As is, users can use a shared plugin dir (which is the default?) and an absolute path to config for recursion:

config=$(realpath .tflint.hcl)
for mod in mod1 mod2; do
  tflint --chdir $mod --config $config
done

Not clear to me why there should be exceptions for varfiles either. If you want to share, you can use a similar pattern. Future recursive inspection could essentially do this internally, passing an absolute path for the config and var files down so that the directory can be changed. These files don't contain any relative paths of their own.

I think we should be preparing to move init and languageserver to commands, with a compatibility later to to transform the existing args format. Probably version and as-plugin as well. These aren't options that modify behavior in the traditional sense. They do something so substantially different that they deserve their own sub-command. Ideally --chdir should support all commands, including init.

In a world with subcommands, --chdir could exactly match Terraform, which requires global flags to be passed before subcommands:

terraform -chdir=mod1 init
terraform -chdir=mod1 validate -json

tflint --chdir=mod1 init
tflint --chdir=mod1 inspect --format json

Happy to do a pass on the code as well once we discuss the behavior.

[wata727]

Thank you for the feedback!

Agreed that the flag should match Terraform's behavior. As you say, one of the main reasons for the first exception is for recursive inspection. You might find this behavior counter-intuitive for the -chdir option, but I believe this makes sense since the terraform.rc file already ignores -chdir.

For the second exception, I didn't want the following case to be an error:

$ tflint --chdir=dir --var-file=dir/varfile.tfvars

When entering the path for --var-file, I think it is intuitive to specify the path relative to the original working directory.

I think we should be preparing to move init and languageserver to commands, with a compatibility later to to transform the existing args format. Probably version and as-plugin as well.

Yes. I think so too. However, I would like tflint without subcommands to behave as same as tflint inspect. I think it is necessary to investigate whether this can be satisfied.

[bendrucker]

I think we'll manage to find ways to preserve backwards compatibility for arguments. The "command not found" handler could call inspect with the given args, for example.

Re .terraformrc, I assume you're referring to this:

Settings in the CLI Configuration are not for a specific subcommand and Terraform processes them before acting on the -chdir option.

Typically, this file is in $HOME and shouldn't contain any relative paths. I'm not even sure whether relative paths would be handled relative to the config file or the cwd.

Seems like the only time this has any visible impact on normal use-cases is this:

If a terraform.d/plugins directory exists in the current working directory then Terraform will also include that directory, regardless of your operating system. This behavior changes when you use the -chdir option with the init command. In that case, Terraform checks for the terraform.d/plugins directory in the launch directory and not in the directory you specified with -chdir.

https://developer.hashicorp.com/terraform/cli/config/config-file#provider_installation

In keeping with Terraform behavior, I'd expect var files to be loaded after changing directory, since var files are generally part of the configuration.

Given the following file tree:

.
└── mod
    ├── main.tf
    └── vars.tfvars

This is the behavior:

$ terraform -chdir=mod apply -var-file=vars.tfvars

No changes. Your infrastructure matches the configuration.

Terraform has compared your real infrastructure against your configuration and found no differences, so no changes are needed.

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

/var/folders/m6/7nt7y_s54lsbrwsjb_xsmx7m0000gn/T/varfile
$ terraform -chdir=mod apply -var-file=mod/vars.tfvars
╷
│ Error: Failed to read variables file
│
│ Given variables file mod/vars.tfvars does not exist.
╵

[wata727]

Regarding subcommands, I opened an issue so let's discuss it here. It is outside the scope of this PR #1618

Typically, this file is in $HOME

Oops, I misunderstood this. This certainly doesn't explain why .tflint.hcl is in the original working directory.

In keeping with Terraform behavior, I'd expect var files to be loaded after changing directory, since var files are generally part of the configuration.

Good point. I missed this behavior. In this case, the path should be relative to the changed directory. Fixed 407bd21
With this in mind, I feel more and more the importance of subcommand and --chdir flag placement.

The last thing to discuss is which directory to load .tflint.hcl from.

The advantage of referring to the changed directory is that it is easier to implement. Process --chdir first, then process as before. This would be interpreted in the world of subcommands as:

$ tflint --chdir=dir inspect --config=.tflint.hcl

On the other hand, the disadvantage is that referencing config in the original working directory is troublesome. You should do something like this:

$ tflint --chdir=dir inspect --config=$(realpath .tflint.hcl)

In this case, config can be easily used by interpreting it as a flag of the same kind as --chdir:

$ tflint --config=.tflint.hcl --chdir=dir inspect

I'm not sure which one is easier to understand. However, I think that varfile(config file) and --var-file(CLI flag) should be relative paths to the same directory, so in the latter case, special consideration needs to be given to resolving varfile relative paths.

Another concern is the working directory of the plugin process. If you refer to .tflint.hcl in the changed directory, the plugin process will of course be launched in that directory. Plugins that use os.Getwd() may cause unintended consequences. See also terraform-linters/tflint-plugin-sdk#218

GetFiles() paths are relative to the original working directory, so you can't get the exact directory:

.
└── mod
    ├── .tflint.hcl
    └── main.tf

$ tflint --chdir=mod

// In the plugin process
os.Getwd() // => ./mod
runner.GetFiles()[0].File.Body. MissingItemRange().Filename // => ./mod/main.tf

Maybe this should provide a new API so that plugins don't depend on os.Getwd().

[wata727]

Finally, I decided to load the config file in the changed directory. Fixed a2b6654

I've tried thinking about why it loads the config file in the original working directory, but I haven't found a reasonable reason. The plugin process working directory issue will be resolved by allowing plugins to refer to the original working directory.

If there are no problems, I plan to merge this PR within a few days.

[bendrucker]

Will review the latest comments and give this a try today

[bendrucker]

All looks good to me! Adding a note to #1618 re: handling this flag differently than subcommand flags.