From beaa790ef9994d15fc54142fa2cb9976e7583aac Mon Sep 17 00:00:00 2001 From: "k.goto" <24818752+go-to-k@users.noreply.github.com> Date: Wed, 25 Oct 2023 05:19:02 +0900 Subject: [PATCH] chore(stepfunctions-tasks): validate cases of action and parameters in CallAwsService (#27635) This PR adds the following validations in CallAwsService. - `action` must be camelCase. - parameter names in `parameters` must be PascalCase. See the doc: https://docs.aws.amazon.com/step-functions/latest/dg/supported-services-awssdk.html > The API action will always be camel case, and parameter names will be Pascal case. For example, you could use Step Functions API action startSyncExecution and specify its parameter as StateMachineArn. CloudFormation fails with a following error if there are not these validations. ``` Deployment failed: Error: The stack named aws-stepfunctions-tasks-call-aws-service-logs-integ failed to deploy: UPDATE_ROLLBACK_COMPLETE: Resource handler returned message: "Invalid State Machine Definition: 'SCHEMA_VALIDATION_FAILED: The resource provided arn:aws:states:::aws-sdk:cloudwatchlogs:CreateLogStream is not recognized. The value is not a valid resource ARN, or the resource is not available in this region. at /States/SendTaskSuccess/Resource' (Service: AWSStepFunctions; Status Code: 400; Error Code: InvalidDefinition; Request ID: xxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxx; Proxy: null)" (RequestToken: xxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxx, HandlerErrorCode: InvalidRequest) ``` I think it is a good thing to make these errors in the synth phase, since there were actually cases of confusion as follows. https://github.com/aws/aws-cdk/pull/27623#issuecomment-1773761113 I also thought to not validate but translate to camel (or pascal) cases. However I thought it would allow input that violates the explanation defined in the API documentation, so I decided not to. On the other hands, the `action` is also used for IAM actions so the IAM actions will be to camel cases (like `logs:createLogStream`). But I allowed it because IAM actions are case insensitive. If a translation is a better way to do it rather than the validation, I will consider that as well. https://github.com/aws/aws-cdk/blob/09c809b52fd2eeb27ac5bbc91d425ecf54e31bf9/packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/aws-sdk/call-aws-service.ts#L92-L94 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../lib/aws-sdk/call-aws-service.ts | 9 +++++++ .../test/aws-sdk/call-aws-service.test.ts | 24 +++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/aws-sdk/call-aws-service.ts b/packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/aws-sdk/call-aws-service.ts index 556c0becdac08..16d37f7fb1aec 100644 --- a/packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/aws-sdk/call-aws-service.ts +++ b/packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/aws-sdk/call-aws-service.ts @@ -79,6 +79,15 @@ export class CallAwsService extends sfn.TaskStateBase { if (this.props.integrationPattern === sfn.IntegrationPattern.RUN_JOB) { throw new Error('The RUN_JOB integration pattern is not supported for CallAwsService'); } + if (!Token.isUnresolved(this.props.action) && !this.props.action.startsWith(this.props.action[0]?.toLowerCase())) { + throw new Error(`action must be camelCase, got: ${this.props.action}`); + } + if (this.props.parameters) { + const invalidKeys = Object.keys(this.props.parameters).filter(key => !key.startsWith(key[0]?.toUpperCase())); + if (invalidKeys.length) { + throw new Error(`parameter names must be PascalCase, got: ${invalidKeys.join(', ')}`); + } + } const iamServiceMap: Record = { sfn: 'states', diff --git a/packages/aws-cdk-lib/aws-stepfunctions-tasks/test/aws-sdk/call-aws-service.test.ts b/packages/aws-cdk-lib/aws-stepfunctions-tasks/test/aws-sdk/call-aws-service.test.ts index cd5ac68818c15..3eceee2639073 100644 --- a/packages/aws-cdk-lib/aws-stepfunctions-tasks/test/aws-sdk/call-aws-service.test.ts +++ b/packages/aws-cdk-lib/aws-stepfunctions-tasks/test/aws-sdk/call-aws-service.test.ts @@ -161,6 +161,30 @@ test('throws with invalid integration pattern', () => { })).toThrow(/The RUN_JOB integration pattern is not supported for CallAwsService/); }); +test('throws if action is not camelCase', () => { + expect(() => new tasks.CallAwsService(stack, 'GetObject', { + service: 's3', + action: 'GetObject', + parameters: { + Bucket: 'my-bucket', + Key: sfn.JsonPath.stringAt('$.key'), + }, + iamResources: ['*'], + })).toThrow(/action must be camelCase, got: GetObject/); +}); + +test('throws if parameters has keys as not PascalCase', () => { + expect(() => new tasks.CallAwsService(stack, 'GetObject', { + service: 's3', + action: 'getObject', + parameters: { + bucket: 'my-bucket', + key: sfn.JsonPath.stringAt('$.key'), + }, + iamResources: ['*'], + })).toThrow(/parameter names must be PascalCase, got: bucket, key/); +}); + test('can pass additional IAM statements', () => { // WHEN const task = new tasks.CallAwsService(stack, 'DetectLabels', {