fix: stack generation logic when multiple paths ref same Lambda (aws-…

…amplify#8673)
sachscode · Nov 4, 2021 · fdbab02 · fdbab02
1 parent cb4332b
commit fdbab02
Show file tree

Hide file tree

Showing 5 changed files with 62 additions and 43 deletions.
diff --git a/...ategory-api/src/provider-utils/awscloudformation/cdk-stack-builder/apigw-stack-builder.ts b/...ategory-api/src/provider-utils/awscloudformation/cdk-stack-builder/apigw-stack-builder.ts
@@ -78,7 +78,7 @@ export class AmplifyApigwResourceStack extends cdk.Stack implements AmplifyApigw
    */
   addCfnParameter(props: cdk.CfnParameterProps, logicalId: string): void {
     if (this._cfnParameterMap.has(logicalId)) {
-      throw new Error('logical id already exists');
+      throw new Error(`logical id "${logicalId}" already exists`);
     }
     this._cfnParameterMap.set(logicalId, new cdk.CfnParameter(this, logicalId, props));
   }
@@ -196,6 +196,7 @@ export class AmplifyApigwResourceStack extends cdk.Stack implements AmplifyApigw
   };
 
   private _constructCfnPaths(resourceName: string) {
+    const addedFunctionPermissions = new Set();
     for (const [pathName, path] of Object.entries(this._props.paths)) {
       let lambdaPermissionLogicalId: string;
       if (resourceName === 'AdminQueries') {
@@ -207,23 +208,26 @@ export class AmplifyApigwResourceStack extends cdk.Stack implements AmplifyApigw
         lambdaPermissionLogicalId = `function${path.lambdaFunction}Permission${resourceName}`;
       }
 
-      this.addLambdaPermissionCfnResource(
-        {
-          functionName: cdk.Fn.ref(`function${path.lambdaFunction}Name`),
-          action: 'lambda:InvokeFunction',
-          principal: 'apigateway.amazonaws.com',
-          sourceArn: cdk.Fn.join('', [
-            'arn:aws:execute-api:',
-            cdk.Fn.ref('AWS::Region'),
-            ':',
-            cdk.Fn.ref('AWS::AccountId'),
-            ':',
-            cdk.Fn.ref(resourceName),
-            '/*/*/*',
-          ]),
-        },
-        lambdaPermissionLogicalId,
-      );
+      if (addedFunctionPermissions.has(path.lambdaFunction)) {
+        addedFunctionPermissions.add(path.lambdaFunction);
+        this.addLambdaPermissionCfnResource(
+          {
+            functionName: cdk.Fn.ref(`function${path.lambdaFunction}Name`),
+            action: 'lambda:InvokeFunction',
+            principal: 'apigateway.amazonaws.com',
+            sourceArn: cdk.Fn.join('', [
+              'arn:aws:execute-api:',
+              cdk.Fn.ref('AWS::Region'),
+              ':',
+              cdk.Fn.ref('AWS::AccountId'),
+              ':',
+              cdk.Fn.ref(resourceName),
+              '/*/*/*',
+            ]),
+          },
+          lambdaPermissionLogicalId,
+        );
+      }
     }
   }
 

diff --git a/...egory-api/src/provider-utils/awscloudformation/cdk-stack-builder/apigw-stack-transform.ts b/...egory-api/src/provider-utils/awscloudformation/cdk-stack-builder/apigw-stack-transform.ts
@@ -76,21 +76,25 @@ export class ApigwStackTransform {
     }
 
     // Add Parameters
