Skip to content
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

[Discuss/Feature] Additional S3 Bucket Resources #4418

Closed
9 tasks done
devonbleak opened this issue May 1, 2018 · 15 comments · Fixed by #5132
Closed
9 tasks done

[Discuss/Feature] Additional S3 Bucket Resources #4418

devonbleak opened this issue May 1, 2018 · 15 comments · Fixed by #5132
Assignees
Labels
new-resource Introduces a new resource. proposal Proposes new design or functionality. service/s3 Issues and PRs that pertain to the s3 service.
Milestone

Comments

@devonbleak
Copy link
Contributor

devonbleak commented May 1, 2018

Currently aws_s3_bucket contains multiple configuration blocks (logging, cors_rule, website, etc) as the only way to manage those configurations on the bucket. This means that it's very difficult to create buckets in modules and have a way to modify that configuration as you can't use count/conditionals on those configuration blocks.

I'm proposing that we add the following new resources to help enable more flexibility with bucket configurations:

  • aws_s3_bucket_acceleration_status
  • aws_s3_bucket_cors_rule
  • aws_s3_bucket_lifecycle_rule
  • aws_s3_bucket_logging
  • aws_s3_bucket_replication_configuration
  • aws_s3_bucket_request_payer
  • aws_s3_bucket_server_side_encryption_configuration
  • aws_s3_bucket_versioning
  • aws_s3_bucket_website

I've basically borrowed the same names as the configuration blocks, and the arguments for each would probably look very similar to the existing arguments available in those blocks.

Once we have a consensus on a direction here I'd break this issue into an issue per resource to make tracking the implementation more manageable.

Let me know what you think!

@bflad bflad added new-resource Introduces a new resource. proposal Proposes new design or functionality. service/s3 Issues and PRs that pertain to the s3 service. labels May 12, 2018
@austinbyers
Copy link

austinbyers commented Jun 6, 2018

1000% I keep running into this too - trying to put S3 buckets in a configurable module

See also #716

@tomelliff
Copy link
Contributor

@devonbleak is this still worth pursuing with the upcoming changes in 0.12 and HCL2? I think that should make things more configurable as is without needing to split the resources as you are proposing.

@devonbleak
Copy link
Contributor Author

@tomelliff good question - i haven't been able to find anything beyond the blog posts about the dynamic blocks. If they solve the problem of s3 buckets getting created in modules and not being able to create the required additional configuration that the module author left out then we can probably de-prioritize this in favor of something of higher value.

I get the feeling it's still going to depend on module authors building something appropriate for this and it's targeted more towards repeated blocks like in the example from the blog and we're still going to need these.

Also as these are actually all discrete API calls they should probably have discrete resources.

@devonbleak
Copy link
Contributor Author

@tomelliff found https://github.com/hashicorp/terraform/blob/v0.12-dev/website/docs/configuration/expressions.html.md which describes and gives an example. At this point I think while it makes it possible to do what these resources would do, the resources themselves are still more flexible especially when it comes to configuring s3 buckets that are created by modules where the consumer of the module doesn't necessarily control the source of that module. And from the docs we should "avoid creating modules that are just thin wrappers around single resources, passing through all of the input variables directly to resource arguments" which is pretty much exactly what you'd have to do to replace these resources with dynamic blocks.

Let me know if you disagree!

@tony-kerz
Copy link

in scanning the features of tf 0.12, it isn't obvious to me how i might, for instance, make the logging section of an aws_s3_bucket optional based on, say, a value provided for a variable to be used for logging.target_bucket. given the current conditional strategy, i might imagine something like:

resource "aws_s3_bucket" "this" {
  bucket = "${var.bucket}"

  logging {
    count = "${var.logging_target_bucket ? 1 : 0}"
    target_bucket = "${var.logging_target_bucket}"
  }
}

can anyone show how this would be accomplished with syntax changes slated for 0.12...?

@devonbleak
Copy link
Contributor Author

devonbleak commented Jul 23, 2018

@tony-kerz from what I'm seeing in the docs it'd look something like

resource "aws_s3_bucket" "b" {
  bucket = "${var.bucket}"
  
  dynamic "logging" {
    for_each = var.logging
    content {
      target_bucket = logging.target_bucket
      target_prefix = logging.target_prefix
    }
  }
}

Instantiating the module would then look something like

module "s3_bucket" {
  source = "..."

  logging = [{
    target_bucket = "${aws_s3_bucket.logging_bucket.bucket}"
    target_prefix = "log/"
  }]
}

IMO it's clunky for things like this and still relies on the module author putting in all the dynamic blocks for settings you might want to update on the bucket. And with S3 buckets in particular you're even likely to end up in a position where you're creating nested dynamic blocks for things like lifecycle rules.

@ewbankkit
Copy link
Contributor

ewbankkit commented Oct 22, 2018

Another important use case for this is the ability to separate the configuration of various sub-components of an S3 bucket into separate HCL files/Terraform state files.
This would enable scenarios where one group/IAM role manages the base S3 bucket configuration (usually a more privileged group as this has security implications) and other groups/IAM roles can manage the sub-component configurations.

In terms of implementation, there are major opportunities to refactor and share much of the code from the current aws_s3_bucket implementation.
We need to think about compatibility with the existing nested attributes similar to what has been done in the past with resources such as aws_vpc_endpoint/aws_vpc_endpoint_route_table_association.

@gdavison
Copy link
Contributor

Thanks for this, @devonbleak. It's a great idea, and looks like it would help a bunch of people. We are considering this for after we release 3.0 within the next few months.

@dtserekhman-starz
Copy link

Any update on this issue? Sounds like very frequent use case (for s3 buckets' bidirectional replication), and looks like no solution has been made available yet? I'm running into this now too. Big showstopper!

@alexjurkiewicz
Copy link
Contributor

Implementing this would make it impossible to assert that a bucket has eg NO lifecycle rules, right?

This is a similar limitation to IAM Roles, where it was not possible to ensure a role had no inline policies, until the ~3.41.0 introduction of managed_policy_arns property on aws_iam_role resource.

@anGie44
Copy link
Contributor

anGie44 commented Feb 3, 2022

Hi @devonbleak et al., all 9 of the suggested independent resources have been added and will release with v4.0 of the AWS provider. While they will be available, just a reminder that as part of #20433 , many of the arguments themselves in the aws_s3_bucket resource will be read-only, so upgrading to 4.0 will involve upgrading existing s3 bucket configurations in addition to adoption of the new resources (more details will be added in the Upgrade Guide).

@anGie44 anGie44 closed this as completed Feb 3, 2022
@devonbleak
Copy link
Contributor Author

@anGie44 great news!!! Huge effort on this one 🎉

@github-actions
Copy link

This functionality has been released in v4.0.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

@github-actions
Copy link

I'm going to lock this issue 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 similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
new-resource Introduces a new resource. proposal Proposes new design or functionality. service/s3 Issues and PRs that pertain to the s3 service.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants