Feature Request - S3 ACL Policy Grant

[forestoden]

Hi there,

We have a requirement to implement Bucket ACLs on a few buckets in S3 and have been using this module for other buckets we have created, so we'd like to keep some consistency if possible. It looks like this module doesn't support grants (https://www.terraform.io/docs/providers/aws/r/s3_bucket.html#grant).

Is this not supported by design? I know there's some weirdness with acl vs grant using the S3 Resources. I have some time over the weekend and might be able to work on this, but if it's purposefully not supported I wouldn't want to waste my time

[antonbabenko]

Hi @forestoden !

@Chhed13 finally got his PR hashicorp/terraform-provider-aws#3728 merged (congrats & 🎉🎉🎉🎉🎉🎉🎉 )

Since that time this module didn't get enough attention from me or other community member to add support for grant which was introduced last week (see CHANGELOG for v2.52.0).

If you have a possibility to add it, please do. Please keep in mind potential breaking change (see hashicorp/terraform-provider-aws#12332) and make a proper workaround, if necessary.

[forestoden]

Oh wow, I didn't realize it was that new or that the PR had been open for so long! We just got the requirement this week, perfect timing I guess.

I'll come up to speed with that breaking change. This is my first time contributing to an open source Terraform Module, I don't see any test suites, is testing just done manually?

[antonbabenko]

Testing = run code in examples.

[forestoden]

Quick update, I was able to get the code in place for this but unfortunately that bug hashicorp/terraform-provider-aws#12332 looks like it has the potential to cause issues even if users aren't setting grant.

I tried a couple workarounds but couldn't get anything to work quite right. I'll monitor that issue, but for now I'm stuck. Once that gets fixed, I'll come back to this

[Chhed13]

Hi there,
as I know some about the grant feature, I'll try to describe it here.
grant is the permission block that is always present on the bucket. Till version 2.52 of aws provider terraform didn't track this block and as result - ignores it. Begin from version 2.52 terraform start two-side sync this block, and because even if you didn't set any of 'grant' - default policy is always present.
As alternative mechanics terraform always use simplified version called acl. But it only works in style of force-push. You can set acl, but you get only grant objects and there is no strait-forward rule to translate one to another for all cases. In the default case it's event not possible to determine which one were requested by terraform - acl or grant, because in terraform acl = "private" is the default behavior.
The default case in terms of grant (synonym to acl = "private") is:

data "aws_canonical_user_id" "current" {}
resource "aws_s3_bucket" "bucket" {
	bucket = "bucket-name"
	grant {
            id = "${data.aws_canonical_user_id.current.id}"
            type = "CanonicalUser"
            permissions = ["FULL_CONTROL"]
    }
}

Avoid it, because it causes state flapping.
I think all this tricky things of AWS API keeps terraform from implementing this feature for so long.
I can't reproduce hashicorp/terraform-provider-aws#12332 bug fast, and don't have enough time to go deeper now, but if you show exact problems maybe I can help.

[forestoden]

Hey @Chhed13 I just pushed my code to my fork here: https://github.com/forestoden/terraform-aws-s3-bucket/tree/add-acl-grants if you wanted to take a look but I'll try to describe the problem and I was able to reproduce it off my fork.

1. Pin AWS provider version to v2.51.0 and deploy an S3 bucket from the examples/complete-grant example. Note: you'll have to comment out the grant bit in the module definition, as obviously v2.51.0 doesn't support that

2. Manually add a grant to the S3 bucket. Because the AWS Provider didn't support this before, this will get us into a state that users who are using Grants with the module would be at.

3. Upgrade AWS provider to v2.52.0 and uncomment code to add grants support in this module.

4. Re-apply terraform and you will see Terraform wants to remove the FULL_CONTROL grant for the owner user

My plan looks something like this

      - grant {
          - id          = "<redacted>" -> null
          - permissions = [
              - "READ",
              - "READ_ACP",
            ] -> null
          - type        = "CanonicalUser" -> null
        }
      + grant {
          + id          = "<redacted>"
          + permissions = [
              + "READ",
              + "READ_ACP",
            ]
          + type        = "CanonicalUser"
        }
      - grant {
          - id          = "<owner-redacted>" -> null
          - permissions = [
              - "FULL_CONTROL",
            ] -> null
          - type        = "CanonicalUser" -> null
        }

I tried adding a grant in TF to configure the owner with FULL_CONTROL but even that shows a diff, and it sounds like you're saying that's not even recommended

[forestoden]

Hey @Chhed13 I just pushed my code to my fork here: https://github.com/forestoden/terraform-aws-s3-bucket/tree/add-acl-grants if you wanted to take a look but I'll try to describe the problem and I was able to reproduce it off my fork.

1. Pin AWS provider version to v2.51.0 and deploy an S3 bucket from the examples/complete-grant example. Note: you'll have to comment out the grant bit in the module definition, as obviously v2.51.0 doesn't support that

2. Manually add a grant to the S3 bucket. Because the AWS Provider didn't support this before, this will get us into a state that users who are using Grants with the module would be at.

3. Upgrade AWS provider to v2.52.0 and uncomment code to add grants support in this module.

4. Re-apply terraform and you will see Terraform wants to remove the FULL_CONTROL grant for the owner user

My plan looks something like this

      - grant {
          - id          = "<redacted>" -> null
          - permissions = [
              - "READ",
              - "READ_ACP",
            ] -> null
          - type        = "CanonicalUser" -> null
        }
      + grant {
          + id          = "<redacted>"
          + permissions = [
              + "READ",
              + "READ_ACP",
            ]
          + type        = "CanonicalUser"
        }
      - grant {
          - id          = "<owner-redacted>" -> null
          - permissions = [
              - "FULL_CONTROL",
            ] -> null
          - type        = "CanonicalUser" -> null
        }

I tried adding a grant in TF to configure the owner with FULL_CONTROL but even that shows a diff, and it sounds like you're saying that's not even recommended

[Chhed13]

Hi,
now I understand the problem.

1. First of all - section of FULL_CONTOL can and should present if you plan to use grant feature. It is not recommended only in form that I described alone. That is ok to use in all other combinations like you show here
2. Why re-creation - the main reason: when terraform calculate the hash of your objects - it doesn't match with expected ones. I didn't trace this deeper now - I'll try take look later today. But I'm pretty sure that it'll recreate it in place and every thing will be ok.
   Of course that's not a solution for clients - so I'll debug it.

[Chhed13]

Ok, I can see an issue.
For reproduce you don't need switch 2.51 -> 2.52. It is reproducible the same sort on 2.52 solo.
I see that logic of creation grant is correct and it looks like a sorting issue inside terraform state.
I was able to get zero diff with you example, but I had to make my terraform config 100% match including ordering.
The ordering issue looks strange because storage of grant block made on hashes, not lists.

[flavioneri]

Hi, I'm using the version 2.51.0 as a workaround.
Looking forward to getting this resolved.

Thank you, guys.

[jamessthompson]

Is there any idea if/when this will be merged? I'd like to use this module but the lack of grant support is an issue for me.

[antonbabenko]

@jamessthompson If there is anyone who can make a PR adding this feature to the module, we will have it. I don't have time to make this myself.

Anyone?

[forestoden]

I've been following hashicorp/terraform-provider-aws#12332 hoping for a resolution there before coming back to this. I don't have the time to try to work around that bug in order to push this through

[so0k]

For my module I'm doing the following

  # fix for https://github.com/terraform-aws-modules/terraform-aws-s3-bucket/issues/20#issuecomment-601564334
  # don't generate this grant if there are no additional grants T_T
  dynamic "grant" {
    for_each = length(var.additional_grants) > 0 ? [ data.aws_canonical_user_id.current.id ] : []
    content {
      id          = grant.key
      type        = "CanonicalUser"
      permissions = ["FULL_CONTROL"]
    }
  }

  dynamic "grant" {
    for_each = var.additional_grants
    content {
      type        = grant.value.type
      permissions = grant.value.permissions
      id          = grant.value.id
      uri         = grant.value.uri
    }
  }

[kkost]

This is great but I have a slightly different problem and I'm curious if anyone else runs in to this. (tldr; I think my problem would be resolved by having a separate resource to create grants, e.g. aws_s3_bucket_grant or something, sort of like how there is a separate resource for aws_s3_bucket_policy).

I have a bucket in a primary account that I plan to use in several workspaces (pulled in as a data resource using a specific provider). I want to add a separate grant for each workspace (each workspace uses a separate account).

I envision something like this-

data "aws_s3_bucket" "bucket" {
  provider = aws.main
  bucket = "bucket"
}

data "aws_canonical_user_id" "current" {
  provider = aws.subaccount
}

resource "aws_s3_bucket_grant" "grant" {
  provider = aws.main
  bucket = data.aws_s3_bucket.name
  grant {
    type = "CanonicalUser"
    permissions = ["READ"]
    id = data.aws_canonical_user_id.current.id
  }
}

EDIT: For a workaround, I am cooking up an external resource to call a script to "append" this out-of-band.

[github-actions]

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.