[cdc_rsync] [cdc_stream] Remove SSH port argument (#41)

This CL removes the port arguments for both tools.

The port argument can also be specified via the ssh-command and
scp-command flags. In fact, if a port is specified by both port flags
and ssh/scp commands, they interfere with each other. For ssh, the one
specified in ssh-command wins. For scp, the one specified in
scp-command wins. To fix this, one would have to parse scp-command and
remove the port arg there. Or we could just remove the ssh-port arg.
This is what this CL does. Note that if you need a custom port, it's
very likely that you also have to define custom ssh and scp commands.

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
+If additional arguments are required, it is recommended to provide an SSH config
-required, it is recommended to set the environment variables `CDC_SSH_COMMAND`
+file. By default, both `ssh.exe` and `scp.exe` use the file at
-and `CDC_SCP_COMMAND`. The following example specifies a custom path to the SSH
+`%USERPROFILE%\.ssh\config` on Windows, if it exists. A possible config file
-and SCP binaries, a custom SSH config file, a key file and a known hosts 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"""
+Host linux_device
-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"""
+	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
 

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)) {

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<std::string> sources_;
-  const std::string user_host_;
   const std::string destination_;
   WinProcessFactory process_factory_;
   RemoteUtil remote_util_;

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' "
+          "Server connection timed out. Verify that the host '%s' "
-          "are correct, or specify a larger timeout with --contimeout.",
+          "is correct, or specify a larger timeout with --contimeout.",
-          params.user_host, params.options.port);
+          params.user_host);
       return ReturnCode::kConnectionTimeout;
 
     case cdc_ft::Tag::kCount:

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);

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<int>(std::size(argv)) - 1, argv, &parameters_));
-  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<int>(std::size(argv)) - 1, argv, &parameters_));
-  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[] = {
+  const char* argv[] = {"cdc_rsync.exe",
-      "cdc_rsync.exe", "--compress-level", "11", "--contimeout", "99", "--port",
+                        "--compress-level",
-      "4086",          "--copy-dest=dest", kSrc, kUserHostDst,   NULL};
+                        "11",
+                        "--contimeout",
+                        "99",
+                        "--copy-dest=dest",
+                        kSrc,
+                        kUserHostDst,
+                        NULL};
   EXPECT_TRUE(Parse(static_cast<int>(std::size(argv)) - 1, argv, &parameters_));
   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<int>(std::size(argv)) - 1, argv, &parameters_));
-  ExpectError("--port must specify a valid port");
-}
-
 TEST_F(ParamsTest, ParseFailsOnDeleteNeedsRecursive) {
   const char* argv[] = {"cdc_rsync.exe", "--delete", kSrc, kUserHostDst, NULL};
   EXPECT_FALSE(

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

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);

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);

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;
+  ASSIGN_OR_RETURN(instance_ip,
-  RETURN_IF_ERROR(InitSsh(*instance_id, *project_id, *organization_id,
+                   InitSsh(*instance_id, *project_id, *organization_id));
-                          &instance_ip, &instance_port));
 
   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<uint16_t>(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<std::string> LocalAssetsStreamManagerServiceImpl::InitSsh(
     const std::string& instance_id, const std::string& project_id,
-    const std::string& organization_id, std::string* instance_ip,
+    const std::string& organization_id) {
-    uint16_t* instance_port) {
   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: <instance_ip ip>".
-  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: <port>".
+  return instance_ip;
-  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<uint16_t>(int_port);
-  return absl::OkStatus();
 }
 
 }  // namespace cdc_ft

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).
+  // Returns the instance's IP address.
-  absl::Status InitSsh(const std::string& instance_id,
+  absl::StatusOr<std::string> InitSsh(const std::string& instance_id,
                                       const std::string& project_id,
-                       const std::string& organization_id,
+                                      const std::string& organization_id);
-                       std::string* instance_ip, uint16_t* instance_port);
 
   const SessionConfig cfg_;
   SessionManager* const session_manager_;

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);
   }

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.

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,8 +108,7 @@ absl::Status StartCommand::Run() {
       user_host_dir_, &user_host, &mount_dir));
 
   LocalAssetsStreamManagerClient client(CreateChannel(service_port_));