+    const addedFunctions = new Set();
     for (const path of Object.values(this.cliInputs.paths)) {
-      this.resourceTemplateObj.addCfnParameter(
-        {
-          type: 'String',
-          default: `function${path.lambdaFunction}Name`,
-        },
-        `function${path.lambdaFunction}Name`,
-      );
-      this.resourceTemplateObj.addCfnParameter(
-        {
-          type: 'String',
-          default: `function${path.lambdaFunction}Arn`,
-        },
-        `function${path.lambdaFunction}Arn`,
-      );
+      if (!addedFunctions.has(path.lambdaFunction)) {
+        addedFunctions.add(path.lambdaFunction);
+        this.resourceTemplateObj.addCfnParameter(
+          {
+            type: 'String',
+            default: `function${path.lambdaFunction}Name`,
+          },
+          `function${path.lambdaFunction}Name`,
+        );
+        this.resourceTemplateObj.addCfnParameter(
+          {
+            type: 'String',
+            default: `function${path.lambdaFunction}Arn`,
+          },
+          `function${path.lambdaFunction}Arn`,
+        );
+      }
     }
 
     this.resourceTemplateObj.addCfnParameter(

diff --git a/...tegory-api/src/provider-utils/awscloudformation/service-walkthroughs/apigw-walkthrough.ts b/...tegory-api/src/provider-utils/awscloudformation/service-walkthroughs/apigw-walkthrough.ts
@@ -338,7 +338,7 @@ async function ensureAuth(context: $TSContext, apiRequirements: ApiRequirements,
 
 async function askCRUD(userType: string, permissions: string[] = []) {
   const crudOptions = ['create', 'read', 'update', 'delete'];
-  const crudAnswers = await prompter.pick<'many', string>(`What permissions do you want to grant to ${userType}`, crudOptions, {
+  const crudAnswers = await prompter.pick<'many', string>(`What permissions do you want to grant to ${userType} users?`, crudOptions, {
     returnSize: 'many',
     initial: permissions.map(p => crudOptions.indexOf(p)),
   });

diff --git a/packages/amplify-e2e-core/src/categories/api.ts b/packages/amplify-e2e-core/src/categories/api.ts
@@ -404,6 +404,7 @@ export function updateAPIWithResolutionStrategyWithModels(cwd: string, settings:
 type RestApiSettings = {
   allowGuestUsers?: boolean;
   existingLambda?: boolean;
+  hasUserPoolGroups?: boolean;
   isFirstRestApi?: boolean;
   isCrud?: boolean;
   path?: string;
@@ -427,8 +428,10 @@ export function addRestApi(cwd: string, settings: RestApiSettings) {
           .sendConfirmYes()
           .wait('Select the REST API you want to update')
           .sendCarriageReturn() // Select the first REST API
+          .wait('What would you like to do?')
+          .sendCarriageReturn()
           .wait('Provide a path')
-          .sendLine(settings.path)
+          .sendLine(settings.path ?? RETURN)
           .wait('Choose a lambda source')
           .sendKeyDown()
           .sendCarriageReturn() // Existing lambda
@@ -496,22 +499,30 @@ export function addRestApi(cwd: string, settings: RestApiSettings) {
     chain.wait('Restrict API access');
 
     if (settings.restrictAccess) {
-      chain.sendConfirmYes().wait('Who should have access');
+      chain.sendConfirmYes();
+
+      if (settings.hasUserPoolGroups) {
+        chain.wait('Restrict access by?').sendCarriageReturn(); // Auth/Guest Users
+      }
+
+      chain.wait('Who should have access?');
 
       if (!settings.allowGuestUsers) {
         chain
           .sendCarriageReturn() // Authenticated users only
-          .wait('What kind of access do you want for Authenticated users')
-          .sendLine('a'); // CRUD permissions
+          .wait('What permissions do you want to grant to Authenticated')
+          .sendCtrlA()
+          .sendCarriageReturn(); // CRUD permissions
       } else {
         chain
           .sendKeyDown()
           .sendCarriageReturn() // Authenticated and Guest users
-          .wait('What kind of access do you want for Authenticated users')
-          .sendLine('a') // CRUD permissions for authenticated users
-          .wait('What kind of access do you want for Guest users')
-          .sendKeyDown()
-          .send(' '); // R permissions for guest users
+          .wait('What permissions do you want to grant to Authenticated')
+          .sendCtrlA() // CRUD permissions for Authenticated users
+          .sendCarriageReturn()
+          .wait('What permissions do you want to grant to Guest')
+          .send(' ') // C permissions for guest users
+          .sendCarriageReturn();
       }
     } else {
       chain.sendConfirmNo(); // Do not restrict access

diff --git a/packages/amplify-e2e-tests/src/__tests__/apigw.test.ts b/packages/amplify-e2e-tests/src/__tests__/apigw.test.ts
@@ -31,7 +31,7 @@ describe('API Gateway e2e tests', () => {
     await addAuthWithGroupsAndAdminAPI(projRoot); // Groups: Admins, Users
     await amplifyPushAuth(projRoot);
     await addRestApi(projRoot, { isFirstRestApi: false, path: '/foo' });
-    await addRestApi(projRoot, { restrictAccess: true, allowGuestUsers: true });
+    await addRestApi(projRoot, { isFirstRestApi: false, restrictAccess: true, allowGuestUsers: true, hasUserPoolGroups: true });
     await amplifyPushAuth(projRoot); // Pushes multiple rest api updates
     const projMeta = getProjectMeta(projRoot);
     expect(projMeta).toBeDefined();