Recursive inspection (again)

wata727 commented 2 years ago

Recursive inspection for Terraform modules is a feature requested by many users. So far, I have thought that TFLint cannot support it for the following reasons:

Some expressions depend on the execution directory and the root module (e.g. path.cwd, terraform.workspace)
TFLint uses Terraform's internal API to load modules, which does not support recursive loading

However, it is bad to need to write shell scripts to perform inspections. To make matters worse, some of the examples I've seen haven't been able to inspect the module correctly. Proper implementation of recursive inspection requires a high degree of knowledge of Terraform and TFLint, which is too hard for end-users.

Fortunately, the second limitation has been mitigated by the impact of Terraform's package internalization. We are already working to reduce the dependency on Terraform's internal API, and in the future the module loader can be completely rewritten for TFLint.

The first limitation is a difficult one, but it may be worth exploring the possibility of heuristic evaluation rather than a complete reproduction of Terraform semantics.

andredesousa commented 2 years ago

I'm using the next command to check my modules: tflint modules/** and I see a result like:

1 issue(s) found:

modules/kubernetes/main.tf:32:18: Error - "Standard_D2_v2sdsds" is an invalid value as vm_size (azurerm_kubernetes_cluster_default_node_pool_invalid_vm_size)

wata727 commented 2 years ago

I'm now looking into other tools that have the same issue to achieve this feature.

tfsec

https://github.com/aquasecurity/tfsec

tfsec is a security-focused linter for Terraform, similar to TFLint. tfsec recursively searches for modules by default. Check whether it is a module by whether it contains a tf file. https://github.com/aquasecurity/defsec/blob/v0.73.0/pkg/scanners/terraform/scanner.go#L279-L325

Since tfsec focuses on obvious security errors, it may be less complicated than TFLint. In the case of TFLint, the rules you want to apply to the root module and child modules may be different, so it would be difficult to simply adopt this policy.

Terragrunt

https://github.com/gruntwork-io/terragrunt

Terragrunt is a wrapper made to work with multiple Terraform modules. It's not a linter, but it would be helpful to see how it provides an interface to configure multiple modules with different contexts.

Terragrunt requires a terragrunt.hcl for each module. terragrunt run-all executes commands to the directory containing terragrunt.hcl. Each module's context can be described individually in this config file. https://terragrunt.gruntwork.io/docs/features/execute-terraform-commands-on-multiple-modules-at-once/

This design makes sense, but the issue is that it makes the config redundant. Terragrunt provides a helper called find_in_parent_folders that allows you to inherit the config from the root directory. https://terragrunt.gruntwork.io/docs/features/keep-your-terragrunt-architecture-dry/

This avoids having the same config repeatedly in every module, but it still feels redundant. Especially in the case of TFLint this issue will be more pronounced as all child modules must have the config as well.

ESLint

https://eslint.org/

ESLint is a popular and widely used linter. Although there are many differences between JavaScript and Terraform, the design is very informative. In particular, this blog post published recently was very thought-provoking. https://eslint.org/blog/2022/08/new-config-system-part-1/

The article mentions regretting adopting directory-based cascading for config inheritance. If we do not take the policy of placing .tflint.hcl in all modules, this cascading is powerful. But I think that the issue mentioned here is likely to occur in TFLint as well. Additionally, Terraform's module context does not always match the parent-child relationship of the directory structure, so directory-based cascading may not be appropriate.

I'm now interested in the "flat config" like they adopted. Declare a set of modules and configs to apply to them in .tflint.hcl in the root directory. Imagine something like this:

modules = [
  {
    path   = "aws/**"
    config = "tflint/aws/root_module.hcl"
  },
  {
    path   = "google/**"
    config = "tflint/google/root_module.hcl"
  }
  {
    path   = "aws/modules/**"
    config = "tflint/aws/child_module.hcl"
  }
  {
    path   = "google/modules/**"
    config = "tflint/google/child_module.hcl"
  }
  {
    path   = "aws/legacy_product/production"
    config = "tflint/legacy.hcl"
  }
]

This allows users to flexibly configure which rules to apply to which modules.

However, this way also has some issues:

Running TFLint directly on the module directory may return different results than recursive inspection.
- This is because the config does not exist in the modules directory. To deal with this, I think it is necessary to introduce a syntax that inherits the config of another directory, just like Terragrunt. It's better than Terragrunt's way because you only need to create .tflint.hcl when you need it.
Where to put the dependency lockfile
- In the future, if we introduce a dependecy lockfile that records the checksum of plugins, then there is the question of where to put that file. It's probably safe to put it in the same directory as the config file, but we'll have to investigate.
How to run module inspection on all modules
- If you want to run module inspection on all modules, you have to run terraform init (or terraform get) on each one. Probably, we need to provide a hook that executes the command like Terragrunt does. But I'm not sure if that's appropriate.

These issues need further investigation, but I would like to recommend a flat config (flat cascade) design.

syerm commented 1 year ago

@wata727 I have a lot of folders and run linter with --recursive - it works well, but is there are any way to print several errors(if I have more than one in several folders) at once? repo all_folders

terraform-linters / tflint