From f7fcbd785fd5c195bb837758e6e704f4e0ea7f34 Mon Sep 17 00:00:00 2001 From: sakurai-ryo Date: Wed, 15 May 2024 02:16:25 +0900 Subject: [PATCH 1/5] fix: failed to use intrinsic func in Fail state --- .../aws-cdk-lib/aws-stepfunctions/README.md | 10 ++++ .../aws-stepfunctions/lib/states/fail.ts | 34 ++++++++++- .../test/state-machine-resources.test.ts | 57 ++++++++++++++++++- 3 files changed, 97 insertions(+), 4 deletions(-) diff --git a/packages/aws-cdk-lib/aws-stepfunctions/README.md b/packages/aws-cdk-lib/aws-stepfunctions/README.md index 3b709afca60ae..db4d5cce70e09 100644 --- a/packages/aws-cdk-lib/aws-stepfunctions/README.md +++ b/packages/aws-cdk-lib/aws-stepfunctions/README.md @@ -487,6 +487,16 @@ const fail = new sfn.Fail(this, 'Fail', { }); ``` +You can also use an intrinsic function that returns a string to specify CausePath and ErrorPath. +The available functions include States.Format, States.JsonToString, States.ArrayGetItem, States.Base64Encode, States.Base64Decode, States.Hash, and States.UUID. + +```ts +const fail = new sfn.Fail(this, 'Fail', { + errorPath: sfn.JsonPath.format('error: {}.', stepfunctions.JsonPath.stringAt('$.someError')), + causePath: "States.Format('cause: {}.', $.someCause)", +}); +``` + ### Map A `Map` state can be used to run a set of steps for each element of an input array. diff --git a/packages/aws-cdk-lib/aws-stepfunctions/lib/states/fail.ts b/packages/aws-cdk-lib/aws-stepfunctions/lib/states/fail.ts index efe067c8ba3a5..15e687c23fc5a 100644 --- a/packages/aws-cdk-lib/aws-stepfunctions/lib/states/fail.ts +++ b/packages/aws-cdk-lib/aws-stepfunctions/lib/states/fail.ts @@ -1,6 +1,7 @@ import { Construct } from 'constructs'; import { StateType } from './private/state-type'; import { renderJsonPath, State } from './state'; +import { Token } from '../../../core'; import { INextable } from '../types'; /** @@ -31,6 +32,9 @@ export interface FailProps { /** * JsonPath expression to select part of the state to be the error to this state. * + * You can also use an intrinsic function that returns a string to specify this property. + * The allowed functions include States.Format, States.JsonToString, States.ArrayGetItem, States.Base64Encode, States.Base64Decode, States.Hash, and States.UUID. + * * @default - No error path */ readonly errorPath?: string; @@ -45,6 +49,9 @@ export interface FailProps { /** * JsonPath expression to select part of the state to be the cause to this state. * + * You can also use an intrinsic function that returns a string to specify this property. + * The allowed functions include States.Format, States.JsonToString, States.ArrayGetItem, States.Base64Encode, States.Base64Decode, States.Hash, and States.UUID. + * * @default - No cause path */ readonly causePath?: string; @@ -80,9 +87,30 @@ export class Fail extends State { Type: StateType.FAIL, Comment: this.comment, Error: this.error, - ErrorPath: renderJsonPath(this.errorPath), + ErrorPath: this.isIntrinsicString(this.errorPath) ? this.errorPath : renderJsonPath(this.errorPath), Cause: this.cause, - CausePath: renderJsonPath(this.causePath), + CausePath: this.isIntrinsicString(this.causePath) ? this.causePath : renderJsonPath(this.causePath), }; } -} \ No newline at end of file + + /** + * Validate this state + */ + protected validateState(): string[] { + const errors = super.validateState(); + + if (this.error && this.errorPath) { + errors.push('Fail state cannot have both error and errorPath'); + } + + if (this.cause && this.causePath) { + errors.push('Fail state cannot have both cause and causePath'); + } + + return errors; + } + + private isIntrinsicString(jsonPath?: string): boolean { + return !Token.isUnresolved(jsonPath) && !jsonPath?.startsWith('$'); + } +} diff --git a/packages/aws-cdk-lib/aws-stepfunctions/test/state-machine-resources.test.ts b/packages/aws-cdk-lib/aws-stepfunctions/test/state-machine-resources.test.ts index 69d0a24e1a5d0..ae9eae5a674c1 100644 --- a/packages/aws-cdk-lib/aws-stepfunctions/test/state-machine-resources.test.ts +++ b/packages/aws-cdk-lib/aws-stepfunctions/test/state-machine-resources.test.ts @@ -96,6 +96,62 @@ describe('State Machine Resources', () => { }); }), + test.each([ + [ + "States.Format('error: {}.', $.error)", + "States.Format('cause: {}.', $.cause)", + ], + [ + stepfunctions.JsonPath.format('error: {}.', stepfunctions.JsonPath.stringAt('$.error')), + stepfunctions.JsonPath.format('cause: {}.', stepfunctions.JsonPath.stringAt('$.cause')), + ], + ])('Fail should render ErrorPath / CausePath correctly when specifying CausePath using intrinsics', (errorPath, causePath) => { + // GIVEN + const stack = new cdk.Stack(); + const fail = new stepfunctions.Fail(stack, 'Fail', { + errorPath, + causePath, + }); + + // WHEN + const failState = stack.resolve(fail.toStateJson()); + + // THEN + expect(failState).toStrictEqual({ + CausePath: "States.Format('cause: {}.', $.cause)", + ErrorPath: "States.Format('error: {}.', $.error)", + Type: 'Fail', + }); + }), + + test('fails in synthesis if error and errorPath are defined in Fail state', () => { + // GIVEN + const app = new cdk.App(); + const stack = new cdk.Stack(app); + + // WHEN + new stepfunctions.Fail(stack, 'Fail', { + error: 'error', + errorPath: '$.error', + }); + + expect(() => app.synth()).toThrow(/Fail state cannot have both error and errorPath/); + }), + + test('fails in synthesis if cause and causePath are defined in Fail state', () => { + // GIVEN + const app = new cdk.App(); + const stack = new cdk.Stack(app); + + // WHEN + new stepfunctions.Fail(stack, 'Fail', { + cause: 'cause', + causePath: '$.cause', + }); + + expect(() => app.synth()).toThrow(/Fail state cannot have both cause and causePath/); + }), + testDeprecated('Task should render InputPath / Parameters / OutputPath correctly', () => { // GIVEN const stack = new cdk.Stack(); @@ -721,7 +777,6 @@ describe('State Machine Resources', () => { ], }); }); - }); interface FakeTaskProps extends stepfunctions.TaskStateBaseProps { From 3604180b5affae44dde7f97f1a4687cdfb4a841d Mon Sep 17 00:00:00 2001 From: sakurai-ryo Date: Wed, 15 May 2024 02:51:10 +0900 Subject: [PATCH 2/5] fix: build error --- packages/aws-cdk-lib/aws-stepfunctions/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/aws-cdk-lib/aws-stepfunctions/README.md b/packages/aws-cdk-lib/aws-stepfunctions/README.md index db4d5cce70e09..6f85c657e05c4 100644 --- a/packages/aws-cdk-lib/aws-stepfunctions/README.md +++ b/packages/aws-cdk-lib/aws-stepfunctions/README.md @@ -492,7 +492,7 @@ The available functions include States.Format, States.JsonToString, States.Array ```ts const fail = new sfn.Fail(this, 'Fail', { - errorPath: sfn.JsonPath.format('error: {}.', stepfunctions.JsonPath.stringAt('$.someError')), + errorPath: sfn.JsonPath.format('error: {}.', sfn.JsonPath.stringAt('$.someError')), causePath: "States.Format('cause: {}.', $.someCause)", }); ``` From b0efe952fce25b45b57b72178118669d78623b09 Mon Sep 17 00:00:00 2001 From: sakurai-ryo Date: Wed, 15 May 2024 22:59:51 +0900 Subject: [PATCH 3/5] fix: test case name --- .../aws-stepfunctions/test/state-machine-resources.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/aws-cdk-lib/aws-stepfunctions/test/state-machine-resources.test.ts b/packages/aws-cdk-lib/aws-stepfunctions/test/state-machine-resources.test.ts index ae9eae5a674c1..c57b6e5a2b5b4 100644 --- a/packages/aws-cdk-lib/aws-stepfunctions/test/state-machine-resources.test.ts +++ b/packages/aws-cdk-lib/aws-stepfunctions/test/state-machine-resources.test.ts @@ -105,7 +105,7 @@ describe('State Machine Resources', () => { stepfunctions.JsonPath.format('error: {}.', stepfunctions.JsonPath.stringAt('$.error')), stepfunctions.JsonPath.format('cause: {}.', stepfunctions.JsonPath.stringAt('$.cause')), ], - ])('Fail should render ErrorPath / CausePath correctly when specifying CausePath using intrinsics', (errorPath, causePath) => { + ])('Fail should render ErrorPath / CausePath correctly when specifying ErrorPath / CausePath using intrinsics', (errorPath, causePath) => { // GIVEN const stack = new cdk.Stack(); const fail = new stepfunctions.Fail(stack, 'Fail', { From 5ee2537340e6b61cf0a130f7c3f5fde81db0618a Mon Sep 17 00:00:00 2001 From: sakurai-ryo Date: Thu, 30 May 2024 23:18:58 +0900 Subject: [PATCH 4/5] add: test for validation error --- .../aws-stepfunctions/lib/states/fail.ts | 22 +++++++++++++++ .../test/state-machine-resources.test.ts | 27 ++++++++++++++++++- 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/packages/aws-cdk-lib/aws-stepfunctions/lib/states/fail.ts b/packages/aws-cdk-lib/aws-stepfunctions/lib/states/fail.ts index 15e687c23fc5a..13bc5c702d2a8 100644 --- a/packages/aws-cdk-lib/aws-stepfunctions/lib/states/fail.ts +++ b/packages/aws-cdk-lib/aws-stepfunctions/lib/states/fail.ts @@ -63,6 +63,16 @@ export interface FailProps { * Reaching a Fail state terminates the state execution in failure. */ export class Fail extends State { + private static allowedIntrinsics = [ + 'States.Format', + 'States.JsonToString', + 'States.ArrayGetItem', + 'States.Base64Encode', + 'States.Base64Decode', + 'States.Hash', + 'States.UUID', + ]; + public readonly endStates: INextable[] = []; private readonly error?: string; @@ -99,6 +109,14 @@ export class Fail extends State { protected validateState(): string[] { const errors = super.validateState(); + if (this.errorPath && this.isIntrinsicString(this.errorPath) && !this.isAllowedIntrinsic(this.errorPath)) { + errors.push(`You must specify a valid intrinsic function in errorPath. Must be one of ${Fail.allowedIntrinsics.join(', ')}`); + } + + if (this.causePath && this.isIntrinsicString(this.causePath) && !this.isAllowedIntrinsic(this.causePath)) { + errors.push(`You must specify a valid intrinsic function in causePath. Must be one of ${Fail.allowedIntrinsics.join(', ')}`); + } + if (this.error && this.errorPath) { errors.push('Fail state cannot have both error and errorPath'); } @@ -113,4 +131,8 @@ export class Fail extends State { private isIntrinsicString(jsonPath?: string): boolean { return !Token.isUnresolved(jsonPath) && !jsonPath?.startsWith('$'); } + + private isAllowedIntrinsic(intrinsic: string): boolean { + return Fail.allowedIntrinsics.some(allowed => intrinsic.startsWith(allowed)); + } } diff --git a/packages/aws-cdk-lib/aws-stepfunctions/test/state-machine-resources.test.ts b/packages/aws-cdk-lib/aws-stepfunctions/test/state-machine-resources.test.ts index c57b6e5a2b5b4..3ca0a4638b524 100644 --- a/packages/aws-cdk-lib/aws-stepfunctions/test/state-machine-resources.test.ts +++ b/packages/aws-cdk-lib/aws-stepfunctions/test/state-machine-resources.test.ts @@ -107,7 +107,8 @@ describe('State Machine Resources', () => { ], ])('Fail should render ErrorPath / CausePath correctly when specifying ErrorPath / CausePath using intrinsics', (errorPath, causePath) => { // GIVEN - const stack = new cdk.Stack(); + const app = new cdk.App(); + const stack = new cdk.Stack(app); const fail = new stepfunctions.Fail(stack, 'Fail', { errorPath, causePath, @@ -122,6 +123,7 @@ describe('State Machine Resources', () => { ErrorPath: "States.Format('error: {}.', $.error)", Type: 'Fail', }); + expect(() => app.synth()).not.toThrow(); }), test('fails in synthesis if error and errorPath are defined in Fail state', () => { @@ -152,6 +154,29 @@ describe('State Machine Resources', () => { expect(() => app.synth()).toThrow(/Fail state cannot have both cause and causePath/); }), + test.each([ + 'States.Array($.Id)', + 'States.ArrayPartition($.inputArray, 4)', + 'States.ArrayContains($.inputArray, $.lookingFor)', + 'States.ArrayRange(1, 9, 2)', + 'States.ArrayLength($.inputArray)', + 'States.JsonMerge($.json1, $.json2, false)', + 'States.StringToJson($.escapedJsonString)', + ])('fails in synthesis if specifying invalid intrinsic functions in the causePath and errorPath (%s)', (intrinsic) => { + // GIVEN + const app = new cdk.App(); + const stack = new cdk.Stack(app); + + // WHEN + new stepfunctions.Fail(stack, 'Fail', { + causePath: intrinsic, + errorPath: intrinsic, + }); + + expect(() => app.synth()).toThrow(/You must specify a valid intrinsic function in causePath. Must be one of States.Format, States.JsonToString, States.ArrayGetItem, States.Base64Encode, States.Base64Decode, States.Hash, States.UUID/); + expect(() => app.synth()).toThrow(/You must specify a valid intrinsic function in errorPath. Must be one of States.Format, States.JsonToString, States.ArrayGetItem, States.Base64Encode, States.Base64Decode, States.Hash, States.UUID/); + }), + testDeprecated('Task should render InputPath / Parameters / OutputPath correctly', () => { // GIVEN const stack = new cdk.Stack(); From 60c1bbafc5cd991a1f6948f808eaa7adaa0798b9 Mon Sep 17 00:00:00 2001 From: sakurai-ryo Date: Thu, 30 May 2024 23:25:34 +0900 Subject: [PATCH 5/5] chore: add a test case --- .../aws-stepfunctions/test/state-machine-resources.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/aws-cdk-lib/aws-stepfunctions/test/state-machine-resources.test.ts b/packages/aws-cdk-lib/aws-stepfunctions/test/state-machine-resources.test.ts index 3ca0a4638b524..62b088ebe048e 100644 --- a/packages/aws-cdk-lib/aws-stepfunctions/test/state-machine-resources.test.ts +++ b/packages/aws-cdk-lib/aws-stepfunctions/test/state-machine-resources.test.ts @@ -162,6 +162,7 @@ describe('State Machine Resources', () => { 'States.ArrayLength($.inputArray)', 'States.JsonMerge($.json1, $.json2, false)', 'States.StringToJson($.escapedJsonString)', + 'plainString', ])('fails in synthesis if specifying invalid intrinsic functions in the causePath and errorPath (%s)', (intrinsic) => { // GIVEN const app = new cdk.App();