(stepfunctions): the service does not support JSON paths for MaxConcurrency on a Map state but CDK can generate one

wvauclain commented 2 years ago

Describe the bug

Allowing JSON paths for MaxConcurrency was added in response to https://github.com/aws/aws-cdk/issues/20152, but JSON paths are not actually supported for the MaxConcurrency key in a Map state by Step Functions.

Expected Behavior

I would expect synth to fail in the reproduction example.

Current Behavior

Synth succeeds, but deploying fails with the following error:

11:31:04 AM | CREATE_FAILED        | AWS::StepFunctions::StateMachine | StateMachine2E01A3A5
Resource handler returned message: "Invalid State Machine Definition: 'SCHEMA_VALIDATION_FAILED: Expected value of type Integer at /States/Map State/MaxConcurrency'"

Reproduction Steps

import { Duration, Stack, StackProps } from 'aws-cdk-lib';
import * as stepfunctions from 'aws-cdk-lib/aws-stepfunctions';
import { Construct } from 'constructs';

export class SfReproStack extends Stack {
  constructor(scope: Construct, id: string, props?: StackProps) {
    super(scope, id, props);

    const map = new stepfunctions.Map(this, 'Map State', {
      maxConcurrency: stepfunctions.JsonPath.numberAt('$.maxConcurrency'),
      itemsPath: stepfunctions.JsonPath.stringAt('$.inputForMap'),
    });
    map.iterator(new stepfunctions.Pass(this, 'Pass State'));

    const machine = new stepfunctions.StateMachine(this, "StateMachine", {
      definition: map,
      stateMachineName: 'ReproStateMachine',
      timeout: Duration.minutes(5),
    });
  }
}

Possible Solution

I think https://github.com/aws/aws-cdk/pull/20279 should be reverted until Step Functions supports this.

Additional Information/Context

No response

CDK CLI Version

2.28.1 (build d035432)

Framework Version

No response

Node.js Version

v18.4.0

OS

macOS 12.3.1

Language

Typescript

Language Version

No response

Other information

No response

kaizencc commented 2 years ago

Hi @wvauclain! all #20279 was not invalidate on tokens, so it should stay. but i think you are right, max concurrency does not support json paths so that is bad. The fix here is that we should check to see if we've been given a json path and throw a more descriptive error in that case. Thanks for bringing this to our attention!

connorgiles commented 9 months ago

Also running into this issue. It appears it might need to be provided as MaxConcurrencyPath instead of MaxConcurrency based on these docs.

Is there any reason to not want to expose the additional ASL property?

abdelnn commented 6 months ago

Update: I've added support for MaxConcurrencyPath in #29330 , however that change does not include invalidating JsonPath values for MaxConcurrency

aws / aws-cdk