Fix Route53HostedZone loopvar scoping

[sbocinec]

Description

I was testing the current master HEAD of cloud-nuke to test deletion of Route53 hosted zones to notice, it failed deleting any of the hosted zones in my AWS account (with 150+ hosted zones to delete). Using latest v0.37.2 worked without an issue.

Inspecting the code, I noticed the issue was introduced on migration to aws SDK v2 in PR #796:
https://github.com/gruntwork-io/cloud-nuke/pull/796/files#diff-b2873b3af2b9d6f0921199d7c781bbe53417e4264feda1aa000eecb8906cb3cdL28-R28

The change caused, all values of the HostedZonesDomains map were pointing to the same zone address, causing the deletion problem.

Additionally, due to the same loopvar issue, it was unable to delete the ChangeResourceRecordSets, as it was failing with:

  ERROR   [Failed] unable to nuke resource record set: operation error Route 53: ChangeResourceRecordSets, https response error StatusCode: 400, RequestID: d7f9a096-7811-41d6-828a-5ee2144d0af7, InvalidChangeBatch: [The request contains an invalid set of changes for a resource record set 'SOA cd2o99ufsia5fk25ebig.dev.cloud.example.net.']

Attaching the error log:
route3-hosted-zone-error.txt

TODOs

Read the Gruntwork contribution guidelines.

• Update the docs.
• Run the relevant tests successfully, including pre-commit checks.
• Ensure any 3rd party code adheres with our license policy or delete this line if its not applicable.
• Include release notes. If this PR is backward incompatible, include a migration guide.
• Attention Grunts - if this PR adds support for a new resource, ensure the nuke_sandbox and nuke_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)

Fixed Route53HostedZone loopvar scoping

Migration Guide

[wakeful]

hey @sbocinec

What’s the version of your Go tool chain? The mentioned issue should be addressed in the newer Go version -> https://go.dev/blog/loopvar-preview

Perhaps it’s worth just updating the go.mod file instead?

[sbocinec]

hey @sbocinec

What’s the version of your Go tool chain? The mentioned issue should be addressed in the newer Go version -> https://go.dev/blog/loopvar-preview

Perhaps it’s worth just updating the go.mod file instead?

I had only go 1.22.4 version available on the laptop where I build cloud-nuke, but the project is explicit about the go toolchain version (using 1.21 atm https://github.com/gruntwork-io/cloud-nuke/blob/master/go.mod#L3 ).

[wakeful]

the project is explicit about the go toolchain version (using 1.21 atm https://github.com/gruntwork-io/cloud-nuke/blob/master/go.mod#L3 ).

Exactly. It seems like cloud-nuke is still built using an old toolchain. I run a custom fork with the latest Go build, which is why I suggested bumping the go.mod and .circleci/config.yml, not sure if we might encounter a similar problem with other resources - cc: @james03160927

[james03160927]

the project is explicit about the go toolchain version (using 1.21 atm https://github.com/gruntwork-io/cloud-nuke/blob/master/go.mod#L3 ).

Exactly. It seems like cloud-nuke is still built using an old toolchain. I run a custom fork with the latest Go build, which is why I suggested bumping the go.mod and .circleci/config.yml, not sure if we might encounter a similar problem with other resources - cc: @james03160927

I would prefer updating the go version as it could solve other unknown issue they may have been introduced with the SDK v2 migration. Will see if there's any concern from the team and update here.

[james03160927]

Here is the PR to increase the golang version to 1.22.6 #821.

[james03160927]

Here is the PR to increase the golang version to 1.22.6 #821.

This change has been merged. Do we still need this change or can we just close it?

[sbocinec]

This change has been merged. Do we still need this change or can we just close it?

@james03160927 it should be addressed now, so I'm closing the PR. Thanks for driving the change and fixing the issue globally.

[james03160927]

Sounds good. Thanks!