-
-
Notifications
You must be signed in to change notification settings - Fork 3.8k
feat: Support S3 Directory Bucket #310
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
feat: Support S3 Directory Bucket #310
Conversation
examples/directory-bucket/main.tf
Outdated
@@ -0,0 +1,111 @@ | |||
locals { | |||
region = "eu-west-1" | |||
zone_id = "euw1-az1" |
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.
Could you make AZ ID discoverable using a data source? There are cases where some AZs are not available in some accounts.
modules/directory-bucket/main.tf
Outdated
bucket_name = "${var.bucket_name_prefix}--${var.availability_zone_id}--x-s3" | ||
} | ||
|
||
resource "aws_s3_directory_bucket" "this" { |
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.
This submodule creates one unique resource type (aws_s3_directory_bucket
), and the rest are the same as those in the main S3 bucket module.
Given there are rather big differences in features, do you think we can have this resource aws_s3_directory_bucket
in the main module and make the creation of it conditionally (e.g. is_directory_bucket = true/false
)? This way, we will have less code repetition.
Or am I missing a bigger picture? What do you think?
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.
That works as well. I was initially thinking since most of the resources in the main module didn't apply to the directory bucket, it would be easier to have a sub-module, but using is_directory_bucket
also works. I refactored to this. Let me know if that looks good to you.
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.
Minor comment and we are good to go :) Thanks!
examples/directory-bucket/main.tf
Outdated
|
||
is_directory_bucket = true | ||
bucket = "${random_pet.this.id}-complete" | ||
availability_zone = "${local.region}b" |
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.
As I mentioned earlier. b
or other AZs may not be present in some AWS accounts. I hit this issue in the past myself. Please update the example to use data-source to find first-available AZ in the account.
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.
Oh I see, my apologies, I misunderstood initially. I added a data source to find available AZs but I found that even though an AZ is available to me, it might not be an option for directory buckets. In the example, if I set it to index 0
, it attempts to use euw1-az2
which is available for my account but not available for directory buckets so I used index 1
here. https://docs.aws.amazon.com/AmazonS3/latest/userguide/s3-express-Endpoints.html
I guess this may not work for everyone so I'm happy to modify this.
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.
Thank you! 👍
## [4.6.0](v4.5.0...v4.6.0) (2025-02-12) ### Features * Support S3 Directory Bucket ([#310](#310)) ([0700a07](0700a07))
This PR is included in version 4.6.0 🎉 |
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. |
Description
Support for directory buckets.
Motivation and Context
https://docs.aws.amazon.com/AmazonS3/latest/userguide/directory-buckets-overview.html
id
attributes on Framework resources hashicorp/terraform-provider-aws#40626Breaking Changes
No.
How Has This Been Tested?
examples/*
to demonstrate and validate my change(s)examples/*
projectspre-commit run -a
on my pull request