Fixed error while calling emptyBucket() (#11)

.gitignore

@@ -1,3 +1,6 @@
+# IDE
+.idea
+
 # Logs
 logs
 *.log

CHANGELOG.MD

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/).
 
 # <<<----- ADD NEW VERSIONS HERE
 
+## 1.1.2 - (November 16, 2020)
+### Changed
+- [Issue-9](https://github.com/xavier-thomas/aws-cfn-stack-cleanup/issues/9) - Fixed Bug while calling emptyBucket()
+
 ## 1.1.1 - (October 27, 2020)
 ### Changed
 - Updated dependencies

README.md

@@ -54,7 +54,7 @@ Resources:
     Properties:
       Location:
         ApplicationId: arn:aws:serverlessrepo:us-east-1:673103718481:applications/Cfn-Stack-Cleanup
-      SemanticVersion: 1.1.1
+      SemanticVersion: 1.1.2
       # Optional Parameter to control the export name of the nested stack
       Parameters:
         ExportPrefix: !Ref AWS::StackName
@@ -228,8 +228,8 @@ The project's maintainers will need to approve this before it can be merged in a
 
 
 ## Authors
-**[Xavier Thomas](https://github.com/xavier-thomas)**
-**[Matthew Farrow](https://github.com/mfarrow701)**
+- **[Xavier Thomas](https://github.com/xavier-thomas)**
+- **[Matthew Farrow](https://github.com/mfarrow701)**
 
 ## Licence
 **[3-Clause BSD](./LICENCE)**

cfn-stack-cleanup.yaml

@@ -26,7 +26,7 @@ Metadata:
     LicenseUrl: LICENSE
     Name: Cfn-Stack-Cleanup
     ReadmeUrl: README.md
-    SemanticVersion: 1.1.1
+    SemanticVersion: 1.1.2
     SourceCodeUrl: https://github.com/xavier-thomas/aws-cfn-stack-cleanup
     SpdxLicenseId: BSD-3-Clause
 

package.json

@@ -1,6 +1,6 @@
 {
   "name": "aws-cfn-stack-cleanup",
-  "version": "1.1.1",
+  "version": "1.1.2",
   "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": [

src/deleteBucket.js

@@ -0,0 +1,30 @@
+import { S3Client } from '@aws-sdk/client-s3';
+import { emptyBucket } from './emptyBucket';
+
+/**
+ * Delete an S3 Bucket
+ * Attempt to empty it first before deleting.
+ * @param {String} srcBucket - The S3 Bucket Name to Delete
+ *
+ */
+
+export const deleteBucket = async (srcBucket) => {
+	const s3 = new S3Client();
+
+	try {
+		// Attempt to empty the bucket first before deleting it.
+		await emptyBucket(srcBucket);
+
+		const response = await s3.deleteBucket({ Bucket: srcBucket });
+		console.info(`Bucket: [${srcBucket}] deleted.`);
+		return response;
+	} catch (err) {
+		if (err.code === 'NoSuchBucket') {
+			//Bucket likely already Deleted
+			console.info(`Bucket: [${srcBucket}] not found. May have already been deleted.`);
+			return;
+		} else {
+			throw new Error(`Error Deleting Bucket: [${srcBucket}] - ${err.message}`);
+		}
+	}
+};

src/deleteBucket.test.js

@@ -0,0 +1,66 @@
+import * as emptyBucket from './emptyBucket';
+import { MOCK_ERROR_S3_BUCKET_NOT_EXIST, MOCK_ERROR_S3_BUCKET_UNKNOWN, MOCK_S3_SOURCE_BUCKET } from './mocks';
+import { S3Client } from '@aws-sdk/client-s3';
+import { deleteBucket } from './deleteBucket';
+
+jest.mock('@aws-sdk/client-s3');
+
+describe('[s3.js] unit tests', () => {
+	const mock_deleteBucket = jest.fn();
+	const mock_deleteObjects = jest.fn();
+	const mock_listObjectsV2 = jest.fn();
+
+	// What to do before test is executed
+	beforeEach(() => {
+		jest.restoreAllMocks();
+		S3Client.mockImplementation(() => ({
+			deleteBucket: mock_deleteBucket,
+			deleteObjects: mock_deleteObjects,
+			listObjectsV2: mock_listObjectsV2,
+		}));
+		jest.spyOn(console, 'error').mockImplementation(() => {});
+		jest.spyOn(console, 'info').mockImplementation(() => {});
+		jest.spyOn(console, 'log').mockImplementation(() => {});
+	});
+
+	// What to do after each test is executed
+	afterEach(() => {
+		jest.clearAllMocks();
+	});
+
+	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({});
+			const response = await deleteBucket(MOCK_S3_SOURCE_BUCKET);
+			expect(mock_deleteBucket).toHaveBeenCalledWith({
+				Bucket: MOCK_S3_SOURCE_BUCKET,
+			});
+			expect(emptyBucket.emptyBucket).toHaveBeenCalledTimes(1);
+			expect(emptyBucket.emptyBucket).toHaveBeenCalledWith(MOCK_S3_SOURCE_BUCKET);
+			expect(console.info).toHaveBeenCalledWith(`Bucket: [${MOCK_S3_SOURCE_BUCKET}] deleted.`);
+			expect(response).toEqual({});
+		});
+
+		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);
+			const response = await deleteBucket(MOCK_S3_SOURCE_BUCKET);
+			expect(console.info).toHaveBeenCalledWith(
+				`Bucket: [${MOCK_S3_SOURCE_BUCKET}] not found. May have already been deleted.`
+			);
+			expect(response).toEqual();
+		});
+
+		it('must throw all other errors', async () => {
+			jest.spyOn(emptyBucket, 'emptyBucket').mockResolvedValue({});
+			mock_deleteBucket.mockRejectedValue(MOCK_ERROR_S3_BUCKET_UNKNOWN);
+			await expect(deleteBucket(MOCK_S3_SOURCE_BUCKET)).rejects.toThrowError(
+				`Error Deleting Bucket: [fake_bucket] - Unknown Error`
+			);
+			expect(console.info).not.toHaveBeenCalled();
+		});
+
+		// TODO: Test an error being thrown by the empty bucket function
+	});
+});

src/s3.js → src/emptyBucket.js

@@ -52,31 +52,3 @@ export const emptyBucket = async (srcBucket, count = 0) => {
 		throw new Error(`Error Deleting Objects in Bucket: [${srcBucket}] - ${err.message}`);
 	}
 };
-
-/**
- * Delete an S3 Bucket
- * Attempt to empty it first before deleting.
- * @param {String} srcBucket - The S3 Bucket Name to Delete
- *
- */
-
-export const deleteBucket = async (srcBucket, _emptyBucket = this.emptyBucket) => {
-	const s3 = new S3Client();
-
-	try {
-		// Attempt to empty the bucket first before deleting it.
-		await _emptyBucket(srcBucket);
-
-		const response = await s3.deleteBucket({ Bucket: srcBucket });
-		console.info(`Bucket: [${srcBucket}] deleted.`);
-		return response;
-	} catch (err) {
-		if (err.code === 'NoSuchBucket') {
-			//Bucket likely already Deleted
-			console.info(`Bucket: [${srcBucket}] not found. May have already been deleted.`);
-			return;
-		} else {
-			throw new Error(`Error Deleting Bucket: [${srcBucket}] - ${err.message}`);
-		}
-	}
-};

src/s3.test.js → src/emptyBucket.test.js

@@ -4,16 +4,15 @@ import {
 	MOCK_S3_SOURCE_BUCKET,
 	generateDummyObjectList,
 } from './mocks';
-import { deleteBucket, emptyBucket } from './s3.js';
 import { S3Client } from '@aws-sdk/client-s3';
+import { emptyBucket } from './emptyBucket';
 
 jest.mock('@aws-sdk/client-s3');
 
 describe('[s3.js] unit tests', () => {
 	const mock_deleteBucket = jest.fn();
 	const mock_deleteObjects = jest.fn();
 	const mock_listObjectsV2 = jest.fn();
-	const mock_emptyBucket = jest.fn();
 
 	// What to do before test is executed
 	beforeEach(() => {
@@ -33,44 +32,6 @@ describe('[s3.js] unit tests', () => {
 		jest.clearAllMocks();
 	});
 
-	describe('[deleteBucket] when a bucket name is passed', () => {
-		it('must first empty then delete the bucket when the bucket exists', async () => {
-			mock_deleteBucket.mockResolvedValue({});
-
-			mock_emptyBucket.mockResolvedValue({});
-
-			const response = await deleteBucket(MOCK_S3_SOURCE_BUCKET, mock_emptyBucket);
-
-			expect(mock_deleteBucket).toHaveBeenCalledWith({
-				Bucket: MOCK_S3_SOURCE_BUCKET,
-			});
-			expect(mock_emptyBucket).toHaveBeenCalledTimes(1);
-			expect(mock_emptyBucket).toHaveBeenCalledWith(MOCK_S3_SOURCE_BUCKET);
-			expect(console.info).toHaveBeenCalledWith(`Bucket: [${MOCK_S3_SOURCE_BUCKET}] deleted.`);
-			expect(response).toEqual({});
-		});
-
-		it('must ignore a bucket not found error as the bucket may have already been deleted', async () => {
-			mock_deleteBucket.mockRejectedValue(MOCK_ERROR_S3_BUCKET_NOT_EXIST);
-			const response = await deleteBucket(MOCK_S3_SOURCE_BUCKET, mock_emptyBucket);
-			expect(console.info).toHaveBeenCalledWith(
-				`Bucket: [${MOCK_S3_SOURCE_BUCKET}] not found. May have already been deleted.`
-			);
-			expect(response).toEqual();
-		});
-
-		it('must throw all other errors', async () => {
-			mock_deleteBucket.mockRejectedValue(MOCK_ERROR_S3_BUCKET_UNKNOWN);
-
-			await expect(deleteBucket(MOCK_S3_SOURCE_BUCKET, mock_emptyBucket)).rejects.toThrowError(
-				`Error Deleting Bucket: [fake_bucket] - Unknown Error`
-			);
-			expect(console.info).not.toHaveBeenCalled();
-		});
-
-		// TODO: Test an error being thrown by the empty bucket function
-	});
-
 	describe('[emptyBucket] when a bucket name is passed', () => {
 		it('must first list objects in the bucket', async () => {
 			const data = generateDummyObjectList(1);

src/index.js

@@ -1,5 +1,5 @@
 import { FAILED, SUCCESS, send } from 'cfn-response-promise';
-import { deleteBucket } from './s3.js';
+import { deleteBucket } from './deleteBucket.js';
 
 /**
  * Invokes the CFN Cleanup Lambda

src/index.test.js

@@ -6,10 +6,10 @@ import {
 	MOCK_EVENT_DELETE_BUCKETS_EMPTY_NO_PROP,
 } from './mocks';
 import cfnResponse from 'cfn-response-promise';
-import { deleteBucket } from './s3.js';
+import { deleteBucket } from './deleteBucket';
 import { handler } from './index';
 
-jest.mock('./s3.js');
+jest.mock('./deleteBucket.js');
 
 describe('[index.js] unit tests', () => {
 	const mockCfnResponse = jest.fn();