From 92cd7a94f2a836423015960591fbb05df57468be Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Thu, 18 Oct 2018 16:39:31 +0200 Subject: [PATCH 1/4] fix(aws-apigateway): change 'proxyPath' to 'proxyAll' The LambdaRestApi construct now takes a 'proxyAll' argument to enable forwarding all requests to the given Lambda Function. BREAKING CHANGE: specifying a path no longer works. If you used to provide a '/', now supply true. Otherwise, you will have to construct more complex resource paths yourself. Fixes #959. --- .../@aws-cdk/aws-apigateway/lib/lambda-api.ts | 41 ++++++++++++------- .../aws-apigateway/test/test.lambda-api.ts | 37 ++++------------- 2 files changed, 34 insertions(+), 44 deletions(-) diff --git a/packages/@aws-cdk/aws-apigateway/lib/lambda-api.ts b/packages/@aws-cdk/aws-apigateway/lib/lambda-api.ts index 1b08b2533b165..57cedd75d0896 100644 --- a/packages/@aws-cdk/aws-apigateway/lib/lambda-api.ts +++ b/packages/@aws-cdk/aws-apigateway/lib/lambda-api.ts @@ -1,6 +1,8 @@ import lambda = require('@aws-cdk/aws-lambda'); import cdk = require('@aws-cdk/cdk'); import { LambdaIntegration } from './integrations'; +import { Method } from './method'; +import { Resource } from './resource'; import { RestApi, RestApiProps } from './restapi'; export interface LambdaRestApiProps { @@ -13,15 +15,14 @@ export interface LambdaRestApiProps { handler: lambda.Function; /** - * An API path for a greedy proxy with an "ANY" method, which will route all - * requests under that path to the defined handler. + * If true, route all requests to the Lambda Function * - * If not defined, you will need to explicitly define the API model using + * If not set to true, you will need to explicitly define the API model using * `addResource` and `addMethod` (or `addProxy`). * - * @default undefined + * @default false */ - proxyPath?: string; + proxyAll?: boolean; /** * Further customization of the REST API. @@ -49,16 +50,26 @@ export class LambdaRestApi extends RestApi { ...props.options }); - // if proxyPath is specified, add a proxy at the specified path - // we will need to create all resources along the path. - const proxyPath = props.proxyPath; - if (proxyPath) { - const route = proxyPath.split('/').filter(x => x); - let curr = this.root; - for (const part of route) { - curr = curr.addResource(part); - } - curr.addProxy(); + if (props.proxyAll) { + this.root.addProxy(); + this.root.addMethod('ANY'); + + // Make sure users cannot call any other resource adding function + this.root.addResource = addResourceThrows; + this.root.addMethod = addMethodThrows; + this.root.addProxy = addProxyThrows; } } +} + +function addResourceThrows(): Resource { + throw new Error(`Cannot call 'addResource' on a proyxing LambdaRestApi; set 'proxyAll' to false`); +} + +function addMethodThrows(): Method { + throw new Error(`Cannot call 'addMethod' on a proyxing LambdaRestApi; set 'proxyAll' to false`); +} + +function addProxyThrows(): Resource { + throw new Error(`Cannot call 'addProxy' on a proyxing LambdaRestApi; set 'proxyAll' to false`); } \ No newline at end of file diff --git a/packages/@aws-cdk/aws-apigateway/test/test.lambda-api.ts b/packages/@aws-cdk/aws-apigateway/test/test.lambda-api.ts index 2c9ebfb2f21a8..b6f158e2c102f 100644 --- a/packages/@aws-cdk/aws-apigateway/test/test.lambda-api.ts +++ b/packages/@aws-cdk/aws-apigateway/test/test.lambda-api.ts @@ -18,9 +18,14 @@ export = { }); // WHEN - new apigw.LambdaRestApi(stack, 'lambda-rest-api', { handler, proxyPath: '/' }); + const api = new apigw.LambdaRestApi(stack, 'lambda-rest-api', { handler, proxyAll: true }); - // THEN + // THEN -- can't customize further + test.throws(() => { + api.root.addResource('cant-touch-this'); + }); + + // THEN -- template proxies everything expect(stack).to(haveResource('AWS::ApiGateway::Resource', { "PathPart": "{proxy+}" })); @@ -81,32 +86,6 @@ export = { test.done(); }, - 'proxyPath can be used to attach the proxy to any route'(test: Test) { - // GIVEN - const stack = new cdk.Stack(); - - const handler = new lambda.Function(stack, 'handler', { - handler: 'index.handler', - code: lambda.Code.inline('boom'), - runtime: lambda.Runtime.NodeJS610, - }); - - // WHEN - new apigw.LambdaRestApi(stack, 'lambda-rest-api', { - handler, - proxyPath: '/backend/v2' - }); - - // THEN - expect(stack).to(haveResource('AWS::ApiGateway::Method', { - "ResourceId": { - "Ref": "lambdarestapibackendv2proxyC4980BD5" - } - })); - - test.done(); - }, - 'when "proxyPath" is not specified, users need to define the model'(test: Test) { // GIVEN const stack = new cdk.Stack(); @@ -162,5 +141,5 @@ export = { }), /Cannot specify \"options\.defaultIntegration\" since Lambda integration is automatically defined/); test.done(); - } + }, }; \ No newline at end of file From 456654493116d5915b1abe2ba88ad88b88fc8631 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Fri, 19 Oct 2018 10:16:22 +0200 Subject: [PATCH 2/4] Make proxy default to true, implicitly add ANY to root --- .../@aws-cdk/aws-apigateway/lib/lambda-api.ts | 15 +++++++-------- packages/@aws-cdk/aws-apigateway/lib/restapi.ts | 2 ++ .../aws-apigateway/test/test.lambda-api.ts | 6 +++--- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/packages/@aws-cdk/aws-apigateway/lib/lambda-api.ts b/packages/@aws-cdk/aws-apigateway/lib/lambda-api.ts index 57cedd75d0896..b0e5c749a351b 100644 --- a/packages/@aws-cdk/aws-apigateway/lib/lambda-api.ts +++ b/packages/@aws-cdk/aws-apigateway/lib/lambda-api.ts @@ -17,12 +17,12 @@ export interface LambdaRestApiProps { /** * If true, route all requests to the Lambda Function * - * If not set to true, you will need to explicitly define the API model using + * If set to false, you will need to explicitly define the API model using * `addResource` and `addMethod` (or `addProxy`). * - * @default false + * @default true */ - proxyAll?: boolean; + proxy?: boolean; /** * Further customization of the REST API. @@ -50,9 +50,8 @@ export class LambdaRestApi extends RestApi { ...props.options }); - if (props.proxyAll) { + if (props.proxy !== false) { this.root.addProxy(); - this.root.addMethod('ANY'); // Make sure users cannot call any other resource adding function this.root.addResource = addResourceThrows; @@ -63,13 +62,13 @@ export class LambdaRestApi extends RestApi { } function addResourceThrows(): Resource { - throw new Error(`Cannot call 'addResource' on a proyxing LambdaRestApi; set 'proxyAll' to false`); + throw new Error(`Cannot call 'addResource' on a proxying LambdaRestApi; set 'proxy' to false`); } function addMethodThrows(): Method { - throw new Error(`Cannot call 'addMethod' on a proyxing LambdaRestApi; set 'proxyAll' to false`); + throw new Error(`Cannot call 'addMethod' on a proxying LambdaRestApi; set 'proxy' to false`); } function addProxyThrows(): Resource { - throw new Error(`Cannot call 'addProxy' on a proyxing LambdaRestApi; set 'proxyAll' to false`); + throw new Error(`Cannot call 'addProxy' on a proxying LambdaRestApi; set 'proxy' to false`); } \ No newline at end of file diff --git a/packages/@aws-cdk/aws-apigateway/lib/restapi.ts b/packages/@aws-cdk/aws-apigateway/lib/restapi.ts index e9bd84f2422c9..f01fb4ebb71b7 100644 --- a/packages/@aws-cdk/aws-apigateway/lib/restapi.ts +++ b/packages/@aws-cdk/aws-apigateway/lib/restapi.ts @@ -214,6 +214,8 @@ export class RestApi extends RestApiRef implements cdk.IDependable { return new Method(this, httpMethod, { resource: this.root, httpMethod, integration, options }); }, addProxy: (options?: ResourceOptions) => { + // Must have ANY method on the root when doing this + this.root.addMethod('ANY'); return new ProxyResource(this, '{proxy+}', { parent: this.root, ...options }); }, defaultIntegration: props.defaultIntegration, diff --git a/packages/@aws-cdk/aws-apigateway/test/test.lambda-api.ts b/packages/@aws-cdk/aws-apigateway/test/test.lambda-api.ts index b6f158e2c102f..45f1ec60e1479 100644 --- a/packages/@aws-cdk/aws-apigateway/test/test.lambda-api.ts +++ b/packages/@aws-cdk/aws-apigateway/test/test.lambda-api.ts @@ -18,7 +18,7 @@ export = { }); // WHEN - const api = new apigw.LambdaRestApi(stack, 'lambda-rest-api', { handler, proxyAll: true }); + const api = new apigw.LambdaRestApi(stack, 'lambda-rest-api', { handler }); // THEN -- can't customize further test.throws(() => { @@ -86,7 +86,7 @@ export = { test.done(); }, - 'when "proxyPath" is not specified, users need to define the model'(test: Test) { + 'when "proxy" is set to false, users need to define the model'(test: Test) { // GIVEN const stack = new cdk.Stack(); @@ -97,7 +97,7 @@ export = { }); // WHEN - const api = new apigw.LambdaRestApi(stack, 'lambda-rest-api', { handler }); + const api = new apigw.LambdaRestApi(stack, 'lambda-rest-api', { handler, proxy: false }); const tasks = api.root.addResource('tasks'); tasks.addMethod('GET'); From 751cf282367f16701a1f51e626ef3292779b3248 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Fri, 19 Oct 2018 10:26:30 +0200 Subject: [PATCH 3/4] Update README, move addMethod() logic into the proxy class --- packages/@aws-cdk/aws-apigateway/README.md | 12 ++++++------ packages/@aws-cdk/aws-apigateway/lib/resource.ts | 5 +++++ packages/@aws-cdk/aws-apigateway/lib/restapi.ts | 2 -- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/packages/@aws-cdk/aws-apigateway/README.md b/packages/@aws-cdk/aws-apigateway/README.md index 771dfe0d6cce8..c7e5b244ec8a2 100644 --- a/packages/@aws-cdk/aws-apigateway/README.md +++ b/packages/@aws-cdk/aws-apigateway/README.md @@ -34,24 +34,24 @@ book.addMethod('DELETE'); A very common practice is to use Amazon API Gateway with AWS Lambda as the backend integration. The `LambdaRestApi` construct makes it easy: -The following code defines a REST API that uses a greedy `{proxy+}` resource -mounted under `/api/v1` and integrates all methods (`"ANY"`) with the specified -AWS Lambda function: +The following code defines a REST API that routes all requests to the +specified AWS Lambda function: ```ts const backend = new lambda.Function(...); new apigateway.LambdaRestApi(this, 'myapi', { handler: backend, - proxyPath: '/api/v1' }); ``` -If `proxyPath` is not defined, you will have to explicitly define the API model: +You can also supply `proxy: false`, in which case you will have to explicitly +define the API model: ```ts const backend = new lambda.Function(...); const api = new apigateway.LambdaRestApi(this, 'myapi', { - handler: backend + handler: backend, + proxy: false }); const items = api.root.addResource('items'); diff --git a/packages/@aws-cdk/aws-apigateway/lib/resource.ts b/packages/@aws-cdk/aws-apigateway/lib/resource.ts index ca469e7f5bb04..ffec678958b69 100644 --- a/packages/@aws-cdk/aws-apigateway/lib/resource.ts +++ b/packages/@aws-cdk/aws-apigateway/lib/resource.ts @@ -179,6 +179,11 @@ export class ProxyResource extends Resource { defaultMethodOptions: props.defaultMethodOptions, }); + // Add ANY method to parent but only if it's the root + if (props.parent.resourcePath === '/') { + props.parent.addMethod('ANY'); + } + const anyMethod = props.anyMethod !== undefined ? props.anyMethod : true; if (anyMethod) { this.anyMethod = this.addMethod('ANY'); diff --git a/packages/@aws-cdk/aws-apigateway/lib/restapi.ts b/packages/@aws-cdk/aws-apigateway/lib/restapi.ts index f01fb4ebb71b7..e9bd84f2422c9 100644 --- a/packages/@aws-cdk/aws-apigateway/lib/restapi.ts +++ b/packages/@aws-cdk/aws-apigateway/lib/restapi.ts @@ -214,8 +214,6 @@ export class RestApi extends RestApiRef implements cdk.IDependable { return new Method(this, httpMethod, { resource: this.root, httpMethod, integration, options }); }, addProxy: (options?: ResourceOptions) => { - // Must have ANY method on the root when doing this - this.root.addMethod('ANY'); return new ProxyResource(this, '{proxy+}', { parent: this.root, ...options }); }, defaultIntegration: props.defaultIntegration, From dd839da7f9d1a4c400e3a8b2026346e0c69e44d2 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Fri, 19 Oct 2018 10:42:23 +0200 Subject: [PATCH 4/4] Move parent-proxying behavior to addMethod() --- .../@aws-cdk/aws-apigateway/lib/lambda-api.ts | 4 ++-- packages/@aws-cdk/aws-apigateway/lib/resource.ts | 16 ++++++++++++---- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/packages/@aws-cdk/aws-apigateway/lib/lambda-api.ts b/packages/@aws-cdk/aws-apigateway/lib/lambda-api.ts index b0e5c749a351b..bf2488a3f2531 100644 --- a/packages/@aws-cdk/aws-apigateway/lib/lambda-api.ts +++ b/packages/@aws-cdk/aws-apigateway/lib/lambda-api.ts @@ -2,7 +2,7 @@ import lambda = require('@aws-cdk/aws-lambda'); import cdk = require('@aws-cdk/cdk'); import { LambdaIntegration } from './integrations'; import { Method } from './method'; -import { Resource } from './resource'; +import { ProxyResource, Resource } from './resource'; import { RestApi, RestApiProps } from './restapi'; export interface LambdaRestApiProps { @@ -69,6 +69,6 @@ function addMethodThrows(): Method { throw new Error(`Cannot call 'addMethod' on a proxying LambdaRestApi; set 'proxy' to false`); } -function addProxyThrows(): Resource { +function addProxyThrows(): ProxyResource { throw new Error(`Cannot call 'addProxy' on a proxying LambdaRestApi; set 'proxy' to false`); } \ No newline at end of file diff --git a/packages/@aws-cdk/aws-apigateway/lib/resource.ts b/packages/@aws-cdk/aws-apigateway/lib/resource.ts index ffec678958b69..c4c1d62724e2e 100644 --- a/packages/@aws-cdk/aws-apigateway/lib/resource.ts +++ b/packages/@aws-cdk/aws-apigateway/lib/resource.ts @@ -171,6 +171,8 @@ export class ProxyResource extends Resource { */ public readonly anyMethod?: Method; + private readonly parentResource: IRestApiResource; + constructor(parent: cdk.Construct, id: string, props: ProxyResourceProps) { super(parent, id, { parent: props.parent, @@ -179,16 +181,22 @@ export class ProxyResource extends Resource { defaultMethodOptions: props.defaultMethodOptions, }); - // Add ANY method to parent but only if it's the root - if (props.parent.resourcePath === '/') { - props.parent.addMethod('ANY'); - } + this.parentResource = props.parent; const anyMethod = props.anyMethod !== undefined ? props.anyMethod : true; if (anyMethod) { this.anyMethod = this.addMethod('ANY'); } } + + public addMethod(httpMethod: string, integration?: Integration, options?: MethodOptions): Method { + // In case this proxy is mounted under the root, also add this method to + // the root so that empty paths are proxied as well. + if (this.parentResource.resourcePath === '/') { + this.parentResource.addMethod(httpMethod); + } + return super.addMethod(httpMethod, integration, options); + } } function validateResourcePathPart(part: string) {