Skip to content

Commit 226ca0b

Browse files
cogwirrelcogwirrel
committed
fix(apigateway): consolidate lambda permissions when reused for multiple operations
The maximum Lambda permission policy size could be exceeded for APIs which reused the same Lambda function for multiple operations, as the integration added a new permission for each operation, scoped down to the specific operation. This change updates both the REST and HTTP API lambda integrations to consolidate permissions when the same handler is used for two or more methods, creating a permission scoped to the entire API rather than the operation. The behaviour remains the same where individual lambdas are used for operations. Fixes #9327 Fixes #19535
1 parent effa46d commit 226ca0b

File tree

4 files changed

+295
-24
lines changed

4 files changed

+295
-24
lines changed

‎packages/aws-cdk-lib/aws-apigateway/lib/integrations/lambda.ts‎

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -59,21 +59,40 @@ export class LambdaIntegration extends AwsIntegration {
5959
const bindResult = super.bind(method);
6060
const principal = new iam.ServicePrincipal('apigateway.amazonaws.com');
6161

62-
const desc = `${Names.nodeUniqueId(method.api.node)}.${method.httpMethod}.${method.resource.path.replace(/\//g, '.')}`;
62+
// Permission prefix is unique to the handler and the api
63+
const descPrefix = `${Names.nodeUniqueId(this.handler.node)}.${Names.nodeUniqueId(method.api.node)}`;
64+
const desc = `${descPrefix}.${method.httpMethod}.${method.resource.path.replace(/\//g, '.')}`;
6365

64-
this.handler.addPermission(`ApiPermission.${desc}`, {
65-
principal,
66-
scope: method,
67-
sourceArn: Lazy.string({ produce: () => method.methodArn }),
68-
});
66+
// Find any existing permissions for this handler and this API
67+
const otherHandlerPermissions = method.api.node.findAll()
68+
.filter(c => c instanceof lambda.CfnPermission && (
69+
c.node.id.startsWith(`ApiPermission.${descPrefix}.`)
70+
|| c.node.id.startsWith(`ApiPermission.Test.${descPrefix}.`)));
6971

70-
// add permission to invoke from the console
71-
if (this.enableTest) {
72-
this.handler.addPermission(`ApiPermission.Test.${desc}`, {
72+
if (otherHandlerPermissions.length > 0) {
73+
// If there are existing permissions, remove them in favour of a single permission scoped only to the API
74+
otherHandlerPermissions.forEach(permission => permission.node.scope?.node?.tryRemoveChild?.(permission.node.id));
75+
this.handler.addPermission(`ApiPermission.${descPrefix}.Any`, {
76+
principal,
77+
scope: method.api,
78+
sourceArn: Lazy.string({ produce: () => method.api.arnForExecuteApi() }),
79+
});
80+
} else {
81+
// No other permissions exist, so scope the permission to the specific method
82+
this.handler.addPermission(`ApiPermission.${desc}`, {
7383
principal,
7484
scope: method,
75-
sourceArn: method.testMethodArn,
85+
sourceArn: Lazy.string({ produce: () => method.methodArn }),
7686
});
87+
88+
// add permission to invoke from the console
89+
if (this.enableTest) {
90+
this.handler.addPermission(`ApiPermission.Test.${desc}`, {
91+
principal,
92+
scope: method,
93+
sourceArn: method.testMethodArn,
94+
});
95+
}
7796
}
7897

7998
let functionName;

‎packages/aws-cdk-lib/aws-apigateway/test/integrations/lambda.test.ts‎

Lines changed: 196 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,4 +297,200 @@ describe('lambda', () => {
297297
// should be a literal string.
298298
expect(bindResult?.deploymentToken).toEqual(JSON.stringify({ functionName: 'myfunc' }));
299299
});
300+
301+
test('creates single API-scoped permission when handler is reused', () => {
302+
// GIVEN
303+
const stack = new cdk.Stack();
304+
const api = new apigateway.RestApi(stack, 'my-api');
305+
const handler = new lambda.Function(stack, 'Handler', {
306+
runtime: lambda.Runtime.NODEJS_LATEST,
307+
handler: 'index.handler',
308+
code: lambda.Code.fromInline('foo'),
309+
});
310+
311+
// WHEN - Add two methods with lambda integrations with the same handler
312+
const integ1 = new apigateway.LambdaIntegration(handler);
313+
api.root.addMethod('GET', integ1);
314+
const integ2 = new apigateway.LambdaIntegration(handler);
315+
api.root.addMethod('POST', integ2);
316+
317+
// THEN - Should have API-scoped permissions instead of method-specific ones
318+
const template = Template.fromStack(stack);
319+
320+
// Should have 1 permission total
321+
template.resourceCountIs('AWS::Lambda::Permission', 1);
322+
323+
// Verify the API-scoped permission exists (without stage or method in the ARN)
324+
template.hasResourceProperties('AWS::Lambda::Permission', {
325+
Action: 'lambda:InvokeFunction',
326+
FunctionName: {
327+
'Fn::GetAtt': [
328+
'Handler886CB40B',
329+
'Arn',
330+
],
331+
},
332+
Principal: 'apigateway.amazonaws.com',
333+
SourceArn: {
334+
'Fn::Join': [
335+
'',
336+
[
337+
'arn:',
338+
{ Ref: 'AWS::Partition' },
339+
':execute-api:',
340+
{ Ref: 'AWS::Region' },
341+
':',
342+
{ Ref: 'AWS::AccountId' },
343+
':',
344+
{ Ref: 'myapi4C7BF186' },
345+
'/*/*/*',
346+
],
347+
],
348+
},
349+
});
350+
});
351+
352+
test('different handlers maintain method-specific permissions', () => {
353+
// GIVEN
354+
const stack = new cdk.Stack();
355+
const api = new apigateway.RestApi(stack, 'my-api');
356+
const handler1 = new lambda.Function(stack, 'Handler1', {
357+
runtime: lambda.Runtime.NODEJS_LATEST,
358+
handler: 'index.handler',
359+
code: lambda.Code.fromInline('foo'),
360+
});
361+
const handler2 = new lambda.Function(stack, 'Handler2', {
362+
runtime: lambda.Runtime.NODEJS_LATEST,
363+
handler: 'index.handler',
364+
code: lambda.Code.fromInline('bar'),
365+
});
366+
367+
// WHEN - Add methods with different handlers
368+
const integ1 = new apigateway.LambdaIntegration(handler1);
369+
api.root.addMethod('GET', integ1);
370+
371+
const integ2 = new apigateway.LambdaIntegration(handler2);
372+
api.root.addMethod('POST', integ2);
373+
374+
// THEN - Each handler should have its own method-specific permissions
375+
const template = Template.fromStack(stack);
376+
377+
// Should have 4 permissions total: 2 method-specific + 2 test invoke permissions
378+
template.resourceCountIs('AWS::Lambda::Permission', 4);
379+
380+
// Verify handler1 has method-specific permission for GET
381+
template.hasResourceProperties('AWS::Lambda::Permission', {
382+
Action: 'lambda:InvokeFunction',
383+
FunctionName: {
384+
'Fn::GetAtt': [
385+
'Handler11CDD30AA',
386+
'Arn',
387+
],
388+
},
389+
Principal: 'apigateway.amazonaws.com',
390+
SourceArn: {
391+
'Fn::Join': [
392+
'',
393+
[
394+
'arn:',
395+
{ Ref: 'AWS::Partition' },
396+
':execute-api:',
397+
{ Ref: 'AWS::Region' },
398+
':',
399+
{ Ref: 'AWS::AccountId' },
400+
':',
401+
{ Ref: 'myapi4C7BF186' },
402+
'/',
403+
{ Ref: 'myapiDeploymentStageprod298F01AF' },
404+
'/GET/',
405+
],
406+
],
407+
},
408+
});
409+
410+
// Verify handler1 has test invoke permission for GET
411+
template.hasResourceProperties('AWS::Lambda::Permission', {
412+
Action: 'lambda:InvokeFunction',
413+
FunctionName: {
414+
'Fn::GetAtt': [
415+
'Handler11CDD30AA',
416+
'Arn',
417+
],
418+
},
419+
Principal: 'apigateway.amazonaws.com',
420+
SourceArn: {
421+
'Fn::Join': [
422+
'',
423+
[
424+
'arn:',
425+
{ Ref: 'AWS::Partition' },
426+
':execute-api:',
427+
{ Ref: 'AWS::Region' },
428+
':',
429+
{ Ref: 'AWS::AccountId' },
430+
':',
431+
{ Ref: 'myapi4C7BF186' },
432+
'/test-invoke-stage/GET/',
433+
],
434+
],
435+
},
436+
});
437+
438+
// Verify handler2 has method-specific permission for POST
439+
template.hasResourceProperties('AWS::Lambda::Permission', {
440+
Action: 'lambda:InvokeFunction',
441+
FunctionName: {
442+
'Fn::GetAtt': [
443+
'Handler267EDD214',
444+
'Arn',
445+
],
446+
},
447+
Principal: 'apigateway.amazonaws.com',
448+
SourceArn: {
449+
'Fn::Join': [
450+
'',
451+
[
452+
'arn:',
453+
{ Ref: 'AWS::Partition' },
454+
':execute-api:',
455+
{ Ref: 'AWS::Region' },
456+
':',
457+
{ Ref: 'AWS::AccountId' },
458+
':',
459+
{ Ref: 'myapi4C7BF186' },
460+
'/',
461+
{ Ref: 'myapiDeploymentStageprod298F01AF' },
462+
'/POST/',
463+
],
464+
],
465+
},
466+
});
467+
468+
// Verify handler2 has test invoke permission for POST
469+
template.hasResourceProperties('AWS::Lambda::Permission', {
470+
Action: 'lambda:InvokeFunction',
471+
FunctionName: {
472+
'Fn::GetAtt': [
473+
'Handler267EDD214',
474+
'Arn',
475+
],
476+
},
477+
Principal: 'apigateway.amazonaws.com',
478+
SourceArn: {
479+
'Fn::Join': [
480+
'',
481+
[
482+
'arn:',
483+
{ Ref: 'AWS::Partition' },
484+
':execute-api:',
485+
{ Ref: 'AWS::Region' },
486+
':',
487+
{ Ref: 'AWS::AccountId' },
488+
':',
489+
{ Ref: 'myapi4C7BF186' },
490+
'/test-invoke-stage/POST/',
491+
],
492+
],
493+
},
494+
});
495+
});
300496
});

‎packages/aws-cdk-lib/aws-apigatewayv2-integrations/lib/http/lambda.ts‎

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,9 @@ import {
77
ParameterMapping,
88
} from '../../../aws-apigatewayv2';
99
import { ServicePrincipal } from '../../../aws-iam';
10+
import * as lambda from '../../../aws-lambda';
1011
import { IFunction } from '../../../aws-lambda';
11-
import { Duration, Stack } from '../../../core';
12+
import { Duration, Names, Stack } from '../../../core';
1213

1314
/**
1415
* Lambda Proxy integration properties
@@ -58,15 +59,40 @@ export class HttpLambdaIntegration extends HttpRouteIntegration {
5859

5960
protected completeBind(options: HttpRouteIntegrationBindOptions) {
6061
const route = options.route;
61-
this.handler.addPermission(`${this._id}-Permission`, {
62-
scope: options.scope,
63-
principal: new ServicePrincipal('apigateway.amazonaws.com'),
64-
sourceArn: Stack.of(route).formatArn({
65-
service: 'execute-api',
66-
resource: route.httpApi.apiId,
67-
resourceName: `*/*${route.path ?? ''}`, // empty string in the case of the catch-all route $default
68-
}),
69-
});
62+
63+
// Permission prefix is unique to the handler and the API
64+
const descPrefix = `${Names.nodeUniqueId(this.handler.node)}.${Names.nodeUniqueId(route.httpApi.node)}`;
65+
const desc = `${descPrefix}.${this._id}`;
66+
67+
// Find any existing permissions for this handler and this API
68+
const otherHandlerPermissions = route.httpApi.node.findAll()
69+
.filter(c => c instanceof lambda.CfnPermission &&
70+
c.node.id.startsWith(`ApiPermission.${descPrefix}.`));
71+
72+
if (otherHandlerPermissions.length > 0) {
73+
// If there are existing permissions, remove them in favour of a single permission scoped to the API
74+
otherHandlerPermissions.forEach(permission => permission.node.scope?.node?.tryRemoveChild?.(permission.node.id));
75+
this.handler.addPermission(`ApiPermission.${descPrefix}.Any`, {
76+
scope: route.httpApi,
77+
principal: new ServicePrincipal('apigateway.amazonaws.com'),
78+
sourceArn: Stack.of(route).formatArn({
79+
service: 'execute-api',
80+
resource: route.httpApi.apiId,
81+
resourceName: '*/*/*',
82+
}),
83+
});
84+
} else {
85+
// No other permissions exist, so scope the permission to the specific route
86+
this.handler.addPermission(`ApiPermission.${desc}`, {
87+
scope: options.scope,
88+
principal: new ServicePrincipal('apigateway.amazonaws.com'),
89+
sourceArn: Stack.of(route).formatArn({
90+
service: 'execute-api',
91+
resource: route.httpApi.apiId,
92+
resourceName: `*/*${route.path ?? ''}`, // empty string in the case of the catch-all route $default
93+
}),
94+
});
95+
}
7096
}
7197

7298
public bind(_options: HttpRouteIntegrationBindOptions): HttpRouteIntegrationConfig {
@@ -79,3 +105,4 @@ export class HttpLambdaIntegration extends HttpRouteIntegration {
79105
};
80106
}
81107
}
108+

‎packages/aws-cdk-lib/aws-apigatewayv2-integrations/test/http/lambda.test.ts‎

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -108,20 +108,49 @@ describe('LambdaProxyIntegration', () => {
108108
integration,
109109
});
110110

111-
// Make sure we have two permissions -- one for each method -- but a single integration
111+
// Make sure we have only one permission scoped to the full api
112112
Template.fromStack(stack).hasResourceProperties('AWS::Lambda::Permission', {
113+
SourceArn: {
114+
'Fn::Join': ['', Match.arrayWith([':execute-api:', '/*/*/*'])],
115+
},
116+
});
117+
118+
Template.fromStack(stack).resourceCountIs('AWS::ApiGatewayV2::Integration', 1);
119+
});
120+
121+
test('different handlers maintain route-specific permissions', () => {
122+
const stack = new Stack();
123+
const api = new HttpApi(stack, 'HttpApi');
124+
const handler1 = fooFunction(stack, 'Handler1');
125+
const handler2 = fooFunction(stack, 'Handler2');
126+
127+
new HttpRoute(stack, 'Route1', {
128+
httpApi: api,
129+
integration: new HttpLambdaIntegration('Integration1', handler1),
130+
routeKey: HttpRouteKey.with('/foo'),
131+
});
132+
133+
new HttpRoute(stack, 'Route2', {
134+
httpApi: api,
135+
integration: new HttpLambdaIntegration('Integration2', handler2),
136+
routeKey: HttpRouteKey.with('/bar'),
137+
});
138+
139+
// Make sure we have two permissions -- one for each handler
140+
const template = Template.fromStack(stack);
141+
template.resourceCountIs('AWS::Lambda::Permission', 2);
142+
143+
template.hasResourceProperties('AWS::Lambda::Permission', {
113144
SourceArn: {
114145
'Fn::Join': ['', Match.arrayWith([':execute-api:', '/*/*/foo'])],
115146
},
116147
});
117148

118-
Template.fromStack(stack).hasResourceProperties('AWS::Lambda::Permission', {
149+
template.hasResourceProperties('AWS::Lambda::Permission', {
119150
SourceArn: {
120151
'Fn::Join': ['', Match.arrayWith([':execute-api:', '/*/*/bar'])],
121152
},
122153
});
123-
124-
Template.fromStack(stack).resourceCountIs('AWS::ApiGatewayV2::Integration', 1);
125154
});
126155
});
127156

0 commit comments

Comments
 (0)