diff --git a/README.md b/README.md index 4562cfd..949eeca 100644 --- a/README.md +++ b/README.md @@ -191,7 +191,7 @@ To build the tools from source, the following steps have to be executed on ``` Finally, install an SSH client on the Windows device if not present. -The file transfer tools require `ssh.exe` and `scp.exe`. +The file transfer tools require `ssh.exe` and `sftp.exe`. ## Building @@ -227,24 +227,24 @@ The two tools can be built and used independently. ## Usage -The tools require a setup where you can use SSH and SCP from the Windows machine -to the Linux device without entering a password, e.g. by using key-based +The tools require a setup where you can use SSH and SFTP from the Windows +machine to the Linux device without entering a password, e.g. by using key-based authentication. -### Configuring SSH and SCP +### Configuring SSH and SFTP -By default, the tools search `ssh.exe` and `scp.exe` from the path environment +By default, the tools search `ssh.exe` and `sftp.exe` from the path environment variable. If you can run the following commands in a Windows cmd without entering your password, you are all set: ``` ssh user@linux.device.com -scp somefile.txt user@linux.device.com: +sftp 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 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 +file. By default, both `ssh.exe` and `sftp.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: @@ -256,21 +256,21 @@ Host linux_device 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` +If `ssh.exe` or `sftp.exe` cannot be found, you can specify the full paths via +the command line arguments `--ssh-command` and `--sftp-command` for `cdc_rsync` and `cdc_stream start` (see below), or set the environment variables -`CDC_SSH_COMMAND` and `CDC_SCP_COMMAND`, e.g. +`CDC_SSH_COMMAND` and `CDC_SFTP_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" +set CDC_SFTP_COMMAND="C:\path with space\to\sftp.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 +set CDC_SFTP_COMMAND=C:\path\to\sftp.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`. +Note the small `-p` for `ssh.exe` and the capital `-P` for `sftp.exe`. #### Google Specific @@ -278,7 +278,7 @@ For Google internal usage, set the following environment variables to enable SSH authentication using a Google security key: ``` set CDC_SSH_COMMAND=C:\gnubby\bin\ssh.exe -set CDC_SCP_COMMAND=C:\gnubby\bin\scp.exe +set CDC_SFTP_COMMAND=C:\gnubby\bin\sftp.exe ``` Note that you will have to touch the security key multiple times during the first run. Subsequent runs only require a single touch. @@ -352,7 +352,7 @@ instead of to the file. `cdc_rsync` always logs to the console. To increase log verbosity, pass `-vvv` for debug logs or `-vvvv` for verbose logs. -For both sync and stream, the debug logs contain all SSH and SCP commands that +For both sync and stream, the debug logs contain all SSH and SFTP commands that are attempted to run, which is very useful for troubleshooting. If a command fails unexpectedly, copy it and run it in isolation. Pass `-vv` or `-vvv` for additional debug output. diff --git a/cdc_rsync/cdc_rsync_client.cc b/cdc_rsync/cdc_rsync_client.cc index 677f7ec..06cea47 100644 --- a/cdc_rsync/cdc_rsync_client.cc +++ b/cdc_rsync/cdc_rsync_client.cc @@ -109,8 +109,8 @@ CdcRsyncClient::CdcRsyncClient(const Options& options, if (!options_.ssh_command.empty()) { remote_util_.SetSshCommand(options_.ssh_command); } - if (!options_.scp_command.empty()) { - remote_util_.SetScpCommand(options_.scp_command); + if (!options_.sftp_command.empty()) { + remote_util_.SetSftpCommand(options_.sftp_command); } } @@ -413,26 +413,10 @@ absl::Status CdcRsyncClient::DeployServer(const ServerArch& arch) { } printer_.Print(deploy_msg, true, Util::GetConsoleWidth()); - // scp cdc_rsync_server to a temp location on the gamelet. - std::string remoteServerPath = - arch.RemoteToolsBinDir(ServerArch::UseCase::kScp) + - arch.CdcServerFilename(); - std::string remoteServerTmpPath = remoteServerPath + Util::GenerateUniqueId(); - std::string localServerPath = path::Join(exe_dir, arch.CdcServerFilename()); - status = remote_util_.Scp({localServerPath}, remoteServerTmpPath, - /*compress=*/true); - if (!status.ok()) { - return WrapStatus(status, "Failed to copy cdc_rsync_server to instance"); - } - - // Replace cdc_rsync_server file by the temp file. - std::string replace_cmd = - arch.GetDeployReplaceCommand(remoteServerPath, remoteServerTmpPath); - status = remote_util_.Run(replace_cmd, "replace"); - if (!status.ok()) { - return WrapStatus(status, - "Failed to replace old cdc_rsync_server by new one"); - } + // sftp cdc_rsync_server to the target. + std::string commands = arch.GetDeploySftpCommands(); + RETURN_IF_ERROR(remote_util_.Sftp(commands, exe_dir, /*compress=*/false), + "Failed to deploy cdc_rsync_server"); return absl::OkStatus(); } diff --git a/cdc_rsync/cdc_rsync_client.h b/cdc_rsync/cdc_rsync_client.h index d6e9b63..0af1635 100644 --- a/cdc_rsync/cdc_rsync_client.h +++ b/cdc_rsync/cdc_rsync_client.h @@ -54,10 +54,14 @@ class CdcRsyncClient { int forward_port_first = 44450; int forward_port_last = 44459; std::string ssh_command; - std::string scp_command; + std::string sftp_command; std::string sources_dir; // Base dir for files loaded for --files-from. PathFilter filter; + // Backwards compatibility for switching from scp to sftp. + // Used internally, do not use. + std::string deprecated_scp_command; + // Compression level 0 is invalid. static constexpr int kMinCompressLevel = -5; static constexpr int kMaxCompressLevel = 22; diff --git a/cdc_rsync/params.cc b/cdc_rsync/params.cc index 241f6a7..69be6ac 100644 --- a/cdc_rsync/params.cc +++ b/cdc_rsync/params.cc @@ -21,6 +21,7 @@ #include "absl/strings/str_split.h" #include "common/path.h" #include "common/port_range_parser.h" +#include "common/remote_util.h" #include "lib/zstd.h" namespace cdc_ft { @@ -75,9 +76,9 @@ Options: --ssh-command Path and arguments of ssh command to use, e.g. "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 -P 12345 -i id_rsa -oUserKnownHostsFile=known_hosts" - Can also be specified by the CDC_SCP_COMMAND environment variable. + --sftp-command Path and arguments of sftp command to use, e.g. + "C:\path\to\sftp.exe -P 12345 -i id_rsa -oUserKnownHostsFile=known_hosts" + Can also be specified by the CDC_SFTP_COMMAND environment variable. --forward-port TCP port or range used for SSH port forwarding (default: 44450-44459). If a range is specified, searches for available ports (slower). -h --help Help for cdc_rsync @@ -85,12 +86,15 @@ Options: constexpr char kSshCommandEnvVar[] = "CDC_SSH_COMMAND"; constexpr char kScpCommandEnvVar[] = "CDC_SCP_COMMAND"; +constexpr char kSftpCommandEnvVar[] = "CDC_SFTP_COMMAND"; // Populates some parameters from environment variables. void PopulateFromEnvVars(Parameters* parameters) { path::GetEnv(kSshCommandEnvVar, ¶meters->options.ssh_command) .IgnoreError(); - path::GetEnv(kScpCommandEnvVar, ¶meters->options.scp_command) + path::GetEnv(kScpCommandEnvVar, ¶meters->options.deprecated_scp_command) + .IgnoreError(); + path::GetEnv(kSftpCommandEnvVar, ¶meters->options.sftp_command) .IgnoreError(); } @@ -287,8 +291,15 @@ OptionResult HandleParameter(const std::string& key, const char* value, } if (key == "scp-command") { + // Backwards compatibility. Note that this flag is hidden from the help. if (!ValidateValue(key, value)) return OptionResult::kError; - params->options.scp_command = value; + params->options.deprecated_scp_command = value; + return OptionResult::kConsumedKeyValue; + } + + if (key == "sftp-command") { + if (!ValidateValue(key, value)) return OptionResult::kError; + params->options.sftp_command = value; return OptionResult::kConsumedKeyValue; } @@ -491,6 +502,27 @@ bool Parse(int argc, const char* const* argv, Parameters* parameters) { PopUserHost(¶meters->destination, ¶meters->user_host); + // Backwards compabitility after switching to sftp. Convert scp to sftp + // command Note that this flag is hidden from the help. + if (parameters->options.sftp_command.empty() && + !parameters->options.deprecated_scp_command.empty()) { + LOG_WARNING( + "The CDC_SCP_COMMAND environment variable and the --scp-command flag " + "are deprecated. Please set CDC_SFTP_COMMAND or --sftp-command " + "instead."); + + parameters->options.sftp_command = RemoteUtil::ScpToSftpCommand( + parameters->options.deprecated_scp_command); + if (!parameters->options.sftp_command.empty()) { + LOG_WARNING("Converted scp command '%s' to sftp command '%s'.", + parameters->options.deprecated_scp_command, + parameters->options.sftp_command); + } else { + LOG_WARNING("Failed to convert scp command '%s' to sftp command.", + parameters->options.deprecated_scp_command); + } + } + if (!ValidateParameters(*parameters, help)) { return false; } diff --git a/cdc_rsync/params_test.cc b/cdc_rsync/params_test.cc index 24ae2fa..9b8a2f7 100644 --- a/cdc_rsync/params_test.cc +++ b/cdc_rsync/params_test.cc @@ -32,6 +32,10 @@ constexpr char kUserHostDst[] = "user@host:destination"; constexpr char kUserHost[] = "user@host"; constexpr char kDst[] = "destination"; +constexpr char kSshCommandEnvVar[] = "CDC_SSH_COMMAND"; +constexpr char kScpCommandEnvVar[] = "CDC_SCP_COMMAND"; +constexpr char kSftpCommandEnvVar[] = "CDC_SFTP_COMMAND"; + class TestLog : public Log { public: explicit TestLog() : Log(LogLevel::kInfo) {} @@ -60,6 +64,11 @@ class ParamsTest : public ::testing::Test { void TearDown() override { std::cout.rdbuf(prev_stdout_); std::cerr.rdbuf(prev_stderr_); + + // Clear env. They seem to be sticky sometimes and leak into other tests. + path::SetEnv(kSshCommandEnvVar, ""); + path::SetEnv(kScpCommandEnvVar, ""); + path::SetEnv(kSftpCommandEnvVar, ""); } protected: @@ -152,24 +161,57 @@ TEST_F(ParamsTest, ParseFailsOnContimeoutEqualsNoValue) { ExpectError(NeedsValueError("contimeout")); } -TEST_F(ParamsTest, ParseSucceedsWithSshScpCommands) { - const char* argv[] = {"cdc_rsync.exe", kSrc, - kUserHostDst, "--ssh-command=sshcmd", - "--scp-command=scpcmd", NULL}; +TEST_F(ParamsTest, ParseSucceedsWithSshSftpCommands) { + const char* argv[] = { + "cdc_rsync.exe", kSrc, kUserHostDst, "--ssh-command=sshcmd", + "--sftp-command=sftpcmd", NULL}; EXPECT_TRUE(Parse(static_cast(std::size(argv)) - 1, argv, ¶meters_)); - EXPECT_EQ(parameters_.options.scp_command, "scpcmd"); + EXPECT_EQ(parameters_.options.sftp_command, "sftpcmd"); EXPECT_EQ(parameters_.options.ssh_command, "sshcmd"); } -TEST_F(ParamsTest, ParseSucceedsWithSshScpCommandsByEnvVars) { - EXPECT_OK(path::SetEnv("CDC_SSH_COMMAND", "sshcmd")); - EXPECT_OK(path::SetEnv("CDC_SCP_COMMAND", "scpcmd")); +TEST_F(ParamsTest, ParseSucceedsWithSshSftpCommandsByEnvVars) { + EXPECT_OK(path::SetEnv(kSshCommandEnvVar, "sshcmd")); + EXPECT_OK(path::SetEnv(kSftpCommandEnvVar, "sftpcmd")); const char* argv[] = {"cdc_rsync.exe", kSrc, kUserHostDst, NULL}; EXPECT_TRUE(Parse(static_cast(std::size(argv)) - 1, argv, ¶meters_)); - EXPECT_EQ(parameters_.options.scp_command, "scpcmd"); + EXPECT_EQ(parameters_.options.sftp_command, "sftpcmd"); EXPECT_EQ(parameters_.options.ssh_command, "sshcmd"); } +TEST_F(ParamsTest, ParseSucceedsWithScpCommandFallback) { + const char* argv[] = {"cdc_rsync.exe", kSrc, kUserHostDst, + "--scp-command=C:\\scp.exe foo", NULL}; + EXPECT_TRUE(Parse(static_cast(std::size(argv)) - 1, argv, ¶meters_)); + EXPECT_EQ(parameters_.options.sftp_command, "C:\\sftp.exe foo"); +} + +TEST_F(ParamsTest, ParseSucceedsWithScpCommandFallbackByEnvVar) { + EXPECT_OK(path::SetEnv(kScpCommandEnvVar, "C:\\scp.exe foo")); + const char* argv[] = {"cdc_rsync.exe", kSrc, kUserHostDst, NULL}; + EXPECT_TRUE(Parse(static_cast(std::size(argv)) - 1, argv, ¶meters_)); + EXPECT_EQ(parameters_.options.sftp_command, "C:\\sftp.exe foo"); +} + +TEST_F(ParamsTest, ParseSucceedsWithSftpOverwritingScp) { + const char* argv[] = {"cdc_rsync.exe", + kSrc, + kUserHostDst, + "--scp-command=C:\\scp.exe foo", + "--sftp-command=sftpcmd", + NULL}; + EXPECT_TRUE(Parse(static_cast(std::size(argv)) - 1, argv, ¶meters_)); + EXPECT_EQ(parameters_.options.sftp_command, "sftpcmd"); +} + +TEST_F(ParamsTest, ParseSucceedsWithSftpEnvVarOverwritingScp) { + EXPECT_OK(path::SetEnv(kSftpCommandEnvVar, "sftpcmd")); + const char* argv[] = {"cdc_rsync.exe", kSrc, kUserHostDst, + "--scp-command=C:\\scp.exe foo", NULL}; + EXPECT_TRUE(Parse(static_cast(std::size(argv)) - 1, argv, ¶meters_)); + EXPECT_EQ(parameters_.options.sftp_command, "sftpcmd"); +} + TEST_F(ParamsTest, ParseSucceedsWithNoSshCommand) { const char* argv[] = {"cdc_rsync.exe", kSrc, kUserHostDst, "--ssh-command=", NULL}; @@ -178,12 +220,12 @@ TEST_F(ParamsTest, ParseSucceedsWithNoSshCommand) { ExpectError(NeedsValueError("ssh-command")); } -TEST_F(ParamsTest, ParseSucceedsWithNoScpCommand) { - const char* argv[] = {"cdc_rsync.exe", kSrc, kUserHostDst, "--scp-command", +TEST_F(ParamsTest, ParseSucceedsWithNoSftpCommand) { + const char* argv[] = {"cdc_rsync.exe", kSrc, kUserHostDst, "--sftp-command", NULL}; EXPECT_FALSE( Parse(static_cast(std::size(argv)) - 1, argv, ¶meters_)); - ExpectError(NeedsValueError("scp-command")); + ExpectError(NeedsValueError("sftp-command")); } TEST_F(ParamsTest, ParseFailsOnNoUserHost) { diff --git a/cdc_rsync/server_arch.cc b/cdc_rsync/server_arch.cc index d0fb0a5..26070a7 100644 --- a/cdc_rsync/server_arch.cc +++ b/cdc_rsync/server_arch.cc @@ -18,8 +18,10 @@ #include "absl/strings/match.h" #include "absl/strings/str_format.h" +#include "absl/strings/str_split.h" #include "common/path.h" #include "common/remote_util.h" +#include "common/util.h" namespace cdc_ft { @@ -72,19 +74,13 @@ std::string ServerArch::CdcServerFilename() const { } } -std::string ServerArch::RemoteToolsBinDir(UseCase use_case) const { +std::string ServerArch::RemoteToolsBinDir() const { switch (type_) { case Type::kWindows: { - // TODO(ljusten): Unfortunately, scp doesn't seem to support shell var - // expansion, so %AppData% can't be used. This relies on scp copying - // files relative to %UserProfile% and %AppData% mapping to - // "AppData\\Roaming" relative to that. - std::string app_data = - use_case == UseCase::kScp ? "AppData\\Roaming" : "$env:appdata"; - return app_data + "\\cdc-file-transfer\\bin\\"; + return "AppData\\Roaming\\cdc-file-transfer\\bin\\"; } case Type::kLinux: - return "~/.cache/cdc-file-transfer/bin/"; + return ".cache/cdc-file-transfer/bin/"; default: assert(!kErrorArchTypeUnhandled); return std::string(); @@ -93,11 +89,7 @@ std::string ServerArch::RemoteToolsBinDir(UseCase use_case) const { std::string ServerArch::GetStartServerCommand(int exit_code_not_found, const std::string& args) const { - std::string server_dir = RemoteToolsBinDir(UseCase::kSsh); - std::string server_path = - RemoteUtil::QuoteForSsh(server_dir + CdcServerFilename()); - path::EnsureDoesNotEndWithPathSeparator(&server_dir); - server_dir = RemoteUtil::QuoteForSsh(server_dir); + std::string server_path = RemoteToolsBinDir() + CdcServerFilename(); switch (type_) { case Type::kWindows: @@ -106,53 +98,50 @@ std::string ServerArch::GetStartServerCommand(int exit_code_not_found, // a minor issue and means we display "Deploying server..." instead of // "Server not deployed. Deploying..."; return RemoteUtil::QuoteForWindows( - absl::StrFormat("Set-StrictMode -Version 2; " + absl::StrFormat("powershell -Command \" " + "Set-StrictMode -Version 2; " "$ErrorActionPreference = 'Stop'; " - "$server_dir = %s; " - "$server_path = %s; " - "$u=New-Item $server_dir -ItemType Directory -Force; " - "if (-not (Test-Path -Path $server_path)) { " + "if (-not (Test-Path -Path '%s')) { " " exit %i; " "} " - "& $server_path %s", - server_dir, server_path, exit_code_not_found, args)); + "%s %s " + "\"", + server_path, exit_code_not_found, server_path, args)); case Type::kLinux: - return absl::StrFormat( - "mkdir -p %s; if [ ! -f %s ]; then exit %i; fi; %s %s", server_dir, - server_path, exit_code_not_found, server_path, args); + return absl::StrFormat("if [ ! -f %s ]; then exit %i; fi; %s %s", + server_path, exit_code_not_found, server_path, + args); default: assert(!kErrorArchTypeUnhandled); return std::string(); } } -std::string ServerArch::GetDeployReplaceCommand( - const std::string& old_server_path, - const std::string& new_server_path) const { - const std::string old_path = RemoteUtil::QuoteForSsh(old_server_path); - const std::string new_path = RemoteUtil::QuoteForSsh(new_server_path); +std::string ServerArch::GetDeploySftpCommands() const { + std::string commands; - switch (type_) { - case Type::kWindows: - return RemoteUtil::QuoteForWindows(absl::StrFormat( - "Set-StrictMode -Version 2; " - "$ErrorActionPreference = 'Stop'; " - "$old_path = %s; " - "$new_path = %s; " - "if (Test-Path -Path $old_path) { " - " Get-Item -Path $old_path | " - " Set-ItemProperty -Name IsReadOnly -Value $false; " - "} " - "Move-Item -Path $new_path -Destination $old_path -Force", - old_path, new_path)); - case Type::kLinux: - return absl::StrFormat( - "([ ! -f %s ] || chmod u+w %s) && chmod a+x %s && mv %s %s", old_path, - old_path, new_path, new_path, old_path); - default: - assert(!kErrorArchTypeUnhandled); - return std::string(); + // Create the remote tools bin dir if it doesn't exist yet. + // This assumes that sftp's remote startup directory is the home directory. + const std::string server_dir = path::ToUnix(RemoteToolsBinDir()); + std::vector dir_parts = + absl::StrSplit(server_dir, '/', absl::SkipEmpty()); + for (const std::string& dir : dir_parts) { + // Use -mkdir to ignore errors if the directory already exists. + commands += absl::StrFormat("-mkdir %s\ncd %s\n", dir, dir); } + + // Copy the server binary to a temp location. This assumes that sftp's local + // startup directory is cdc_rsync's exe dir. + const std::string server_file = CdcServerFilename(); + const std::string server_temp_file = server_file + Util::GenerateUniqueId(); + commands += absl::StrFormat("put %s %s\n", server_file, server_temp_file); + + // Restore permissions in case they changed and propagate temp file. + commands += absl::StrFormat("-chmod 755 %s\n", server_file); + commands += absl::StrFormat("chmod 755 %s\n", server_temp_file); + commands += absl::StrFormat("rename %s %s\n", server_temp_file, server_file); + + return commands; } } // namespace cdc_ft diff --git a/cdc_rsync/server_arch.h b/cdc_rsync/server_arch.h index 7be2ca0..40f9da0 100644 --- a/cdc_rsync/server_arch.h +++ b/cdc_rsync/server_arch.h @@ -29,8 +29,6 @@ class ServerArch { kWindows = 1, }; - enum class UseCase { kSsh = 0, kScp = 1 }; - // Detects the architecture type based on the destination path, e.g. path // starting with C: indicate Windows. static Type Detect(const std::string& destination); @@ -42,16 +40,10 @@ class ServerArch { std::string CdcServerFilename() const; // Returns the arch-specific directory where cdc_rsync_server is deployed. - // On Windows, |use_case| determines what type of env variables to use: - // - kSsh uses $env:appdata and works for ssh commands. - // - kScp uses AppData\\Roaming and works for scp commands. - // On Linux, this flag is ignored. - std::string RemoteToolsBinDir(UseCase use_case) const; + std::string RemoteToolsBinDir() const; // Returns an arch-specific SSH shell command that gets invoked in order to // start cdc_rsync_server. The command - // - creates RemoteToolsBinDir() if it does not exist (so that the server can - // be deployed there subsequently; scp can't create directories), // - returns |exit_code_not_found| if cdc_rsync_server does not exist (to // prevent the confusing bash output message // "bash: .../cdc_rsync_server: No such file or directory"), and @@ -59,13 +51,15 @@ class ServerArch { std::string GetStartServerCommand(int exit_code_not_found, const std::string& args) const; - // Returns an arch-specific SSH shell command that gets invoked after - // cdc_rsync_server has been copied to a temp location. The command - // - makes the old cdc_rsync_server writable (if it exists), - // - makes the new cdc_rsync_server executable (Linux only) and - // - replaces the old cdc_rsync_server by the new one. - std::string GetDeployReplaceCommand(const std::string& old_server_path, - const std::string& new_server_path) const; + // Returns an arch-specific SFTP command sequence that deploys the server + // component on the target gets invoked after + // cdc_rsync_server has been copied to a temp location. The commands + // - create the cdc-file-transfer/bin folder if it doesn't exist yet, + // - make the old cdc_rsync_server writable if it exists, + // - copy cdc_rsync_server to a temp location, + // - make the new cdc_rsync_server executable (Linux only) and + // - replaces the existing cdc_rsync_server by the temp one. + std::string GetDeploySftpCommands() const; private: Type type_; diff --git a/cdc_rsync/server_arch_test.cc b/cdc_rsync/server_arch_test.cc index 0cb6f5b..6518082 100644 --- a/cdc_rsync/server_arch_test.cc +++ b/cdc_rsync/server_arch_test.cc @@ -60,47 +60,29 @@ TEST(ServerArchTest, CdcServerFilename) { } TEST(ServerArchTest, RemoteToolsBinDir) { - const std::string linux_ssh_dir = - ServerArch(kLinux).RemoteToolsBinDir(ServerArch::UseCase::kSsh); - EXPECT_TRUE(absl::StrContains(linux_ssh_dir, "/")); + const std::string linux_dir = ServerArch(kLinux).RemoteToolsBinDir(); + EXPECT_TRUE(absl::StrContains(linux_dir, ".cache/")); - const std::string linux_scp_dir = - ServerArch(kLinux).RemoteToolsBinDir(ServerArch::UseCase::kScp); - EXPECT_EQ(linux_ssh_dir, linux_scp_dir); - - const std::string win_ssh_dir = - ServerArch(kWindows).RemoteToolsBinDir(ServerArch::UseCase::kSsh); - EXPECT_TRUE(absl::StrContains(win_ssh_dir, "\\")); - EXPECT_TRUE(absl::StrContains(win_ssh_dir, "$env:appdata")); - - std::string win_scp_dir = - ServerArch(kWindows).RemoteToolsBinDir(ServerArch::UseCase::kScp); - EXPECT_TRUE(absl::StrContains(win_scp_dir, "\\")); - EXPECT_TRUE(absl::StrContains(win_scp_dir, "AppData\\Roaming")); + std::string win_dir = ServerArch(kWindows).RemoteToolsBinDir(); + EXPECT_TRUE(absl::StrContains(win_dir, "AppData\\Roaming\\")); } TEST(ServerArchTest, GetStartServerCommand) { std::string cmd = ServerArch(kWindows).GetStartServerCommand(123, "foo bar"); EXPECT_TRUE(absl::StrContains(cmd, "123")); - EXPECT_TRUE(absl::StrContains(cmd, "foo bar")); - EXPECT_TRUE(absl::StrContains(cmd, "New-Item ")); + EXPECT_TRUE(absl::StrContains(cmd, "cdc_rsync_server.exe foo bar")); cmd = ServerArch(kLinux).GetStartServerCommand(123, "foo bar"); EXPECT_TRUE(absl::StrContains(cmd, "123")); - EXPECT_TRUE(absl::StrContains(cmd, "foo bar")); - EXPECT_TRUE(absl::StrContains(cmd, "mkdir -p")); + EXPECT_TRUE(absl::StrContains(cmd, "cdc_rsync_server foo bar")); } TEST(ServerArchTest, GetDeployReplaceCommand) { - std::string cmd = ServerArch(kWindows).GetDeployReplaceCommand("aaa", "bbb"); - EXPECT_TRUE(absl::StrContains(cmd, "aaa")); - EXPECT_TRUE(absl::StrContains(cmd, "bbb")); - EXPECT_TRUE(absl::StrContains(cmd, "Move-Item ")); + std::string cmd = ServerArch(kWindows).GetDeploySftpCommands(); + EXPECT_TRUE(absl::StrContains(cmd, "cdc_rsync_server.exe")); - cmd = ServerArch(kLinux).GetDeployReplaceCommand("aaa", "bbb"); - EXPECT_TRUE(absl::StrContains(cmd, "aaa")); - EXPECT_TRUE(absl::StrContains(cmd, "bbb")); - EXPECT_TRUE(absl::StrContains(cmd, "mv ")); + cmd = ServerArch(kLinux).GetDeploySftpCommands(); + EXPECT_TRUE(absl::StrContains(cmd, "cdc_rsync_server")); } } // namespace diff --git a/cdc_stream/asset_stream_config.cc b/cdc_stream/asset_stream_config.cc index 41995df..bb62ea8 100644 --- a/cdc_stream/asset_stream_config.cc +++ b/cdc_stream/asset_stream_config.cc @@ -157,9 +157,9 @@ void AssetStreamConfig::RegisterCommandLineFlags(lyra::command& cmd, "connection to the host. See also --dev-src-dir.")); cmd.add_argument( - lyra::opt(dev_target_.scp_command, "cmd") - .name("--dev-scp-command") - .help("Scp command and extra flags to use for the " + lyra::opt(dev_target_.sftp_command, "cmd") + .name("--dev-sftp-command") + .help("Sftp command and extra flags to use for the " "connection to the host. See also --dev-src-dir.")); cmd.add_argument( @@ -258,7 +258,7 @@ std::string AssetStreamConfig::ToString() { ss << "dev-user-host = " << dev_target_.user_host << std::endl; ss << "dev-ssh-command = " << dev_target_.ssh_command << std::endl; - ss << "dev-scp-command = " << dev_target_.scp_command + ss << "dev-sftp-command = " << dev_target_.sftp_command << std::endl; ss << "dev-mount-dir = " << dev_target_.mount_dir << std::endl; return ss.str(); diff --git a/cdc_stream/cdc_fuse_manager.cc b/cdc_stream/cdc_fuse_manager.cc index 28c4c0f..5e411c9 100644 --- a/cdc_stream/cdc_fuse_manager.cc +++ b/cdc_stream/cdc_fuse_manager.cc @@ -16,6 +16,7 @@ #include "absl/strings/match.h" #include "absl/strings/str_format.h" +#include "absl/strings/str_split.h" #include "cdc_fuse_fs/constants.h" #include "common/gamelet_component.h" #include "common/log.h" @@ -30,7 +31,7 @@ constexpr char kExeFilename[] = "cdc_stream.exe"; constexpr char kFuseFilename[] = "cdc_fuse_fs"; constexpr char kLibFuseFilename[] = "libfuse.so"; constexpr char kFuseStdoutPrefix[] = "cdc_fuse_fs_stdout"; -constexpr char kRemoteToolsBinDir[] = "~/.cache/cdc-file-transfer/bin/"; +constexpr char kRemoteToolsBinDir[] = ".cache/cdc-file-transfer/bin/"; // Cache directory on the gamelet to store data chunks. constexpr char kCacheDir[] = "~/.cache/cdc-file-transfer/chunks"; @@ -54,23 +55,25 @@ absl::Status CdcFuseManager::Deploy() { std::string exe_dir; RETURN_IF_ERROR(path::GetExeDir(&exe_dir), "Failed to get exe directory"); - // Set the cwd to the exe dir and pass the filenames to scp. Otherwise, some - // scp implementations can get confused and create the wrong remote filenames. - path::SetCwd(exe_dir); + // Create the remote tools bin dir if it doesn't exist yet. + // This assumes that sftp's remote startup directory is the home directory. + std::vector dir_parts = + absl::StrSplit(kRemoteToolsBinDir, '/', absl::SkipEmpty()); + std::string sftp_commands; + for (const std::string& dir : dir_parts) { + // Use -mkdir to ignore errors if the directory already exists. + sftp_commands += absl::StrFormat("-mkdir %s\ncd %s\n", dir, dir); + } - // Copy FUSE to the gamelet. - LOG_DEBUG("Copying FUSE"); - RETURN_IF_ERROR(remote_util_->Scp({kFuseFilename, kLibFuseFilename}, - kRemoteToolsBinDir, /*compress=*/false), - "Failed to copy FUSE to gamelet"); - LOG_DEBUG("Copying FUSE succeeded"); + sftp_commands += absl::StrFormat("put %s\n", kFuseFilename); + sftp_commands += absl::StrFormat("put %s\n", kLibFuseFilename); + sftp_commands += absl::StrFormat("chmod 755 %s\n", kFuseFilename); - // Make FUSE executable. Note that sync does it automatically. - LOG_DEBUG("Making FUSE executable"); - std::string remotePath = path::JoinUnix(kRemoteToolsBinDir, kFuseFilename); - RETURN_IF_ERROR(remote_util_->Chmod("a+x", remotePath), - "Failed to set executable flag on FUSE"); - LOG_DEBUG("Making FUSE succeeded"); + LOG_DEBUG("Deploying FUSE"); + RETURN_IF_ERROR( + remote_util_->Sftp(sftp_commands, exe_dir, /*compress=*/false), + "Failed to deploy FUSE"); + LOG_DEBUG("Deploying FUSE succeeded"); return absl::OkStatus(); } @@ -104,14 +107,13 @@ absl::Status CdcFuseManager::Start(const std::string& mount_dir, // Build the remote command. std::string remotePath = path::JoinUnix(kRemoteToolsBinDir, kFuseFilename); std::string remote_command = absl::StrFormat( - "mkdir -p %s; LD_LIBRARY_PATH=%s %s " + "LD_LIBRARY_PATH=%s %s " "--instance=%s " "--components=%s --port=%i --cache_dir=%s " "--verbosity=%i --cleanup_timeout=%i --access_idle_timeout=%i --stats=%i " "--check=%i --cache_capacity=%u -- -o allow_root -o ro -o nonempty -o " "auto_unmount %s%s%s", - kRemoteToolsBinDir, kRemoteToolsBinDir, remotePath, - RemoteUtil::QuoteForSsh(instance_), + kRemoteToolsBinDir, remotePath, RemoteUtil::QuoteForSsh(instance_), RemoteUtil::QuoteForSsh(component_args), remote_port, kCacheDir, verbosity, cleanup_timeout_sec, access_idle_timeout_sec, enable_stats, check, cache_capacity, debug ? "-d " : "", singlethreaded ? "-s " : "", diff --git a/cdc_stream/local_assets_stream_manager_client.cc b/cdc_stream/local_assets_stream_manager_client.cc index 181cb35..e816d51 100644 --- a/cdc_stream/local_assets_stream_manager_client.cc +++ b/cdc_stream/local_assets_stream_manager_client.cc @@ -38,13 +38,13 @@ LocalAssetsStreamManagerClient::~LocalAssetsStreamManagerClient() = default; absl::Status LocalAssetsStreamManagerClient::StartSession( 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) { + const std::string& sftp_command) { StartSessionRequest request; request.set_workstation_directory(src_dir); request.set_user_host(user_host); request.set_mount_dir(mount_dir); request.set_ssh_command(ssh_command); - request.set_scp_command(scp_command); + request.set_sftp_command(sftp_command); grpc::ClientContext context; StartSessionResponse response; diff --git a/cdc_stream/local_assets_stream_manager_client.h b/cdc_stream/local_assets_stream_manager_client.h index 15dfdeb..b92fab1 100644 --- a/cdc_stream/local_assets_stream_manager_client.h +++ b/cdc_stream/local_assets_stream_manager_client.h @@ -43,12 +43,12 @@ class LocalAssetsStreamManagerClient { // |user_host| is the Linux host, formatted as [user@: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. + // |sftp_command| is the sftp command and extra arguments to use. absl::Status StartSession(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); + const std::string& sftp_command); // Stops the streaming session to the Linux target |user_host|:|mount_dir|. // |user_host| is the Linux host, formatted as [user@:host]. diff --git a/cdc_stream/local_assets_stream_manager_service_impl.cc b/cdc_stream/local_assets_stream_manager_service_impl.cc index c4e033b..ec55f37 100644 --- a/cdc_stream/local_assets_stream_manager_service_impl.cc +++ b/cdc_stream/local_assets_stream_manager_service_impl.cc @@ -208,7 +208,7 @@ LocalAssetsStreamManagerServiceImpl::GetTargetForStadia( SessionTarget target; target.mount_dir = request.mount_dir(); target.ssh_command = request.ssh_command(); - target.scp_command = request.scp_command(); + target.sftp_command = request.sftp_command(); // Parse instance/project/org id. if (!ParseInstanceName(request.gamelet_name(), instance_id, project_id, @@ -223,7 +223,7 @@ LocalAssetsStreamManagerServiceImpl::GetTargetForStadia( InitSsh(*instance_id, *project_id, *organization_id)); target.user_host = "cloudcast@" + instance_ip; - // Note: Port must be set with ssh_command (-p) and scp_command (-P). + // Note: Port must be set with ssh_command (-p) and sftp_command (-P). return target; } @@ -233,7 +233,7 @@ SessionTarget LocalAssetsStreamManagerServiceImpl::GetTarget( target.user_host = request.user_host(); target.mount_dir = request.mount_dir(); target.ssh_command = request.ssh_command(); - target.scp_command = request.scp_command(); + target.sftp_command = request.sftp_command(); *instance_id = absl::StrCat(target.user_host, ":", target.mount_dir); return target; diff --git a/cdc_stream/session.cc b/cdc_stream/session.cc index e6c0cd9..e1237c4 100644 --- a/cdc_stream/session.cc +++ b/cdc_stream/session.cc @@ -55,8 +55,8 @@ Session::Session(std::string instance_id, const SessionTarget& target, if (!target.ssh_command.empty()) { remote_util_.SetSshCommand(target.ssh_command); } - if (!target.scp_command.empty()) { - remote_util_.SetScpCommand(target.scp_command); + if (!target.sftp_command.empty()) { + remote_util_.SetSftpCommand(target.sftp_command); } } diff --git a/cdc_stream/session.h b/cdc_stream/session.h index 14e6695..5db3733 100644 --- a/cdc_stream/session.h +++ b/cdc_stream/session.h @@ -38,8 +38,8 @@ struct SessionTarget { std::string user_host; // 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. - std::string scp_command; + // Sftp command to use to copy files to the remote target. + std::string sftp_command; // Directory on the remote target where to mount the streamed directory. std::string mount_dir; }; diff --git a/cdc_stream/start_command.cc b/cdc_stream/start_command.cc index 0fdd19c..c5559d2 100644 --- a/cdc_stream/start_command.cc +++ b/cdc_stream/start_command.cc @@ -22,6 +22,7 @@ #include "common/log.h" #include "common/path.h" #include "common/process.h" +#include "common/remote_util.h" #include "common/status_macros.h" #include "common/stopwatch.h" #include "common/util.h" @@ -73,17 +74,28 @@ void StartCommand::RegisterCommandLineFlags(lyra::command& cmd) { path::GetEnv("CDC_SSH_COMMAND", &ssh_command_).IgnoreError(); cmd.add_argument( - lyra::opt(ssh_command_, "ssh_command") + lyra::opt(ssh_command_, "cmd") .name("--ssh-command") .help("Path and arguments of ssh command to use, e.g. " "\"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(); + path::GetEnv("CDC_SFTP_COMMAND", &sftp_command_).IgnoreError(); cmd.add_argument( - lyra::opt(scp_command_, "scp_command") + lyra::opt(sftp_command_, "cmd") + .name("--sftp-command") + .help( + "Path and arguments of sftp command to use, e.g. " + "\"C:\\path\\to\\sftp.exe -F config_file -P 1234\". Can also be " + "specified by the CDC_SFTP_COMMAND environment variable.")); + + path::GetEnv("CDC_SCP_COMMAND", &deprecated_scp_command_).IgnoreError(); + cmd.add_argument( + lyra::opt(deprecated_scp_command_, "cmd") .name("--scp-command") - .help("Path and arguments of scp command to use, e.g. " + .help("[Deprecated, use --sftp-command] Path and arguments of scp " + "command to " + "use, e.g. " "\"C:\\path\\to\\scp.exe -F config_file -P 1234\". Can also be " "specified by the CDC_SCP_COMMAND environment variable.")); @@ -106,9 +118,26 @@ absl::Status StartCommand::Run() { RETURN_IF_ERROR(LocalAssetsStreamManagerClient::ParseUserHostDir( user_host_dir_, &user_host, &mount_dir)); + // Backwards compatibility after switching from scp to sftp. + if (sftp_command_.empty() && !deprecated_scp_command_.empty()) { + LOG_WARNING( + "The CDC_SCP_COMMAND environment variable and the --scp-command flag " + "are deprecated. Please set CDC_SFTP_COMMAND or --sftp-command " + "instead."); + + sftp_command_ = RemoteUtil::ScpToSftpCommand(deprecated_scp_command_); + if (!sftp_command_.empty()) { + LOG_WARNING("Converted scp command '%s' to sftp command '%s'.", + deprecated_scp_command_, sftp_command_); + } else { + LOG_WARNING("Failed to convert scp command '%s' to sftp command.", + deprecated_scp_command_); + } + } + LocalAssetsStreamManagerClient client(CreateChannel(service_port_)); absl::Status status = client.StartSession(full_src_dir, user_host, mount_dir, - ssh_command_, scp_command_); + ssh_command_, sftp_command_); if (absl::IsUnavailable(status)) { LOG_DEBUG("StartSession status: %s", status.ToString()); @@ -122,7 +151,7 @@ absl::Status StartCommand::Run() { // state. LocalAssetsStreamManagerClient new_client(CreateChannel(service_port_)); status = new_client.StartSession(full_src_dir, user_host, mount_dir, - ssh_command_, scp_command_); + ssh_command_, sftp_command_); } } diff --git a/cdc_stream/start_command.h b/cdc_stream/start_command.h index a4eb01f..2ed4070 100644 --- a/cdc_stream/start_command.h +++ b/cdc_stream/start_command.h @@ -44,9 +44,11 @@ class StartCommand : public BaseCommand { int verbosity_ = 0; uint16_t service_port_ = 0; std::string ssh_command_; - std::string scp_command_; + std::string sftp_command_; std::string src_dir_; std::string user_host_dir_; + + std::string deprecated_scp_command_; }; } // namespace cdc_ft diff --git a/cdc_stream/start_service_command.cc b/cdc_stream/start_service_command.cc index 5035143..cd54369 100644 --- a/cdc_stream/start_service_command.cc +++ b/cdc_stream/start_service_command.cc @@ -141,7 +141,7 @@ absl::Status StartServiceCommand::RunService() { request.set_user_host(cfg_.dev_target().user_host); request.set_mount_dir(cfg_.dev_target().mount_dir); request.set_ssh_command(cfg_.dev_target().ssh_command); - request.set_scp_command(cfg_.dev_target().scp_command); + request.set_sftp_command(cfg_.dev_target().sftp_command); localassetsstreammanager::StartSessionResponse response; RETURN_ABSL_IF_ERROR( session_service.StartSession(nullptr, &request, &response)); diff --git a/proto/local_assets_stream_manager.proto b/proto/local_assets_stream_manager.proto index 4e4b55f..a23a431 100644 --- a/proto/local_assets_stream_manager.proto +++ b/proto/local_assets_stream_manager.proto @@ -48,9 +48,9 @@ message StartSessionRequest { // SSH command to connect to the remote instance. // Optional, falls back to searching ssh. string ssh_command = 10; - // SCP command to copy files to the remote instance. - // Optional, falls back to searching scp. - string scp_command = 11; + // SFTP command to copy files to the remote instance. + // Optional, falls back to searching sftp. + string sftp_command = 11; reserved 1, 3, 4, 9; }