Terraform-provider-aws: [feature request] Support for complete ACL on S3 buckets

[cjeanneret]

Hello,

Using terraform v0.9.8, it's apparently impossible to set proper ACLs on an S3 bucket.
To be precise, the provider currently supports only the "canned ACLs".

But there's another range of ACL we can set, and those are really interesting as they can allow external users (i.e. from anoter AWS account) to write in the bucket.
This specific right can be given using the following AWS-CLI command (example from aws s3api put-bucket-acl help):

aws s3api put-bucket-acl --bucket MyBucket \
  --grant-full-control [email protected],[email protected] \
  --grant-read uri=http://acs.amazonaws.com/groups/global/AllUsers

The grant part can take either an email address or an account id (or even an URI for groups), as explained in AWS documentation.

Care to tackle that so that we can access the full power of S3 ACLs?

Thanks a lot!

Cheers,
C.

[raphink]

I'm guessing the syntax would be like this:

resource "aws_s3_bucket" "b" {
  acl {
    acl = "public-read"
    grant_full_control = "[email protected],[email protected]"
    grant_read = "uri=http://acs.amazonaws.com/groups/global/AllUsers"
  }
}

[cjeanneret]

Something like that would also be interesting, maybe more suitable seeing the JSON calls from AWS, and is probably more backward-compatible:

resource "aws_s3_bucket" "b" {
  bucket = "my_tf_test_bucket"
  acl = "private"
  grant {
    display_name = "string"
    email_address = "string"
    id = "int"
    type = "CanonicalUser"|"AmazonCustomerByEmail"|"Group"
    uri  = "string",
    permission = "FULL_CONTROL"|"WRITE"|"WRITE_ACP"|"READ"|"READ_ACP"
  }
  grant {
    ...
  }
}

[raphink]

So if I understand the doc and API properly, for each grantee specified in the grant parameter, a BucketInput must be created.

[raphink]

@cjeanneret your example actually looks a lot like the Access Control Policy, which can be passed as a JSON object:

          {
            "Grants": [
              {
                "Grantee": {
                  "DisplayName": "string",
                  "EmailAddress": "string",
                  "ID": "string",
                  "Type": "CanonicalUser"|"AmazonCustomerByEmail"|"Group",
                  "URI": "string"
                },
                "Permission": "FULL_CONTROL"|"WRITE"|"WRITE_ACP"|"READ"|"READ_ACP"
              }
              ...
            ],
            "Owner": {
              "DisplayName": "string",
              "ID": "string"
            }
          }

Should we implement it with this?

[cjeanneret]

@raphink might be even better that way, yep. Reusing existing resource/data seems to be pretty correct.

[veetow]

This would be super helpful.

[ameir]

I actually implemented this a while back, but am missing tests:

https://github.com/hashicorp/terraform/pull/13448

I don't have too many cycles these days, but I'd love to get this merged in. We're currently using a fork with these changes since canned ACLs aren't sufficient for us.

[damascenorakuten]

Any update on this? It would be really nice to have something similar to the "policy" parameter. Canned ACLs are not enough when you have a complex list of ACLs 👍

[Chhed13]

@ameir do you have plans on PR your implementation? If not, I can transfer it and work on tests.

[Chhed13]

I checked JSON version's and configuration via grant variant. Looks like @cjeanneret variant the most clear way, if no objections - I'll implement it.

[ameir]

Sorry for the delayed reply, @Chhed13. If you have cycles to work on this, that would be awesome. To clarify, I used JSON because it was consistent with the way bucket policies are managed in TF, and also because you are able to make a simple API call to pull down the full ACL to easily compare.

At work, I wrote some code to do a terraform import of all of our S3 buckets and generate HCL from that state, and having things in JSON made it easier. But, either way you go, we are better off than we are now (no advanced ACL support). Thanks for digging in!

[orfin]

Hi! Any thoughts on this?
This feature is especially needed when creating S3 bucket for CloudFront logs, as CF needs "awsdatafeeds" user to write logs with this setting: https://d.pr/i/I8AZMS

So without having a possibility to control that kind of ACL -- it's impossible to control such buckets in Terraform.

