From ad41c445a4fbd62737d8b4f0285d00b32dcd31f3 Mon Sep 17 00:00:00 2001 From: Darrell Hamilton Date: Sun, 14 Jul 2013 22:42:49 -0700 Subject: [PATCH 1/5] Check for redir before forwarding ports Make a system call out to `which` to see if redir exists on the PATH before trying to forward ports. Raises a VagrantError if it does not. --- lib/vagrant-lxc/action/forward_ports.rb | 37 +++++++++++++++---------- lib/vagrant-lxc/errors.rb | 3 ++ locales/en.yml | 3 ++ 3 files changed, 29 insertions(+), 14 deletions(-) diff --git a/lib/vagrant-lxc/action/forward_ports.rb b/lib/vagrant-lxc/action/forward_ports.rb index df150c3..4e6e466 100644 --- a/lib/vagrant-lxc/action/forward_ports.rb +++ b/lib/vagrant-lxc/action/forward_ports.rb @@ -26,28 +26,33 @@ module Vagrant if @env[:forwarded_ports].any? env[:ui].info I18n.t("vagrant.actions.vm.forward_ports.forwarding") - forward_ports + + if redir_installed? + forward_ports + else + raise Errors::RedirNotInstalled + end end end def forward_ports @container_ip = @env[:machine].provider.driver.assigned_ip - @env[:forwarded_ports].each do |fp| - message_attributes = { - # TODO: Add support for multiple adapters - :adapter => 'eth0', - :guest_port => fp[:guest], - :host_port => fp[:host] - } + @env[:forwarded_ports].each do |fp| + message_attributes = { + # TODO: Add support for multiple adapters + :adapter => 'eth0', + :guest_port => fp[:guest], + :host_port => fp[:host] + } - # TODO: Remove adapter from logging - @env[:ui].info(I18n.t("vagrant.actions.vm.forward_ports.forwarding_entry", - message_attributes)) + # TODO: Remove adapter from logging + @env[:ui].info(I18n.t("vagrant.actions.vm.forward_ports.forwarding_entry", + message_attributes)) - redir_pid = redirect_port(fp[:host], fp[:guest]) - store_redir_pid(fp[:host], redir_pid) - end + redir_pid = redirect_port(fp[:host], fp[:guest]) + store_redir_pid(fp[:host], redir_pid) + end end private @@ -79,6 +84,10 @@ module Vagrant pid_file.write(redir_pid) end end + + def redir_installed? + system "sudo which redir" + end end end end diff --git a/lib/vagrant-lxc/errors.rb b/lib/vagrant-lxc/errors.rb index 169e567..3ec0531 100644 --- a/lib/vagrant-lxc/errors.rb +++ b/lib/vagrant-lxc/errors.rb @@ -17,6 +17,9 @@ module Vagrant class IncompatibleBox < Vagrant::Errors::VagrantError error_key(:lxc_incompatible_box) end + class RedirNotInstalled < Vagrant::Errors::VagrantError + error_key(:lxc_redir_not_installed) + end end end end diff --git a/locales/en.yml b/locales/en.yml index 50edd3d..b7aa5bf 100644 --- a/locales/en.yml +++ b/locales/en.yml @@ -40,3 +40,6 @@ en: lxc_template_file_missing: |- The template file used for creating the container was not found for %{name} box. + + lxc_redir_not_installed: |- + `redir` is not installed or is not accessible on the PATH. From a2a9cb99d183135627064395de100c785987ff2f Mon Sep 17 00:00:00 2001 From: Darrell Hamilton Date: Sun, 14 Jul 2013 22:50:14 -0700 Subject: [PATCH 2/5] repair whitespace --- lib/vagrant-lxc/action/forward_ports.rb | 26 ++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/lib/vagrant-lxc/action/forward_ports.rb b/lib/vagrant-lxc/action/forward_ports.rb index 4e6e466..61c3c0a 100644 --- a/lib/vagrant-lxc/action/forward_ports.rb +++ b/lib/vagrant-lxc/action/forward_ports.rb @@ -38,21 +38,21 @@ module Vagrant def forward_ports @container_ip = @env[:machine].provider.driver.assigned_ip - @env[:forwarded_ports].each do |fp| - message_attributes = { - # TODO: Add support for multiple adapters - :adapter => 'eth0', - :guest_port => fp[:guest], - :host_port => fp[:host] - } + @env[:forwarded_ports].each do |fp| + message_attributes = { + # TODO: Add support for multiple adapters + :adapter => 'eth0', + :guest_port => fp[:guest], + :host_port => fp[:host] + } - # TODO: Remove adapter from logging - @env[:ui].info(I18n.t("vagrant.actions.vm.forward_ports.forwarding_entry", - message_attributes)) + # TODO: Remove adapter from logging + @env[:ui].info(I18n.t("vagrant.actions.vm.forward_ports.forwarding_entry", + message_attributes)) - redir_pid = redirect_port(fp[:host], fp[:guest]) - store_redir_pid(fp[:host], redir_pid) - end + redir_pid = redirect_port(fp[:host], fp[:guest]) + store_redir_pid(fp[:host], redir_pid) + end end private From 041f1853957d93b8792c82f963446bc2f8fb46f0 Mon Sep 17 00:00:00 2001 From: Darrell Hamilton Date: Mon, 15 Jul 2013 10:49:02 -0700 Subject: [PATCH 3/5] Check for redir before booting the machine --- lib/vagrant-lxc/action/forward_ports.rb | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/lib/vagrant-lxc/action/forward_ports.rb b/lib/vagrant-lxc/action/forward_ports.rb index 61c3c0a..1e408df 100644 --- a/lib/vagrant-lxc/action/forward_ports.rb +++ b/lib/vagrant-lxc/action/forward_ports.rb @@ -10,12 +10,13 @@ module Vagrant def call(env) @env = env - # Continue, we need the VM to be booted in order to grab its IP - @app.call env - # Get the ports we're forwarding env[:forwarded_ports] = compile_forwarded_ports(env[:machine].config) + if @env[:forwarded_ports].any? and not redir_installed? + raise Errors::RedirNotInstalled + end + # Warn if we're port forwarding to any privileged ports env[:forwarded_ports].each do |fp| if fp[:host] <= 1024 @@ -24,14 +25,12 @@ module Vagrant end end + # Continue, we need the VM to be booted in order to grab its IP + @app.call env + if @env[:forwarded_ports].any? env[:ui].info I18n.t("vagrant.actions.vm.forward_ports.forwarding") - - if redir_installed? - forward_ports - else - raise Errors::RedirNotInstalled - end + forward_ports end end @@ -86,7 +85,7 @@ module Vagrant end def redir_installed? - system "sudo which redir" + system "sudo which redir > /dev/null" end end end From 17211387d0fad9d351fd1567108124fa656e9040 Mon Sep 17 00:00:00 2001 From: Darrell Hamilton Date: Mon, 15 Jul 2013 11:30:40 -0700 Subject: [PATCH 4/5] stub out call to system --- spec/unit/action/forward_ports_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/unit/action/forward_ports_spec.rb b/spec/unit/action/forward_ports_spec.rb index 4c31edc..f462b94 100644 --- a/spec/unit/action/forward_ports_spec.rb +++ b/spec/unit/action/forward_ports_spec.rb @@ -23,6 +23,7 @@ describe Vagrant::LXC::Action::ForwardPorts do subject.stub(exec: true) subject.stub(spawn: pid) + subject.stub(system: true) subject.call(env) end From eb0854b6fb5452e462fbd4182469ba89d4c20752 Mon Sep 17 00:00:00 2001 From: Darrell Hamilton Date: Mon, 15 Jul 2013 21:56:40 -0700 Subject: [PATCH 5/5] Test coverage for 'check for redir' Includes a minor refactor allowing variation of the result of the system call per test --- spec/unit/action/forward_ports_spec.rb | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/spec/unit/action/forward_ports_spec.rb b/spec/unit/action/forward_ports_spec.rb index f462b94..65a0d81 100644 --- a/spec/unit/action/forward_ports_spec.rb +++ b/spec/unit/action/forward_ports_spec.rb @@ -23,20 +23,27 @@ describe Vagrant::LXC::Action::ForwardPorts do subject.stub(exec: true) subject.stub(spawn: pid) - subject.stub(system: true) - subject.call(env) end after { FileUtils.rm_rf data_dir.to_s } it 'forwards ports using redir' do + subject.stub(system: true) + subject.call(env) subject.should have_received(:spawn).with( "sudo redir --laddr=127.0.0.1 --lport=#{host_port} --cport=#{guest_port} --caddr=#{container_ip} 2>/dev/null" ) end it "stores redir pids on machine's data dir" do + subject.stub(system: true) + subject.call(env) pid_file = data_dir.join('pids', "redir_#{host_port}.pid").read pid_file.should == pid end + + it 'raises RedirNotInstalled error if `redir` is not installed' do + subject.stub(system: false) + lambda { subject.call(env) }.should raise_error(Vagrant::LXC::Errors::RedirNotInstalled) + end end