feat: S3_hosting create CF but use existing bucket

[davidcheung]

this is useful for multiple CF pointing to one bucket,
user can manually create the s3 bucket policy allowing both CF to view
using their assets acess identity

Description

Please explain the changes you made here and link to any relevant issues.

Checklist

• Validation tests are passing
• README.md has been updated after any changes to variables and outputs. See https://github.com/commitdev/terraform-aws-zero/#doc-generation

[github-actions]

Terraform Format and Style 🖌success

Terraform Initialization ⚙️success

Terraform Validation 🤖Success! The configuration is valid.

[bmonkman]

Sheesh, why is this module using camel case locals?.. 😢

[bmonkman]

Is there a use case where you would want to use an existing bucket but also create a policy? Seems like if the bucket already exists, a policy should also exist.

[bmonkman]

Actually wait, it's probably the other way around.. If you're associating CF with an existing bucket is there any case where it would already have a policy?

[bmonkman]

Oh right, it's just a single policy, you can't append multiple, so you'd need to have a single policy with mulitple principals.

[davidcheung]

yeah, i originally tried to have another variable allowing extra_s3_bucket_access_principal_arn
but then I'd end up with both modules depending on each other. so i just added this option to let people do it manually

module s3_hosting "foo" {
  // creating bucket
  // depends on the output of the 2nd s3_hosting module
  extra_s3_bucket_access_principal_arn = module.s3_hosting.bar.asset_access_identity_arn 
}

module s3_hosting "bar" {
  // using bucket foo
  // depends on the bucket to be created

 // outputs `module.s3_hosting.bar.asset_access_identity_arn`
}

[bmonkman]

When you're implementing the real world version of this how are you setting the bucket policy?

[davidcheung]

I had both the module set to create_s3_bucket_policy = false
then i have the entire policy json / resource outside with the s3 bucket as data

data "aws_s3_bucket" "client_assets" {
  bucket = "deployables.zero-david.online"
}

resource "aws_s3_bucket_policy" "client_assets" {
  depends_on = [module.static_pages_cf_only, module.deployables]
  bucket     = data.aws_s3_bucket.client_assets.id
  policy     = data.aws_iam_policy_document.assets_origin.json
}

data "aws_iam_policy_document" "assets_origin" {
  statement {
    actions   = ["s3:GetObject"]
    resources = ["arn:aws:s3:::${data.aws_s3_bucket.client_assets.id}/*"]

    principals {
      type        = "AWS"
      identifiers = [module.static_pages_cf_only.cf_origin_assets_access_identity_arn, module.deployables.cf_origin_assets_access_identity_arn]
    }
  }

  statement {
    actions   = ["s3:ListBucket"]
    resources = ["arn:aws:s3:::${data.aws_s3_bucket.client_assets.id}"]

    principals {
      type        = "AWS"
      identifiers = [module.static_pages_cf_only.cf_origin_assets_access_identity_arn, module.deployables.cf_origin_assets_access_identity_arn]
    }
  }
}

[bmonkman]

Should be fine, though we may need some documentation around it since it happens outside the modules. Also you probably want to add a depends_on to your aws_s3_bucket data source.

[bmonkman]

Some grammar / typo issues here..
How about something like "Useful when multiple CF distributions access the same bucket. You would need to create a bucket policy that allows access from multiple distributions"

[github-actions]

Terraform Format and Style 🖌success

Terraform Initialization ⚙️success

Terraform Validation 🤖Success! The configuration is valid.