Skip to content

Commit

Permalink
Fixed S3 promises (#14)
Browse files Browse the repository at this point in the history
  • Loading branch information
xavier-thomas committed Nov 17, 2020
1 parent b33295a commit 5cd4a49
Show file tree
Hide file tree
Showing 8 changed files with 87 additions and 51 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.MD
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/).

# <<<----- ADD NEW VERSIONS HERE

## 1.1.6 - (November 17, 2020)
### Changed
- Hotfix to v1.1.5

## 1.1.5 - (November 17, 2020)
### Changed
- Rollback Scoped AWS S3 dependency that was introduced in v1.1.0
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ Resources:
Properties:
Location:
ApplicationId: arn:aws:serverlessrepo:us-east-1:673103718481:applications/Cfn-Stack-Cleanup
SemanticVersion: 1.1.5
SemanticVersion: 1.1.6
# Optional Parameter to control the export name of the nested stack
Parameters:
ExportPrefix: !Ref AWS::StackName
Expand Down
2 changes: 1 addition & 1 deletion cfn-stack-cleanup.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ Metadata:
LicenseUrl: LICENSE
Name: Cfn-Stack-Cleanup
ReadmeUrl: README.md
SemanticVersion: 1.1.5
SemanticVersion: 1.1.6
SourceCodeUrl: https://github.com/xavier-thomas/aws-cfn-stack-cleanup
SpdxLicenseId: BSD-3-Clause

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "aws-cfn-stack-cleanup",
"version": "1.1.5",
"version": "1.1.6",
"description": "An AWS Lambda that can be invoked as a custom resource from an AWS CloudFormation stack to clean up any left over stack resources such as S3 buckets, Log Groups, Sub-Stacks, etc...",
"main": "src/index.js",
"keywords": [
Expand Down
2 changes: 1 addition & 1 deletion src/deleteBucket.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export const deleteBucket = async (srcBucket) => {
// Attempt to empty the bucket first before deleting it.
await emptyBucket(srcBucket);

const response = await s3.deleteBucket({ Bucket: srcBucket });
const response = await s3.deleteBucket({ Bucket: srcBucket }).promise();
console.info(`Bucket: [${srcBucket}] deleted.`);
return response;
} catch (err) {
Expand Down
13 changes: 10 additions & 3 deletions src/deleteBucket.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ describe('[s3.js] unit tests', () => {
describe('[deleteBucket] when a bucket name is passed', () => {
it('must first empty then delete the bucket when the bucket exists', async () => {
jest.spyOn(emptyBucket, 'emptyBucket').mockResolvedValue({});
mock_deleteBucket.mockResolvedValue({});
mock_deleteBucket.mockReturnValue({
promise: jest.fn().mockResolvedValue({}),
});
const response = await deleteBucket(MOCK_S3_SOURCE_BUCKET);
expect(mock_deleteBucket).toHaveBeenCalledWith({
Bucket: MOCK_S3_SOURCE_BUCKET,
Expand All @@ -44,7 +46,10 @@ describe('[s3.js] unit tests', () => {

it('must ignore a bucket not found error as the bucket may have already been deleted', async () => {
jest.spyOn(emptyBucket, 'emptyBucket').mockResolvedValue({});
mock_deleteBucket.mockRejectedValue(MOCK_ERROR_S3_BUCKET_NOT_EXIST);

mock_deleteBucket.mockReturnValue({
promise: jest.fn().mockRejectedValue(MOCK_ERROR_S3_BUCKET_NOT_EXIST),
});
const response = await deleteBucket(MOCK_S3_SOURCE_BUCKET);
expect(console.info).toHaveBeenCalledWith(
`Bucket: [${MOCK_S3_SOURCE_BUCKET}] not found. May have already been deleted.`
Expand All @@ -54,7 +59,9 @@ describe('[s3.js] unit tests', () => {

it('must throw all other errors', async () => {
jest.spyOn(emptyBucket, 'emptyBucket').mockResolvedValue({});
mock_deleteBucket.mockRejectedValue(MOCK_ERROR_S3_BUCKET_UNKNOWN);
mock_deleteBucket.mockReturnValue({
promise: jest.fn().mockRejectedValue(MOCK_ERROR_S3_BUCKET_UNKNOWN),
});
await expect(deleteBucket(MOCK_S3_SOURCE_BUCKET)).rejects.toThrowError(
`Error Deleting Bucket: [fake_bucket] - Unknown Error`
);
Expand Down
20 changes: 11 additions & 9 deletions src/emptyBucket.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export const emptyBucket = async (srcBucket, count = 0) => {
const s3 = new S3();
let data;
try {
data = await s3.listObjectsV2({ Bucket: srcBucket });
data = await s3.listObjectsV2({ Bucket: srcBucket }).promise();
if (data.Contents.length === 0) {
if (count > 0) {
console.info(`Deleted [${count}] objects from Bucket: [${srcBucket}].`);
Expand All @@ -31,14 +31,16 @@ export const emptyBucket = async (srcBucket, count = 0) => {
}

try {
const deleteData = await s3.deleteObjects({
Bucket: srcBucket,
Delete: {
Objects: data.Contents.map((c) => {
return { Key: c.Key };
}),
},
});
const deleteData = await s3
.deleteObjects({
Bucket: srcBucket,
Delete: {
Objects: data.Contents.map((c) => {
return { Key: c.Key };
}),
},
})
.promise();
count += deleteData.Deleted.length;

// S3 list method cannot return more than 1K items,
Expand Down
93 changes: 58 additions & 35 deletions src/emptyBucket.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,15 @@ describe('[s3.js] unit tests', () => {
describe('[emptyBucket] when a bucket name is passed', () => {
it('must first list objects in the bucket', async () => {
const data = generateDummyObjectList(1);
mock_listObjectsV2.mockResolvedValueOnce({
Contents: data,
mock_listObjectsV2.mockReturnValue({
promise: jest.fn().mockResolvedValueOnce({
Contents: data,
}),
});
mock_deleteObjects.mockResolvedValueOnce({
Deleted: data,
mock_deleteObjects.mockReturnValue({
promise: jest.fn().mockResolvedValueOnce({
Deleted: data,
}),
});
const response = await emptyBucket(MOCK_S3_SOURCE_BUCKET);
expect(mock_listObjectsV2).toHaveBeenCalledTimes(1);
Expand All @@ -57,8 +61,10 @@ describe('[s3.js] unit tests', () => {
});

it('must return success if there are no objects in the bucket', async () => {
mock_listObjectsV2.mockResolvedValueOnce({
Contents: generateDummyObjectList(0),
mock_listObjectsV2.mockReturnValue({
promise: jest.fn().mockResolvedValueOnce({
Contents: generateDummyObjectList(0),
}),
});
const response = await emptyBucket(MOCK_S3_SOURCE_BUCKET);
expect(mock_listObjectsV2).toHaveBeenCalledTimes(1);
Expand All @@ -68,16 +74,19 @@ describe('[s3.js] unit tests', () => {
});

it('must ignore a bucket not found error as the bucket may have already been deleted', async () => {
mock_listObjectsV2.mockRejectedValue(MOCK_ERROR_S3_BUCKET_NOT_EXIST);

mock_listObjectsV2.mockReturnValue({
promise: jest.fn().mockRejectedValue(MOCK_ERROR_S3_BUCKET_NOT_EXIST),
});
const response = await emptyBucket(MOCK_S3_SOURCE_BUCKET);
expect(mock_listObjectsV2).toHaveBeenCalledTimes(1);
expect(mock_deleteObjects).not.toHaveBeenCalled();
expect(response).toBeUndefined();
});

it('must throw all other errors generated while listing objects', async () => {
mock_listObjectsV2.mockRejectedValue(MOCK_ERROR_S3_BUCKET_UNKNOWN);
mock_listObjectsV2.mockReturnValue({
promise: jest.fn().mockRejectedValue(MOCK_ERROR_S3_BUCKET_UNKNOWN),
});

await expect(emptyBucket(MOCK_S3_SOURCE_BUCKET)).rejects.toThrowError(
`Error Listing Objects in Bucket: [fake_bucket] - Unknown Error`
Expand All @@ -88,11 +97,16 @@ describe('[s3.js] unit tests', () => {

it('must log the number of files deleted', async () => {
const count = 500;
mock_listObjectsV2.mockResolvedValueOnce({
Contents: generateDummyObjectList(count),

mock_listObjectsV2.mockReturnValue({
promise: jest.fn().mockResolvedValueOnce({
Contents: generateDummyObjectList(count),
}),
});
mock_deleteObjects.mockResolvedValueOnce({
Deleted: generateDummyObjectList(count),
mock_deleteObjects.mockReturnValue({
promise: jest.fn().mockResolvedValueOnce({
Deleted: generateDummyObjectList(count),
}),
});
const response = await emptyBucket(MOCK_S3_SOURCE_BUCKET);
expect(console.info).toHaveBeenCalledWith(`Deleted [${count}] objects from Bucket: [${MOCK_S3_SOURCE_BUCKET}].`);
Expand All @@ -102,11 +116,14 @@ describe('[s3.js] unit tests', () => {
});

it('must throw all errors generated while deleting objects', async () => {
mock_listObjectsV2.mockResolvedValueOnce({
Contents: generateDummyObjectList(10),
mock_listObjectsV2.mockReturnValue({
promise: jest.fn().mockResolvedValueOnce({
Contents: generateDummyObjectList(10),
}),
});
mock_deleteObjects.mockReturnValue({
promise: jest.fn().mockRejectedValue(MOCK_ERROR_S3_BUCKET_UNKNOWN),
});
mock_deleteObjects.mockRejectedValue(MOCK_ERROR_S3_BUCKET_UNKNOWN);

await expect(emptyBucket(MOCK_S3_SOURCE_BUCKET)).rejects.toThrowError(
`Error Deleting Objects in Bucket: [fake_bucket] - Unknown Error`
);
Expand All @@ -115,21 +132,26 @@ describe('[s3.js] unit tests', () => {
});

it('must recursively call itself when there are more than 1000 objects in the bucket', async () => {
mock_listObjectsV2
.mockResolvedValueOnce({
Contents: generateDummyObjectList(1000),
})
.mockResolvedValueOnce({
Contents: generateDummyObjectList(100),
});
mock_deleteObjects
.mockResolvedValueOnce({
Deleted: generateDummyObjectList(1000),
})
.mockResolvedValueOnce({
Deleted: generateDummyObjectList(100),
});

mock_listObjectsV2.mockReturnValue({
promise: jest
.fn()
.mockResolvedValueOnce({
Contents: generateDummyObjectList(1000),
})
.mockResolvedValueOnce({
Contents: generateDummyObjectList(100),
}),
});
mock_deleteObjects.mockReturnValue({
promise: jest
.fn()
.mockResolvedValueOnce({
Deleted: generateDummyObjectList(1000),
})
.mockResolvedValueOnce({
Deleted: generateDummyObjectList(100),
}),
});
const result = await emptyBucket(MOCK_S3_SOURCE_BUCKET);

expect(mock_listObjectsV2).toHaveBeenCalledTimes(2);
Expand All @@ -140,10 +162,11 @@ describe('[s3.js] unit tests', () => {
});

it('must log the total count of deleted objects when there are no more items in the bucket', async () => {
mock_listObjectsV2.mockResolvedValueOnce({
Contents: generateDummyObjectList(0),
mock_listObjectsV2.mockReturnValue({
promise: jest.fn().mockResolvedValueOnce({
Contents: generateDummyObjectList(0),
}),
});

const count = 5432;

const result = await emptyBucket(MOCK_S3_SOURCE_BUCKET, count);
Expand Down

0 comments on commit 5cd4a49

Please sign in to comment.