Merge pull request #36 from hearde/release/v1.8.7

Release v1.8.7

CHANGELOG.md

@@ -5,6 +5,11 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [1.8.7] - 2024-01-30
+### Fixed
+
+- Handling of aws sdk v3 errors
+
 ## [1.8.6] - 2023-12-21
 
 ### Changed

source/lambda/multi_account_custom_resources/lib/eventbridge.js

@@ -76,7 +76,7 @@ const RemovePermission = async (principal, eventBusName) => {
 
     return response;
   } catch (err) {
-    if (err.code !== 'ResourceNotFoundException') {
+    if (err.name !== 'ResourceNotFoundException') {
       LOGGER.log('ERROR', `[RemovePermission] Error when removing permission from event bus: ${err.message}`);
       throw err;
     }

source/lambda/multi_account_custom_resources/lib/s3_bucket_policy.js

@@ -29,7 +29,7 @@ const GetS3BucketPolicy = async bucketName => {
 
     return response;
   } catch (err) {
-    if (err.code !== 'NoSuchBucketPolicy') {
+    if (err.name !== 'NoSuchBucketPolicy') {
       LOGGER.log('ERROR', `[GetS3BucketPolicy] Error when getting s3 bucket policy: ${err.message}`);
       throw err;
     }
@@ -81,7 +81,7 @@ const DeleteS3BucketPolicy = async bucketName => {
 
     return response;
   } catch (err) {
-    if (err.code !== 'NoSuchBucketPolicy') {
+    if (err.name !== 'NoSuchBucketPolicy') {
       LOGGER.log(
         'ERROR',
         `[DeleteS3BucketPolicy] Error when deleting bucket policy from s3 bucket ${bucketName}: ${err.message}`

source/lambda/multi_account_custom_resources/manage_s3_bucket_policy.js

@@ -117,7 +117,7 @@ const GetExistingS3BucketPolicy = async bucketName => {
     return policyStatements;
   } catch (err) {
     // If no bucket policy exists, don't fail but return empty json object
-    if (err.statusCode === 404) return {};
+    if (err.name === 'NoSuchBucketPolicy') return {};
   }
 };
 

source/lambda/multi_account_custom_resources/test/eventbridge.spec.js

@@ -3,8 +3,13 @@
 
 'use strict';
 
+const mockEventBridge = {
+  putPermission: jest.fn(),
+  removePermission: jest.fn(),
+  describeEventBus: jest.fn(),
+};
 const eb = require('../lib/eventbridge.js');
-
+const { ServiceException } = require('@smithy/smithy-client');
 const accountPrincipalType = 'Account';
 const accountPrincipal = 'testAccount';
 const orgPrincipalType = 'Account';
@@ -13,21 +18,19 @@ const eventBusName = 'testEventBusName';
 
 jest.mock(
   '@aws-sdk/client-eventbridge',
-  () => {
-    const mockEventBridgeService = {
-      putPermission: jest.fn(),
-      removePermission: jest.fn(),
-      describeEventBus: jest.fn(),
-    };
-    return {
+  () => ({
       __esmodule: true,
-      EventBridge: jest.fn(() => mockEventBridgeService)
-    };
-  },
-  { virual: true }
+      EventBridge: jest.fn(() => mockEventBridge),
+    })
 );
 
 describe('When testing event bridge APIs', () => {
+  beforeEach(() => {
+    mockEventBridge.putPermission.mockReset();
+    mockEventBridge.removePermission.mockReset();
+    mockEventBridge.describeEventBus.mockReset();
+  });
+
   it('should successfully put permission for account', async () => {
     const response = await eb.putPermission(accountPrincipalType, accountPrincipal, eventBusName);
     expect(response).not.toBeNull();
@@ -51,4 +54,15 @@ describe('When testing event bridge APIs', () => {
   const response = await eb.describeEventBus(eventBusName);
     expect(response).not.toBeNull();
   });
+  it('should return undefined when resource is not found', async () => {
+    const mockError = new ServiceException({name: 'ResourceNotFoundException'});
+    mockEventBridge.removePermission.mockImplementation(async () => { throw mockError });
+    const response = await eb.removePermission(orgPrincipal, eventBusName);
+    expect(response).toEqual(undefined);
+  });
+    it('should throw when removing permissions throws anything other than ResourceNotFoundException', async () => {
+    const mockError = new ServiceException({name: 'OtherException'});
+    mockEventBridge.removePermission.mockImplementation(async () => { throw mockError });
+    expect(eb.removePermission(orgPrincipal, eventBusName)).rejects.toThrow(mockError);
+  });
 });

source/lambda/multi_account_custom_resources/test/manage_s3_bucket_policy.spec.js

@@ -2,8 +2,9 @@
 // SPDX-License-Identifier: Apache-2.0
 
 'use strict';
-
+const mockGetS3BucketPolicy = jest.fn();
 const manageBucketPolicy = require('../manage_s3_bucket_policy');
+const { ServiceException } = require('@smithy/smithy-client');
 
 const accountPrincipalType = 'Account';
 const accountPrincipalList = ['111111111111', '222222222222'];
@@ -45,12 +46,11 @@ jest.mock(
   '../lib/s3_bucket_policy',
   () => ({
     __esmodule: true,
-    getS3BucketPolicy: jest.fn().mockReturnValue(existingPS),
+    getS3BucketPolicy: mockGetS3BucketPolicy.mockReturnValue(existingPS),
     putS3BucketPolicy: jest.fn().mockReturnThis(),
     deleteS3BucketPolicy: jest.fn().mockReturnThis(),
     promise: jest.fn()
-  }),
-  { virual: true }
+  })
 );
 
 describe('Test managing s3 bucket policy', () => {
@@ -84,4 +84,10 @@ describe('Test managing s3 bucket policy', () => {
     );
     expect(response).not.toBeNull();
   });
+  it('Should return empty object when there is no bucket policy ', async () => {
+    const mockError = new ServiceException({name: 'NoSuchBucketPolicy'});
+    mockGetS3BucketPolicy.mockImplementation(async () => { throw  mockError });
+    const response = await manageBucketPolicy.getExistingS3BucketPolicy('test-bucket');
+    expect(response).toEqual({});
+  });
 });

source/lambda/multi_account_custom_resources/test/s3_bucket_policy.spec.js

@@ -2,29 +2,44 @@
 // SPDX-License-Identifier: Apache-2.0
 
 'use strict';
-
+const mockS3Service = {
+  deleteBucketPolicy: jest.fn(),
+  getBucketPolicy: jest.fn(),
+};
 const s3BucketPolicy = require('../lib/s3_bucket_policy');
-
+const { ServiceException } = require('@smithy/smithy-client');
 const bucketName = 'testBucketName';
-
 jest.mock(
   '@aws-sdk/client-s3',
-  () => {
-    const mockS3Service = {
-      deleteBucketPolicy: jest.fn(),
-    };
-    return {
+  () => ({
       __esmodule: true,
-      S3: jest.fn(() => mockS3Service)
-    };
-  },
-  { virual: true }
+      S3: jest.fn(() => mockS3Service),
+  })
 );
 
 describe('Test s3 bucket policy', () => {
+  beforeEach(() => {
+    mockS3Service.getBucketPolicy.mockReset();
+    mockS3Service.deleteBucketPolicy.mockReset();
+  });
+
   it('should delete s3 bucket policy', async () => {
     const response = await s3BucketPolicy.deleteS3BucketPolicy(bucketName);
     expect(response).not.toBeNull();
   });
+  it('should return undefined when getting bucket policy that does not exist', async () => {
+    const mockError = new ServiceException({name: 'NoSuchBucketPolicy'});
+    mockS3Service.getBucketPolicy.mockImplementation(async () => { throw mockError });
+    try {
+      await s3BucketPolicy.getS3BucketPolicy(bucketName);
+    } catch (err) {
+      expect(err).toEqual(mockError);
+    }
+  });
+  it('should throw error when deleting bucket policy returns exception other than NoSuchBucketPolicy', async () => {
+    const mockError = new ServiceException({name: 'OtherException'});
+    mockS3Service.deleteBucketPolicy.mockImplementation(async () => { throw mockError });
+    expect(s3BucketPolicy.deleteS3BucketPolicy(bucketName)).rejects.toThrow(mockError);
+  });
 });
 

source/test/__snapshots__/aws_devops_monitoring_dashboard_stack.test.ts.snap

@@ -447,7 +447,7 @@ exports[`Snapshot test for primary devopsDashboardStack 1`] = `
           "S3Bucket": {
             "Fn::Sub": "cdk-hnb659fds-assets-\${AWS::AccountId}-\${AWS::Region}",
           },
-          "S3Key": "9bab9fc99496429fd9d965ba3c1ee064421b8016e9d53c824ed179e8207f7e73.zip",
+          "S3Key": "6c17f15e76c409ea0abb72561f561686216c1a0430be042b2b909663bf0640dd.zip",
         },
         "Description": "DevOps Monitoring Dashboard on AWS solution - This function runs on a daily schedule and adds a new partition to Amazon Athena table",
         "Environment": {
@@ -3445,7 +3445,7 @@ exports[`Snapshot test for primary devopsDashboardStack 1`] = `
           "S3Bucket": {
             "Fn::Sub": "cdk-hnb659fds-assets-\${AWS::AccountId}-\${AWS::Region}",
           },
-          "S3Key": "9bab9fc99496429fd9d965ba3c1ee064421b8016e9d53c824ed179e8207f7e73.zip",
+          "S3Key": "6c17f15e76c409ea0abb72561f561686216c1a0430be042b2b909663bf0640dd.zip",
         },
         "Description": "DevOps Monitoring Dashboard on AWS solution - This function runs Amazon Athena queries and creates views.",
         "Environment": {
@@ -3745,7 +3745,7 @@ exports[`Snapshot test for primary devopsDashboardStack 1`] = `
           "S3Bucket": {
             "Fn::Sub": "cdk-hnb659fds-assets-\${AWS::AccountId}-\${AWS::Region}",
           },
-          "S3Key": "208dee6ae45001bf671384b2c4e44122097a7a4c79307328a6e02738b24f6ba7.zip",
+          "S3Key": "c4deb04178754c3929b060836cc4b9507b72b8ecaf28d154f95378058fd25f54.zip",
         },
         "Description": "DevOps Monitoring Dashboard on AWS solution - This function queries CodeCommit, CodeBuild, and CodePipeline resources for tag information.",
         "Environment": {
@@ -4362,7 +4362,7 @@ exports[`Snapshot test for primary devopsDashboardStack 1`] = `
           "S3Bucket": {
             "Fn::Sub": "cdk-hnb659fds-assets-\${AWS::AccountId}-\${AWS::Region}",
           },
-          "S3Key": "f42e97c9040d87b26dd36871fff40e6dad27098ff34de804b7084625d324e391.zip",
+          "S3Key": "043da9dcc0e0497efa3432045eb0337bbda45f36c01fe70f65bb23f2bf44a5d8.zip",
         },
         "Description": "DevOps Monitoring Dashboard on AWS solution - This function manages permissions in the central monitoring account that are required for processing metrics sent by other accounts.",
         "Environment": {

source/test/__snapshots__/sharing_account_stack.test.ts.snap

@@ -1204,7 +1204,7 @@ exports[`Snapshot test for primary SharingAccountStack 1`] = `
           "S3Bucket": {
             "Fn::Sub": "cdk-hnb659fds-assets-\${AWS::AccountId}-\${AWS::Region}",
           },
-          "S3Key": "208dee6ae45001bf671384b2c4e44122097a7a4c79307328a6e02738b24f6ba7.zip",
+          "S3Key": "c4deb04178754c3929b060836cc4b9507b72b8ecaf28d154f95378058fd25f54.zip",
         },
         "Description": "DevOps Monitoring Dashboard on AWS solution - This function queries CodeCommit, CodeBuild, and CodePipeline resources for tag information.",
         "Environment": {