-
Notifications
You must be signed in to change notification settings - Fork 74
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
s3_bucket_name
: add all documented naming rules
#571
s3_bucket_name
: add all documented naming rules
#571
Conversation
e803a9a
to
0ad1b56
Compare
If everything's OK, I'll only need to write a test for each item in |
Unit testing this rule should be fine, no integration tests needed. |
@@ -81,7 +81,7 @@ func (r *AwsS3BucketNameRule) Check(runner tflint.Runner) error { | |||
}{ | |||
{ | |||
Regexp: *regexp.MustCompile(fmt.Sprintf( | |||
"^(.{0,%d})?(.{%d,})?$", | |||
"^.{0,%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.
Found out about the alternation operator in Go and thought it was easier to read and more go-friendly.
Regexp: *regexp.MustCompile("^[^a-z0-9]|[^a-z0-9]$"), | ||
Description: "Bucket names must begin and end with a lowercase letter or number.", |
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.
The alternation operator could also be used here to merge these two rules in one, just as the docs.
Thanks! Will do another review and edit as needed later this week. |
s3_bucket_name
: add validationss3_bucket_name
: add all documented naming rules
From this discussion.
Also an alternative to remove hardcoded values from this past issue.