Skip to content

Commit

Permalink
fix(s3): bucket is not emptied before update when the name changes (#…
Browse files Browse the repository at this point in the history 
…16203)

Changing the bucket name leads CloudFormation to try to delete the bucket and create a new one with the new name. If the bucket is not empty, the deployment will fail. With this change, the custom resource will clear the old bucket when it detects that there has been a name change.

NB: this custom resource is created only when `autoDeleteObjects: true` is passed to the bucket.

Fixes #14011.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
@otaviomacedo
otaviomacedo authored Aug 25, 2021
1 parent e678f10 commit b1d69d7
Show file tree
Hide file tree
Showing 4 changed files with 139 additions and 24 deletions.
18 changes: 9 additions & 9 deletions packages/@aws-cdk/aws-kinesisfirehose-destinations/test/integ.s3-bucket.lit.expected.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@
"Properties": {
"Code": {
"S3Bucket": {
"Ref": "AssetParameters1a8becf42c48697a059094af1e94aa6bc6df0512d30433db8c22618ca02dfca1S3BucketF01ADF6B"
"Ref": "AssetParametersfe5df38824187483182e1459db47adfae2a515aa4caedd437fc4033a0c5b3de9S3BucketD715D8B0"
},
"S3Key": {
"Fn::Join": [
Expand All @@ -115,7 +115,7 @@
"Fn::Split": [
"||",
{
"Ref": "AssetParameters1a8becf42c48697a059094af1e94aa6bc6df0512d30433db8c22618ca02dfca1S3VersionKey6FC34F51"
"Ref": "AssetParametersfe5df38824187483182e1459db47adfae2a515aa4caedd437fc4033a0c5b3de9S3VersionKey6E76822C"
}
]
}
Expand All @@ -128,7 +128,7 @@
"Fn::Split": [
"||",
{
"Ref": "AssetParameters1a8becf42c48697a059094af1e94aa6bc6df0512d30433db8c22618ca02dfca1S3VersionKey6FC34F51"
"Ref": "AssetParametersfe5df38824187483182e1459db47adfae2a515aa4caedd437fc4033a0c5b3de9S3VersionKey6E76822C"
}
]
}
Expand Down Expand Up @@ -751,17 +751,17 @@
}
},
"Parameters": {
"AssetParameters1a8becf42c48697a059094af1e94aa6bc6df0512d30433db8c22618ca02dfca1S3BucketF01ADF6B": {
"AssetParametersfe5df38824187483182e1459db47adfae2a515aa4caedd437fc4033a0c5b3de9S3BucketD715D8B0": {
"Type": "String",
"Description": "S3 bucket for asset \"1a8becf42c48697a059094af1e94aa6bc6df0512d30433db8c22618ca02dfca1\""
"Description": "S3 bucket for asset \"fe5df38824187483182e1459db47adfae2a515aa4caedd437fc4033a0c5b3de9\""
},
"AssetParameters1a8becf42c48697a059094af1e94aa6bc6df0512d30433db8c22618ca02dfca1S3VersionKey6FC34F51": {
"AssetParametersfe5df38824187483182e1459db47adfae2a515aa4caedd437fc4033a0c5b3de9S3VersionKey6E76822C": {
"Type": "String",
"Description": "S3 key for asset version \"1a8becf42c48697a059094af1e94aa6bc6df0512d30433db8c22618ca02dfca1\""
"Description": "S3 key for asset version \"fe5df38824187483182e1459db47adfae2a515aa4caedd437fc4033a0c5b3de9\""
},
"AssetParameters1a8becf42c48697a059094af1e94aa6bc6df0512d30433db8c22618ca02dfca1ArtifactHash9ECACDFD": {
"AssetParametersfe5df38824187483182e1459db47adfae2a515aa4caedd437fc4033a0c5b3de9ArtifactHash9AE3702B": {
"Type": "String",
"Description": "Artifact hash for asset \"1a8becf42c48697a059094af1e94aa6bc6df0512d30433db8c22618ca02dfca1\""
"Description": "Artifact hash for asset \"fe5df38824187483182e1459db47adfae2a515aa4caedd437fc4033a0c5b3de9\""
},
"AssetParameters5ee078f2a1957fe672d6cfd84faf49e07b8460758b5cd2669b3df1212a14cd19S3BucketFEDDFB43": {
"Type": "String",
Expand Down
24 changes: 19 additions & 5 deletions packages/@aws-cdk/aws-s3/lib/auto-delete-objects-handler/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,25 @@ const s3 = new S3();
export async function handler(event: AWSLambda.CloudFormationCustomResourceEvent) {
switch (event.RequestType) {
case 'Create':
case 'Update':
return;
case 'Update':
return onUpdate(event);
case 'Delete':
return onDelete(event);
return onDelete(event.ResourceProperties?.BucketName);
}
}

async function onUpdate(event: AWSLambda.CloudFormationCustomResourceEvent) {
const updateEvent = event as AWSLambda.CloudFormationCustomResourceUpdateEvent;
const oldBucketName = updateEvent.OldResourceProperties?.BucketName;
const newBucketName = updateEvent.ResourceProperties?.BucketName;
const bucketNameHasChanged = newBucketName != null && oldBucketName != null && newBucketName !== oldBucketName;

/* If the name of the bucket has changed, CloudFormation will try to delete the bucket
and create a new one with the new name. So we have to delete the contents of the
bucket so that this operation does not fail. */
if (bucketNameHasChanged) {
return onDelete(oldBucketName);
}
}

Expand All @@ -23,7 +38,7 @@ async function emptyBucket(bucketName: string) {
const contents = [...listedObjects.Versions ?? [], ...listedObjects.DeleteMarkers ?? []];
if (contents.length === 0) {
return;
};
}

const records = contents.map((record: any) => ({ Key: record.Key, VersionId: record.VersionId }));
await s3.deleteObjects({ Bucket: bucketName, Delete: { Objects: records } }).promise();
Expand All @@ -33,8 +48,7 @@ async function emptyBucket(bucketName: string) {
}
}

async function onDelete(deleteEvent: AWSLambda.CloudFormationCustomResourceDeleteEvent) {
const bucketName = deleteEvent.ResourceProperties?.BucketName;
async function onDelete(bucketName?: string) {
if (!bucketName) {
throw new Error('No BucketName was provided.');
}
Expand Down
103 changes: 102 additions & 1 deletion packages/@aws-cdk/aws-s3/test/auto-delete-objects-handler.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,51 @@ test('does nothing on create event', async () => {
expect(mockS3Client.deleteObjects).toHaveBeenCalledTimes(0);
});

test('does nothing on update event', async () => {
test('does nothing on update event when everything remains the same', async () => {
// GIVEN
const event: Partial<AWSLambda.CloudFormationCustomResourceUpdateEvent> = {
RequestType: 'Update',
ResourceProperties: {
ServiceToken: 'Foo',
BucketName: 'MyBucket',
},
OldResourceProperties: {
ServiceToken: 'Foo',
BucketName: 'MyBucket',
},
};

// WHEN
await invokeHandler(event);

// THEN
expect(mockS3Client.listObjectVersions).toHaveBeenCalledTimes(0);
expect(mockS3Client.deleteObjects).toHaveBeenCalledTimes(0);
});

test('does nothing on update event when the bucket name remains the same but the service token changes', async () => {
// GIVEN
const event: Partial<AWSLambda.CloudFormationCustomResourceUpdateEvent> = {
RequestType: 'Update',
ResourceProperties: {
ServiceToken: 'Foo',
BucketName: 'MyBucket',
},
OldResourceProperties: {
ServiceToken: 'Bar',
BucketName: 'MyBucket',
},
};

// WHEN
await invokeHandler(event);

// THEN
expect(mockS3Client.listObjectVersions).toHaveBeenCalledTimes(0);
expect(mockS3Client.deleteObjects).toHaveBeenCalledTimes(0);
});

test('does nothing on update event when the old resource properties are absent', async () => {
// GIVEN
const event: Partial<AWSLambda.CloudFormationCustomResourceUpdateEvent> = {
RequestType: 'Update',
Expand All @@ -55,6 +99,63 @@ test('does nothing on update event', async () => {
expect(mockS3Client.deleteObjects).toHaveBeenCalledTimes(0);
});

test('does nothing on update event when the new resource properties are absent', async () => {
// GIVEN
const event: Partial<AWSLambda.CloudFormationCustomResourceUpdateEvent> = {
RequestType: 'Update',
OldResourceProperties: {
ServiceToken: 'Foo',
BucketName: 'MyBucket',
},
};

// WHEN
await invokeHandler(event);

// THEN
expect(mockS3Client.listObjectVersions).toHaveBeenCalledTimes(0);
expect(mockS3Client.deleteObjects).toHaveBeenCalledTimes(0);
});

test('deletes all objects when the name changes on update event', async () => {
// GIVEN
mockS3Client.promise.mockResolvedValue({ // listObjectVersions() call
Versions: [
{ Key: 'Key1', VersionId: 'VersionId1' },
{ Key: 'Key2', VersionId: 'VersionId2' },
],
});

const event: Partial<AWSLambda.CloudFormationCustomResourceUpdateEvent> = {
RequestType: 'Update',
OldResourceProperties: {
ServiceToken: 'Foo',
BucketName: 'MyBucket',
},
ResourceProperties: {
ServiceToken: 'Foo',
BucketName: 'MyBucket-renamed',
},
};

// WHEN
await invokeHandler(event);

// THEN
expect(mockS3Client.listObjectVersions).toHaveBeenCalledTimes(1);
expect(mockS3Client.listObjectVersions).toHaveBeenCalledWith({ Bucket: 'MyBucket' });
expect(mockS3Client.deleteObjects).toHaveBeenCalledTimes(1);
expect(mockS3Client.deleteObjects).toHaveBeenCalledWith({
Bucket: 'MyBucket',
Delete: {
Objects: [
{ Key: 'Key1', VersionId: 'VersionId1' },
{ Key: 'Key2', VersionId: 'VersionId2' },
],
},
});
});

test('deletes no objects on delete event when bucket has no objects', async () => {
// GIVEN
mockS3Client.promise.mockResolvedValue({ Versions: [] }); // listObjectVersions() call
Expand Down
18 changes: 9 additions & 9 deletions packages/@aws-cdk/aws-s3/test/integ.bucket-auto-delete-objects.expected.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@
"Properties": {
"Code": {
"S3Bucket": {
"Ref": "AssetParameters1a8becf42c48697a059094af1e94aa6bc6df0512d30433db8c22618ca02dfca1S3BucketF01ADF6B"
"Ref": "AssetParametersfe5df38824187483182e1459db47adfae2a515aa4caedd437fc4033a0c5b3de9S3BucketD715D8B0"
},
"S3Key": {
"Fn::Join": [
Expand All @@ -115,7 +115,7 @@
"Fn::Split": [
"||",
{
"Ref": "AssetParameters1a8becf42c48697a059094af1e94aa6bc6df0512d30433db8c22618ca02dfca1S3VersionKey6FC34F51"
"Ref": "AssetParametersfe5df38824187483182e1459db47adfae2a515aa4caedd437fc4033a0c5b3de9S3VersionKey6E76822C"
}
]
}
Expand All @@ -128,7 +128,7 @@
"Fn::Split": [
"||",
{
"Ref": "AssetParameters1a8becf42c48697a059094af1e94aa6bc6df0512d30433db8c22618ca02dfca1S3VersionKey6FC34F51"
"Ref": "AssetParametersfe5df38824187483182e1459db47adfae2a515aa4caedd437fc4033a0c5b3de9S3VersionKey6E76822C"
}
]
}
Expand Down Expand Up @@ -289,17 +289,17 @@
}
},
"Parameters": {
"AssetParameters1a8becf42c48697a059094af1e94aa6bc6df0512d30433db8c22618ca02dfca1S3BucketF01ADF6B": {
"AssetParametersfe5df38824187483182e1459db47adfae2a515aa4caedd437fc4033a0c5b3de9S3BucketD715D8B0": {
"Type": "String",
"Description": "S3 bucket for asset \"1a8becf42c48697a059094af1e94aa6bc6df0512d30433db8c22618ca02dfca1\""
"Description": "S3 bucket for asset \"fe5df38824187483182e1459db47adfae2a515aa4caedd437fc4033a0c5b3de9\""
},
"AssetParameters1a8becf42c48697a059094af1e94aa6bc6df0512d30433db8c22618ca02dfca1S3VersionKey6FC34F51": {
"AssetParametersfe5df38824187483182e1459db47adfae2a515aa4caedd437fc4033a0c5b3de9S3VersionKey6E76822C": {
"Type": "String",
"Description": "S3 key for asset version \"1a8becf42c48697a059094af1e94aa6bc6df0512d30433db8c22618ca02dfca1\""
"Description": "S3 key for asset version \"fe5df38824187483182e1459db47adfae2a515aa4caedd437fc4033a0c5b3de9\""
},
"AssetParameters1a8becf42c48697a059094af1e94aa6bc6df0512d30433db8c22618ca02dfca1ArtifactHash9ECACDFD": {
"AssetParametersfe5df38824187483182e1459db47adfae2a515aa4caedd437fc4033a0c5b3de9ArtifactHash9AE3702B": {
"Type": "String",
"Description": "Artifact hash for asset \"1a8becf42c48697a059094af1e94aa6bc6df0512d30433db8c22618ca02dfca1\""
"Description": "Artifact hash for asset \"fe5df38824187483182e1459db47adfae2a515aa4caedd437fc4033a0c5b3de9\""
},
"AssetParametersf7ee44e9b6217d201200d9abd42c6493b0b11e86be8a7f36163c3ea049c54653S3BucketDB5FAF47": {
"Type": "String",
Expand Down

0 comments on commit b1d69d7

Please sign in to comment.