Skip to content

Commit

Permalink
fix(ecs): unnecessary CloudWatch logs ResourcePolicy (#28495)
Browse files Browse the repository at this point in the history
This PR modified to avoid creating unnecessary `ResourcePolicy` in CloudWatch Logs.

## issue summary
The related issue reports an error when using the awslogs driver on ECS.
This error is caused by the creation of a ResourcePolicy in CloudWatch Logs that reaches the maximum number of ResourcePolicies.
https://docs.aws.amazon.com/AmazonCloudWatch/latest/logs/cloudwatch_limits_cwl.html

## Current behavior
In some cases, this ResourcePolicy will be created and in other cases it will not be created.
Currently, `Grant.addToPrincipalOrResource` is used to grant permissions to ExecutionRole and Log Group in the ECS taskDef.
https://github.com/aws/aws-cdk/blob/607dccb0fd920d25f0fe2613b83c9830322c439e/packages/aws-cdk-lib/aws-ecs/lib/log-drivers/aws-log-driver.ts#L138
https://github.com/aws/aws-cdk/blob/607dccb0fd920d25f0fe2613b83c9830322c439e/packages/aws-cdk-lib/aws-logs/lib/log-group.ts#L194
https://github.com/aws/aws-cdk/blob/607dccb0fd920d25f0fe2613b83c9830322c439e/packages/aws-cdk-lib/aws-iam/lib/grant.ts#L122

`Grant.addToPrincipalOrResource` first grants permissions to the Grantee (ExecutionRole) and creates a resource base policy for cross account access in cases where certain conditions are not met.
This condition is determined by the contents of the `principalAccount` of the ExecutionRole and the accountID in the `env.account` and whether or not these are Tokens, but in this scenario, cross account access is not necessary.
https://github.com/aws/aws-cdk/blob/607dccb0fd920d25f0fe2613b83c9830322c439e/packages/aws-cdk-lib/aws-iam/lib/grant.ts#L141

Also, when the `LogGroup.grantWrite` call was added to `aws-log-driver.ts`, the ResourcePolicy for logs could not be created from CFn and only granted to the ExecutionRole.
#1291
![スクリーンショット 2023-12-27 1 08 20](https://github.com/aws/aws-cdk/assets/58683719/5a17a041-d560-45fa-bac6-cdc3894b18bc)
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/ReleaseHistory.html

Therefore, the resource base policy should not be necessary when using the awslogs driver.

## Major changes
This PR changed to grant permissions only to ExecutionRole when using the awslogs driver.
With this fix, ResourcePolicy will no longer be created when using the awslogs driver.
I don't consider this a breaking change, as it changes the content of the generated template, but does not change the behavior of forwarding logs to CloudWatch Logs.
However, if this is a breaking change, I think it is necessary to use the feature flag.

fixes #22307, fixes #20313

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
sakurai-ryo authored Jan 10, 2024
1 parent 7d68fee commit 5f96d13
Show file tree
Hide file tree
Showing 12 changed files with 36,691 additions and 2 deletions.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 5f96d13

Please sign in to comment.