Skip to content

Commit

Permalink
fix(s3-deployment): default role does not get PutAcl permissions on… (
Browse files Browse the repository at this point in the history
#20492)

… destination bucket when used with accessControl

With the feature flag `@aws-cdk/aws-s3:grantWriteWithoutAcl` you no longer get `s3:PutObjectAcl` and `s3:PutObjectVersionAcl` permissions in the default role. These are however required when using the `accessControl` property.


----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
fouxarn committed May 25, 2022
1 parent 75bfce7 commit 3e6ec5c
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 0 deletions.
3 changes: 3 additions & 0 deletions packages/@aws-cdk/aws-s3-deployment/lib/bucket-deployment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,9 @@ export class BucketDeployment extends CoreConstruct {
const sources: SourceConfig[] = props.sources.map((source: ISource) => source.bind(this, { handlerRole }));

props.destinationBucket.grantReadWrite(handler);
if (props.accessControl) {
props.destinationBucket.grantPutAcl(handler);
}
if (props.distribution) {
handler.addToRolePolicy(new iam.PolicyStatement({
effect: iam.Effect.ALLOW,
Expand Down
40 changes: 40 additions & 0 deletions packages/@aws-cdk/aws-s3-deployment/test/bucket-deployment.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -708,6 +708,46 @@ testFutureBehavior('lambda execution role gets permissions to read from the sour
});
});

testFutureBehavior('lambda execution role gets putObjectAcl permission when deploying with accessControl', s3GrantWriteCtx, cdk.App, (app) => {
// GIVEN
const stack = new cdk.Stack(app);
const source = new s3.Bucket(stack, 'Source');
const bucket = new s3.Bucket(stack, 'Dest');

// WHEN
new s3deploy.BucketDeployment(stack, 'Deploy', {
sources: [s3deploy.Source.bucket(source, 'file.zip')],
destinationBucket: bucket,
accessControl: s3.BucketAccessControl.PUBLIC_READ,
});

// THEN
const map = Template.fromStack(stack).findResources('AWS::IAM::Policy');
expect(map).toBeDefined();
const resource = map[Object.keys(map)[0]];
expect(resource.Properties.PolicyDocument.Statement).toContainEqual({
Action: [
's3:PutObjectAcl',
's3:PutObjectVersionAcl',
],
Effect: 'Allow',
Resource: {
'Fn::Join': [
'',
[
{
'Fn::GetAtt': [
'DestC383B82A',
'Arn',
],
},
'/*',
],
],
},
});
});

test('memoryLimit can be used to specify the memory limit for the deployment resource handler', () => {
// GIVEN
const stack = new cdk.Stack();
Expand Down

0 comments on commit 3e6ec5c

Please sign in to comment.