fix(stepfunctions-tasks): allow passing apiEndpoint as intrinsic function

[MathieuGilbert]

Issue #29925

Closes #29925, #30749.

Reason for this change

The apiEndpoint prop currently only works when it's a string (ie. TaskInput.fromText('some/text')), with the task failing when passed as a reference (ie. TaskInput.fromText(JsonPath.format('some/text/{}', '123')). This is needed to allow for dynamic parts in the path.

Description of changes

• Change the ApiEndpoint task parameter to use the JsonPath.format intrinsic function to combine the apiRoot and apiEndpoint props, instead of basic string concatenation.
• Update README entry with more complex example.

Description of how you validated changes

• A unit test was added to cover passing formatted input.
• An integration test was added using fromJsonPathAt for the endpoint.
• A test stack was deployed with an API Gateway endpoint with basic auth Connection and was successfully invoked with dynamic payload:

    const httpInvokeTask = new HttpInvoke(this, 'HttpInvoke', {
      apiRoot: api.url,
      apiEndpoint: TaskInput.fromJsonPathAt('$.endpointName'),
      method: TaskInput.fromText('GET'),
      connection,
      outputPath: '$.ResponseBody',
    })

Checklist

• My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

---

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

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.92%. Comparing base (f1d9a7d) to head (a42d5ce).
Report is 12 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #32139   +/-   ##
=======================================
  Coverage   80.92%   80.92%           
=======================================
  Files         236      236           
  Lines       14253    14253           
  Branches     2490     2490           
=======================================
  Hits        11534    11534           
  Misses       2434     2434           
  Partials      285      285           

Flag	Coverage Δ
suite.unit	80.92% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
packages/aws-cdk	79.73% <ø> (ø)
packages/aws-cdk-lib/core	82.20% <ø> (ø)

[MathieuGilbert]

Thanks for your help in getting these changes to green @ePak. I think it was the npm test after running the old tests that got it to pass. Unfortunately I don't get much time to work on this, so it's been dragging on.

[aws-cdk-automation]

This PR has been in the MERGE CONFLICTS state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.

[aws-cdk-automation]

AWS CodeBuild CI Report

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

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

[MathieuGilbert]

I'm not sure what I'm supposed to do about generated snapshot files failing CodeQL checks.

[avanderm-pfm]

Workaround with JSONata in the mean time, tested to work with CDK 2.178.1 thanks to #28673 and #32343:

const callApi = HttpInvoke.jsonata(this, 'CallApi', {
  apiRoot: 'https://...',
  apiEndpoint: TaskInput.fromText("{% 'some/path/' & $states.input.someVariable %}"),
  connection,
  integrationPattern: IntegrationPattern.REQUEST_RESPONSE,
  method: TaskInput.fromText('GET'),
});

[samson-keung]

Left a question.

Additionally, I see that this fix changes all the apiEndpoint values to use State.Format(..). I am wondering if this may push some existing Step Function Definition over the length limit, hence, causing a breaking change to those CDK users. I understand that this may be a very edge case but I would like to get more input from the CDK team to see if we need a feature flag for this fix. I will get back asap. Thank you.

[samson-keung]

I see on line 72 above that this state definition uses JSONata. Does States.Format(...) work with JSONata?

[MathieuGilbert]

I'm not sure about how JSONata works, as that was all added in while I was trying to get the original fix in. Some brief research suggests it would not be compatible.

JsonPath and Jsonata should probably be handled separately in the construct, but that jsonPath static method was already added to just call the existing constructor, so it can't even be used for this fix now. Maybe the existing logic can be reworked without breaking newly-existing functionality.

[samson-keung]

Hi @MathieuGilbert, I have consulted other members in the CDK team. We think that this fix requires a feature flag because the current approach of the fix will introduce changes to existing CDK stacks and potentially cause problems. Other than the length limit problem, for stacks without this fix, the ApiEndpoint value would be static in the CFN template, after the fix, the value will be resolved at the Step Function run time. This has the potential of breaking customers who rely on the static value.

To move forward, I suggest implementing the fix behind feature flag. Thank you for looking into this bug and making contributions to the CDK repo.

[MathieuGilbert]

Thanks for looking at this @samson-keung. I'm not sure how much more effort I can put towards this. This is my 3rd PR since July trying to get the fix in, and attempting to contribute has been a very discouraging experience overall.

If I can justify the extra hours, I'll try figuring out feature flags, address the issue exposed by that jsonata test you commented on, and spin my wheels a bit longer on integration tests and the CI linter errors in the generated snapshot files.

[samson-keung]

@MathieuGilbert Understandable. We are currently improving our workflow to better track PRs that have been for a review for a long time.

For now, I will mark this PR as "Request changes" for tracking purposes. If there are team members available, we can pick this up.

Appreciate your effort on this. I am happy to take notes on what is most discouraging you from contributing if you want to share your thoughts. I cannot make guarantees but I will think about what I can do.

Thanks again!