Skip to content

Commit

Permalink
fix(apigatewayv2-integrations): in case of multiple routes, only one …
Browse files Browse the repository at this point in the history
…execute permission is created (#18716)

When multiple routes are defined for a single lambda integration, only one of
the routes gets permission to execute the function. This is because the
permissions are added when the integration is bound to the route, which happens
only once per integration.

Split the `_bindToRoute` workflow into two parts:

1. The actual bind, followed by the creation of an `HttpIntegration`. We keep
doing this only once per integration.
2. A post-bind step, that happens for every route. 

In the case of `HttpLambdaIntegration`, adding the permission has been moved to
the post bind step. All other integrations remain the same.

Fixes #18201.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
otaviomacedo authored Mar 8, 2022
1 parent 05f8b6f commit 1e352ca
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export class HttpLambdaIntegration extends HttpRouteIntegration {
this._id = id;
}

public bind(options: HttpRouteIntegrationBindOptions): HttpRouteIntegrationConfig {
protected completeBind(options: HttpRouteIntegrationBindOptions) {
const route = options.route;
this.handler.addPermission(`${this._id}-Permission`, {
scope: options.scope,
Expand All @@ -61,7 +61,9 @@ export class HttpLambdaIntegration extends HttpRouteIntegration {
resourceName: `*/*${route.path ?? ''}`, // empty string in the case of the catch-all route $default
}),
});
}

public bind(_: HttpRouteIntegrationBindOptions): HttpRouteIntegrationConfig {
return {
type: HttpIntegrationType.AWS_PROXY,
uri: this.handler.functionArn,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Template } from '@aws-cdk/assertions';
import { Match, Template } from '@aws-cdk/assertions';
import { HttpApi, HttpRoute, HttpRouteKey, MappingValue, ParameterMapping, PayloadFormatVersion } from '@aws-cdk/aws-apigatewayv2';
import { Code, Function, Runtime } from '@aws-cdk/aws-lambda';
import { App, Stack } from '@aws-cdk/core';
Expand Down Expand Up @@ -71,6 +71,41 @@ describe('LambdaProxyIntegration', () => {

expect(() => app.synth()).not.toThrow();
});

test('multiple routes for the same lambda integration', () => {
const app = new App();
const lambdaStack = new Stack(app, 'lambdaStack');
const fooFn = fooFunction(lambdaStack, 'Fn');

const stack = new Stack(app, 'apigwStack');
const api = new HttpApi(stack, 'httpApi');
const integration = new HttpLambdaIntegration('Integration', fooFn);

api.addRoutes({
path: '/foo',
integration,
});

api.addRoutes({
path: '/bar',
integration,
});

// Make sure we have two permissions -- one for each method -- but a single integration
Template.fromStack(stack).hasResourceProperties('AWS::Lambda::Permission', {
SourceArn: {
'Fn::Join': ['', Match.arrayWith([':execute-api:', '/*/*/foo'])],
},
});

Template.fromStack(stack).hasResourceProperties('AWS::Lambda::Permission', {
SourceArn: {
'Fn::Join': ['', Match.arrayWith([':execute-api:', '/*/*/bar'])],
},
});

Template.fromStack(stack).resourceCountIs('AWS::ApiGatewayV2::Integration', 1);
});
});

function fooFunction(stack: Stack, id: string) {
Expand Down
13 changes: 13 additions & 0 deletions packages/@aws-cdk/aws-apigatewayv2/lib/http/integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -332,9 +332,22 @@ export abstract class HttpRouteIntegration {
credentials: config.credentials,
});
}
this.completeBind(options);
return { integrationId: this.integration.integrationId };
}

/**
* Complete the binding of the integration to the route. In some cases, there is
* some additional work to do, such as adding permissions for the API to access
* the target. This work is necessary whether the integration has just been
* created for this route or it is an existing one, previously created for other
* routes. In most cases, however, concrete implementations do not need to
* override this method.
*/
protected completeBind(_options: HttpRouteIntegrationBindOptions): void {
// no-op by default
}

/**
* Bind this integration to the route.
*/
Expand Down

0 comments on commit 1e352ca

Please sign in to comment.