From 36f68d804b6d470761cf8123fd0f2814cb5ecffa Mon Sep 17 00:00:00 2001 From: Tim Nguyen Date: Mon, 11 Oct 2021 17:04:14 -0400 Subject: [PATCH 1/3] Working delete egress store in workflow --- .../lib/data-egress/data-egress-service.js | 3 ++- .../terminate-product/terminate-product.js | 10 +++++++++- .../backend/config/infra/cloudformation.yml | 17 ++++------------- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/data-egress/data-egress-service.js b/addons/addon-base-raas/packages/base-raas-services/lib/data-egress/data-egress-service.js index 8eff8577f5..392a5ae1bc 100644 --- a/addons/addon-base-raas/packages/base-raas-services/lib/data-egress/data-egress-service.js +++ b/addons/addon-base-raas/packages/base-raas-services/lib/data-egress/data-egress-service.js @@ -215,7 +215,8 @@ class DataEgressService extends Service { true, ); } - await this.deleteMainAccountEgressStoreRole(egressStoreInfo.id); + // TODO: Move this to the terminate workflow loop runner + // await this.deleteMainAccountEgressStoreRole(egressStoreInfo.id); const egressStoreDdbLockId = `egress-store-ddb-access-${egressStoreInfo.id}`; egressStoreInfo.status = TERMINATED_STATUS_CODE; diff --git a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/terminate-product/terminate-product.js b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/terminate-product/terminate-product.js index d37e7f6a61..2908e2b1ee 100644 --- a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/terminate-product/terminate-product.js +++ b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/terminate-product/terminate-product.js @@ -33,6 +33,7 @@ const inPayloadKeys = { }; const settingKeys = { envMgmtRoleArn: 'envMgmtRoleArn', + enableEgressStore: 'enableEgressStore', }; const pluginConstants = { @@ -51,8 +52,9 @@ class TerminateProduct extends StepBase { } async start() { - const [provisionedProductId] = await Promise.all([ + const [provisionedProductId, envId] = await Promise.all([ this.payloadOrConfig.optionalString(inPayloadKeys.provisionedProductId, ''), + this.payloadOrConfig.string(inPayloadKeys.envId), ]); if (provisionedProductId) { @@ -68,6 +70,12 @@ class TerminateProduct extends StepBase { this.state.setKey('RECORD_ID', recordDetail.RecordId); } + const enableEgressStore = this.settings.getBoolean(settingKeys.enableEgressStore); + if (enableEgressStore) { + const [dataEgressService] = await this.mustFindServices(['dataEgressService']); + await dataEgressService.deleteMainAccountEgressStoreRole(envId); + } + return ( this.wait(5) // check every 5 seconds // keep doing it for 1*1296000 seconds = 15 days diff --git a/main/solution/backend/config/infra/cloudformation.yml b/main/solution/backend/config/infra/cloudformation.yml index 4af7aca456..d4eb7ee341 100644 --- a/main/solution/backend/config/infra/cloudformation.yml +++ b/main/solution/backend/config/infra/cloudformation.yml @@ -589,19 +589,6 @@ Resources: - appstream:UpdateImagePermissions Resource: - !Sub 'arn:aws:appstream:${AWS::Region}:${AWS::AccountId}:image/ServiceWorkbench*' - - Effect: Allow - Action: - - iam:GetRole - - iam:DeleteRole - - iam:DetachRolePolicy - - iam:ListAttachedRolePolicies - Resource: - - !Sub 'arn:${AWS::Partition}:iam::${AWS::AccountId}:role/swb-study-*' - - Effect: Allow - Action: - - iam:DeletePolicy - Resource: - - !Sub 'arn:${AWS::Partition}:iam::${AWS::AccountId}:policy/swb-study-*' # IAM Role for the apiHandler Function RoleApiHandler: Type: 'AWS::IAM::Role' @@ -754,11 +741,15 @@ Resources: - Effect: Allow Action: - iam:AttachRolePolicy + - iam:DeleteRole + - iam:DetachRolePolicy + - iam:ListAttachedRolePolicies Resource: - !Sub 'arn:${AWS::Partition}:iam::${AWS::AccountId}:role/swb-study-*' - Effect: Allow Action: - iam:CreatePolicy + - iam:DeletePolicy Resource: - !Sub 'arn:${AWS::Partition}:iam::${AWS::AccountId}:policy/swb-study-*' From 168b1d0a66ff5eb84038a6ccac9be0af73c6bef5 Mon Sep 17 00:00:00 2001 From: Tim Nguyen Date: Mon, 11 Oct 2021 18:30:50 -0400 Subject: [PATCH 2/3] Add unit test for terminate-product --- .../__test__/data-egress-service.test.js | 63 ++++++------- .../lib/data-egress/data-egress-service.js | 2 - .../lib/steps/__test__/launch-product.test.js | 2 +- .../steps/__test__/terminate-product.test.js | 88 +++++++++++++++++++ .../terminate-product/terminate-product.js | 2 +- 5 files changed, 123 insertions(+), 34 deletions(-) create mode 100644 addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/__test__/terminate-product.test.js diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/data-egress/__test__/data-egress-service.test.js b/addons/addon-base-raas/packages/base-raas-services/lib/data-egress/__test__/data-egress-service.test.js index 10725873c0..8847e6e32d 100644 --- a/addons/addon-base-raas/packages/base-raas-services/lib/data-egress/__test__/data-egress-service.test.js +++ b/addons/addon-base-raas/packages/base-raas-services/lib/data-egress/__test__/data-egress-service.test.js @@ -554,38 +554,9 @@ describe('DataEgressService', () => { lockService.tryWriteLockAndRun = jest.fn((_params, callback) => callback()); AWSMock.mock('S3', 'putBucketPolicy', putBucketPolicyMock); - mockDeleteEgressStoreRole(egressStoreId); - await dataEgressService.terminateEgressStore(requestContext, envId); }); - function mockDeleteEgressStoreRole(egressStoreId) { - const policyArn = 'test-PermissionBoundaryArn'; - AWSMock.mock('IAM', 'listAttachedRolePolicies', (params, callback) => { - expect(params.RoleName).toEqual(`swb-study-${egressStoreId}`); - callback(null, { - AttachedPolicies: [ - { - PolicyName: 'test-PermissionBoundaryName', - PolicyArn: policyArn, - }, - ], - }); - }); - AWSMock.mock('IAM', 'detachRolePolicy', (params, callback) => { - expect(params).toMatchObject({ RoleName: `swb-study-${egressStoreId}`, PolicyArn: policyArn }); - callback(); - }); - AWSMock.mock('IAM', 'deleteRole', (params, callback) => { - expect(params).toMatchObject({ RoleName: `swb-study-${egressStoreId}` }); - callback(); - }); - AWSMock.mock('IAM', 'deletePolicy', (params, callback) => { - expect(params).toMatchObject({ PolicyArn: policyArn }); - callback(); - }); - } - it('should successfully delete egress store that is in CREATED state: deleteEgressStoreInCreatedStateTest = true', async () => { await deleteEgressStoreInCreatedStateTest(true); }); @@ -661,7 +632,6 @@ describe('DataEgressService', () => { // Mock locking so that the putBucketPolicy actually gets called lockService.tryWriteLockAndRun = jest.fn((_params, callback) => callback()); AWSMock.mock('S3', 'putBucketPolicy', putBucketPolicyMock); - mockDeleteEgressStoreRole(egressStoreId); await dataEgressService.terminateEgressStore(requestContext, envId); } @@ -1236,4 +1206,37 @@ describe('DataEgressService', () => { ); }); }); + + it('should successfully delete egress store role', async () => { + // BUILD + const policyArn = 'test-PermissionBoundaryArn'; + AWSMock.mock('IAM', 'listAttachedRolePolicies', (params, callback) => { + expect(params.RoleName).toEqual(`swb-study-${egressStoreId}`); + callback(null, { + AttachedPolicies: [ + { + PolicyName: 'test-PermissionBoundaryName', + PolicyArn: policyArn, + }, + ], + }); + }); + AWSMock.mock('IAM', 'detachRolePolicy', (params, callback) => { + expect(params).toMatchObject({ RoleName: `swb-study-${egressStoreId}`, PolicyArn: policyArn }); + callback(); + }); + AWSMock.mock('IAM', 'deleteRole', (params, callback) => { + expect(params).toMatchObject({ RoleName: `swb-study-${egressStoreId}` }); + callback(); + }); + AWSMock.mock('IAM', 'deletePolicy', (params, callback) => { + expect(params).toMatchObject({ PolicyArn: policyArn }); + callback(); + }); + + const egressStoreId = 'abc'; + + // OPERATE and CHECK + await expect(dataEgressService.deleteMainAccountEgressStoreRole(egressStoreId)).resolves.toBeUndefined(); + }); }); diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/data-egress/data-egress-service.js b/addons/addon-base-raas/packages/base-raas-services/lib/data-egress/data-egress-service.js index 392a5ae1bc..b215d686a2 100644 --- a/addons/addon-base-raas/packages/base-raas-services/lib/data-egress/data-egress-service.js +++ b/addons/addon-base-raas/packages/base-raas-services/lib/data-egress/data-egress-service.js @@ -215,8 +215,6 @@ class DataEgressService extends Service { true, ); } - // TODO: Move this to the terminate workflow loop runner - // await this.deleteMainAccountEgressStoreRole(egressStoreInfo.id); const egressStoreDdbLockId = `egress-store-ddb-access-${egressStoreInfo.id}`; egressStoreInfo.status = TERMINATED_STATUS_CODE; diff --git a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/__test__/launch-product.test.js b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/__test__/launch-product.test.js index 9d18df64ad..d9b903a295 100644 --- a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/__test__/launch-product.test.js +++ b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/__test__/launch-product.test.js @@ -126,4 +126,4 @@ describe('LaunchProduct', () => { expect(namespaceIndex).toEqual(-1); }); }); -}); \ No newline at end of file +}); diff --git a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/__test__/terminate-product.test.js b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/__test__/terminate-product.test.js new file mode 100644 index 0000000000..11d20c3900 --- /dev/null +++ b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/__test__/terminate-product.test.js @@ -0,0 +1,88 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +const WorkflowPayload = require('@aws-ee/workflow-engine/lib/workflow-payload'); +const TerminateProduct = require('../terminate-product/terminate-product'); + +describe('TerminateProduct', () => { + const meta = {}; + const input = {}; + + const tp = new TerminateProduct({ + step: { config: {} }, + workflowPayload: new WorkflowPayload({ meta, input, workflowInstance: {} }), + }); + + beforeAll(async () => { + tp.payload = { + getValue: value => { + if (value === 'envId') { + return 'abc'; + } + return ''; + }, + }; + }); + + describe('start', () => { + it('deleteMainAccountEgressStoreRole should be called once if Egress Store is enabled', async () => { + // BUILD + const deleteMainAccountEgressStoreRole = jest.fn(); + mockDataEgressService(deleteMainAccountEgressStoreRole); + mockEnableEgressStoreSettings(true); + + // OPERATE + await tp.start(); + + // CHECK + expect(deleteMainAccountEgressStoreRole).toHaveBeenCalledTimes(1); + }); + + it('deleteMainAccountEgressStoreRole should NOT be called if Egress Store is disabled', async () => { + // BUILD + const deleteMainAccountEgressStoreRole = jest.fn(); + mockDataEgressService(deleteMainAccountEgressStoreRole); + mockEnableEgressStoreSettings(false); + + // OPERATE + await tp.start(); + + // CHECK + expect(deleteMainAccountEgressStoreRole).toHaveBeenCalledTimes(0); + }); + + function mockDataEgressService(deleteMainAccountEgressStoreRole) { + const dataEgressService = {}; + dataEgressService.deleteMainAccountEgressStoreRole = deleteMainAccountEgressStoreRole; + tp.mustFindServices = jest.fn().mockImplementation(param => { + if (param === 'dataEgressService') { + return dataEgressService; + } + throw new Error('Unexpected service'); + }); + } + + function mockEnableEgressStoreSettings(isEnabled) { + tp.settings = { + getBoolean: key => { + if (key === 'enableEgressStore') { + return isEnabled; + } + throw new Error('Unexpected key'); + }, + }; + } + }); +}); diff --git a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/terminate-product/terminate-product.js b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/terminate-product/terminate-product.js index 2908e2b1ee..a3188ae9f5 100644 --- a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/terminate-product/terminate-product.js +++ b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/terminate-product/terminate-product.js @@ -72,7 +72,7 @@ class TerminateProduct extends StepBase { const enableEgressStore = this.settings.getBoolean(settingKeys.enableEgressStore); if (enableEgressStore) { - const [dataEgressService] = await this.mustFindServices(['dataEgressService']); + const dataEgressService = await this.mustFindServices('dataEgressService'); await dataEgressService.deleteMainAccountEgressStoreRole(envId); } From ee7303626645305c1e99d2b4b9a0614719408452 Mon Sep 17 00:00:00 2001 From: Tim Nguyen Date: Mon, 11 Oct 2021 21:19:52 -0400 Subject: [PATCH 3/3] Update unit test --- .../lib/steps/__test__/terminate-product.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/__test__/terminate-product.test.js b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/__test__/terminate-product.test.js index 11d20c3900..c27f9d916d 100644 --- a/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/__test__/terminate-product.test.js +++ b/addons/addon-environment-sc-api/packages/environment-sc-workflow-steps/lib/steps/__test__/terminate-product.test.js @@ -60,7 +60,7 @@ describe('TerminateProduct', () => { await tp.start(); // CHECK - expect(deleteMainAccountEgressStoreRole).toHaveBeenCalledTimes(0); + expect(deleteMainAccountEgressStoreRole).not.toHaveBeenCalled(); }); function mockDataEgressService(deleteMainAccountEgressStoreRole) {