Skip to content

Commit

Permalink
Added Functionality to support Non-Standard SSH Port in the Adapters. (
Browse files Browse the repository at this point in the history
…#797)

* Added Support for override of SSH Port.  ENV key is 'OOD_SSH_PORT'

* Added Support for override of SSH Port in Tests.  ENV key is 'OOD_SSH_PORT'

* Changed 22 to "22" as the method expected a string not an int

* Fixed test to remove OOD_SSH_PORT

* Added Unit Tests

* Moved OOD_SSH_PORT check to helper
  • Loading branch information
gerald-byrket authored Feb 17, 2023
1 parent 382a58f commit ba7ba70
Show file tree
Hide file tree
Showing 10 changed files with 55 additions and 19 deletions.
11 changes: 10 additions & 1 deletion lib/ood_core/job/adapters/helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,20 @@ def self.ssh_wrap(submit_host, cmd, cmd_args, strict_host_checking = true, env =
return cmd, cmd_args if submit_host.to_s.empty?

check_host = strict_host_checking ? "yes" : "no"
args = ['-o', 'BatchMode=yes', '-o', 'UserKnownHostsFile=/dev/null', '-o', "StrictHostKeyChecking=#{check_host}", "#{submit_host}"]

# Have to OodCore::Job::Adapters::Helper.ssh_port instead of self.ssh_port due to test failure
args = ['-p', OodCore::Job::Adapters::Helper.ssh_port, '-o', 'BatchMode=yes', '-o', 'UserKnownHostsFile=/dev/null', '-o', "StrictHostKeyChecking=#{check_host}", "#{submit_host}"]
env.each{|key, value| args.push("export #{key}=#{value};")}

return 'ssh', args + [cmd] + cmd_args
end

# Allows for Non-Standard Port usage in ssh commands
# To set ENV["OOD_SSH_PORT"], add assignment in /etc/ood/config/nginx_stage.yml
def self.ssh_port
return ENV["OOD_SSH_PORT"].nil? ? "22" : "#{ENV['OOD_SSH_PORT'].to_i.to_s}"
end

end
end
end
Expand Down
5 changes: 4 additions & 1 deletion lib/ood_core/job/adapters/linux_host/launcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ def initialize(
# @param script [OodCore::Job::Script] The script object defining the work
def start_remote_session(script)
cmd = ssh_cmd(submit_host(script), ['/usr/bin/env', 'bash'])

session_name = unique_session_name
output = call(*cmd, stdin: wrapped_script(script, session_name))
hostname = parse_hostname(output)
Expand Down Expand Up @@ -122,15 +121,18 @@ def call(cmd, *args, env: {}, stdin: "")
# @param destination_host [#to_s] the destination host you wish to ssh into
# @param cmd [Array<#to_s>] the command to be executed on the destination host
def ssh_cmd(destination_host, cmd)

if strict_host_checking
[
'ssh', '-t',
'-p', OodCore::Job::Adapters::Helper.ssh_port,
'-o', 'BatchMode=yes',
"#{username}@#{destination_host}"
].concat(cmd)
else
[
'ssh', '-t',
'-p', OodCore::Job::Adapters::Helper.ssh_port,
'-o', 'BatchMode=yes',
'-o', 'UserKnownHostsFile=/dev/null',
'-o', 'StrictHostKeyChecking=no',
Expand Down Expand Up @@ -291,4 +293,5 @@ def parse_hostname(output)
line.match(/^(([\w+]|[a-zA-Z0-9][\w*-]*\.))*$/)
end.compact.last.to_s
end

end
2 changes: 2 additions & 0 deletions lib/ood_core/job/adapters/systemd/launcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -104,12 +104,14 @@ def ssh_cmd(destination_host, cmd)
if strict_host_checking
[
'ssh', '-t',
'-p', OodCore::Job::Adapters::Helper.ssh_port,
'-o', 'BatchMode=yes',
"#{username}@#{destination_host}"
].concat(cmd)
else
[
'ssh', '-t',
'-p', OodCore::Job::Adapters::Helper.ssh_port,
'-o', 'BatchMode=yes',
'-o', 'UserKnownHostsFile=/dev/null',
'-o', 'StrictHostKeyChecking=no',
Expand Down
8 changes: 4 additions & 4 deletions spec/job/adapters/helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
let(:submit_host) { "owens.osc.edu" }

it "returns the ssh wrapped command" do
expect(helper.ssh_wrap(submit_host, cmd, cmd_args)).to eq(["ssh", ["-o", "BatchMode=yes", "-o", "UserKnownHostsFile=/dev/null", "-o", "StrictHostKeyChecking=yes", "owens.osc.edu", "sbatch", "-J", "Job Name"]])
expect(helper.ssh_wrap(submit_host, cmd, cmd_args)).to eq(["ssh", ["-p", "22", "-o", "BatchMode=yes", "-o", "UserKnownHostsFile=/dev/null", "-o", "StrictHostKeyChecking=yes", "owens.osc.edu", "sbatch", "-J", "Job Name"]])
end
end

Expand All @@ -60,7 +60,7 @@
let(:strict_host_checking) { true }

it "defaults host checking to yes" do
expect(helper.ssh_wrap(submit_host, cmd, cmd_args, strict_host_checking)).to eq(["ssh", ["-o", "BatchMode=yes", "-o", "UserKnownHostsFile=/dev/null", "-o", "StrictHostKeyChecking=yes", "owens.osc.edu", "sbatch", "-J", "Job Name"]])
expect(helper.ssh_wrap(submit_host, cmd, cmd_args, strict_host_checking)).to eq(["ssh", ["-p", "22", "-o", "BatchMode=yes", "-o", "UserKnownHostsFile=/dev/null", "-o", "StrictHostKeyChecking=yes", "owens.osc.edu", "sbatch", "-J", "Job Name"]])
end
end

Expand All @@ -69,8 +69,8 @@
let(:strict_host_checking) { false }

it "defaults host checking to no" do
expect(helper.ssh_wrap(submit_host, cmd, cmd_args, strict_host_checking)).to eq(["ssh", ["-o", "BatchMode=yes", "-o", "UserKnownHostsFile=/dev/null", "-o", "StrictHostKeyChecking=no", "owens.osc.edu", "sbatch", "-J", "Job Name"]])
expect(helper.ssh_wrap(submit_host, cmd, cmd_args, strict_host_checking)).to eq(["ssh", ["-p", "22", "-o", "BatchMode=yes", "-o", "UserKnownHostsFile=/dev/null", "-o", "StrictHostKeyChecking=no", "owens.osc.edu", "sbatch", "-J", "Job Name"]])
end
end
end
end
end
28 changes: 25 additions & 3 deletions spec/job/adapters/linux_host/launcher_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ def content
allow(Open3).to receive(:capture3).and_return([alt_submit_host, '', exit_success])
# RSpec doesn't seem to have a good way to test a non-first argument in a variadic list
allow(subject).to receive(:call)
.with("ssh", "-t", "-o", "BatchMode=yes", "#{username}@#{alt_submit_host}", any_args)
.with("ssh", "-t", "-p", "22", "-o", "BatchMode=yes", "#{username}@#{alt_submit_host}", any_args)
.and_return('')

subject.start_remote_session(build_script(native: {'submit_host_override' => alt_submit_host}))
Expand Down Expand Up @@ -264,7 +264,7 @@ def content
let(:ssh_cmd) { subject.send(:ssh_cmd, 'remote_host', ['/bin/bash']) }

it "uses the correct SSH options" do
expect(ssh_cmd).to eq(['ssh', '-t', '-o', 'BatchMode=yes', "#{username}@remote_host", '/bin/bash'])
expect(ssh_cmd).to eq(['ssh', '-t', '-p', '22', '-o', 'BatchMode=yes', "#{username}@remote_host", '/bin/bash'])
end
end

Expand All @@ -276,6 +276,7 @@ def content
it "uses the correct SSH options" do
expect(ssh_cmd).to eq([
'ssh', '-t',
'-p', '22',
'-o', 'BatchMode=yes',
'-o', 'UserKnownHostsFile=/dev/null',
'-o', 'StrictHostKeyChecking=no',
Expand Down Expand Up @@ -600,4 +601,25 @@ def content
end
end
end
end

# test ssh_cmd when OOD_SSH_PORT is set
context "when OOD_SSH_PORT is set" do
let(:username) { Etc.getlogin }
let(:ssh_cmd) { subject.send(:ssh_cmd, 'remote_host', ['/bin/bash']) }
it "uses the correct SSH options" do
with_modified_env({OOD_SSH_PORT: "1022"}) do
expect(ssh_cmd).to eq(['ssh', '-t', '-p', '1022', '-o', 'BatchMode=yes', "#{username}@remote_host", '/bin/bash'])
end
end
end

# test ssh_cmd when OOD_SSH_PORT is not set
context "when OOD_SSH_PORT is not set" do
let(:username) { Etc.getlogin }
let(:ssh_cmd) { subject.send(:ssh_cmd, 'remote_host', ['/bin/bash']) }
it "uses the correct SSH options" do
expect(ssh_cmd).to eq(['ssh', '-t', '-p', '22', '-o', 'BatchMode=yes', "#{username}@remote_host", '/bin/bash'])
end
end

end
4 changes: 2 additions & 2 deletions spec/job/adapters/lsf/batch_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@
allow(Open3).to receive(:capture3).and_return(["job.123", "", double("success?" => true)])

batch.submit_string(script.content)
expect(Open3).to have_received(:capture3).with(anything, 'ssh', '-o', 'BatchMode=yes', '-o', 'UserKnownHostsFile=/dev/null', '-o', 'StrictHostKeyChecking=yes', 'owens.osc.edu', 'bsub', any_args)
expect(Open3).to have_received(:capture3).with(anything, 'ssh', '-p', '22', '-o', 'BatchMode=yes', '-o', 'UserKnownHostsFile=/dev/null', '-o', 'StrictHostKeyChecking=yes', 'owens.osc.edu', 'bsub', any_args)
end
end

Expand All @@ -287,7 +287,7 @@
allow(Open3).to receive(:capture3).and_return(["job.123", "", double("success?" => true)])

batch.submit_string(script.content)
expect(Open3).to have_received(:capture3).with(anything, 'ssh', '-o', 'BatchMode=yes', '-o', 'UserKnownHostsFile=/dev/null', '-o', 'StrictHostKeyChecking=no', 'owens.osc.edu', 'bsub', any_args)
expect(Open3).to have_received(:capture3).with(anything, 'ssh', '-p', '22', '-o', 'BatchMode=yes', '-o', 'UserKnownHostsFile=/dev/null', '-o', 'StrictHostKeyChecking=no', 'owens.osc.edu', 'bsub', any_args)
end
end

Expand Down
4 changes: 2 additions & 2 deletions spec/job/adapters/pbspro_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -867,7 +867,7 @@ def build_script(opts = {})
allow(Open3).to receive(:capture3).and_return(["job.123", "", double("success?" => true)])

OodCore::Job::Adapters::PBSPro.new(pbspro: batch, qstat_factor: nil).submit script
expect(Open3).to have_received(:capture3).with(anything,'ssh', '-o', 'BatchMode=yes', '-o', 'UserKnownHostsFile=/dev/null', '-o', 'StrictHostKeyChecking=yes', 'owens.osc.edu', 'qsub', '-j', 'oe', any_args)
expect(Open3).to have_received(:capture3).with(anything,'ssh', '-p', '22', '-o', 'BatchMode=yes', '-o', 'UserKnownHostsFile=/dev/null', '-o', 'StrictHostKeyChecking=yes', 'owens.osc.edu', 'qsub', '-j', 'oe', any_args)
end
end

Expand All @@ -877,7 +877,7 @@ def build_script(opts = {})
allow(Open3).to receive(:capture3).and_return(["job.123", "", double("success?" => true)])

OodCore::Job::Adapters::PBSPro.new(pbspro: batch, qstat_factor: nil).submit script
expect(Open3).to have_received(:capture3).with(anything,'ssh', '-o', 'BatchMode=yes', '-o', 'UserKnownHostsFile=/dev/null', '-o', 'StrictHostKeyChecking=no', 'owens.osc.edu', 'qsub', '-j', 'oe', any_args)
expect(Open3).to have_received(:capture3).with(anything,'ssh', '-p', '22', '-o', 'BatchMode=yes', '-o', 'UserKnownHostsFile=/dev/null', '-o', 'StrictHostKeyChecking=no', 'owens.osc.edu', 'qsub', '-j', 'oe', any_args)
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions spec/job/adapters/sge/batch_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ def load_resource_file(file_name)
allow(Open3).to receive(:capture3).and_return(["Your job 123", "", double("success?" => true)])

OodCore::Job::Adapters::Sge.new(batch: batch).submit script
expect(Open3).to have_received(:capture3).with(anything, 'ssh', '-o', 'BatchMode=yes', '-o', 'UserKnownHostsFile=/dev/null', '-o', 'StrictHostKeyChecking=yes', 'owens.osc.edu', 'qsub', '-cwd', any_args)
expect(Open3).to have_received(:capture3).with(anything, 'ssh', '-p', '22', '-o', 'BatchMode=yes', '-o', 'UserKnownHostsFile=/dev/null', '-o', 'StrictHostKeyChecking=yes', 'owens.osc.edu', 'qsub', '-cwd', any_args)
end
end

Expand All @@ -370,7 +370,7 @@ def load_resource_file(file_name)
allow(Open3).to receive(:capture3).and_return(["Your job 123", "", double("success?" => true)])

OodCore::Job::Adapters::Sge.new(batch: batch).submit script
expect(Open3).to have_received(:capture3).with(anything, 'ssh', '-o', 'BatchMode=yes', '-o', 'UserKnownHostsFile=/dev/null', '-o', 'StrictHostKeyChecking=no', 'owens.osc.edu', 'qsub', '-cwd', any_args)
expect(Open3).to have_received(:capture3).with(anything, 'ssh', '-p', '22', '-o', 'BatchMode=yes', '-o', 'UserKnownHostsFile=/dev/null', '-o', 'StrictHostKeyChecking=no', 'owens.osc.edu', 'qsub', '-cwd', any_args)
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions spec/job/adapters/slurm_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1177,7 +1177,7 @@ def job_info(opts = {})
allow(Open3).to receive(:capture3).and_return(["job.123", "", double("success?" => true)])

OodCore::Job::Adapters::Slurm.new(slurm: batch).submit script
expect(Open3).to have_received(:capture3).with(anything, 'ssh', '-o', 'BatchMode=yes', '-o', 'UserKnownHostsFile=/dev/null', '-o', 'StrictHostKeyChecking=yes', 'owens.osc.edu', 'sbatch', '--export', 'NONE', '--parsable', '-M', 'owens.osc.edu', any_args)
expect(Open3).to have_received(:capture3).with(anything, 'ssh', '-p', '22', '-o', 'BatchMode=yes', '-o', 'UserKnownHostsFile=/dev/null', '-o', 'StrictHostKeyChecking=yes', 'owens.osc.edu', 'sbatch', '--export', 'NONE', '--parsable', '-M', 'owens.osc.edu', any_args)
end
end

Expand All @@ -1187,7 +1187,7 @@ def job_info(opts = {})
allow(Open3).to receive(:capture3).and_return(["job.123", "", double("success?" => true)])

OodCore::Job::Adapters::Slurm.new(slurm: batch).submit script
expect(Open3).to have_received(:capture3).with(anything, 'ssh', '-o', 'BatchMode=yes', '-o', 'UserKnownHostsFile=/dev/null', '-o', 'StrictHostKeyChecking=no', 'owens.osc.edu', 'sbatch', '--export', 'NONE', '--parsable', '-M', 'owens.osc.edu', any_args)
expect(Open3).to have_received(:capture3).with(anything, 'ssh', '-p', '22', '-o', 'BatchMode=yes', '-o', 'UserKnownHostsFile=/dev/null', '-o', 'StrictHostKeyChecking=no', 'owens.osc.edu', 'sbatch', '--export', 'NONE', '--parsable', '-M', 'owens.osc.edu', any_args)
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions spec/job/adapters/torque/batch_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@
allow(Open3).to receive(:capture3).and_return(["job.123", "", double("success?" => true)])

batch.submit script.content
expect(Open3).to have_received(:capture3).with(anything,'ssh', '-o', 'BatchMode=yes', '-o', 'UserKnownHostsFile=/dev/null', '-o', 'StrictHostKeyChecking=yes', 'owens.osc.edu', /['export PBS_DEFAULT','export LD_LIBRARY_PATH','qsub']/, any_args)
expect(Open3).to have_received(:capture3).with(anything,'ssh', '-p', '22', '-o', 'BatchMode=yes', '-o', 'UserKnownHostsFile=/dev/null', '-o', 'StrictHostKeyChecking=yes', 'owens.osc.edu', /['export PBS_DEFAULT','export LD_LIBRARY_PATH','qsub']/, any_args)
end
end

Expand All @@ -203,7 +203,7 @@
allow(Open3).to receive(:capture3).and_return(["job.123", "", double("success?" => true)])

batch.submit script.content
expect(Open3).to have_received(:capture3).with(anything,'ssh', '-o', 'BatchMode=yes', '-o', 'UserKnownHostsFile=/dev/null', '-o', 'StrictHostKeyChecking=no', 'owens.osc.edu', /['export PBS_DEFAULT','export LD_LIBRARY_PATH','qsub']/, any_args)
expect(Open3).to have_received(:capture3).with(anything,'ssh', '-p', '22', '-o', 'BatchMode=yes', '-o', 'UserKnownHostsFile=/dev/null', '-o', 'StrictHostKeyChecking=no', 'owens.osc.edu', /['export PBS_DEFAULT','export LD_LIBRARY_PATH','qsub']/, any_args)
end
end
end
Expand Down

0 comments on commit ba7ba70

Please sign in to comment.