Add ACM Support #466

robpickerill · 2023-05-30T18:08:47Z

Description

Fixes #445.

Adds support for nuking AWS Certificate Manager Certificates.

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)

Added ACM support

Migration Guide

robpickerill · 2023-05-30T18:12:57Z

First PR, so feedback is welcome. I wasn't sure how to add tests here, it looks like you create resources in your test accounts then delete them - did you want to wire up a test domain here for the validation for certificates, or I can refactor the code that discovers the certificates to allow for us to mock the ACM API response via an interface, passing in ListCertificatesOutput that we create.

robpickerill · 2023-05-31T17:34:13Z

mock the ACM API response

I opted to mock the ACM API via the httptest package, passing the URL to the session struct that is then passed into the Cloud Nuke ACM functions.

I figured if you wanted to wire in a certification and domain validation then it could be tested behind a flag as certs can be slow to provision and validate. I also didn't have permission to set up any certifications or domain validation, so did what I could here.

Please let me know what you think.

ellisonc · 2023-06-01T18:56:42Z

@robpickerill This is looking great, thanks for putting this together. I think the approach you took to testing makes sense given the limitations, we will likely set something up on our internal testing accounts eventually. I think the only thing missing is adding a new line to config/config_test.go to allow the tests to build correctly. You'll see a bunch of rows like this: ResourceType{FilterRule{}, FilterRule{}}, and just need to add another one to the end to make the counts match up.

Once that's fixed I'll kick of the tests and get this merged in!

robpickerill · 2023-06-01T19:15:30Z

Thanks @ellisonc - let me patch up the config tests shortly then you can run it through CI

robpickerill · 2023-06-01T19:40:01Z

Tests failed due to: TooManyRequestsException: Too Many Requests in the APIGateway test suite. I can push an empty git commit to restart the test job if you want that.

Edit: still the same so I dropped the commit to keep history clean, I'll look into the session creation later, maybe we can increase the exponential backoff on the retryer.

…estapi call

robpickerill · 2023-06-04T18:01:47Z

aws/apigateway_test.go

+			// The defaultretryer is overridden here as the default values for MinRetryDelay and MinThrottleDelay
+			// are too low for the API Gateway API.
+			// https://pkg.go.dev/github.com/aws/[email protected]/aws/client#DefaultRetryer
+			Retryer: client.DefaultRetryer{


I'm not sure if you'd want this here, but offering the ability to increase the min retry on the session as a way to get over the rate limit exceptions on the API Gateway createrestapi in the tests.

If useful it would be better as a generic test function, that all tests can reference.

Also happy to back this out if you don't want to adjust the session config.

Thanks for bringing this up. Can you create a new issue to introduce retry logic similar to this? I think it would be better to revert this change for this PR, as it's irrelevant to ACM support.

robpickerill · 2023-06-10T15:17:14Z

@ellisonc - if you have any free time, would you have another look over this PR. The apigateway tests failed due to throttling, I added a change to try to accomodate it, but I'm not sure if you want to modify the session.

Thanks!

james03160927

LGTM. Left one small nit comment. Will trigger the test pipeline for this.

james03160927 · 2023-06-28T01:16:35Z

aws/apigateway_test.go

+			// The defaultretryer is overridden here as the default values for MinRetryDelay and MinThrottleDelay
+			// are too low for the API Gateway API.
+			// https://pkg.go.dev/github.com/aws/[email protected]/aws/client#DefaultRetryer
+			Retryer: client.DefaultRetryer{


Thanks for bringing this up. Can you create a new issue to introduce retry logic similar to this? I think it would be better to revert this change for this PR, as it's irrelevant to ACM support.

james03160927 · 2023-06-28T01:16:59Z

aws/acm_types.go

+	return "acm"
+}
+
+// ResourceIdentifiers - The volume ids of the ebs volumes


is the resource identifiers referring to ebs volumes?

thanks, fixed that comment

… createrestapi call" This reverts commit 81428c5.

zackproser · 2023-06-28T15:11:25Z

Thanks @robpickerill! I've kicked off tests for this one.

james03160927

Test pipeline passed. LGTM

james03160927 · 2023-06-30T20:15:52Z

Thanks for the contribution @robpickerill !

add acm

ed54920

robpickerill requested review from zackproser, pete0emerson, arsci, ellisonc, james03160927 and denis256 as code owners May 30, 2023 18:08

robpickerill added 4 commits May 30, 2023 19:13

fix acm comment

a442e04

docs: add acm

7761406

add initial tests

dbc8882

add tests that mock ACM APIs

33c0fa6

add check for inuse

d8aeb21

fix empty config test

fb247ae

robpickerill changed the title ~~[WIP] add ACM~~ Add ACM Support Jun 1, 2023

fix apigateway_test rate limiting by relaxing retry delay for creater…

81428c5

…estapi call

robpickerill commented Jun 4, 2023

View reviewed changes

Merge branch 'gruntwork-io:master' into master

6f01ca2

james03160927 reviewed Jun 28, 2023

View reviewed changes

robpickerill added 3 commits June 28, 2023 06:44

Revert "fix apigateway_test rate limiting by relaxing retry delay for…

ad84ddd

… createrestapi call" This reverts commit 81428c5.

fix comment for resource identifiers

cacb526

add record for termination

1a7ff9d

robpickerill added 2 commits June 28, 2023 17:36

fix missing resourcetype

85a3b0b

resolve merge conflict

78fe00e

fix config test from merge

b969597

james03160927 approved these changes Jun 30, 2023

View reviewed changes

james03160927 merged commit 8d9464b into gruntwork-io:master Jun 30, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ACM Support #466

Add ACM Support #466

robpickerill commented May 30, 2023 •

edited

Loading

robpickerill commented May 30, 2023 •

edited

Loading

robpickerill commented May 31, 2023 •

edited

Loading

ellisonc commented Jun 1, 2023

robpickerill commented Jun 1, 2023

robpickerill commented Jun 1, 2023 •

edited

Loading

robpickerill Jun 4, 2023

james03160927 Jun 28, 2023

robpickerill commented Jun 10, 2023

james03160927 left a comment

james03160927 Jun 28, 2023

james03160927 Jun 28, 2023

robpickerill Jun 28, 2023

zackproser commented Jun 28, 2023

james03160927 left a comment

james03160927 commented Jun 30, 2023

Add ACM Support #466

Add ACM Support #466

Conversation

robpickerill commented May 30, 2023 • edited Loading

Description

TODOs

Release Notes (draft)

Migration Guide

robpickerill commented May 30, 2023 • edited Loading

robpickerill commented May 31, 2023 • edited Loading

ellisonc commented Jun 1, 2023

robpickerill commented Jun 1, 2023

robpickerill commented Jun 1, 2023 • edited Loading

robpickerill Jun 4, 2023

Choose a reason for hiding this comment

james03160927 Jun 28, 2023

Choose a reason for hiding this comment

robpickerill commented Jun 10, 2023

james03160927 left a comment

Choose a reason for hiding this comment

james03160927 Jun 28, 2023

Choose a reason for hiding this comment

james03160927 Jun 28, 2023

Choose a reason for hiding this comment

robpickerill Jun 28, 2023

Choose a reason for hiding this comment

zackproser commented Jun 28, 2023

james03160927 left a comment

Choose a reason for hiding this comment

james03160927 commented Jun 30, 2023

robpickerill commented May 30, 2023 •

edited

Loading

robpickerill commented May 30, 2023 •

edited

Loading

robpickerill commented May 31, 2023 •

edited

Loading

robpickerill commented Jun 1, 2023 •

edited

Loading