-
Notifications
You must be signed in to change notification settings - Fork 355
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: terraform_module_version #1182
Conversation
Whoops, tagged the wrong # above |
"github.com/terraform-linters/tflint/tflint" | ||
) | ||
|
||
var exactVersionRegexp = regexp.MustCompile(`^=?\s*\d+\.\d+\.\d+$`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a little worried if this regex is enough. For instance, go-version uses the following regexp:
https://github.com/hashicorp/go-version/blob/v1.3.0/version.go#L18-L31
Ideally, it would be nice to be able to verify that the f
in the Constraint
is the constraintEqual
, but this struct doesn't seem to expose the logic...
https://github.com/hashicorp/go-version/blob/v1.3.0/constraint.go#L30-L31
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree. Spent a bunch of time looking at constraints and saw that the constraint type was not exposed. Was thinking of this more complicated regexp compilation:
But the version one is easy to reuse.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After trying this, the go-version regex doesn't require major/minor/patch and the test ensuring 1.0
is rejected as exact fails. Grabbed a regex from the semver.org docs and added a test ensuring that prerelease suffixes work.
Revised per your comments, ready for another look |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Adds a
terraform_module_version
rule that Terraform modules from the registry should set a source. Similar toterraform_module_pinned_source
, this is enabled by default.Users can optionally specify
exact = true
to require thatversion
be an exact 3 part version.This brings back some Terraform files in
addrs
as well as dependent packages. They were necessary to determine whether asource
resolves to a registry:https://github.com/hashicorp/terraform/blob/120629848761f5434c75b9876a030f8145abf043/internal/addrs/module_source.go#L63-L73
This could also be useful for terraform-linters/tflint-ruleset-terraform#18 which would want to check whether a
source
is a local one before applying some rules to it.Fixes #1176