-
Notifications
You must be signed in to change notification settings - Fork 152
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 aws_default_vpc and aws_vpc support #25
Conversation
Codecov Report
@@ Coverage Diff @@
## main #25 +/- ##
==========================================
- Coverage 67.06% 67.00% -0.07%
==========================================
Files 140 146 +6
Lines 3073 3179 +106
==========================================
+ Hits 2061 2130 +69
- Misses 783 809 +26
- Partials 229 240 +11
|
5dd001a
to
f8e97b3
Compare
f8e97b3
to
37efbde
Compare
ecfec35
to
1ca42a2
Compare
1ca42a2
to
35cc32a
Compare
pkg/remote/aws/vpc_supplier_test.go
Outdated
IsDefault: aws.Bool(false), | ||
}, | ||
}, | ||
}, true) |
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.
Shouldn't it be false
here ?
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.
Even though this won't change the test itself and its results, we should make it so that it's logic for reading PR purposes.
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.
Sure, I've inverted this bool meaning in my mind ππ» nice catch
return resourceaws.AwsVpcResourceType | ||
} | ||
|
||
func (s VPCDeserializer) Deserialize(recordList []cty.Value) ([]resource.Resource, error) { |
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.
You kept the recordList from Route53 records, can you change it to something more inline with vpc, like vpcList
and update all the lines below.
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.
Nice catch, let's update it for something more generic like rawList
as we use rawResource
as iterator name in the loop.
return resourceaws.AwsDefaultVpcResourceType | ||
} | ||
|
||
func (s DefaultVPCDeserializer) Deserialize(recordList []cty.Value) ([]resource.Resource, error) { |
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.
Same here for recordList
35cc32a
to
864ecc4
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
864ecc4
to
bbe91ff
Compare
Description
Add support for
aws_default_vpc
aws_vpc