feat: Reset default value of s3_bucket_arns to empty

[antonbabenko]

Kudos to Adam K for this suggestion.

Reset the default value of s3_bucket_arns to prevent users from misconfiguring IAM policies by providing wider permissions than necessary.

[bryantbiggs]

Reset the default value of s3_bucket_arns to prevent users from misconfiguring IAM policies by providing wider permissions than necessary.

What does it mean to "reset default value"? I agree that "*" is not great, but its hard to infer a default setting for something like this. if users are not specifying something like

terraform-aws-emr/examples/virtual-cluster/main.tf

Lines 46 to 49 in a2dd3d2

	s3_bucket_arns = [
	module.s3_bucket.s3_bucket_arn,
	"${module.s3_bucket.s3_bucket_arn}/*"
	]

then this will fail as an invalid policy for them. If we add a check to see if ARN(s) have been provided like https://github.com/terraform-aws-modules/terraform-aws-dms/blob/7d25c92a3732b29dbc62703febff672434d0c8f8/main.tf#L581, then again, they may be caught off guard with permission denied errors and not knowing why they don't have the correct permissions. I'm open to ideas though - the way it was setup was "hey, we'll make it easy for you out of the box but its up to you to ensure the 'best practices' are followed for least privilege"

[antonbabenko]

By "reset default value" I meant that we probably can set it to something less open. I am thinking that setting [] is like setting to null for type list, while ["*"] is a rather concrete value we normally don't use in modules' defaults. Hope this explanation makes sense. :)

If we leave it [], users will get an error in IAM policy, and it will be on them to understand and explicitly set to ["*"] or something else they prefer.

@bryantbiggs It is up to you to decide the future of this PR - approve or close. :)

Offtopic (plan for next couple of years, hehe): In the future version of Terraform (starting from 1.9+), we hopefully can utilize validation blocks in variables and cross-reference variables to have some meaningful validation.

[bryantbiggs]

lets give it a try, we can always make additional changes if needed

[antonbabenko]

This PR is included in version 2.1.0 🎉

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.