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

(stepfunctions_tasks): BatchSubmitJob emits incorrect RoleDefaultPolicy and invalid CFN when jobQueueArn is defined later. #24084

Open endian675 opened 1 year ago

endian675 commented 1 year ago

Describe the bug

BatchSubmitJob() generates an invalid RoleDefaultPolicy by emitting the literal value $.jobQueueArn instead of wildcard. This behaviour is inconsistent with jobDefinitionArn, which emits a wildcard ~when the value of the variable is not available~ always ~(i.e., it will be created later, outside of CDK/CFN)~.

MonteCarloPipelineRoleDefaultPolicy3E3E3B10:
    Type: AWS::IAM::Policy
    Properties:
      PolicyDocument:
        Statement:
          - Action: lambda:InvokeFunction
            Effect: Allow
            Resource:
              Fn::GetAtt:
                - Preprocessing329E01E4
                - Arn
          - Action: batch:SubmitJob
            Effect: Allow
            Resource:
              - Fn::Join:
                  - ""
                  - - "arn:"
                    - Ref: AWS::Partition
                    - ":batch:"
                    - Ref: AWS::Region
                    - ":"
                    - Ref: AWS::AccountId
                    - :job-definition/*
              - $.jobQueueArn <----------INVALID

Expected Behavior

MonteCarloPipelineRoleDefaultPolicy3E3E3B10:
    Type: AWS::IAM::Policy
    Properties:
      PolicyDocument:
        Statement:
          - Action: lambda:InvokeFunction
            Effect: Allow
            Resource:
              Fn::GetAtt:
                - Preprocessing329E01E4
                - Arn
          - Action: batch:SubmitJob
            Effect: Allow
            Resource:
              - Fn::Join:
                  - ""
                  - - "arn:"
                    - Ref: AWS::Partition
                    - ":batch:"
                    - Ref: AWS::Region
                    - ":"
                    - Ref: AWS::AccountId
                    - :job-definition/*
              - Fn::Join:
                - ""
                - - "arn:"
                  - Ref: AWS::Partition
                  - ":batch:"
                  - Ref: AWS::Region
                  - ":"
                  - Ref: AWS::AccountId
                  - :job-queue/

Current Behavior

  MonteCarloPipelineRoleDefaultPolicy3E3E3B10:
    Type: AWS::IAM::Policy
    Properties:
      PolicyDocument:
        Statement:
          - Action: lambda:InvokeFunction
            Effect: Allow
            Resource:
              Fn::GetAtt:
                - Preprocessing329E01E4
                - Arn
          - Action: batch:SubmitJob
            Effect: Allow
            Resource:
              - Fn::Join:
                  - ""
                  - - "arn:"
                    - Ref: AWS::Partition
                    - ":batch:"
                    - Ref: AWS::Region
                    - ":"
                    - Ref: AWS::AccountId
                    - :job-definition/*
              - $jobQueueArn

Reproduction Steps

montecarlo_task = stepfunctions_tasks.BatchSubmitJob(self, 'MonteCarlo',
          job_name=stepfunctions.JsonPath.string_at('$.jobName'),
          job_queue_arn=stepfunctions.JsonPath.string_at('$.jobQueueArn'),
          job_definition_arn=stepfunctions.JsonPath.string_at('$.jobDefinitionArn'),
          result_path='$.output',
          array_size=stepfunctions.JsonPath.number_at('$.output.Payload.body.arrayJobSize'),
          payload=
              stepfunctions.TaskInput.from_object({
                      "action": "price",
                      "inputUri": stepfunctions.JsonPath.string_at('$.inputUri'),
                      "outputUri": stepfunctions.JsonPath.string_at('$.outputUri'),
                      "bucket": stepfunctions.JsonPath.string_at('$.output.Payload.body.bucket'),
                      "key": stepfunctions.JsonPath.string_at('$.output.Payload.body.key')
              })
          )

Possible Solution

jobDefinitionArn is handled correctly, jobQueueArn should be handled the same way

Additional Information/Context

No response

CDK CLI Version

2.64.0 (build fb67c77)

Framework Version

No response

Node.js Version

v16.18.1

OS

Amazon Linux 2

Language

Python

Language Version

Python (3.7.16)

Other information

No response

endian675 commented 1 year ago

This appears to be by-design in packages/@aws-cdk/aws-stepfunctions-tasks/lib/batch/submit-job.ts configurePolicyStatements() but I think it should be changed to be consistent with the treatment of jobDefinitionArn

peterwoodworth commented 1 year ago

You're right, thanks for reporting this issue. I can confirm the invalid template gets generated when attempting to use JsonPath. As you've mentioned, we are handling this by just directly passing in what was passed in to props

https://github.com/aws/aws-cdk/blob/fb67c77855967e7d493f85f4f2d31532efdd560c/packages/%40aws-cdk/aws-stepfunctions-tasks/lib/batch/submit-job.ts#L268-L283

How did you achieve the behavior you describe in CurrentBehavior where a wildcard is supplied?

endian675 commented 1 year ago

@peterwoodworth I put the wrong item into CurrentBehaviour, let me correct it. CurrentBehaviour is to emit the incorrect $.jobQueueArn.

peterwoodworth commented 1 year ago

Thanks for clarifying! Makes sense, just making sure I wasn't missing something 🙂