Move port checker method to class rather than instance

This commit also moves out the ipv4_interfaces method to be a util
method, so that the port checker can also access it as a class mehtod
This commit is contained in:
Brian Cain 2020-05-12 10:54:23 -07:00
parent b2d9abe344
commit 5aff6660fb
No known key found for this signature in database
GPG Key ID: 9FC4639B2E4510A0
6 changed files with 86 additions and 70 deletions

View File

@ -4,6 +4,7 @@ require "log4r"
require "socket"
require "vagrant/util/is_port_open"
require "vagrant/util/ipv4_interfaces"
module Vagrant
module Action
@ -26,6 +27,7 @@ module Vagrant
#
class HandleForwardedPortCollisions
include Util::IsPortOpen
include Util::IPv4Interfaces
def initialize(app, env)
@app = app
@ -254,22 +256,19 @@ module Vagrant
return extra_in_use.fetch(hostport).include?(hostip)
end
def ipv4_interfaces
Socket.getifaddrs.select do |ifaddr|
ifaddr.addr && ifaddr.addr.ipv4?
end.map do |ifaddr|
[ifaddr.name, ifaddr.addr.ip_address]
end
def port_check(host_ip, host_port)
self.class.port_check(@machine, host_ip, host_port)
end
def port_check(host_ip, host_port)
def self.port_check(machine, host_ip, host_port)
@logger = Log4r::Logger.new("vagrant::action::builtin::handle_port_collisions")
# If no host_ip is specified, intention taken to be listen on all interfaces.
test_host_ip = host_ip || "0.0.0.0"
if Util::Platform.windows? && test_host_ip == "0.0.0.0"
@logger.debug("Testing port #{host_port} on all IPv4 interfaces...")
available_interfaces = ipv4_interfaces.select do |interface|
available_interfaces = Vagrant::Util::IPv4Interfaces.ipv4_interfaces.select do |interface|
@logger.debug("Testing #{interface[0]} with IP address #{interface[1]}")
!is_port_open?(interface[1], host_port)
!Vagrant::Util::IsPortOpen.is_port_open?(interface[1], host_port)
end
if available_interfaces.empty?
@logger.debug("Cannot forward port #{host_port} on any interfaces.")
@ -280,10 +279,10 @@ module Vagrant
end
else
# Do a regular check
if test_host_ip == "0.0.0.0" || ipv4_interfaces.detect { |iface| iface[1] == test_host_ip }
is_port_open?(test_host_ip, host_port)
if test_host_ip == "0.0.0.0" || Vagrant::Util::IPv4Interfaces.ipv4_interfaces.detect { |iface| iface[1] == test_host_ip }
Vagrant::Util::IsPortOpen.is_port_open?(test_host_ip, host_port)
else
raise Errors::ForwardPortHostIPNotFound, name: @machine.name, host_ip: host_ip
raise Errors::ForwardPortHostIPNotFound, name: machine.name, host_ip: host_ip
end
end
end

View File

@ -0,0 +1,13 @@
module Vagrant
module Util
module IPv4Interfaces
def self.ipv4_interfaces
Socket.getifaddrs.select do |ifaddr|
ifaddr.addr && ifaddr.addr.ipv4?
end.map do |ifaddr|
[ifaddr.name, ifaddr.addr.ip_address]
end
end
end
end
end

View File

@ -13,7 +13,7 @@ module Vagrant
# @param [Integer] port Port to check.
# @return [Boolean] `true` if the port is open (listening), `false`
# otherwise.
def is_port_open?(host, port)
def self.is_port_open?(host, port)
# We wrap this in a timeout because once in awhile the TCPSocket
# _will_ hang, but this signals that the port is closed.
Timeout.timeout(1) do

View File

