aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.68k stars 3.93k forks source link

AppStagingSynthesizer: BucketDeployment fails copy #26672

Open elgohr opened 1 year ago

elgohr commented 1 year ago

Describe the bug

When using BucketDeployment for deploying a local folder along with AppStagingSynthesizer (@aws-cdk/app-staging-synthesizer-alpha), it fails.

BucketDeployment is used as

new BucketDeployment(this, 'Content', {
      sources: [Source.asset(path.join(__dirname, 'content'))],
      destinationBucket: contentBucket,
      vpc: customVpc,
      vpcSubnets: {
        subnets: customVpc.connectivitySubnets,
      },
});

The custom ressource within BucketDeployment returns

Received response status [FAILED] from custom resource. Message returned: Command '['/opt/awscli/aws', 's3', 'cp', 's3://${CORRECT_BUCKET_ADDRESS}/deploy-time/9ece006bc2680af0997c046110f690bd28ba8f97707060b46e3b7eaeeaa74a12.zip', '/tmp/tmplfzrh7br/f5e46c0a-4142-4006-80d4-53555676295f']' returned non-zero exit status 1. (RequestId: 879bce4c-63f9-4cc6-8da2-ebe1ad652d39)

The same error occurs when separating the asset (like https://docs.aws.amazon.com/cdk/api/v2/docs/app-staging-synthesizer-alpha-readme.html#deploy-time-s3-assets).

Running aws s3 cp s3://${CORRECT_BUCKET_ADDRESS}/deploy-time/9ece006bc2680af0997c046110f690bd28ba8f97707060b46e3b7eaeeaa74a12.zip ./ locally copies the data as expected.

Expected Behavior

It should work as without AppStagingSynthesizer.

Reproduction Steps

Create a deployment that uses BucketDeployment together with AppStagingSynthesizer.

CDK CLI Version

2.90.0 (build 8c535e4)

Node.js Version

v18.16.0

OS

OS X 13.5

Language

Typescript

elgohr commented 1 year ago

Tracked it down. The target bucket of BucketDeployment is encrypted. It looks like the KMS permission for the key is missing. Trying a custom role for BucketDeployment (which wasn't needed before).

pahud commented 1 year ago

Yes custom role should fix that.

elgohr commented 1 year ago

Comparing the cdk key policy, it looks like that it's a little different to the usual cdk-bootstrap key. Old key policy:

{
    "Version": "2008-10-17",
    "Statement": [
        {
            "Effect": "Allow",
            "Principal": {
                "AWS": "arn:aws:iam::{ACCOUNT_ID}:root"
            },
            "Action": [
                "kms:Create*",
                "kms:Describe*",
                "kms:Enable*",
                "kms:List*",
                "kms:Put*",
                "kms:Update*",
                "kms:Revoke*",
                "kms:Disable*",
                "kms:Get*",
                "kms:Delete*",
                "kms:ScheduleKeyDeletion",
                "kms:CancelKeyDeletion",
                "kms:GenerateDataKey",
                "kms:TagResource",
                "kms:UntagResource"
            ],
            "Resource": "*"
        },
        {
            "Effect": "Allow",
            "Principal": {
                "AWS": "*"
            },
            "Action": [
                "kms:Decrypt",
                "kms:DescribeKey",
                "kms:Encrypt",
                "kms:ReEncrypt*",
                "kms:GenerateDataKey*"
            ],
            "Resource": "*",
            "Condition": {
                "StringEquals": {
                    "kms:CallerAccount": "{ACCOUNT_ID}",
                    "kms:ViaService": "s3.eu-west-1.amazonaws.com"
                }
            }
        },
        {
            "Effect": "Allow",
            "Principal": {
                "AWS": "arn:aws:iam::{ACCOUNT_ID}:role/cdk-hnb659fdf-file-publishing-role-{ACCOUNT_ID}-eu-west-1"
            },
            "Action": [
                "kms:Decrypt",
                "kms:DescribeKey",
                "kms:Encrypt",
                "kms:ReEncrypt*",
                "kms:GenerateDataKey*"
            ],
            "Resource": "*"
        }
    ]
}

AppStagingSynthesizer KMS key policy:

{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Effect": "Allow",
            "Principal": {
                "AWS": "arn:aws:iam::{ACCOUNT_ID}:root"
            },
            "Action": "kms:*",
            "Resource": "*"
        },
        {
            "Effect": "Allow",
            "Principal": {
                "AWS": "arn:aws:iam::{ACCOUNT_ID}:root"
            },
            "Action": [
                "kms:Create*",
                "kms:Describe*",
                "kms:Enable*",
                "kms:List*",
                "kms:Put*",
                "kms:Update*",
                "kms:Revoke*",
                "kms:Disable*",
                "kms:Get*",
                "kms:Delete*",
                "kms:TagResource",
                "kms:UntagResource",
                "kms:ScheduleKeyDeletion",
                "kms:CancelKeyDeletion"
            ],
            "Resource": "*"
        }
    ]
}

So adding

{
            "Effect": "Allow",
            "Principal": {
                "AWS": "*"
            },
            "Action": [
                "kms:Decrypt",
                "kms:DescribeKey",
                "kms:Encrypt",
                "kms:ReEncrypt*",
                "kms:GenerateDataKey*"
            ],
            "Resource": "*",
            "Condition": {
                "StringEquals": {
                    "kms:CallerAccount": "{ACCOUNT_ID}",
                    "kms:ViaService": "s3.eu-west-1.amazonaws.com"
                }
            }
        },

into AppStagingSynthesizer KMS key policy would solve the issue.

BrianFarnhill commented 1 year ago

I resolved this by using the custom role approach and just giving it this inline policy for the decryption to work (I didn't want to do a lookup for the KMS key to get it by the alias), which uses a similar condition that elgohr listed above:

        new PolicyStatement({
          actions: ['kms:Decrypt'],
          resources: ['*'],
          conditions: {
            StringEquals: {
              'kms:ViaService': `s3.${Aws.REGION}.amazonaws.com`,
            },
            'ForAnyValue:StringEquals': {
              'kms:ResourceAliases': `alias/cdk-${appId}-staging`,
            },
          },
        }),

I figure this at least keeps it to just that role, and it uses the alias of the key to control the permissions as wellas the s3 service call - not as bullet proof as having the ARN of the key but I felt like it was an OK alternative in the mean time.