aws-cloudformation / rain

A development workflow tool for working with AWS CloudFormation.
Apache License 2.0
750 stars 69 forks source link

rain package fails uploading to a Cross Account Bucket that does not support ACLs #388

Closed georgealton closed 1 month ago

georgealton commented 1 month ago

Hey! Thanks for rain - I've been using it in a few places to format templates, and it's been really helpful.

I was just looking to migrate some of my templates to use rain for packaging, but found it's incompatible with the buckets I've use for my infrastructure.

The Buckets I have are all set to have object owner enforced. When it comes to upload objects it rain tries to set the PrivateCannedACL on them: https://github.com/aws-cloudformation/rain/blob/32014d665e6c44a4794ee524add9fa659d2b44e5/internal/aws/s3/s3.go#L159-L164

This is rejected by the Bucket Ownership Controls and causes the upload to fail. I'd love to see rain support these buckets.

Here's the output from the console

$ brew install rain
==> Downloading https://ghcr.io/v2/homebrew/core/rain/manifests/1.8.6
==> Fetching rain
==> Downloading https://ghcr.io/v2/homebrew/core/rain/blobs/sha256:bcf377e7c51314276be817a2d6140a429f67eb752ff7c14c683fd741aea482b4
==> Pouring rain--1.8.6.x86_64_linux.bottle.tar.gz
==> Caveats
Deploying CloudFormation stacks with rain requires the AWS CLI to be installed.
All other functionality works without the AWS CLI.

Bash completion has been installed to:
  /home/linuxbrew/.linuxbrew/etc/bash_completion.d
==> Summary
🍺  /home/linuxbrew/.linuxbrew/Cellar/rain/1.8.6: 9 files, 36.3MB
==> Running `brew cleanup rain`...
Disable this behaviour by setting HOMEBREW_NO_INSTALL_CLEANUP.
Hide these hints with HOMEBREW_NO_ENV_HINTS (see `man brew`).
$ rain package "$SRC_TEMPLATE" \
$    --s3-bucket "$BUCKET" \
$    --s3-prefix "$AWS_ACCOUNT_ID" \
$    --output "$PACKAGED_TEMPLATE"
unable to package template './infra/template.cfn.yaml': The bucket does not allow ACLs
georgealton commented 1 month ago

Here's a scripted repro. The ACLs Denied Response happens when you're PutObjects Cross Account.

#!/bin/bash
ORG_ID="$1"
BUCKETS_ACCOUNT_PROFILE="$2"
DIFFERENT_ACCOUNT="$3"

aws cloudformation create-stack --parameters "ParameterKey=OrgId,ParameterValue=$ORG_ID" --stack-name 'rain-issue-338' --profile $BUCKETS_ACCOUNT_PROFILE --template-body '{
    "Parameters": {
        "OrgId":{
            "Type": "String"
        }
    },
    "Resources":
        {
            "Enforced":{
                "Type":"AWS::S3::Bucket",
                "Properties": {
                    "OwnershipControls": {
                        "Rules": [
                            {
                                "ObjectOwnership": "BucketOwnerEnforced"
                            }
                        ]
                    }
                }
            },
                "BucketPolicy": {
                    "Type": "AWS::S3::BucketPolicy",
                    "Properties": {
                        "Bucket": {"Ref": "Enforced"},
                        "PolicyDocument": {
                            "Version": "2012-10-17",
                            "Statement": [
                                {
                                    "Effect": "Allow",
                                    "Principal": {
                                        "AWS": "*"
                                    },
                                    "Action": "s3:ListBucket",
                                    "Resource": { "Fn::Sub": "${Enforced.Arn}"},
                                    "Condition": {
                                        "StringEquals": {
                                            "aws:PrincipalOrgId": {"Ref": "OrgId"}
                                        }
                                    }
                                },
                                {
                                    "Action": [
                                        "s3:GetObject",
                                        "s3:PutObject"
                                    ],
                                    "Effect": "Allow",
                                    "Resource": { "Fn::Sub": "${Enforced.Arn}/*"},
                                    "Principal": {
                                        "AWS": "*"
                                    },
                                    "Condition": {
                                        "StringEquals": {
                                            "aws:PrincipalOrgId": {"Ref": "OrgId"}
                                        }
                                    }
                                }
                            ]
                        }
                    }
                }
        },
        "Outputs": {
            "Enforced": {"Value": {"Ref": "Enforced"}}
        }
    }
'
aws cloudformation wait stack-create-complete --stack-name rain-issue-338

