chore: address PR comments

aws-amplify · May 7, 2021 · 9ff685b · 9ff685b
1 parent cbe9a0f
commit 9ff685b
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 36 deletions.
diff --git a/packages/amplify-provider-awscloudformation/src/__tests__/initializer.test.ts b/packages/amplify-provider-awscloudformation/src/__tests__/initializer.test.ts
@@ -10,9 +10,7 @@ jest.mock('../aws-utils/aws-cfn');
 jest.mock('fs-extra');
 jest.mock('../amplify-service-manager');
 jest.mock('amplify-cli-core');
-jest.mock('../permission-boundary/permission-boundary', () => ({
-  configurePermissionBoundaryForInit: jest.fn(),
-}));
+jest.mock('../permission-boundary/permission-boundary');
 
 const CloudFormation_mock = CloudFormation as jest.MockedClass<typeof CloudFormation>;
 const amplifyServiceManager_mock = amplifyServiceManager as jest.Mocked<typeof amplifyServiceManager>;

diff --git a/...-provider-awscloudformation/src/__tests__/permission-boundary/permission-boundary.test.ts b/...-provider-awscloudformation/src/__tests__/permission-boundary/permission-boundary.test.ts
@@ -2,7 +2,7 @@ import { $TSContext } from 'amplify-cli-core';
 import { configurePermissionBoundaryForInit } from '../../permission-boundary/permission-boundary';
 import { setPermissionBoundaryArn, getPermissionBoundaryArn } from 'amplify-cli-core';
 import { prompt } from 'inquirer';
-import { getIAMClient } from '../../aws-utils/aws-iam';
+import { IAMClient } from '../../aws-utils/aws-iam';
 import { IAM } from 'aws-sdk';
 
 const permissionBoundaryArn = 'arn:aws:iam::123456789012:policy/some-policy-name';
@@ -16,7 +16,7 @@ jest.mock('../../aws-utils/aws-iam');
 const setPermissionBoundaryArn_mock = setPermissionBoundaryArn as jest.MockedFunction<typeof setPermissionBoundaryArn>;
 const getPermissionBoundaryArn_mock = getPermissionBoundaryArn as jest.MockedFunction<typeof getPermissionBoundaryArn>;
 const prompt_mock = prompt as jest.MockedFunction<typeof prompt>;
