From 77bcd9919d82473510859c4b4adec401da1fd999 Mon Sep 17 00:00:00 2001 From: Robert O'Brien Date: Tue, 25 May 2010 13:38:22 +1200 Subject: [PATCH] Enable port forwarding to be directed at a specific adapter (NIC), prevented entirely making standard use of TCP/IP and ensuring that port forwarding is only attempted when the NIC is using the VirtualBox NAT. --- config/default.rb | 1 + lib/vagrant/actions/vm/forward_ports.rb | 25 +++++++++++++------ lib/vagrant/config.rb | 6 +++-- lib/vagrant/ssh.rb | 14 +++++++++-- test/vagrant/actions/vm/forward_ports_test.rb | 19 +++++++++++++- 5 files changed, 53 insertions(+), 12 deletions(-) diff --git a/config/default.rb b/config/default.rb index 51143d93c..1eb5251dd 100644 --- a/config/default.rb +++ b/config/default.rb @@ -7,6 +7,7 @@ Vagrant::Config.run do |config| config.ssh.username = "vagrant" config.ssh.password = "vagrant" config.ssh.host = "localhost" + config.ssh.port = 22 config.ssh.forwarded_port_key = "ssh" config.ssh.max_tries = 10 config.ssh.timeout = 30 diff --git a/lib/vagrant/actions/vm/forward_ports.rb b/lib/vagrant/actions/vm/forward_ports.rb index 400e4bf18..ff6aeee28 100644 --- a/lib/vagrant/actions/vm/forward_ports.rb +++ b/lib/vagrant/actions/vm/forward_ports.rb @@ -9,7 +9,7 @@ module Vagrant vm.forwarded_ports.each do |fp| @runner.env.config.vm.forwarded_ports.each do |name, options| if fp.hostport.to_s == options[:hostport].to_s - raise ActionException.new(:vm_port_collision, :name => name, :hostport => fp.hostport.to_s, :guestport => options[:guestport].to_s) + raise ActionException.new(:vm_port_collision, :name => name, :hostport => fp.hostport.to_s, :guestport => options[:guestport].to_s, :instance => options[:instance]) end end end @@ -30,12 +30,23 @@ module Vagrant logger.info "Forwarding ports..." @runner.env.config.vm.forwarded_ports.each do |name, options| - logger.info "Forwarding \"#{name}\": #{options[:guestport]} => #{options[:hostport]}" - port = VirtualBox::ForwardedPort.new - port.name = name - port.hostport = options[:hostport] - port.guestport = options[:guestport] - @runner.vm.forwarded_ports << port + adapter = options[:instance] + + # Assuming the only reason to establish port forwarding is because the VM is using Virtualbox NAT networking. + # Host-only or Bridged networking don't require port-forwarding and establishing forwarded ports on these + # attachment types has uncertain behaviour. + if @runner.vm.network_adapters[adapter].attachment_type == :nat + logger.info "Forwarding \"#{name}\": #{options[:guestport]} on Adapter\##{adapter+1} => #{options[:hostport]}" + port = VirtualBox::ForwardedPort.new + port.name = name + port.hostport = options[:hostport] + port.guestport = options[:guestport] + port.instance = adapter + @runner.vm.forwarded_ports << port + else + logger.info "VirtualBox Adapter\##{adapter+1} not configured as \"NAT\"." + logger.info "Skipped setting forwarding \"#{name}\": #{options[:guestport]} on Adapter\##{adapter+1} => #{options[:hostport]}" + end end @runner.vm.save diff --git a/lib/vagrant/config.rb b/lib/vagrant/config.rb index ac22ab9dc..b10d2eb22 100644 --- a/lib/vagrant/config.rb +++ b/lib/vagrant/config.rb @@ -63,6 +63,7 @@ module Vagrant attr_accessor :username attr_accessor :password attr_accessor :host + attr_accessor :port attr_accessor :forwarded_port_key attr_accessor :max_tries attr_accessor :timeout @@ -101,11 +102,12 @@ module Vagrant @provisioner = nil end - def forward_port(name, guestport, hostport, protocol="TCP") + def forward_port(name, guestport, hostport, protocol="TCP", instance=0) forwarded_ports[name] = { :guestport => guestport, :hostport => hostport, - :protocol => protocol + :protocol => protocol, + :instance => instance } end diff --git a/lib/vagrant/ssh.rb b/lib/vagrant/ssh.rb index 0c1ae59ce..a221b5361 100644 --- a/lib/vagrant/ssh.rb +++ b/lib/vagrant/ssh.rb @@ -114,9 +114,19 @@ module Vagrant # Returns the port which is either given in the options hash or taken from # the config by finding it in the forwarded ports hash based on the - # `config.ssh.forwarded_port_key` + # `config.ssh.forwarded_port_key` or use the default port given by `config.ssh.port` + # when port forwarding isn't used. def port(opts={}) - opts[:port] || env.config.vm.forwarded_ports[env.config.ssh.forwarded_port_key][:hostport] + # Check if port was specified in options hash + pnum = opts[:port] + return pnum if pnum + + # Check if we have an SSH forwarded port + pnum = env.config.vm.forwarded_ports[env.config.ssh.forwarded_port_key] + return pnum[:hostport] if pnum + + # Fall back to the default + return env.config.ssh.port end end end diff --git a/test/vagrant/actions/vm/forward_ports_test.rb b/test/vagrant/actions/vm/forward_ports_test.rb index 878c209e8..1de0fb053 100644 --- a/test/vagrant/actions/vm/forward_ports_test.rb +++ b/test/vagrant/actions/vm/forward_ports_test.rb @@ -71,12 +71,17 @@ class ForwardPortsActionTest < Test::Unit::TestCase context "forwarding ports" do should "create a port forwarding for the VM" do forwarded_ports = mock("forwarded_ports") - + network_adapter = mock("network_adapter") + + @vm.expects(:network_adapters).returns([network_adapter]) + network_adapter.expects(:attachment_type).returns(:nat) + @mock_vm.env.config.vm.forwarded_ports.each do |name, opts| forwarded_ports.expects(:<<).with do |port| assert_equal name, port.name assert_equal opts[:hostport], port.hostport assert_equal opts[:guestport], port.guestport + assert_equal opts[:instance], port.instance true end end @@ -87,6 +92,18 @@ class ForwardPortsActionTest < Test::Unit::TestCase end end + context "Not forwarding ports" do + should "No port forwarding for non NAT interfaces" do + forwarded_ports = mock("forwarded_ports") + network_adapter = mock("network_adapter") + + @vm.expects(:network_adapters).returns([network_adapter]) + network_adapter.expects(:attachment_type).returns(:host_only) + @vm.expects(:save).once + @action.forward_ports + end + end + context "clearing forwarded ports" do should "call destroy on all forwarded ports" do forwarded_ports = []