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 · 7 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
Contributor

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

@Tohaker
Copy link

Tohaker commented Aug 8, 2024

Has there been any progress on this? I've faced this problem and it's a real blocker

@MathieuGilbert
Copy link
Contributor

I have a draft PR to fix this here and just need to figure out how to update the integration test without breaking CI.

@richhouse83
Copy link

Has there been any progress on this? I've faced this problem and it's a real blocker

@Tohaker Temporary fix in TS can use patch-package:

diff --git a/node_modules/aws-cdk-lib/aws-stepfunctions-tasks/lib/http/invoke.js b/node_modules/aws-cdk-lib/aws-stepfunctions-tasks/lib/http/invoke.js index cf8da39..756b2b6 100644 --- a/node_modules/aws-cdk-lib/aws-stepfunctions-tasks/lib/http/invoke.js +++ b/node_modules/aws-cdk-lib/aws-stepfunctions-tasks/lib/http/invoke.js @@ -1 +1 @@ -"use strict";var _a;Object.defineProperty(exports,"__esModule",{value:!0}),exports.HttpInvoke=exports.URLEncodingFormat=void 0;var jsiiDeprecationWarnings=()=>{var tmp=require("../../../.warnings.jsii.js");return jsiiDeprecationWarnings=()=>tmp,tmp};const JSII_RTTI_SYMBOL_1=Symbol.for("jsii.rtti");var iam=()=>{var tmp=require("../../../aws-iam");return iam=()=>tmp,tmp},sfn=()=>{var tmp=require("../../../aws-stepfunctions");return sfn=()=>tmp,tmp},task_utils_1=()=>{var tmp=require("../private/task-utils");return task_utils_1=()=>tmp,tmp},URLEncodingFormat;(function(URLEncodingFormat2){URLEncodingFormat2.BRACKETS="BRACKETS",URLEncodingFormat2.COMMAS="COMMAS",URLEncodingFormat2.DEFAULT="DEFAULT",URLEncodingFormat2.INDICES="INDICES",URLEncodingFormat2.NONE="NONE",URLEncodingFormat2.REPEAT="REPEAT"})(URLEncodingFormat||(exports.URLEncodingFormat=URLEncodingFormat={}));class HttpInvoke extends sfn().TaskStateBase{constructor(scope,id,props){super(scope,id,props),this.props=props;try{jsiiDeprecationWarnings().aws_cdk_lib_aws_stepfunctions_tasks_HttpInvokeProps(props)}catch(error){throw process.env.JSII_DEBUG!=="1"&&error.name==="DeprecationError"&&Error.captureStackTrace(error,HttpInvoke),error}this.taskPolicies=this.buildTaskPolicyStatements()}_renderTask(){return{Resource:(0,task_utils_1().integrationResourceArn)("http","invoke"),Parameters:sfn().FieldUtils.renderObject(this.buildTaskParameters())}}buildTaskPolicyStatements(){return[new(iam()).PolicyStatement({actions:["events:RetrieveConnectionCredentials"],resources:[this.props.connection.connectionArn]}),new(iam()).PolicyStatement({actions:["secretsmanager:GetSecretValue","secretsmanager:DescribeSecret"],resources:[this.props.connection.connectionSecretArn]}),new(iam()).PolicyStatement({actions:["states:InvokeHTTPEndpoint"],resources:["*"],conditions:{StringLike:{"states:HTTPEndpoint":${this.props.apiRoot}}}})]}buildTaskParameters(){const parameters={ApiEndpoint:${this.props.apiRoot}/${this.props.apiEndpoint.value},Authentication:{ConnectionArn:this.props.connection.connectionArn},Method:this.props.method.value,Headers:this.props.headers?.value,RequestBody:this.props.body?.value,QueryParameters:this.props.queryStringParameters?.value};return this.props.urlEncodingFormat!=null&&this.props.urlEncodingFormat!==URLEncodingFormat.NONE&&(parameters.Headers={...parameters.Headers,"Content-Type":"application/x-www-form-urlencoded"},parameters.Transform={RequestBodyEncoding:"URL_ENCODED"},this.props.urlEncodingFormat!==URLEncodingFormat.DEFAULT&&(parameters.Transform.RequestEncodingOptions={ArrayFormat:this.props.urlEncodingFormat})),parameters}}exports.HttpInvoke=HttpInvoke,_a=JSII_RTTI_SYMBOL_1,HttpInvoke[_a]={fqn:"aws-cdk-lib.aws_stepfunctions_tasks.HttpInvoke",version:"2.149.0"}; +"use strict";var _a;Object.defineProperty(exports,"__esModule",{value:!0}),exports.HttpInvoke=exports.URLEncodingFormat=void 0;var jsiiDeprecationWarnings=()=>{var tmp=require("../../../.warnings.jsii.js");return jsiiDeprecationWarnings=()=>tmp,tmp};const JSII_RTTI_SYMBOL_1=Symbol.for("jsii.rtti");var iam=()=>{var tmp=require("../../../aws-iam");return iam=()=>tmp,tmp},sfn=()=>{var tmp=require("../../../aws-stepfunctions");return sfn=()=>tmp,tmp},task_utils_1=()=>{var tmp=require("../private/task-utils");return task_utils_1=()=>tmp,tmp},URLEncodingFormat;(function(URLEncodingFormat2){URLEncodingFormat2.BRACKETS="BRACKETS",URLEncodingFormat2.COMMAS="COMMAS",URLEncodingFormat2.DEFAULT="DEFAULT",URLEncodingFormat2.INDICES="INDICES",URLEncodingFormat2.NONE="NONE",URLEncodingFormat2.REPEAT="REPEAT"})(URLEncodingFormat||(exports.URLEncodingFormat=URLEncodingFormat={}));class HttpInvoke extends sfn().TaskStateBase{constructor(scope,id,props){super(scope,id,props),this.props=props;try{jsiiDeprecationWarnings().aws_cdk_lib_aws_stepfunctions_tasks_HttpInvokeProps(props)}catch(error){throw process.env.JSII_DEBUG!=="1"&&error.name==="DeprecationError"&&Error.captureStackTrace(error,HttpInvoke),error}this.taskPolicies=this.buildTaskPolicyStatements()}_renderTask(){return{Resource:(0,task_utils_1().integrationResourceArn)("http","invoke"),Parameters:sfn().FieldUtils.renderObject(this.buildTaskParameters())}}buildTaskPolicyStatements(){return[new(iam()).PolicyStatement({actions:["events:RetrieveConnectionCredentials"],resources:[this.props.connection.connectionArn]}),new(iam()).PolicyStatement({actions:["secretsmanager:GetSecretValue","secretsmanager:DescribeSecret"],resources:[this.props.connection.connectionSecretArn]}),new(iam()).PolicyStatement({actions:["states:InvokeHTTPEndpoint"],resources:["*"],conditions:{StringLike:{"states:HTTPEndpoint":${this.props.apiRoot}}}})]}buildTaskParameters(){const parameters={ApiEndpoint:sfn().JsonPath.format(${this.props.apiRoot}/{}, this.props.apiEndpoint.value),Authentication:{ConnectionArn:this.props.connection.connectionArn},Method:this.props.method.value,Headers:this.props.headers?.value,RequestBody:this.props.body?.value,QueryParameters:this.props.queryStringParameters?.value};return this.props.urlEncodingFormat!=null&&this.props.urlEncodingFormat!==URLEncodingFormat.NONE&&(parameters.Headers={...parameters.Headers,"Content-Type":"application/x-www-form-urlencoded"},parameters.Transform={RequestBodyEncoding:"URL_ENCODED"},this.props.urlEncodingFormat!==URLEncodingFormat.DEFAULT&&(parameters.Transform.RequestEncodingOptions={ArrayFormat:this.props.urlEncodingFormat})),parameters}}exports.HttpInvoke=HttpInvoke,_a=JSII_RTTI_SYMBOL_1,HttpInvoke[_a]={fqn:"aws-cdk-lib.aws_stepfunctions_tasks.HttpInvoke",version:"2.149.0"};

Only real change is {ApiEndpoint:${this.props.apiRoot}/${this.props.apiEndpoint.value}, to {ApiEndpoint:sfn().JsonPath.format(${this.props.apiRoot}/{},

Hope that helps

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

Successfully merging a pull request may close this issue.

6 participants