Moreover -- when you import to Terraform existing bucket with "awsdatafeeds" user and then change ACL settings in terraform, $ terraform apply causes awsdatafeeds to be deleted -- so that may be major bug. Also, this change is not visible in $ terraform plan

cheers!

[ameir]

@Chhed13, not sure where you're at with this. I may be able to take a look at this today.

[Chhed13]

Sorry, tough days. I'm here and on it. PR will be ready in a day.

[Chhed13]

@orfin @ameir PR is ready.
Canned ACL and ACL policy grants is not something crystal clear. They contain a lot of ambiguity because "what you get is not what you set".
Canned ACL - can only be set, and we should live with this.
I skip AmazonCustomerByEmail option - it can only be set too.
For DisplayName - does anybody need it? I didn't find the real case for it.

[ameir]

Thanks for taking a stab at this, @Chhed13! I haven't tested it yet, but it does look thorough.

A few days ago I updated my PR and just pushed it up to https://github.com/terraform-providers/terraform-provider-aws/pull/3757 . I am open to whichever approach everyone thinks is best.

[eredi93]

I just stumped onto this one as well 😞 would be great if we could get this out

[galdor]

I could really use this one too.

[eredi93]

@galdor seems that the best way to get this prioritised is to +1 on the initial PR message https://github.com/terraform-providers/terraform-provider-aws/pull/3728#issuecomment-427964123

[galdor]

Done. Thank you :)

--
Nicolas Martyanoff
http://snowsyn.net
[email protected]

[four43]

Thumbs up for the PR fixing this: https://github.com/terraform-providers/terraform-provider-aws/pull/3757

This seems like the PR is ready to go, what's the hold up here? This is deleting things it shouldn't, that's a big deal!

Moreover -- when you import to Terraform existing bucket with "awsdatafeeds" user and then change ACL settings in terraform, $ terraform apply causes awsdatafeeds to be deleted -- so that may be major bug. Also, this change is not visible in $ terraform plan

[mrg2k8]

I've also bumped into this today while configuring logging for CloudFront and would like to see it merged.

[illegalnumbers]

I also really could use this! What needs to happen for this to merge?

[earchibald-lv]

What's holding this up? This is a significant omission from the S3 bucket provider and means that a non-trivial percentage of people's S3 buckets will be consigned to non-terraform management.

That's death by a thousand cuts for a solution. There are probably hacks to work around this -- local-exec or something dumping a file through AWS CLI - which will promptly be code baggage, making infrastructure code brittle to changes.

[bgoyal]

I'm also facing the same issue. Is this addressed? How to use grants for S3 bucket acls?

[chadsmall]

+1 was looking for this today and ended up here what looks like something close.

[neg3ntropy]

It would be very nice if:

1. the docs said that you can only do canned acls at the moment. It takes half an hour to land here.
2. somebody posted an example workaround here with local commands.

[neg3ntropy]

Answering myself, I have published my workaround for s3 log delivery ACLs here

[Chhed13]

I'm still waiting feedback from maintainers on PR #3728. Thumbs up on it - maybe that can help.

[charlottemach]

is this planned for any future releases?

[Chhed13]

I'm on updating my PR after review. But I can't say about the whole process

[nergdron]

just ran into the lack of this today. this is a pretty core feature for S3 buckets, I'm super surprised this hasn't been implemented since being brought up 2.5 years ago. can anyone from terraform comment on why this is still missing?

[tomelliff]

@nergdron The above linked MR (https://github.com/terraform-providers/terraform-provider-aws/pull/3728) was merged yesterday so it will be in the next release.

[Chhed13]

Thanks @gdavison, that will be very soon! 🎉

[nergdron]

@tomelliff oh, fantastic news! thanks so much!

[bflad]

Closing as this was implemented in https://github.com/terraform-providers/terraform-provider-aws/pull/3728 and will release with version 2.52.0 of the Terraform AWS Provider, later today or tomorrow.

[hashibot[bot]]

This has been released in version 2.52.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 for triage. Thanks!

[Sytten]

I am probably not the only one wondering what the heck happened in terraform that all my S3 buckets suddenly needs to be modified. See this link for the way to revert back to the canned default policy. Contrary to the documentation, this doesn't seem to be optional at all for the basic use case.

[hashibot[bot]]

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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!