From 604b344ca34130c8c2a74d821adb02352be37ec3 Mon Sep 17 00:00:00 2001 From: Niranjan Jayakar Date: Tue, 30 Jun 2020 13:08:07 +0100 Subject: [PATCH] fix(apigateway): permission error in lambda integration when function name is modified Changing function name triggers a resource replacement - the old function is removed and replaced with a new function. However, the RestApi deployment remains untouched and is still pointing at the ARN of the, now stale, function. The fix is to trigger a new deployment if the function name changes. fixes #5306 --- .../aws-apigateway/lib/integration.ts | 16 ++++++++++-- .../aws-apigateway/lib/integrations/aws.ts | 5 ++-- .../aws-apigateway/lib/integrations/lambda.ts | 15 ++++++++--- .../@aws-cdk/aws-apigateway/lib/method.ts | 26 ++++++++----------- .../integ.restapi.multistack.expected.json | 4 +-- 5 files changed, 42 insertions(+), 24 deletions(-) diff --git a/packages/@aws-cdk/aws-apigateway/lib/integration.ts b/packages/@aws-cdk/aws-apigateway/lib/integration.ts index d7a9ec74f3b34..a2afbb8a41331 100644 --- a/packages/@aws-cdk/aws-apigateway/lib/integration.ts +++ b/packages/@aws-cdk/aws-apigateway/lib/integration.ts @@ -134,6 +134,18 @@ export interface IntegrationProps { readonly options?: IntegrationOptions; } +/** + * Result of binding an Integration to the Method + */ +export interface IntegrationConfig { + /** + * Triggers a new Deployment if the value of this property changes. + * Use this property to trigger new deployments for changes to values in this integration. + * @default - deployments are not triggered for any change of this integration. + */ + readonly triggerDeployment?: any; +} + /** * Base class for backend integrations for an API Gateway method. * @@ -156,8 +168,8 @@ export class Integration { * Can be overridden by subclasses to allow the integration to interact with the method * being integrated, access the REST API object, method ARNs, etc. */ - public bind(_method: Method) { - return; + public bind(_method: Method): IntegrationConfig { + return {}; } } diff --git a/packages/@aws-cdk/aws-apigateway/lib/integrations/aws.ts b/packages/@aws-cdk/aws-apigateway/lib/integrations/aws.ts index cd3854e2abd22..c2b7cc46d4cf1 100644 --- a/packages/@aws-cdk/aws-apigateway/lib/integrations/aws.ts +++ b/packages/@aws-cdk/aws-apigateway/lib/integrations/aws.ts @@ -1,5 +1,5 @@ import * as cdk from '@aws-cdk/core'; -import { Integration, IntegrationOptions, IntegrationType } from '../integration'; +import { Integration, IntegrationConfig, IntegrationOptions, IntegrationType } from '../integration'; import { Method } from '../method'; import { parseAwsApiCall } from '../util'; @@ -92,7 +92,8 @@ export class AwsIntegration extends Integration { }); } - public bind(method: Method) { + public bind(method: Method): IntegrationConfig { this.scope = method; + return {}; } } diff --git a/packages/@aws-cdk/aws-apigateway/lib/integrations/lambda.ts b/packages/@aws-cdk/aws-apigateway/lib/integrations/lambda.ts index fe5e81678e85e..98f372bef032c 100644 --- a/packages/@aws-cdk/aws-apigateway/lib/integrations/lambda.ts +++ b/packages/@aws-cdk/aws-apigateway/lib/integrations/lambda.ts @@ -1,7 +1,7 @@ import * as iam from '@aws-cdk/aws-iam'; import * as lambda from '@aws-cdk/aws-lambda'; -import { Lazy } from '@aws-cdk/core'; -import { IntegrationOptions } from '../integration'; +import { Lazy, Token } from '@aws-cdk/core'; +import { IntegrationConfig, IntegrationOptions } from '../integration'; import { Method } from '../method'; import { AwsIntegration } from './aws'; @@ -52,7 +52,7 @@ export class LambdaIntegration extends AwsIntegration { this.enableTest = options.allowTestInvoke === undefined ? true : false; } - public bind(method: Method) { + public bind(method: Method): IntegrationConfig { super.bind(method); const principal = new iam.ServicePrincipal('apigateway.amazonaws.com'); @@ -72,5 +72,14 @@ export class LambdaIntegration extends AwsIntegration { sourceArn: method.testMethodArn, }); } + + const cfnFunction = this.handler.node.defaultChild as lambda.CfnFunction; + let triggerDeployment; + if (!Token.isUnresolved(cfnFunction.functionName)) { + triggerDeployment = { functionName: cfnFunction.functionName }; + } + return { + triggerDeployment, + }; } } diff --git a/packages/@aws-cdk/aws-apigateway/lib/method.ts b/packages/@aws-cdk/aws-apigateway/lib/method.ts index 8721d9e9da114..b4077c0daac83 100644 --- a/packages/@aws-cdk/aws-apigateway/lib/method.ts +++ b/packages/@aws-cdk/aws-apigateway/lib/method.ts @@ -192,6 +192,9 @@ export class Method extends Resource { authorizer._attachToApi(this.api); } + const integration = props.integration ?? this.resource.defaultIntegration ?? new MockIntegration(); + const bindResult = integration.bind(this); + const methodProps: CfnMethodProps = { resourceId: props.resource.resourceId, restApiId: this.api.restApiId, @@ -201,7 +204,7 @@ export class Method extends Resource { authorizationType, authorizerId, requestParameters: options.requestParameters || defaultMethodOptions.requestParameters, - integration: this.renderIntegration(props.integration), + integration: this.renderIntegration(integration), methodResponses: this.renderMethodResponses(options.methodResponses), requestModels: this.renderRequestModels(options.requestModels), requestValidatorId: this.requestValidatorId(options), @@ -219,7 +222,12 @@ export class Method extends Resource { const deployment = props.resource.api.latestDeployment; if (deployment) { deployment.node.addDependency(resource); - deployment.addToLogicalId({ method: methodProps }); + deployment.addToLogicalId({ + method: { + ...methodProps, + ...bindResult?.triggerDeployment, + }, + }); } } @@ -255,19 +263,7 @@ export class Method extends Resource { return this.api.arnForExecuteApi(this.httpMethod, pathForArn(this.resource.path), 'test-invoke-stage'); } - private renderIntegration(integration?: Integration): CfnMethod.IntegrationProperty { - if (!integration) { - // use defaultIntegration from API if defined - if (this.resource.defaultIntegration) { - return this.renderIntegration(this.resource.defaultIntegration); - } - - // fallback to mock - return this.renderIntegration(new MockIntegration()); - } - - integration.bind(this); - + private renderIntegration(integration: Integration): CfnMethod.IntegrationProperty { const options = integration._props.options || { }; let credentials; diff --git a/packages/@aws-cdk/aws-apigateway/test/integ.restapi.multistack.expected.json b/packages/@aws-cdk/aws-apigateway/test/integ.restapi.multistack.expected.json index 3404f37880155..22c144974ed4d 100644 --- a/packages/@aws-cdk/aws-apigateway/test/integ.restapi.multistack.expected.json +++ b/packages/@aws-cdk/aws-apigateway/test/integ.restapi.multistack.expected.json @@ -120,7 +120,7 @@ "BooksApi60AC975F" ] }, - "BooksApiDeployment86CA39AF7e6c771d47a1a3777eba99bffc037822": { + "BooksApiDeployment86CA39AFdfee6753c946e6cbf8b3d649e0cf1377": { "Type": "AWS::ApiGateway::Deployment", "Properties": { "RestApiId": { @@ -141,7 +141,7 @@ "Ref": "BooksApi60AC975F" }, "DeploymentId": { - "Ref": "BooksApiDeployment86CA39AF7e6c771d47a1a3777eba99bffc037822" + "Ref": "BooksApiDeployment86CA39AFdfee6753c946e6cbf8b3d649e0cf1377" }, "StageName": "prod" }