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

Add rule for external private module references #208

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kunickiaj
Copy link

@kunickiaj kunickiaj commented Sep 6, 2024

Hashicorp's convention is to allow external reference of nested modules if there is a README.md present, or treat as private/internal if there is not one present.

Since there is no enforcement of this by terraform itself, a tflint rule can be used to enforce this convention.

Ref: https://developer.hashicorp.com/terraform/language/modules/develop/structure

Hashicorp's convention is to allow external reference
of nested modules if there is a README.md present, or treat
as private/internal if there is not one present.

Since there is no enforcement of this by terraform itself, a
tflint rule can be used to enforce this convention.
@kunickiaj
Copy link
Author

Hoping I can get your 👀 on this @wata727

Copy link
Member

@bendrucker bendrucker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is an appropriate addition. This guidance is for module authors and you've written a rule targeted at consumers.

When authoring a module, you're encouraged to put any child modules that might be directly used in a modules/ sub-directory. The Terraform Registry will display these modules as "submodules" and uses the inclusion of a readme to determine whether they are "internal" or external, e.g.:

https://registry.terraform.io/modules/terraform-aws-modules/rds/aws/1.0.5/submodules/db_instance

This is only going to provide any warning for module consumers when all modules are locally accessible. Given that the vast majority of users source their modules from registries, this rule feels more like something that fits your organization's conventions but doesn't solve the problem thoroughly.

@kunickiaj
Copy link
Author

I felt like this was in line with that. An issue we have organizationally (and I assumed it was not unique) is that we'd like to have more control over the visibility (usability) of modules intended to be internal only.

A module author, intending that a module is internal, likely does not want a consumer using it, but there is nothing preventing that consumer from doing so. This is a foot-gun for the consumer I'd like to protect them from.

@bendrucker
Copy link
Member

The whole reason this advisory pattern was introduced is to support the registry. The advice is targeted at module authors and implemented as the various UI treatments in the Registry.

The standard module structure is a file and directory layout we recommend for reusable modules distributed in separate repositories.

In your issue messages you are talking to the user like they are both the consumer and an author of the module. By default, you can only assume the former.

If your goal is to warn users about consuming sub-modules that the author did not explicitly intend as public, you should pursue that generally, not just in the edge case where the modules happen to be local. Which, again, is where the standard module structure is the most advisory and least valuable. And then:

  • terraform init is required before using the rule.
  • You'll need to give more nuanced advice on "how to fix" (do not use OR ask author to publish new version with readme).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants