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.

Sign up for GitHub

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

Jump to bottom

Add aws_lb & aws_alb #1450

Merged
merged 2 commits into from
Apr 7, 2022
Merged

Add aws_lb & aws_alb #1450

merged 2 commits into from
Apr 7, 2022

Conversation

sundowndev-snyk
Copy link
Contributor

@sundowndev-snyk sundowndev-snyk commented Mar 30, 2022 •
edited
Loading

Q A
πŸ› Bug fix? no
πŸš€ New feature? yes
⚠ Deprecations? no
❌ BC Break no
πŸ”— Related issues #1393
❓ Documentation snyk/driftctl-docs#233

Description

This PR add support for aws_lb and aws_alb, which both provide the exact same functionality.

@sundowndev-snyk sundowndev-snyk added the kind/enhancement New feature or improvement label Mar 30, 2022
@codecov-commenter
Copy link

codecov-commenter commented Mar 30, 2022 •
edited
Loading

Codecov Report

Merging #1450 (d22f668) into main (b54c694) will decrease coverage by 0.00%.
The diff coverage is 80.82%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1450      +/-   ##
==========================================
- Coverage   81.95%   81.94%   -0.01%     
==========================================
  Files         423      427       +4     
  Lines       15413    15486      +73     
==========================================
+ Hits        12631    12690      +59     
- Misses       2471     2484      +13     
- Partials      311      312       +1
Impacted Files Coverage Ξ”
pkg/remote/aws/init.go 0.00% <0.00%> (ΓΈ)
pkg/resource/resource_types.go 100.00% <ΓΈ> (ΓΈ)
pkg/resource/aws/aws_lb.go 16.66% <16.66%> (ΓΈ)
pkg/remote/aws/repository/elbv2_repository.go 68.75% <68.75%> (ΓΈ)
pkg/driftctl.go 74.80% <100.00%> (+0.19%) ⬆️
pkg/middlewares/aws_alb_transformer.go 100.00% <100.00%> (ΓΈ)
pkg/remote/aws/load_balancer_enumerator.go 100.00% <100.00%> (ΓΈ)
pkg/resource/aws/metadatas.go 100.00% <100.00%> (ΓΈ)

@sundowndev-snyk sundowndev-snyk marked this pull request as ready for review March 30, 2022 13:05
@sundowndev-snyk sundowndev-snyk requested a review from a team as a code owner March 30, 2022 13:05
This was linked to issues Mar 30, 2022
Support AWS resource: aws_alb #1391
Closed
Support AWS resource: aws_lb #1393
Closed
@sundowndev-snyk sundowndev-snyk requested a review from a team as a code owner April 4, 2022 10:29
wbeuil
wbeuil suggested changes Apr 5, 2022
View reviewed changes
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.

Can you add tests in the metadata.test.go file.

PS: can you add also the one for EBS encryption as well since I missed this one in your other resource. Or in another PR, as you like.

pkg/remote/aws/init.go Outdated Show resolved Hide resolved
pkg/remote/aws/init.go Outdated Show resolved Hide resolved
pkg/remote/elb_scanner_test.go Outdated Show resolved Hide resolved
pkg/remote/aws/repository/elbv2_repository.go Outdated Show resolved Hide resolved
pkg/remote/aws/repository/elbv2_repository.go Outdated Show resolved Hide resolved
test/aws/elbv2.go Show resolved Hide resolved
pkg/middlewares/aws_alb_transformer.go Outdated Show resolved Hide resolved
pkg/middlewares/aws_alb_transformer.go Outdated Show resolved Hide resolved
wbeuil
wbeuil suggested changes Apr 6, 2022
View reviewed changes
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.

Still misses the tests under the metadata.test.go.

#1450 (review)

test/aws/elbv2.go Outdated Show resolved Hide resolved
pkg/remote/aws/repository/mock_ELBv2Repository.go Outdated Show resolved Hide resolved
pkg/remote/elb_scanner_test.go Outdated Show resolved Hide resolved
pkg/remote/elb_scanner_test.go Outdated Show resolved Hide resolved
wbeuil
wbeuil suggested changes Apr 7, 2022
View reviewed changes
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.

Can you rename the scanner file to aws_elbv2_scanner_test.go (look at the prefix AWS)

...alancing_us-east-1_533948124879_loadbalancer_app_test-lb-tf_35be57d467ff22b9.res.golden.json Outdated Show resolved Hide resolved
pkg/remote/test/aws_lb_list/results.golden.json Outdated Show resolved Hide resolved
pkg/remote/elbv2_scanner_test.go Outdated Show resolved Hide resolved
@sundowndev-snyk
test(aws): aws_lb
c42fc3d 
Delete unnecesary files and improve tests for that resource.
wbeuil
wbeuil approved these changes Apr 7, 2022
View reviewed changes
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.

LGTM

@sundowndev-snyk sundowndev-snyk merged commit 181e8a1 into main Apr 7, 2022
@sundowndev-snyk sundowndev-snyk deleted the feat/add_aws_lb branch April 7, 2022 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wbeuil wbeuil wbeuil approved these changes

Assignees
No one assigned
Labels
kind/enhancement New feature or improvement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support AWS resource: aws_lb Support AWS resource: aws_alb
3 participants
@sundowndev-snyk @codecov-commenter @wbeuil