Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(aws_stepfunctions_tasks): HttpInvoke task does not permit reference path and intrinsic function in the apiEndpoint #29925

Open
lafeuil opened this issue Apr 22, 2024 · 4 comments
Labels
@aws-cdk/aws-stepfunctions-tasks bug This issue is a bug. effort/small Small work item – less than a day of effort p2

Comments

@lafeuil
Copy link

lafeuil commented Apr 22, 2024

Describe the bug

As describe in the AWS step function documentation, the ApiEndpoint parameter of the HttpInvoke task can be configured with intrinsic function and reference path. An example from the AWS documentation :

"ApiEndpoint.$":"States.Format('https://api.stripe.com/v1/customers/{}', $.customer_id)"

In AWS CDK, the ApiEndpoint is generated by concatenating apiRoot and apiEndpoint. But, when using an intrinsic function in the apiEndpoint property, the synth process fails with this error : Error: Field references must be the entire string, cannot concatenate them

I think the issue is with this concatenation here in the code with :

ApiEndpoint: `${this.props.apiRoot}/${this.props.apiEndpoint.value}`,

Expected Behavior

The API endpoint should be configurable with reference path and intrinsic function.

Current Behavior

Example from the AWS step functions documentation

new HttpInvoke(scope, 'http invoke', {
      apiRoot: 'https://api.stripe.com',
      apiEndpoint: sfn.TaskInput.fromText(
        sfn.JsonPath.format('v1/customers/{}', sfn.JsonPath.stringAt('$.customer_id')),
      ),
[...]

I have this error :

Error: Field references must be the entire string, cannot concatenate them (found 'https://api.stripe.com/${Token[States.Format..v1.customers.......customer_id..1211]}')
    at jsonPathString (/home/sesamm/dev/precompute/orchestration/node_modules/aws-cdk-lib/aws-stepfunctions/lib/private/json-path.js:1:4404)
    at Object.renderString (/home/sesamm/dev/precompute/orchestration/node_modules/aws-cdk-lib/aws-stepfunctions/lib/private/json-path.js:1:3700)
    at recurseObject (/home/sesamm/dev/precompute/orchestration/node_modules/aws-cdk-lib/aws-stepfunctions/lib/private/json-path.js:1:2408)
    at renderObject (/home/sesamm/dev/precompute/orchestration/node_modules/aws-cdk-lib/aws-stepfunctions/lib/private/json-path.js:1:986)
    at Function.renderObject (/home/sesamm/dev/precompute/orchestration/node_modules/aws-cdk-lib/aws-stepfunctions/lib/fields.js:1:6666)
    at GetQueryStatus._renderTask (/home/sesamm/dev/precompute/orchestration/node_modules/aws-cdk-lib/aws-stepfunctions-tasks/lib/http/invoke.js:1:1376)
    at GetQueryStatus.toStateJson (/home/sesamm/dev/precompute/orchestration/node_modules/aws-cdk-lib/aws-stepfunctions/lib/states/task-base.js:1:2233)
    at StateGraph.toGraphJson (/home/sesamm/dev/precompute/orchestration/node_modules/aws-cdk-lib/aws-stepfunctions/lib/state-graph.js:1:2159)
    at ChainDefinitionBody.bind (/home/sesamm/dev/precompute/orchestration/node_modules/aws-cdk-lib/aws-stepfunctions/lib/state-machine.js:1:12008)
    at new StateMachine (/home/sesamm/dev/precompute/orchestration/node_modules/aws-cdk-lib/aws-stepfunctions/lib/state-machine.js:1:6368)

Reproduction Steps

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.138.0

Framework Version

No response

Node.js Version

18

OS

Linux

Language

TypeScript

Language Version

Typescript 4.9

Other information

No response

@lafeuil lafeuil added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Apr 22, 2024
@ashishdhingra ashishdhingra added needs-reproduction This issue needs reproduction. and removed needs-triage This issue or PR still needs to be triaged. labels Apr 22, 2024
@ashishdhingra ashishdhingra self-assigned this Apr 22, 2024
@ashishdhingra
Copy link

Reproducible using below code:

import { SecretValue, Stack, StackProps } from 'aws-cdk-lib';
import * as events from 'aws-cdk-lib/aws-events';
import * as sfn from 'aws-cdk-lib/aws-stepfunctions';
import * as tasks from 'aws-cdk-lib/aws-stepfunctions-tasks';
import { Construct } from 'constructs';

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

    const connection = new events.Connection(this, 'Connection', {
      authorization: events.Authorization.basic('username', SecretValue.unsafePlainText('password')),
    });

    const task = new tasks.HttpInvoke(this, 'Invoke HTTP API', {
      apiRoot: 'https://api.stripe.com',
      apiEndpoint: sfn.TaskInput.fromText(
        sfn.JsonPath.format('v1/customers/{}', sfn.JsonPath.stringAt('$.customer_id')),
      ),
      body: sfn.TaskInput.fromObject({ foo: 'bar' }),
      connection,
      headers: sfn.TaskInput.fromObject({ 'Content-Type': 'application/json' }),
      method: sfn.TaskInput.fromText('POST'),
      queryStringParameters: sfn.TaskInput.fromObject({ id: '123' }),
      urlEncodingFormat: tasks.URLEncodingFormat.BRACKETS,
    });

    new sfn.StateMachine(this, 'MyStateMachine', {
      definitionBody: sfn.DefinitionBody.fromChainable(task)
    });
  }
}

