feat(ecs): add function to grant run permissions to task definition#20281
Conversation
|
|
TheRealAmazonKendra
left a comment
There was a problem hiding this comment.
Thank you for your contribution. This change requires a README change. Also, please ensure the PR title and body follow our pull request guidelines in our Contributing Guide.
Pull request has been modified.
TheRealAmazonKendra
left a comment
There was a problem hiding this comment.
@jusdino Looks like the build is failing due to the spacing in the README. Also, please allow us to push to your branch or mergify will not be able to rebase and merge this once it's approved.
Pull request has been modified.
So I have not figured out how to allow that when opening a pull request from my org into another. Would you happen to know or have a reference I can go look at? |
|
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). |
|
Pull request has been modified.
Looks like this is the instructions for org owned. https://docs.github.com/en/organizations/managing-organization-settings/managing-the-forking-policy-for-your-organization We can't merge PRs that can't be edited by us due to the mergify workflow. If this one can't be edited the best course of action is to close it and reopen it under your personal fork. |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Yeah, @TheRealAmazonKendra, it looks like I can't let you push back to my branch in the org-owned fork so I did as you suggested and opened a new PR from a personal fork. |
…21241) Adding a grantRun(IGrantable) method to aws-ecs.TaskDefinition to make it easier to set up permissions for running task definitions. I'm not super familiar with the case where executionRole is undefined, but I believe leaving that role off the PolicyStatement is the best behavior in that case. Hopefully somebody can sanity check me there. This method is a touch more complicated than I initially planned so it can gracefully handle the case where .addContainer() causes executionRole to be created after a grantRun() was already called (this case is included in the new tests). Reopening #20281 from a personal fork, since the original org-owned fork doesn't allow PR owners to push to my branch. @TheRealAmazonKendra 👋 ---- ### All Submissions: * [*] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…ws#21241) Adding a grantRun(IGrantable) method to aws-ecs.TaskDefinition to make it easier to set up permissions for running task definitions. I'm not super familiar with the case where executionRole is undefined, but I believe leaving that role off the PolicyStatement is the best behavior in that case. Hopefully somebody can sanity check me there. This method is a touch more complicated than I initially planned so it can gracefully handle the case where .addContainer() causes executionRole to be created after a grantRun() was already called (this case is included in the new tests). Reopening aws#20281 from a personal fork, since the original org-owned fork doesn't allow PR owners to push to my branch. @TheRealAmazonKendra 👋 ---- ### All Submissions: * [*] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Adding a
grantRun(IGrantable)method toaws-ecs.TaskDefinitionto make it easier to set up permissions for running task definitions. I'm not super familiar with the case whereexecutionRoleis undefined, but I believe leaving that role off the PolicyStatement is the best behavior in that case. Hopefully somebody can sanity check me there. This method is a touch more complicated than I initially planned so it can gracefully handle the case where.addContainer()causesexecutionRoleto be created after agrantRun()was already called (this case is included in the new tests).All Submissions:
Adding new Unconventional Dependencies:
New Features
yarn integto deploy the infrastructure and generate the snapshot (i.e.yarn integwithout--dry-run)?By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license