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 resource factory #389

Merged
merged 2 commits into from
Apr 7, 2021
Merged

Add resource factory #389

merged 2 commits into from
Apr 7, 2021

Conversation

eliecharra
Copy link
Contributor

Q A
πŸ› Bug fix? no
πŸš€ New feature? no
⚠ Deprecations? no
❌ BC Break no
πŸ”— Related issues #347
❓ Documentation no

Description

  • Add ResourceFactory
  • Update middlewares to use resource factory and thus resources created from middlewares can now be filtered

@eliecharra eliecharra added the kind/maintenance Refactoring or changes to the workspace label Mar 29, 2021
@eliecharra eliecharra added this to the v0.8.0 milestone Mar 29, 2021
@eliecharra eliecharra requested a review from a team March 29, 2021 16:13
@codecov
Copy link

codecov bot commented Mar 29, 2021

Codecov Report

Merging #389 (406a6c8) into main (141ffbe) will increase coverage by 0.01%.
The diff coverage is 66.84%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #389      +/-   ##
==========================================
+ Coverage   70.23%   70.25%   +0.01%     
==========================================
  Files         281      282       +1     
  Lines        6149     6277     +128     
==========================================
+ Hits         4319     4410      +91     
- Misses       1476     1503      +27     
- Partials      354      364      +10     
Impacted Files Coverage Ξ”
pkg/cmd/scan.go 81.90% <0.00%> (-0.38%) ⬇️
pkg/resource/resource.go 80.00% <ΓΈ> (ΓΈ)
pkg/terraform/providers.go 27.77% <0.00%> (-22.23%) ⬇️
pkg/terraform/resource_factory.go 0.00% <0.00%> (ΓΈ)
pkg/middlewares/vpc_security_group_rules.go 65.62% <58.49%> (+4.33%) ⬆️
pkg/middlewares/aws_bucket_policy_expander.go 80.95% <80.00%> (-0.87%) ⬇️
pkg/middlewares/aws_sns_topic_policy_expander.go 80.95% <80.00%> (-0.87%) ⬇️
pkg/middlewares/aws_sqs_queue_policy_expander.go 85.71% <80.00%> (-2.17%) ⬇️
pkg/middlewares/aws_instance_block_device.go 94.36% <86.20%> (-5.64%) ⬇️
pkg/middlewares/aws_route_table_expander.go 89.39% <90.90%> (+0.75%) ⬆️
... and 2 more

@eliecharra eliecharra force-pushed the add_resource_factory branch 2 times, most recently from 17fe6cc to 4dcd669 Compare April 1, 2021 09:00
@eliecharra eliecharra marked this pull request as ready for review April 6, 2021 14:44
@@ -118,12 +123,21 @@ func TestAwsInstanceBlockDeviceResourceMapper_Execute(t *testing.T) {
},
},
},
func(factory *terraform.MockResourceFactory) {
factory.On("CreateResource", mock.Anything, "aws_ebs_volume").Times(2).Return(nil, nil)
Copy link
Contributor

Choose a reason for hiding this comment

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

Just FYI, you can use here Twice() instead of Times(2) but no need to change it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I know but I prefer to always use Times

Copy link
Contributor

Choose a reason for hiding this comment

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

If i == 1 you use Once and if i > 1 Times(i) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah and I was wondering myself if I should use Times everywhere

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed, it is more explicit.

Copy link
Contributor

@wbeuil wbeuil left a comment

Choose a reason for hiding this comment

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

Nice one ! I will push really soon a PR that start testing those middlewares.

@eliecharra eliecharra merged commit e7f5c1a into main Apr 7, 2021
@eliecharra eliecharra deleted the add_resource_factory branch April 7, 2021 08:51
@eliecharra eliecharra linked an issue Apr 8, 2021 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/maintenance Refactoring or changes to the workspace
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Terraform AWS Provider v2 support
2 participants