Skip to content
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 support to delete ec2 key pairs using cloud-nuke #379

Merged
merged 2 commits into from
Dec 13, 2022

Conversation

hongil0316
Copy link
Contributor

@hongil0316 hongil0316 commented Dec 8, 2022

Description

Fixes https://gruntwork.atlassian.net/browse/CORE-284.

Testing

  • Tested running the cloud-nuke command line locally to delete an existing EC2 key pair
aws-vault exec gruntwork-customer-access-test -- go run main.go aws --resource-type ec2-keypairs
  • Unit tests that creates & nuke an EC2 key pair.
  • Tested aws-inspect on ec2 key pair
aws-vault exec gruntwork-customer-access-test -- go run main.go inspect-aws --resource-type ec2-keypairs

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 / Removed / Updated [X].

Migration Guide

@hongil0316 hongil0316 changed the title Add support to delete ec2 key pairs using cloud-nuke [WIP] Add support to delete ec2 key pairs using cloud-nuke Dec 8, 2022
@hongil0316 hongil0316 changed the title [WIP] Add support to delete ec2 key pairs using cloud-nuke Add support to delete ec2 key pairs using cloud-nuke Dec 8, 2022
Copy link
Contributor

@ellisonc ellisonc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a folder in the repo called config that handles resources when using a config.yml file. The new resource should be added to the Config struct as well as any usages.

Readme also needs to be updated

aws/ec2_key_pair_test.go Outdated Show resolved Hide resolved
aws/ec2_key_pair_test.go Outdated Show resolved Hide resolved
)

// getAllEc2KeyPairs extracts the list of existing ec2 key pairs.
func getAllEc2KeyPairs(session *session.Session) ([]*string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function should contain logic for deciding if an ec2 key pair should be included in the list to be nuked. If you do a repo search for shouldInclude* you should find examples. This is what powers the regex and time based filtering

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah that's nice. I wonder if that should be a separate ticket to handle? https://gruntwork.atlassian.net/browse/CORE-284 -- the acceptance criteria does not seem to mention about the capability to support such feature.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a problem with adding it to a separate ticket, but I don't think we should release it without this functionality since all of the other resources support it. Especially with a tool designed for destruction like cloud-nuke. It would be very easy for someone to create a config file with a regex and accidentally delete something because the regex feature wasn't added.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah that's a great point. Let's add that as part of this if that's the case. Let me update the PR 👍

@hongil0316
Copy link
Contributor Author

There's a folder in the repo called config that handles resources when using a config.yml file. The new resource should be added to the Config struct as well as any usages.

Readme also needs to be updated

Got it. Updated.

@ellisonc
Copy link
Contributor

ellisonc commented Dec 8, 2022

Need to update the readme and add release notes to the PR

@hongil0316
Copy link
Contributor Author

Need to update the readme and add release notes to the PR

Hmm I though I updated the readme :S I guess it was not included somehow.
Don't we auto-generate the release note based on the PRs?

Is this one special because it's an open source repository?

@hongil0316 hongil0316 force-pushed the cloud-nuke/ec2_key_pairs branch 2 times, most recently from 2204f79 to 4786bfa Compare December 9, 2022 10:45
aws/ec2_key_pair.go Outdated Show resolved Hide resolved
@hongil0316 hongil0316 merged commit 43ed22b into master Dec 13, 2022
@hongil0316 hongil0316 deleted the cloud-nuke/ec2_key_pairs branch December 13, 2022 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants