search

hashicorp / terraform-config-inspect

A helper library for shallow inspection of Terraform configurations
Mozilla Public License 2.0
383 stars 76 forks source link

Add defined providers to module inspection #18

Closed moatra closed 4 years ago

moatra commented 5 years ago

Provider name and alias information are included potentially multiple times in the ManagedResources and DataResources fields. Provider names (but not aliases) are available via the RequiredProviders field. It would be convenient to have both names and aliases available in a single place.

This PR adds the DefinedProviders field that is a succinct representation of the provider name/alias combinations defined by the inspected module.

mildwonkey commented 5 years ago

Hi @moatra , thanks for submitting this PR!

Let me start by saying that I understand the need to include provider aliases in the output, that makes solid sense.

🤔 I am concerned that the field name 'defined_providers' is misleading - this is not limiting the list to providers that are explicitly defined in the configuration, it's including the implicit providers. Perhaps that either this should be renamed, or should only include explicitly-defined providers. I lean towards the latter solution but I'm open to alternative proposals.

moatra commented 5 years ago

Hey @mildwonkey! Thanks for taking a look!

Good call out. I've updated the PR with a compromise - implicit providers are a separate field on the module struct now. Thoughts?

mildwonkey commented 5 years ago

@moatra I like this! I'm pulling in a few other people to take a look - the terraform registry uses terraform-config-inspect so I want to be very careful - but overall I think this is a solid PR.

moatra commented 5 years ago

@mildwonkey @paultyng - Thanks for the review! I've pushed a commit that should address your comments.

If I could ask another favor while you're here, could you take a look at my other PR (#17)? It would allow generating markdown output that contains the new info exposed by this PR.

Thanks!

mildwonkey commented 4 years ago

Hi @moatra! I won't re-iterate my post from your PR #17, but I owe you a second apology.

The scope of this library is reading the parts of a Terraform module that are relevant when it's being used as a child module, such as when distributed via a module registry. We didn't intend for it to expose the parts that live in the root module only and apply to the whole configuration. We're intentionally keeping the scope of this codebase minimal because it's essentially a forked subset of Terraform's own decoder and so the more that is implemented over here the more likely it is that the two implementations will diverge over time as the language changes.

In broad terms, we'd expect that a child module not contain any provider configuration, so these fields do not add to the stated use case of terraform-config-inspect.

If you haven't already, I'd suggest you open an issue in the terraform repository that explains your use case so we can discuss the design of a tool or command more specifically tailored to the community's needs.

Thank you!

moatra commented 4 years ago

Sounds good, thanks for the reply!

prehor commented 4 years ago

In broad terms, we'd expect that a child module not contain any provider configuration, so these fields do not add to the stated use case of terraform-config-inspect.

Remember that modules need to define provider proxy configurations blocks with aliases needed to explicitly pass providers.

https://www.terraform.io/docs/configuration/modules.html#passing-providers-explicitly