fix: Delete egress role in workflow instead of API (#740)

Co-authored-by: Tim Nguyen <thingut@amazon.com>

addons/addon-base-raas/packages/base-raas-services/lib/data-egress/__test__/data-egress-service.test.js

@@ -550,38 +550,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);
     });
@@ -655,7 +626,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);
     }
@@ -1226,4 +1196,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();
+  });
 });

addons/addon-base-raas/packages/base-raas-services/lib/data-egress/data-egress-service.js

@@ -206,7 +206,6 @@ class DataEgressService extends Service {
           true,
         );
       }
-      await this.deleteMainAccountEgressStoreRole(egressStoreInfo.id);
 
       const egressStoreDdbLockId = `egress-store-ddb-access-${egressStoreInfo.id}`;
       egressStoreInfo.status = TERMINATED_STATUS_CODE;

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);
     });
   });
-});
+});

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).not.toHaveBeenCalled();
+    });
+
+    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');
+        },
+      };
+    }
+  });
+});

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

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-*'