-
-
Notifications
You must be signed in to change notification settings - Fork 356
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
add sns filter by name and time #482
Conversation
add time filter for SNS
Note, I wanted to switch SNS over entirely to aws go SDK v2, as it uses both v1 and v2, but this PR is already quite large and I'll switch over to v2 in a separate 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.
Overall LGTM. Left a nit comment. I can trigger the test pipeline
aws/sns.go
Outdated
|
||
for i := range response.Tags { | ||
if *response.Tags[i].Key == key { | ||
firstSeenTime, err := time.Parse(time.RFC3339, *response.Tags[i].Value) |
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.
can we define a new constant in globals.go file the format of the time we will store the firstSeenTime
, to prevent inconsistency in the future?
then, we can replace the usage of time.RFC3339
here and below in set function
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.
Thanks, I updated this in sns.go, and found occurrences in ec2_vpc.go and ecs_cluster.go. I can push these changes to a new PR if you'd like to keep the context to SNS in this PR. Either works for me.
FTR, I have tested the change in our environment and have verified that it fixes #480 for us. Thanks. |
Tests looking good. Let's merge it in |
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
Description
Fixes #359 and #480.
Adds the ability to include/exclude SNS topic by name for nuking. Additionally, the ability to exclude SNS topics by time is also added.
Read the Gruntwork contribution guidelines.
nuke_sandbox
andnuke_phxdevops
jobs in.circleci/config.yml
have been updated with appropriate exclusions (either directly in the job or via the.circleci/nuke_config.yml
file) to prevent nuking IAM roles, groups, resources, etc that are important for the test accounts.Release Notes (draft)
Adds the ability to include/exclude SNS topic by name for nuking. Additionally, the ability to exclude SNS topics by time is also added.
Migration Guide