terraform_unused_required_providers: check child modules for required_providers

[bendrucker]

Currently, terraform_unused_required_providers has a tricky bug that conflicts with official module development recommendations, as detailed in terraform-linters/tflint-ruleset-terraform#21. This is a hacky fix, which, when built, correctly detects that a child module declares the required provider and that the parent is setting a more specific requirement. I'm putting this up for discussion around creating the supporting APIs so that terraform-linters/tflint-ruleset-terraform#21 can be solved without resorting to hacks.

Issues include:

• This relies on calling NewModuleRunners, which a plugin using the SDK would be unable to do. More generally, rules right now are expected to evaluate a single module and broadcast issues up, not inspect children directly.
• Rules cannot directly detect whether inspection was invoked with module mode on or off. Seems like NewModuleRunners returns empty when the modules aren't loaded so perhaps len(moduleRunners) == 0 && len(moduleBlocks) > 0 is enough to indicate that module mode could have but wasn't used. This rule, if it's going to attempt to detect required providers in child modules, should presumably include a warning when those modules are unavailable.
• TestRunner expects all files to be in the same dir, and we'd probably want to add a separate method for testing configs across multiple modules.

[wata727]

More generally, rules right now are expected to evaluate a single module and broadcast issues up, not inspect children directly.

Correct. We need to be able to get the complete module tree in the context of the root module to detect this issue.
There's probably a way to provide an option to get the child module with GetModuleContent, but the issue is that you can't always get the contents of the child module.

Another issue with extending the GetModuleContent API is that we don't know which child module to get.
If it returns the contents of all child modules, you may need the module name or hierarchy of the retrieved content, so it might be better to add a new API such as GetChildModuleContent to include this metadata in the response.

Rules cannot directly detect whether inspection was invoked with module mode on or off. Seems like NewModuleRunners returns empty when the modules aren't loaded so perhaps len(moduleRunners) == 0 && len(moduleBlocks) > 0 is enough to indicate that module mode could have but wasn't used. This rule, if it's going to attempt to detect required providers in child modules, should presumably include a warning when those modules are unavailable.

Agreed. For rules that depend on child modules, it might be nice to have an API that returns whether module inspection is enabled.
Whether a warning should be included is a bit debatable. In my opinion, it should include a warning if the user explicitly enables this rule, and ignore false negatives if enabled by default.

When I cut out rules to the ruleset-terraform plugin, I plan to introduce a concept called "preset" (imagine something like eslint:recommended) to enable many rules that are currently turned off. Maybe we need to change this behavior at that time

TestRunner expects all files to be in the same dir, and we'd probably want to add a separate method for testing configs across multiple modules.

Yes. I think the test helpers have a lot of room for improvement. The GetModuleContent options are already always ignored.

[wata727]

The current TFLint design concept assumes that all inspections are completed within a single module, and considering such rules (requires child modules) may require a redesign of the APIs.

[wata727]

Rules cannot directly detect whether inspection was invoked with module mode on or off.

It might be a good idea to add the Module to the global configuration that applies to plugins.
https://github.com/terraform-linters/tflint-plugin-sdk/blob/v0.11.0/tflint/config.go#L3-L7

[wata727]

I had planned to include a fix for this issue in v0.40, but realized the issue was difficult.

For example, setting --ignore-module can produce a partially incomplete module tree, and the module option cannot tell whether you have a complete tree.

Ultimately, I think we'll have to change TFLint's inspection model. Opened terraform-linters/tflint-plugin-sdk#193. This change involves a major change in the plugin interface and should be discussed over time.

Thank you for raising this issue. This PR is closed as the change is for a rule that will be removed in v0.40.