feat(apigateway): add useDefaultMethodResponses property for StepFunctionsIntegrations #27645

go-to-k · 2023-10-23T07:44:58Z

In StepFunctionsIntegration, method response models (methodResponses) with 200, 400 and 500 status codes are added by default.

This PR adds useDefaultMethodResponses property not to add default method response models by set it to false.

Closes #27520.

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

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

lpizzinidev

Nice 👍
Just some documentation and test minor-changes needed in my opinion.

lpizzinidev · 2023-10-24T14:28:16Z

packages/aws-cdk-lib/aws-apigateway/lib/integrations/stepfunctions.ts

+  private readonly useDefaultMethodResponses: boolean;
  constructor(stateMachine: sfn.IStateMachine, options: StepFunctionsExecutionIntegrationOptions = {}) {


Suggested change

private readonly useDefaultMethodResponses: boolean;

constructor(stateMachine: sfn.IStateMachine, options: StepFunctionsExecutionIntegrationOptions = {}) {

private readonly useDefaultMethodResponses: boolean;

constructor(stateMachine: sfn.IStateMachine, options: StepFunctionsExecutionIntegrationOptions = {}) {

lpizzinidev · 2023-10-24T14:29:02Z

packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-api.ts

@@ -35,6 +35,7 @@ class StepFunctionsRestApiDeploymentStack extends cdk.Stack {
        accountId: true,
        userArn: true,
      },
+      useDefaultMethodResponses: true,


Suggested change

useDefaultMethodResponses: true,

Redundant.

lpizzinidev · 2023-10-24T14:31:45Z

packages/aws-cdk-lib/aws-apigateway/test/integrations/stepfunctions.test.ts

@@ -488,6 +488,44 @@ describe('StepFunctionsIntegration', () => {
      },
    });
  });