-  absl::Status status =
+  absl::Status status = client.StartSession(full_src_dir, user_host, mount_dir,
-      client.StartSession(full_src_dir, user_host, ssh_port_, mount_dir,
                                             ssh_command_, scp_command_);
 
   if (absl::IsUnavailable(status)) {
@@ -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_,
+      status = new_client.StartSession(full_src_dir, user_host, mount_dir,
-                                       mount_dir, ssh_command_, scp_command_);
+                                       ssh_command_, scp_command_);
     }
   }
 

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_;

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;

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<ConsoleLog>(LogLevel::kInfo));
-    remote_util_.SetUserHostAndPort(kUserHost, kSshPort);
   }
 
   void TearDown() override { Log::Shutdown(); }

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<std::string> 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<std::string> source_filepaths,
   ProcessStartInfo start_info;
   start_info.flags = ProcessFlags::kNoWindow;
   start_info.command = absl::StrFormat(
-      "%s "
+      "%s %s %s -p -T "
-      "%s %s -p -T "
+      "%s %s:%s",
-      "-P %i %s "
-      "%s:%s",
       scp_command_, quiet_ || verbosity_ < 2 ? "-q" : "", compress ? "-C" : "",
-      ssh_port_, source_args, QuoteForWindows(user_host_),
+      source_args, QuoteForWindows(user_host_), QuoteForWindows(dest));
-      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 %s -tt %s "
-      "%s -tt "
       "-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

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,
+  RemoteUtil(std::string user_host, int verbosity, bool quiet,
-             bool forward_output_to_log);
+             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);
 
   // Sets the SCP command binary path and additional arguments, e.g.
-  //   C:\path\to\scp.exe -F <ssh_config> -i <key_file>
+  //   C:\path\to\scp.exe -p 1234 -i <key_file> -oUserKnownHostsFile=known_hosts
-  //     -oStrictHostKeyChecking=yes -oUserKnownHostsFile="""file"""
   // 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 <ssh_config> -i <key_file>
+  //   C:\path\to\ssh.exe -P 1234 -i <key_file> -oUserKnownHostsFile=known_hosts
-  //     -oStrictHostKeyChecking=yes -oUserKnownHostsFile="""file"""
   // 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<std::string> 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

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<ConsoleLog>(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,
+  ExpectContains(si.command, {"ssh", kUserHostArg, kPortForwardingArg});
-                 {"ssh", kSshPortArg, kUserHostArg, kPortForwardingArg});
 
   si = util_.BuildProcessStartInfoForSshPortForward(kLocalPort, kRemotePort,
                                                     kReverse);
-  ExpectContains(si.command,
+  ExpectContains(si.command, {"ssh", kUserHostArg, kReversePortForwardingArg});
-                 {"ssh", kSshPortArg, kUserHostArg, kReversePortForwardingArg});
 }
 
 TEST_F(RemoteUtilTest, BuildProcessStartInfoForSshPortForwardAndCommand) {
   ProcessStartInfo si = util_.BuildProcessStartInfoForSshPortForwardAndCommand(
       kLocalPort, kRemotePort, kRegular, kCommand);
-  ExpectContains(si.command, {"ssh", kSshPortArg, kUserHostArg,
+  ExpectContains(si.command,
-                              kPortForwardingArg, kCommand});
+                 {"ssh", kUserHostArg, kPortForwardingArg, kCommand});
 
   si = util_.BuildProcessStartInfoForSshPortForwardAndCommand(
       kLocalPort, kRemotePort, kReverse, kCommand);
-  ExpectContains(si.command, {"ssh", kSshPortArg, kUserHostArg,
+  ExpectContains(si.command,
-                              kReversePortForwardingArg, kCommand});
+                 {"ssh", kUserHostArg, kReversePortForwardingArg, kCommand});
 }
 TEST_F(RemoteUtilTest, BuildProcessStartInfoForSshWithCustomCommand) {
   constexpr char kCustomSshCmd[] = "C:\\path\\to\\ssh.exe --fooarg --bararg=42";

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 {}