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

Support multiple canned ACLs for AWS S3 buckets #144

Open
hashibot opened this issue Jun 13, 2017 · 20 comments
Open

Support multiple canned ACLs for AWS S3 buckets #144

hashibot opened this issue Jun 13, 2017 · 20 comments
Labels
enhancement Requests to existing resources that expand the functionality or scope. service/s3 Issues and PRs that pertain to the s3 service.

Comments

@hashibot
Copy link

This issue was originally opened by @maxrothman as hashicorp/terraform#6139. It was migrated here as part of the provider split. The original body of the issue is below.


The aws_s3_bucket resource's acl field currently accepts a string. AWS supports multiple grants to multiple users (ref). Perhaps the acl field could take a map or even broken out into a separate resource.

@hashibot hashibot added the enhancement Requests to existing resources that expand the functionality or scope. label Jun 13, 2017
@radeksimko radeksimko added the service/s3 Issues and PRs that pertain to the s3 service. label Jan 25, 2018
@wobeng
Copy link

wobeng commented Jun 24, 2019

any update on this

@ewbankkit
Copy link
Contributor

@maxrothman Does the new aws_s3_bucket.grant argument added via #989, #3728 and available in version 2.52.0 of the Terraform AWS provider address your use case?

@maxrothman
Copy link

No it does not. AWS supports both ACL policy grants and canned ACLs, and if you use canned ACLs, AWS supports assigning more than one to a bucket. If AWS supports it, Terraform should too.

@BastienM
Copy link

Bumping this issue.

Right now the provider only take a non-comma separated string as value for the acl arg, but it does actually correctly read and format it to a comma-separated string upon refreshing the state provided you manually edited the bucket's ACL via the web console.

See below:

 ~ aws_s3_bucket.bucket                                                                            
      acl:    "private,log-delivery-write" => "private"

@wanieldilson
Copy link

Bumping this issue.

Right now the provider only take a non-comma separated string as value for the acl arg, but it does actually correctly read and format it to a comma-separated string upon refreshing the state provided you manually edited the bucket's ACL via the web console.

See below:

 ~ aws_s3_bucket.bucket                                                                            
      acl:    "private,log-delivery-write" => "private"

Funnily enough this is exactly what I'm trying to do. Creating a new bucket for logs but it needs to be private. Is this possible in the current version?

@itsjwala
Copy link

itsjwala commented Feb 24, 2021

We cannot specify more than one Canned ACL according to note in the end.

Though my issue with Grant block is it conflicts with ACL which defaults to private and in scenarios where we need to import terraform resource we weren't able to do so.

for example

  acl           = null
  grant {
    permissions = ["READ_ACP", "WRITE"]
    type        = "Group"
    uri         = "http://acs.amazonaws.com/groups/s3/LogDelivery"
  }
  grant {
    permissions = ["READ_ACP"]
    type        = "Group"
    uri         = "http://acs.amazonaws.com/groups/global/AllUsers"
  }
  grant {
    id          = "someid"
    permissions = ["READ", "READ_ACP", "WRITE", "WRITE_ACP"]
    type        = "CanonicalUser"
  }
# skipped ...

it generates the following diff

+ acl                         = "private"
# skipped ...

and this wipes the grants from the previous import above which is expected. Any help on how to circumvent this issue is appreciated.

@wanieldilson
Copy link

Here's what I did, in the end, to solve my issue...

Screenshot 2021-02-24 at 11 52 54

@itsjwala
Copy link

itsjwala commented Feb 24, 2021

Thanks @wanieldilson, This issue only arises when importing the existing bucket it doesn't seem to honor the grant block instead shows it needs to add ACL private, and thus the diff.

