-
-
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
Ensure all Route53HostedZones are deleted by adding pagination support #815
Ensure all Route53HostedZones are deleted by adding pagination support #815
Conversation
aws/resources/route53_hostedzone.go
Outdated
logging.Errorf("[Failed] unable to list resource record set: %s", err) | ||
return err | ||
} | ||
for { |
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.
Wouldn’t using NewListResourceRecordSetsPaginator be easier? You could then use HasMorePages()
as an iterator for the loop, e.q. example from docs
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.
Great idea! I have updated the PR
4562e37
to
3a987d4
Compare
aws/resources/route53_hostedzone.go
Outdated
marker *string | ||
) | ||
|
||
for { |
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 one could also use a paginator NewListHostedZonesPaginator
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 already updated the PR.
Add pagination support to list/nuke all hosted zones as the list response is truncated when there many hosted zones exist.
3a987d4
to
5e875b9
Compare
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. Will trigger tests for this.
Passed all the tests. Good to merge the change now. Thanks for the contribution! |
Description
Add pagination support for Route53HostedZones resource to list and delete all existing resources. Currently, the list operation did only a single request so it was only able to delete 100 HostedZones/Records (default max number of the element returned by the
ListHostedZone
/ListResourceRecordSets
API calls.Ref: https://github.com/aws/aws-sdk-go-v2/blob/service/route53/v1.46.3/service/route53/api_op_ListHostedZones.go#L38-L63
Fixes #817.
TODOs
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)
Add Route53HostedZones pagination support to list & delete all hosted zones [X].
Migration Guide