fix(apigatewayv2-integrations): in case of multiple routes, only one …

…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*
aws · Mar 8, 2022 · 1e352ca · 1e352ca
1 parent 05f8b6f
commit 1e352ca
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 2 deletions.
diff --git a/packages/@aws-cdk/aws-apigatewayv2-integrations/lib/http/lambda.ts b/packages/@aws-cdk/aws-apigatewayv2-integrations/lib/http/lambda.ts
@@ -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,
@@ -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,

diff --git a/packages/@aws-cdk/aws-apigatewayv2-integrations/test/http/lambda.test.ts b/packages/@aws-cdk/aws-apigatewayv2-integrations/test/http/lambda.test.ts
@@ -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';
@@ -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) {

diff --git a/packages/@aws-cdk/aws-apigatewayv2/lib/http/integration.ts b/packages/@aws-cdk/aws-apigatewayv2/lib/http/integration.ts
@@ -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.
    */