diff --git a/README.md b/README.md index 78adace..be60045 100644 --- a/README.md +++ b/README.md @@ -167,14 +167,34 @@ scp somefile.txt user@linux.device.com: Here, `user` is the Linux user and `linux.device.com` is the Linux host to SSH into or copy the file to. -If `ssh.exe` or `scp.exe` cannot be found, or if additional arguments are -required, it is recommended to set the environment variables `CDC_SSH_COMMAND` -and `CDC_SCP_COMMAND`. The following example specifies a custom path to the SSH -and SCP binaries, a custom SSH config file, a key file and a known hosts file: +If additional arguments are required, it is recommended to provide an SSH config +file. By default, both `ssh.exe` and `scp.exe` use the file at +`%USERPROFILE%\.ssh\config` on Windows, if it exists. A possible config file +that sets a username, a port, an identity file and a known host file could look +as follows: ``` -set CDC_SSH_COMMAND="C:\path with space\to\ssh.exe" -F C:\path\to\ssh_config -i C:\path\to\id_rsa -oStrictHostKeyChecking=yes -oUserKnownHostsFile="""C:\path\to\known_hosts""" -set CDC_SCP_COMMAND="C:\path with space\to\scp.exe" -F C:\path\to\ssh_config -i C:\path\to\id_rsa -oStrictHostKeyChecking=yes -oUserKnownHostsFile="""C:\path\to\known_hosts""" +Host linux_device + HostName linux.device.com + User user + Port 12345 + IdentityFile C:\path\to\id_rsa + UserKnownHostsFile C:\path\to\known_hosts ``` +If `ssh.exe` or `scp.exe` cannot be found, you can specify the full paths via +the command line arguments `--ssh-command` and `--scp-command` for `cdc_rsync` +and `cdc_stream start` (see below), or set the environment variables +`CDC_SSH_COMMAND` and `CDC_SCP_COMMAND`, e.g. +``` +set CDC_SSH_COMMAND="C:\path with space\to\ssh.exe" +set CDC_SCP_COMMAND="C:\path with space\to\scp.exe" +``` +Note that you can also specify SSH configuration via the environment variables +instead of using a config file: +``` +set CDC_SSH_COMMAND=C:\path\to\ssh.exe -p 12345 -i C:\path\to\id_rsa -oUserKnownHostsFile=C:\path\to\known_hosts +set CDC_SCP_COMMAND=C:\path\to\scp.exe -P 12345 -i C:\path\to\id_rsa -oUserKnownHostsFile=C:\path\to\known_hosts +``` +Note the small `-p` for `ssh.exe` and the capital `-P` for `scp.exe`. #### Google Specific diff --git a/cdc_rsync/cdc_rsync_client.cc b/cdc_rsync/cdc_rsync_client.cc index c595bcb..0efacf9 100644 --- a/cdc_rsync/cdc_rsync_client.cc +++ b/cdc_rsync/cdc_rsync_client.cc @@ -99,9 +99,9 @@ CdcRsyncClient::CdcRsyncClient(const Options& options, std::string user_host, std::string destination) : options_(options), sources_(std::move(sources)), - user_host_(std::move(user_host)), destination_(std::move(destination)), - remote_util_(options.verbosity, options.quiet, &process_factory_, + remote_util_(std::move(user_host), options.verbosity, options.quiet, + &process_factory_, /*forward_output_to_log=*/false), port_manager_("cdc_rsync_ports_f77bcdfe-368c-4c45-9f01-230c5e7e2132", kForwardPortFirst, kForwardPortLast, &process_factory_, @@ -122,9 +122,6 @@ CdcRsyncClient::~CdcRsyncClient() { } absl::Status CdcRsyncClient::Run() { - // Initialize |remote_util_|. - remote_util_.SetUserHostAndPort(user_host_, options_.port); - // Start the server process. absl::Status status = StartServer(); if (HasTag(status, Tag::kDeployServer)) { diff --git a/cdc_rsync/cdc_rsync_client.h b/cdc_rsync/cdc_rsync_client.h index e9680d2..ecc73d1 100644 --- a/cdc_rsync/cdc_rsync_client.h +++ b/cdc_rsync/cdc_rsync_client.h @@ -36,7 +36,6 @@ class ZstdStream; class CdcRsyncClient { public: struct Options { - int port = RemoteUtil::kDefaultSshPort; bool delete_ = false; bool recursive = false; int verbosity = 0; @@ -118,7 +117,6 @@ class CdcRsyncClient { Options options_; std::vector sources_; - const std::string user_host_; const std::string destination_; WinProcessFactory process_factory_; RemoteUtil remote_util_; diff --git a/cdc_rsync/main.cc b/cdc_rsync/main.cc index 3dd475e..4595bba 100644 --- a/cdc_rsync/main.cc +++ b/cdc_rsync/main.cc @@ -75,9 +75,9 @@ ReturnCode TagToMessage(cdc_ft::Tag tag, case cdc_ft::Tag::kConnectionTimeout: // Server connection timed out. SSH probably stale. *msg = absl::StrFormat( - "Server connection timed out. Verify that host '%s' and port '%i' " - "are correct, or specify a larger timeout with --contimeout.", - params.user_host, params.options.port); + "Server connection timed out. Verify that the host '%s' " + "is correct, or specify a larger timeout with --contimeout.", + params.user_host); return ReturnCode::kConnectionTimeout; case cdc_ft::Tag::kCount: diff --git a/cdc_rsync/params.cc b/cdc_rsync/params.cc index aa6d91c..7a33620 100644 --- a/cdc_rsync/params.cc +++ b/cdc_rsync/params.cc @@ -51,8 +51,6 @@ Parameters: destination Remote destination directory Options: - --ip string Gamelet IP. Required. - --port number SSH port to use. Required. --contimeout sec Gamelet connection timeout in seconds (default: 10) -q, --quiet Quiet mode, only print errors -v, --verbose Increase output verbosity @@ -74,10 +72,10 @@ Options: --existing Skip creating new files on instance --copy-dest dir Use files from dir as sync base if files are missing --ssh-command Path and arguments of ssh command to use, e.g. - C:\path\to\ssh.exe -F config -i id_rsa -oStrictHostKeyChecking=yes -oUserKnownHostsFile="""known_hosts""" + "C:\path\to\ssh.exe -p 12345 -i id_rsa -oUserKnownHostsFile=known_hosts" Can also be specified by the CDC_SSH_COMMAND environment variable. --scp-command Path and arguments of scp command to use, e.g. - C:\path\to\scp.exe -F config -i id_rsa -oStrictHostKeyChecking=yes -oUserKnownHostsFile="""known_hosts""" + "C:\path\to\scp.exe -P 12345 -i id_rsa -oUserKnownHostsFile=known_hosts" Can also be specified by the CDC_SCP_COMMAND environment variable. -h --help Help for cdc_rsync )"; @@ -164,13 +162,6 @@ bool LoadFilesFrom(const std::string& files_from, OptionResult HandleParameter(const std::string& key, const char* value, Parameters* params, bool* help) { - if (key == "port") { - if (value) { - params->options.port = atoi(value); - } - return OptionResult::kConsumedKeyValue; - } - if (key == "delete") { params->options.delete_ = true; return OptionResult::kConsumedKey; @@ -302,11 +293,6 @@ bool ValidateParameters(const Parameters& params, bool help) { return false; } - if (params.options.port <= 0 || params.options.port > UINT16_MAX) { - PrintError("--port must specify a valid port"); - return false; - } - // Note: ZSTD_minCLevel() is ridiculously small (-131072), so use a // reasonable value. assert(ZSTD_minCLevel() <= Options::kMinCompressLevel); diff --git a/cdc_rsync/params_test.cc b/cdc_rsync/params_test.cc index ea3b8c0..e038df0 100644 --- a/cdc_rsync/params_test.cc +++ b/cdc_rsync/params_test.cc @@ -97,7 +97,6 @@ class ParamsTest : public ::testing::Test { TEST_F(ParamsTest, ParseSucceedsDefaults) { const char* argv[] = {"cdc_rsync.exe", kSrc, kUserHostDst, NULL}; EXPECT_TRUE(Parse(static_cast(std::size(argv)) - 1, argv, ¶meters_)); - EXPECT_EQ(RemoteUtil::kDefaultSshPort, parameters_.options.port); EXPECT_FALSE(parameters_.options.delete_); EXPECT_FALSE(parameters_.options.recursive); EXPECT_EQ(0, parameters_.options.verbosity); @@ -145,13 +144,6 @@ TEST_F(ParamsTest, ParseFailsOnCompressLevelEqualsNoValue) { ExpectError(NeedsValueError("compress-level")); } -TEST_F(ParamsTest, ParseFailsOnPortEqualsNoValue) { - const char* argv[] = {"cdc_rsync.exe", "--port=", kSrc, kUserHostDst, NULL}; - EXPECT_FALSE( - Parse(static_cast(std::size(argv)) - 1, argv, ¶meters_)); - ExpectError(NeedsValueError("port")); -} - TEST_F(ParamsTest, ParseFailsOnContimeoutEqualsNoValue) { const char* argv[] = {"cdc_rsync.exe", "--contimeout=", kSrc, kUserHostDst, NULL}; @@ -285,13 +277,18 @@ TEST_F(ParamsTest, ParseFailsOnUnknownKey) { } TEST_F(ParamsTest, ParseSucceedsWithSupportedKeyValue) { - const char* argv[] = { - "cdc_rsync.exe", "--compress-level", "11", "--contimeout", "99", "--port", - "4086", "--copy-dest=dest", kSrc, kUserHostDst, NULL}; + const char* argv[] = {"cdc_rsync.exe", + "--compress-level", + "11", + "--contimeout", + "99", + "--copy-dest=dest", + kSrc, + kUserHostDst, + NULL}; EXPECT_TRUE(Parse(static_cast(std::size(argv)) - 1, argv, ¶meters_)); EXPECT_EQ(parameters_.options.compress_level, 11); EXPECT_EQ(parameters_.options.connection_timeout_sec, 99); - EXPECT_EQ(parameters_.options.port, 4086); EXPECT_EQ(parameters_.options.copy_dest, "dest"); ExpectNoError(); } @@ -304,13 +301,6 @@ TEST_F(ParamsTest, ParseSucceedsWithSupportedKeyValueWithoutEqualityForChars) { ExpectNoError(); } -TEST_F(ParamsTest, ParseFailsOnInvalidPort) { - const char* argv[] = {"cdc_rsync.exe", "--port=0", kSrc, kUserHostDst, NULL}; - EXPECT_FALSE( - Parse(static_cast(std::size(argv)) - 1, argv, ¶meters_)); - ExpectError("--port must specify a valid port"); -} - TEST_F(ParamsTest, ParseFailsOnDeleteNeedsRecursive) { const char* argv[] = {"cdc_rsync.exe", "--delete", kSrc, kUserHostDst, NULL}; EXPECT_FALSE( diff --git a/cdc_stream/asset_stream_config.cc b/cdc_stream/asset_stream_config.cc index 1ca2ab9..af38e95 100644 --- a/cdc_stream/asset_stream_config.cc +++ b/cdc_stream/asset_stream_config.cc @@ -135,14 +135,6 @@ void AssetStreamConfig::RegisterCommandLineFlags(lyra::command& cmd, .name("--dev-user-host") .help("Username and host to stream to. See also --dev-src-dir.")); - dev_target_.ssh_port = RemoteUtil::kDefaultSshPort; - cmd.add_argument( - lyra::opt(dev_target_.ssh_port, "port") - .name("--dev-ssh-port") - .help("SSH port to use for the connection to the host, default: " + - std::to_string(RemoteUtil::kDefaultSshPort) + - ". See also --dev-src-dir.")); - cmd.add_argument( lyra::opt(dev_target_.ssh_command, "cmd") .name("--dev-ssh-command") @@ -245,7 +237,6 @@ std::string AssetStreamConfig::ToString() { << session_cfg_.file_change_wait_duration_ms << std::endl; ss << "dev-src-dir = " << dev_src_dir_ << std::endl; ss << "dev-user-host = " << dev_target_.user_host << std::endl; - ss << "dev-ssh-port = " << dev_target_.ssh_port << std::endl; ss << "dev-ssh-command = " << dev_target_.ssh_command << std::endl; ss << "dev-scp-command = " << dev_target_.scp_command diff --git a/cdc_stream/local_assets_stream_manager_client.cc b/cdc_stream/local_assets_stream_manager_client.cc index 9edd208..181cb35 100644 --- a/cdc_stream/local_assets_stream_manager_client.cc +++ b/cdc_stream/local_assets_stream_manager_client.cc @@ -36,13 +36,12 @@ LocalAssetsStreamManagerClient::LocalAssetsStreamManagerClient( LocalAssetsStreamManagerClient::~LocalAssetsStreamManagerClient() = default; absl::Status LocalAssetsStreamManagerClient::StartSession( - const std::string& src_dir, const std::string& user_host, uint16_t ssh_port, + const std::string& src_dir, const std::string& user_host, const std::string& mount_dir, const std::string& ssh_command, const std::string& scp_command) { StartSessionRequest request; request.set_workstation_directory(src_dir); request.set_user_host(user_host); - request.set_port(ssh_port); request.set_mount_dir(mount_dir); request.set_ssh_command(ssh_command); request.set_scp_command(scp_command); diff --git a/cdc_stream/local_assets_stream_manager_client.h b/cdc_stream/local_assets_stream_manager_client.h index afda9c4..15dfdeb 100644 --- a/cdc_stream/local_assets_stream_manager_client.h +++ b/cdc_stream/local_assets_stream_manager_client.h @@ -41,12 +41,11 @@ class LocalAssetsStreamManagerClient { // Starting a second session to the same target will stop the first one. // |src_dir| is the Windows source directory to stream. // |user_host| is the Linux host, formatted as [user@:host]. - // |ssh_port| is the SSH port to use while connecting to the host. // |mount_dir| is the Linux target directory to stream to. // |ssh_command| is the ssh command and extra arguments to use. // |scp_command| is the scp command and extra arguments to use. absl::Status StartSession(const std::string& src_dir, - const std::string& user_host, uint16_t ssh_port, + const std::string& user_host, const std::string& mount_dir, const std::string& ssh_command, const std::string& scp_command); diff --git a/cdc_stream/local_assets_stream_manager_service_impl.cc b/cdc_stream/local_assets_stream_manager_service_impl.cc index 98dcdf9..c4e033b 100644 --- a/cdc_stream/local_assets_stream_manager_service_impl.cc +++ b/cdc_stream/local_assets_stream_manager_service_impl.cc @@ -219,12 +219,11 @@ LocalAssetsStreamManagerServiceImpl::GetTargetForStadia( // Run 'ggp ssh init' to determine IP (host) and port. std::string instance_ip; - uint16_t instance_port = 0; - RETURN_IF_ERROR(InitSsh(*instance_id, *project_id, *organization_id, - &instance_ip, &instance_port)); + ASSIGN_OR_RETURN(instance_ip, + InitSsh(*instance_id, *project_id, *organization_id)); target.user_host = "cloudcast@" + instance_ip; - target.ssh_port = instance_port; + // Note: Port must be set with ssh_command (-p) and scp_command (-P). return target; } @@ -235,9 +234,6 @@ SessionTarget LocalAssetsStreamManagerServiceImpl::GetTarget( target.mount_dir = request.mount_dir(); target.ssh_command = request.ssh_command(); target.scp_command = request.scp_command(); - target.ssh_port = request.port() > 0 && request.port() <= UINT16_MAX - ? static_cast(request.port()) - : RemoteUtil::kDefaultSshPort; *instance_id = absl::StrCat(target.user_host, ":", target.mount_dir); return target; @@ -257,13 +253,10 @@ metrics::RequestOrigin LocalAssetsStreamManagerServiceImpl::ConvertOrigin( } } -absl::Status LocalAssetsStreamManagerServiceImpl::InitSsh( +absl::StatusOr LocalAssetsStreamManagerServiceImpl::InitSsh( const std::string& instance_id, const std::string& project_id, - const std::string& organization_id, std::string* instance_ip, - uint16_t* instance_port) { + const std::string& organization_id) { SdkUtil sdk_util; - instance_ip->clear(); - *instance_port = 0; ProcessStartInfo start_info; start_info.command = absl::StrFormat( @@ -305,22 +298,13 @@ absl::Status LocalAssetsStreamManagerServiceImpl::InitSsh( } // Parse gamelet IP. Should be "Host: ". - if (!ParseValue(output, "Host", instance_ip)) { + std::string instance_ip; + if (!ParseValue(output, "Host", &instance_ip)) { return MakeStatus("Failed to parse host from ggp ssh init response\n%s", output); } - // Parse ssh port. Should be "Port: ". - std::string port_string; - const bool result = ParseValue(output, "Port", &port_string); - int int_port = atoi(port_string.c_str()); - if (!result || int_port == 0 || int_port <= 0 || int_port > UINT_MAX) { - return MakeStatus("Failed to parse ssh port from ggp ssh init response\n%s", - output); - } - - *instance_port = static_cast(int_port); - return absl::OkStatus(); + return instance_ip; } } // namespace cdc_ft diff --git a/cdc_stream/local_assets_stream_manager_service_impl.h b/cdc_stream/local_assets_stream_manager_service_impl.h index 96a3aa9..24e8283 100644 --- a/cdc_stream/local_assets_stream_manager_service_impl.h +++ b/cdc_stream/local_assets_stream_manager_service_impl.h @@ -93,11 +93,10 @@ class LocalAssetsStreamManagerServiceImpl final // Initializes an ssh connection to a gamelet by calling 'ggp ssh init'. // |instance_id| must be set, |project_id|, |organization_id| are optional. - // Returns |instance_ip| and |instance_port| (SSH port). - absl::Status InitSsh(const std::string& instance_id, - const std::string& project_id, - const std::string& organization_id, - std::string* instance_ip, uint16_t* instance_port); + // Returns the instance's IP address. + absl::StatusOr InitSsh(const std::string& instance_id, + const std::string& project_id, + const std::string& organization_id); const SessionConfig cfg_; SessionManager* const session_manager_; diff --git a/cdc_stream/session.cc b/cdc_stream/session.cc index e014c8f..835ab55 100644 --- a/cdc_stream/session.cc +++ b/cdc_stream/session.cc @@ -47,11 +47,11 @@ Session::Session(std::string instance_id, const SessionTarget& target, mount_dir_(target.mount_dir), cfg_(std::move(cfg)), process_factory_(process_factory), - remote_util_(cfg_.verbosity, cfg_.quiet, process_factory, + remote_util_(target.user_host, cfg_.verbosity, cfg_.quiet, + process_factory, /*forward_output_to_logging=*/true), metrics_recorder_(std::move(metrics_recorder)) { assert(metrics_recorder_); - remote_util_.SetUserHostAndPort(target.user_host, target.ssh_port); if (!target.ssh_command.empty()) { remote_util_.SetSshCommand(target.ssh_command); } diff --git a/cdc_stream/session.h b/cdc_stream/session.h index c9dc9f3..14e6695 100644 --- a/cdc_stream/session.h +++ b/cdc_stream/session.h @@ -36,8 +36,6 @@ class Process; struct SessionTarget { // SSH username and hostname of the remote target, formed as [user@]host. std::string user_host; - // Port to use for SSH connections to the remote target. - uint16_t ssh_port; // Ssh command to use to connect to the remote target. std::string ssh_command; // Scp command to use to copy files to the remote target. diff --git a/cdc_stream/start_command.cc b/cdc_stream/start_command.cc index 2f4bc06..393ccd3 100644 --- a/cdc_stream/start_command.cc +++ b/cdc_stream/start_command.cc @@ -72,20 +72,12 @@ void StartCommand::RegisterCommandLineFlags(lyra::command& cmd) { "asset stream service, default: " + std::to_string(SessionManagementServer::kDefaultServicePort))); - ssh_port_ = RemoteUtil::kDefaultSshPort; - cmd.add_argument( - lyra::opt(ssh_port_, "port") - .name("--ssh-port") - .help("Port to use while connecting to the remote instance being " - "streamed to, default: " + - std::to_string(RemoteUtil::kDefaultSshPort))); - path::GetEnv("CDC_SSH_COMMAND", &ssh_command_).IgnoreError(); cmd.add_argument( lyra::opt(ssh_command_, "ssh_command") .name("--ssh-command") .help("Path and arguments of ssh command to use, e.g. " - "\"C:\\path\\to\\ssh.exe -F config_file\". Can also be " + "\"C:\\path\\to\\ssh.exe -F config_file -p 1234\". Can also be " "specified by the CDC_SSH_COMMAND environment variable.")); path::GetEnv("CDC_SCP_COMMAND", &scp_command_).IgnoreError(); @@ -93,7 +85,7 @@ void StartCommand::RegisterCommandLineFlags(lyra::command& cmd) { lyra::opt(scp_command_, "scp_command") .name("--scp-command") .help("Path and arguments of scp command to use, e.g. " - "\"C:\\path\\to\\scp.exe -F config_file\". Can also be " + "\"C:\\path\\to\\scp.exe -F config_file -P 1234\". Can also be " "specified by the CDC_SCP_COMMAND environment variable.")); cmd.add_argument(lyra::arg(PosArgValidator(&src_dir_), "dir") @@ -116,9 +108,8 @@ absl::Status StartCommand::Run() { user_host_dir_, &user_host, &mount_dir)); LocalAssetsStreamManagerClient client(CreateChannel(service_port_)); - absl::Status status = - client.StartSession(full_src_dir, user_host, ssh_port_, mount_dir, - ssh_command_, scp_command_); + absl::Status status = client.StartSession(full_src_dir, user_host, mount_dir, + ssh_command_, scp_command_); if (absl::IsUnavailable(status)) { LOG_DEBUG("StartSession status: %s", status.ToString()); @@ -131,8 +122,8 @@ absl::Status StartCommand::Run() { // Recreate client. The old channel might still be in a transient failure // state. LocalAssetsStreamManagerClient new_client(CreateChannel(service_port_)); - status = new_client.StartSession(full_src_dir, user_host, ssh_port_, - mount_dir, ssh_command_, scp_command_); + status = new_client.StartSession(full_src_dir, user_host, mount_dir, + ssh_command_, scp_command_); } } diff --git a/cdc_stream/start_command.h b/cdc_stream/start_command.h index bf14dc2..a4eb01f 100644 --- a/cdc_stream/start_command.h +++ b/cdc_stream/start_command.h @@ -43,7 +43,6 @@ class StartCommand : public BaseCommand { int verbosity_ = 0; uint16_t service_port_ = 0; - uint16_t ssh_port_ = 0; std::string ssh_command_; std::string scp_command_; std::string src_dir_; diff --git a/cdc_stream/start_service_command.cc b/cdc_stream/start_service_command.cc index 4e915c4..5035143 100644 --- a/cdc_stream/start_service_command.cc +++ b/cdc_stream/start_service_command.cc @@ -140,7 +140,6 @@ absl::Status StartServiceCommand::RunService() { request.set_workstation_directory(cfg_.dev_src_dir()); request.set_user_host(cfg_.dev_target().user_host); request.set_mount_dir(cfg_.dev_target().mount_dir); - request.set_port(cfg_.dev_target().ssh_port); request.set_ssh_command(cfg_.dev_target().ssh_command); request.set_scp_command(cfg_.dev_target().scp_command); localassetsstreammanager::StartSessionResponse response; diff --git a/common/port_manager_test.cc b/common/port_manager_test.cc index d106185..72451c9 100644 --- a/common/port_manager_test.cc +++ b/common/port_manager_test.cc @@ -49,14 +49,14 @@ constexpr char kRemoteNetstatOutFmt[] = class PortManagerTest : public ::testing::Test { public: PortManagerTest() - : remote_util_(/*verbosity=*/0, /*quiet=*/false, &process_factory_, + : remote_util_(kUserHost, /*verbosity=*/0, /*quiet=*/false, + &process_factory_, /*forward_output_to_log=*/true), port_manager_(kGuid, kFirstPort, kLastPort, &process_factory_, &remote_util_, &system_clock_, &steady_clock_) {} void SetUp() override { Log::Initialize(std::make_unique(LogLevel::kInfo)); - remote_util_.SetUserHostAndPort(kUserHost, kSshPort); } void TearDown() override { Log::Shutdown(); } diff --git a/common/remote_util.cc b/common/remote_util.cc index d3530ea..576f616 100644 --- a/common/remote_util.cc +++ b/common/remote_util.cc @@ -20,7 +20,6 @@ #include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" #include "common/path.h" -#include "common/status.h" namespace cdc_ft { namespace { @@ -35,19 +34,15 @@ std::string GetPortForwardingArg(int local_port, int remote_port, } // namespace -RemoteUtil::RemoteUtil(int verbosity, bool quiet, +RemoteUtil::RemoteUtil(std::string user_host, int verbosity, bool quiet, ProcessFactory* process_factory, bool forward_output_to_log) - : verbosity_(verbosity), + : user_host_(std::move(user_host)), + verbosity_(verbosity), quiet_(quiet), process_factory_(process_factory), forward_output_to_log_(forward_output_to_log) {} -void RemoteUtil::SetUserHostAndPort(std::string user_host, int port) { - user_host_ = std::move(user_host); - ssh_port_ = port; -} - void RemoteUtil::SetScpCommand(std::string scp_command) { scp_command_ = std::move(scp_command); } @@ -58,11 +53,6 @@ void RemoteUtil::SetSshCommand(std::string ssh_command) { absl::Status RemoteUtil::Scp(std::vector source_filepaths, const std::string& dest, bool compress) { - absl::Status status = CheckUserHostPort(); - if (!status.ok()) { - return status; - } - std::string source_args; for (const std::string& sourceFilePath : source_filepaths) { // Workaround for scp thinking that C is a host in C:\path\to\foo. @@ -77,13 +67,10 @@ absl::Status RemoteUtil::Scp(std::vector source_filepaths, ProcessStartInfo start_info; start_info.flags = ProcessFlags::kNoWindow; start_info.command = absl::StrFormat( - "%s " - "%s %s -p -T " - "-P %i %s " - "%s:%s", + "%s %s %s -p -T " + "%s %s:%s", scp_command_, quiet_ || verbosity_ < 2 ? "-q" : "", compress ? "-C" : "", - ssh_port_, source_args, QuoteForWindows(user_host_), - QuoteForWindows(dest)); + source_args, QuoteForWindows(user_host_), QuoteForWindows(dest)); start_info.name = "scp"; start_info.forward_output_to_log = forward_output_to_log_; @@ -100,11 +87,6 @@ absl::Status RemoteUtil::Chmod(const std::string& mode, } absl::Status RemoteUtil::Run(std::string remote_command, std::string name) { - absl::Status status = CheckUserHostPort(); - if (!status.ok()) { - return status; - } - ProcessStartInfo start_info = BuildProcessStartInfoForSsh(std::move(remote_command)); start_info.name = std::move(name); @@ -140,13 +122,12 @@ ProcessStartInfo RemoteUtil::BuildProcessStartInfoForSshInternal( std::string forward_arg, std::string remote_command_arg) { ProcessStartInfo start_info; start_info.command = absl::StrFormat( - "%s " - "%s -tt " + "%s %s -tt %s " "-oServerAliveCountMax=6 " // Number of lost msgs before ssh terminates "-oServerAliveInterval=5 " // Time interval between alive msgs - "%s %s -p %i %s", + "%s %s", ssh_command_, quiet_ || verbosity_ < 2 ? "-q" : "", forward_arg, - QuoteForWindows(user_host_), ssh_port_, remote_command_arg); + QuoteForWindows(user_host_), remote_command_arg); start_info.forward_output_to_log = forward_output_to_log_; start_info.flags = ProcessFlags::kNoWindow; return start_info; @@ -200,12 +181,4 @@ std::string RemoteUtil::QuoteForSsh(const std::string& argument) { escaped.substr(slash_pos + 1), "\"")); } -absl::Status RemoteUtil::CheckUserHostPort() { - if (user_host_.empty() || ssh_port_ == 0) { - return MakeStatus("IP or port not set"); - } - - return absl::OkStatus(); -} - } // namespace cdc_ft diff --git a/common/remote_util.h b/common/remote_util.h index 2916486..8f7d714 100644 --- a/common/remote_util.h +++ b/common/remote_util.h @@ -29,46 +29,36 @@ namespace cdc_ft { // Windows-only. class RemoteUtil { public: - static constexpr int kDefaultSshPort = 22; - + // |user_host| is the SSH [user@]host of the remote instance. // If |verbosity| is > 0 and |quiet| is false, output from scp, ssh etc. // commands is shown. // If |quiet| is true, scp, ssh etc. commands use quiet mode. // If |forward_output_to_log| is true, process output is forwarded to logging // instead of this process's stdout/stderr. - RemoteUtil(int verbosity, bool quiet, ProcessFactory* process_factory, - bool forward_output_to_log); - - // Sets the SSH username and hostname of the remote instance, as well as the - // SSH tunnel port. |user_host| must be of the form [user@]host. - void SetUserHostAndPort(std::string user_host, int port); + RemoteUtil(std::string user_host, int verbosity, bool quiet, + ProcessFactory* process_factory, bool forward_output_to_log); // Sets the SCP command binary path and additional arguments, e.g. - // C:\path\to\scp.exe -F -i - // -oStrictHostKeyChecking=yes -oUserKnownHostsFile="""file""" + // C:\path\to\scp.exe -p 1234 -i -oUserKnownHostsFile=known_hosts // By default, searches scp.exe on the path environment variables. void SetScpCommand(std::string scp_command); // Sets the SSH command binary path and additional arguments, e.g. - // C:\path\to\ssh.exe -F -i - // -oStrictHostKeyChecking=yes -oUserKnownHostsFile="""file""" + // C:\path\to\ssh.exe -P 1234 -i -oUserKnownHostsFile=known_hosts // By default, searches ssh.exe on the path environment variables. void SetSshCommand(std::string ssh_command); // Copies |source_filepaths| to the remote folder |dest| on the gamelet using // scp. If |compress| is true, compressed upload is used. - // Must call SetUserHostAndPort before calling this method. absl::Status Scp(std::vector source_filepaths, const std::string& dest, bool compress); // Calls 'chmod |mode| |remote_path|' on the gamelet. - // Must call SetUserHostAndPort before calling this method. absl::Status Chmod(const std::string& mode, const std::string& remote_path, bool quiet = false); // Runs |remote_command| on the gamelet. The command must be properly escaped. // |name| is the name of the command displayed in the logs. - // Must call SetUserHostAndPort before calling this method. absl::Status Run(std::string remote_command, std::string name); // Builds an SSH command that executes |remote_command| on the gamelet. @@ -77,7 +67,6 @@ class RemoteUtil { // Builds an SSH command that runs SSH port forwarding to the gamelet, using // the given |local_port| and |remote_port|. // If |reverse| is true, sets up reverse port forwarding. - // Must call SetUserHostAndPort before calling this method. ProcessStartInfo BuildProcessStartInfoForSshPortForward(int local_port, int remote_port, bool reverse); @@ -85,7 +74,6 @@ class RemoteUtil { // Builds an SSH command that executes |remote_command| on the gamelet, using // port forwarding with given |local_port| and |remote_port|. // If |reverse| is true, sets up reverse port forwarding. - // Must call SetUserHostAndPort before calling this method. ProcessStartInfo BuildProcessStartInfoForSshPortForwardAndCommand( int local_port, int remote_port, bool reverse, std::string remote_command); @@ -117,9 +105,6 @@ class RemoteUtil { static std::string QuoteForSsh(const std::string& argument); private: - // Verifies that both |user_host_| and |ssh_port_| are set. - absl::Status CheckUserHostPort(); - // Common code for BuildProcessStartInfoForSsh*. ProcessStartInfo BuildProcessStartInfoForSshInternal( std::string forward_arg, std::string remote_command); @@ -132,7 +117,6 @@ class RemoteUtil { std::string scp_command_ = "scp"; std::string ssh_command_ = "ssh"; std::string user_host_; - int ssh_port_ = kDefaultSshPort; }; } // namespace cdc_ft diff --git a/common/remote_util_test.cc b/common/remote_util_test.cc index 11293b8..3b68f1d 100644 --- a/common/remote_util_test.cc +++ b/common/remote_util_test.cc @@ -21,9 +21,6 @@ namespace cdc_ft { namespace { -constexpr int kSshPort = 12345; -constexpr char kSshPortArg[] = "-p 12345"; - constexpr char kUserHost[] = "user@example.com"; constexpr char kUserHostArg[] = "\"user@example.com\""; @@ -39,12 +36,11 @@ constexpr char kCommand[] = "my_command"; class RemoteUtilTest : public ::testing::Test { public: RemoteUtilTest() - : util_(/*verbosity=*/0, /*quiet=*/false, &process_factory_, + : util_(kUserHost, /*verbosity=*/0, /*quiet=*/false, &process_factory_, /*forward_output_to_log=*/true) {} void SetUp() override { Log::Initialize(std::make_unique(LogLevel::kInfo)); - util_.SetUserHostAndPort(kUserHost, kSshPort); } void TearDown() override { Log::Shutdown(); } @@ -64,31 +60,29 @@ class RemoteUtilTest : public ::testing::Test { TEST_F(RemoteUtilTest, BuildProcessStartInfoForSsh) { ProcessStartInfo si = util_.BuildProcessStartInfoForSsh(kCommand); - ExpectContains(si.command, {"ssh", kSshPortArg, kUserHostArg, kCommand}); + ExpectContains(si.command, {"ssh", kUserHostArg, kCommand}); } TEST_F(RemoteUtilTest, BuildProcessStartInfoForSshPortForward) { ProcessStartInfo si = util_.BuildProcessStartInfoForSshPortForward( kLocalPort, kRemotePort, kRegular); - ExpectContains(si.command, - {"ssh", kSshPortArg, kUserHostArg, kPortForwardingArg}); + ExpectContains(si.command, {"ssh", kUserHostArg, kPortForwardingArg}); si = util_.BuildProcessStartInfoForSshPortForward(kLocalPort, kRemotePort, kReverse); - ExpectContains(si.command, - {"ssh", kSshPortArg, kUserHostArg, kReversePortForwardingArg}); + ExpectContains(si.command, {"ssh", kUserHostArg, kReversePortForwardingArg}); } TEST_F(RemoteUtilTest, BuildProcessStartInfoForSshPortForwardAndCommand) { ProcessStartInfo si = util_.BuildProcessStartInfoForSshPortForwardAndCommand( kLocalPort, kRemotePort, kRegular, kCommand); - ExpectContains(si.command, {"ssh", kSshPortArg, kUserHostArg, - kPortForwardingArg, kCommand}); + ExpectContains(si.command, + {"ssh", kUserHostArg, kPortForwardingArg, kCommand}); si = util_.BuildProcessStartInfoForSshPortForwardAndCommand( kLocalPort, kRemotePort, kReverse, kCommand); - ExpectContains(si.command, {"ssh", kSshPortArg, kUserHostArg, - kReversePortForwardingArg, kCommand}); + ExpectContains(si.command, + {"ssh", kUserHostArg, kReversePortForwardingArg, kCommand}); } TEST_F(RemoteUtilTest, BuildProcessStartInfoForSshWithCustomCommand) { constexpr char kCustomSshCmd[] = "C:\\path\\to\\ssh.exe --fooarg --bararg=42"; diff --git a/proto/local_assets_stream_manager.proto b/proto/local_assets_stream_manager.proto index f43eae6..4e4b55f 100644 --- a/proto/local_assets_stream_manager.proto +++ b/proto/local_assets_stream_manager.proto @@ -45,9 +45,6 @@ message StartSessionRequest { string user_host = 7; // Remote directory where to mount the streamed directory. string mount_dir = 8; - // SSH port to use while connecting to the remote instance. - // Optional, falls back to port 22 (default SSH port). - int32 port = 9; // SSH command to connect to the remote instance. // Optional, falls back to searching ssh. string ssh_command = 10; @@ -55,7 +52,7 @@ message StartSessionRequest { // Optional, falls back to searching scp. string scp_command = 11; - reserved 1, 3, 4; + reserved 1, 3, 4, 9; } message StartSessionResponse {}