aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.71k stars 3.94k forks source link

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

Open lafeuil opened 7 months ago

lafeuil commented 7 months ago

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

ashishdhingra commented 7 months ago

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
nmussy commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 3 months ago

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

MathieuGilbert commented 3 months ago

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 commented 3 months ago

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