ENFORCED_BUCKET=$(aws cloudformation describe-stacks --profile $BUCKETS_ACCOUNT_PROFILE --stack-name 'rain-issue-338' --query 'Stacks[0].Outputs[?OutputKey==`Enforced`].OutputValue' --output text)

echo '{"Resources":{"A":{"Type":"AWS::CloudFormation::WaitConditionHandle"}}}' > inner_template.cfn.json
echo '{"Resources":{"Stack": {"Type": "AWS::CloudFormation::Stack", "Properties": {"TemplateURL": "inner_template.cfn.json"}}}}}' > template.cfn.json

rain package template.cfn.json --debug --s3-bucket "${ENFORCED_BUCKET}" --s3-prefix test --profile "${BUCKETS_ACCOUNT_PROFILE}"
rain package template.cfn.json --debug --s3-bucket "${ENFORCED_BUCKET}" --s3-prefix test --profile "${DIFFERENT_ACCOUNT}"

Output:

DEBUG: Packaging template 'template.cfn.json'
DEBUG: No changes this pass: 1
DEBUG: Uploading: /var/folders/vy/cgcny6kx3s34w1q915gvh_zr0000gn/T/3996781204.template

DEBUG: Loading AWS config
DEBUG: Artifact bucket: rain-issue-338-enforced-qdeof2doiaip
DEBUG: Artifact key: test/2b42f26d17559924ba17ca9d43a0a05ba5b55da4a6cf36e956f8c48aa84efd51
DEBUG: Need another pass: 1
DEBUG: No changes this pass: 2
Resources:
  Stack:
    Type: AWS::CloudFormation::Stack
    Properties:
      TemplateURL: https://rain-issue-338-enforced-qdeof2doiaip.s3.us-east-1.amazonaws.com/test/2b42f26d17559924ba17ca9d43a0a05ba5b55da4a6cf36e956f8c48aa84efd51

DEBUG: Packaging template 'template.cfn.json'
DEBUG: No changes this pass: 1
DEBUG: Uploading: /var/folders/vy/cgcny6kx3s34w1q915gvh_zr0000gn/T/1545025507.template

DEBUG: Loading AWS config
DEBUG: Artifact bucket: rain-issue-338-enforced-qdeof2doiaip
DEBUG: Artifact key: test/2b42f26d17559924ba17ca9d43a0a05ba5b55da4a6cf36e956f8c48aa84efd51
DEBUG: Error handling S3: operation error S3: PutObject, https response error StatusCode: 400, RequestID: 8HER03Y6KR811KFZ, HostID: TQJ7r8iJz6hUS9jf8c6gLVeCFOAVdrSpFUufjDwWHubFqp9KwRC/Nar8nPqOPADw3tw7iLPLol8=, api error AccessControlListNotSupported: The bucket does not allow ACLs

DEBUG: No changes this pass: 1
Resources:
  Stack:
    Type: AWS::CloudFormation::Stack
    Properties:
      TemplateURL: inner_template.cfn.json

So running rain from $DIFFERENT_ACCOUNT the error:

DEBUG: Error handling S3: operation error S3: PutObject, https response error StatusCode: 400, RequestID: 8HER03Y6KR811KFZ, HostID: TQJ7r8iJz6hUS9jf8c6gLVeCFOAVdrSpFUufjDwWHubFqp9KwRC/Nar8nPqOPADw3tw7iLPLol8=, api error AccessControlListNotSupported: The bucket does not allow ACLs

So my architectures has some centralised IAC Buckets and every account is allowed to write objects starting with their "${aws:PrincipalAccountId}". That's why in my rain package command I used the --s3-prefix option:

$ rain package "$SRC_TEMPLATE" \
$    --s3-bucket "$BUCKET" \
$    --s3-prefix "$AWS_ACCOUNT_ID" \
$    --output "$PACKAGED_TEMPLATE"
ericzbeard commented 1 month ago

I'm not sure why we're even putting an ACL on the bucket, that's a bit of an outdated mechanism at this point.

georgealton commented 1 month ago

BucketOwnerEnforced certainly made my life a lot easier, not having to explain the bucket policy and acl difference was great.

I set BucketOwnerEnforced on all my Buckets now.

What would be your preference out of handling the error by retrying the PutObject without an ACL, or dropping the ACL from PutObject entirely?

ericzbeard commented 1 month ago

I think we should drop the ACL entirely. I need to go back and make sure the buckets rain creates are secure by default without ACLs, and then think through your scenario. Have to be careful here to not to create anything that's open to the world by mistake.