@ -79,7 +79,7 @@ describe Vagrant::Action::Builtin::HandleForwardedPortCollisions do
it "should check if host port is in use" do
expect(instance).to receive(:is_forwarded_already).and_return(false)
expect(instance).to receive(:is_port_open?).and_return(false)
expect(Vagrant::Util::IsPortOpen).to receive(:is_port_open?).and_return(false)
instance.call(env)
end
@ -147,45 +147,6 @@ describe Vagrant::Action::Builtin::HandleForwardedPortCollisions do
describe "#recover" do
end
describe "#ipv4_interfaces" do
let(:name) { double("name") }
let(:address) { double("address") }
let(:ipv4_ifaddr) do
double("ipv4_ifaddr").tap do |ifaddr|
allow(ifaddr).to receive(:name).and_return(name)
allow(ifaddr).to receive_message_chain(:addr, :ipv4?).and_return(true)
allow(ifaddr).to receive_message_chain(:addr, :ip_address).and_return(address)
end
end
let(:ipv6_ifaddr) do
double("ipv6_ifaddr").tap do |ifaddr|
allow(ifaddr).to receive(:name)
allow(ifaddr).to receive_message_chain(:addr, :ipv4?).and_return(false)
end
end
let(:ifaddrs) { [ ipv4_ifaddr, ipv6_ifaddr ] }
before do
allow(Socket).to receive(:getifaddrs).and_return(ifaddrs)
end
it "returns a list of IPv4 interfaces with their names and addresses" do
expect(instance.send(:ipv4_interfaces)).to eq([ [name, address] ])
end
context "with nil interface address" do
let(:nil_ifaddr) { double("nil_ifaddr", addr: nil ) }
let(:ifaddrs) { [ ipv4_ifaddr, ipv6_ifaddr, nil_ifaddr ] }
it "filters out nil addr info" do
expect(instance.send(:ipv4_interfaces)).to eq([ [name, address] ])
end
end
end
describe "#port_check" do
let(:host_ip){ "127.0.0.1" }
let(:host_port){ 8080 }
@ -193,11 +154,11 @@ describe Vagrant::Action::Builtin::HandleForwardedPortCollisions do
before do
instance.instance_variable_set(:@machine, machine)
allow(instance).to receive(:ipv4_interfaces).and_return(interfaces)
allow(Vagrant::Util::IPv4Interfaces).to receive(:ipv4_interfaces).and_return(interfaces)
end
it "should check if the port is open" do
expect(instance).to receive(:is_port_open?).with(host_ip, host_port).and_return(true)
expect(Vagrant::Util::IsPortOpen).to receive(:is_port_open?).with(host_ip, host_port).and_return(true)
instance.send(:port_check, host_ip, host_port)
end
@ -210,23 +171,23 @@ describe Vagrant::Action::Builtin::HandleForwardedPortCollisions do
end
it "should check the port on every IPv4 interface" do
expect(instance).to receive(:is_port_open?).with(interfaces[0][1], host_port)
expect(instance).to receive(:is_port_open?).with(interfaces[1][1], host_port)
expect(Vagrant::Util::IsPortOpen).to receive(:is_port_open?).with(interfaces[0][1], host_port)
expect(Vagrant::Util::IsPortOpen).to receive(:is_port_open?).with(interfaces[1][1], host_port)
instance.send(:port_check, host_ip, host_port)
end
it "should return false if the port is closed on any IPv4 interfaces" do
expect(instance).to receive(:is_port_open?).with(interfaces[0][1], host_port).
expect(Vagrant::Util::IsPortOpen).to receive(:is_port_open?).with(interfaces[0][1], host_port).
and_return(true)
expect(instance).to receive(:is_port_open?).with(interfaces[1][1], host_port).
expect(Vagrant::Util::IsPortOpen).to receive(:is_port_open?).with(interfaces[1][1], host_port).
and_return(false)
expect(instance.send(:port_check, host_ip, host_port)).to be(false)
end
it "should return true if the port is open on all IPv4 interfaces" do
expect(instance).to receive(:is_port_open?).with(interfaces[0][1], host_port).
expect(Vagrant::Util::IsPortOpen).to receive(:is_port_open?).with(interfaces[0][1], host_port).
and_return(true)
expect(instance).to receive(:is_port_open?).with(interfaces[1][1], host_port).
expect(Vagrant::Util::IsPortOpen).to receive(:is_port_open?).with(interfaces[1][1], host_port).
and_return(true)
expect(instance.send(:port_check, host_ip, host_port)).to be(true)
end

View File

@ -0,0 +1,47 @@
require File.expand_path("../../../base", __FILE__)
require "vagrant/util/ipv4_interfaces"
describe Vagrant::Util::IPv4Interfaces do
subject { described_class }
describe "#ipv4_interfaces" do
let(:name) { double("name") }
let(:address) { double("address") }
let(:ipv4_ifaddr) do
double("ipv4_ifaddr").tap do |ifaddr|
allow(ifaddr).to receive(:name).and_return(name)
allow(ifaddr).to receive_message_chain(:addr, :ipv4?).and_return(true)
allow(ifaddr).to receive_message_chain(:addr, :ip_address).and_return(address)
end
end
let(:ipv6_ifaddr) do
double("ipv6_ifaddr").tap do |ifaddr|
allow(ifaddr).to receive(:name)
allow(ifaddr).to receive_message_chain(:addr, :ipv4?).and_return(false)
end
end
let(:ifaddrs) { [ ipv4_ifaddr, ipv6_ifaddr ] }
before do
allow(Socket).to receive(:getifaddrs).and_return(ifaddrs)
end
it "returns a list of IPv4 interfaces with their names and addresses" do
expect(subject.ipv4_interfaces).to eq([ [name, address] ])
end
context "with nil interface address" do
let(:nil_ifaddr) { double("nil_ifaddr", addr: nil ) }
let(:ifaddrs) { [ ipv4_ifaddr, ipv6_ifaddr, nil_ifaddr ] }
it "filters out nil addr info" do
expect(subject.ipv4_interfaces).to eq([ [name, address] ])
end
end
end
end

View File

@ -5,11 +5,7 @@ require "socket"
require "vagrant/util/is_port_open"
describe Vagrant::Util::IsPortOpen do
let(:klass) do
Class.new do
extend Vagrant::Util::IsPortOpen
end
end
subject { described_class }
let(:open_port) { 52811 }
let(:closed_port) { 52811 }
@ -36,7 +32,7 @@ describe Vagrant::Util::IsPortOpen do
end
# Verify that we report the port is open
expect(klass.is_port_open?("127.0.0.1", open_port)).to be
expect(subject.is_port_open?("127.0.0.1", open_port)).to be
# Kill the thread
thr[:die] = true
@ -47,17 +43,17 @@ describe Vagrant::Util::IsPortOpen do
# This CAN fail, since port 52811 might actually be in use, but I'm
# not sure what to do except choose some random port and hope for the
# best, really.
expect(klass.is_port_open?("127.0.0.1", closed_port)).not_to be
expect(subject.is_port_open?("127.0.0.1", closed_port)).not_to be
end
it "should handle connection refused" do
expect(TCPSocket).to receive(:new).with("0.0.0.0", closed_port).and_raise(Errno::ECONNREFUSED)
expect(klass.is_port_open?("0.0.0.0", closed_port)).to be(false)
expect(subject.is_port_open?("0.0.0.0", closed_port)).to be(false)
end
it "should raise an error if cannot assign requested address" do
expect(TCPSocket).to receive(:new).with("0.0.0.0", open_port).and_raise(Errno::EADDRNOTAVAIL)
expect { klass.is_port_open?("0.0.0.0", open_port) }.to raise_error(Errno::EADDRNOTAVAIL)
expect { subject.is_port_open?("0.0.0.0", open_port) }.to raise_error(Errno::EADDRNOTAVAIL)
end
end