From 10771f8a50c9f84ccbeff436839eac2fe66b66fd Mon Sep 17 00:00:00 2001 From: Jonathan Goldwasser Date: Mon, 21 Mar 2022 10:34:38 +0100 Subject: [PATCH] fix(apigateway): missing rmethod responses and role in Step Functions integration The method responses and role were only automatically created when using the `StepFunctionsRestApi` construct. Move the logic inside the the integration. It's now possible to do: ```ts api.root.addResource('sfn').addMethod('POST', StepFunctionsIntegration.startExecution(stateMachine)); ``` Previously this did not create the proper method responses and required a role to be passed. --- .../lib/integrations/stepfunctions.ts | 47 +++++++++-- .../@aws-cdk/aws-apigateway/lib/method.ts | 15 +++- .../aws-apigateway/lib/stepfunctions-api.ts | 61 +++----------- .../integ.stepfunctions-api.expected.json | 12 +-- .../test/stepfunctions-api.test.ts | 84 ++++++++++++++++++- 5 files changed, 154 insertions(+), 65 deletions(-) diff --git a/packages/@aws-cdk/aws-apigateway/lib/integrations/stepfunctions.ts b/packages/@aws-cdk/aws-apigateway/lib/integrations/stepfunctions.ts index f9461eb0b44e8..335c87fd22d9d 100644 --- a/packages/@aws-cdk/aws-apigateway/lib/integrations/stepfunctions.ts +++ b/packages/@aws-cdk/aws-apigateway/lib/integrations/stepfunctions.ts @@ -6,6 +6,7 @@ import { Token } from '@aws-cdk/core'; import { RequestContext } from '.'; import { IntegrationConfig, IntegrationOptions, PassthroughBehavior } from '../integration'; import { Method } from '../method'; +import { Model } from '../model'; import { AwsIntegration } from './aws'; /** * Options when configuring Step Functions synchronous integration with Rest API @@ -94,6 +95,7 @@ export class StepFunctionsIntegration { * @example * * const stateMachine = new stepfunctions.StateMachine(this, 'MyStateMachine', { + * stateMachineType: stepfunctions.StateMachineType.EXPRESS, * definition: stepfunctions.Chain.start(new stepfunctions.Pass(this, 'Pass')), * }); * @@ -127,9 +129,11 @@ class StepFunctionsExecutionIntegration extends AwsIntegration { public bind(method: Method): IntegrationConfig { const bindResult = super.bind(method); - const principal = new iam.ServicePrincipal('apigateway.amazonaws.com'); - this.stateMachine.grantExecution(principal, 'states:StartSyncExecution'); + const credentialsRole = bindResult.options?.credentialsRole ?? new iam.Role(method, 'StartSyncExecutionRole', { + assumedBy: new iam.ServicePrincipal('apigateway.amazonaws.com'), + }); + this.stateMachine.grantStartSyncExecution(credentialsRole); let stateMachineName; @@ -152,8 +156,17 @@ class StepFunctionsExecutionIntegration extends AwsIntegration { if (stateMachineName !== undefined && !Token.isUnresolved(stateMachineName)) { deploymentToken = JSON.stringify({ stateMachineName }); } + + for (const methodResponse of METHOD_RESPONSES) { + method.addMethodResponse(methodResponse); + } + return { ...bindResult, + options: { + ...bindResult.options, + credentialsRole, + }, deploymentToken, }; } @@ -200,8 +213,8 @@ function integrationResponse() { /* eslint-disable */ 'application/json': [ '#set($inputRoot = $input.path(\'$\'))', - '#if($input.path(\'$.status\').toString().equals("FAILED"))', - '#set($context.responseOverride.status = 500)', + '#if($input.path(\'$.status\').toString().equals("FAILED"))', + '#set($context.responseOverride.status = 500)', '{', '"error": "$input.path(\'$.error\')",', '"cause": "$input.path(\'$.cause\')"', @@ -301,4 +314,28 @@ function requestContext(requestContextObj: RequestContext | undefined): string { const doublequotes = '"'; const replaceWith = '@@'; return contextAsString.split(doublequotes).join(replaceWith); -} \ No newline at end of file +} + +/** + * Method response model for each HTTP code response + */ +const METHOD_RESPONSES = [ + { + statusCode: '200', + responseModels: { + 'application/json': Model.EMPTY_MODEL, + }, + }, + { + statusCode: '400', + responseModels: { + 'application/json': Model.ERROR_MODEL, + }, + }, + { + statusCode: '500', + responseModels: { + 'application/json': Model.ERROR_MODEL, + }, + }, +]; diff --git a/packages/@aws-cdk/aws-apigateway/lib/method.ts b/packages/@aws-cdk/aws-apigateway/lib/method.ts index 5a99d03270f41..8746189f9bdaa 100644 --- a/packages/@aws-cdk/aws-apigateway/lib/method.ts +++ b/packages/@aws-cdk/aws-apigateway/lib/method.ts @@ -1,4 +1,4 @@ -import { ArnFormat, Resource, Stack } from '@aws-cdk/core'; +import { ArnFormat, Lazy, Resource, Stack } from '@aws-cdk/core'; import { Construct } from 'constructs'; import { CfnMethod, CfnMethodProps } from './apigateway.generated'; import { Authorizer, IAuthorizer } from './authorizer'; @@ -168,6 +168,8 @@ export class Method extends Resource { */ public readonly api: IRestApi; + private methodResponses: MethodResponse[]; + constructor(scope: Construct, id: string, props: MethodProps) { super(scope, id); @@ -196,6 +198,8 @@ export class Method extends Resource { authorizer._attachToApi(this.api); } + this.methodResponses = options.methodResponses ?? []; + const integration = props.integration ?? this.resource.defaultIntegration ?? new MockIntegration(); const bindResult = integration.bind(this); @@ -209,7 +213,7 @@ export class Method extends Resource { authorizerId, requestParameters: options.requestParameters || defaultMethodOptions.requestParameters, integration: this.renderIntegration(bindResult), - methodResponses: this.renderMethodResponses(options.methodResponses), + methodResponses: Lazy.any({ produce: () => this.renderMethodResponses(this.methodResponses) }, { omitEmptyArray: true }), requestModels: this.renderRequestModels(options.requestModels), requestValidatorId: this.requestValidatorId(options), authorizationScopes: options.authorizationScopes ?? defaultMethodOptions.authorizationScopes, @@ -267,6 +271,13 @@ export class Method extends Resource { return this.api.arnForExecuteApi(this.httpMethod, pathForArn(this.resource.path), 'test-invoke-stage'); } + /** + * Add a method response to this method + */ + public addMethodResponse(methodResponse: MethodResponse): void { + this.methodResponses.push(methodResponse); + } + private renderIntegration(bindResult: IntegrationConfig): CfnMethod.IntegrationProperty { const options = bindResult.options ?? {}; let credentials; diff --git a/packages/@aws-cdk/aws-apigateway/lib/stepfunctions-api.ts b/packages/@aws-cdk/aws-apigateway/lib/stepfunctions-api.ts index bbae7158f1ac1..c48b497160bd7 100644 --- a/packages/@aws-cdk/aws-apigateway/lib/stepfunctions-api.ts +++ b/packages/@aws-cdk/aws-apigateway/lib/stepfunctions-api.ts @@ -4,7 +4,6 @@ import { Construct } from 'constructs'; import { RestApi, RestApiProps } from '.'; import { RequestContext } from './integrations'; import { StepFunctionsIntegration } from './integrations/stepfunctions'; -import { Model } from './model'; /** * Properties for StepFunctionsRestApi @@ -89,6 +88,14 @@ export interface StepFunctionsRestApiProps extends RestApiProps { * @default false */ readonly authorizer?: boolean; + + /** + * An IAM role that API Gateway will assume to start the execution of the + * state machine. + * + * @default - a new role is created + */ + readonly role?: iam.IRole; } /** @@ -105,7 +112,7 @@ export class StepFunctionsRestApi extends RestApi { } const stepfunctionsIntegration = StepFunctionsIntegration.startExecution(props.stateMachine, { - credentialsRole: role(scope, props), + credentialsRole: props.role, requestContext: props.requestContext, path: props.path?? true, querystring: props.querystring?? true, @@ -115,54 +122,6 @@ export class StepFunctionsRestApi extends RestApi { super(scope, id, props); - this.root.addMethod('ANY', stepfunctionsIntegration, { - methodResponses: methodResponse(), - }); + this.root.addMethod('ANY', stepfunctionsIntegration); } } - -/** - * Defines the IAM Role for API Gateway with required permissions - * to invoke a synchronous execution for the provided state machine - * - * @param scope - * @param props - * @returns Role - IAM Role - */ -function role(scope: Construct, props: StepFunctionsRestApiProps): iam.Role { - const roleName: string = 'StartSyncExecutionRole'; - const apiRole = new iam.Role(scope, roleName, { - assumedBy: new iam.ServicePrincipal('apigateway.amazonaws.com'), - }); - - props.stateMachine.grantStartSyncExecution(apiRole); - - return apiRole; -} - -/** - * Defines the method response modelfor each HTTP code response - * @returns methodResponse - */ -function methodResponse() { - return [ - { - statusCode: '200', - responseModels: { - 'application/json': Model.EMPTY_MODEL, - }, - }, - { - statusCode: '400', - responseModels: { - 'application/json': Model.ERROR_MODEL, - }, - }, - { - statusCode: '500', - responseModels: { - 'application/json': Model.ERROR_MODEL, - }, - }, - ]; -} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-apigateway/test/integ.stepfunctions-api.expected.json b/packages/@aws-cdk/aws-apigateway/test/integ.stepfunctions-api.expected.json index afbccd88a73ae..ce9b25be6ecbe 100644 --- a/packages/@aws-cdk/aws-apigateway/test/integ.stepfunctions-api.expected.json +++ b/packages/@aws-cdk/aws-apigateway/test/integ.stepfunctions-api.expected.json @@ -44,7 +44,7 @@ "StateMachineRoleB840431D" ] }, - "StartSyncExecutionRoleDE73CB90": { + "StepFunctionsRestApiANYStartSyncExecutionRole425C03BB": { "Type": "AWS::IAM::Role", "Properties": { "AssumeRolePolicyDocument": { @@ -61,7 +61,7 @@ } } }, - "StartSyncExecutionRoleDefaultPolicy5A5803F8": { + "StepFunctionsRestApiANYStartSyncExecutionRoleDefaultPolicy7B6D0CED": { "Type": "AWS::IAM::Policy", "Properties": { "PolicyDocument": { @@ -76,10 +76,10 @@ ], "Version": "2012-10-17" }, - "PolicyName": "StartSyncExecutionRoleDefaultPolicy5A5803F8", + "PolicyName": "StepFunctionsRestApiANYStartSyncExecutionRoleDefaultPolicy7B6D0CED", "Roles": [ { - "Ref": "StartSyncExecutionRoleDE73CB90" + "Ref": "StepFunctionsRestApiANYStartSyncExecutionRole425C03BB" } ] } @@ -152,7 +152,7 @@ "Integration": { "Credentials": { "Fn::GetAtt": [ - "StartSyncExecutionRoleDE73CB90", + "StepFunctionsRestApiANYStartSyncExecutionRole425C03BB", "Arn" ] }, @@ -289,4 +289,4 @@ } } } -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-apigateway/test/stepfunctions-api.test.ts b/packages/@aws-cdk/aws-apigateway/test/stepfunctions-api.test.ts index 5922d61025965..44643ea0814ff 100644 --- a/packages/@aws-cdk/aws-apigateway/test/stepfunctions-api.test.ts +++ b/packages/@aws-cdk/aws-apigateway/test/stepfunctions-api.test.ts @@ -3,6 +3,7 @@ import * as sfn from '@aws-cdk/aws-stepfunctions'; import { StateMachine } from '@aws-cdk/aws-stepfunctions'; import * as cdk from '@aws-cdk/core'; import * as apigw from '../lib'; +import { StepFunctionsIntegration } from '../lib'; describe('Step Functions api', () => { test('StepFunctionsRestApi defines correct REST API resources', () => { @@ -33,7 +34,7 @@ describe('Step Functions api', () => { Integration: { Credentials: { 'Fn::GetAtt': [ - 'StartSyncExecutionRoleDE73CB90', + 'StepFunctionsRestApiANYStartSyncExecutionRole425C03BB', 'Arn', ], }, @@ -75,6 +76,87 @@ describe('Step Functions api', () => { }); }); + test('StepFunctionsExecutionIntegration on a method', () => { + // GIVEN + const stack = new cdk.Stack(); + const api = new apigw.RestApi(stack, 'Api'); + const stateMachine = new sfn.StateMachine(stack, 'StateMachine', { + stateMachineType: sfn.StateMachineType.EXPRESS, + definition: new sfn.Pass(stack, 'Pass'), + }); + + // WHEN + api.root.addResource('sfn').addMethod('POST', StepFunctionsIntegration.startExecution(stateMachine)); + + // THEN + Template.fromStack(stack).hasResourceProperties('AWS::ApiGateway::Method', { + HttpMethod: 'POST', + MethodResponses: getMethodResponse(), + Integration: { + Credentials: { + 'Fn::GetAtt': [ + 'ApisfnPOSTStartSyncExecutionRole8E8879B0', + 'Arn', + ], + }, + IntegrationHttpMethod: 'POST', + IntegrationResponses: getIntegrationResponse(), + RequestTemplates: { + 'application/json': { + 'Fn::Join': [ + '', + [ + "## Velocity Template used for API Gateway request mapping template\n##\n## This template forwards the request body, header, path, and querystring\n## to the execution input of the state machine.\n##\n## \"@@\" is used here as a placeholder for '\"' to avoid using escape characters.\n\n#set($inputString = '')\n#set($includeHeaders = false)\n#set($includeQueryString = true)\n#set($includePath = true)\n#set($includeAuthorizer = false)\n#set($allParams = $input.params())\n{\n \"stateMachineArn\": \"", + { + Ref: 'StateMachine2E01A3A5', + }, + "\",\n\n #set($inputString = \"$inputString,@@body@@: $input.body\")\n\n #if ($includeHeaders)\n #set($inputString = \"$inputString, @@header@@:{\")\n #foreach($paramName in $allParams.header.keySet())\n #set($inputString = \"$inputString @@$paramName@@: @@$util.escapeJavaScript($allParams.header.get($paramName))@@\")\n #if($foreach.hasNext)\n #set($inputString = \"$inputString,\")\n #end\n #end\n #set($inputString = \"$inputString }\")\n \n #end\n\n #if ($includeQueryString)\n #set($inputString = \"$inputString, @@querystring@@:{\")\n #foreach($paramName in $allParams.querystring.keySet())\n #set($inputString = \"$inputString @@$paramName@@: @@$util.escapeJavaScript($allParams.querystring.get($paramName))@@\")\n #if($foreach.hasNext)\n #set($inputString = \"$inputString,\")\n #end\n #end\n #set($inputString = \"$inputString }\")\n #end\n\n #if ($includePath)\n #set($inputString = \"$inputString, @@path@@:{\")\n #foreach($paramName in $allParams.path.keySet())\n #set($inputString = \"$inputString @@$paramName@@: @@$util.escapeJavaScript($allParams.path.get($paramName))@@\")\n #if($foreach.hasNext)\n #set($inputString = \"$inputString,\")\n #end\n #end\n #set($inputString = \"$inputString }\")\n #end\n \n #if ($includeAuthorizer)\n #set($inputString = \"$inputString, @@authorizer@@:{\")\n #foreach($paramName in $context.authorizer.keySet())\n #set($inputString = \"$inputString @@$paramName@@: @@$util.escapeJavaScript($context.authorizer.get($paramName))@@\")\n #if($foreach.hasNext)\n #set($inputString = \"$inputString,\")\n #end\n #end\n #set($inputString = \"$inputString }\")\n #end\n\n #set($requestContext = \"\")\n ## Check if the request context should be included as part of the execution input\n #if($requestContext && !$requestContext.empty)\n #set($inputString = \"$inputString,\")\n #set($inputString = \"$inputString @@requestContext@@: $requestContext\")\n #end\n\n #set($inputString = \"$inputString}\")\n #set($inputString = $inputString.replaceAll(\"@@\",'\"'))\n #set($len = $inputString.length() - 1)\n \"input\": \"{$util.escapeJavaScript($inputString.substring(1,$len))}\"\n}\n", + ], + ], + }, + }, + Type: 'AWS', + Uri: { + 'Fn::Join': [ + '', + [ + 'arn:', + { + Ref: 'AWS::Partition', + }, + ':apigateway:', + { + Ref: 'AWS::Region', + }, + ':states:action/StartSyncExecution', + ], + ], + }, + PassthroughBehavior: 'NEVER', + }, + }); + + Template.fromStack(stack).hasResourceProperties('AWS::IAM::Policy', { + PolicyDocument: { + Statement: [ + { + Action: 'states:StartSyncExecution', + Effect: 'Allow', + Resource: { + Ref: 'StateMachine2E01A3A5', + }, + }, + ], + Version: '2012-10-17', + }, + Roles: [ + { + Ref: 'ApisfnPOSTStartSyncExecutionRole8E8879B0', + }, + ], + }); + }); + test('fails if options.defaultIntegration is set', () => { //GIVEN const { stack, stateMachine } = givenSetup();