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

Find all addresses when multiple are attached to a single interface. #30

Closed
wants to merge 2 commits into from

Conversation

breser
Copy link

@breser breser commented May 10, 2019

Currently this code would only find the first public ip address
attached to a network interface rather than finding all of them.

We properly iterate the private_ip_addresses which can have associations
with separate public ips. Spec has been updated with a test case
showing this as well.

Fixes issue #28

Currently this code would only find the first public ip address
attached to a network interface rather than finding all of them.

We properly iterate the private_ip_addresses which can have associations
with separate public ips.  Spec has been updated with a test case
showing this as well.
@coveralls
Copy link

coveralls commented May 10, 2019

Pull Request Test Coverage Report for Build 117

  • 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 116: 0.0%
Covered Lines: 1092
Relevant Lines: 1092

💛 - Coveralls

@arkadiyt
Copy link
Owner

Thanks for sending this! If you fix up the rubocop warnings / get the build passing then I can merge this in and deploy a new version.

You could either give me access to edit this pull request (like here) and I can add the commit, or alternatively you could apply this patch:

diff --git a/lib/aws_public_ips/checks/ec2.rb b/lib/aws_public_ips/checks/ec2.rb
index e869841..5f01525 100644
--- a/lib/aws_public_ips/checks/ec2.rb
+++ b/lib/aws_public_ips/checks/ec2.rb
@@ -23,7 +23,7 @@ module AwsPublicIps
                 public_ip = []

                 interface.private_ip_addresses.flat_map do |private_ip|
-                  if private_ip.association and private_ip.association.public_ip
+                  if private_ip.association && private_ip.association.public_ip
                     public_ip << private_ip.association.public_ip
                   end
                 end
diff --git a/spec/aws_public_ips/checks/ec2_spec.rb b/spec/aws_public_ips/checks/ec2_spec.rb
index 5210e7f..12c2176 100644
--- a/spec/aws_public_ips/checks/ec2_spec.rb
+++ b/spec/aws_public_ips/checks/ec2_spec.rb
@@ -21,7 +21,8 @@ describe ::AwsPublicIps::Checks::Ec2 do
         hostname: nil,
         ip_addresses: %w[2600:1f18:60f3:eb00:1c6e:5184:8955:170c]
       },
-      { id: 'i-01e4cbbe2c7fb98f6',
+      {
+        id: 'i-01e4cbbe2c7fb98f6',
         hostname: 'ec2-50-112-85-68.us-west-2.compute.amazonaws.com',
         ip_addresses: %w[50.112.85.68 54.214.97.117]
       }

@arkadiyt
Copy link
Owner

arkadiyt commented Nov 7, 2019

I've gone ahead and applied the resolved patch at #36, attributed to you. Thanks for the PR!

@breser
Copy link
Author

breser commented Dec 9, 2019

Thanks for applying the fix. Sorry I was busy and didn't get to this before you did.

@arkadiyt
Copy link
Owner

No worries - it took me months to get to your original PR so I'm just as guilty.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants