Skip to content

Commit

Permalink
fix(apigateway): permission error in lambda integration when function…
Browse files Browse the repository at this point in the history
… name is modified (#8813)

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


----

*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 Jul 2, 2020
1 parent 66f9634 commit f1b37ef
Show file tree
Hide file tree
Showing 6 changed files with 87 additions and 23 deletions.
16 changes: 15 additions & 1 deletion packages/@aws-cdk/aws-apigateway/lib/integration.ts
Expand Up @@ -134,6 +134,20 @@ export interface IntegrationProps {
readonly options?: IntegrationOptions;
}

/**
* Result of binding an Integration to the Method
*/
export interface IntegrationConfig {
/**
* This value is included in computing the Deployment's fingerprint. When the fingerprint
* changes, a new deployment is triggered.
* This property should contain values associated with the Integration that upon changing
* should trigger a fresh the Deployment needs to be refreshed.
* @default undefined deployments are not triggered for any change to this integration.
*/
readonly deploymentToken?: string;
}

/**
* Base class for backend integrations for an API Gateway method.
*
Expand All @@ -156,7 +170,7 @@ 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) {
public bind(_method: Method): IntegrationConfig | undefined {
return;
}
}
Expand Down
5 changes: 3 additions & 2 deletions 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';

Expand Down Expand Up @@ -92,7 +92,8 @@ export class AwsIntegration extends Integration {
});
}

public bind(method: Method) {
public bind(method: Method): IntegrationConfig | undefined {
this.scope = method;
return;
}
}
15 changes: 12 additions & 3 deletions 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';

Expand Down Expand Up @@ -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 | undefined {
super.bind(method);
const principal = new iam.ServicePrincipal('apigateway.amazonaws.com');

Expand All @@ -72,5 +72,14 @@ export class LambdaIntegration extends AwsIntegration {
sourceArn: method.testMethodArn,
});
}

const cfnFunction = this.handler.node.defaultChild as lambda.CfnFunction;
let deploymentToken;
if (!Token.isUnresolved(cfnFunction.functionName)) {
deploymentToken = JSON.stringify({ functionName: cfnFunction.functionName });
}
return {
deploymentToken,
};
}
}
26 changes: 11 additions & 15 deletions packages/@aws-cdk/aws-apigateway/lib/method.ts
Expand Up @@ -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,
Expand All @@ -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),
Expand All @@ -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,
integrationToken: bindResult?.deploymentToken,
},
});
}
}

Expand Down Expand Up @@ -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;
Expand Down
Expand Up @@ -120,7 +120,7 @@
"BooksApi60AC975F"
]
},
"BooksApiDeployment86CA39AF7e6c771d47a1a3777eba99bffc037822": {
"BooksApiDeployment86CA39AFc1570c78b1ea90526c0309cd74b7b8d0": {
"Type": "AWS::ApiGateway::Deployment",
"Properties": {
"RestApiId": {
Expand All @@ -141,7 +141,7 @@
"Ref": "BooksApi60AC975F"
},
"DeploymentId": {
"Ref": "BooksApiDeployment86CA39AF7e6c771d47a1a3777eba99bffc037822"
"Ref": "BooksApiDeployment86CA39AFc1570c78b1ea90526c0309cd74b7b8d0"
},
"StageName": "prod"
}
Expand Down
44 changes: 44 additions & 0 deletions packages/@aws-cdk/aws-apigateway/test/test.lambda.ts
Expand Up @@ -215,4 +215,48 @@ export = {

test.done();
},

'fingerprint is computed when functionName is specified'(test: Test) {
// GIVEN
const stack = new cdk.Stack();
const restapi = new apigateway.RestApi(stack, 'RestApi');
const method = restapi.root.addMethod('ANY');
const handler = new lambda.Function(stack, 'MyFunc', {
functionName: 'ThisFunction',
runtime: lambda.Runtime.NODEJS_10_X,
handler: 'index.handler',
code: lambda.Code.fromInline('loo'),
});
const integration = new apigateway.LambdaIntegration(handler);

// WHEN
const bindResult = integration.bind(method);

// THEN
test.ok(bindResult?.deploymentToken);
test.deepEqual(bindResult!.deploymentToken, '{"functionName":"ThisFunction"}');

test.done();
},

'fingerprint is not computed when functionName is not specified'(test: Test) {
// GIVEN
const stack = new cdk.Stack();
const restapi = new apigateway.RestApi(stack, 'RestApi');
const method = restapi.root.addMethod('ANY');
const handler = new lambda.Function(stack, 'MyFunc', {
runtime: lambda.Runtime.NODEJS_10_X,
handler: 'index.handler',
code: lambda.Code.fromInline('loo'),
});
const integration = new apigateway.LambdaIntegration(handler);

// WHEN
const bindResult = integration.bind(method);

// THEN
test.equals(bindResult?.deploymentToken, undefined);

test.done();
},
};

0 comments on commit f1b37ef

Please sign in to comment.