Skip to content

Commit

Permalink
fix: stack generation logic when multiple paths ref same Lambda (#8673)
Browse files Browse the repository at this point in the history
  • Loading branch information
jhockett authored Nov 4, 2021
1 parent cb4332b commit fdbab02
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 43 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down Expand Up @@ -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') {
Expand All @@ -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,
);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
});
Expand Down
29 changes: 20 additions & 9 deletions packages/amplify-e2e-core/src/categories/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,7 @@ export function updateAPIWithResolutionStrategyWithModels(cwd: string, settings:
type RestApiSettings = {
allowGuestUsers?: boolean;
existingLambda?: boolean;
hasUserPoolGroups?: boolean;
isFirstRestApi?: boolean;
isCrud?: boolean;
path?: string;
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion packages/amplify-e2e-tests/src/__tests__/apigw.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit fdbab02

Please sign in to comment.