Add s3 dev uploads bucket

[Wynndow]

This is to add a single bucket, digitalmarketplace-dev-uploads to the dev aws account. This bucket will be used for all operations in the dev environment instead of separate buckets for separate upload types - see this PR for more details.

The policy assigned to the bucket should allow any user in the main account all bucket object operations. I don't think it will allow any permissions for bucket operations (such as deleting the bucket or it's policies).

I've tried to keep the module fairly generic, but how much scope there is in future for creating more s3 buckets with this particular policy I'm not sure. It may be worth just moving this bucket resource and policy to the dm-aws-dev directory instead of importing the module. Thoughts are most welcome.

[bandesz]

The overall module name "s3-bucket" is too general, you should rename it to "dev-s3-bucket" or similar.

[galund]

Because I don't know anything about terraform, my only contribution here is (trivially) to complain that 'EOF' is a confusing heredoc terminator, and END_POLICY might be better :)

[Wynndow]

Yeah that makes sense. I don't actually know what heredoc is, was just copying other examples! I think you can use POLICY instead of EOF which would be better.

[bandesz]

The Terraform examples use it like this, probably because this could be a file reference, so we can say this is an inline policy file.

[bandesz]

This solution only covers one side, to set up S3 cross account access you also have to give permission to the users to the S3 bucket from the other side as well.

Check the complete solution in this PR: #248

You also asked about how to delete managed S3 buckets with Terraform: you have to set the force_destroy = true parameter on those resources first.

[bandesz]

this is too broad, this should be:

{
   "Version": "2012-10-17",
   "Statement": [
      {
        "Principal": {
           "AWS": "arn:aws:iam::${var.access_account_id}:root"
        },
        "Action": [
          "s3:ListBucket",
          "s3:GetBucketLocation"
        ],
        "Effect": "Allow",
        "Resource": "arn:aws:s3:::${var.bucket_name}"
      },
      {
        "Principal": {
           "AWS": "arn:aws:iam::${var.access_account_id}:root"
        },
        "Action": [
          "s3:DeleteObject",
          "s3:GetObject",
          "s3:PutObject"
        ],
        "Effect": "Allow",
        "Resource": "arn:aws:s3:::${var.bucket_name}/*"
      }
   ]
}

[Wynndow]

Why are the List and Get actions required on the bucket? What would a user not be able to do without them?

[bandesz]

You mean the ListBucket and GetBucketLocation ones? ListBucket allows you to list the contents of the bucket, the GetBucketLocation helps you find the region for the bucket (although this is only needed probably when the region is not explicitly set in the client)

See: http://docs.aws.amazon.com/IAM/latest/UserGuide/access_policies_examples.html#iam-policy-example-s3

[Wynndow]

Yeah they're the ones. So does this mean that you can ListBucket and GetBucketLocation from the aws cli? Or the console as well? And if the bucket was only going to be used via the apps, would you still need these permissions; if the apps know which bucket to use is being able to Delete/Get/PutObject enough?

[bandesz]

The console uses the same IAM permissions.

The list permission changes the behaviour of non-existing items in the bucket:

• if you don't have listing permission then a non-existing item responds with 403, if you have it it's the expected 404.

[bandesz]

In this case the module name should be "dev_uploads_s3_bucket" or similar. Also it's a good rule to use underscores instead of dashes in module/resource names.

[Wynndow]

If I rename the module to something less broad, like dev-s3-bucket, does it make sense to just define the bucket resource within root/aws-dm-dev as it's the only environment where it's actually going to be used? Or is it better practise to define everything in modules and import them, even if that module would only ever be used once?

[bandesz]

If the module is not reused then I would say it's even better if you just define it directly in aws-dm-dev.

[Wynndow]

Cool. My original thought was to just have an s3-bucket module which could then be pulled in by any of the environments and parameterised to suit, but as you can't reuse the same name I don't think it'd work. I think this bucket/policy is quite specific so I'll move it in.

[Wynndow]

@bandesz I've made a few changes based on the above, if you could let me know what you think? It plans fine on the aws-dm-dev side of things, but I don't have permissions on the aws-dm side.

[bandesz]

Looks good!