feat(apigatewayv2): add aws service integration -- step functions #14498

zxkane · 2021-05-03T09:23:23Z

general changes for supporting aws integration
add StepFunctions StartExecution and StartSyncExecution implementation

closes #11947

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

gitpod-io · 2021-05-03T09:23:25Z

nija-at · 2021-05-10T16:52:38Z

Thanks so much for submitting this pull request. I am marking this pull request as p2, which means that we are unable to work on it immediately, but it's definitely still on our radar.

We use +1s to help prioritize our work, and are happy to revaluate this pull request based on community feedback. You can reach out to the cdk.dev community on Slack to solicit support for reprioritization.

saqibdhuka · 2021-09-27T23:38:03Z

This is a good feature.

I was testing it out with a EXPRESS Synchronous State Machine and the following bug was found:

Environment
Testing it on Linux OS
Software: Visual Studo Code
Language: Typescript

Steps to reproduce the bug

Set up the State Machine to be of type EXPRESS and set it as a Pass State Machine with result value: "Hello"
Use StepFunctionsStartSyncExecutionIntegration within api.addRoutes()
Set input parameter within the integration to be '{"test":"out"}' (with single quotations)
Deploy the CDK Stack
Ue "curl -X POST <invoke URL>/start" (without quotations)

Code to reproduce the bug
`

const passTask2 = new sfn.Pass(this, 'passTask2', {
  result: { value: "Hello" },
});

const handler: sfn.IStateMachine = new sfn.StateMachine(this, 'handler', {
  definition: passTask2,
  stateMachineType: sfn.StateMachineType.EXPRESS,
});

//Step Functions IAM Role
const stepFunctionRole = new iam.Role(this, "sfnRole", {
  assumedBy: new iam.ServicePrincipal("apigateway.amazonaws.com"),
  inlinePolicies: {
    AllowSFNExec: new iam.PolicyDocument({
      statements: [
        new iam.PolicyStatement({
          actions: ["states:StartSyncExecution"],
          effect: iam.Effect.ALLOW,
          resources: [handler.stateMachineArn]
        })
      ]
    })
  }
});

const api = new apigwv2.HttpApi(this, 'HttpApiEndpoint');

api.addRoutes({
  path: '/start',
  methods: [ apigwv2.HttpMethod.POST ],
  integration: new StepFunctionsStartSyncExecutionIntegration({
    stateMachine: handler,
    input: '{"test":"out"}',
  }),
});

`

