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

Fix aws_ebs_encryption_by_default_reconciler middleware #1464

Merged

Conversation

sundowndev-snyk
Copy link
Contributor

@sundowndev-snyk sundowndev-snyk commented Apr 5, 2022

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

Description

A simple break statement in that middleware was preventing many remote resources to be included in the scan, resulting in many missing resources. Bug introduced in #1448 but not released yet. The reason why we didn't notice it is because we didn't test it with many remote resources, not even in unit tests.

@sundowndev-snyk sundowndev-snyk requested a review from a team as a code owner April 5, 2022 13:41
@codecov-commenter
Copy link

Codecov Report

Merging #1464 (c508c36) into main (6546ae4) will increase coverage by 7.11%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1464      +/-   ##
==========================================
+ Coverage   74.87%   81.99%   +7.11%     
==========================================
  Files         496      424      -72     
  Lines       18717    15488    -3229     
==========================================
- Hits        14014    12699    -1315     
+ Misses       4406     2476    -1930     
- Partials      297      313      +16     
Impacted Files Coverage Ξ”
...ewares/aws_ebs_encryption_by_default_reconciler.go 100.00% <ΓΈ> (ΓΈ)
pkg/terraform/plugin_client.go 0.00% <0.00%> (-100.00%) ⬇️
pkg/remote/aws/client/s3_client_factory.go 0.00% <0.00%> (-100.00%) ⬇️
pkg/remote/aws/init.go 0.00% <0.00%> (-95.78%) ⬇️
pkg/resource/azurerm/azurerm_image.go 12.50% <0.00%> (-87.50%) ⬇️
pkg/resource/azurerm/azurerm_resource_group.go 12.50% <0.00%> (-87.50%) ⬇️
pkg/resource/google/google_compute_address.go 14.28% <0.00%> (-85.72%) ⬇️
...g/resource/google/google_compute_global_address.go 14.28% <0.00%> (-85.72%) ⬇️
...ce/google/google_compute_instance_group_manager.go 14.28% <0.00%> (-85.72%) ⬇️
pkg/remote/azurerm/init.go 0.00% <0.00%> (-84.22%) ⬇️
... and 167 more

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.

Really good catch, note to myself always check with a bunch a resources without a driftignore file :)

@sundowndev-snyk sundowndev-snyk merged commit 39b0a8a into main Apr 5, 2022
@sundowndev-snyk sundowndev-snyk deleted the fix/aws_ebs_encryption_by_default_reconciler_bis branch April 5, 2022 14:01
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.

3 participants