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

[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 82.38%. Comparing base (e307404) to head (49d80dc).

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #32139   +/-   ##
=======================================
  Coverage   82.38%   82.38%           
=======================================
  Files         120      120           
  Lines        6937     6937           
  Branches     1170     1170           
=======================================
  Hits         5715     5715           
  Misses       1119     1119           
  Partials      103      103           

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

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

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[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.

[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!

[aws-cdk-automation]

This PR has been in the CHANGES REQUESTED state for 3 weeks, and looks abandoned. Note that PRs with failing linting check or builds are not reviewed, please ensure your build is passing

To prevent automatic closure:

• Resume work on the PR
• OR request an exemption by adding a comment containing 'Exemption Request' with justification e.x "Exemption Request: "
• OR request clarification by adding a comment containing 'Clarification Request' with a question e.x "Clarification Request: "

This PR will automatically close in 14 days if no action is taken.

[Abogical]

@Mergifyio update

[mergify]

update

☑️ Nothing to do, the required conditions are not met

• #commits-behind > 0 [📌 update requirement]
• -closed [📌 update requirement]
• -conflict [📌 update requirement]
• queue-position = -1 [📌 update requirement]

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[mergify]

This pull request has been removed from the queue for the following reason: pull request branch update failed.

The pull request can't be updated.

You should update or rebase your pull request manually. If you do, this pull request will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue, you can requeue the pull request, without updating it, by posting a @mergifyio requeue comment.

[Abogical]

@Mergifyio update

[mergify]

update

❌ Mergify doesn't have permission to update

For security reasons, Mergify can't update this pull request. Try updating locally.
GitHub response: refusing to allow a GitHub App to create or update workflow .github/workflows/codebuild-pr-build.yml without workflows permission

[Abogical]

@Mergifyio rebase

[mergify]

rebase

❌ Base branch update has failed

Git reported the following error:

Rebasing (1/12)
Rebasing (2/12)
Rebasing (3/12)
Performing inexact rename detection:  25% (1972/7712)
Performing inexact rename detection:  26% (2006/7712)
Performing inexact rename detection:  27% (2084/7712)
Performing inexact rename detection:  28% (2160/7712)
Performing inexact rename detection:  29% (2238/7712)
Performing inexact rename detection:  30% (2314/7712)
Performing inexact rename detection:  31% (2392/7712)
Performing inexact rename detection:  32% (2468/7712)
Performing inexact rename detection:  33% (2546/7712)
Performing inexact rename detection:  34% (2624/7712)
Performing inexact rename detection:  35% (2700/7712)
Performing inexact rename detection:  36% (2778/7712)
Performing inexact rename detection:  37% (2854/7712)
Performing inexact rename detection:  38% (2932/7712)
Performing inexact rename detection:  39% (3008/7712)
Performing inexact rename detection:  40% (3086/7712)
Performing inexact rename detection:  41% (3162/7712)
Performing inexact rename detection:  42% (3240/7712)
Performing inexact rename detection:  43% (3318/7712)
Performing inexact rename detection:  44% (3394/7712)
Performing inexact rename detection:  45% (3472/7712)
Performing inexact rename detection:  46% (3548/7712)
Performing inexact rename detection:  47% (3626/7712)
Performing inexact rename detection:  48% (3702/7712)
Performing inexact rename detection:  49% (3780/7712)
Performing inexact rename detection:  50% (3856/7712)
Performing inexact rename detection:  51% (3934/7712)
Performing inexact rename detection:  52% (4012/7712)
Performing inexact rename detection:  53% (4088/7712)
Performing inexact rename detection:  54% (4166/7712)
Performing inexact rename detection:  55% (4242/7712)
Performing inexact rename detection:  56% (4320/7712)
Performing inexact rename detection:  57% (4396/7712)
Performing inexact rename detection:  58% (4474/7712)
Performing inexact rename detection:  59% (4552/7712)
Performing inexact rename detection:  60% (4628/7712)
Performing inexact rename detection:  61% (4706/7712)
Performing inexact rename detection:  62% (4782/7712)
Performing inexact rename detection:  63% (4860/7712)
Performing inexact rename detection:  64% (4936/7712)
Performing inexact rename detection:  65% (5014/7712)
Performing inexact rename detection:  66% (5090/7712)
Performing inexact rename detection:  66% (5164/7712)
Performing inexact rename detection:  67% (5168/7712)
Performing inexact rename detection:  68% (5246/7712)
Performing inexact rename detection:  69% (5322/7712)
Performing inexact rename detection:  70% (5400/7712)
Performing inexact rename detection:  71% (5476/7712)
Performing inexact rename detection:  72% (5554/7712)
Performing inexact rename detection:  73% (5630/7712)
Performing inexact rename detection:  74% (5708/7712)
Performing inexact rename detection:  75% (5784/7712)
Performing inexact rename detection:  76% (5862/7712)
Performing inexact rename detection:  77% (5940/7712)
Performing inexact rename detection:  78% (6016/7712)
Performing inexact rename detection:  79% (6094/7712)
Performing inexact rename detection:  80% (6170/7712)
Performing inexact rename detection:  81% (6248/7712)
Performing inexact rename detection:  82% (6324/7712)
Performing inexact rename detection:  83% (6402/7712)
Performing inexact rename detection:  84% (6480/7712)
Performing inexact rename detection:  85% (6556/7712)
Performing inexact rename detection:  86% (6634/7712)
Performing inexact rename detection:  87% (6710/7712)
Performing inexact rename detection:  88% (6788/7712)
Performing inexact rename detection:  89% (6864/7712)
Performing inexact rename detection:  90% (6942/7712)
Performing inexact rename detection:  91% (7018/7712)
Performing inexact rename detection:  92% (7096/7712)
Performing inexact rename detection:  93% (7174/7712)
Performing inexact rename detection:  94% (7250/7712)
Performing inexact rename detection:  95% (7328/7712)
Performing inexact rename detection:  95% (7362/7712)
Performing inexact rename detection:  96% (7404/7712)
Performing inexact rename detection:  97% (7482/7712)
Performing inexact rename detection:  98% (7558/7712)
Performing inexact rename detection:  99% (7636/7712)
Performing inexact rename detection: 100% (7712/7712)
Performing inexact rename detection: 100% (7712/7712), done.
Auto-merging packages/@aws-cdk-testing/framework-integ/test/aws-stepfunctions-tasks/test/http/integ.invoke.js.snapshot/HttpInvokeTestDefaultTestDeployAssertBA1B4726.assets.json
CONFLICT (content): Merge conflict in packages/@aws-cdk-testing/framework-integ/test/aws-stepfunctions-tasks/test/http/integ.invoke.js.snapshot/HttpInvokeTestDefaultTestDeployAssertBA1B4726.assets.json
Auto-merging packages/@aws-cdk-testing/framework-integ/test/aws-stepfunctions-tasks/test/http/integ.invoke.js.snapshot/HttpInvokeTestDefaultTestDeployAssertBA1B4726.template.json
CONFLICT (content): Merge conflict in packages/@aws-cdk-testing/framework-integ/test/aws-stepfunctions-tasks/test/http/integ.invoke.js.snapshot/HttpInvokeTestDefaultTestDeployAssertBA1B4726.template.json
Auto-merging packages/@aws-cdk-testing/framework-integ/test/aws-stepfunctions-tasks/test/http/integ.invoke.js.snapshot/asset.65c9f57ca55e2d458059a930e997f2bc96560b3a3654cc442616d12089449d39.bundle/index.js
CONFLICT (rename/rename): packages/@aws-cdk-testing/framework-integ/test/aws-stepfunctions-tasks/test/http/integ.invoke.js.snapshot/asset.65c9f57ca55e2d458059a930e997f2bc96560b3a3654cc442616d12089449d39.bundle/index.js renamed to packages/@aws-cdk-testing/framework-integ/test/aws-apigatewayv2-integrations/test/http/integ.sqs.js.snapshot/asset.556f7b960c212183e2f73e1410a097107af3ccf13c8880a717edbcadd89611a4.bundle/index.js in HEAD and to packages/@aws-cdk-testing/framework-integ/test/aws-stepfunctions-tasks/test/http/integ.invoke.js.snapshot/asset.bfcf4228d6660f50e81f19efdb64c6388dda6796296eb239decdff1dbc2f4981.bundle/index.js in 2d3c18608 (Add directive indicating which stack to test, which apparently is needed now.).
Auto-merging packages/@aws-cdk-testing/framework-integ/test/aws-stepfunctions-tasks/test/http/integ.invoke.js.snapshot/aws-stepfunctions-tasks-http-invoke-integ.assets.json
CONFLICT (content): Merge conflict in packages/@aws-cdk-testing/framework-integ/test/aws-stepfunctions-tasks/test/http/integ.invoke.js.snapshot/aws-stepfunctions-tasks-http-invoke-integ.assets.json
Auto-merging packages/@aws-cdk-testing/framework-integ/test/aws-stepfunctions-tasks/test/http/integ.invoke.js.snapshot/aws-stepfunctions-tasks-http-invoke-integ.template.json
Auto-merging packages/@aws-cdk-testing/framework-integ/test/aws-stepfunctions-tasks/test/http/integ.invoke.js.snapshot/cdk.out
CONFLICT (content): Merge conflict in packages/@aws-cdk-testing/framework-integ/test/aws-stepfunctions-tasks/test/http/integ.invoke.js.snapshot/cdk.out
Auto-merging packages/@aws-cdk-testing/framework-integ/test/aws-stepfunctions-tasks/test/http/integ.invoke.js.snapshot/integ.json
CONFLICT (content): Merge conflict in packages/@aws-cdk-testing/framework-integ/test/aws-stepfunctions-tasks/test/http/integ.invoke.js.snapshot/integ.json
Auto-merging packages/@aws-cdk-testing/framework-integ/test/aws-stepfunctions-tasks/test/http/integ.invoke.js.snapshot/manifest.json
CONFLICT (content): Merge conflict in packages/@aws-cdk-testing/framework-integ/test/aws-stepfunctions-tasks/test/http/integ.invoke.js.snapshot/manifest.json
Auto-merging packages/@aws-cdk-testing/framework-integ/test/aws-stepfunctions-tasks/test/http/integ.invoke.js.snapshot/tree.json
CONFLICT (content): Merge conflict in packages/@aws-cdk-testing/framework-integ/test/aws-stepfunctions-tasks/test/http/integ.invoke.js.snapshot/tree.json
error: could not apply 2d3c18608... Add directive indicating which stack to test, which apparently is needed now.
hint: Resolve all conflicts manually, mark them as resolved with
hint: "git add/rm <conflicted_files>", then run "git rebase --continue".
hint: You can instead skip this commit: run "git rebase --skip".
hint: To abort and get back to the state before "git rebase", run "git rebase --abort".
Could not apply 2d3c18608... Add directive indicating which stack to test, which apparently is needed now.

Pull request has been modified.

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[github-actions]

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.