Running cdk synth gives below error:

Error: Field references must be the entire string, cannot concatenate them (found 'https://api.stripe.com/${Token[States.Format..v1.customers........customer_id..18]}')
    at jsonPathString (/Users/ashdhin/dev/repros/cdk/issue29925/node_modules/aws-cdk-lib/aws-stepfunctions/lib/private/json-path.js:1:4404)
    at Object.renderString (/Users/ashdhin/dev/repros/cdk/issue29925/node_modules/aws-cdk-lib/aws-stepfunctions/lib/private/json-path.js:1:3700)
    at recurseObject (/Users/ashdhin/dev/repros/cdk/issue29925/node_modules/aws-cdk-lib/aws-stepfunctions/lib/private/json-path.js:1:2408)
    at renderObject (/Users/ashdhin/dev/repros/cdk/issue29925/node_modules/aws-cdk-lib/aws-stepfunctions/lib/private/json-path.js:1:986)
    at Function.renderObject (/Users/ashdhin/dev/repros/cdk/issue29925/node_modules/aws-cdk-lib/aws-stepfunctions/lib/fields.js:1:6666)
    at HttpInvoke._renderTask (/Users/ashdhin/dev/repros/cdk/issue29925/node_modules/aws-cdk-lib/aws-stepfunctions-tasks/lib/http/invoke.js:1:1376)
    at HttpInvoke.toStateJson (/Users/ashdhin/dev/repros/cdk/issue29925/node_modules/aws-cdk-lib/aws-stepfunctions/lib/states/task-base.js:1:2233)
    at StateGraph.toGraphJson (/Users/ashdhin/dev/repros/cdk/issue29925/node_modules/aws-cdk-lib/aws-stepfunctions/lib/state-graph.js:1:2159)
    at ChainDefinitionBody.bind (/Users/ashdhin/dev/repros/cdk/issue29925/node_modules/aws-cdk-lib/aws-stepfunctions/lib/state-machine.js:1:12008)
    at new StateMachine (/Users/ashdhin/dev/repros/cdk/issue29925/node_modules/aws-cdk-lib/aws-stepfunctions/lib/state-machine.js:1:6368)

Subprocess exited with error 1

@ashishdhingra ashishdhingra added p2 effort/small Small work item – less than a day of effort and removed needs-reproduction This issue needs reproduction. labels Apr 22, 2024
@ashishdhingra ashishdhingra removed their assignment Apr 22, 2024
@nmussy
Copy link
Contributor

nmussy commented Apr 23, 2024

It's going to be a little awkward to resolve with the current props design. Even if we join the apiRoot and apiEndpoint, we would get the following:

"ApiEndpoint": "https://api.stripe.com/States.Format('v1/customers/{}', $.customer_id)"

Looking at the docs, I'm pretty sure this would be the accepted format:

"ApiEndpoint.$": "States.Format('https://api.stripe.com/v1/customers/{}', $.customer_id)"

We both need to prepend the first format parameter and change the ApiEndpoint key to tell StepFunctions to resolve it

@lafeuil
Copy link
Author

lafeuil commented Apr 23, 2024

I propose this patch but I don't know if the isEncodedJsonPath function is the good way to test the presence of JsonPath in the apiEndpoint value.

const parameters: TaskParameters = {
      ApiEndpoint:
        sfn.JsonPath.isEncodedJsonPath(this.props.apiEndpoint.value)
          ? sfn.JsonPath.format(`${this.props.apiRoot}/{}`, this.props.apiEndpoint.value)
          : `${this.props.apiRoot}/${this.props.apiEndpoint.value}`,
      Authentication: {

This code generates :

"Parameters":{
  "ApiEndpoint.$":"States.Format('https://api.stripe.com/{}', States.Format('v1/customers/{}', $.customer_id))",
  "Authentication":{
[...]

@nmussy
Copy link
Contributor

nmussy commented Apr 23, 2024

The $ suffix is already added automatically? That makes it a little easier. And I think we should first test whether the apiEndpoint is an unresolvable token, and throw if we're not able to parse it with JsonPath. It'll make unit testing easier, and we can warn the user about an "unsupported token in apiEndpoint" or whatever

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-stepfunctions-tasks bug This issue is a bug. effort/small Small work item – less than a day of effort p2
Projects
None yet
Development

No branches or pull requests

3 participants