so my question is how can we avoid ACL when the grant is specified while importing (it shouldn't show any diff)

sorry to hijack this thread, I've created a new #17791 for this

@anGie44
Copy link
Contributor

anGie44 commented Jan 6, 2022

Relates #4418

@valera-yakovenko
Copy link

valera-yakovenko commented Apr 20, 2023

Bumping this issue.
@anGie44 @ewbankkit Do you have any updates on this?
I want to be able to use multiple canned ACL's in one or two resources

resource "aws_s3_bucket_acl" "files" {
  bucket = "files"
  acl    = "private,log-delivery-write"
  OR
  acl    = "private"
  acl    = "log-delivery-write"
}

OR - Be able to create a second aws_s3_bucket_acl resource to append the previous(if in previous we will have only "acl = "private")

resource "aws_s3_bucket_acl" "files-log" {
  bucket = "files"
  acl        = "log-delivery-write"
}

AWS has such option to add two canned ACL's but in two separate requests

❯ aws s3api put-bucket-acl --bucket files --acl private
❯ echo $?
0
❯ aws s3api get-bucket-acl --bucket files --output json | jq
{
  "Owner": {
    "DisplayName": "***",
    "ID": "***"
  },
  "Grants": [
    {
      "Grantee": {
        "DisplayName": "***",
        "ID": "***",
        "Type": "CanonicalUser"
      },
      "Permission": "FULL_CONTROL"
    }
  ]
}
❯ aws s3api put-bucket-acl --bucket files --acl log-delivery-write
❯ echo $?
0
❯ aws s3api get-bucket-acl --bucket files --output json | jq
{
  "Owner": {
    "DisplayName": "***",
    "ID": "***"
  },
  "Grants": [
    {
      "Grantee": {
        "DisplayName": "***",
        "ID": "***",
        "Type": "CanonicalUser"
      },
      "Permission": "FULL_CONTROL"
    },
    {
      "Grantee": {
        "Type": "Group",
        "URI": "http://acs.amazonaws.com/groups/s3/LogDelivery"
      },
      "Permission": "WRITE"
    },
    {
      "Grantee": {
        "Type": "Group",
        "URI": "http://acs.amazonaws.com/groups/s3/LogDelivery"
      },
      "Permission": "READ_ACP"
    }
  ]
}

Thank's in advance!

@sousmangoosta
Copy link
Contributor

sousmangoosta commented May 19, 2023

@valera-yakovenko I think for this you need an access_control_policy block instead of acl example here

@sousmangoosta
Copy link
Contributor

Full example for private + log-delivery-write :

data "aws_caller_identity" "this" {}
data "aws_canonical_user_id" "this" {}

resource "aws_s3_bucket" "files" {
  bucket = "${data.aws_caller_identity.this.id}-files"

}

resource "aws_s3_bucket_ownership_controls" "files" {
  bucket = aws_s3_bucket.files.id
  rule {
    object_ownership = "BucketOwnerPreferred"
  }
}

resource "aws_s3_bucket_acl" "files" {
  bucket = aws_s3_bucket.files.id

  access_control_policy {
    owner {
      id = data.aws_canonical_user_id.this.id
      display_name = data.aws_canonical_user_id.this.display_name
    }
    grant {
      permission = "FULL_CONTROL"
      grantee {
        id   = data.aws_canonical_user_id.this.id
        type = "CanonicalUser"
      }
    }
    grant {
      permission = "WRITE"
      grantee {
        type = "Group"
        uri = "http://acs.amazonaws.com/groups/s3/LogDelivery"
      }
    }
    grant {
      permission = "READ_ACP"
      grantee {
        type = "Group"
        uri = "http://acs.amazonaws.com/groups/s3/LogDelivery"
      }
    }
  }
  depends_on = [aws_s3_bucket_ownership_controls.files]
}

@valera-yakovenko
Copy link

valera-yakovenko commented May 22, 2023

@sousmangoosta I Agree, this is a way how to aim the goal and provide the permissions needed, but the question\ask here is to do it with canned acl instead of the access_control_policy block which is kinda messy. The issue is about that.

Also, consider that AWS supports multiple ACLs for one bucket.

@sousmangoosta
Copy link
Contributor

@valera-yakovenko I tested it again and it's not possible currently by using the S3 API, the only way to do this is to create the access_control_policies in the terraform provider code and to manage all changes made by amazon with a new version of the provider. I'm not sure the maintainers would accept that, the positive thing I think, would be that we would be able to delete the ACL, as it's not currently possible.

@valera-yakovenko
Copy link

@sousmangoosta I am not sure how you were testing it, but as I showed in the previous comment it is possible to add two canned ACL's in two separate aws s3api commands and if it is possible to add it with cli command I would assume that it also possible to do it with AWS API. Did you try to do it with API?

@sousmangoosta
Copy link
Contributor

@valera-yakovenko Yeah I tried with the API, I don't say it's not possible doing this with the API, but not with a simple PUT API call on bucket like in the doc

@sousmangoosta
Copy link
Contributor

I will try to have a look in debug mode see what is the difference on the calls from cli and from terraform

@sousmangoosta
Copy link
Contributor

@valera-yakovenko For your example it works in both terraform and CLI with this example :

data "aws_caller_identity" "this" {}
data "aws_canonical_user_id" "this" {}

resource "aws_s3_bucket" "files" {
  bucket = "${data.aws_caller_identity.this.id}-files"

}

resource "aws_s3_bucket_ownership_controls" "files" {
  bucket = aws_s3_bucket.files.id
  rule {
    object_ownership = "BucketOwnerPreferred"
  }
}

resource "aws_s3_bucket_acl" "files" {
  bucket     = aws_s3_bucket.files.id
  acl        = "private"
  depends_on = [aws_s3_bucket_ownership_controls.files]
}

resource "aws_s3_bucket_acl" "files_logs" {
  bucket     = aws_s3_bucket.files.id
  acl        = "log-delivery-write"
  depends_on = [
    aws_s3_bucket_ownership_controls.files,
    aws_s3_bucket_acl.files
  ]
}

but on both terraform and cli, if I add the aws-exec-read canned ACL, it remove the log one

$ aws s3api get-bucket-acl --bucket ***-files --output json              
{
    "Owner": {
        "DisplayName": "***",
        "ID": "***"
    },
    "Grants": [
        {
            "Grantee": {
                "DisplayName": "***",
                "ID": "***",
                "Type": "CanonicalUser"
            },
            "Permission": "FULL_CONTROL"
        },
        {
            "Grantee": {
                "Type": "Group",
                "URI": "http://acs.amazonaws.com/groups/s3/LogDelivery"
            },
            "Permission": "WRITE"
        },
        {
            "Grantee": {
                "Type": "Group",
                "URI": "http://acs.amazonaws.com/groups/s3/LogDelivery"
            },
            "Permission": "READ_ACP"
        }
    ]
}
$ aws s3api put-bucket-acl --bucket ***-files --acl aws-exec-read        
$ aws s3api get-bucket-acl --bucket ***-files --output json
{
    "Owner": {
        "DisplayName": "***",
        "ID": "***"
    },
    "Grants": [
        {
            "Grantee": {
                "DisplayName": "***",
                "ID": "***",
                "Type": "CanonicalUser"
            },
            "Permission": "FULL_CONTROL"
        },
        {
            "Grantee": {
                "DisplayName": "za-team",
                "ID": "6aa5a366c34c1cbe25dc49211496e913e0351eb0e8c37aa3477e40942ec6b97c",
                "Type": "CanonicalUser"
            },
            "Permission": "READ"
        }
    ]
}
$

So I cannot add this in the terraform

resource "aws_s3_bucket_acl" "ec2_ami_read" {
  bucket     = aws_s3_bucket.files.id
  acl        = "aws-exec-read"
  depends_on = [
    aws_s3_bucket_ownership_controls.files,
    aws_s3_bucket_acl.files,
    aws_s3_bucket_acl.files_logs
  ]
}

@valera-yakovenko
Copy link

@sousmangoosta This Seems to be true. I also managed to add a second acl resource with canned acl "log-delivery-write".

resource "aws_s3_bucket_acl" "files" {
  bucket = aws_s3_bucket.files.id
  acl    = "private"
}

resource "aws_s3_bucket_acl" "files-logs" {
  bucket = aws_s3_bucket.files.id
  acl    = "log-delivery-write"
  depends_on = [aws_s3_bucket_acl.files]
}

AND

aws s3api get-bucket-acl --bucket files --out json
{
    "Owner": {
        "DisplayName": "********",
        "ID": "********"
    },
    "Grants": [
        {
            "Grantee": {
                "DisplayName": "********",
                "ID": "********",
                "Type": "CanonicalUser"
            },
            "Permission": "FULL_CONTROL"
        },
        {
            "Grantee": {
                "Type": "Group",
                "URI": "http://acs.amazonaws.com/groups/s3/LogDelivery"
            },
            "Permission": "WRITE"
        },
        {
            "Grantee": {
                "Type": "Group",
                "URI": "http://acs.amazonaws.com/groups/s3/LogDelivery"
            },
            "Permission": "READ_ACP"
        }
    ]
}

It is really strange that I didn't notice this before, I may sware that I was trying to add the second canned acl with a second tf resource(but something went wrong there I assume).
I also saw that adding some additional canned acl do an override\overwrite of existing acl instead of upending them, I really don't know why and how that works....

@valera-yakovenko
Copy link

@sousmangoosta Thanks for your replies, and for pointing me to the proper solution!

nuno407 added a commit to nuno407/terraform-provider-aws that referenced this issue Apr 17, 2024
…#144)

* No issue: Correct integration tests on MDFParser component

In this commit the integration tests of the MDFParser compoenent were correct.
For the tests correction the following was done:
* A refactor was done on the code responsible for loading the service configuration. This was done in order to abstract how the config was loading. Needed for testing

Co-authored-by: Michael Krebs <michael.krebs@bosch.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Requests to existing resources that expand the functionality or scope. service/s3 Issues and PRs that pertain to the s3 service.
Projects
None yet
Development

No branches or pull requests