Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@
"Arn"
]
},
"/*"
"/deploy/here/*"
]
]
}
Expand Down
Binary file not shown.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Large diffs are not rendered by default.

Binary file not shown.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@
"Arn"
]
},
"/*"
"/deploy/here/*"
]
]
},
Expand Down Expand Up @@ -1282,7 +1282,7 @@
"Arn"
]
},
"/*"
"/efs/*"
]
]
}
Expand Down

Large diffs are not rendered by default.

17 changes: 15 additions & 2 deletions packages/aws-cdk-lib/aws-s3-deployment/lib/bucket-deployment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -400,9 +400,13 @@

this.sources = props.sources.map((source: ISource) => source.bind(this, { handlerRole: this.handlerRole }));

this.destinationBucket.grantReadWrite(handler);
const objectPattern = props.destinationKeyPrefix
? `${this.normalizePrefix(props.destinationKeyPrefix)}*`
: '*';

this.destinationBucket.grantReadWrite(handler, objectPattern);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By default prune is true, which means when we remove source files from the CDK app, the lambda tries to clean up the corresponding files in S3. But it fails because it doesn't have delete permissions for paths that are no longer in the app.

I tested this by:

  • deploying a cdk app with stuff in app/frontend and app/backend
  • removing app/backend source and deploying it again
  • second deployment fails because lambda can't delete the old backend files

We should give the lambda delete permissions for the whole bucket (*) to make deletion work properly.

if (props.accessControl) {
this.destinationBucket.grantPutAcl(handler);
this.destinationBucket.grantPutAcl(handler, objectPattern);
}
if (props.distribution) {
handler.addToRolePolicy(new iam.PolicyStatement({
Expand Down Expand Up @@ -645,6 +649,15 @@
const uuid = `BucketDeploymentEFS-VPC-${fileSystemProps.vpc.node.addr}`;
return stack.node.tryFindChild(uuid) as efs.FileSystem ?? new efs.FileSystem(scope, uuid, fileSystemProps);
}
/**
* Normalize the prefix for S3 object keys.
* @param prefix prefix string
* @returns normalized prefix string
*/
normalizePrefix(prefix?: string): string {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can make this a private method.

if (!prefix) return '';
return prefix.replace(/^\/+/, '').replace(/\/+$/, '') + '/';
}
}

export interface DeployTimeSubstitutedFileProps {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1765,3 +1765,40 @@ test('outputObjectKeys default value is true', () => {
OutputObjectKeys: true,
});
});

test.each([
['my-prefix', '/my-prefix/*'],
['my-prefix/', '/my-prefix/*'],
['/my-prefix', '/my-prefix/*'],
['/my-prefix/', '/my-prefix/*'],
[undefined, '/*'],
])('lambda execution role gets permissions with destinationKeyPrefix "%s"', (prefix, expectedSuffix) => {
const stack = new cdk.Stack();
const bucket = new s3.Bucket(stack, 'Dest');
new s3deploy.BucketDeployment(stack, 'Deploy', {
sources: [s3deploy.Source.asset(path.join(__dirname, 'my-website.zip'))],
destinationBucket: bucket,
destinationKeyPrefix: prefix,
});

Template.fromStack(stack).hasResourceProperties('AWS::IAM::Policy', {
PolicyDocument: Match.objectLike({
Statement: Match.arrayWith([
Match.objectLike({
Resource: Match.arrayWith([
{ 'Fn::GetAtt': ['DestC383B82A', 'Arn'] },
{
'Fn::Join': [
'',
[
{ 'Fn::GetAtt': ['DestC383B82A', 'Arn'] },
expectedSuffix,
],
],
},
]),
}),
]),
}),
});
});
Loading