From 64798af2b46114f64098c018d76c93d07e7c28c0 Mon Sep 17 00:00:00 2001 From: Niranjan Jayakar Date: Fri, 26 Nov 2021 14:10:28 +0000 Subject: [PATCH] fix(apigatewayv2): integration class does not render an integration resource (#17729) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Routes on APIGateway V2 can integrate with different backends. This is achieved by creating the CFN resource `AWS::ApiGatewayV2::Integration` that is then referenced in the resource for the Route. Currently, the `IHttpRouteIntegration` (and `IWebSocketRouteIntegration`) interface represents a unique backend that a route can integrate with, using the CDK "bind" pattern. An integration can be bound to any number of routes but should be rendered into a single instance of `AWS::ApiGatewayV2::Integration` resource. To achieve this currently, the `HttpApi` (and `WebSocketApi`) class holds a cache of all integrations defined against its routes. This is the wrong level of caching and causes a number of problems. 1. We rely on the configuration of the `AWS::ApiGateway::Integration` resource to determine if one already exists. This means that two instances of `IHttpRouteIntegration` can result in rendering only one instance of `AWS::ApiGateway::Integration` resource. Users may want to intentionally generate multiple instances of `AWS::ApiGateway::Integration` classes with the same configuration. Taking away this power with CDK "magic" is just confusing. 2. Currently, we allow using the same instance of `IHttpRouteIntegration` (or `IWebSocketRouteIntegration`) to be bound to routes in different `HttpApi`. When bound to the route, the CDK renders an instance of `AWS::ApiGatewayV2::Integration` for each API. This is another "magic" that has the potential for user confusion and bugs. The solution is to KeepItSimpleā„¢. Remove the API level caching and simply cache at the level of each integration. This ensures that each instance of `HttpRouteIntegration` (previously `IHttpRouteIntegration`) renders to exactly one instance of `AWS::ApiGatewayV2::Integration`. Disallow using the same instance of `HttpRouteIntegration` across different instances of `HttpApi`. fixes #13213 BREAKING CHANGE: The interface `IHttpRouteIntegration` is replaced by the abstract class `HttpRouteIntegration`. * **apigatewayv2:** The interface `IWebSocketRouteIntegration` is now replaced by the abstract class `WebSocketRouteIntegration`. * **apigatewayv2:** Previously, we allowed the usage of integration classes to be used with routes defined in multiple `HttpApi` instances (or `WebSocketApi` instances). This is now disallowed, and separate instances must be created for each instance of `HttpApi` or `WebSocketApi`. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../test/http/jwt.test.ts | 4 +- .../test/http/lambda.test.ts | 4 +- .../test/http/user-pool.test.ts | 4 +- .../lib/http/http-proxy.ts | 5 +- .../lib/http/lambda.ts | 5 +- .../lib/http/private/integration.ts | 4 +- .../lib/websocket/lambda.ts | 8 ++-- .../aws-apigatewayv2/lib/common/base.ts | 5 -- .../@aws-cdk/aws-apigatewayv2/lib/http/api.ts | 35 +------------- .../aws-apigatewayv2/lib/http/integration.ts | 42 ++++++++++++++-- .../aws-apigatewayv2/lib/http/route.ts | 10 ++-- .../lib/private/integration-cache.ts | 29 ----------- .../aws-apigatewayv2/lib/websocket/api.ts | 25 ---------- .../lib/websocket/integration.ts | 37 ++++++++++++-- .../aws-apigatewayv2/lib/websocket/route.ts | 10 ++-- .../aws-apigatewayv2/test/http/api.test.ts | 4 +- .../aws-apigatewayv2/test/http/route.test.ts | 48 +++++++++---------- .../test/websocket/api.test.ts | 4 +- .../test/websocket/route.test.ts | 44 ++++++++++++++++- 19 files changed, 172 insertions(+), 155 deletions(-) delete mode 100644 packages/@aws-cdk/aws-apigatewayv2/lib/private/integration-cache.ts diff --git a/packages/@aws-cdk/aws-apigatewayv2-authorizers/test/http/jwt.test.ts b/packages/@aws-cdk/aws-apigatewayv2-authorizers/test/http/jwt.test.ts index f97e3d4c24d74..c65642d8df4d3 100644 --- a/packages/@aws-cdk/aws-apigatewayv2-authorizers/test/http/jwt.test.ts +++ b/packages/@aws-cdk/aws-apigatewayv2-authorizers/test/http/jwt.test.ts @@ -1,5 +1,5 @@ import { Template } from '@aws-cdk/assertions'; -import { HttpApi, HttpIntegrationType, HttpRouteIntegrationBindOptions, IHttpRouteIntegration, PayloadFormatVersion } from '@aws-cdk/aws-apigatewayv2'; +import { HttpApi, HttpIntegrationType, HttpRouteIntegrationBindOptions, HttpRouteIntegration, PayloadFormatVersion } from '@aws-cdk/aws-apigatewayv2'; import { Stack } from '@aws-cdk/core'; import { HttpJwtAuthorizer } from '../../lib'; @@ -59,7 +59,7 @@ describe('HttpJwtAuthorizer', () => { }); }); -class DummyRouteIntegration implements IHttpRouteIntegration { +class DummyRouteIntegration extends HttpRouteIntegration { public bind(_: HttpRouteIntegrationBindOptions) { return { payloadFormatVersion: PayloadFormatVersion.VERSION_2_0, diff --git a/packages/@aws-cdk/aws-apigatewayv2-authorizers/test/http/lambda.test.ts b/packages/@aws-cdk/aws-apigatewayv2-authorizers/test/http/lambda.test.ts index c9fdf62c17d57..28b007dafd359 100644 --- a/packages/@aws-cdk/aws-apigatewayv2-authorizers/test/http/lambda.test.ts +++ b/packages/@aws-cdk/aws-apigatewayv2-authorizers/test/http/lambda.test.ts @@ -1,5 +1,5 @@ import { Match, Template } from '@aws-cdk/assertions'; -import { HttpApi, HttpIntegrationType, HttpRouteIntegrationBindOptions, IHttpRouteIntegration, PayloadFormatVersion } from '@aws-cdk/aws-apigatewayv2'; +import { HttpApi, HttpIntegrationType, HttpRouteIntegrationBindOptions, HttpRouteIntegration, PayloadFormatVersion } from '@aws-cdk/aws-apigatewayv2'; import { Code, Function, Runtime } from '@aws-cdk/aws-lambda'; import { Duration, Stack } from '@aws-cdk/core'; import { HttpLambdaAuthorizer, HttpLambdaResponseType } from '../../lib'; @@ -170,7 +170,7 @@ describe('HttpLambdaAuthorizer', () => { }); }); -class DummyRouteIntegration implements IHttpRouteIntegration { +class DummyRouteIntegration extends HttpRouteIntegration { public bind(_: HttpRouteIntegrationBindOptions) { return { payloadFormatVersion: PayloadFormatVersion.VERSION_2_0, diff --git a/packages/@aws-cdk/aws-apigatewayv2-authorizers/test/http/user-pool.test.ts b/packages/@aws-cdk/aws-apigatewayv2-authorizers/test/http/user-pool.test.ts index 127b389b8b0f2..7189c060b877b 100644 --- a/packages/@aws-cdk/aws-apigatewayv2-authorizers/test/http/user-pool.test.ts +++ b/packages/@aws-cdk/aws-apigatewayv2-authorizers/test/http/user-pool.test.ts @@ -1,5 +1,5 @@ import { Template } from '@aws-cdk/assertions'; -import { HttpApi, HttpIntegrationType, HttpRouteIntegrationBindOptions, IHttpRouteIntegration, PayloadFormatVersion } from '@aws-cdk/aws-apigatewayv2'; +import { HttpApi, HttpIntegrationType, HttpRouteIntegrationBindOptions, HttpRouteIntegration, PayloadFormatVersion } from '@aws-cdk/aws-apigatewayv2'; import { UserPool } from '@aws-cdk/aws-cognito'; import { Stack } from '@aws-cdk/core'; import { HttpUserPoolAuthorizer } from '../../lib'; @@ -112,7 +112,7 @@ describe('HttpUserPoolAuthorizer', () => { }); }); -class DummyRouteIntegration implements IHttpRouteIntegration { +class DummyRouteIntegration extends HttpRouteIntegration { public bind(_: HttpRouteIntegrationBindOptions) { return { payloadFormatVersion: PayloadFormatVersion.VERSION_2_0, diff --git a/packages/@aws-cdk/aws-apigatewayv2-integrations/lib/http/http-proxy.ts b/packages/@aws-cdk/aws-apigatewayv2-integrations/lib/http/http-proxy.ts index 70873c9582fc8..879912ee881cf 100644 --- a/packages/@aws-cdk/aws-apigatewayv2-integrations/lib/http/http-proxy.ts +++ b/packages/@aws-cdk/aws-apigatewayv2-integrations/lib/http/http-proxy.ts @@ -3,7 +3,7 @@ import { HttpRouteIntegrationBindOptions, HttpRouteIntegrationConfig, HttpMethod, - IHttpRouteIntegration, + HttpRouteIntegration, ParameterMapping, PayloadFormatVersion, } from '@aws-cdk/aws-apigatewayv2'; @@ -34,8 +34,9 @@ export interface HttpProxyIntegrationProps { /** * The HTTP Proxy integration resource for HTTP API */ -export class HttpProxyIntegration implements IHttpRouteIntegration { +export class HttpProxyIntegration extends HttpRouteIntegration { constructor(private readonly props: HttpProxyIntegrationProps) { + super(); } public bind(_: HttpRouteIntegrationBindOptions): HttpRouteIntegrationConfig { diff --git a/packages/@aws-cdk/aws-apigatewayv2-integrations/lib/http/lambda.ts b/packages/@aws-cdk/aws-apigatewayv2-integrations/lib/http/lambda.ts index 358263f724bda..ff5750da71077 100644 --- a/packages/@aws-cdk/aws-apigatewayv2-integrations/lib/http/lambda.ts +++ b/packages/@aws-cdk/aws-apigatewayv2-integrations/lib/http/lambda.ts @@ -2,7 +2,7 @@ import { HttpIntegrationType, HttpRouteIntegrationBindOptions, HttpRouteIntegrationConfig, - IHttpRouteIntegration, + HttpRouteIntegration, PayloadFormatVersion, ParameterMapping, } from '@aws-cdk/aws-apigatewayv2'; @@ -37,9 +37,10 @@ export interface LambdaProxyIntegrationProps { /** * The Lambda Proxy integration resource for HTTP API */ -export class LambdaProxyIntegration implements IHttpRouteIntegration { +export class LambdaProxyIntegration extends HttpRouteIntegration { constructor(private readonly props: LambdaProxyIntegrationProps) { + super(); } public bind(options: HttpRouteIntegrationBindOptions): HttpRouteIntegrationConfig { diff --git a/packages/@aws-cdk/aws-apigatewayv2-integrations/lib/http/private/integration.ts b/packages/@aws-cdk/aws-apigatewayv2-integrations/lib/http/private/integration.ts index 6d32b22794722..c7e671f59879c 100644 --- a/packages/@aws-cdk/aws-apigatewayv2-integrations/lib/http/private/integration.ts +++ b/packages/@aws-cdk/aws-apigatewayv2-integrations/lib/http/private/integration.ts @@ -3,7 +3,7 @@ import { HttpIntegrationType, HttpRouteIntegrationBindOptions, HttpRouteIntegrationConfig, - IHttpRouteIntegration, + HttpRouteIntegration, PayloadFormatVersion, HttpMethod, IVpcLink, @@ -37,7 +37,7 @@ export interface VpcLinkConfigurationOptions { * * @internal */ -export abstract class HttpPrivateIntegration implements IHttpRouteIntegration { +export abstract class HttpPrivateIntegration extends HttpRouteIntegration { protected httpMethod = HttpMethod.ANY; protected payloadFormatVersion = PayloadFormatVersion.VERSION_1_0; // 1.0 is required and is the only supported format protected integrationType = HttpIntegrationType.HTTP_PROXY; diff --git a/packages/@aws-cdk/aws-apigatewayv2-integrations/lib/websocket/lambda.ts b/packages/@aws-cdk/aws-apigatewayv2-integrations/lib/websocket/lambda.ts index 8dedb470ce6af..8e41b30d65295 100644 --- a/packages/@aws-cdk/aws-apigatewayv2-integrations/lib/websocket/lambda.ts +++ b/packages/@aws-cdk/aws-apigatewayv2-integrations/lib/websocket/lambda.ts @@ -1,5 +1,5 @@ import { - IWebSocketRouteIntegration, + WebSocketRouteIntegration, WebSocketIntegrationType, WebSocketRouteIntegrationBindOptions, WebSocketRouteIntegrationConfig, @@ -21,8 +21,10 @@ export interface LambdaWebSocketIntegrationProps { /** * Lambda WebSocket Integration */ -export class LambdaWebSocketIntegration implements IWebSocketRouteIntegration { - constructor(private props: LambdaWebSocketIntegrationProps) {} +export class LambdaWebSocketIntegration extends WebSocketRouteIntegration { + constructor(private props: LambdaWebSocketIntegrationProps) { + super(); + } bind(options: WebSocketRouteIntegrationBindOptions): WebSocketRouteIntegrationConfig { const route = options.route; diff --git a/packages/@aws-cdk/aws-apigatewayv2/lib/common/base.ts b/packages/@aws-cdk/aws-apigatewayv2/lib/common/base.ts index 26d9ca64e391a..26342c75b5a0f 100644 --- a/packages/@aws-cdk/aws-apigatewayv2/lib/common/base.ts +++ b/packages/@aws-cdk/aws-apigatewayv2/lib/common/base.ts @@ -1,6 +1,5 @@ import * as cloudwatch from '@aws-cdk/aws-cloudwatch'; import { Resource } from '@aws-cdk/core'; -import { IntegrationCache } from '../private/integration-cache'; import { IApi } from './api'; import { ApiMapping } from './api-mapping'; import { DomainMappingOptions, IStage } from './stage'; @@ -12,10 +11,6 @@ import { DomainMappingOptions, IStage } from './stage'; export abstract class ApiBase extends Resource implements IApi { abstract readonly apiId: string; abstract readonly apiEndpoint: string; - /** - * @internal - */ - protected _integrationCache: IntegrationCache = new IntegrationCache(); public metric(metricName: string, props?: cloudwatch.MetricOptions): cloudwatch.Metric { return new cloudwatch.Metric({ diff --git a/packages/@aws-cdk/aws-apigatewayv2/lib/http/api.ts b/packages/@aws-cdk/aws-apigatewayv2/lib/http/api.ts index 32dfe0a0c2120..bf24c31dbaeca 100644 --- a/packages/@aws-cdk/aws-apigatewayv2/lib/http/api.ts +++ b/packages/@aws-cdk/aws-apigatewayv2/lib/http/api.ts @@ -6,7 +6,7 @@ import { IApi } from '../common/api'; import { ApiBase } from '../common/base'; import { DomainMappingOptions } from '../common/stage'; import { IHttpRouteAuthorizer } from './authorizer'; -import { IHttpRouteIntegration, HttpIntegration, HttpRouteIntegrationConfig } from './integration'; +import { HttpRouteIntegration } from './integration'; import { BatchHttpRouteOptions, HttpMethod, HttpRoute, HttpRouteKey } from './route'; import { IHttpStage, HttpStage, HttpStageOptions } from './stage'; import { VpcLink, VpcLinkProps } from './vpc-link'; @@ -71,12 +71,6 @@ export interface IHttpApi extends IApi { * Add a new VpcLink */ addVpcLink(options: VpcLinkProps): VpcLink - - /** - * Add a http integration - * @internal - */ - _addIntegration(scope: Construct, config: HttpRouteIntegrationConfig): HttpIntegration; } /** @@ -99,7 +93,7 @@ export interface HttpApiProps { * An integration that will be configured on the catch-all route ($default). * @default - none */ - readonly defaultIntegration?: IHttpRouteIntegration; + readonly defaultIntegration?: HttpRouteIntegration; /** * Whether a default stage and deployment should be automatically created. @@ -286,31 +280,6 @@ abstract class HttpApiBase extends ApiBase implements IHttpApi { // note that th return vpcLink; } - - /** - * @internal - */ - public _addIntegration(scope: Construct, config: HttpRouteIntegrationConfig): HttpIntegration { - const { configHash, integration: existingIntegration } = this._integrationCache.getIntegration(scope, config); - if (existingIntegration) { - return existingIntegration as HttpIntegration; - } - - const integration = new HttpIntegration(scope, `HttpIntegration-${configHash}`, { - httpApi: this, - integrationType: config.type, - integrationUri: config.uri, - method: config.method, - connectionId: config.connectionId, - connectionType: config.connectionType, - payloadFormatVersion: config.payloadFormatVersion, - secureServerName: config.secureServerName, - parameterMapping: config.parameterMapping, - }); - this._integrationCache.saveIntegration(scope, config, integration); - - return integration; - } } /** diff --git a/packages/@aws-cdk/aws-apigatewayv2/lib/http/integration.ts b/packages/@aws-cdk/aws-apigatewayv2/lib/http/integration.ts index df0cf84c13da0..6865875af5c80 100644 --- a/packages/@aws-cdk/aws-apigatewayv2/lib/http/integration.ts +++ b/packages/@aws-cdk/aws-apigatewayv2/lib/http/integration.ts @@ -1,5 +1,6 @@ /* eslint-disable quotes */ -import { Resource } from '@aws-cdk/core'; +import * as crypto from 'crypto'; +import { Resource, Stack } from '@aws-cdk/core'; import { Construct } from 'constructs'; import { CfnIntegration } from '../apigatewayv2.generated'; import { IIntegration } from '../common'; @@ -191,11 +192,46 @@ export interface HttpRouteIntegrationBindOptions { /** * The interface that various route integration classes will inherit. */ -export interface IHttpRouteIntegration { +export abstract class HttpRouteIntegration { + private integration?: HttpIntegration; + + /** + * Internal method called when binding this integration to the route. + * @internal + */ + public _bindToRoute(options: HttpRouteIntegrationBindOptions): { readonly integrationId: string } { + if (this.integration && this.integration.httpApi.node.addr !== options.route.httpApi.node.addr) { + throw new Error('A single integration cannot be associated with multiple APIs.'); + } + + if (!this.integration) { + const config = this.bind(options); + + this.integration = new HttpIntegration(options.scope, `HttpIntegration-${hash(config)}`, { + httpApi: options.route.httpApi, + integrationType: config.type, + integrationUri: config.uri, + method: config.method, + connectionId: config.connectionId, + connectionType: config.connectionType, + payloadFormatVersion: config.payloadFormatVersion, + secureServerName: config.secureServerName, + parameterMapping: config.parameterMapping, + }); + + function hash(x: any) { + const stringifiedConfig = JSON.stringify(Stack.of(options.scope).resolve(x)); + const configHash = crypto.createHash('md5').update(stringifiedConfig).digest('hex'); + return configHash; + } + } + return { integrationId: this.integration.integrationId }; + } + /** * Bind this integration to the route. */ - bind(options: HttpRouteIntegrationBindOptions): HttpRouteIntegrationConfig; + public abstract bind(options: HttpRouteIntegrationBindOptions): HttpRouteIntegrationConfig; } /** diff --git a/packages/@aws-cdk/aws-apigatewayv2/lib/http/route.ts b/packages/@aws-cdk/aws-apigatewayv2/lib/http/route.ts index ecc54b7f6acd9..7894defce6077 100644 --- a/packages/@aws-cdk/aws-apigatewayv2/lib/http/route.ts +++ b/packages/@aws-cdk/aws-apigatewayv2/lib/http/route.ts @@ -4,7 +4,7 @@ import { CfnRoute, CfnRouteProps } from '../apigatewayv2.generated'; import { IRoute } from '../common'; import { IHttpApi } from './api'; import { IHttpRouteAuthorizer } from './authorizer'; -import { IHttpRouteIntegration } from './integration'; +import { HttpRouteIntegration } from './integration'; /** * Represents a Route for an HTTP API. @@ -88,7 +88,7 @@ export interface BatchHttpRouteOptions { /** * The integration to be configured on this route. */ - readonly integration: IHttpRouteIntegration; + readonly integration: HttpRouteIntegration; } /** @@ -149,13 +149,11 @@ export class HttpRoute extends Resource implements IHttpRoute { this.httpApi = props.httpApi; this.path = props.routeKey.path; - const config = props.integration.bind({ + const config = props.integration._bindToRoute({ route: this, scope: this, }); - const integration = props.httpApi._addIntegration(this, config); - const authBindResult = props.authorizer ? props.authorizer.bind({ route: this, scope: this.httpApi instanceof Construct ? this.httpApi : this, // scope under the API if it's not imported @@ -181,7 +179,7 @@ export class HttpRoute extends Resource implements IHttpRoute { const routeProps: CfnRouteProps = { apiId: props.httpApi.apiId, routeKey: props.routeKey.key, - target: `integrations/${integration.integrationId}`, + target: `integrations/${config.integrationId}`, authorizerId: authBindResult?.authorizerId, authorizationType: authBindResult?.authorizationType ?? 'NONE', // must be explicitly NONE (not undefined) for stack updates to work correctly authorizationScopes, diff --git a/packages/@aws-cdk/aws-apigatewayv2/lib/private/integration-cache.ts b/packages/@aws-cdk/aws-apigatewayv2/lib/private/integration-cache.ts deleted file mode 100644 index 2401d28e20d2d..0000000000000 --- a/packages/@aws-cdk/aws-apigatewayv2/lib/private/integration-cache.ts +++ /dev/null @@ -1,29 +0,0 @@ -import * as crypto from 'crypto'; -import { Stack } from '@aws-cdk/core'; -import { Construct } from 'constructs'; -import { IIntegration } from '../common/integration'; -import { HttpRouteIntegrationConfig } from '../http'; -import { WebSocketRouteIntegrationConfig } from '../websocket'; - -type IntegrationConfig = HttpRouteIntegrationConfig | WebSocketRouteIntegrationConfig; - -export class IntegrationCache { - private integrations: Record = {}; - - getIntegration(scope: Construct, config: IntegrationConfig) { - const configHash = this.integrationConfigHash(scope, config); - const integration = this.integrations[configHash]; - return { configHash, integration }; - } - - saveIntegration(scope: Construct, config: IntegrationConfig, integration: IIntegration) { - const configHash = this.integrationConfigHash(scope, config); - this.integrations[configHash] = integration; - } - - private integrationConfigHash(scope: Construct, config: IntegrationConfig): string { - const stringifiedConfig = JSON.stringify(Stack.of(scope).resolve(config)); - const configHash = crypto.createHash('md5').update(stringifiedConfig).digest('hex'); - return configHash; - } -} diff --git a/packages/@aws-cdk/aws-apigatewayv2/lib/websocket/api.ts b/packages/@aws-cdk/aws-apigatewayv2/lib/websocket/api.ts index fdcfbdbce6d30..d78d16842e295 100644 --- a/packages/@aws-cdk/aws-apigatewayv2/lib/websocket/api.ts +++ b/packages/@aws-cdk/aws-apigatewayv2/lib/websocket/api.ts @@ -4,18 +4,12 @@ import { Construct } from 'constructs'; import { CfnApi } from '../apigatewayv2.generated'; import { IApi } from '../common/api'; import { ApiBase } from '../common/base'; -import { WebSocketRouteIntegrationConfig, WebSocketIntegration } from './integration'; import { WebSocketRoute, WebSocketRouteOptions } from './route'; /** * Represents a WebSocket API */ export interface IWebSocketApi extends IApi { - /** - * Add a websocket integration - * @internal - */ - _addIntegration(scope: Construct, config: WebSocketRouteIntegrationConfig): WebSocketIntegration } /** @@ -100,25 +94,6 @@ export class WebSocketApi extends ApiBase implements IWebSocketApi { } } - /** - * @internal - */ - public _addIntegration(scope: Construct, config: WebSocketRouteIntegrationConfig): WebSocketIntegration { - const { configHash, integration: existingIntegration } = this._integrationCache.getIntegration(scope, config); - if (existingIntegration) { - return existingIntegration as WebSocketIntegration; - } - - const integration = new WebSocketIntegration(scope, `WebSocketIntegration-${configHash}`, { - webSocketApi: this, - integrationType: config.type, - integrationUri: config.uri, - }); - this._integrationCache.saveIntegration(scope, config, integration); - - return integration; - } - /** * Add a new route */ diff --git a/packages/@aws-cdk/aws-apigatewayv2/lib/websocket/integration.ts b/packages/@aws-cdk/aws-apigatewayv2/lib/websocket/integration.ts index e75bd00b63d95..3c59269b474f6 100644 --- a/packages/@aws-cdk/aws-apigatewayv2/lib/websocket/integration.ts +++ b/packages/@aws-cdk/aws-apigatewayv2/lib/websocket/integration.ts @@ -1,4 +1,5 @@ -import { Resource } from '@aws-cdk/core'; +import * as crypto from 'crypto'; +import { Resource, Stack } from '@aws-cdk/core'; import { Construct } from 'constructs'; import { CfnIntegration } from '../apigatewayv2.generated'; import { IIntegration } from '../common'; @@ -87,11 +88,41 @@ export interface WebSocketRouteIntegrationBindOptions { /** * The interface that various route integration classes will inherit. */ -export interface IWebSocketRouteIntegration { +export abstract class WebSocketRouteIntegration { + private integration?: WebSocketIntegration; + + /** + * Internal method called when binding this integration to the route. + * @internal + */ + public _bindToRoute(options: WebSocketRouteIntegrationBindOptions): { readonly integrationId: string } { + if (this.integration && this.integration.webSocketApi.node.addr !== options.route.webSocketApi.node.addr) { + throw new Error('A single integration cannot be associated with multiple APIs.'); + } + + if (!this.integration) { + const config = this.bind(options); + + this.integration = new WebSocketIntegration(options.scope, `WebSocketIntegration-${hash(config)}`, { + webSocketApi: options.route.webSocketApi, + integrationType: config.type, + integrationUri: config.uri, + }); + + function hash(x: any) { + const stringifiedConfig = JSON.stringify(Stack.of(options.scope).resolve(x)); + const configHash = crypto.createHash('md5').update(stringifiedConfig).digest('hex'); + return configHash; + } + } + + return { integrationId: this.integration.integrationId }; + } + /** * Bind this integration to the route. */ - bind(options: WebSocketRouteIntegrationBindOptions): WebSocketRouteIntegrationConfig; + public abstract bind(options: WebSocketRouteIntegrationBindOptions): WebSocketRouteIntegrationConfig; } /** diff --git a/packages/@aws-cdk/aws-apigatewayv2/lib/websocket/route.ts b/packages/@aws-cdk/aws-apigatewayv2/lib/websocket/route.ts index 0588889a603bc..38316c6449c13 100644 --- a/packages/@aws-cdk/aws-apigatewayv2/lib/websocket/route.ts +++ b/packages/@aws-cdk/aws-apigatewayv2/lib/websocket/route.ts @@ -3,7 +3,7 @@ import { Construct } from 'constructs'; import { CfnRoute } from '../apigatewayv2.generated'; import { IRoute } from '../common'; import { IWebSocketApi } from './api'; -import { IWebSocketRouteIntegration } from './integration'; +import { WebSocketRouteIntegration } from './integration'; /** * Represents a Route for an WebSocket API. @@ -28,7 +28,7 @@ export interface WebSocketRouteOptions { /** * The integration to be configured on this route. */ - readonly integration: IWebSocketRouteIntegration; + readonly integration: WebSocketRouteIntegration; } @@ -67,17 +67,15 @@ export class WebSocketRoute extends Resource implements IWebSocketRoute { this.webSocketApi = props.webSocketApi; this.routeKey = props.routeKey; - const config = props.integration.bind({ + const config = props.integration._bindToRoute({ route: this, scope: this, }); - const integration = props.webSocketApi._addIntegration(this, config); - const route = new CfnRoute(this, 'Resource', { apiId: props.webSocketApi.apiId, routeKey: props.routeKey, - target: `integrations/${integration.integrationId}`, + target: `integrations/${config.integrationId}`, }); this.routeId = route.ref; } diff --git a/packages/@aws-cdk/aws-apigatewayv2/test/http/api.test.ts b/packages/@aws-cdk/aws-apigatewayv2/test/http/api.test.ts index 200933eefbca9..658b139193612 100644 --- a/packages/@aws-cdk/aws-apigatewayv2/test/http/api.test.ts +++ b/packages/@aws-cdk/aws-apigatewayv2/test/http/api.test.ts @@ -6,7 +6,7 @@ import { Duration, Stack } from '@aws-cdk/core'; import { CorsHttpMethod, DomainName, HttpApi, HttpAuthorizer, HttpIntegrationType, HttpMethod, HttpRouteAuthorizerBindOptions, HttpRouteAuthorizerConfig, - HttpRouteIntegrationBindOptions, HttpRouteIntegrationConfig, IHttpRouteAuthorizer, IHttpRouteIntegration, HttpNoneAuthorizer, PayloadFormatVersion, + HttpRouteIntegrationBindOptions, HttpRouteIntegrationConfig, IHttpRouteAuthorizer, HttpRouteIntegration, HttpNoneAuthorizer, PayloadFormatVersion, } from '../../lib'; describe('HttpApi', () => { @@ -531,7 +531,7 @@ describe('HttpApi', () => { }); }); -class DummyRouteIntegration implements IHttpRouteIntegration { +class DummyRouteIntegration extends HttpRouteIntegration { public bind(_: HttpRouteIntegrationBindOptions): HttpRouteIntegrationConfig { return { payloadFormatVersion: PayloadFormatVersion.VERSION_2_0, diff --git a/packages/@aws-cdk/aws-apigatewayv2/test/http/route.test.ts b/packages/@aws-cdk/aws-apigatewayv2/test/http/route.test.ts index 75d744b6b5bcc..b5ddae7919a62 100644 --- a/packages/@aws-cdk/aws-apigatewayv2/test/http/route.test.ts +++ b/packages/@aws-cdk/aws-apigatewayv2/test/http/route.test.ts @@ -2,7 +2,7 @@ import { Template } from '@aws-cdk/assertions'; import { Stack, App } from '@aws-cdk/core'; import { HttpApi, HttpAuthorizer, HttpAuthorizerType, HttpConnectionType, HttpIntegrationType, HttpMethod, HttpRoute, - HttpRouteAuthorizerBindOptions, HttpRouteAuthorizerConfig, HttpRouteIntegrationConfig, HttpRouteKey, IHttpRouteAuthorizer, IHttpRouteIntegration, + HttpRouteAuthorizerBindOptions, HttpRouteAuthorizerConfig, HttpRouteIntegrationConfig, HttpRouteKey, IHttpRouteAuthorizer, HttpRouteIntegration, MappingValue, ParameterMapping, PayloadFormatVersion, @@ -81,42 +81,42 @@ describe('HttpRoute', () => { Template.fromStack(stack).resourceCountIs('AWS::ApiGatewayV2::Integration', 1); }); - test('integration can be used across HttpApis', () => { + test('integration cannot be used across HttpApis', () => { // GIVEN const integration = new DummyIntegration(); // WHEN - const stack1 = new Stack(); - const httpApi1 = new HttpApi(stack1, 'HttpApi1'); + const stack = new Stack(); + const httpApi1 = new HttpApi(stack, 'HttpApi1'); + const httpApi2 = new HttpApi(stack, 'HttpApi2'); - new HttpRoute(stack1, 'HttpRoute1', { + new HttpRoute(stack, 'HttpRoute1', { httpApi: httpApi1, integration, routeKey: HttpRouteKey.with('/books', HttpMethod.GET), }); - new HttpRoute(stack1, 'HttpRoute2', { - httpApi: httpApi1, - integration, - routeKey: HttpRouteKey.with('/books', HttpMethod.POST), - }); - - const stack2 = new Stack(); - const httpApi2 = new HttpApi(stack2, 'HttpApi2'); - new HttpRoute(stack2, 'HttpRoute1', { + expect(() => new HttpRoute(stack, 'HttpRoute2', { httpApi: httpApi2, integration, routeKey: HttpRouteKey.with('/books', HttpMethod.GET), + })).toThrow(/cannot be associated with multiple APIs/); + }); + + test('associating integrations in different APIs creates separate AWS::ApiGatewayV2::Integration', () => { + const stack = new Stack(); + + const api = new HttpApi(stack, 'HttpApi'); + api.addRoutes({ + path: '/books', + integration: new DummyIntegration(), }); - new HttpRoute(stack2, 'HttpRoute2', { - httpApi: httpApi2, - integration, - routeKey: HttpRouteKey.with('/books', HttpMethod.POST), + api.addRoutes({ + path: '/magazines', + integration: new DummyIntegration(), }); - // THEN - Template.fromStack(stack1).resourceCountIs('AWS::ApiGatewayV2::Integration', 1); - Template.fromStack(stack2).resourceCountIs('AWS::ApiGatewayV2::Integration', 1); + Template.fromStack(stack).hasResource('AWS::ApiGatewayV2::Integration', 2); }); test('route defined in a separate stack does not create cycles', () => { @@ -167,7 +167,7 @@ describe('HttpRoute', () => { const stack = new Stack(); const httpApi = new HttpApi(stack, 'HttpApi'); - class PrivateIntegration implements IHttpRouteIntegration { + class PrivateIntegration extends HttpRouteIntegration { public bind(): HttpRouteIntegrationConfig { return { method: HttpMethod.ANY, @@ -212,7 +212,7 @@ describe('HttpRoute', () => { const stack = new Stack(); const httpApi = new HttpApi(stack, 'HttpApi'); - class PrivateIntegration implements IHttpRouteIntegration { + class PrivateIntegration extends HttpRouteIntegration { public bind(): HttpRouteIntegrationConfig { return { method: HttpMethod.ANY, @@ -310,7 +310,7 @@ describe('HttpRoute', () => { }); }); -class DummyIntegration implements IHttpRouteIntegration { +class DummyIntegration extends HttpRouteIntegration { public bind(): HttpRouteIntegrationConfig { return { type: HttpIntegrationType.HTTP_PROXY, diff --git a/packages/@aws-cdk/aws-apigatewayv2/test/websocket/api.test.ts b/packages/@aws-cdk/aws-apigatewayv2/test/websocket/api.test.ts index 24337a3f7c3f2..0a5e7c05b706d 100644 --- a/packages/@aws-cdk/aws-apigatewayv2/test/websocket/api.test.ts +++ b/packages/@aws-cdk/aws-apigatewayv2/test/websocket/api.test.ts @@ -2,7 +2,7 @@ import { Match, Template } from '@aws-cdk/assertions'; import { User } from '@aws-cdk/aws-iam'; import { Stack } from '@aws-cdk/core'; import { - IWebSocketRouteIntegration, WebSocketApi, WebSocketIntegrationType, + WebSocketRouteIntegration, WebSocketApi, WebSocketIntegrationType, WebSocketRouteIntegrationBindOptions, WebSocketRouteIntegrationConfig, } from '../../lib'; @@ -126,7 +126,7 @@ describe('WebSocketApi', () => { }); }); -class DummyIntegration implements IWebSocketRouteIntegration { +class DummyIntegration extends WebSocketRouteIntegration { bind(_options: WebSocketRouteIntegrationBindOptions): WebSocketRouteIntegrationConfig { return { type: WebSocketIntegrationType.AWS_PROXY, diff --git a/packages/@aws-cdk/aws-apigatewayv2/test/websocket/route.test.ts b/packages/@aws-cdk/aws-apigatewayv2/test/websocket/route.test.ts index 07eadc5300a85..512006fb8a2a4 100644 --- a/packages/@aws-cdk/aws-apigatewayv2/test/websocket/route.test.ts +++ b/packages/@aws-cdk/aws-apigatewayv2/test/websocket/route.test.ts @@ -1,7 +1,7 @@ import { Template } from '@aws-cdk/assertions'; import { Stack } from '@aws-cdk/core'; import { - IWebSocketRouteIntegration, WebSocketApi, WebSocketIntegrationType, + WebSocketRouteIntegration, WebSocketApi, WebSocketIntegrationType, WebSocketRoute, WebSocketRouteIntegrationBindOptions, WebSocketRouteIntegrationConfig, } from '../../lib'; @@ -41,10 +41,50 @@ describe('WebSocketRoute', () => { IntegrationUri: 'some-uri', }); }); + + test('integration cannot be used across WebSocketApis', () => { + // GIVEN + const integration = new DummyIntegration(); + + // WHEN + const stack = new Stack(); + const webSocketApi1 = new WebSocketApi(stack, 'WebSocketApi1'); + const webSocketApi2 = new WebSocketApi(stack, 'WebSocketApi2'); + + new WebSocketRoute(stack, 'WebSocketRoute1', { + webSocketApi: webSocketApi1, + integration, + routeKey: 'route', + }); + + expect(() => new WebSocketRoute(stack, 'WebSocketRoute2', { + webSocketApi: webSocketApi2, + integration, + routeKey: 'route', + })).toThrow(/cannot be associated with multiple APIs/); + }); + + test('associating integrations in different APIs creates separate AWS::ApiGatewayV2::Integration', () => { + const stack = new Stack(); + + const api = new WebSocketApi(stack, 'WebSocketApi'); + new WebSocketRoute(stack, 'WebSocketRoute1', { + webSocketApi: api, + integration: new DummyIntegration(), + routeKey: '/books', + }); + new WebSocketRoute(stack, 'WebSocketRoute2', { + webSocketApi: api, + integration: new DummyIntegration(), + routeKey: '/magazines', + }); + + Template.fromStack(stack).hasResource('AWS::ApiGatewayV2::Integration', 2); + }); }); -class DummyIntegration implements IWebSocketRouteIntegration { +class DummyIntegration extends WebSocketRouteIntegration { bind(_options: WebSocketRouteIntegrationBindOptions): WebSocketRouteIntegrationConfig { return { type: WebSocketIntegrationType.AWS_PROXY,