No s3 global endpoint rule#499
Conversation
bendrucker
left a comment
bendrucker
left a comment
There was a problem hiding this comment.
Seems like a reasonable rule. Happy to give this another pass when it's closer to ready. For now I've made some high level notes. I'd encourage you to think of test cases that will break your parser. Things like using count/for_each.
|
|
||
| // Enabled returns whether the rule is enabled by default | ||
| func (r *AwsS3NoGlobalEndpointRule) Enabled() bool { | ||
| return true |
There was a problem hiding this comment.
Given that the attribute isn't deprecated in the provider itself, this definitely should not be enabled by default. Terraform will automatically print warnings when deprecated attributes are referenced so if there's truly no need for this today except for exceptional backwards compatibility cases it should probably be deprecated upstream too. Not a requirement for merging this rule but worth kicking off given that it's a 1 line change.
| // is this ever greater than 0 | ||
| v := vars[0] | ||
|
|
||
| if v.RootName() == "aws_s3_bucket" && len(v) == 3 && v[2].(hcl.TraverseAttr).Name == "bucket_domain_name" { |
There was a problem hiding this comment.
This should be more robust, ReferencesInExpr may be helpful:
|
Are you interested in finishing this? |
|
Yes I am. This turned out to be harder to implement than I initially thought and things kept coming up for me so I didn't have time to take a proper look at it. I should have more time in the following few days so will try to pick it up again. |
AleksaC
force-pushed
the
no-s3-global-endpoint
branch
from
1a74307 to
20b5d5e
Compare
2 participants
Hi, I have an idea for a new rule. The idea is to disallow usage of s3 global endpoints which have been deprecated. The most notable issue caused by this is cloudfront redirecting to the regional endpoint for some time after the bucket has been created, as described here.
The
aws_s3_bucketresource returns the global endpoint inbucket_domain_nameoutput which is what my current implementation of the rule catches. However I'd also look for strings that contain something likebucketname.s3.amazonaws.comThis is a rough but working implementation of the rule, so I'm opening a draft PR to get your feedback before doing additional work. If you think this is a useful rule, I'll cover additional cases, clean up the code and add documentation.