+
+  test('default method responses are not created when useDefaultMethodResponses is false', () => {


Can you please also add a similar test using the StepFunctionsRestApi class constructor to increase coverage?

lpizzinidev · 2023-10-24T14:41:53Z

packages/aws-cdk-lib/aws-apigateway/lib/integrations/stepfunctions.ts

@@ -83,6 +83,13 @@ export interface StepFunctionsExecutionIntegrationOptions extends IntegrationOpt
   * @default false
   */
  readonly authorizer?: boolean;
+
+  /**
+   * Enable to add default method response models with 200, 400 and 500 status codes.


Suggested change

* Enable to add default method response models with 200, 400 and 500 status codes.

* Whether to add default response models with 200, 400, and 500 status codes to the method.

Small revision, will need an update in StepFunctionsRestApiProps as well.

lpizzinidev · 2023-10-24T14:53:38Z

packages/aws-cdk-lib/aws-apigateway/README.md

+And in `StepFunctionsRestApi`, method response models (`methodResponses`) with 200, 400 and 500 status codes are added by default.
+
+If you don't want to add default method response models, you can set `useDefaultMethodResponses` to `false` as follows:
+
+```ts fixture=stepfunctions
+new apigateway.StepFunctionsRestApi(this, 'StepFunctionsRestApi', {
+  stateMachine: machine,
+  useDefaultMethodResponses: false,
+});


Suggested change

And in `StepFunctionsRestApi`, method response models (`methodResponses`) with 200, 400 and 500 status codes are added by default.

If you don't want to add default method response models, you can set `useDefaultMethodResponses` to `false` as follows:

```ts fixture=stepfunctions

new apigateway.StepFunctionsRestApi(this, 'StepFunctionsRestApi', {

stateMachine: machine,

useDefaultMethodResponses: false,

});

To disable default response models generation use the `useDefaultMethodResponses` property:

```ts

new apigateway.StepFunctionsRestApi(this, 'StepFunctionsRestApi', {

stateMachine: machine,

useDefaultMethodResponses: false,

});

I would move this here.

go-to-k · 2023-10-25T03:51:23Z

@lpizzinidev

Thanks, I changed. Please review them again.

lpizzinidev

Thanks, just a minor adjustment and it will be good to go for me.

lpizzinidev · 2023-10-25T06:27:38Z

packages/aws-cdk-lib/aws-apigateway/test/stepfunctions-api.test.ts

+      AuthorizationType: 'NONE',
+      RestApiId: {
+        Ref: 'StepFunctionsRestApiC6E3E883',
+      },
+      ResourceId: {
+        'Fn::GetAtt': [
+          'StepFunctionsRestApiC6E3E883',
+          'RootResourceId',
+        ],
+      },
+      Integration: {
+        Credentials: {
+          'Fn::GetAtt': [
+            'StepFunctionsRestApiANYStartSyncExecutionRole425C03BB',
+            '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)).replaceAll(\"\\\\'\",\"'\")}\"\n}\n",
+              ],
+            ],
+          },
+        },
+        Type: 'AWS',
+        Uri: {
+          'Fn::Join': [
+            '',
+            [
+              'arn:',
+              {
+                Ref: 'AWS::Partition',
+              },
+              ':apigateway:',
+              {
+                Ref: 'AWS::Region',
+              },
+              ':states:action/StartSyncExecution',
+            ],
+          ],
+        },
+        PassthroughBehavior: 'NEVER',


Suggested change

AuthorizationType: 'NONE',

RestApiId: {

Ref: 'StepFunctionsRestApiC6E3E883',

},

ResourceId: {

'Fn::GetAtt': [

'StepFunctionsRestApiC6E3E883',

'RootResourceId',

],

},

Integration: {

Credentials: {

'Fn::GetAtt': [

'StepFunctionsRestApiANYStartSyncExecutionRole425C03BB',

'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)).replaceAll(\"\\\\'\",\"'\")}\"\n}\n",

],

],

},

},

Type: 'AWS',

Uri: {

'Fn::Join': [

'',

[

'arn:',

{

Ref: 'AWS::Partition',

},

':apigateway:',

{

Ref: 'AWS::Region',

},

':states:action/StartSyncExecution',

],

],

},

PassthroughBehavior: 'NEVER',

Let's match only on HttpMethod and MethodResponses to keep this concise.

go-to-k · 2023-10-25T06:58:05Z

Thanks, I changed it.

lpizzinidev

Thanks 👍

scanlonp

Any reason to not have an integ test for the StepFunctionsRestApi invocation? Otherwise looks good to go.

Pull request has been modified.

go-to-k · 2023-10-26T04:56:35Z

@scanlonp

I was satisfied with just the StepFunctionsIntegration.startExecution test. I added that integ test.

I did not want to interfere with the behavior of the existing StepFunctionsRestApi test with useDefaultMethodResponses is true as a default value, so I added a new test.

scanlonp

Great, thanks for the new integ test. The optional parameter is a good way to disable automatic behavior, and the tests cover both the template validation and deploy success.

mergify · 2023-10-26T13:58:44Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2023-10-26T14:29:12Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: 257285a
Result: SUCCEEDED
Build Logs (available for 30 days)

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

mergify · 2023-10-26T14:29:36Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

…tionsIntegrations (#27645) In `StepFunctionsIntegration`, method response models (`methodResponses`) with 200, 400 and 500 status codes are added by default. This PR adds `useDefaultMethodResponses` property not to add default method response models by set it to `false`. Closes #27520. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

feat(apigateway): add useDefaultMethodResponses

f4a4724

aws-cdk-automation requested a review from a team October 23, 2023 07:45

github-actions bot added admired-contributor [Pilot] contributed between 13-24 PRs to the CDK effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2 labels Oct 23, 2023

aws-cdk-automation previously requested changes Oct 23, 2023

View reviewed changes

go-to-k added 3 commits October 23, 2023 17:14

change doc

cd34e90

apply for stepfunctions-api

80c9a75

change README

2ef1079

change README for statements

f641b81

go-to-k marked this pull request as ready for review October 23, 2023 09:25

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Oct 23, 2023

lpizzinidev suggested changes Oct 24, 2023

View reviewed changes

chore: change for the review

e6617fb

fix: README for rosetta

28f60f2

lpizzinidev suggested changes Oct 25, 2023

View reviewed changes

change an unit test

dd49136

lpizzinidev approved these changes Oct 25, 2023

View reviewed changes

aws-cdk-automation added pr/needs-maintainer-review This PR needs a review from a Core Team Member and removed pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. labels Oct 25, 2023

scanlonp previously requested changes Oct 25, 2023

View reviewed changes

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Oct 25, 2023

test: integ test for StepFunctionsRestApi

5fa96b6

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Oct 26, 2023

scanlonp approved these changes Oct 26, 2023

View reviewed changes

Merge branch 'main' into feat/apigateway-useDefaultMethodResponses

257285a

scanlonp self-assigned this Oct 26, 2023

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Oct 26, 2023

mergify bot merged commit 22a3234 into aws:main Oct 26, 2023
12 checks passed

github-actions bot mentioned this pull request Nov 1, 2023

Monthly PR metrics report - October 2023 #27786

Closed

go-to-k deleted the feat/apigateway-useDefaultMethodResponses branch November 23, 2023 01:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(apigateway): add useDefaultMethodResponses property for StepFunctionsIntegrations #27645

feat(apigateway): add useDefaultMethodResponses property for StepFunctionsIntegrations #27645

go-to-k commented Oct 23, 2023 •

edited

aws-cdk-automation left a comment

lpizzinidev left a comment

lpizzinidev Oct 24, 2023

lpizzinidev Oct 24, 2023

lpizzinidev Oct 24, 2023

lpizzinidev Oct 24, 2023

lpizzinidev Oct 24, 2023

go-to-k commented Oct 25, 2023

lpizzinidev left a comment

lpizzinidev Oct 25, 2023

go-to-k commented Oct 25, 2023

lpizzinidev left a comment

scanlonp left a comment

go-to-k commented Oct 26, 2023

scanlonp left a comment

mergify bot commented Oct 26, 2023

aws-cdk-automation commented Oct 26, 2023

mergify bot commented Oct 26, 2023

		private readonly useDefaultMethodResponses: boolean;
		constructor(stateMachine: sfn.IStateMachine, options: StepFunctionsExecutionIntegrationOptions = {}) {

	* Enable to add default method response models with 200, 400 and 500 status codes.
	* Whether to add default response models with 200, 400, and 500 status codes to the method.

feat(apigateway): add useDefaultMethodResponses property for StepFunctionsIntegrations #27645

feat(apigateway): add useDefaultMethodResponses property for StepFunctionsIntegrations #27645

Conversation

go-to-k commented Oct 23, 2023 • edited

aws-cdk-automation left a comment

Choose a reason for hiding this comment

lpizzinidev left a comment

Choose a reason for hiding this comment

lpizzinidev Oct 24, 2023

Choose a reason for hiding this comment

lpizzinidev Oct 24, 2023

Choose a reason for hiding this comment

lpizzinidev Oct 24, 2023

Choose a reason for hiding this comment

lpizzinidev Oct 24, 2023

Choose a reason for hiding this comment

lpizzinidev Oct 24, 2023

Choose a reason for hiding this comment

go-to-k commented Oct 25, 2023

lpizzinidev left a comment

Choose a reason for hiding this comment

lpizzinidev Oct 25, 2023

Choose a reason for hiding this comment

go-to-k commented Oct 25, 2023

lpizzinidev left a comment

Choose a reason for hiding this comment

scanlonp left a comment

Choose a reason for hiding this comment

go-to-k commented Oct 26, 2023

scanlonp left a comment

Choose a reason for hiding this comment

mergify bot commented Oct 26, 2023

aws-cdk-automation commented Oct 26, 2023

AWS CodeBuild CI Report

mergify bot commented Oct 26, 2023

go-to-k commented Oct 23, 2023 •

edited