-const getIAMClient_mock = getIAMClient as jest.MockedFunction<typeof getIAMClient>;
+const IAMClient_mock = IAMClient as jest.Mocked<typeof IAMClient>;
 
 describe('configure permission boundary on init', () => {
   let context_stub: $TSContext;
@@ -97,23 +97,27 @@ describe('configure permission boundary on env add', () => {
 
   it('applies existing policy to new env when existing policy is accessible', async () => {
     getPermissionBoundaryArn_mock.mockReturnValueOnce(permissionBoundaryArn);
-    getIAMClient_mock.mockResolvedValueOnce(({
-      getPolicy: jest.fn().mockReturnValueOnce({
-        promise: jest.fn(),
-      }),
-    } as unknown) as IAM);
+    IAMClient_mock.getInstance.mockResolvedValueOnce({
+      client: ({
+        getPolicy: jest.fn().mockReturnValueOnce({
+          promise: jest.fn(),
+        }),
+      } as unknown) as IAM,
+    });
     await configurePermissionBoundaryForInit(context_stub);
     expect(setPermissionBoundaryArn_mock.mock.calls[0][0]).toEqual(permissionBoundaryArn);
     expect(prompt_mock).not.toHaveBeenCalled();
   });
 
   it('prompts for new policy when existing one is not accessible', async () => {
     getPermissionBoundaryArn_mock.mockReturnValueOnce(permissionBoundaryArn);
-    getIAMClient_mock.mockResolvedValueOnce(({
-      getPolicy: jest.fn().mockReturnValueOnce({
-        promise: jest.fn().mockRejectedValueOnce('test error'),
-      }),
-    } as unknown) as IAM);
+    IAMClient_mock.getInstance.mockResolvedValueOnce({
+      client: ({
+        getPolicy: jest.fn().mockReturnValueOnce({
+          promise: jest.fn().mockRejectedValueOnce('test error'),
+        }),
+      } as unknown) as IAM,
+    });
     const newPermissionBoundaryArn = 'thisIsANewArn';
     prompt_mock.mockResolvedValueOnce({
       permissionBoundaryArn: newPermissionBoundaryArn,
@@ -125,11 +129,13 @@ describe('configure permission boundary on env add', () => {
   it('fails when existing policy not accessible and --yes specified with no cmd arg', async () => {
     context_stub.input.options.yes = true;
     getPermissionBoundaryArn_mock.mockReturnValueOnce(permissionBoundaryArn);
-    getIAMClient_mock.mockResolvedValueOnce(({
-      getPolicy: jest.fn().mockReturnValueOnce({
-        promise: jest.fn().mockRejectedValueOnce('test error'),
-      }),
-    } as unknown) as IAM);
+    IAMClient_mock.getInstance.mockResolvedValueOnce({
+      client: ({
+        getPolicy: jest.fn().mockReturnValueOnce({
+          promise: jest.fn().mockRejectedValueOnce('test error'),
+        }),
+      } as unknown) as IAM,
+    });
     await expect(configurePermissionBoundaryForInit(context_stub)).rejects.toMatchInlineSnapshot(
       `[Error: A Permission Boundary ARN must be specified using --permission-boundary]`,
     );

diff --git a/packages/amplify-provider-awscloudformation/src/aws-utils/aws-iam.ts b/packages/amplify-provider-awscloudformation/src/aws-utils/aws-iam.ts
@@ -2,19 +2,28 @@ import aws from './aws.js';
 import awstype from 'aws-sdk';
 import { IAM } from 'aws-sdk';
 import { AwsSdkConfig } from '../utils/auth-types.js';
+import { loadConfiguration } from '../configuration-manager';
+import { $TSContext } from 'amplify-cli-core';
 
-let instance: IAM;
+export class IAMClient {
+  private static instance: IAMClient;
+  public readonly client: IAM;
 
-export const getIAMClient = async (sdkConfigProvider: () => Promise<AwsSdkConfig>) => {
-  if (instance) {
-    return instance;
+  static async getInstance(context: $TSContext, options: IAM.ClientConfiguration = {}): Promise<IAMClient> {
+    if (!IAMClient.instance) {
+      let cred: AwsSdkConfig;
+      try {
+        cred = await loadConfiguration(context);
+      } catch (e) {
+        // ignore missing config
+      }
+
+      IAMClient.instance = new IAMClient(cred, options);
+    }
+    return IAMClient.instance;
   }
-  let cred = {};
-  try {
-    cred = await sdkConfigProvider();
-  } catch (err) {
-    // ignore error
+
+  private constructor(creds: AwsSdkConfig, options: IAM.ClientConfiguration = {}) {
+    this.client = new (aws as typeof awstype).IAM({ ...creds, ...options });
   }
-  instance = new (aws as typeof awstype).IAM({ ...cred });
-  return instance;
-};
+}
diff --git a/packages/amplify-provider-awscloudformation/src/permission-boundary/permission-boundary.ts b/packages/amplify-provider-awscloudformation/src/permission-boundary/permission-boundary.ts
@@ -1,6 +1,6 @@
 import { $TSContext, stateManager, getPermissionBoundaryArn, setPermissionBoundaryArn } from 'amplify-cli-core';
 import { prompt } from 'inquirer';
-import { getIAMClient } from '../aws-utils/aws-iam';
+import { IAMClient } from '../aws-utils/aws-iam';
 import { loadConfiguration } from '../configuration-manager';
 
 export const configurePermissionBoundaryForExistingEnv = async (context: $TSContext) => {
@@ -18,7 +18,7 @@ export const configurePermissionBoundaryForInit = async (context: $TSContext) =>
       context.exeInfo.teamProviderInfo,
     );
   } else {
-    // amplfiy env add
+    // amplify env add
     await rolloverPermissionBoundaryToNewEnvironment(context);
   }
 };
@@ -113,9 +113,9 @@ const rolloverPermissionBoundaryToNewEnvironment = async (context: $TSContext) =
 };
 
 const isPolicyAccessible = async (context: $TSContext, policyArn: string) => {
-  const iamClient = await getIAMClient(() => loadConfiguration(context));
+  const iamClient = await IAMClient.getInstance(context);
   try {
-    await iamClient.getPolicy({ PolicyArn: policyArn }).promise();
+    await iamClient.client.getPolicy({ PolicyArn: policyArn }).promise();
   } catch (err) {
     // if there's an error, then the policy wasn't accessible
     return false;