Expected Output
The expected output is to have state machine output ("Hello) along with billing details and other information

Actual Result
Permission Error: Access Denied Exception
{"__type":"com.amazon.coral.service#AccessDeniedException","Message":"User: arn:aws:sts::123456789102:assumed-role/DemoCdkStack-HttpApiEndpointPOSTstartRoleF1234567-xxxxxxxx/xxxxxxx is not authorized to perform: states:StartSyncExecution on resource: arn:aws:states:us-west-2:123456789102:stateMachine:handlerExxxxxxx-xxxxxxxx because no identity-based policy allows the states:StartSyncExecution action"}%

Visual Proof

Severity
Medium - EXPRESS State Machine invocation is not working with StartSyncExecution

Fix
I was able to fix the bug by doing the following:

From your stack, pass stack into the StepFunctionsStartSyncExecutionIntegration as the Construct.
Edit the StepFunctionsStartSyncExecutionIntegration cosntructor to accept a Construct ( private readonly _scope: Construct )
Change the _fulfillRole function implementation to do the following:
- Change 'this._props.stateMachine.grantStartExecution' to 'this._props.stateMachine.grantExecution(credentialsRole.grantPrincipal, 'states:StartSyncExecution')'
- Next use credentialsRole.attachInlinePolicy() and add IAM Policy with PolicyStatement (with 'states:StartSyncExecution' as the actions) along with other parameters. You will need to pass the Construct (_scope) into the attachInlinePolicy().

This should fix the bug.

I would suggest looking into StepFunctionsStartExecutionIntegration implementation too as I dd not see any attached policy and I'm not sure if it works correctly or not.

zxkane · 2021-09-28T02:19:53Z

This is a good feature.

I was testing it out with a EXPRESS Synchronous State Machine and the following bug was found:

Environment
Testing it on Linux OS
Software: Visual Studo Code
Language: Typescript

Steps to reproduce the bug

Set up the State Machine to be of type EXPRESS and set it as a Pass State Machine with result value: "Hello"

Use StepFunctionsStartSyncExecutionIntegration within api.addRoutes()

Set input parameter within the integration to be '{"test":"out"}' (with single quotations)

Deploy the CDK Stack

Ue "curl -X POST /start" (without quotations)

Code to reproduce the bug
`
const passTask2 = new sfn.Pass(this, 'passTask2', {
  result: { value: "Hello" },
});

const handler: sfn.IStateMachine = new sfn.StateMachine(this, 'handler', {
  definition: passTask2,
  stateMachineType: sfn.StateMachineType.EXPRESS,
});

//Step Functions IAM Role
const stepFunctionRole = new iam.Role(this, "sfnRole", {
  assumedBy: new iam.ServicePrincipal("apigateway.amazonaws.com"),
  inlinePolicies: {
    AllowSFNExec: new iam.PolicyDocument({
      statements: [
        new iam.PolicyStatement({
          actions: ["states:StartSyncExecution"],
          effect: iam.Effect.ALLOW,
          resources: [handler.stateMachineArn]
        })
      ]
    })
  }
});

const api = new apigwv2.HttpApi(this, 'HttpApiEndpoint');

api.addRoutes({
  path: '/start',
  methods: [ apigwv2.HttpMethod.POST ],
  integration: new StepFunctionsStartSyncExecutionIntegration({
    stateMachine: handler,
    input: '{"test":"out"}',
  }),
});
`

Expected Output
The expected output is to have state machine output ("Hello) along with billing details and other information

Actual Result
Permission Error: Access Denied Exception
{"__type":"com.amazon.coral.service#AccessDeniedException","Message":"User: arn:aws:sts::123456789102:assumed-role/DemoCdkStack-HttpApiEndpointPOSTstartRoleF1234567-xxxxxxxx/xxxxxxx is not authorized to perform: states:StartSyncExecution on resource: arn:aws:states:us-west-2:123456789102:stateMachine:handlerExxxxxxx-xxxxxxxx because no identity-based policy allows the states:StartSyncExecution action"}%

Visual Proof

Severity
Medium - EXPRESS State Machine invocation is not working with StartSyncExecution

Fix
I was able to fix the bug by doing the following:

From your stack, pass stack into the StepFunctionsStartSyncExecutionIntegration as the Construct.

Edit the StepFunctionsStartSyncExecutionIntegration cosntructor to accept a Construct ( private readonly _scope: Construct )

Change the _fulfillRole function implementation to do the following:

Change 'this._props.stateMachine.grantStartExecution' to 'this._props.stateMachine.grantExecution(credentialsRole.grantPrincipal, 'states:StartSyncExecution')'

Next use credentialsRole.attachInlinePolicy() and add IAM Policy with PolicyStatement (with 'states:StartSyncExecution' as the actions) along with other parameters. You will need to pass the Construct (_scope) into the attachInlinePolicy().

This should fix the bug.

I would suggest looking into StepFunctionsStartExecutionIntegration implementation too as I dd not see any attached policy and I'm not sure if it works correctly or not.

Thanks for your deep dive the exception and proposed fix.

Will keep an eye on it due to the CDK team does not have review bandwidth.

…cutionIntegration fixed to attach Inline Policy for credentialsRole. Fixed README to change input for correct input passing. Fixed Unit and Integration tests. All tests passing. Permissions bug mentioned in the PR is now fixed. Fix Bug awsGH-14498."

saqibdhuka · 2021-09-29T19:48:12Z

This is a good feature.

I was testing it out with a EXPRESS Synchronous State Machine and the following bug was found:

Environment Testing it on Linux OS Software: Visual Studo Code Language: Typescript

Steps to reproduce the bug
1. Set up the State Machine to be of type EXPRESS and set it as a Pass State Machine with result value: "Hello"

2. Use StepFunctionsStartSyncExecutionIntegration within api.addRoutes()

3. Set input parameter within the integration to be '{"test":"out"}' (with single quotations)

4. Deploy the CDK Stack

5. Ue "curl -X POST <invoke URL>/start" (without quotations)
Code to reproduce the bug `
const passTask2 = new sfn.Pass(this, 'passTask2', {
  result: { value: "Hello" },
});

const handler: sfn.IStateMachine = new sfn.StateMachine(this, 'handler', {
  definition: passTask2,
  stateMachineType: sfn.StateMachineType.EXPRESS,
});

//Step Functions IAM Role
const stepFunctionRole = new iam.Role(this, "sfnRole", {
  assumedBy: new iam.ServicePrincipal("apigateway.amazonaws.com"),
  inlinePolicies: {
    AllowSFNExec: new iam.PolicyDocument({
      statements: [
        new iam.PolicyStatement({
          actions: ["states:StartSyncExecution"],
          effect: iam.Effect.ALLOW,
          resources: [handler.stateMachineArn]
        })
      ]
    })
  }
});

const api = new apigwv2.HttpApi(this, 'HttpApiEndpoint');

api.addRoutes({
  path: '/start',
  methods: [ apigwv2.HttpMethod.POST ],
  integration: new StepFunctionsStartSyncExecutionIntegration({
    stateMachine: handler,
    input: '{"test":"out"}',
  }),
});
`

Expected Output The expected output is to have state machine output ("Hello) along with billing details and other information

Actual Result Permission Error: Access Denied Exception {"__type":"com.amazon.coral.service#AccessDeniedException","Message":"User: arn:aws:sts::123456789102:assumed-role/DemoCdkStack-HttpApiEndpointPOSTstartRoleF1234567-xxxxxxxx/xxxxxxx is not authorized to perform: states:StartSyncExecution on resource: arn:aws:states:us-west-2:123456789102:stateMachine:handlerExxxxxxx-xxxxxxxx because no identity-based policy allows the states:StartSyncExecution action"}%

Visual Proof

Severity Medium - EXPRESS State Machine invocation is not working with StartSyncExecution

Fix I was able to fix the bug by doing the following:
1. From your stack, pass stack into the StepFunctionsStartSyncExecutionIntegration as the Construct.

2. Edit the StepFunctionsStartSyncExecutionIntegration cosntructor to accept a Construct ( private readonly _scope: Construct )

3. Change the _fulfillRole function implementation to do the following:
   - Change 'this._props.stateMachine.grantStartExecution' to 'this._props.stateMachine.grantExecution(credentialsRole.grantPrincipal, 'states:StartSyncExecution')'
   - Next use credentialsRole.attachInlinePolicy() and add IAM Policy with PolicyStatement (with 'states:StartSyncExecution' as the actions) along with other parameters. You will need to pass the Construct (_scope) into the attachInlinePolicy().
This should fix the bug.

I would suggest looking into StepFunctionsStartExecutionIntegration implementation too as I dd not see any attached policy and I'm not sure if it works correctly or not.

The bug is now fixed!!

diegotry · 2021-10-02T00:13:03Z

packages/@aws-cdk/aws-apigatewayv2-integrations/lib/http/aws.ts

+ * The StepFunctions-StartExecution integration resource for HTTP API
+ */
+export class StepFunctionsStartExecutionIntegration extends StepFunctionsIntegration {


this one says StartExecution but later on L79 you are giving permission to StartSyncExecution.

Also resources [*] should be avoided if you have the state machine.

Good Note. I will fix this. Thank you

diegotry · 2021-10-02T00:16:00Z

packages/@aws-cdk/aws-apigatewayv2-integrations/lib/http/aws.ts

+    credentialsRole.attachInlinePolicy(
+      new iam.Policy(this._scope, 'AllowSfnSyncExec', {
+        statements: [
+          new iam.PolicyStatement({
+            actions: ['states:StartSyncExecution'],
+            effect: iam.Effect.ALLOW,
+            resources: ['*'],
+          }),
+        ],
+      }),
+    );


I believe this should be removed.

This attaches the policy and fixes the permission bug mentioned above.

but this is for StartExecution, not StartSyncExecution right? They are different constructs and therefore requires different permissions.

yes I have fixed that in the push

diegotry · 2021-10-02T00:16:38Z

packages/@aws-cdk/aws-apigatewayv2-integrations/lib/http/aws.ts

+   */
+  protected _fulfillRole(credentialsRole: iam.IRole): void {
+
+    this._props.stateMachine.grantExecution(credentialsRole.grantPrincipal, 'states:StartSyncExecution');


could you please clarify what is this line of code doing?

It grants the execution permission, but does not attach the policy. That needs to be done explicitly.

diegotry · 2021-10-02T00:17:19Z

packages/@aws-cdk/aws-apigatewayv2-integrations/lib/http/aws.ts

+          new iam.PolicyStatement({
+            actions: ['states:StartSyncExecution'],
+            effect: iam.Effect.ALLOW,
+            resources: ['*'],


I believe permission to * should be avoided as you have the state machine. It should give permission only to the state machine ARN as a best practice (least privilege).

Yes that is correct. I will fix this. Thank you.

diegotry · 2021-10-02T00:17:52Z

packages/@aws-cdk/aws-apigatewayv2-integrations/lib/http/aws.ts

+   *
+   * @default - undefined
+   */
+  readonly traceHeader?: string;


doesn't StartExecution also accepts a traceheader?

Yes it does.

diegotry · 2021-10-02T00:18:20Z

packages/@aws-cdk/aws-apigatewayv2-integrations/lib/http/private/integration.ts

+
+  public bind(_options: HttpRouteIntegrationBindOptions): HttpRouteIntegrationConfig {
+
+    const role = new Role(_options.scope, 'Role', {


should this be called ApiGatewayIntegrationRole?

Yes. I will fix that

diegotry · 2021-10-02T00:22:50Z

I believe the template files should be removed from your request.

diegotry

there's a bunch of files that are not related to your change. Example: Lambda Hot swap

diegotry · 2021-10-02T01:29:57Z

packages/@aws-cdk/aws-apigatewayv2-integrations/lib/http/private/integration.ts

+
+  public bind(_options: HttpRouteIntegrationBindOptions): HttpRouteIntegrationConfig {
+
+    const ApiGatewayIntegrationRole = new Role(_options.scope, 'ApiGatewayIntegrationRole', {


I believe this should be lowercase.

I will fix that

diegotry · 2021-10-02T01:31:45Z

packages/@aws-cdk/aws-apigatewayv2-integrations/test/http/aws.test.ts

+      PayloadFormatVersion: '1.0',
+      RequestParameters: {
+        StateMachineArn: {
+          Ref: 'MyStateMachine6C968CA5',


I wonder if this should be the state machine arn instead of the resource. Did it work if you try to deploy?

nope it does not work. it needs the resource.

diegotry · 2021-10-02T01:32:52Z

packages/@aws-cdk/aws-apigatewayv2-integrations/test/http/aws.test.ts

+    expect(stack).toHaveResource('AWS::ApiGatewayV2::Integration', {
+      IntegrationType: 'AWS_PROXY',
+      IntegrationSubtype: 'StepFunctions-StartSyncExecution',
+      PayloadFormatVersion: '1.0',
+      RequestParameters: {
+        StateMachineArn: {
+          Ref: 'MyStateMachine6C968CA5',
+        },
+        Input: {
+          a: 'b',
+        },
+      },
+    });


should you validate if the other resources are also created? The IAM Role for example

IAM Role did not show up in the properties when testing, but I will add more extended tests for both.

diegotry · 2021-10-02T01:36:47Z

...ges/@aws-cdk/aws-apigatewayv2-integrations/test/http/integ.aws-integration-startExecution.ts

+const endpoint = new HttpApi(stack, 'AwsIntegrationApi', {
+  defaultIntegration: new StepFunctionsStartExecutionIntegration(stack, {
+    stateMachine: state,
+    input: '$request.body',
+  }),
+});
+


should this include some assertions? What is this test file doing?

It is integration testing. While testing, it validates the test against expected json.

diegotry · 2021-10-02T01:37:46Z

...@aws-cdk/aws-apigatewayv2-integrations/test/http/integ.aws-integration-startSyncExecution.ts

+const endpoint = new HttpApi(stack, 'AwsIntegrationApi', {
+  defaultIntegration: new StepFunctionsStartSyncExecutionIntegration(stack, {
+    stateMachine: state,
+    input: '$request.body',
+  }),
+});


same question as above.

It is integration testing. While testing, it validates the test against expected json.

nija-at

This PR is changing unrelated files and has merge conflicts. Please update the PR to the latest master and make sure only the necessary changes are here.
Please add a section to the README with a sample usage.

Pull request has been modified.

…ncExecutionIntegration. Added unit and integration tests. Fixed issues mentioned in the Pull-request. Added example to README Fixed unit test closes aws#11947.

aws-cdk-automation · 2021-11-04T19:37:13Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
Commit ID: 1e2999c
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

TheRealAmazonKendra · 2022-03-30T21:03:22Z

This PR has been deemed to be abandoned, and will be closed. Please create a new PR for these changes if you think this decision has been made in error.

github-actions bot added @aws-cdk/aws-apigatewayv2 Related to Amazon API Gateway v2 @aws-cdk/aws-stepfunctions Related to AWS StepFunctions labels May 3, 2021

github-actions bot assigned nija-at and shivlaks May 3, 2021

nija-at added effort/medium Medium work item – several days of effort p2 and removed @aws-cdk/aws-stepfunctions Related to AWS StepFunctions labels May 10, 2021

nija-at unassigned nija-at and shivlaks May 10, 2021

RomainMuller force-pushed the master branch from 924c117 to ebfd5f2 Compare September 29, 2021 13:07

saqibdhuka force-pushed the apiv2-aws-integration branch 2 times, most recently from 711b6cb to 16447ea Compare October 2, 2021 00:04

diegotry reviewed Oct 2, 2021

View reviewed changes

saqibdhuka force-pushed the apiv2-aws-integration branch 3 times, most recently from b73875a to 6551bec Compare October 2, 2021 01:24

diegotry reviewed Oct 2, 2021

View reviewed changes

saqibdhuka force-pushed the apiv2-aws-integration branch from 6551bec to 009114d Compare October 2, 2021 23:17

nija-at previously requested changes Oct 7, 2021

View reviewed changes

saqibdhuka force-pushed the apiv2-aws-integration branch from 009114d to afc9ef9 Compare October 7, 2021 16:38

saqibdhuka force-pushed the apiv2-aws-integration branch from afc9ef9 to 0a6a1b1 Compare October 14, 2021 00:00

saqibdhuka force-pushed the apiv2-aws-integration branch 3 times, most recently from 98e87a2 to a50b049 Compare October 14, 2021 17:40

saqibdhuka force-pushed the apiv2-aws-integration branch from a50b049 to 9c04142 Compare November 4, 2021 17:33

Added StepFunctionsStartExecutionIntegration and StepFunctionsStartSy…

1e2999c

…ncExecutionIntegration. Added unit and integration tests. Fixed issues mentioned in the Pull-request. Added example to README Fixed unit test closes aws#11947.

saqibdhuka force-pushed the apiv2-aws-integration branch from 9c04142 to 1e2999c Compare November 4, 2021 18:46

rix0rrr added feature-request A feature should be added or improved. p2 and removed p2 effort/medium Medium work item – several days of effort @aws-cdk/aws-apigatewayv2 Related to Amazon API Gateway v2 feature-request A feature should be added or improved. labels Mar 4, 2022

TheRealAmazonKendra closed this Mar 30, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(apigatewayv2): add aws service integration -- step functions #14498

feat(apigatewayv2): add aws service integration -- step functions #14498

zxkane commented May 3, 2021

gitpod-io bot commented May 3, 2021

nija-at commented May 10, 2021

saqibdhuka commented Sep 27, 2021

zxkane commented Sep 28, 2021

saqibdhuka commented Sep 29, 2021

diegotry Oct 2, 2021

saqibdhuka Oct 2, 2021

diegotry Oct 2, 2021

saqibdhuka Oct 2, 2021

diegotry Oct 2, 2021

saqibdhuka Oct 2, 2021

diegotry Oct 2, 2021

saqibdhuka Oct 2, 2021

diegotry Oct 2, 2021

saqibdhuka Oct 2, 2021

diegotry Oct 2, 2021

saqibdhuka Oct 2, 2021

diegotry Oct 2, 2021

saqibdhuka Oct 2, 2021

diegotry commented Oct 2, 2021

diegotry left a comment

diegotry Oct 2, 2021

saqibdhuka Oct 2, 2021

diegotry Oct 2, 2021

saqibdhuka Oct 2, 2021

diegotry Oct 2, 2021

saqibdhuka Oct 2, 2021

diegotry Oct 2, 2021

saqibdhuka Oct 2, 2021

diegotry Oct 2, 2021

saqibdhuka Oct 2, 2021

nija-at left a comment

aws-cdk-automation commented Nov 4, 2021

TheRealAmazonKendra commented Mar 30, 2022


		public bind(_options: HttpRouteIntegrationBindOptions): HttpRouteIntegrationConfig {

		const role = new Role(_options.scope, 'Role', {


		public bind(_options: HttpRouteIntegrationBindOptions): HttpRouteIntegrationConfig {

		const ApiGatewayIntegrationRole = new Role(_options.scope, 'ApiGatewayIntegrationRole', {

feat(apigatewayv2): add aws service integration -- step functions #14498

feat(apigatewayv2): add aws service integration -- step functions #14498

Conversation

zxkane commented May 3, 2021

gitpod-io bot commented May 3, 2021

nija-at commented May 10, 2021

saqibdhuka commented Sep 27, 2021

zxkane commented Sep 28, 2021

saqibdhuka commented Sep 29, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

diegotry commented Oct 2, 2021

diegotry left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nija-at left a comment

Choose a reason for hiding this comment

aws-cdk-automation commented Nov 4, 2021

AWS CodeBuild CI Report

TheRealAmazonKendra commented Mar 30, 2022