-
Notifications
You must be signed in to change notification settings - Fork 96
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
feat: Add usage example for monitoring alerts on KMS key versions #162
feat: Add usage example for monitoring alerts on KMS key versions #162
Conversation
…kms into feat/add-autokey
…rm-google-kms into feat/add-autokey
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.
This is extremely cool, nicely done!!
I guess my only comment is that maybe it's worth to also offer a version (through another input variable) where the module doesn't necessarily create a new key version. But I guess this is an example and not a true standalone module, so feel free to ignore. I imagine one of the reasons to have it in there was to also make testing easier in terms of having a sample key ready to be destroyed, otherwise that part would have to be in the test logic itself. Other than that, LGTM!
We'll see if @erlanderlo has more to say :)
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.
lgtm - good testing ideas.
mostly nits and few minor comments
Thanks for your review! I added a comment with instructions for users wanting to use an existing KMS key. |
@apeabody could you PTAL on this PR? |
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.
Already reviewed
The goal of this PR is to provide an example of an alert being sent when a KMS key version is marked to be deleted.