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

Imported LogGroups don't have :* in resource ARN grants #7096

Closed
kohidave opened this issue Mar 31, 2020 · 0 comments · Fixed by #7668
Closed

Imported LogGroups don't have :* in resource ARN grants #7096

kohidave opened this issue Mar 31, 2020 · 0 comments · Fixed by #7668
Assignees
Labels
@aws-cdk/aws-logs Related to Amazon CloudWatch Logs bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. p1

Comments

@kohidave
Copy link

❓ General Issue

Feedback around LogGroupBase:grant

The Question

Right now, there seems to be an assumption that the LogGroupArn is always appended with :* - I think this is true when the value is retrieved from GetAtt.

public grant(grantee: iam.IGrantable, ...actions: string[]) {

But when customers construct the LogGroup themselves with fromLogGroupArn or fromLogGroupName then they would probably exclude the :* suffix.

I think this ends up being confusing when a policy is created through grant when passed one of these constructed log groups - since the generated policy won't have permissions to create streams.

When using fromLogGroupArn or fromLogGroupName the generated policy looks like:

{
	"Action": [
		"logs:CreateLogStream",
		"logs:PutLogEvents"
	],
	"Resource": "arn:aws:logs:us-west-2:***:log-group:SOME_STREAM_PREFIX",
	"Effect": "Allow"
}

But it should look like:

{
	"Action": [
		"logs:CreateLogStream",
		"logs:PutLogEvents"
	],
	"Resource": "arn:aws:logs:us-west-2:***:log-group:SOME_STREAM_PREFIX:*",
	"Effect": "Allow"
}

Environment

  • CDK CLI Version: v1.31.0
  • OS: all
  • Language: all

Other information

This is related to one of the comments in this thread: #5954

THANK YOU!

@kohidave kohidave added the needs-triage This issue or PR still needs to be triaged. label Mar 31, 2020
@kohidave kohidave changed the title LogGroupBase grant assumes a * suffix but helper functions do not generate generate that suffix LogGroupBase grant assumes an arn with a * suffix, but helper functions do not generate generate that suffix Mar 31, 2020
@kohidave kohidave changed the title LogGroupBase grant assumes an arn with a * suffix, but helper functions do not generate generate that suffix LogGroupBase grant assumes an arn with a * suffix, but helper functions do not generate that suffix Mar 31, 2020
@SomayaB SomayaB added the @aws-cdk/aws-logs Related to Amazon CloudWatch Logs label Mar 31, 2020
@SomayaB SomayaB added feature-request A feature should be added or improved. bug This issue is a bug. labels Mar 31, 2020
@rix0rrr rix0rrr changed the title LogGroupBase grant assumes an arn with a * suffix, but helper functions do not generate that suffix Imported LogGroups don't have :* in resource ARN grants Apr 1, 2020
@rix0rrr rix0rrr added p2 p1 and removed feature-request A feature should be added or improved. p2 labels Apr 1, 2020
rix0rrr added a commit that referenced this issue Apr 29, 2020
In CloudFormation, `{ Fn::GetAtt: [MyLogGroup, Arn] }` always returned
the ARN with a `:*` appended, presumably so you could stick the returned
value directly into an IAM policy and get the result you wanted (doing
something to Log Groups usually entails doing something to the Log
*Streams* inside them).

The CDK construct did not do anything special, leading to imports done
without a `:*` at the end having incorrect permissions.

This change makes the behavior between imported and constructed Log
Groups consistent.

Fixes #7096.
@mergify mergify bot closed this as completed in #7668 Apr 30, 2020
mergify bot pushed a commit that referenced this issue Apr 30, 2020
In CloudFormation, `{ Fn::GetAtt: [MyLogGroup, Arn] }` always returned
the ARN with a `:*` appended, presumably so you could stick the returned
value directly into an IAM policy and get the result you wanted (doing
something to Log Groups usually entails doing something to the Log
*Streams* inside them).

The CDK construct did not do anything special, leading to imports done
without a `:*` at the end having incorrect permissions.

This change makes the behavior between imported and constructed Log
Groups consistent.

Fixes #7096.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-logs Related to Amazon CloudWatch Logs bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants