Refactor nuking default resources code. #673
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
45f2761 to
acd7a11
Compare
denis256
reviewed
Apr 14, 2024
aws/resources/ec2_vpc.go
Outdated
| func (v *EC2VPCs) getAll(c context.Context, configObj config.Config) ([]*string, error) { | ||
| func (v *EC2VPCs) getAll(_ context.Context, configObj config.Config) ([]*string, error) { | ||
| // Note: This filter initially handles non-default resources and can be overridden by passing the only-default filter to choose default VPCs. | ||
| var isdefault = "false" |
There was a problem hiding this comment.
make sense to use boolean variables? if left as string, in the future(during refactoring maybe) someone may try to put different value than true/false
There was a problem hiding this comment.
Hey @denis256, this is the value we pass in as part of the DescribeVpcs endpoint.
We use boolean inside the configObj for this. Essentailly you need to set either true or false as a string. That's how the API is defined.
There was a problem hiding this comment.
Yes, I was thinking of representing value before passing it to AWS, and don't allow invalid value to be set in:
var isdefault := false
...
Values: awsgo.StringSlice([]string{fmt.Sprintf("%v", isdefault})
...
...
isdefault can take only valid values, and will be transformed in string when passed to AWS
denis256
approved these changes
Apr 16, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Partially Fixing #651
TODO: make sure to use default on all EC2 resources.
TODOs
Read the Gruntwork contribution guidelines.
nuke_sandboxandnuke_phxdevopsjobs in.circleci/config.ymlhave been updated with appropriate exclusions (either directly in the job or via the.circleci/nuke_config.ymlfile) to prevent nuking IAM roles, groups, resources, etc that are important for the test accounts.Release Notes (draft)
Added / Removed / Updated [X].
Migration Guide