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-events-targets): EcsTask target with tags does not get ecs:TagResource permission added to role #28854

Closed
blimmer opened this issue Jan 25, 2024 · 8 comments · Fixed by #28898
Labels
@aws-cdk/aws-events-targets bug This issue is a bug. effort/medium Medium work item – several days of effort p1

Comments

@blimmer
Copy link
Contributor

blimmer commented Jan 25, 2024

Describe the bug

When using the EcsTask EventBridge target and providing tags, the auto-generated role does not get the ecs:TagResource IAM permission.

This causes problems when the AWS account has the tagResourceAuthorization setting enabled. According to an email I recently received from Amazon, this setting will be enabled by default on 29 March 2024:

March 29, 2024 - Tagging Authorization will be turned on for all AWS accounts. The account level setting will no longer be
used and will be removed from the ECS Account Settings page in the AWS Console.

Once the setting is enabled by default on all accounts, these event targets will start failing.

Expected Behavior

When I provide a list of tags on the EcsTask event target, it should automatically grant the ecs:TagResource permission for the specified tags.

Current Behavior

The role does not get the ecs:TagResource permission, which causes the event invocation to fail.

The event fails with:

User: arn:aws:sts::ACCOUNT:assumed-role/TravelBlogDeploymentPipel-StaticWordpressEcsTaskTas-MSjLKMJsnohL/b07e937039c03003adf70077b02829a0 is not authorized to perform: ecs:TagResource on resource: arn:aws:ecs:us-west-2:ACCOUNT:task/CLUSTER/* because no identity-based policy allows the ecs:TagResource action

Reproduction Steps

Consider a basic event rule, as specified in the CDK documentation:

import * as ecs from 'aws-cdk-lib/aws-ecs';

declare const cluster: ecs.ICluster;
declare const taskDefinition: ecs.TaskDefinition;

const rule = new events.Rule(this, 'Rule', {
  schedule: events.Schedule.rate(cdk.Duration.hours(1)),
});

rule.addTarget(
  new targets.EcsTask({
    cluster: cluster,
    taskDefinition: taskDefinition,
    propagateTags: ecs.PropagatedTagSource.TASK_DEFINITION,
    tags: [
      {
        key: 'my-tag',
        value: 'my-tag-value',
      },
    ],
  }),
);

This code produces an IAM role that does not allow ecs:TagResource, as you can see in the source code:

private createEventRolePolicyStatements(): iam.PolicyStatement[] {
const policyStatements = [new iam.PolicyStatement({
actions: ['ecs:RunTask'],
resources: [this.taskDefinition.taskDefinitionArn],
conditions: {
ArnEquals: { 'ecs:cluster': this.cluster.clusterArn },
},
})];
// If it so happens that a Task Execution Role was created for the TaskDefinition,
// then the EventBridge Role must have permissions to pass it (otherwise it doesn't).
if (this.taskDefinition.executionRole !== undefined) {
policyStatements.push(new iam.PolicyStatement({
actions: ['iam:PassRole'],
resources: [this.taskDefinition.executionRole.roleArn],
}));
}
// For Fargate task we need permission to pass the task role.
if (this.taskDefinition.isFargateCompatible) {
policyStatements.push(new iam.PolicyStatement({
actions: ['iam:PassRole'],
resources: [this.taskDefinition.taskRole.roleArn],
}));
}
return policyStatements;
}
}

The event triggers without errors when tagResourceAuthorization is disabled. However, when you enable tagResourceAuthorization via:

aws ecs put-account-setting-default --name tagResourceAuthorization --value on

The task will start to fail because of the missing permission.

Possible Solution

The createEventRolePolicyStatements method should be updated. If tags are present the IAM policy should include ecs:TagResource for the specified tags. The docs should be reviewed and the appropriate restrictive conditions should be applied.

Additional Information/Context

No response

CDK CLI Version

2.123.0

Framework Version

No response

Node.js Version

20.11.0

OS

MacOS

Language

TypeScript

Language Version

No response

Other information

I'm unsure if this is needed if propagateTags is specified. More digging in the docs on tagResourceAuthorization is needed.

@blimmer blimmer added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jan 25, 2024
@blimmer
Copy link
Contributor Author

blimmer commented Jan 25, 2024

Here's how I'm working around this for now:

const manualRuleTags = [
  { key: "my-tag-1", value: "true" },
  { key: "my-tag-2", value: "false" },
];
const manualRuleTarget = new EcsTaskTarget({
  // other props per docs
  tags: manualRuleTags,
});
// Grant ecs:TagResource to work around this bug: https://github.com/aws/aws-cdk/issues/28854
((manualRuleTarget as any).role as Role).addToPrincipalPolicy(
  new PolicyStatement({
    sid: "AllowTaggingEcsResource",
    actions: ["ecs:TagResource"],
    resources: [`arn:aws:ecs:us-west-2:*:task/${ecsCluster.clusterName}/*`],
    // allow tagging with the specified tags
    conditions: {
      "ForAllValues:StringEquals": {
        "aws:TagKeys": manualRuleTags.map((t) => t.key),
      },
    },
  })
);

@msambol
Copy link
Contributor

msambol commented Jan 26, 2024

I'll take this.

@blimmer
Copy link
Contributor Author

blimmer commented Jan 26, 2024

Thanks! I've been deep in client work, so I have just worked around this for now. I appreciate you picking it up. I think the workaround code is accurate for what needs to be added.

@msambol
Copy link
Contributor

msambol commented Jan 26, 2024

@blimmer Hmm, I wasn't able to reproduce this. I first ran:

aws ecs put-account-setting-default --name tagResourceAuthorization --value on

I then deployed the stack here and I can get it to trigger and execute properly:
https://github.com/aws/aws-cdk/blob/main/packages/%40aws-cdk-testing/framework-integ/test/aws-events-targets/test/ecs/integ.event-ec2-task.ts

Am I missing something?

@blimmer
Copy link
Contributor Author

blimmer commented Jan 26, 2024

Hmm, at a glance, it looks like that's providing tags and should fail.

A few things:

  1. Do you see that the default is enabled when you run: aws ecs list-account-settings --effective-settings? You should see something like:

    {
        "name": "tagResourceAuthorization",
        "value": "on",
        "principalArn": "arn:aws:iam::ACCOUNT:role/aws-reserved/sso.amazonaws.com/AWSReservedSSO_MY_ROLE",
        "type": "user"
    },

    Note that this setting is regional so make sure it's enabled for the region you're deploying your stack to.

  2. Did the ECS task actually start? EventBridge will respond without errors to put-event, but then you'll see a FailedInvocation metric in the EventBridge UI for the rule, and the ECS task will never start.

Let me know! I might have some time this afternoon to try with that stack if you're still not able to reproduce.

@msambol
Copy link
Contributor

msambol commented Jan 26, 2024

Ok think I sorted it. I didn't enable in right Region. My bad!

@tim-finnigan
Copy link

Thanks for reporting this issue and noting the workaround. I'll add the appropriate labels for tracking. I saw one other issue (#25768) referencing that error but in the context of using CodePipeline.

@tim-finnigan tim-finnigan self-assigned this Jan 27, 2024
@tim-finnigan tim-finnigan added investigating This issue is being investigated and/or work is in progress to resolve the issue. p1 effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. investigating This issue is being investigated and/or work is in progress to resolve the issue. labels Jan 27, 2024
@tim-finnigan tim-finnigan removed their assignment Jan 27, 2024
msambol added a commit to msambol/aws-cdk that referenced this issue Jan 30, 2024
ConnorRobertson added a commit to msambol/aws-cdk that referenced this issue Feb 3, 2024
msambol added a commit to msambol/aws-cdk that referenced this issue Feb 6, 2024
msambol added a commit to msambol/aws-cdk that referenced this issue Feb 7, 2024
msambol added a commit to msambol/aws-cdk that referenced this issue Feb 11, 2024
msambol added a commit to msambol/aws-cdk that referenced this issue Feb 27, 2024
msambol added a commit to msambol/aws-cdk that referenced this issue Feb 28, 2024
msambol added a commit to msambol/aws-cdk that referenced this issue Mar 1, 2024
paulhcsun added a commit to msambol/aws-cdk that referenced this issue Mar 1, 2024
paulhcsun added a commit to msambol/aws-cdk that referenced this issue Mar 1, 2024
msambol added a commit to msambol/aws-cdk that referenced this issue Mar 2, 2024
aaythapa added a commit to msambol/aws-cdk that referenced this issue Mar 4, 2024
@mergify mergify bot closed this as completed in #28898 Mar 4, 2024
mergify bot pushed a commit that referenced this issue Mar 4, 2024
I enabled the following:

`aws ecs put-account-setting-default --name tagResourceAuthorization --value on`

And then confirmed the task completes successfully.

Closes #28854.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Copy link

github-actions bot commented Mar 4, 2024

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-events-targets bug This issue is a bug. effort/medium Medium work item – several days of effort p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants