Merge pull request openshift#466 from caruccio/master

Merged by openshift-bot

lib/rhc/commands/snapshot.rb

@@ -18,17 +18,19 @@ class Snapshot < Base
     default_action :help
 
     summary "Save a snapshot of your app to disk"
-    syntax "<application> [--filepath FILE]"
+    syntax "<application> [--filepath FILE] [--ssh path_to_ssh_executable]"
     option ["-n", "--namespace NAME"], "Namespace of the application you are saving a snapshot", :context => :namespace_context, :required => true
     option ["-f", "--filepath FILE"], "Local path to save tarball (default: ./$APPNAME.tar.gz)"
+    option ["--ssh PATH"], "Full path to your SSH executable with additional options"
     argument :app, "Application you are saving a snapshot", ["-a", "--app NAME"]
     alias_action :"app snapshot save", :root_command => true, :deprecated => true
     def save(app)
+      ssh = check_ssh_executable! options.ssh
       rest_app = rest_client.find_application(options.namespace, app)
       ssh_uri = URI.parse(rest_app.ssh_url)
       filename = options.filepath ? options.filepath : "#{app}.tar.gz"
 
-      ssh_cmd = "ssh #{ssh_uri.user}@#{ssh_uri.host} 'snapshot' > #{filename}"
+      ssh_cmd = "#{ssh} #{ssh_uri.user}@#{ssh_uri.host} 'snapshot' > #{filename}"
       debug ssh_cmd
 
       say "Pulling down a snapshot to #{filename}..."
@@ -63,13 +65,14 @@ def save(app)
     end
 
     summary "Restores a previously saved snapshot"
-    syntax "<application> [--filepath FILE]"
+    syntax "<application> [--filepath FILE] [--ssh path_to_ssh_executable]"
     option ["-n", "--namespace NAME"], "Namespace of the application you are restoring a snapshot", :context => :namespace_context, :required => true
     option ["-f", "--filepath FILE"], "Local path to restore tarball"
+    option ["--ssh PATH"], "Full path to your SSH executable with additional options"
     argument :app, "Application of which you are restoring a snapshot", ["-a", "--app NAME"]
     alias_action :"app snapshot restore", :root_command => true, :deprecated => true
     def restore(app)
-
+      ssh = check_ssh_executable! options.ssh
       filename = options.filepath ? options.filepath : "#{app}.tar.gz"
 
       if File.exists? filename
@@ -78,7 +81,7 @@ def restore(app)
         rest_app = rest_client.find_application(options.namespace, app)
         ssh_uri = URI.parse(rest_app.ssh_url)
 
-        ssh_cmd = "cat '#{filename}' | ssh #{ssh_uri.user}@#{ssh_uri.host} 'restore#{include_git ? ' INCLUDE_GIT' : ''}'"
+        ssh_cmd = "cat '#{filename}' | #{ssh} #{ssh_uri.user}@#{ssh_uri.host} 'restore#{include_git ? ' INCLUDE_GIT' : ''}'"
 
         say "Restoring from snapshot #{filename}..."
         debug ssh_cmd
@@ -125,5 +128,8 @@ def restore(app)
       0
     end
 
+    protected
+      include RHC::SSHHelpers
+
   end
 end

lib/rhc/commands/ssh.rb

@@ -29,9 +29,8 @@ def run(app_name, command)
       raise ArgumentError, "No application specified" unless app_name.present?
       raise ArgumentError, "--gears requires a command" if options.gears && command.blank?
       raise ArgumentError, "--limit must be an integer greater than zero" if options.limit && options.limit < 1
-      raise OptionParser::InvalidOption, "No system SSH available. Please use the --ssh option to specify the path to your SSH executable, or install SSH." unless options.ssh or has_ssh?
-      raise OptionParser::InvalidOption, "SSH executable '#{options.ssh}' does not exist." if options.ssh and not File.exist?(options.ssh)
-      raise OptionParser::InvalidOption, "SSH executable '#{options.ssh}' is not executable." if options.ssh and not File.executable?(options.ssh)
+
+      ssh = check_ssh_executable! options.ssh
 
       if options.gears
         groups = rest_client.find_application_gear_groups(options.namespace, app_name)
@@ -41,10 +40,9 @@ def run(app_name, command)
         rest_app = rest_client.find_application(options.namespace, app_name)
         $stderr.puts "Connecting to #{rest_app.ssh_string.to_s} ..." unless command.present?
 
-        ssh = options.ssh || 'ssh'
         debug "Using user specified SSH: #{options.ssh}" if options.ssh
 
-        command_line = [ssh, rest_app.ssh_string.to_s, command].compact.flatten
+        command_line = [ ssh.split, rest_app.ssh_string.to_s, command].flatten.compact
         debug "Invoking Kernel.exec with #{command_line.inspect}"
         Kernel.send(:exec, *command_line)
       end

lib/rhc/exceptions.rb

@@ -181,4 +181,10 @@ def initialize(uri)
       super "Invalid URI specified: #{uri}"
     end
   end
+
+  class InvalidSSHExecutableException < Exception
+    def initialize(message="Invalid or missing SSH executable")
+      super message
+    end
+  end
 end

lib/rhc/ssh_helpers.rb

@@ -290,6 +290,20 @@ def has_ssh?
       end
     end
 
+    # return supplied ssh executable, if valid (executable, searches $PATH).
+    # if none was supplied, return installed ssh, if any.
+    def check_ssh_executable!(path)
+      if not path
+        raise RHC::InvalidSSHExecutableException.new("No system SSH available. Please use the --ssh option to specify the path to your SSH executable, or install SSH.") unless has_ssh?
+        'ssh'
+      else
+        bin_path = path.split(' ').first
+        raise RHC::InvalidSSHExecutableException.new("SSH executable '#{bin_path}' does not exist.") unless File.exist?(bin_path) or exe?(bin_path)
+        raise RHC::InvalidSSHExecutableException.new("SSH executable '#{bin_path}' is not executable.") unless File.executable?(bin_path) or exe?(bin_path)
+        path
+      end
+    end
+
     private
 
     def ssh_add

spec/rhc/commands/snapshot_spec.rb

@@ -13,13 +13,13 @@
     user_config
     @app = rest_client.add_domain("mockdomain").add_application APP_NAME, 'mock-1.0'
     @ssh_uri = URI.parse @app.ssh_url
-    filename = APP_NAME + '.tar.gz'
-    FileUtils.cp(File.expand_path('../../assets/targz_sample.tar.gz', __FILE__), filename)
+    @targz_filename = APP_NAME + '.tar.gz'
+    FileUtils.cp(File.expand_path('../../assets/targz_sample.tar.gz', __FILE__), @targz_filename)
+    File.chmod 0644, @targz_filename unless File.executable? @targz_filename
   end
 
   after do
-    filename = APP_NAME + '.tar.gz'
-    File.delete filename if File.exist? filename
+    File.delete @targz_filename if File.exist? @targz_filename
   end
 
   describe 'snapshot without an action' do
@@ -41,6 +41,7 @@
     context 'when failing to save a snapshot' do
       before(:each) do
         `(exit 1)`
+        subject.class.any_instance.should_receive(:has_ssh?).and_return(true)
         Kernel.should_receive(:`).with("ssh #{@ssh_uri.user}@#{@ssh_uri.host} 'snapshot' > #{@app.name}.tar.gz")
       end
       it { expect { run }.to exit_with_code(130) }
@@ -72,6 +73,16 @@
 
   end
 
+  describe 'snapshot save with invalid ssh executable' do
+    let(:arguments) {['snapshot', 'save', '--trace', '--noprompt', '-l', '[email protected]', '-p', 'password', '--app', 'mockapp', '--ssh', 'path_to_ssh']}
+    it('should raise') { expect{ run }.to raise_error(RHC::InvalidSSHExecutableException, /SSH executable 'path_to_ssh' does not exist./) }
+  end
+
+  describe 'snapshot save when ssh is not executable' do
+    let(:arguments) {['snapshot', 'save', '--trace', '--noprompt', '-l', '[email protected]', '-p', 'password', '--app', 'mockapp', '--ssh', @targz_filename]}
+    it('should raise') { expect{ run }.to raise_error(RHC::InvalidSSHExecutableException, /SSH executable '#{@targz_filename}' is not executable./) }
+  end
+
   describe 'snapshot restore' do
     let(:arguments) {['snapshot', 'restore', '--noprompt', '-l', '[email protected]', '-p', 'password', '--app', 'mockapp']}
 
@@ -89,6 +100,7 @@
       before(:each) do
         File.stub(:exists?).and_return(true)
         RHC::TarGz.stub(:contains).and_return(true)
+        subject.class.any_instance.should_receive(:has_ssh?).and_return(true)
         Kernel.should_receive(:`).with("cat '#{@app.name}.tar.gz' | ssh #{@ssh_uri.user}@#{@ssh_uri.host} 'restore INCLUDE_GIT'")
         $?.stub(:exitstatus) { 1 }
       end
@@ -142,5 +154,14 @@
     end
   end
 
+  describe 'snapshot restore with invalid ssh executable' do
+    let(:arguments) {['snapshot', 'restore', '--trace', '--noprompt', '-l', '[email protected]', '-p', 'password', '--app', 'mockapp', '--ssh', 'path_to_ssh']}
+    it('should raise') { expect{ run }.to raise_error(RHC::InvalidSSHExecutableException, /SSH executable 'path_to_ssh' does not exist./) }
+  end
+
+  describe 'snapshot save when ssh is not executable' do
+    let(:arguments) {['snapshot', 'restore', '--trace', '--noprompt', '-l', '[email protected]', '-p', 'password', '--app', 'mockapp', '--ssh', @targz_filename]}
+    it('should raise') { expect{ run }.to raise_error(RHC::InvalidSSHExecutableException, /SSH executable '#{@targz_filename}' is not executable./) }
+  end
 end
 

spec/rhc/commands/ssh_spec.rb

@@ -94,7 +94,7 @@
         @domain.add_application("app1", "mock_type")
         RHC::Commands::Ssh.any_instance.should_receive(:has_ssh?).and_return(false)
       end
-      it { run_output.should match("Please use the --ssh option to specify the path to your SSH executable, or install SSH.") }
+      it { run_output.should match("No system SSH available. Please use the --ssh option to specify the path to your SSH executable, or install SSH.") }
       it { expect { run }.to exit_with_code(1) }
     end
   end
@@ -119,7 +119,7 @@
         @domain.add_application("app1", "mock_type")
         RHC::Commands::Ssh.any_instance.should_not_receive(:has_ssh?)
         File.should_receive(:exist?).with("path_to_ssh").once.and_return(true)
-        File.should_receive(:executable?).with("path_to_ssh").once.and_return(false)
+        File.should_receive(:executable?).with(/.*path_to_ssh/).at_least(1).and_return(false)
       end
       it { run_output.should match("SSH executable 'path_to_ssh' is not executable.") }
       it { expect { run }.to exit_with_code(1) }