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.41k stars 3.8k forks source link

(stepfunctions-tasks): SageMaker CreateModel roleArn should be able to take a JsonPath #24903

Open jbarz1 opened 1 year ago

jbarz1 commented 1 year ago

Describe the bug

the role parameter should be able to take in a JsonPath from a stepfunction workflow.

https://docs.aws.amazon.com/cdk/api/v1/docs/@aws-cdk_aws-stepfunctions-tasks.SageMakerCreateModel.html#role

Expected Behavior

successful build

Current Behavior

error TS2322: Type '{ role: TaskRole; }' is not assignable to type 'IRole'.
  Object literal may only specify known properties, and 'role' does not exist in type 'IRole'.

38             role: { role: TaskRole.fromRoleArnJsonPath('$.roleArn') },
                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

  node_modules/aws-cdk-lib/aws-stepfunctions-tasks/lib/sagemaker/create-model.d.ts:17:14
    17     readonly role?: iam.IRole;
                    ~~~~
    The expected type comes from property 'role' which is declared here on type 'SageMakerCreateModelProps'

Found 1 error.

Reproduction Steps

new SageMakerCreateModel(this, 'Create Model', {
            modelName: JsonPath.format(
                '{}-{}',
                JsonPath.stringAt('$.jobName'),
                JsonPath.stringAt('$.suffix')
            ),
            role: { role: TaskRole.fromRoleArnJsonPath('$.roleArn') },
            credentials: { role: TaskRole.fromRoleArnJsonPath('$.roleArn') },
            primaryContainer: new ContainerDefinition({
                image: DockerImage.fromJsonExpression(
                    JsonPath.stringAt('$.image')
                ),
                modelS3Location: S3Location.fromJsonExpression(
                    JsonPath.stringAt('$.modelS3Location')
                ),
                environmentVariables: TaskInput.fromJsonPathAt(
                    JsonPath.stringAt('$.environmentVariables')
                ),
            })

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.14.0

Framework Version

No response

Node.js Version

14

OS

linux

Language

Typescript

Language Version

No response

Other information

No response

pahud commented 1 year ago

fromRoleArnJsonPath essentially returns TaskRole rather than the expected IRole.

I am not sure if it's appropriate to have TaskRole implement IRole.

https://github.com/aws/aws-cdk/blob/a2c633f1e698249496f11338312ab42bd7b1e4f0/packages/aws-cdk-lib/aws-stepfunctions/lib/task-credentials.ts#L33

I am making it p2 and any pull requests are welcome and appreciated.