Skip to content

Commit

Permalink
fix(apigateway): error defining lambda integration on imported RestApi (
Browse files Browse the repository at this point in the history
#8785)

Root cause here was that `LambdaIntegration` used the deprecated
`restApi` property on its binding Method. This property does not work
on imported RestApi and has been superceded by the property `api`.

A further bug that was discovered was that the API `methodArn()` on the
Method construct did not work on imported RestApi. This has been fixed
up such that if a `deploymentStage` is not defined for the imported
RestApi, the ARN will return '*' as its stage (shorthand for 'all
stages').

fixes #8679


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
Niranjan Jayakar committed Jun 29, 2020
1 parent af69f80 commit 05aaf42
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 25 deletions.
Expand Up @@ -56,7 +56,7 @@ export class LambdaIntegration extends AwsIntegration {
super.bind(method);
const principal = new iam.ServicePrincipal('apigateway.amazonaws.com');

const desc = `${method.restApi.node.uniqueId}.${method.httpMethod}.${method.resource.path.replace(/\//g, '.')}`;
const desc = `${method.api.node.uniqueId}.${method.httpMethod}.${method.resource.path.replace(/\//g, '.')}`;

this.handler.addPermission(`ApiPermission.${desc}`, {
principal,
Expand Down
15 changes: 5 additions & 10 deletions packages/@aws-cdk/aws-apigateway/lib/method.ts
Expand Up @@ -237,27 +237,22 @@ export class Method extends Resource {
* arn:aws:execute-api:{region}:{account}:{restApiId}/{stage}/{method}/{path}
*
* NOTE: {stage} will refer to the `restApi.deploymentStage`, which will
* automatically set if auto-deploy is enabled.
* automatically set if auto-deploy is enabled, or can be explicitly assigned.
* When not configured, {stage} will be set to '*', as a shorthand for 'all stages'.
*
* @attribute
*/
public get methodArn(): string {
if (!this.restApi.deploymentStage) {
throw new Error(
`Unable to determine ARN for method "${this.node.id}" since there is no stage associated with this API.\n` +
'Either use the `deploy` prop or explicitly assign `deploymentStage` on the RestApi');
}

const stage = this.restApi.deploymentStage.stageName.toString();
return this.restApi.arnForExecuteApi(this.httpMethod, pathForArn(this.resource.path), stage);
const stage = this.api.deploymentStage?.stageName;
return this.api.arnForExecuteApi(this.httpMethod, pathForArn(this.resource.path), stage);
}

/**
* Returns an execute-api ARN for this method's "test-invoke-stage" stage.
* This stage is used by the AWS Console UI when testing the method.
*/
public get testMethodArn(): string {
return this.restApi.arnForExecuteApi(this.httpMethod, pathForArn(this.resource.path), 'test-invoke-stage');
return this.api.arnForExecuteApi(this.httpMethod, pathForArn(this.resource.path), 'test-invoke-stage');
}

private renderIntegration(integration?: Integration): CfnMethod.IntegrationProperty {
Expand Down
27 changes: 17 additions & 10 deletions packages/@aws-cdk/aws-apigateway/lib/restapi.ts
Expand Up @@ -38,6 +38,11 @@ export interface IRestApi extends IResourceBase {
*/
readonly latestDeployment?: Deployment;

/**
* API Gateway stage that points to the latest deployment (if defined).
*/
deploymentStage: Stage;

/**
* Represents the root resource ("/") of this API. Use it to define the API model:
*
Expand All @@ -46,6 +51,17 @@ export interface IRestApi extends IResourceBase {
*
*/
readonly root: IResource;

/**
* Gets the "execute-api" ARN
* @returns The "execute-api" ARN.
* @default "*" returns the execute API ARN for all methods/resources in
* this API.
* @param method The method (default `*`)
* @param path The resource path. Must start with '/' (default `*`)
* @param stage The stage (default `*`)
*/
arnForExecuteApi(method?: string, path?: string, stage?: string): string;
}

/**
Expand Down Expand Up @@ -324,15 +340,6 @@ export abstract class RestApiBase extends Resource implements IRestApi {
return new UsagePlan(this, id, props);
}

/**
* Gets the "execute-api" ARN
* @returns The "execute-api" ARN.
* @default "*" returns the execute API ARN for all methods/resources in
* this API.
* @param method The method (default `*`)
* @param path The resource path. Must start with '/' (default `*`)
* @param stage The stage (default `*`)
*/
public arnForExecuteApi(method: string = '*', path: string = '/*', stage: string = '*') {
if (!path.startsWith('/')) {
throw new Error(`"path" must begin with a "/": '${path}'`);
Expand Down Expand Up @@ -496,7 +503,7 @@ export class RestApi extends RestApiBase {
* Import an existing RestApi.
*/
public static fromRestApiId(scope: Construct, id: string, restApiId: string): IRestApi {
class Import extends Resource implements IRestApi {
class Import extends RestApiBase {
public readonly restApiId = restApiId;

public get root(): IResource {
Expand Down
24 changes: 24 additions & 0 deletions packages/@aws-cdk/aws-apigateway/test/test.lambda.ts
Expand Up @@ -191,4 +191,28 @@ export = {

test.done();
},

'works for imported RestApi'(test: Test) {
const stack = new cdk.Stack();
const api = apigateway.RestApi.fromRestApiAttributes(stack, 'RestApi', {
restApiId: 'imported-rest-api-id',
rootResourceId: 'imported-root-resource-id',
});

const handler = new lambda.Function(stack, 'MyFunc', {
runtime: lambda.Runtime.NODEJS_10_X,
handler: 'index.handler',
code: lambda.Code.fromInline('loo'),
});

api.root.addMethod('ANY', new apigateway.LambdaIntegration(handler));

expect(stack).to(haveResource('AWS::ApiGateway::Method', {
RestApiId: 'imported-rest-api-id',
ResourceId: 'imported-root-resource-id',
HttpMethod: 'ANY',
}));

test.done();
},
};
24 changes: 20 additions & 4 deletions packages/@aws-cdk/aws-apigateway/test/test.method.ts
Expand Up @@ -207,15 +207,31 @@ export = {
test.done();
},

'"methodArn" fails if the API does not have a deployment stage'(test: Test) {
'"methodArn" returns an arn with "*" as its stage when deploymentStage is not set'(test: Test) {
// GIVEN
const stack = new cdk.Stack();
const api = new apigw.RestApi(stack, 'test-api', { deploy: false });

// WHEN
const method = new apigw.Method(stack, 'my-method', { httpMethod: 'POST', resource: api.root });

// WHEN + THEN
test.throws(() => method.methodArn,
/Unable to determine ARN for method "my-method" since there is no stage associated with this API./);
// THEN
test.deepEqual(stack.resolve(method.methodArn), {
'Fn::Join': [
'',
[
'arn:',
{ Ref: 'AWS::Partition' },
':execute-api:',
{ Ref: 'AWS::Region' },
':',
{ Ref: 'AWS::AccountId' },
':',
{ Ref: 'testapiD6451F70' },
'/*/POST/',
],
],
});

test.done();
},
Expand Down

0 comments on commit 05aaf42

Please sign in to comment.