From 2821dcee7f3a4081f44b5edd24f48d3633b8e06b Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 21 Jan 2011 23:38:42 -0800 Subject: [PATCH] SSH commands which use sudo compatible with sudo < 1.7.0 --- CHANGELOG.md | 3 +++ lib/vagrant/provisioners/chef.rb | 6 ++--- lib/vagrant/provisioners/chef_server.rb | 9 ++++--- lib/vagrant/provisioners/chef_solo.rb | 6 ++--- lib/vagrant/provisioners/puppet.rb | 9 ++++--- lib/vagrant/provisioners/puppet_server.rb | 10 +++---- lib/vagrant/ssh/session.rb | 27 +++++++++++++++++-- test/vagrant/provisioners/chef_server_test.rb | 6 ++--- test/vagrant/provisioners/chef_solo_test.rb | 4 +-- test/vagrant/provisioners/chef_test.rb | 6 ++--- .../provisioners/puppet_server_test.rb | 11 ++++---- test/vagrant/provisioners/puppet_test.rb | 4 +-- 12 files changed, 63 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 755dd93bb..bf5265518 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ upstream net-ssh fix. - Fix issue causing warnings to show with `forwardx11` enabled for SSH. [GH-279] - FreeBSD support for host only networks, NFS, halting, etc. [GH-275] + - Make SSH commands which use sudo compatible with sudo < 1.7.0. [GH-278] + - Fix broken puppet server provisioner which called a nonexistent + method. ## 0.7.0 (January 19, 2011) diff --git a/lib/vagrant/provisioners/chef.rb b/lib/vagrant/provisioners/chef.rb index acd9b8422..b4809922c 100644 --- a/lib/vagrant/provisioners/chef.rb +++ b/lib/vagrant/provisioners/chef.rb @@ -12,14 +12,14 @@ module Vagrant vm.ssh.execute do |ssh| # Checks for the existence of chef binary and error if it # doesn't exist. - ssh.exec!("sudo -i which #{binary}", :error_class => ChefError, :_key => :chef_not_detected, :binary => binary) + ssh.sudo!("which #{binary}", :error_class => ChefError, :_key => :chef_not_detected, :binary => binary) end end def chown_provisioning_folder vm.ssh.execute do |ssh| - ssh.exec!("sudo mkdir -p #{config.provisioning_path}") - ssh.exec!("sudo chown #{env.config.ssh.username} #{config.provisioning_path}") + ssh.sudo!("mkdir -p #{config.provisioning_path}") + ssh.sudo!("chown #{env.config.ssh.username} #{config.provisioning_path}") end end diff --git a/lib/vagrant/provisioners/chef_server.rb b/lib/vagrant/provisioners/chef_server.rb index e605ffd74..3c60f7607 100644 --- a/lib/vagrant/provisioners/chef_server.rb +++ b/lib/vagrant/provisioners/chef_server.rb @@ -50,7 +50,7 @@ module Vagrant path = Pathname.new(config.client_key_path) vm.ssh.execute do |ssh| - ssh.exec!("sudo mkdir -p #{path.dirname}") + ssh.sudo!("mkdir -p #{path.dirname}") end end @@ -70,13 +70,14 @@ module Vagrant end def run_chef_client - command = "sudo -i 'cd #{config.provisioning_path} && chef-client -c client.rb -j dna.json'" + commands = ["cd #{config.provisioning_path}", + "chef-client -c client.rb -j dna.json"] env.ui.info I18n.t("vagrant.provisioners.chef.running_client") vm.ssh.execute do |ssh| - ssh.exec!(command) do |channel, type, data| + ssh.sudo!(commands) do |channel, type, data| if type == :exit_status - ssh.check_exit_status(data, command) + ssh.check_exit_status(data, commands) else env.ui.info("#{data}: #{type}") end diff --git a/lib/vagrant/provisioners/chef_solo.rb b/lib/vagrant/provisioners/chef_solo.rb index 9bf753eb6..59276de32 100644 --- a/lib/vagrant/provisioners/chef_solo.rb +++ b/lib/vagrant/provisioners/chef_solo.rb @@ -60,12 +60,12 @@ module Vagrant end def run_chef_solo - command = "sudo -i 'cd #{config.provisioning_path} && chef-solo -c solo.rb -j dna.json'" + commands = ["cd #{config.provisioning_path}", "chef-solo -c solo.rb -j dna.json"] env.ui.info I18n.t("vagrant.provisioners.chef.running_solo") vm.ssh.execute do |ssh| - ssh.exec!(command) do |channel, type, data| - ssh.check_exit_status(data, command) if type == :exit_status + ssh.sudo!(commands) do |channel, type, data| + ssh.check_exit_status(data, commands) if type == :exit_status env.ui.info("#{data}: #{type}") if type != :exit_status end end diff --git a/lib/vagrant/provisioners/puppet.rb b/lib/vagrant/provisioners/puppet.rb index bed551fc8..35de27762 100644 --- a/lib/vagrant/provisioners/puppet.rb +++ b/lib/vagrant/provisioners/puppet.rb @@ -107,7 +107,7 @@ module Vagrant def verify_binary(binary) vm.ssh.execute do |ssh| - ssh.exec!("sudo -i which #{binary}", :error_class => PuppetError, :_key => :puppet_not_detected, :binary => binary) + ssh.sudo!("which #{binary}", :error_class => PuppetError, :_key => :puppet_not_detected, :binary => binary) end end @@ -117,14 +117,15 @@ module Vagrant options << config.computed_manifest_file options = options.join(" ") - command = "sudo -i 'cd #{manifests_guest_path}; puppet #{options}'" + commands = ["cd #{manifests_guest_path}", + "puppet #{options}"] env.ui.info I18n.t("vagrant.provisioners.puppet.running_puppet", :manifest => config.computed_manifest_file) vm.ssh.execute do |ssh| - ssh.exec! command do |ch, type, data| + ssh.sudo! commands do |ch, type, data| if type == :exit_status - ssh.check_exit_status(data, command) + ssh.check_exit_status(data, commands) else env.ui.info(data) end diff --git a/lib/vagrant/provisioners/puppet_server.rb b/lib/vagrant/provisioners/puppet_server.rb index 91c913366..0d81f6db1 100644 --- a/lib/vagrant/provisioners/puppet_server.rb +++ b/lib/vagrant/provisioners/puppet_server.rb @@ -26,9 +26,7 @@ module Vagrant def verify_binary(binary) vm.ssh.execute do |ssh| - ssh.shell do |sh| - sh.execute("sudo -i which #{binary}", :error_class => PuppetServerError, :_key => :puppetd_not_detected, :binary => binary) - end + ssh.sudo!("which #{binary}", :error_class => PuppetServerError, :_key => :puppetd_not_detected, :binary => binary) end end @@ -41,13 +39,13 @@ module Vagrant cn = env.config.vm.box end - command = "sudo -i puppetd #{options} --server #{config.puppet_server} --certname #{cn}" + commands = "puppetd #{options} --server #{config.puppet_server} --certname #{cn}" env.ui.info I18n.t("vagrant.provisioners.puppet_server.running_puppetd") vm.ssh.execute do |ssh| - ssh.exec!(command) do |channel, type, data| - ssh.check_exit_status(data, command) if type == :exit_status + ssh.sudo!(commands) do |channel, type, data| + ssh.check_exit_status(data, commands) if type == :exit_status env.ui.info(data) if type != :exit_status end end diff --git a/lib/vagrant/ssh/session.rb b/lib/vagrant/ssh/session.rb index f4d97b67b..f6c1ebb4a 100644 --- a/lib/vagrant/ssh/session.rb +++ b/lib/vagrant/ssh/session.rb @@ -22,6 +22,29 @@ module Vagrant false end + # Executes a given command on the SSH session using `sudo` and + # blocks until the command completes. This takes the same parameters + # as {#exec!}. The only difference is that the command can be an + # array of commands, which will be placed into the same script. + # + # This is different than just calling {#exec!} with `sudo`, since + # this command is tailor-made to be compliant with older versions + # of `sudo`. + def sudo!(commands, options=nil, &block) + # First, make a temporary file to store the script + filename = exec!("mktemp /tmp/vagrant-command-#{'X' * 10}") + + # Output each command into the temporary file + [commands].flatten.each do |command| + exec!("echo #{command} >> #{filename}") + end + + # Finally, execute the file, passing in the parameters since this + # is the expected command to run. + exec!("sudo chmod +x #{filename}") + exec!("sudo -i #{filename}", options, &block) + end + # Executes a given command on the SSH session and blocks until # the command completes. This is an almost line for line copy of # the actual `exec!` implementation, except that this @@ -65,12 +88,12 @@ module Vagrant # Checks for an erroroneous exit status and raises an exception # if so. - def check_exit_status(exit_status, command, options=nil) + def check_exit_status(exit_status, commands, options=nil) if exit_status != 0 options = { :_error_class => Errors::VagrantError, :_key => :ssh_bad_exit_status, - :command => command + :command => [commands].flatten.join("\n") }.merge(options || {}) raise options[:_error_class], options diff --git a/test/vagrant/provisioners/chef_server_test.rb b/test/vagrant/provisioners/chef_server_test.rb index a72acf7c6..4b8e7b7ac 100644 --- a/test/vagrant/provisioners/chef_server_test.rb +++ b/test/vagrant/provisioners/chef_server_test.rb @@ -117,7 +117,7 @@ class ChefServerProvisionerTest < Test::Unit::TestCase should "create the folder using the dirname of the path" do ssh = mock("ssh") - ssh.expects(:exec!).with("sudo mkdir -p #{@path.dirname}").once + ssh.expects(:sudo!).with("mkdir -p #{@path.dirname}").once @vm.ssh.expects(:execute).yields(ssh) @action.create_client_key_folder end @@ -173,12 +173,12 @@ class ChefServerProvisionerTest < Test::Unit::TestCase end should "cd into the provisioning directory and run chef client" do - @ssh.expects(:exec!).with("sudo -i 'cd #{@config.provisioning_path} && chef-client -c client.rb -j dna.json'").once + @ssh.expects(:sudo!).with(["cd #{@config.provisioning_path}", "chef-client -c client.rb -j dna.json"]).once @action.run_chef_client end should "check the exit status if that is given" do - @ssh.stubs(:exec!).yields(nil, :exit_status, :foo) + @ssh.stubs(:sudo!).yields(nil, :exit_status, :foo) @ssh.expects(:check_exit_status).with(:foo, anything).once @action.run_chef_client end diff --git a/test/vagrant/provisioners/chef_solo_test.rb b/test/vagrant/provisioners/chef_solo_test.rb index ebf92f6b6..aeaab52e9 100644 --- a/test/vagrant/provisioners/chef_solo_test.rb +++ b/test/vagrant/provisioners/chef_solo_test.rb @@ -206,12 +206,12 @@ class ChefSoloProvisionerTest < Test::Unit::TestCase end should "cd into the provisioning directory and run chef solo" do - @ssh.expects(:exec!).with("sudo -i 'cd #{@config.provisioning_path} && chef-solo -c solo.rb -j dna.json'").once + @ssh.expects(:sudo!).with(["cd #{@config.provisioning_path}", "chef-solo -c solo.rb -j dna.json"]).once @action.run_chef_solo end should "check the exit status if that is given" do - @ssh.stubs(:exec!).yields(nil, :exit_status, :foo) + @ssh.stubs(:sudo!).yields(nil, :exit_status, :foo) @ssh.expects(:check_exit_status).with(:foo, anything).once @action.run_chef_solo end diff --git a/test/vagrant/provisioners/chef_test.rb b/test/vagrant/provisioners/chef_test.rb index 4946de9dc..f2bdf822b 100644 --- a/test/vagrant/provisioners/chef_test.rb +++ b/test/vagrant/provisioners/chef_test.rb @@ -73,7 +73,7 @@ class ChefProvisionerTest < Test::Unit::TestCase should "verify binary exists" do binary = "foo" - @ssh.expects(:exec!).with("sudo -i which #{binary}", anything) + @ssh.expects(:sudo!).with("which #{binary}", anything) @action.verify_binary(binary) end end @@ -82,8 +82,8 @@ class ChefProvisionerTest < Test::Unit::TestCase should "create and chown the folder to the ssh user" do ssh_seq = sequence("ssh_seq") ssh = mock("ssh") - ssh.expects(:exec!).with("sudo mkdir -p #{@config.provisioning_path}").once.in_sequence(ssh_seq) - ssh.expects(:exec!).with("sudo chown #{@env.config.ssh.username} #{@config.provisioning_path}").once.in_sequence(ssh_seq) + ssh.expects(:sudo!).with("mkdir -p #{@config.provisioning_path}").once.in_sequence(ssh_seq) + ssh.expects(:sudo!).with("chown #{@env.config.ssh.username} #{@config.provisioning_path}").once.in_sequence(ssh_seq) @vm.ssh.expects(:execute).yields(ssh) @action.chown_provisioning_folder end diff --git a/test/vagrant/provisioners/puppet_server_test.rb b/test/vagrant/provisioners/puppet_server_test.rb index 1d9da2fb2..4bbdf9245 100644 --- a/test/vagrant/provisioners/puppet_server_test.rb +++ b/test/vagrant/provisioners/puppet_server_test.rb @@ -25,13 +25,12 @@ class PuppetServerProvisionerTest < Test::Unit::TestCase setup do @ssh = mock("ssh") @shell = mock("shell") - @ssh.stubs(:shell).yields(@shell) @vm.ssh.stubs(:execute).yields(@ssh) end should "verify binary exists" do binary = "foo" - @shell.expects(:execute).with("sudo -i which #{binary}", anything) + @ssh.expects(:sudo!).with("which #{binary}", anything) @action.verify_binary(binary) end end @@ -44,24 +43,24 @@ class PuppetServerProvisionerTest < Test::Unit::TestCase end should "run the puppetd client" do - @ssh.expects(:exec!).with("sudo -i puppetd --server #{@config.puppet_server} --certname #{@cn}").once + @ssh.expects(:sudo!).with("puppetd --server #{@config.puppet_server} --certname #{@cn}").once @action.run_puppetd_client end should "run puppetd with given options when given as an array" do @config.options = ["--modulepath", "modules", "--verbose"] - @ssh.expects(:exec!).with("sudo -i puppetd --modulepath modules --verbose --server #{@config.puppet_server} --certname #{@cn}").once + @ssh.expects(:sudo!).with("puppetd --modulepath modules --verbose --server #{@config.puppet_server} --certname #{@cn}").once @action.run_puppetd_client end should "run puppetd with the options when given as a string" do @config.options = "--modulepath modules --verbose" - @ssh.expects(:exec!).with("sudo -i puppetd --modulepath modules --verbose --server #{@config.puppet_server} --certname #{@cn}").once + @ssh.expects(:sudo!).with("puppetd --modulepath modules --verbose --server #{@config.puppet_server} --certname #{@cn}").once @action.run_puppetd_client end should "check the exit status if that is given" do - @ssh.stubs(:exec!).yields(nil, :exit_status, :foo) + @ssh.stubs(:sudo!).yields(nil, :exit_status, :foo) @ssh.expects(:check_exit_status).with(:foo, anything).once @action.run_puppetd_client end diff --git a/test/vagrant/provisioners/puppet_test.rb b/test/vagrant/provisioners/puppet_test.rb index 81eab3045..0308bacf8 100644 --- a/test/vagrant/provisioners/puppet_test.rb +++ b/test/vagrant/provisioners/puppet_test.rb @@ -138,7 +138,7 @@ class PuppetProvisionerTest < Test::Unit::TestCase should "verify binary exists" do binary = "foo" - @ssh.expects(:exec!).with("sudo -i which #{binary}", anything) + @ssh.expects(:sudo!).with("which #{binary}", anything) @action.verify_binary(binary) end end @@ -151,7 +151,7 @@ class PuppetProvisionerTest < Test::Unit::TestCase end def expect_puppet_command(command) - @ssh.expects(:exec!).with("sudo -i 'cd #{@action.manifests_guest_path}; #{command}'") + @ssh.expects(:sudo!).with(["cd #{@action.manifests_guest_path}", command]) end should "cd into the pp_path directory and run puppet" do