fix(stepfunctions-tasks): missing tags & perms for emr cluster creation #28327
Conversation
github-actions
bot
added
star-contributor
| @@ -192,6 +193,14 @@ export class EmrCreateCluster extends sfn.TaskStateBase { | |||
| this._serviceRole = this.props.serviceRole ?? this.createServiceRole(); | |||
| this._clusterRole = this.props.clusterRole ?? this.createClusterRole(); | |||
|
|
|||
| // Service role must be able to iam:PassRole on the cluster role | |||
| this._clusterRole.grantPassRole(this._serviceRole); | |||
There was a problem hiding this comment.
AmazonEMRServicePolicy_v2 only grants iam:PassRole on the default role. Need this here for the cluster role we create.
| 'elasticmapreduce:RunJobFlow', | ||
| 'elasticmapreduce:DescribeCluster', | ||
| 'elasticmapreduce:TerminateJobFlows', | ||
| 'elasticmapreduce:AddTags', |
| conditions: { | ||
| StringEquals: { 'aws:RequestTag/for-use-with-amazon-emr-managed-policies': 'true' }, | ||
| }, | ||
| }), |
There was a problem hiding this comment.
This condition isn't needed and was actually causing it to fail.
There was a problem hiding this comment.
Could you explain your reasoning for why it's not needed? Also how was it causing the integ tests to fail?
There was a problem hiding this comment.
When included, I get the following error:
EMR service role arn:aws:iam::<REDACTED>:role/aws-cdk-emr-create-cluste-EmrCreateClusterServiceRo-GrrjLUAR1a2Q is invalid
There was a problem hiding this comment.
Gotcha, I see the part where it's being added to the _base_tags as well. Thanks for clarifying.
| new sfn.StateMachine(stack, 'SM', { | ||
| definition: step, | ||
| }); | ||
|
|
There was a problem hiding this comment.
this wasn't actually deploying a state machine. I added this and ran the step function.
msambol
force-pushed
the
emr-policy-tag
branch
from
4aaed16
to
e02fc1d
Compare
aws-cdk-automation
added
the
pr/needs-community-review
| conditions: { | ||
| StringEquals: { 'aws:RequestTag/for-use-with-amazon-emr-managed-policies': 'true' }, | ||
| }, | ||
| }), |
There was a problem hiding this comment.
Could you explain your reasoning for why it's not needed? Also how was it causing the integ tests to fail?
packages/aws-cdk-lib/aws-stepfunctions-tasks/test/emr/emr-create-cluster.test.ts
Outdated
Show resolved
Hide resolved
aws-cdk-automation
removed
the
pr/needs-community-review
mergify
bot
dismissed
paulhcsun’s stale review
Pull request has been modified.
aws-cdk-automation
added
the
pr/needs-community-review
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
aws-cdk-automation
removed
the
pr/needs-community-review
Per the documentation:
for-use-with-amazon-emr-managed-policies = truewhen you provision a cluster with the CLI, SDK, or another method.Also,
AmazonEMRServicePolicy_v2only hasiam:PassRoleon the default EMR roles and needs this on the cluster role created by the CDK.Running the step function:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license