From ae0d9935a77cf6acfb2430c6cfc13cef9e86a4a3 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 2 Jul 2010 08:37:13 -0700 Subject: [PATCH] Detect if any collisions with non-hostonly networks for networking. [closes GH-102] --- lib/vagrant/actions/vm/network.rb | 25 ++++++++++ templates/strings.yml | 5 ++ test/vagrant/actions/vm/network_test.rb | 66 +++++++++++++++++++------ vagrant.gemspec | 2 +- 4 files changed, 81 insertions(+), 17 deletions(-) diff --git a/lib/vagrant/actions/vm/network.rb b/lib/vagrant/actions/vm/network.rb index 085c1aaa4..13b928a92 100644 --- a/lib/vagrant/actions/vm/network.rb +++ b/lib/vagrant/actions/vm/network.rb @@ -2,6 +2,13 @@ module Vagrant module Actions module VM class Network < Base + def prepare + # Verify that the given network options are valid + runner.env.config.vm.network_options.compact.each do |network_options| + verify_no_bridge_collision(network_options) + end + end + def before_destroy # We need to check if the host only network specified by any # of the adapters would not have any more clients if it was @@ -55,6 +62,24 @@ module Vagrant end end + # Verifies that there is no collision with a bridged network interface + # for the given network options. + def verify_no_bridge_collision(net_options) + # First try to find a matching network + interfaces = VirtualBox::Global.global.host.network_interfaces + interfaces.each do |ni| + next if ni.interface_type == :host_only + + result = if net_options[:name] + true if net_options[:name] == ni.name + else + true if matching_network?(ni, net_options) + end + + raise ActionException.new(:network_collides) if result + end + end + # Returns the name of the proper host only network, or creates # it if it does not exist. Vagrant determines if the host only # network exists by comparing the netmask and the IP. diff --git a/templates/strings.yml b/templates/strings.yml index cb5cabf06..674b5c6cf 100644 --- a/templates/strings.yml +++ b/templates/strings.yml @@ -142,6 +142,11 @@ If the name specification is removed, Vagrant will create a new host only network for you. Alternatively, please create the specified network manually. +:network_collides: |- + The specified host network collides with a non-hostonly network! + This will cause your specified IP to be inaccessible. Please change + the IP or name of your host only network to not match that of + a bridged or non-hostonly network. :package_include_file_doesnt_exist: |- File specified to include: '<%= filename %>' does not exist! :package_multivm: |- diff --git a/test/vagrant/actions/vm/network_test.rb b/test/vagrant/actions/vm/network_test.rb index 39b47576d..6856247ee 100644 --- a/test/vagrant/actions/vm/network_test.rb +++ b/test/vagrant/actions/vm/network_test.rb @@ -4,6 +4,36 @@ class NetworkTest < Test::Unit::TestCase setup do @runner, @vm, @action = mock_action(Vagrant::Actions::VM::Network) @runner.stubs(:system).returns(linux_system(@vm)) + + @interfaces = [] + VirtualBox::Global.global.host.stubs(:network_interfaces).returns(@interfaces) + end + + def mock_interface(options=nil) + options = { + :interface_type => :host_only, + :name => "foo" + }.merge(options || {}) + + interface = mock("interface") + options.each do |k,v| + interface.stubs(k).returns(v) + end + + @interfaces << interface + interface + end + + context "preparing" do + should "verify no bridge collisions for each network enabled" do + @runner.env.config.vm.network("foo") + @action.expects(:verify_no_bridge_collision).once.with() do |options| + assert_equal "foo", options[:ip] + true + end + + @action.prepare + end end context "before destroy" do @@ -112,29 +142,33 @@ class NetworkTest < Test::Unit::TestCase end end - context "network name" do + context "verify no bridge collision" do setup do - @interfaces = [] - VirtualBox::Global.global.host.stubs(:network_interfaces).returns(@interfaces) - @action.stubs(:matching_network?).returns(false) - @options = { :ip => :foo, :netmask => :bar, :name => nil } end - def mock_interface(options=nil) - options = { - :interface_type => :host_only, - :name => "foo" - }.merge(options || {}) + should "do nothing if everything is okay" do + mock_interface - interface = mock("interface") - options.each do |k,v| - interface.stubs(k).returns(v) - end + assert_nothing_raised { @action.verify_no_bridge_collision(@options) } + end - @interfaces << interface - interface + should "raise an exception if a collision is found" do + mock_interface(:interface_type => :bridged) + @action.stubs(:matching_network?).returns(true) + + assert_raises(Vagrant::Actions::ActionException) { + @action.verify_no_bridge_collision(@options) + } + end + end + + context "network name" do + setup do + @action.stubs(:matching_network?).returns(false) + + @options = { :ip => :foo, :netmask => :bar, :name => nil } end should "return the network which matches" do diff --git a/vagrant.gemspec b/vagrant.gemspec index c3976e188..a72d79b40 100644 --- a/vagrant.gemspec +++ b/vagrant.gemspec @@ -9,7 +9,7 @@ Gem::Specification.new do |s| s.required_rubygems_version = Gem::Requirement.new("> 1.3.1") if s.respond_to? :required_rubygems_version= s.authors = ["Mitchell Hashimoto", "John Bender"] - s.date = %q{2010-07-01} + s.date = %q{2010-07-02} s.default_executable = %q{vagrant} s.description = %q{Vagrant is a tool for building and distributing virtualized development environments.} s.email = ["mitchell.hashimoto@gmail.com", "john.m.bender@gmail.com"]