mbonig / state-machine

a CDK construct for Step Functions State Machines that allows for easy integration with Workflow Studio
MIT License
40 stars 4 forks source link

Attempting to override Array elements causes new elements to be created #9

Closed a-bigelow closed 2 years ago

a-bigelow commented 2 years ago

I'm trying to override some parameters in multiple members of a Branches array. Attempting to override the parameters by simply providing providing my own array is causing duplicate members to be created, rather than overriding the specified values on the existing array members.

Given this ASL array:

ASL Array ```json "Branches": [ { "StartAt": "ResumeCluster", "States": { "ResumeCluster": { "Type": "Task", "Parameters": { "ClusterIdentifier": "MyData" }, "Resource": "arn:aws:states:::aws-sdk:redshift:resumeCluster", "Next": "DescribeClusters" }, "DescribeClusters": { "Type": "Task", "Parameters": { "ClusterIdentifier": "" }, "Resource": "arn:aws:states:::aws-sdk:redshift:describeClusters", "Next": "Evaluate Cluster Status" }, "Evaluate Cluster Status": { "Type": "Choice", "Choices": [ { "Variable": "$.Clusters[0].ClusterStatus", "StringEquals": "available", "Next": "Redshift Pass" } ], "Default": "Redshift Wait" }, "Redshift Pass": { "Type": "Pass", "End": true }, "Redshift Wait": { "Type": "Wait", "Seconds": 5, "Next": "DescribeClusters" } } }, { "StartAt": "StartInstances", "States": { "StartInstances": { "Type": "Task", "Parameters": { "InstanceIds": [ "MyData" ] }, "Resource": "arn:aws:states:::aws-sdk:ec2:startInstances", "Next": "DescribeInstanceStatus" }, "DescribeInstanceStatus": { "Type": "Task", "Next": "Evaluate Instance Status", "Parameters": { "InstanceIds": [ "MyData" ] }, "Resource": "arn:aws:states:::aws-sdk:ec2:describeInstanceStatus" }, "Evaluate Instance Status": { "Type": "Choice", "Choices": [ { "And": [ { "Variable": "$.InstanceStatuses[0].InstanceState.Name", "StringEquals": "running" }, { "Variable": "$.InstanceStatuses[0].SystemStatus.Details[0].Status", "StringEquals": "passed" }, { "Variable": "$.InstanceStatuses[0].InstanceStatus.Details[0].Status", "StringEquals": "passed" } ], "Next": "EC2 Pass" } ], "Default": "EC2 Wait" }, "EC2 Pass": { "Type": "Pass", "End": true }, "EC2 Wait": { "Type": "Wait", "Seconds": 5, "Next": "DescribeInstanceStatus" } } } ] ```

And this override: (Stripped to just what's relevant)

Override ```python "Branches": [{ "StartAt": "ResumeCluster", "States": { "ResumeCluster": { "Parameters": { "ClusterIdentifier": "CLUSTER_NAME" # TODO Sub this out } }, "DescribeClusters": { "Parameters": { "ClusterIdentifier": "CLUSTER_NAME" # TODO Sub this out } }, } }, { "StartAt": "StartInstances", "States": { "StartInstances": { "Parameters": { "InstanceIds": ["INSTANCE_ID"] # TODO Sub this out } }, "DescribeInstanceStatus": { "Parameters": { "InstanceIds": ["INSTANCE_ID"] # TODO Sub this out } } } }] ```

I'm left with this JSON in the state language definition. You can see clearly that it's appended the overrides as additional members rather than overriding the existing ones:

Resulting ASL ```json "Branches": [ { "StartAt": "ResumeCluster", "States": { "ResumeCluster": { "Type": "Task", "Parameters": { "ClusterIdentifier": "MyData" }, "Resource": "arn:aws:states:::aws-sdk:redshift:resumeCluster", "Next": "DescribeClusters" }, "DescribeClusters": { "Type": "Task", "Parameters": { "ClusterIdentifier": "" }, "Resource": "arn:aws:states:::aws-sdk:redshift:describeClusters", "Next": "Evaluate Cluster Status" }, "Evaluate Cluster Status": { "Type": "Choice", "Choices": [ { "Variable": "$.Clusters[0].ClusterStatus", "StringEquals": "available", "Next": "Redshift Pass" } ], "Default": "Redshift Wait" }, "Redshift Pass": { "Type": "Pass", "End": true }, "Redshift Wait": { "Type": "Wait", "Seconds": 5, "Next": "DescribeClusters" } } }, { "StartAt": "StartInstances", "States": { "StartInstances": { "Type": "Task", "Parameters": { "InstanceIds": [ "MyData" ] }, "Resource": "arn:aws:states:::aws-sdk:ec2:startInstances", "Next": "DescribeInstanceStatus" }, "DescribeInstanceStatus": { "Type": "Task", "Next": "Evaluate Instance Status", "Parameters": { "InstanceIds": [ "MyData" ] }, "Resource": "arn:aws:states:::aws-sdk:ec2:describeInstanceStatus" }, "Evaluate Instance Status": { "Type": "Choice", "Choices": [ { "And": [ { "Variable": "$.InstanceStatuses[0].InstanceState.Name", "StringEquals": "running" }, { "Variable": "$.InstanceStatuses[0].SystemStatus.Details[0].Status", "StringEquals": "passed" }, { "Variable": "$.InstanceStatuses[0].InstanceStatus.Details[0].Status", "StringEquals": "passed" } ], "Next": "EC2 Pass" } ], "Default": "EC2 Wait" }, "EC2 Pass": { "Type": "Pass", "End": true }, "EC2 Wait": { "Type": "Wait", "Seconds": 5, "Next": "DescribeInstanceStatus" } } }, { "StartAt": "ResumeCluster", "States": { "ResumeCluster": { "Parameters": { "ClusterIdentifier": "CLUSTER_NAME" } }, "DescribeClusters": { "Parameters": { "ClusterIdentifier": "CLUSTER_NAME" } } } }, { "StartAt": "StartInstances", "States": { "StartInstances": { "Parameters": { "InstanceIds": [ "INSTANCE_ID" ] } }, "DescribeInstanceStatus": { "Parameters": { "InstanceIds": [ "INSTANCE_ID" ] } } } } ] ```

The expected behavior would be for the construct to drill into the existing array members in-order (since I have no way to target an array element via JMES or anything like that) and perform the overrides.

a-bigelow commented 2 years ago

@mbonig The merge method from lodash handles arrays fairly well, but it still requires you to define the other objects in an array members up-to-and-including the element you're trying to override.

Basic implementation on the construct:

export class StateMachine extends aws_stepfunctions.StateMachine {
  /**
   * The result of the lodash merge performed on the definition and override props.
   * Used for testing purposes.
   */
  public mergedResult: any;

  constructor(scope: Construct, id: string, props: StateMachineProps) {
    super(scope, id, {
      ...props,
      definition: new Pass(scope, 'THISWILLBEDELETEDRIGHTAWAY'),
    });
    scope.node.tryRemoveChild('THISWILLBEDELETEDRIGHTAWAY');

    this.mergedResult = _.merge(props.definition, props.overrides)

    (this.node.defaultChild as CfnStateMachine).definitionString = JSON.stringify(this.mergedResult);
  }
}

The result of the example from this issue:

{
   "Branches":[
      {
         "StartAt":"ResumeCluster",
         "States":{
            "ResumeCluster":{
               "Type":"Task",
               "Parameters":{
                  "ClusterIdentifier":"CLUSTER_NAME"
               },
               "Resource":"arn:aws:states:::aws-sdk:redshift:resumeCluster",
               "Next":"DescribeClusters"
            },
            "DescribeClusters":{
               "Type":"Task",
               "Parameters":{
                  "ClusterIdentifier":"CLUSTER_NAME"
               },
               "Resource":"arn:aws:states:::aws-sdk:redshift:describeClusters",
               "Next":"Evaluate Cluster Status"
            },
            "Evaluate Cluster Status":{
               "Type":"Choice",
               "Choices":[
                  {
                     "Variable":"$.Clusters[0].ClusterStatus",
                     "StringEquals":"available",
                     "Next":"Redshift Pass"
                  }
               ],
               "Default":"Redshift Wait"
            },
            "Redshift Pass":{
               "Type":"Pass",
               "End":true
            },
            "Redshift Wait":{
               "Type":"Wait",
               "Seconds":5,
               "Next":"DescribeClusters"
            }
         }
      },
      {
         "StartAt":"StartInstances",
         "States":{
            "StartInstances":{
               "Type":"Task",
               "Parameters":{
                  "InstanceIds":[
                     "INSTANCE_ID"
                  ]
               },
               "Resource":"arn:aws:states:::aws-sdk:ec2:startInstances",
               "Next":"DescribeInstanceStatus"
            },
            "DescribeInstanceStatus":{
               "Type":"Task",
               "Next":"Evaluate Instance Status",
               "Parameters":{
                  "InstanceIds":[
                     "INSTANCE_ID"
                  ]
               },
               "Resource":"arn:aws:states:::aws-sdk:ec2:describeInstanceStatus"
            },
            "Evaluate Instance Status":{
               "Type":"Choice",
               "Choices":[
                  {
                     "And":[
                        {
                           "Variable":"$.InstanceStatuses[0].InstanceState.Name",
                           "StringEquals":"running"
                        },
                        {
                           "Variable":"$.InstanceStatuses[0].SystemStatus.Details[0].Status",
                           "StringEquals":"passed"
                        },
                        {
                           "Variable":"$.InstanceStatuses[0].InstanceStatus.Details[0].Status",
                           "StringEquals":"passed"
                        }
                     ],
                     "Next":"EC2 Pass"
                  }
               ],
               "Default":"EC2 Wait"
            },
            "EC2 Pass":{
               "Type":"Pass",
               "End":true
            },
            "EC2 Wait":{
               "Type":"Wait",
               "Seconds":5,
               "Next":"DescribeInstanceStatus"
            }
         }
      }
   ]
}

Arrays will still be tricky and human-error prone with this. It might be worth considering moving to an iterable approach, similarly to how the yaml module handles overrides with the setIn() method