-
Notifications
You must be signed in to change notification settings - Fork 32
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
fix(aws-backup): avoid duplicated and ill-routed alerts #508
Conversation
c80107a
to
9b0770c
Compare
9b0770c
to
35b9d3d
Compare
35b9d3d
to
62fafbf
Compare
@@ -0,0 +1,3 @@ | |||
variable "aws_account_id" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think forcing aws_account_id is a good idea, not everyone wants to filter by aws_account_id
It's best to let people choose if they want to by setting the appropriate filter in their own terraform
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello, we don't have much choice because recovery points are not automatically tagged according to the convention used with the detectors (see comment in common-filters.tf
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variable removed after further discussions.
filters = format("filter('namespace', 'AWS/Backup') and filter('stat', 'sum') and filter('BackupVaultName', '*', match_missing=False) and filter('ResourceType', '*', match_missing=False) and filter('aws_account_id', '%s')", var.aws_account_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aren't signals already filtered with most of these variables in the detector conf ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have removed the filtering
field in the detectors conf 👍
4a89a21
to
a3918fe
Compare
No description provided.