fix: Give SNS feedback access to CloudWatch #237
Conversation
aldenquimby
changed the title
|
This PR has been automatically marked as stale because it has been open 30 days |
|
This is still valid. Commenting to keep it open |
|
This PR has been automatically marked as stale because it has been open 30 days |
|
Not stale |
| statement { | ||
| effect = "Allow" | ||
| actions = [ | ||
| "logs:CreateLogGroup", |
There was a problem hiding this comment.
Terraform creates the log group, this one can be removed
There was a problem hiding this comment.
Where are the 2 SNS log groups being created? I see a log group for Lambda logs, but that is different. I believe this permission is necessary for the sns/:region/:account/:topic and sns/:region/:account/:topic/Failure log groups that are auto-created by SNS (assuming permissions are wired up correctly)
| "logs:PutMetricFilter", | ||
| "logs:PutRetentionPolicy", | ||
| ] | ||
| resources = ["*"] |
There was a problem hiding this comment.
we know the ARN and can scope it with the aws_cloudwatch_log_group created by the module instead of using a wildcard
There was a problem hiding this comment.
I believe that is the log group for Lambda logs, which is not what we want here. We instead need the SNS feedback log groups, per my other comment. I could restrict it down to sns/:region/:account/:topic and sns/:region/:account/:topic/*? (I know the /Failure group is needed, but I'm not sure about others. The reference post just uses *, so that's not helpful)
| @@ -36,3 +53,11 @@ resource "aws_iam_role" "sns_feedback_role" { | |||
| var.sns_topic_feedback_role_tags, | |||
| ) | |||
| } | |||
|
|
|||
| resource "aws_iam_role_policy" "sns_feedback_role" { | |||
There was a problem hiding this comment.
create a policy and attach it instead of inline policy. name should not be hardcoded - you can see other modules for references
There was a problem hiding this comment.
Ok, can do! Is this what you are referring to? https://github.com/terraform-aws-modules/terraform-aws-vpc/blob/573f574c922782bc658f05523d0c902a4792b0a8/vpc-flow-logs.tf#L115-L129
Would you like me to add a _policy_name and _policy_use_name_prefix variable for this?
Description
Motivation and Context
Breaking Changes
How Has This Been Tested?
examples/*to demonstrate and validate my change(s)examples/*projectspre-commit run -aon my pull request