feat(events): use existing Role when running ECS Task #8145
Conversation
…sk by cloud watch event
|
Hmm... How to document about it to |
|
This is the only documentation about event targets I can find in |
|
Or... it's OK? diff --git a/packages/@aws-cdk/aws-events/README.md b/packages/@aws-cdk/aws-events/README.md
index adade2d15..152bed6a8 100644
--- a/packages/@aws-cdk/aws-events/README.md
+++ b/packages/@aws-cdk/aws-events/README.md
@@ -89,7 +89,7 @@ import { Rule, Schedule } from '@aws-cdk/aws-events';
import { EcsTask } from '@aws-cdk/aws-events-targets';
...
-const ecsTaskTarget = new EcsTask({ cluster, taskDefinition });
+const ecsTaskTarget = new EcsTask({ cluster, taskDefinition, role });
new Rule(this, 'ScheduleRule', {
schedule: Schedule.cron({ minute: '0', hour: '4' }), |
|
OK. I try it to avoid |
| @@ -117,33 +126,8 @@ export class EcsTask implements events.IRuleTarget { | |||
| * Allows using tasks as target of EventBridge events | |||
| */ | |||
| public bind(_rule: events.IRule, _id?: string): events.RuleTargetConfig { | |||
| const policyStatements = [new iam.PolicyStatement({ | |||
There was a problem hiding this comment.
Please revert this. I get that you don't want this for your specific use case right now, but we have to have consistent behavior across the whole of CDK otherwise people will constantly have to be digging in documentation.
If you want the specific role you're passing to not have statements added to them, pass role.withoutPolicyUpdates() instead.
There was a problem hiding this comment.
Looks like you thumbs-upped my comment, but didn't actually make the change.
Is the code now in a situation where the policy statements are added to the role users pass in (if they do)?
There was a problem hiding this comment.
Oops! No, it isn't. I'm agree to your advice and this code should be fix.
But I haven't been able to do anything because I've been so busy lately, unlike 2 month ago. so sorry for late to fix it.
There was a problem hiding this comment.
so I fixed it. I'm happy if you check it. I'm sorry late for the fix.
rix0rrr
changed the title
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
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 master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Fixes #7859
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license