From 0c8ca6f63bdbe18a4fb3c685ed997716a87c7727 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Thu, 21 Apr 2022 14:06:03 -0300 Subject: [PATCH 01/48] [utils] Execute many commands through SSH. --- include/multipass/ssh/ssh_client.h | 2 ++ src/ssh/ssh_client.cpp | 36 +++++++++++++++++++++++------- 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/include/multipass/ssh/ssh_client.h b/include/multipass/ssh/ssh_client.h index 238f113a03..c8280c06e7 100644 --- a/include/multipass/ssh/ssh_client.h +++ b/include/multipass/ssh/ssh_client.h @@ -43,10 +43,12 @@ class SSHClient SSHClient(SSHSessionUPtr ssh_session, ConsoleCreator console_creator); int exec(const std::vector& args); + int exec(const std::vector>& argss); void connect(); private: void handle_ssh_events(); + int exec_string(const std::string& cmd_line); SSHSessionUPtr ssh_session; ChannelUPtr channel; diff --git a/src/ssh/ssh_client.cpp b/src/ssh/ssh_client.cpp index bdd4965d2e..1a5612b0e5 100644 --- a/src/ssh/ssh_client.cpp +++ b/src/ssh/ssh_client.cpp @@ -51,20 +51,27 @@ mp::SSHClient::SSHClient(SSHSessionUPtr ssh_session, ConsoleCreator console_crea void mp::SSHClient::connect() { - exec({}); + exec(std::vector{}); } int mp::SSHClient::exec(const std::vector& args) { - if (args.empty()) - SSH::throw_on_error(channel, *ssh_session, "[ssh client] shell request failed", ssh_channel_request_shell); - else - SSH::throw_on_error(channel, *ssh_session, "[ssh client] exec request failed", ssh_channel_request_exec, - utils::to_cmd(args, mp::utils::QuoteType::quote_every_arg).c_str()); + return exec_string(utils::to_cmd(args, mp::utils::QuoteType::quote_every_arg)); +} - handle_ssh_events(); +int mp::SSHClient::exec(const std::vector>& argss) +{ + std::string cmd_line; - return ssh_channel_get_exit_status(channel.get()); + if (argss.size()) + { + auto args_it = argss.begin(); + cmd_line = utils::to_cmd(*args_it++, mp::utils::QuoteType::quote_every_arg); + for (; args_it != argss.end(); ++args_it) + cmd_line += ";" + utils::to_cmd(*args_it, mp::utils::QuoteType::quote_every_arg); + } + + return exec_string(cmd_line); } void mp::SSHClient::handle_ssh_events() @@ -99,3 +106,16 @@ void mp::SSHClient::handle_ssh_events() ssh_event_remove_connector(event.get(), connector_out.get()); ssh_event_remove_connector(event.get(), connector_err.get()); } + +int mp::SSHClient::exec_string(const std::string& cmd_line) +{ + if (cmd_line.empty()) + SSH::throw_on_error(channel, *ssh_session, "[ssh client] shell request failed", ssh_channel_request_shell); + else + SSH::throw_on_error(channel, *ssh_session, "[ssh client] exec request failed", ssh_channel_request_exec, + cmd_line.c_str()); + + handle_ssh_events(); + + return ssh_channel_get_exit_status(channel.get()); +} From e37f93c27cb7def744b56c049f2c97715cc8a24e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Fri, 22 Apr 2022 10:35:01 -0300 Subject: [PATCH 02/48] [cli] Add option --working-directory to `exec`. This option also works for executing aliases. --- src/client/cli/cmd/exec.cpp | 30 ++++++++++++++++++++++++++---- src/client/cli/cmd/exec.h | 4 +++- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/src/client/cli/cmd/exec.cpp b/src/client/cli/cmd/exec.cpp index cbd015c7c8..9e864f35cf 100644 --- a/src/client/cli/cmd/exec.cpp +++ b/src/client/cli/cmd/exec.cpp @@ -24,6 +24,11 @@ namespace mp = multipass; namespace cmd = multipass::cmd; +namespace +{ +const QString work_dir_option_name{"working-directory"}; +} + mp::ReturnCode cmd::Exec::run(mp::ArgParser* parser) { auto ret = parse_args(parser); @@ -36,7 +41,13 @@ mp::ReturnCode cmd::Exec::run(mp::ArgParser* parser) for (int i = 1; i < parser->positionalArguments().size(); ++i) args.push_back(parser->positionalArguments().at(i).toStdString()); - auto on_success = [this, &args](mp::SSHInfoReply& reply) { return exec_success(reply, args, term); }; + std::optional work_dir; + if (parser->isSet(work_dir_option_name)) + work_dir = parser->value(work_dir_option_name).toStdString(); + + auto on_success = [this, &args, &work_dir](mp::SSHInfoReply& reply) { + return exec_success(reply, work_dir, args, term); + }; auto on_failure = [this, parser](grpc::Status& status) { if (status.error_code() == grpc::StatusCode::ABORTED) @@ -69,8 +80,8 @@ QString cmd::Exec::description() const return QStringLiteral("Run a command on an instance"); } -mp::ReturnCode cmd::Exec::exec_success(const mp::SSHInfoReply& reply, const std::vector& args, - mp::Terminal* term) +mp::ReturnCode cmd::Exec::exec_success(const mp::SSHInfoReply& reply, const mp::optional& dir, + const std::vector& args, mp::Terminal* term) { // TODO: mainly for testing - need a better way to test parsing if (reply.ssh_info().empty()) @@ -86,7 +97,14 @@ mp::ReturnCode cmd::Exec::exec_success(const mp::SSHInfoReply& reply, const std: { auto console_creator = [&term](auto channel) { return Console::make_console(channel, term); }; mp::SSHClient ssh_client{host, port, username, priv_key_blob, console_creator}; - return static_cast(ssh_client.exec(args)); + + std::vector> all_args; + if (dir) + all_args = {{"cd", *dir}, {args}}; + else + all_args = {{args}}; + + return static_cast(ssh_client.exec(all_args)); } catch (const std::exception& e) { @@ -100,6 +118,10 @@ mp::ParseCode cmd::Exec::parse_args(mp::ArgParser* parser) parser->addPositionalArgument("name", "Name of instance to execute the command on", ""); parser->addPositionalArgument("command", "Command to execute on the instance", "[--] "); + QCommandLineOption workDirOption({"w", work_dir_option_name}, "Change to before execution", "dir"); + + parser->addOptions({workDirOption}); + auto status = parser->commandParse(this); if (status != ParseCode::Ok) diff --git a/src/client/cli/cmd/exec.h b/src/client/cli/cmd/exec.h index 0fd75d91d0..f7dd590b46 100644 --- a/src/client/cli/cmd/exec.h +++ b/src/client/cli/cmd/exec.h @@ -22,6 +22,7 @@ #include #include +#include namespace multipass { @@ -41,7 +42,8 @@ class Exec final : public Command QString short_help() const override; QString description() const override; - static ReturnCode exec_success(const SSHInfoReply& reply, const std::vector& args, Terminal* term); + static ReturnCode exec_success(const SSHInfoReply& reply, const multipass::optional& dir, + const std::vector& args, Terminal* term); private: SSHInfoRequest request; From 5eb1eed1018fd132e7b75e1054cd1027d293ed67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Fri, 22 Apr 2022 10:41:31 -0300 Subject: [PATCH 03/48] [bash] Add `exec --working-dir` to completions. --- completions/bash/multipass | 3 +++ 1 file changed, 3 insertions(+) diff --git a/completions/bash/multipass b/completions/bash/multipass index cc96368b7d..a0b93312d7 100644 --- a/completions/bash/multipass +++ b/completions/bash/multipass @@ -193,6 +193,9 @@ _multipass_complete() fi case "${cmd}" in + "exec") + opts="${opts} --working-dir" + ;; "info") opts="${opts} --all --format" ;; From 13cddaf0b2b29e05805dea220e147e811bfc2861 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Fri, 22 Apr 2022 14:19:48 -0300 Subject: [PATCH 04/48] [cli] Use `-d` as alias for `--working directory`. It was `-w`, but `-d` sounds like a better shortcut, as Saviq suggested. --- src/client/cli/cmd/exec.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/cli/cmd/exec.cpp b/src/client/cli/cmd/exec.cpp index 9e864f35cf..08157d03fa 100644 --- a/src/client/cli/cmd/exec.cpp +++ b/src/client/cli/cmd/exec.cpp @@ -118,7 +118,7 @@ mp::ParseCode cmd::Exec::parse_args(mp::ArgParser* parser) parser->addPositionalArgument("name", "Name of instance to execute the command on", ""); parser->addPositionalArgument("command", "Command to execute on the instance", "[--] "); - QCommandLineOption workDirOption({"w", work_dir_option_name}, "Change to before execution", "dir"); + QCommandLineOption workDirOption({"d", work_dir_option_name}, "Change to before execution", "dir"); parser->addOptions({workDirOption}); From d1d6db0e30d1d9b6dc2802a1576859c9b02cd5ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Fri, 22 Apr 2022 14:20:49 -0300 Subject: [PATCH 05/48] [bash] Corrected `--working-directory`. --- completions/bash/multipass | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/completions/bash/multipass b/completions/bash/multipass index a0b93312d7..089ec454c8 100644 --- a/completions/bash/multipass +++ b/completions/bash/multipass @@ -194,7 +194,7 @@ _multipass_complete() case "${cmd}" in "exec") - opts="${opts} --working-dir" + opts="${opts} --working-directory" ;; "info") opts="${opts} --all --format" From 9a0280e6e05f2cae375b0406f65a8ab266eff3a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Mon, 25 Apr 2022 09:49:35 -0300 Subject: [PATCH 06/48] [utils] Use `&&` to concatenate commands. Instead of `;`. --- src/ssh/ssh_client.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ssh/ssh_client.cpp b/src/ssh/ssh_client.cpp index 1a5612b0e5..f4a3ff59bb 100644 --- a/src/ssh/ssh_client.cpp +++ b/src/ssh/ssh_client.cpp @@ -68,7 +68,7 @@ int mp::SSHClient::exec(const std::vector>& argss) auto args_it = argss.begin(); cmd_line = utils::to_cmd(*args_it++, mp::utils::QuoteType::quote_every_arg); for (; args_it != argss.end(); ++args_it) - cmd_line += ";" + utils::to_cmd(*args_it, mp::utils::QuoteType::quote_every_arg); + cmd_line += "&&" + utils::to_cmd(*args_it, mp::utils::QuoteType::quote_every_arg); } return exec_string(cmd_line); From 541c0cc1187b500b530803521639dac09471adb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Fri, 20 May 2022 13:46:28 -0300 Subject: [PATCH 07/48] [cli] Implemented automatic directory mapping. If an alias is invoked on a certain path in the host, which is mounted on the instance, the execution path in the instance is automatically changed to the mounted equivalent. --- src/client/cli/cmd/exec.cpp | 79 +++++++++++++++++++++++++++++++++---- src/client/cli/cmd/exec.h | 3 +- 2 files changed, 73 insertions(+), 9 deletions(-) diff --git a/src/client/cli/cmd/exec.cpp b/src/client/cli/cmd/exec.cpp index 08157d03fa..6cbc50a8c8 100644 --- a/src/client/cli/cmd/exec.cpp +++ b/src/client/cli/cmd/exec.cpp @@ -27,6 +27,20 @@ namespace cmd = multipass::cmd; namespace { const QString work_dir_option_name{"working-directory"}; + +bool is_dir_mounted(const QStringList& split_exec_dir, const QStringList& split_source_dir) +{ + int exec_dir_size = split_exec_dir.size(); + + if (exec_dir_size < split_source_dir.size()) + return false; + + for (int i = 0; i < exec_dir_size; ++i) + if (split_exec_dir[i] != split_source_dir[i]) + return false; + + return true; +} } mp::ReturnCode cmd::Exec::run(mp::ArgParser* parser) @@ -37,32 +51,81 @@ mp::ReturnCode cmd::Exec::run(mp::ArgParser* parser) return parser->returnCodeFrom(ret); } + std::string instance_name = ssh_info_request.instance_name(0); + std::vector args; for (int i = 1; i < parser->positionalArguments().size(); ++i) args.push_back(parser->positionalArguments().at(i).toStdString()); std::optional work_dir; if (parser->isSet(work_dir_option_name)) + { + // If the user asked for a working directory, prepend the appropriate `cd`. work_dir = parser->value(work_dir_option_name).toStdString(); + } + else + { + // If we are executing an alias and the current working directory is mounted in the instance, then prepend the + // appropriate `cd` to the command to be ran. + if (parser->executeAlias()) + { + // The host directory on which the user is executing the command. + QString clean_exec_dir = QDir::cleanPath(QDir::current().canonicalPath()); + QStringList split_exec_dir = clean_exec_dir.split('/', Qt::SkipEmptyParts, Qt::CaseSensitive); + + auto on_info_success = [this, &work_dir, &split_exec_dir](mp::InfoReply& reply) { + for (const auto& mount : reply.info(0).mount_info().mount_paths()) + { + auto source_dir = QDir(QString::fromStdString(mount.source_path())); + auto clean_source_dir = QDir::cleanPath(source_dir.canonicalPath()); + QStringList split_source_dir = clean_source_dir.split('/', Qt::SkipEmptyParts, Qt::CaseSensitive); + + // If the directory is mounted, we need to `cd` to it in the instance before executing the command. + if (is_dir_mounted(split_exec_dir, split_source_dir)) + { + for (int i = 0; i < split_source_dir.size(); ++i) + split_exec_dir.removeFirst(); + work_dir = mount.target_path() + '/' + split_exec_dir.join('/').toStdString(); + } + } + + return ReturnCode::Ok; + }; + + auto on_info_failure = [this](grpc::Status& status) { + return standard_failure_handler_for(name(), cerr, status); + }; + + info_request.set_verbosity_level(parser->verbosityLevel()); + + InstanceNames instance_names; + auto info_instance_name = instance_names.add_instance_name(); + info_instance_name->append(instance_name); + info_request.mutable_instance_names()->CopyFrom(instance_names); + + dispatch(&RpcMethod::info, info_request, on_info_success, on_info_failure); + // TODO: what to do with the returned value? + } + } auto on_success = [this, &args, &work_dir](mp::SSHInfoReply& reply) { return exec_success(reply, work_dir, args, term); }; - auto on_failure = [this, parser](grpc::Status& status) { + auto on_failure = [this, &instance_name, parser](grpc::Status& status) { if (status.error_code() == grpc::StatusCode::ABORTED) - return run_cmd_and_retry({"multipass", "start", QString::fromStdString(request.instance_name(0))}, parser, - cout, cerr); + return run_cmd_and_retry({"multipass", "start", QString::fromStdString(instance_name)}, parser, cout, cerr); else return standard_failure_handler_for(name(), cerr, status); }; - request.set_verbosity_level(parser->verbosityLevel()); - ReturnCode return_code; - while ((return_code = dispatch(&RpcMethod::ssh_info, request, on_success, on_failure)) == ReturnCode::Retry) + ssh_info_request.set_verbosity_level(parser->verbosityLevel()); + ReturnCode ssh_return_code; + while ((ssh_return_code = dispatch(&RpcMethod::ssh_info, ssh_info_request, on_success, on_failure)) == + ReturnCode::Retry) ; - return return_code; + return ssh_return_code; } std::string cmd::Exec::name() const @@ -143,7 +206,7 @@ mp::ParseCode cmd::Exec::parse_args(mp::ArgParser* parser) } else { - auto entry = request.add_instance_name(); + auto entry = ssh_info_request.add_instance_name(); entry->append(parser->positionalArguments().first().toStdString()); } diff --git a/src/client/cli/cmd/exec.h b/src/client/cli/cmd/exec.h index f7dd590b46..9ad080f2ba 100644 --- a/src/client/cli/cmd/exec.h +++ b/src/client/cli/cmd/exec.h @@ -46,7 +46,8 @@ class Exec final : public Command const std::vector& args, Terminal* term); private: - SSHInfoRequest request; + SSHInfoRequest ssh_info_request; + InfoRequest info_request; AliasDict aliases; ParseCode parse_args(ArgParser* parser); From 80938d0f556060ecab0ec4020fa71efa62ceaf86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Fri, 20 May 2022 14:41:14 -0300 Subject: [PATCH 08/48] [rpc] Add `--no-runtime-information` to `info`. This hidden parameter dramatically speeds up the daemon answer when only mounts are needed. --- src/client/cli/cmd/info.cpp | 7 +++++++ src/daemon/daemon.cpp | 2 +- src/rpc/multipass.proto | 1 + 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/client/cli/cmd/info.cpp b/src/client/cli/cmd/info.cpp index c4e6df456e..25e1488f78 100644 --- a/src/client/cli/cmd/info.cpp +++ b/src/client/cli/cmd/info.cpp @@ -63,6 +63,12 @@ mp::ParseCode cmd::Info::parse_args(mp::ArgParser* parser) QCommandLineOption all_option(all_option_name, "Display info for all instances"); parser->addOption(all_option); + QCommandLineOption noRuntimeInfoOption( + "no-runtime-information", + "Retrieve from the daemon only the information obtained without running commands on the instance"); + noRuntimeInfoOption.setFlags(QCommandLineOption::HiddenFromHelp); + parser->addOption(noRuntimeInfoOption); + QCommandLineOption formatOption( "format", "Output info in the requested format.\nValid formats are: table (default), json, csv and yaml", "format", "table"); @@ -80,6 +86,7 @@ mp::ParseCode cmd::Info::parse_args(mp::ArgParser* parser) return parse_code; request.mutable_instance_names()->CopyFrom(add_instance_names(parser)); + request.set_no_runtime_information(parser->isSet(noRuntimeInfoOption)); status = handle_format_option(parser, &chosen_formatter, cerr); diff --git a/src/daemon/daemon.cpp b/src/daemon/daemon.cpp index 2aa564f6b7..a1122d9da6 100644 --- a/src/daemon/daemon.cpp +++ b/src/daemon/daemon.cpp @@ -1281,7 +1281,7 @@ try // clang-format on } } - if (mp::utils::is_running(present_state)) + if (!request->no_runtime_information() && mp::utils::is_running(present_state)) { mp::SSHSession session{vm->ssh_hostname(), vm->ssh_port(), vm_specs.ssh_username, *config->ssh_key_provider}; diff --git a/src/rpc/multipass.proto b/src/rpc/multipass.proto index 51ccc5c9b8..7a15be66ae 100644 --- a/src/rpc/multipass.proto +++ b/src/rpc/multipass.proto @@ -150,6 +150,7 @@ message InstanceNames { message InfoRequest { InstanceNames instance_names = 1; int32 verbosity_level = 2; + bool no_runtime_information = 3; } message IdMap { From 490ea462a8b566db40247379494e227e8a1df5e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Fri, 20 May 2022 14:45:20 -0300 Subject: [PATCH 09/48] [cli] Use `info --no-runtime-information`. When asking the daemon for mounts on an instance, the runtime information is not needed. --- src/client/cli/cmd/exec.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/client/cli/cmd/exec.cpp b/src/client/cli/cmd/exec.cpp index 6cbc50a8c8..3c93fc1bdd 100644 --- a/src/client/cli/cmd/exec.cpp +++ b/src/client/cli/cmd/exec.cpp @@ -102,6 +102,7 @@ mp::ReturnCode cmd::Exec::run(mp::ArgParser* parser) auto info_instance_name = instance_names.add_instance_name(); info_instance_name->append(instance_name); info_request.mutable_instance_names()->CopyFrom(instance_names); + info_request.set_no_runtime_information(true); dispatch(&RpcMethod::info, info_request, on_info_success, on_info_failure); // TODO: what to do with the returned value? From 630086377321416eca741014d218b0a326456072 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Fri, 20 May 2022 15:05:26 -0300 Subject: [PATCH 10/48] [tests] Adapt to the new alias execution. Now, executing an alias makes an extra `info` request to the daemon. We mocked that call in the tests. --- tests/test_cli_client.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_cli_client.cpp b/tests/test_cli_client.cpp index cc9a1ac8cc..fa6899767b 100644 --- a/tests/test_cli_client.cpp +++ b/tests/test_cli_client.cpp @@ -2795,6 +2795,7 @@ TEST_F(ClientAlias, execute_existing_alias) { populate_db_file(AliasesVector{{"some_alias", {"some_instance", "some_command"}}}); + EXPECT_CALL(mock_daemon, info(_, _, _)).WillOnce(info_function); EXPECT_CALL(mock_daemon, ssh_info(_, _, _)); EXPECT_EQ(send_command({"some_alias"}), mp::ReturnCode::Ok); @@ -2815,6 +2816,7 @@ TEST_F(ClientAlias, execute_alias_with_arguments) { populate_db_file(AliasesVector{{"some_alias", {"some_instance", "some_command"}}}); + EXPECT_CALL(mock_daemon, info(_, _, _)).WillOnce(info_function); EXPECT_CALL(mock_daemon, ssh_info(_, _, _)); EXPECT_EQ(send_command({"some_alias", "some_argument"}), mp::ReturnCode::Ok); From d722e90b55c833405b4afbbf0a6ad26eeca80bf8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Fri, 20 May 2022 15:13:22 -0300 Subject: [PATCH 11/48] [cli] Fix formatting. --- src/client/cli/cmd/exec.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/cli/cmd/exec.cpp b/src/client/cli/cmd/exec.cpp index 3c93fc1bdd..0fa30401fc 100644 --- a/src/client/cli/cmd/exec.cpp +++ b/src/client/cli/cmd/exec.cpp @@ -41,7 +41,7 @@ bool is_dir_mounted(const QStringList& split_exec_dir, const QStringList& split_ return true; } -} +} // namespace mp::ReturnCode cmd::Exec::run(mp::ArgParser* parser) { From cbb2c3ae35b8d365842afd5e7754f2ed9d23b2de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Fri, 20 May 2022 19:15:26 -0300 Subject: [PATCH 12/48] [cli] Fix compilation with Clang. --- src/client/cli/cmd/exec.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/client/cli/cmd/exec.cpp b/src/client/cli/cmd/exec.cpp index 0fa30401fc..83c451cecf 100644 --- a/src/client/cli/cmd/exec.cpp +++ b/src/client/cli/cmd/exec.cpp @@ -71,14 +71,14 @@ mp::ReturnCode cmd::Exec::run(mp::ArgParser* parser) { // The host directory on which the user is executing the command. QString clean_exec_dir = QDir::cleanPath(QDir::current().canonicalPath()); - QStringList split_exec_dir = clean_exec_dir.split('/', Qt::SkipEmptyParts, Qt::CaseSensitive); + QStringList split_exec_dir = clean_exec_dir.split('/'); - auto on_info_success = [this, &work_dir, &split_exec_dir](mp::InfoReply& reply) { + auto on_info_success = [&work_dir, &split_exec_dir](mp::InfoReply& reply) { for (const auto& mount : reply.info(0).mount_info().mount_paths()) { auto source_dir = QDir(QString::fromStdString(mount.source_path())); auto clean_source_dir = QDir::cleanPath(source_dir.canonicalPath()); - QStringList split_source_dir = clean_source_dir.split('/', Qt::SkipEmptyParts, Qt::CaseSensitive); + QStringList split_source_dir = clean_source_dir.split('/'); // If the directory is mounted, we need to `cd` to it in the instance before executing the command. if (is_dir_mounted(split_exec_dir, split_source_dir)) From b004fd44ee3283542563cbb97ef186a3473238eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Mon, 23 May 2022 11:52:17 -0300 Subject: [PATCH 13/48] [util] Fix segfault. --- src/utils/utils.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/utils/utils.cpp b/src/utils/utils.cpp index c02c8aad48..f5931fee2e 100644 --- a/src/utils/utils.cpp +++ b/src/utils/utils.cpp @@ -215,6 +215,9 @@ QTemporaryFile mp::utils::create_temp_file_with_path(const QString& filename_tem std::string mp::utils::to_cmd(const std::vector& args, QuoteType quote_type) { + if (args.empty()) + return ""; + fmt::memory_buffer buf; for (auto const& arg : args) { From a81d5a9d56507a5dec606bebd0a668a4d61aebe6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Tue, 24 May 2022 11:01:24 -0300 Subject: [PATCH 14/48] [tests][util] Check to_cmd with empty input. --- tests/test_utils.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/test_utils.cpp b/tests/test_utils.cpp index 4c7a0062c4..acced609e3 100644 --- a/tests/test_utils.cpp +++ b/tests/test_utils.cpp @@ -355,6 +355,13 @@ TEST(Utils, generateScryptHashErrorThrows) mpt::match_what(StrEq("Cannot generate passphrase hash"))); } +TEST(Utils, to_cmd_returns_empty_string_on_empty_input) +{ + std::vector args{}; + auto output = mp::utils::to_cmd(args, mp::utils::QuoteType::quote_every_arg); + EXPECT_THAT(output, ::testing::StrEq("")); +} + TEST(Utils, to_cmd_output_has_no_quotes) { std::vector args{"hello", "world"}; From 0c902a2204969c9e3a714c2235e514edfb1be1ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Tue, 24 May 2022 11:41:10 -0300 Subject: [PATCH 15/48] [tests][ssh_client] Check exec multiple commands. --- tests/test_ssh_client.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/test_ssh_client.cpp b/tests/test_ssh_client.cpp index 18a3353a84..0072f16b54 100644 --- a/tests/test_ssh_client.cpp +++ b/tests/test_ssh_client.cpp @@ -76,13 +76,21 @@ TEST_F(SSHClient, standardCtorDoesNotThrow) EXPECT_NO_THROW(mp::SSHClient("a", 42, "foo", key_data, console_creator)); } -TEST_F(SSHClient, execReturnsOKNoFailure) +TEST_F(SSHClient, execSingleCommandReturnsOKNoFailure) { auto client = make_ssh_client(); EXPECT_EQ(client.exec({"foo"}), SSH_OK); } +TEST_F(SSHClient, execMultipleCommandsReturnsOKNoFailure) +{ + auto client = make_ssh_client(); + + std::vector> commands{{"ls", "-la"}, {"pwd"}}; + EXPECT_EQ(client.exec(commands), SSH_OK); +} + TEST_F(SSHClient, execReturnsErrorCodeOnFailure) { const int failure_code{127}; From 500127f4e4f54ac86108c6e8553fd32883f965a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Thu, 26 May 2022 11:43:19 -0300 Subject: [PATCH 16/48] [tests][ssh_client] Refactor key string location. --- tests/fake_key_data.h | 55 +++++++++++++++++++++++++++++++++++++++ tests/test_ssh_client.cpp | 48 ++-------------------------------- 2 files changed, 57 insertions(+), 46 deletions(-) create mode 100644 tests/fake_key_data.h diff --git a/tests/fake_key_data.h b/tests/fake_key_data.h new file mode 100644 index 0000000000..dfe7117f2f --- /dev/null +++ b/tests/fake_key_data.h @@ -0,0 +1,55 @@ +/* + * Copyright (C) 2018-2022 Canonical, Ltd. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 3. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +#ifndef MULTIPASS_FAKE_KEY_DATA_H +#define MULTIPASS_FAKE_KEY_DATA_H + +namespace multipass +{ +namespace test +{ +constexpr auto fake_key_data = "-----BEGIN RSA PRIVATE KEY-----\n" + "MIIEpAIBAAKCAQEAv3lEFtxT3kd2OrWQ8k3v1SHILNDwwm9U7awNbLDqVEresZNd\n" + "mRGmH381fO8tHpNdeQ+XEMff16FZiMKRQWx5OlHTQ33cS7X/huZ5Ge5YVKsBMmqy\n" + "vJADK7Ud38mNaKi3hqepD87labVmY1ARTNHSLDG68c5bIyquvzbawwwkM7qPTbw5\n" + "ZHOEpKehwPZ/034oPnmPV3BAbX1HySi/zrOZE/D1JW3uHvhF1yprDWhJumOVBSYB\n" + "zDloJSsFfFEEYOzkdmAwZ0q3yfMVxLiwTlP2Hhf+i8kOzjQfz29PPfNwroYJZqKT\n" + "Eg8YAqBr28ryHzHa8W+htUoZbntID2w9aDeJ2wIDAQABAoIBABpk0vf7wyve2fNZ\n" + "1/MuvyK4F2nmG2oSArkIgIk9EfAwqeX8lGhnQGkTFgJ0zdlrIvVvKrnLc5W7ziXF\n" + "/FPyafuaD+87yEQ/gEvONV9XtaFmOTID90N67pT10HpqxC1rJHFRZ0KgmIsr0ENc\n" + "ZCYcvkYNTOHMOk/ssE33d8xvPgZLMf/EvVqcgPyhJXXg0Y1po9cLPQjCBCfmigiM\n" + "U+3Hlrz8w6rtp3RUuM2jzrA+uHQGSa4fC/Wn2WT5fR2RZz7BPdJ+kHlTfFRq27oy\n" + "lsTITYDJf26ej1wmsWIV4AqznV33xSRZBK6KZjq98D6MKc28fyfZQKxnc1jWG1Xi\n" + "erLM+YECgYEA73wVxCdX/9eBXlquQ5wrj94UFOV9zlbr/oE0CYZC8SkG6fCf9/a/\n" + "lUUz25CK+kugJcOFHOmOXIjydXzDDFXEUgsf6MeX0WiLy6rvFdlq3xnQ9oUKBzCv\n" + "6gLL9s2Ozo4NMeY3rlqxAswdyGx3f5HHkB722MeUlafjXPkJ82m61GECgYEAzK2V\n" + "iX1v+b76HG9XnDd1eK0L/JINSJrFJUMD1KhY84FmXqPoBWcuoEtUPA+cvOhAzr3l\n" + "TFqKbXbY5JVx1c66uEkCMYYVPYyOVZNnEz+bGOmrK2NaYDwIySG6WhD2Zh69VIXa\n" + "kfvMzba0M26FXjWBDN3oluT6RLBHb2xdZgMHx7sCgYB1B3QziO5t7cggbbve+kAn\n" + "a+TwWT1jSgLFOipNxTiNVPk19QqXSBNTRKAU2cuwiKhYC/XOrSuOeLXTSAagzoDD\n" + "fwA25uJ/yNEX1A5F5RteruT4swa1gMtWVcuKbeUtdylnixMGtvbtYQXk3WyAAKM/\n" + "AIKsaMtpXsOyuVhthOtxwQKBgQCBvIGtzcHdd01IGtdYoNqoLGANr3IWFGxkSw8x\n" + "i6geaWY/FPvr+NRYLIdvLqI2J610nm+qrzVRX2Tpt0SZttkqGLT4OTpbci2CVtWe\n" + "INIpv2uNLAPMPiF/hA6AKoJUhqWR3uqFYCsYNfgRJbwJ1DZBtqNIikmMooQVP4YQ\n" + "NFmJIwKBgQCjxMF4SFzzRbNfiHKLL39D8RHlCPalbmX2CXaiUT4H1rq2oK3EiI+O\n" + "+SzzmxbHAjFRRuKeqhmC9+yhhHssBt6lJe71Fl3e01McjOcW9P1AZQdgYsDyCqR0\n" + "Yy460TKDO1em0N9GlXfsYgiSFJv1WmD7M/kvGpGxSERcnR4+bBd2BQ==\n" + "-----END RSA PRIVATE KEY-----\n"; +} // namespace test +} // namespace multipass + +#endif // MULTIPASS_FAKE_KEY_DATA_H diff --git a/tests/test_ssh_client.cpp b/tests/test_ssh_client.cpp index 0072f16b54..3df50da668 100644 --- a/tests/test_ssh_client.cpp +++ b/tests/test_ssh_client.cpp @@ -1,22 +1,6 @@ -/* - * Copyright (C) 2018-2022 Canonical, Ltd. - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; version 3. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see . - * - */ - #include "common.h" #include "disabling_macros.h" +#include "fake_key_data.h" #include "mock_ssh_client.h" #include "mock_ssh_test_fixture.h" #include "stub_console.h" @@ -45,35 +29,7 @@ struct SSHClient : public testing::Test TEST_F(SSHClient, standardCtorDoesNotThrow) { - constexpr auto key_data = "-----BEGIN RSA PRIVATE KEY-----\n" - "MIIEpAIBAAKCAQEAv3lEFtxT3kd2OrWQ8k3v1SHILNDwwm9U7awNbLDqVEresZNd\n" - "mRGmH381fO8tHpNdeQ+XEMff16FZiMKRQWx5OlHTQ33cS7X/huZ5Ge5YVKsBMmqy\n" - "vJADK7Ud38mNaKi3hqepD87labVmY1ARTNHSLDG68c5bIyquvzbawwwkM7qPTbw5\n" - "ZHOEpKehwPZ/034oPnmPV3BAbX1HySi/zrOZE/D1JW3uHvhF1yprDWhJumOVBSYB\n" - "zDloJSsFfFEEYOzkdmAwZ0q3yfMVxLiwTlP2Hhf+i8kOzjQfz29PPfNwroYJZqKT\n" - "Eg8YAqBr28ryHzHa8W+htUoZbntID2w9aDeJ2wIDAQABAoIBABpk0vf7wyve2fNZ\n" - "1/MuvyK4F2nmG2oSArkIgIk9EfAwqeX8lGhnQGkTFgJ0zdlrIvVvKrnLc5W7ziXF\n" - "/FPyafuaD+87yEQ/gEvONV9XtaFmOTID90N67pT10HpqxC1rJHFRZ0KgmIsr0ENc\n" - "ZCYcvkYNTOHMOk/ssE33d8xvPgZLMf/EvVqcgPyhJXXg0Y1po9cLPQjCBCfmigiM\n" - "U+3Hlrz8w6rtp3RUuM2jzrA+uHQGSa4fC/Wn2WT5fR2RZz7BPdJ+kHlTfFRq27oy\n" - "lsTITYDJf26ej1wmsWIV4AqznV33xSRZBK6KZjq98D6MKc28fyfZQKxnc1jWG1Xi\n" - "erLM+YECgYEA73wVxCdX/9eBXlquQ5wrj94UFOV9zlbr/oE0CYZC8SkG6fCf9/a/\n" - "lUUz25CK+kugJcOFHOmOXIjydXzDDFXEUgsf6MeX0WiLy6rvFdlq3xnQ9oUKBzCv\n" - "6gLL9s2Ozo4NMeY3rlqxAswdyGx3f5HHkB722MeUlafjXPkJ82m61GECgYEAzK2V\n" - "iX1v+b76HG9XnDd1eK0L/JINSJrFJUMD1KhY84FmXqPoBWcuoEtUPA+cvOhAzr3l\n" - "TFqKbXbY5JVx1c66uEkCMYYVPYyOVZNnEz+bGOmrK2NaYDwIySG6WhD2Zh69VIXa\n" - "kfvMzba0M26FXjWBDN3oluT6RLBHb2xdZgMHx7sCgYB1B3QziO5t7cggbbve+kAn\n" - "a+TwWT1jSgLFOipNxTiNVPk19QqXSBNTRKAU2cuwiKhYC/XOrSuOeLXTSAagzoDD\n" - "fwA25uJ/yNEX1A5F5RteruT4swa1gMtWVcuKbeUtdylnixMGtvbtYQXk3WyAAKM/\n" - "AIKsaMtpXsOyuVhthOtxwQKBgQCBvIGtzcHdd01IGtdYoNqoLGANr3IWFGxkSw8x\n" - "i6geaWY/FPvr+NRYLIdvLqI2J610nm+qrzVRX2Tpt0SZttkqGLT4OTpbci2CVtWe\n" - "INIpv2uNLAPMPiF/hA6AKoJUhqWR3uqFYCsYNfgRJbwJ1DZBtqNIikmMooQVP4YQ\n" - "NFmJIwKBgQCjxMF4SFzzRbNfiHKLL39D8RHlCPalbmX2CXaiUT4H1rq2oK3EiI+O\n" - "+SzzmxbHAjFRRuKeqhmC9+yhhHssBt6lJe71Fl3e01McjOcW9P1AZQdgYsDyCqR0\n" - "Yy460TKDO1em0N9GlXfsYgiSFJv1WmD7M/kvGpGxSERcnR4+bBd2BQ==\n" - "-----END RSA PRIVATE KEY-----\n"; - - EXPECT_NO_THROW(mp::SSHClient("a", 42, "foo", key_data, console_creator)); + EXPECT_NO_THROW(mp::SSHClient("a", 42, "foo", mpt::fake_key_data, console_creator)); } TEST_F(SSHClient, execSingleCommandReturnsOKNoFailure) From 67a713c0dd306deccce74b4cc7e7c6a4d1db6bb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Thu, 26 May 2022 16:32:59 -0300 Subject: [PATCH 17/48] [tests] Check SSH execution return. --- tests/test_cli_client.cpp | 82 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 76 insertions(+), 6 deletions(-) diff --git a/tests/test_cli_client.cpp b/tests/test_cli_client.cpp index fa6899767b..5814c5f1a1 100644 --- a/tests/test_cli_client.cpp +++ b/tests/test_cli_client.cpp @@ -15,9 +15,12 @@ * */ +#include "mock_ssh_test_fixture.h" + #include "common.h" #include "disabling_macros.h" #include "fake_alias_config.h" +#include "fake_key_data.h" #include "mock_cert_provider.h" #include "mock_environment_helpers.h" #include "mock_file_ops.h" @@ -30,6 +33,7 @@ #include "mock_utils.h" #include "path.h" #include "stub_cert_store.h" +#include "stub_console.h" #include "stub_terminal.h" #include @@ -56,7 +60,6 @@ using namespace testing; namespace { - struct MockDaemonRpc : public mp::DaemonRpc { using mp::DaemonRpc::DaemonRpc; // ctor @@ -122,11 +125,11 @@ struct Client : public Test void TearDown() override { - Mock::VerifyAndClearExpectations(&mock_daemon); /* We got away without this before because, being a strict mock - every call to mock_daemon had to be explicitly "expected". - Being the best match for incoming calls, each expectation - took precedence over the previous ones, preventing them from - being saturated inadvertently */ + testing::Mock::VerifyAndClearExpectations(&mock_daemon); /* We got away without this before because, being a + strict mock every call to mock_daemon had to be explicitly + "expected". Being the best match for incoming calls, each + expectation took precedence over the previous ones, + preventing them from being saturated inadvertently */ } int setup_client_and_run(const std::vector& command, mp::Terminal& term) @@ -279,6 +282,8 @@ struct Client : public Test mpt::MockSettings& mock_settings = *mock_settings_injection.first; inline static std::stringstream trash_stream; // this may have contents (that we don't care about) static constexpr char petenv_name[] = "the-petenv"; + + mpt::MockSSHTestFixture mock_ssh_test_fixture; }; struct ClientAlias : public Client, public FakeAliasConfig @@ -1140,6 +1145,71 @@ TEST_F(Client, exec_cmd_fails_on_other_absent_instance) EXPECT_THAT(send_command({"exec", instance, "--", "command"}), Eq(mp::ReturnCode::CommandFail)); } +mp::SSHInfo make_ssh_info(const std::string& host = "222.222.222.222", int port = 22, + const std::string& priv_key = mpt::fake_key_data, const std::string& username = "user") +{ + mp::SSHInfo ssh_info; + + ssh_info.set_host(host); + ssh_info.set_port(port); + ssh_info.set_priv_key_base64(priv_key); + ssh_info.set_username(username); + + return ssh_info; +} + +mp::SSHInfoReply make_fake_response(const std::string& instance_name) +{ + mp::SSHInfoReply response; + (*response.mutable_ssh_info())[instance_name] = make_ssh_info(); + + return response; +} + +struct SSHClientReturnTest : Client, WithParamInterface +{ +}; + +TEST_P(SSHClientReturnTest, execCmdWithoutDirWorks) +{ + const int failure_code{GetParam()}; + + REPLACE(ssh_channel_get_exit_status, [&failure_code](auto) { return failure_code; }); + + std::string instance_name{"instance"}; + mp::SSHInfoReply response = make_fake_response(instance_name); + + EXPECT_CALL(mock_daemon, ssh_info(_, _, _)) + .WillOnce([&response](grpc::ServerContext* context, const mp::SSHInfoRequest* request, + grpc::ServerWriter* server) { + server->Write(response); + return grpc::Status{}; + }); + + EXPECT_EQ(send_command({"exec", instance_name, "--", "cmd"}), failure_code); +} + +TEST_P(SSHClientReturnTest, execCmdWithDirWorks) +{ + const int failure_code{GetParam()}; + + REPLACE(ssh_channel_get_exit_status, [&failure_code](auto) { return failure_code; }); + + std::string instance_name{"instance"}; + mp::SSHInfoReply response = make_fake_response(instance_name); + + EXPECT_CALL(mock_daemon, ssh_info(_, _, _)) + .WillOnce([&response](grpc::ServerContext* context, const mp::SSHInfoRequest* request, + grpc::ServerWriter* server) { + server->Write(response); + return grpc::Status{}; + }); + + EXPECT_EQ(send_command({"exec", instance_name, "--working-directory", "/home/ubuntu/", "--", "cmd"}), failure_code); +} + +INSTANTIATE_TEST_SUITE_P(Client, SSHClientReturnTest, Values(0, -1, 1, 127)); + // help cli tests TEST_F(Client, help_cmd_ok_with_valid_single_arg) { From 2137ec94f4c8d33d1b09ba33465fadf75a50581d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Thu, 26 May 2022 16:40:17 -0300 Subject: [PATCH 18/48] [tests][cli] Check that --working-directory works. --- tests/test_cli_client.cpp | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/test_cli_client.cpp b/tests/test_cli_client.cpp index 5814c5f1a1..190a992a6d 100644 --- a/tests/test_cli_client.cpp +++ b/tests/test_cli_client.cpp @@ -1210,6 +1210,32 @@ TEST_P(SSHClientReturnTest, execCmdWithDirWorks) INSTANTIATE_TEST_SUITE_P(Client, SSHClientReturnTest, Values(0, -1, 1, 127)); +TEST_F(Client, execCmdWithDirPrependsCd) +{ + std::string dir{"/home/ubuntu/"}; + std::string cmd{"pwd"}; + + REPLACE(ssh_channel_request_exec, ([&dir, &cmd](ssh_channel, const char* raw_cmd) { + EXPECT_THAT(raw_cmd, StartsWith("'cd' '" + dir + "'")); + EXPECT_THAT(raw_cmd, HasSubstr("&&")); + EXPECT_THAT(raw_cmd, EndsWith("'" + cmd + "'")); + + return SSH_OK; + })); + + std::string instance_name{"instance"}; + mp::SSHInfoReply response = make_fake_response(instance_name); + + EXPECT_CALL(mock_daemon, ssh_info(_, _, _)) + .WillOnce([&response](grpc::ServerContext* context, const mp::SSHInfoRequest* request, + grpc::ServerWriter* server) { + server->Write(response); + return grpc::Status{}; + }); + + EXPECT_EQ(send_command({"exec", instance_name, "--working-directory", dir, "--", cmd}), mp::ReturnCode::Ok); +} + // help cli tests TEST_F(Client, help_cmd_ok_with_valid_single_arg) { From ac013254900cb6e42d99d297e1933140d416a8ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Fri, 27 May 2022 10:56:12 -0300 Subject: [PATCH 19/48] [tests][cli] Check that exec fails if SSH throws. --- tests/test_cli_client.cpp | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/test_cli_client.cpp b/tests/test_cli_client.cpp index 190a992a6d..e50533ba65 100644 --- a/tests/test_cli_client.cpp +++ b/tests/test_cli_client.cpp @@ -42,6 +42,7 @@ #include #include +#include #include #include @@ -1236,6 +1237,31 @@ TEST_F(Client, execCmdWithDirPrependsCd) EXPECT_EQ(send_command({"exec", instance_name, "--working-directory", dir, "--", cmd}), mp::ReturnCode::Ok); } +TEST_F(Client, execCmdFailsIfSshExecThrows) +{ + std::string dir{"/home/ubuntu/"}; + std::string cmd{"pwd"}; + + REPLACE(ssh_channel_request_exec, ([](ssh_channel, const char* raw_cmd) { + throw mp::SSHException("some exception"); + return SSH_OK; + })); + + std::string instance_name{"instance"}; + mp::SSHInfoReply response = make_fake_response(instance_name); + + EXPECT_CALL(mock_daemon, ssh_info(_, _, _)) + .WillOnce([&response](grpc::ServerContext* context, const mp::SSHInfoRequest* request, + grpc::ServerWriter* server) { + server->Write(response); + return grpc::Status{}; + }); + + std::stringstream cerr_stream; + EXPECT_EQ(send_command({"exec", instance_name, "--", cmd}, trash_stream, cerr_stream), mp::ReturnCode::CommandFail); + EXPECT_EQ(cerr_stream.str(), "exec failed: some exception\n"); +} + // help cli tests TEST_F(Client, help_cmd_ok_with_valid_single_arg) { From 92139b5898aa0f7841f8fe82cdeead616cf39175 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Fri, 27 May 2022 14:51:02 -0300 Subject: [PATCH 20/48] [utils] Add `QDir::current()` to FileOps. --- include/multipass/file_ops.h | 1 + src/utils/file_ops.cpp | 5 +++++ tests/mock_file_ops.h | 1 + 3 files changed, 7 insertions(+) diff --git a/include/multipass/file_ops.h b/include/multipass/file_ops.h index 693e2a165d..32f175772c 100644 --- a/include/multipass/file_ops.h +++ b/include/multipass/file_ops.h @@ -39,6 +39,7 @@ class FileOps : public Singleton FileOps(const Singleton::PrivatePass&) noexcept; // QDir operations + virtual QDir current() const; virtual bool isReadable(const QDir& dir) const; virtual bool mkpath(const QDir& dir, const QString& dirName) const; virtual bool rmdir(QDir& dir, const QString& dirName) const; diff --git a/src/utils/file_ops.cpp b/src/utils/file_ops.cpp index 37805eb664..04611499e0 100644 --- a/src/utils/file_ops.cpp +++ b/src/utils/file_ops.cpp @@ -23,6 +23,11 @@ mp::FileOps::FileOps(const Singleton::PrivatePass& pass) noexcept : Sin { } +QDir mp::FileOps::current() const +{ + return QDir::current(); +} + bool mp::FileOps::isReadable(const QDir& dir) const { return dir.isReadable(); diff --git a/tests/mock_file_ops.h b/tests/mock_file_ops.h index d77c43cbf3..2522c57925 100644 --- a/tests/mock_file_ops.h +++ b/tests/mock_file_ops.h @@ -30,6 +30,7 @@ class MockFileOps : public FileOps public: using FileOps::FileOps; + MOCK_CONST_METHOD0(current, QDir()); MOCK_CONST_METHOD1(isReadable, bool(const QDir&)); MOCK_CONST_METHOD2(mkpath, bool(const QDir&, const QString& dirName)); MOCK_CONST_METHOD2(rmdir, bool(QDir&, const QString& dirName)); From b37ec4a99aac5f43d130cf20a596ff1ece99fe0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Mon, 30 May 2022 14:28:20 -0300 Subject: [PATCH 21/48] [cli] Replace QDir::current with FileOps::current. --- src/client/cli/cmd/exec.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/client/cli/cmd/exec.cpp b/src/client/cli/cmd/exec.cpp index 83c451cecf..6739f8a06e 100644 --- a/src/client/cli/cmd/exec.cpp +++ b/src/client/cli/cmd/exec.cpp @@ -19,6 +19,7 @@ #include "common_cli.h" #include +#include #include namespace mp = multipass; @@ -70,7 +71,7 @@ mp::ReturnCode cmd::Exec::run(mp::ArgParser* parser) if (parser->executeAlias()) { // The host directory on which the user is executing the command. - QString clean_exec_dir = QDir::cleanPath(QDir::current().canonicalPath()); + QString clean_exec_dir = QDir::cleanPath(MP_FILEOPS.current().canonicalPath()); QStringList split_exec_dir = clean_exec_dir.split('/'); auto on_info_success = [&work_dir, &split_exec_dir](mp::InfoReply& reply) { From 7d14650a6197b0d9f75069caf13026162b6f1bd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Fri, 3 Jun 2022 10:37:30 -0300 Subject: [PATCH 22/48] [cli] Fix bugs in directory mapping. 1. Fixed the comparison between working directory and source directory (the size of the compared string lists was wrong). 2. Use QDir::absolutePath() instead of QDir::canonicalPath() for target path (the latter used to return an empty string because the path did not exist in the host). --- src/client/cli/cmd/exec.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/client/cli/cmd/exec.cpp b/src/client/cli/cmd/exec.cpp index 6739f8a06e..a90704021c 100644 --- a/src/client/cli/cmd/exec.cpp +++ b/src/client/cli/cmd/exec.cpp @@ -29,15 +29,15 @@ namespace { const QString work_dir_option_name{"working-directory"}; -bool is_dir_mounted(const QStringList& split_exec_dir, const QStringList& split_source_dir) +bool is_dir_mounted(const QStringList& split_current_dir, const QStringList& split_source_dir) { - int exec_dir_size = split_exec_dir.size(); + int source_dir_size = split_source_dir.size(); - if (exec_dir_size < split_source_dir.size()) + if (split_current_dir.size() < source_dir_size) return false; - for (int i = 0; i < exec_dir_size; ++i) - if (split_exec_dir[i] != split_source_dir[i]) + for (int i = 0; i < source_dir_size; ++i) + if (split_current_dir[i] != split_source_dir[i]) return false; return true; @@ -78,7 +78,7 @@ mp::ReturnCode cmd::Exec::run(mp::ArgParser* parser) for (const auto& mount : reply.info(0).mount_info().mount_paths()) { auto source_dir = QDir(QString::fromStdString(mount.source_path())); - auto clean_source_dir = QDir::cleanPath(source_dir.canonicalPath()); + auto clean_source_dir = QDir::cleanPath(source_dir.absolutePath()); QStringList split_source_dir = clean_source_dir.split('/'); // If the directory is mounted, we need to `cd` to it in the instance before executing the command. @@ -165,7 +165,9 @@ mp::ReturnCode cmd::Exec::exec_success(const mp::SSHInfoReply& reply, const mp:: std::vector> all_args; if (dir) + { all_args = {{"cd", *dir}, {args}}; + } else all_args = {{args}}; From 9dfae7c08070aed61b6bc5fd544e4b1964a6a12a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Fri, 3 Jun 2022 10:43:38 -0300 Subject: [PATCH 23/48] [tests][cli] Test automatic working dir mapping. --- tests/test_cli_client.cpp | 133 ++++++++++++++++++++++++++------------ 1 file changed, 92 insertions(+), 41 deletions(-) diff --git a/tests/test_cli_client.cpp b/tests/test_cli_client.cpp index e50533ba65..9f5af88f6b 100644 --- a/tests/test_cli_client.cpp +++ b/tests/test_cli_client.cpp @@ -299,6 +299,43 @@ struct ClientAlias : public Client, public FakeAliasConfig } }; +auto make_info_function(const std::string& source_path = "", const std::string& target_path = "") +{ + auto info_function = [&source_path, &target_path](grpc::ServerContext*, const mp::InfoRequest* request, + grpc::ServerWriter* response) { + mp::InfoReply info_reply; + + if (request->instance_names().instance_name(0) == "primary") + { + auto vm_info = info_reply.add_info(); + vm_info->set_name("primary"); + vm_info->mutable_instance_status()->set_status(mp::InstanceStatus::RUNNING); + + if (!source_path.empty() && !target_path.empty()) + { + auto mount_info = vm_info->mutable_mount_info(); + + auto entry = mount_info->add_mount_paths(); + entry->set_source_path(source_path); + entry->set_target_path(target_path); + + if (source_path.size() > target_path.size()) + mount_info->set_longest_path_len(source_path.size()); + else + mount_info->set_longest_path_len(target_path.size()); + } + + response->Write(info_reply); + + return grpc::Status{}; + } + else + return grpc::Status{grpc::StatusCode::INVALID_ARGUMENT, "msg"}; + }; + + return info_function; +} + typedef std::vector> AliasesVector; // Tests for no postional args given @@ -1159,7 +1196,7 @@ mp::SSHInfo make_ssh_info(const std::string& host = "222.222.222.222", int port return ssh_info; } -mp::SSHInfoReply make_fake_response(const std::string& instance_name) +mp::SSHInfoReply make_fake_ssh_info_response(const std::string& instance_name) { mp::SSHInfoReply response; (*response.mutable_ssh_info())[instance_name] = make_ssh_info(); @@ -1178,7 +1215,7 @@ TEST_P(SSHClientReturnTest, execCmdWithoutDirWorks) REPLACE(ssh_channel_get_exit_status, [&failure_code](auto) { return failure_code; }); std::string instance_name{"instance"}; - mp::SSHInfoReply response = make_fake_response(instance_name); + mp::SSHInfoReply response = make_fake_ssh_info_response(instance_name); EXPECT_CALL(mock_daemon, ssh_info(_, _, _)) .WillOnce([&response](grpc::ServerContext* context, const mp::SSHInfoRequest* request, @@ -1197,7 +1234,7 @@ TEST_P(SSHClientReturnTest, execCmdWithDirWorks) REPLACE(ssh_channel_get_exit_status, [&failure_code](auto) { return failure_code; }); std::string instance_name{"instance"}; - mp::SSHInfoReply response = make_fake_response(instance_name); + mp::SSHInfoReply response = make_fake_ssh_info_response(instance_name); EXPECT_CALL(mock_daemon, ssh_info(_, _, _)) .WillOnce([&response](grpc::ServerContext* context, const mp::SSHInfoRequest* request, @@ -1225,7 +1262,7 @@ TEST_F(Client, execCmdWithDirPrependsCd) })); std::string instance_name{"instance"}; - mp::SSHInfoReply response = make_fake_response(instance_name); + mp::SSHInfoReply response = make_fake_ssh_info_response(instance_name); EXPECT_CALL(mock_daemon, ssh_info(_, _, _)) .WillOnce([&response](grpc::ServerContext* context, const mp::SSHInfoRequest* request, @@ -1248,7 +1285,7 @@ TEST_F(Client, execCmdFailsIfSshExecThrows) })); std::string instance_name{"instance"}; - mp::SSHInfoReply response = make_fake_response(instance_name); + mp::SSHInfoReply response = make_fake_ssh_info_response(instance_name); EXPECT_CALL(mock_daemon, ssh_info(_, _, _)) .WillOnce([&response](grpc::ServerContext* context, const mp::SSHInfoRequest* request, @@ -1579,8 +1616,6 @@ TEST_F(Client, version_info_on_failure) EXPECT_THAT(send_command({"version", "--format=yaml"}), Eq(mp::ReturnCode::Ok)); } -namespace -{ grpc::Status aborted_start_status(const std::vector& absent_instances = {}, const std::vector& deleted_instances = {}) { @@ -1603,7 +1638,6 @@ std::vector concat(const std::vector& v1, const std::v return ret; } -} // namespace TEST_F(Client, start_cmd_launches_petenv_if_absent) { @@ -2749,27 +2783,9 @@ INSTANTIATE_TEST_SUITE_P(Client, ClientLogMessageSuite, std::vector{"mount", "..", "test-vm:test"}, std::vector{"start"}, std::vector{"version"})); -auto info_function = [](grpc::ServerContext*, const mp::InfoRequest* request, - grpc::ServerWriter* response) { - mp::InfoReply info_reply; - - if (request->instance_names().instance_name(0) == "primary") - { - auto vm_info = info_reply.add_info(); - vm_info->set_name("primary"); - vm_info->mutable_instance_status()->set_status(mp::InstanceStatus::RUNNING); - - response->Write(info_reply); - - return grpc::Status{}; - } - else - return grpc::Status{grpc::StatusCode::INVALID_ARGUMENT, "msg"}; -}; - TEST_F(ClientAlias, alias_creates_alias) { - EXPECT_CALL(mock_daemon, info(_, _, _)).Times(AtMost(1)).WillRepeatedly(info_function); + EXPECT_CALL(mock_daemon, info(_, _, _)).Times(AtMost(1)).WillRepeatedly(make_info_function()); populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command"}}}); @@ -2791,7 +2807,7 @@ TEST_P(ClientAliasNameSuite, creates_correct_default_alias_name) { const auto& [command, path] = GetParam(); - EXPECT_CALL(mock_daemon, info(_, _, _)).Times(AtMost(1)).WillRepeatedly(info_function); + EXPECT_CALL(mock_daemon, info(_, _, _)).Times(AtMost(1)).WillRepeatedly(make_info_function()); std::vector arguments{"alias"}; arguments.push_back(fmt::format("primary:{}{}", path, command)); @@ -2814,7 +2830,7 @@ TEST_F(ClientAlias, fails_if_cannot_write_script) { EXPECT_CALL(*mock_platform, create_alias_script(_, _)).Times(1).WillRepeatedly(Throw(std::runtime_error("aaa"))); - EXPECT_CALL(mock_daemon, info(_, _, _)).Times(AtMost(1)).WillRepeatedly(info_function); + EXPECT_CALL(mock_daemon, info(_, _, _)).Times(AtMost(1)).WillRepeatedly(make_info_function()); std::stringstream cerr_stream; EXPECT_EQ(send_command({"alias", "primary:command"}, trash_stream, cerr_stream), mp::ReturnCode::CommandLineError); @@ -2828,7 +2844,7 @@ TEST_F(ClientAlias, fails_if_cannot_write_script) TEST_F(ClientAlias, alias_does_not_overwrite_alias) { - EXPECT_CALL(mock_daemon, info(_, _, _)).Times(AtMost(1)).WillRepeatedly(info_function); + EXPECT_CALL(mock_daemon, info(_, _, _)).Times(AtMost(1)).WillRepeatedly(make_info_function()); populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command"}}}); @@ -2853,7 +2869,7 @@ TEST_P(ArgumentCheckTestsuite, answers_correctly) { auto [arguments, expected_return_code, expected_cout, expected_cerr] = GetParam(); - EXPECT_CALL(mock_daemon, info(_, _, _)).Times(AtMost(1)).WillRepeatedly(info_function); + EXPECT_CALL(mock_daemon, info(_, _, _)).Times(AtMost(1)).WillRepeatedly(make_info_function()); std::stringstream cout_stream, cerr_stream; EXPECT_EQ(send_command(arguments, cout_stream, cerr_stream), expected_return_code); @@ -2917,7 +2933,7 @@ TEST_F(ClientAlias, execute_existing_alias) { populate_db_file(AliasesVector{{"some_alias", {"some_instance", "some_command"}}}); - EXPECT_CALL(mock_daemon, info(_, _, _)).WillOnce(info_function); + EXPECT_CALL(mock_daemon, info(_, _, _)).WillOnce(make_info_function()); EXPECT_CALL(mock_daemon, ssh_info(_, _, _)); EXPECT_EQ(send_command({"some_alias"}), mp::ReturnCode::Ok); @@ -2938,7 +2954,7 @@ TEST_F(ClientAlias, execute_alias_with_arguments) { populate_db_file(AliasesVector{{"some_alias", {"some_instance", "some_command"}}}); - EXPECT_CALL(mock_daemon, info(_, _, _)).WillOnce(info_function); + EXPECT_CALL(mock_daemon, info(_, _, _)).WillOnce(make_info_function()); EXPECT_CALL(mock_daemon, ssh_info(_, _, _)); EXPECT_EQ(send_command({"some_alias", "some_argument"}), mp::ReturnCode::Ok); @@ -2959,7 +2975,7 @@ TEST_F(ClientAlias, fails_executing_alias_without_separator) TEST_F(ClientAlias, alias_refuses_creation_unexisting_instance) { - EXPECT_CALL(mock_daemon, info(_, _, _)).Times(AtMost(1)).WillRepeatedly(info_function); + EXPECT_CALL(mock_daemon, info(_, _, _)).Times(AtMost(1)).WillRepeatedly(make_info_function()); populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command"}}}); @@ -3056,7 +3072,7 @@ TEST_F(ClientAlias, fails_when_remove_backup_alias_file_fails) EXPECT_CALL(*mock_file_ops, write(_, _)).WillOnce(Return(true)); EXPECT_CALL(*mock_file_ops, remove(_)).WillOnce(Return(false)); - EXPECT_CALL(mock_daemon, info(_, _, _)).Times(AtMost(1)).WillRepeatedly(info_function); + EXPECT_CALL(mock_daemon, info(_, _, _)).Times(AtMost(1)).WillRepeatedly(make_info_function()); std::stringstream cerr_stream; send_command({"alias", "primary:command", "alias_name"}, trash_stream, cerr_stream); @@ -3074,7 +3090,7 @@ TEST_F(ClientAlias, fails_renaming_alias_file_fails) EXPECT_CALL(*mock_file_ops, write(_, _)).WillOnce(Return(true)); EXPECT_CALL(*mock_file_ops, rename(_, _)).WillOnce(Return(false)); - EXPECT_CALL(mock_daemon, info(_, _, _)).Times(AtMost(1)).WillRepeatedly(info_function); + EXPECT_CALL(mock_daemon, info(_, _, _)).Times(AtMost(1)).WillRepeatedly(make_info_function()); std::stringstream cerr_stream; send_command({"alias", "primary:command", "alias_name"}, trash_stream, cerr_stream); @@ -3092,7 +3108,7 @@ TEST_F(ClientAlias, fails_creating_alias_file_fails) EXPECT_CALL(*mock_file_ops, write(_, _)).WillOnce(Return(true)); EXPECT_CALL(*mock_file_ops, rename(_, _)).WillOnce(Return(false)); - EXPECT_CALL(mock_daemon, info(_, _, _)).Times(AtMost(1)).WillRepeatedly(info_function); + EXPECT_CALL(mock_daemon, info(_, _, _)).Times(AtMost(1)).WillRepeatedly(make_info_function()); std::stringstream cerr_stream; send_command({"alias", "primary:command", "alias_name"}, trash_stream, cerr_stream); @@ -3102,7 +3118,7 @@ TEST_F(ClientAlias, fails_creating_alias_file_fails) TEST_F(ClientAlias, creating_first_alias_displays_message) { - EXPECT_CALL(mock_daemon, info(_, _, _)).WillOnce(info_function); + EXPECT_CALL(mock_daemon, info(_, _, _)).WillOnce(make_info_function()); std::stringstream cout_stream; EXPECT_EQ(send_command({"alias", "primary:a_command", "an_alias"}, cout_stream), mp::ReturnCode::Ok); @@ -3112,7 +3128,7 @@ TEST_F(ClientAlias, creating_first_alias_displays_message) TEST_F(ClientAlias, creating_first_alias_does_not_display_message_if_path_is_set) { - EXPECT_CALL(mock_daemon, info(_, _, _)).WillOnce(info_function); + EXPECT_CALL(mock_daemon, info(_, _, _)).WillOnce(make_info_function()); auto path = qgetenv("PATH"); #ifdef MULTIPASS_PLATFORM_WINDOWS @@ -3131,7 +3147,7 @@ TEST_F(ClientAlias, creating_first_alias_does_not_display_message_if_path_is_set TEST_F(ClientAlias, fails_when_name_clashes_with_command_alias) { - EXPECT_CALL(mock_daemon, info(_, _, _)).Times(AtMost(1)).WillRepeatedly(info_function); + EXPECT_CALL(mock_daemon, info(_, _, _)).Times(AtMost(1)).WillRepeatedly(make_info_function()); std::stringstream cerr_stream; send_command({"alias", "primary:command", "ls"}, trash_stream, cerr_stream); @@ -3141,11 +3157,46 @@ TEST_F(ClientAlias, fails_when_name_clashes_with_command_alias) TEST_F(ClientAlias, fails_when_name_clashes_with_command_name) { - EXPECT_CALL(mock_daemon, info(_, _, _)).Times(AtMost(1)).WillRepeatedly(info_function); + EXPECT_CALL(mock_daemon, info(_, _, _)).Times(AtMost(1)).WillRepeatedly(make_info_function()); std::stringstream cerr_stream; send_command({"alias", "primary:command", "list"}, trash_stream, cerr_stream); ASSERT_THAT(cerr_stream.str(), Eq("Alias name 'list' clashes with a command name\n")); } + +TEST_F(ClientAlias, execAliasRewritesMountedDir) +{ + std::string alias_name{"an_alias"}; + std::string instance_name{"primary"}; + std::string cmd{"pwd"}; + + QDir current_dir{QDir::current()}; + std::string source_dir{(current_dir.canonicalPath()).toStdString()}; + std::string target_dir{"/home/ubuntu/dir"}; + + EXPECT_CALL(mock_daemon, info(_, _, _)).Times(AtMost(1)).WillRepeatedly(make_info_function(source_dir, target_dir)); + + populate_db_file(AliasesVector{{alias_name, {instance_name, cmd}}}); + + REPLACE(ssh_channel_request_exec, ([&target_dir, &cmd](ssh_channel, const char* raw_cmd) { + EXPECT_THAT(raw_cmd, StartsWith("'cd' '" + target_dir + "/'")); + EXPECT_THAT(raw_cmd, HasSubstr("&&")); + EXPECT_THAT(raw_cmd, EndsWith("'" + cmd + "'")); + + return SSH_OK; + })); + + mp::SSHInfoReply ssh_info_response = make_fake_ssh_info_response(instance_name); + + EXPECT_CALL(mock_daemon, ssh_info(_, _, _)) + .WillOnce([&ssh_info_response](grpc::ServerContext* context, const mp::SSHInfoRequest* request, + grpc::ServerWriter* server) { + server->Write(ssh_info_response); + + return grpc::Status{}; + }); + + EXPECT_EQ(send_command({alias_name}), mp::ReturnCode::Ok); +} } // namespace From 1e70a0dd3cf525138c6bf466db19a0b9e290e8b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Fri, 3 Jun 2022 11:45:41 -0300 Subject: [PATCH 24/48] [cli] Add `--no-map-working-directory` to alias. --- src/client/cli/cmd/exec.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/client/cli/cmd/exec.cpp b/src/client/cli/cmd/exec.cpp index a90704021c..90fb5d0562 100644 --- a/src/client/cli/cmd/exec.cpp +++ b/src/client/cli/cmd/exec.cpp @@ -28,6 +28,7 @@ namespace cmd = multipass::cmd; namespace { const QString work_dir_option_name{"working-directory"}; +const QString no_dir_mapping_option{"no-map-working-directory"}; bool is_dir_mounted(const QStringList& split_current_dir, const QStringList& split_source_dir) { @@ -67,8 +68,8 @@ mp::ReturnCode cmd::Exec::run(mp::ArgParser* parser) else { // If we are executing an alias and the current working directory is mounted in the instance, then prepend the - // appropriate `cd` to the command to be ran. - if (parser->executeAlias()) + // appropriate `cd` to the command to be ran (unless the user specified the non-mapping option). + if (parser->executeAlias() && !parser->isSet(no_dir_mapping_option)) { // The host directory on which the user is executing the command. QString clean_exec_dir = QDir::cleanPath(MP_FILEOPS.current().canonicalPath()); @@ -186,8 +187,12 @@ mp::ParseCode cmd::Exec::parse_args(mp::ArgParser* parser) parser->addPositionalArgument("command", "Command to execute on the instance", "[--] "); QCommandLineOption workDirOption({"d", work_dir_option_name}, "Change to before execution", "dir"); + QCommandLineOption noDirMappingOption({"n", no_dir_mapping_option}, + "Do not map the host execution path to a mounted path"); + noIpv4Option.setFlags(QCommandLineOption::HiddenFromHelp); parser->addOptions({workDirOption}); + parser->addOptions({noDirMappingOption}); auto status = parser->commandParse(this); From fdf9795084f78b00949dfd3a6314b93df9408e34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Fri, 3 Jun 2022 11:46:07 -0300 Subject: [PATCH 25/48] [tests] Check alias `--no-map-working-directory`. --- tests/test_cli_client.cpp | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tests/test_cli_client.cpp b/tests/test_cli_client.cpp index 9f5af88f6b..7910734b99 100644 --- a/tests/test_cli_client.cpp +++ b/tests/test_cli_client.cpp @@ -3199,4 +3199,39 @@ TEST_F(ClientAlias, execAliasRewritesMountedDir) EXPECT_EQ(send_command({alias_name}), mp::ReturnCode::Ok); } + +TEST_F(ClientAlias, execAliasDoesNotRewriteMountedDir) +{ + std::string alias_name{"an_alias"}; + std::string instance_name{"primary"}; + std::string cmd{"pwd"}; + + QDir current_dir{QDir::current()}; + std::string source_dir{(current_dir.canonicalPath()).toStdString()}; + std::string target_dir{"/home/ubuntu/dir"}; + + EXPECT_CALL(mock_daemon, info(_, _, _)).Times(AtMost(1)).WillRepeatedly(make_info_function(source_dir, target_dir)); + + populate_db_file(AliasesVector{{alias_name, {instance_name, cmd}}}); + + REPLACE(ssh_channel_request_exec, ([&target_dir, &cmd](ssh_channel, const char* raw_cmd) { + EXPECT_THAT(raw_cmd, Not(StartsWith("'cd' '"))); + EXPECT_THAT(raw_cmd, Not(HasSubstr("&&"))); + EXPECT_THAT(raw_cmd, EndsWith("'" + cmd + "'")); + + return SSH_OK; + })); + + mp::SSHInfoReply ssh_info_response = make_fake_ssh_info_response(instance_name); + + EXPECT_CALL(mock_daemon, ssh_info(_, _, _)) + .WillOnce([&ssh_info_response](grpc::ServerContext* context, const mp::SSHInfoRequest* request, + grpc::ServerWriter* server) { + server->Write(ssh_info_response); + + return grpc::Status{}; + }); + + EXPECT_EQ(send_command({alias_name, "--no-map-working-directory"}), mp::ReturnCode::Ok); +} } // namespace From 7749d2b46137da8288597e8a0d8e9abf4fa608a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Mon, 6 Jun 2022 11:55:29 -0300 Subject: [PATCH 26/48] [cli] in . --- src/client/cli/cmd/exec.cpp | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/client/cli/cmd/exec.cpp b/src/client/cli/cmd/exec.cpp index 90fb5d0562..b542f1f6ae 100644 --- a/src/client/cli/cmd/exec.cpp +++ b/src/client/cli/cmd/exec.cpp @@ -67,9 +67,9 @@ mp::ReturnCode cmd::Exec::run(mp::ArgParser* parser) } else { - // If we are executing an alias and the current working directory is mounted in the instance, then prepend the - // appropriate `cd` to the command to be ran (unless the user specified the non-mapping option). - if (parser->executeAlias() && !parser->isSet(no_dir_mapping_option)) + // If the current working directory is mounted in the instance, then prepend the appropriate `cd` to the + // command to be ran (unless the user specified the non-mapping option). + if (!parser->isSet(no_dir_mapping_option)) { // The host directory on which the user is executing the command. QString clean_exec_dir = QDir::cleanPath(MP_FILEOPS.current().canonicalPath()); @@ -189,7 +189,6 @@ mp::ParseCode cmd::Exec::parse_args(mp::ArgParser* parser) QCommandLineOption workDirOption({"d", work_dir_option_name}, "Change to before execution", "dir"); QCommandLineOption noDirMappingOption({"n", no_dir_mapping_option}, "Do not map the host execution path to a mounted path"); - noIpv4Option.setFlags(QCommandLineOption::HiddenFromHelp); parser->addOptions({workDirOption}); parser->addOptions({noDirMappingOption}); @@ -208,7 +207,12 @@ mp::ParseCode cmd::Exec::parse_args(mp::ArgParser* parser) return status; } - if (parser->positionalArguments().count() < 2) + if (parser->isSet(work_dir_option_name) && parser->isSet(no_dir_mapping_option)) + { + cerr << fmt::format("Options --{} and --{} clash\n", work_dir_option_name, no_dir_mapping_option); + status = ParseCode::CommandLineError; + } + else if (parser->positionalArguments().count() < 2) { cerr << "Wrong number of arguments\n"; status = ParseCode::CommandLineError; From bcd75ba65c79d13ba8f2ce5ce32eba4facbdaf39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Mon, 6 Jun 2022 11:55:59 -0300 Subject: [PATCH 27/48] [tests] Check dir mapping in `exec`. --- tests/test_cli_client.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_cli_client.cpp b/tests/test_cli_client.cpp index 7910734b99..f8474b0594 100644 --- a/tests/test_cli_client.cpp +++ b/tests/test_cli_client.cpp @@ -1224,6 +1224,8 @@ TEST_P(SSHClientReturnTest, execCmdWithoutDirWorks) return grpc::Status{}; }); + EXPECT_CALL(mock_daemon, info(_, _, _)).WillOnce(make_info_function()); + EXPECT_EQ(send_command({"exec", instance_name, "--", "cmd"}), failure_code); } From 692c35f50d99c2b9712e68b9e7c6fd6ee763bc53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Mon, 6 Jun 2022 12:41:22 -0300 Subject: [PATCH 28/48] [tests] Check argument clash on `exec`. --- tests/test_cli_client.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/test_cli_client.cpp b/tests/test_cli_client.cpp index f8474b0594..7827369fe2 100644 --- a/tests/test_cli_client.cpp +++ b/tests/test_cli_client.cpp @@ -1301,6 +1301,18 @@ TEST_F(Client, execCmdFailsIfSshExecThrows) EXPECT_EQ(cerr_stream.str(), "exec failed: some exception\n"); } +TEST_F(Client, execFailsOnArgumentClash) +{ + std::stringstream cerr_stream; + + EXPECT_THAT(send_command({"exec", "instance", "--working-directory", "/home/ubuntu/", "--no-map-working-directory", + "--", "cmd"}, + trash_stream, cerr_stream), + Eq(mp::ReturnCode::CommandLineError)); + + EXPECT_THAT(cerr_stream.str(), Eq("Options --working-directory and --no-map-working-directory clash\n")); +} + // help cli tests TEST_F(Client, help_cmd_ok_with_valid_single_arg) { From e264d642b6d53746a2784d003c1a480e3d1926ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Mon, 6 Jun 2022 14:50:28 -0300 Subject: [PATCH 29/48] [tests] Avoid calling info twice on daemon tests. --- tests/test_cli_client.cpp | 14 +++++++++++--- tests/test_daemon.cpp | 4 ++-- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/tests/test_cli_client.cpp b/tests/test_cli_client.cpp index 7827369fe2..a21dcf699a 100644 --- a/tests/test_cli_client.cpp +++ b/tests/test_cli_client.cpp @@ -1090,12 +1090,14 @@ TEST_F(Client, purge_cmd_help_ok) TEST_F(Client, exec_cmd_double_dash_ok_cmd_arg) { EXPECT_CALL(mock_daemon, ssh_info(_, _, _)); + EXPECT_CALL(mock_daemon, info(_, _, _)).WillOnce(make_info_function()); EXPECT_THAT(send_command({"exec", "foo", "--", "cmd"}), Eq(mp::ReturnCode::Ok)); } TEST_F(Client, exec_cmd_double_dash_ok_cmd_arg_with_opts) { EXPECT_CALL(mock_daemon, ssh_info(_, _, _)); + EXPECT_CALL(mock_daemon, info(_, _, _)).WillOnce(make_info_function()); EXPECT_THAT(send_command({"exec", "foo", "--", "cmd", "--foo", "--bar"}), Eq(mp::ReturnCode::Ok)); } @@ -1107,12 +1109,14 @@ TEST_F(Client, exec_cmd_double_dash_fails_missing_cmd_arg) TEST_F(Client, exec_cmd_no_double_dash_ok_cmd_arg) { EXPECT_CALL(mock_daemon, ssh_info(_, _, _)); + EXPECT_CALL(mock_daemon, info(_, _, _)).WillOnce(make_info_function()); EXPECT_THAT(send_command({"exec", "foo", "cmd"}), Eq(mp::ReturnCode::Ok)); } TEST_F(Client, exec_cmd_no_double_dash_ok_multiple_args) { EXPECT_CALL(mock_daemon, ssh_info(_, _, _)); + EXPECT_CALL(mock_daemon, info(_, _, _)).WillOnce(make_info_function()); EXPECT_THAT(send_command({"exec", "foo", "cmd", "bar"}), Eq(mp::ReturnCode::Ok)); } @@ -1170,7 +1174,8 @@ TEST_F(Client, exec_cmd_starts_instance_if_stopped_or_suspended) EXPECT_CALL(mock_daemon, start(_, start_matcher, _)).WillOnce(Return(ok)); EXPECT_CALL(mock_daemon, ssh_info(_, ssh_info_matcher, _)).WillOnce(Return(ok)); - EXPECT_THAT(send_command({"exec", instance, "--", "command"}), Eq(mp::ReturnCode::Ok)); + EXPECT_THAT(send_command({"exec", instance, "--no-map-working-directory", "--", "command"}), + Eq(mp::ReturnCode::Ok)); } TEST_F(Client, exec_cmd_fails_on_other_absent_instance) @@ -1180,7 +1185,8 @@ TEST_F(Client, exec_cmd_fails_on_other_absent_instance) const grpc::Status notfound{grpc::StatusCode::NOT_FOUND, "msg"}; EXPECT_CALL(mock_daemon, ssh_info(_, instance_matcher, _)).WillOnce(Return(notfound)); - EXPECT_THAT(send_command({"exec", instance, "--", "command"}), Eq(mp::ReturnCode::CommandFail)); + EXPECT_THAT(send_command({"exec", instance, "--no-map-working-directory", "--", "command"}), + Eq(mp::ReturnCode::CommandFail)); } mp::SSHInfo make_ssh_info(const std::string& host = "222.222.222.222", int port = 22, @@ -1286,6 +1292,8 @@ TEST_F(Client, execCmdFailsIfSshExecThrows) return SSH_OK; })); + EXPECT_CALL(mock_daemon, info(_, _, _)).Times(AtMost(1)).WillRepeatedly(make_info_function()); + std::string instance_name{"instance"}; mp::SSHInfoReply response = make_fake_ssh_info_response(instance_name); @@ -1298,7 +1306,7 @@ TEST_F(Client, execCmdFailsIfSshExecThrows) std::stringstream cerr_stream; EXPECT_EQ(send_command({"exec", instance_name, "--", cmd}, trash_stream, cerr_stream), mp::ReturnCode::CommandFail); - EXPECT_EQ(cerr_stream.str(), "exec failed: some exception\n"); + EXPECT_THAT(cerr_stream.str(), HasSubstr("exec failed: some exception\n")); } TEST_F(Client, execFailsOnArgumentClash) diff --git a/tests/test_daemon.cpp b/tests/test_daemon.cpp index 00673fc446..bbfce1cac1 100644 --- a/tests/test_daemon.cpp +++ b/tests/test_daemon.cpp @@ -150,7 +150,7 @@ TEST_F(Daemon, receives_commands_and_calls_corresponding_slot) EXPECT_CALL(daemon, list(_, _, _)) .WillOnce(Invoke(&daemon, &mpt::MockDaemon::set_promise_value)); EXPECT_CALL(daemon, recover(_, _, _)) - .WillOnce(Invoke(&daemon, &mpt::MockDaemon::set_promise_value)); + .WillRepeatedly(Invoke(&daemon, &mpt::MockDaemon::set_promise_value)); EXPECT_CALL(daemon, start(_, _, _)) .WillOnce(Invoke(&daemon, &mpt::MockDaemon::set_promise_value)); EXPECT_CALL(daemon, stop(_, _, _)) @@ -178,7 +178,7 @@ TEST_F(Daemon, receives_commands_and_calls_corresponding_slot) {"test_create", "foo"}, {"launch", "foo"}, {"delete", "foo"}, - {"exec", "foo", "--", "cmd"}, + {"exec", "foo", "--no-map-working-directory", "--", "cmd"}, {"info", "foo"}, {"list"}, {"purge"}, From 95ea9c67a4bd1f60a43a4974b668d5324049449a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Tue, 7 Jun 2022 08:54:36 -0300 Subject: [PATCH 30/48] [tests] Fix compilation with clang. Removed an unused lambda capture. --- tests/test_cli_client.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_cli_client.cpp b/tests/test_cli_client.cpp index a21dcf699a..c49492704c 100644 --- a/tests/test_cli_client.cpp +++ b/tests/test_cli_client.cpp @@ -3236,7 +3236,7 @@ TEST_F(ClientAlias, execAliasDoesNotRewriteMountedDir) populate_db_file(AliasesVector{{alias_name, {instance_name, cmd}}}); - REPLACE(ssh_channel_request_exec, ([&target_dir, &cmd](ssh_channel, const char* raw_cmd) { + REPLACE(ssh_channel_request_exec, ([&cmd](ssh_channel, const char* raw_cmd) { EXPECT_THAT(raw_cmd, Not(StartsWith("'cd' '"))); EXPECT_THAT(raw_cmd, Not(HasSubstr("&&"))); EXPECT_THAT(raw_cmd, EndsWith("'" + cmd + "'")); From d99a5868d17d7aec5af69225e18bfcd3c93f80f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Wed, 8 Jun 2022 11:28:36 -0300 Subject: [PATCH 31/48] [tests][cli] Finish covering exec.cpp. --- tests/test_cli_client.cpp | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/tests/test_cli_client.cpp b/tests/test_cli_client.cpp index c49492704c..e877a4f777 100644 --- a/tests/test_cli_client.cpp +++ b/tests/test_cli_client.cpp @@ -3222,14 +3222,20 @@ TEST_F(ClientAlias, execAliasRewritesMountedDir) EXPECT_EQ(send_command({alias_name}), mp::ReturnCode::Ok); } -TEST_F(ClientAlias, execAliasDoesNotRewriteMountedDir) +struct NotDirRewriteTestsuite : public ClientAlias, public WithParamInterface> { +}; + +TEST_P(NotDirRewriteTestsuite, execAliasDoesNotRewriteMountedDir) +{ + auto [use_no_map_argument, source_qdir] = GetParam(); + std::string alias_name{"an_alias"}; std::string instance_name{"primary"}; std::string cmd{"pwd"}; QDir current_dir{QDir::current()}; - std::string source_dir{(current_dir.canonicalPath()).toStdString()}; + std::string source_dir{source_qdir.toStdString()}; std::string target_dir{"/home/ubuntu/dir"}; EXPECT_CALL(mock_daemon, info(_, _, _)).Times(AtMost(1)).WillRepeatedly(make_info_function(source_dir, target_dir)); @@ -3254,6 +3260,23 @@ TEST_F(ClientAlias, execAliasDoesNotRewriteMountedDir) return grpc::Status{}; }); - EXPECT_EQ(send_command({alias_name, "--no-map-working-directory"}), mp::ReturnCode::Ok); + std::vector arguments{alias_name}; + if (use_no_map_argument) + arguments.emplace_back("--no-map-working-directory"); + + EXPECT_EQ(send_command(arguments), mp::ReturnCode::Ok); +} + +QString current_cdup() +{ + QDir cur{QDir::current()}; + cur.cdUp(); + + return cur.canonicalPath(); } + +INSTANTIATE_TEST_SUITE_P(ClientAlias, NotDirRewriteTestsuite, + Values(std::make_pair(true, QDir{QDir::current()}.canonicalPath()), + std::make_pair(false, QDir{QDir::current()}.canonicalPath() + "/0/1/2/3/4/5/6/7/8/9"), + std::make_pair(false, current_cdup() + "/different_name"))); } // namespace From 8bf5b7aedbce6bc4b7686ae3ea0daec09e70b02c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Thu, 9 Jun 2022 14:55:19 -0300 Subject: [PATCH 32/48] [cli] Added `alias --no-map-working-directory`. --- include/multipass/cli/alias_definition.h | 1 + src/client/cli/cmd/alias.cpp | 14 +++++++++++++- src/client/common/alias_dict.cpp | 4 +++- 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/include/multipass/cli/alias_definition.h b/include/multipass/cli/alias_definition.h index d8ab998a0c..4eda5c99e1 100644 --- a/include/multipass/cli/alias_definition.h +++ b/include/multipass/cli/alias_definition.h @@ -26,6 +26,7 @@ struct AliasDefinition { std::string instance; std::string command; + bool map_working_directory; }; inline bool operator==(const AliasDefinition& a, const AliasDefinition& b) diff --git a/src/client/cli/cmd/alias.cpp b/src/client/cli/cmd/alias.cpp index dfabdf3372..9ce35a994d 100644 --- a/src/client/cli/cmd/alias.cpp +++ b/src/client/cli/cmd/alias.cpp @@ -27,6 +27,11 @@ namespace mp = multipass; namespace cmd = multipass::cmd; +namespace +{ +const QString no_alias_dir_mapping_option{"no-map-working-directory"}; +} // namespace + mp::ReturnCode cmd::Alias::run(mp::ArgParser* parser) { auto ret = parse_args(parser); @@ -86,10 +91,16 @@ mp::ParseCode cmd::Alias::parse_args(mp::ArgParser* parser) parser->addPositionalArgument("definition", "Alias definition in the form :", ""); parser->addPositionalArgument("name", "Name given to the alias being defined, defaults to ", "[]"); + QCommandLineOption noAliasDirMappingOption({"n", no_alias_dir_mapping_option}, + "Do not automatically map the host execution path to a mounted path"); + + parser->addOptions({noAliasDirMappingOption}); + auto status = parser->commandParse(this); if (status != ParseCode::Ok) return status; + // The number of arguments if (parser->positionalArguments().count() != 1 && parser->positionalArguments().count() != 2) { cerr << "Wrong number of arguments given\n"; @@ -130,6 +141,7 @@ mp::ParseCode cmd::Alias::parse_args(mp::ArgParser* parser) } auto instance = definition.left(colon_pos).toStdString(); + bool map_working_directory(!parser->isSet(no_alias_dir_mapping_option)); info_request.mutable_instance_names()->add_instance_name(instance); info_request.set_verbosity_level(0); @@ -167,7 +179,7 @@ mp::ParseCode cmd::Alias::parse_args(mp::ArgParser* parser) return ParseCode::CommandLineError; } - alias_definition = AliasDefinition{instance, command}; + alias_definition = AliasDefinition{instance, command, map_working_directory}; return ParseCode::Ok; } diff --git a/src/client/common/alias_dict.cpp b/src/client/common/alias_dict.cpp index 8211f1edb8..7144d86101 100644 --- a/src/client/common/alias_dict.cpp +++ b/src/client/common/alias_dict.cpp @@ -142,8 +142,9 @@ void mp::AliasDict::load_dict() auto instance = record["instance"].toString().toStdString(); auto command = record["command"].toString().toStdString(); + auto map_working_directory = record["map_working_directory"].toBool(); // Defaults to false if not present. - aliases.emplace(alias, mp::AliasDefinition{instance, command}); + aliases.emplace(alias, mp::AliasDefinition{instance, command, map_working_directory}); } db_file.close(); @@ -155,6 +156,7 @@ void mp::AliasDict::save_dict() QJsonObject json; json.insert("instance", QString::fromStdString(alias.instance)); json.insert("command", QString::fromStdString(alias.command)); + json.insert("map_working_directory", alias.map_working_directory); return json; }; From cd2e73b5754f209eafa526f1e2cf09c2e2ffc8ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Thu, 9 Jun 2022 14:59:16 -0300 Subject: [PATCH 33/48] [tests] Adapt tests to new alias struct. --- tests/linux/test_platform_linux.cpp | 22 +++++++++++-------- tests/test_alias_dict.cpp | 34 +++++++++++++++-------------- tests/test_argparser.cpp | 2 +- tests/test_cli_client.cpp | 32 +++++++++++++-------------- 4 files changed, 48 insertions(+), 42 deletions(-) diff --git a/tests/linux/test_platform_linux.cpp b/tests/linux/test_platform_linux.cpp index d2e482a2f9..b1c2d5636b 100644 --- a/tests/linux/test_platform_linux.cpp +++ b/tests/linux/test_platform_linux.cpp @@ -763,7 +763,7 @@ TEST_F(PlatformLinux, create_alias_script_works_unconfined) EXPECT_CALL(mpt::MockStandardPaths::mock_instance(), writableLocation(mp::StandardPaths::AppLocalDataLocation)) .WillOnce(Return(tmp_dir.path())); - EXPECT_NO_THROW(MP_PLATFORM.create_alias_script("alias_name", mp::AliasDefinition{"instance", "command"})); + EXPECT_NO_THROW(MP_PLATFORM.create_alias_script("alias_name", mp::AliasDefinition{"instance", "command", true})); QFile checked_script(tmp_dir.path() + "/bin/alias_name"); checked_script.open(QFile::ReadOnly); @@ -788,7 +788,7 @@ TEST_F(PlatformLinux, create_alias_script_works_confined) qputenv("SNAP_NAME", QByteArray{"multipass"}); qputenv("SNAP_USER_COMMON", tmp_dir.path().toUtf8()); - EXPECT_NO_THROW(MP_PLATFORM.create_alias_script("alias_name", mp::AliasDefinition{"instance", "command"})); + EXPECT_NO_THROW(MP_PLATFORM.create_alias_script("alias_name", mp::AliasDefinition{"instance", "command", true})); QFile checked_script(tmp_dir.path() + "/bin/alias_name"); checked_script.open(QFile::ReadOnly); @@ -816,7 +816,8 @@ TEST_F(PlatformLinux, create_alias_script_overwrites) EXPECT_CALL(*mock_file_ops, permissions(_)).WillOnce(Return(QFileDevice::ReadOwner | QFileDevice::WriteOwner)); EXPECT_CALL(*mock_file_ops, setPermissions(_, _)).WillOnce(Return(true)); - EXPECT_NO_THROW(MP_PLATFORM.create_alias_script("alias_name", mp::AliasDefinition{"instance", "other_command"})); + EXPECT_NO_THROW( + MP_PLATFORM.create_alias_script("alias_name", mp::AliasDefinition{"instance", "other_command", true})); } TEST_F(PlatformLinux, create_alias_script_throws_if_cannot_create_path) @@ -825,8 +826,9 @@ TEST_F(PlatformLinux, create_alias_script_throws_if_cannot_create_path) EXPECT_CALL(*mock_file_ops, mkpath(_, _)).WillOnce(Return(false)); - MP_EXPECT_THROW_THAT(MP_PLATFORM.create_alias_script("alias_name", mp::AliasDefinition{"instance", "command"}), - std::runtime_error, mpt::match_what(HasSubstr("failed to create dir '"))); + MP_EXPECT_THROW_THAT( + MP_PLATFORM.create_alias_script("alias_name", mp::AliasDefinition{"instance", "command", true}), + std::runtime_error, mpt::match_what(HasSubstr("failed to create dir '"))); } TEST_F(PlatformLinux, create_alias_script_throws_if_cannot_write_script) @@ -837,8 +839,9 @@ TEST_F(PlatformLinux, create_alias_script_throws_if_cannot_write_script) EXPECT_CALL(*mock_file_ops, open(_, _)).WillOnce(Return(true)); EXPECT_CALL(*mock_file_ops, write(_, _, _)).WillOnce(Return(747)); - MP_EXPECT_THROW_THAT(MP_PLATFORM.create_alias_script("alias_name", mp::AliasDefinition{"instance", "command"}), - std::runtime_error, mpt::match_what(HasSubstr("failed to write to file '"))); + MP_EXPECT_THROW_THAT( + MP_PLATFORM.create_alias_script("alias_name", mp::AliasDefinition{"instance", "command", true}), + std::runtime_error, mpt::match_what(HasSubstr("failed to write to file '"))); } TEST_F(PlatformLinux, create_alias_script_throws_if_cannot_set_permissions) @@ -850,8 +853,9 @@ TEST_F(PlatformLinux, create_alias_script_throws_if_cannot_set_permissions) EXPECT_CALL(*mock_file_ops, permissions(_)).WillOnce(Return(QFileDevice::ReadOwner | QFileDevice::WriteOwner)); EXPECT_CALL(*mock_file_ops, setPermissions(_, _)).WillOnce(Return(false)); - MP_EXPECT_THROW_THAT(MP_PLATFORM.create_alias_script("alias_name", mp::AliasDefinition{"instance", "command"}), - std::runtime_error, mpt::match_what(HasSubstr("cannot set permissions to alias script '"))); + MP_EXPECT_THROW_THAT( + MP_PLATFORM.create_alias_script("alias_name", mp::AliasDefinition{"instance", "command", true}), + std::runtime_error, mpt::match_what(HasSubstr("cannot set permissions to alias script '"))); } TEST_F(PlatformLinux, remove_alias_script_works) diff --git a/tests/test_alias_dict.cpp b/tests/test_alias_dict.cpp index 9642ef02e4..d985814ef8 100644 --- a/tests/test_alias_dict.cpp +++ b/tests/test_alias_dict.cpp @@ -154,9 +154,9 @@ TEST_P(WriteReadTeststuite, writes_and_reads_files) } INSTANTIATE_TEST_SUITE_P(AliasDictionary, WriteReadTeststuite, - Values(AliasesVector{}, AliasesVector{{"w", {"fake", "w"}}}, - AliasesVector{{"ipf", {"instance", "ip"}}}, - AliasesVector{{"lsp", {"primary", "ls"}}, {"llp", {"primary", "ls"}}})); + Values(AliasesVector{}, AliasesVector{{"w", {"fake", "w", true}}}, + AliasesVector{{"ipf", {"instance", "ip", true}}}, + AliasesVector{{"lsp", {"primary", "ls", true}}, {"llp", {"primary", "ls", true}}})); TEST_F(AliasDictionary, correctly_removes_alias) { @@ -164,7 +164,7 @@ TEST_F(AliasDictionary, correctly_removes_alias) mpt::StubTerminal trash_term(trash_stream, trash_stream, trash_stream); mp::AliasDict dict(&trash_term); - dict.add_alias("alias", mp::AliasDefinition{"instance", "command"}); + dict.add_alias("alias", mp::AliasDefinition{"instance", "command", true}); ASSERT_FALSE(dict.empty()); ASSERT_TRUE(dict.remove_alias("alias")); @@ -177,7 +177,7 @@ TEST_F(AliasDictionary, works_when_removing_unexisting_alias) mpt::StubTerminal trash_term(trash_stream, trash_stream, trash_stream); mp::AliasDict dict(&trash_term); - dict.add_alias("alias", mp::AliasDefinition{"instance", "command"}); + dict.add_alias("alias", mp::AliasDefinition{"instance", "command", true}); ASSERT_FALSE(dict.empty()); ASSERT_FALSE(dict.remove_alias("unexisting")); @@ -191,7 +191,7 @@ TEST_F(AliasDictionary, correctly_gets_alias) mp::AliasDict dict(&trash_term); std::string alias_name{"alias"}; - mp::AliasDefinition alias_def{"instance", "command"}; + mp::AliasDefinition alias_def{"instance", "command", true}; dict.add_alias(alias_name, alias_def); ASSERT_FALSE(dict.empty()); @@ -212,12 +212,12 @@ TEST_F(AliasDictionary, get_unexisting_alias_returns_nullopt) TEST_F(AliasDictionary, creates_backup_db) { - populate_db_file(AliasesVector{{"some_alias", {"some_instance", "some_command"}}}); + populate_db_file(AliasesVector{{"some_alias", {"some_instance", "some_command", true}}}); QString bak_filename = QString::fromStdString(db_filename() + ".bak"); ASSERT_FALSE(QFile::exists(bak_filename)); - populate_db_file(AliasesVector{{"another_alias", {"an_instance", "a_command"}}}); + populate_db_file(AliasesVector{{"another_alias", {"an_instance", "a_command", true}}}); ASSERT_TRUE(QFile::exists(bak_filename)); } @@ -262,7 +262,8 @@ std::string csv_head{"Alias,Instance,Command\n"}; INSTANTIATE_TEST_SUITE_P(AliasDictionary, FormatterTeststuite, Values(std::make_tuple(AliasesVector{}, csv_head, "{\n \"aliases\": [\n ]\n}\n", "No aliases defined.\n", "aliases: ~\n"), - std::make_tuple(AliasesVector{{"lsp", {"primary", "ls"}}, {"llp", {"primary", "ls"}}}, + std::make_tuple(AliasesVector{{"lsp", {"primary", "ls", true}}, + {"llp", {"primary", "ls", true}}}, csv_head + "llp,primary,ls\nlsp,primary,ls\n", "{\n \"aliases\": [\n {\n" " \"alias\": \"llp\",\n" @@ -303,13 +304,14 @@ TEST_P(RemoveInstanceTestsuite, removes_instance_aliases) INSTANTIATE_TEST_SUITE_P( AliasDictionary, RemoveInstanceTestsuite, - Values(std::make_pair(AliasesVector{{"some_alias", {"instance_to_remove", "some_command"}}, - {"other_alias", {"other_instance", "other_command"}}, - {"another_alias", {"instance_to_remove", "another_command"}}, - {"yet_another_alias", {"yet_another_instance", "yet_another_command"}}}, + Values(std::make_pair(AliasesVector{{"some_alias", {"instance_to_remove", "some_command", true}}, + {"other_alias", {"other_instance", "other_command", true}}, + {"another_alias", {"instance_to_remove", "another_command", true}}, + {"yet_another_alias", {"yet_another_instance", "yet_another_command", true}}}, std::vector{"other_alias", "yet_another_alias"}), - std::make_pair(AliasesVector{{"alias", {"instance", "command"}}}, std::vector{"alias"}), - std::make_pair(AliasesVector{{"alias", {"instance_to_remove", "command"}}}, std::vector{}))); + std::make_pair(AliasesVector{{"alias", {"instance", "command", true}}}, std::vector{"alias"}), + std::make_pair(AliasesVector{{"alias", {"instance_to_remove", "command", true}}}, + std::vector{}))); typedef std::vector> CmdList; @@ -345,7 +347,7 @@ TEST_P(DaemonAliasTestsuite, purge_removes_purged_instance_aliases_and_scripts) std::string json_contents = make_instance_json(mp::nullopt, {}, {"primary"}); - AliasesVector fake_aliases{{"lsp", {"primary", "ls"}}, {"lsz", {"real-zebraphant", "ls"}}}; + AliasesVector fake_aliases{{"lsp", {"primary", "ls", true}}, {"lsz", {"real-zebraphant", "ls", true}}}; populate_db_file(fake_aliases); diff --git a/tests/test_argparser.cpp b/tests/test_argparser.cpp index a57245bab4..21e0800ba3 100644 --- a/tests/test_argparser.cpp +++ b/tests/test_argparser.cpp @@ -77,7 +77,7 @@ TEST_P(TestAliasArguments, test_alias_arguments) const auto& [pre, post] = GetParam(); - populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command"}}}); + populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command", true}}}); mp::AliasDict alias_dict(&term); auto parser = mp::ArgParser{pre, cmds, oss, oss}; diff --git a/tests/test_cli_client.cpp b/tests/test_cli_client.cpp index e877a4f777..ec842b5130 100644 --- a/tests/test_cli_client.cpp +++ b/tests/test_cli_client.cpp @@ -2809,7 +2809,7 @@ TEST_F(ClientAlias, alias_creates_alias) { EXPECT_CALL(mock_daemon, info(_, _, _)).Times(AtMost(1)).WillRepeatedly(make_info_function()); - populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command"}}}); + populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command", true}}}); EXPECT_EQ(send_command({"alias", "primary:another_command", "another_alias"}), mp::ReturnCode::Ok); @@ -2868,7 +2868,7 @@ TEST_F(ClientAlias, alias_does_not_overwrite_alias) { EXPECT_CALL(mock_daemon, info(_, _, _)).Times(AtMost(1)).WillRepeatedly(make_info_function()); - populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command"}}}); + populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command", true}}}); std::stringstream cerr_stream; EXPECT_EQ(send_command({"alias", "primary:another_command", "an_alias"}, trash_stream, cerr_stream), @@ -2953,7 +2953,7 @@ TEST_F(ClientAlias, too_many_aliases_arguments) TEST_F(ClientAlias, execute_existing_alias) { - populate_db_file(AliasesVector{{"some_alias", {"some_instance", "some_command"}}}); + populate_db_file(AliasesVector{{"some_alias", {"some_instance", "some_command", true}}}); EXPECT_CALL(mock_daemon, info(_, _, _)).WillOnce(make_info_function()); EXPECT_CALL(mock_daemon, ssh_info(_, _, _)); @@ -2963,7 +2963,7 @@ TEST_F(ClientAlias, execute_existing_alias) TEST_F(ClientAlias, execute_unexisting_alias) { - populate_db_file(AliasesVector{{"some_alias", {"some_instance", "some_command"}}}); + populate_db_file(AliasesVector{{"some_alias", {"some_instance", "some_command", true}}}); EXPECT_CALL(mock_daemon, ssh_info(_, _, _)).Times(0); @@ -2974,7 +2974,7 @@ TEST_F(ClientAlias, execute_unexisting_alias) TEST_F(ClientAlias, execute_alias_with_arguments) { - populate_db_file(AliasesVector{{"some_alias", {"some_instance", "some_command"}}}); + populate_db_file(AliasesVector{{"some_alias", {"some_instance", "some_command", true}}}); EXPECT_CALL(mock_daemon, info(_, _, _)).WillOnce(make_info_function()); EXPECT_CALL(mock_daemon, ssh_info(_, _, _)); @@ -2984,7 +2984,7 @@ TEST_F(ClientAlias, execute_alias_with_arguments) TEST_F(ClientAlias, fails_executing_alias_without_separator) { - populate_db_file(AliasesVector{{"some_alias", {"some_instance", "some_command"}}}); + populate_db_file(AliasesVector{{"some_alias", {"some_instance", "some_command", true}}}); EXPECT_CALL(mock_daemon, ssh_info(_, _, _)).Times(0); @@ -2999,7 +2999,7 @@ TEST_F(ClientAlias, alias_refuses_creation_unexisting_instance) { EXPECT_CALL(mock_daemon, info(_, _, _)).Times(AtMost(1)).WillRepeatedly(make_info_function()); - populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command"}}}); + populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command", true}}}); std::stringstream cout_stream, cerr_stream; send_command({"alias", "foo:another_command", "another_alias"}, cout_stream, cerr_stream); @@ -3016,7 +3016,7 @@ TEST_F(ClientAlias, alias_refuses_creation_rpc_error) { EXPECT_CALL(mock_daemon, info(_, _, _)).WillOnce(Return(grpc::Status{grpc::StatusCode::NOT_FOUND, "msg"})); - populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command"}}}); + populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command", true}}}); std::stringstream cout_stream, cerr_stream; send_command({"alias", "foo:another_command", "another_alias"}, cout_stream, cerr_stream); @@ -3031,8 +3031,8 @@ TEST_F(ClientAlias, alias_refuses_creation_rpc_error) TEST_F(ClientAlias, unalias_removes_existing_alias) { - populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command"}}, - {"another_alias", {"another_instance", "another_command"}}}); + populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command", true}}, + {"another_alias", {"another_instance", "another_command", true}}}); EXPECT_EQ(send_command({"unalias", "another_alias"}), mp::ReturnCode::Ok); @@ -3046,8 +3046,8 @@ TEST_F(ClientAlias, unalias_succeeds_even_if_script_cannot_be_removed) { EXPECT_CALL(*mock_platform, remove_alias_script(_)).Times(1).WillRepeatedly(Throw(std::runtime_error("bbb"))); - populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command"}}, - {"another_alias", {"another_instance", "another_command"}}}); + populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command", true}}, + {"another_alias", {"another_instance", "another_command", true}}}); std::stringstream cerr_stream; EXPECT_EQ(send_command({"unalias", "another_alias"}, trash_stream, cerr_stream), mp::ReturnCode::Ok); @@ -3061,8 +3061,8 @@ TEST_F(ClientAlias, unalias_succeeds_even_if_script_cannot_be_removed) TEST_F(ClientAlias, unalias_does_not_remove_unexisting_alias) { - populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command"}}, - {"another_alias", {"another_instance", "another_command"}}}); + populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command", true}}, + {"another_alias", {"another_instance", "another_command", true}}}); std::stringstream cerr_stream; EXPECT_EQ(send_command({"unalias", "unexisting_alias"}, trash_stream, cerr_stream), @@ -3199,7 +3199,7 @@ TEST_F(ClientAlias, execAliasRewritesMountedDir) EXPECT_CALL(mock_daemon, info(_, _, _)).Times(AtMost(1)).WillRepeatedly(make_info_function(source_dir, target_dir)); - populate_db_file(AliasesVector{{alias_name, {instance_name, cmd}}}); + populate_db_file(AliasesVector{{alias_name, {instance_name, cmd, true}}}); REPLACE(ssh_channel_request_exec, ([&target_dir, &cmd](ssh_channel, const char* raw_cmd) { EXPECT_THAT(raw_cmd, StartsWith("'cd' '" + target_dir + "/'")); @@ -3240,7 +3240,7 @@ TEST_P(NotDirRewriteTestsuite, execAliasDoesNotRewriteMountedDir) EXPECT_CALL(mock_daemon, info(_, _, _)).Times(AtMost(1)).WillRepeatedly(make_info_function(source_dir, target_dir)); - populate_db_file(AliasesVector{{alias_name, {instance_name, cmd}}}); + populate_db_file(AliasesVector{{alias_name, {instance_name, cmd, true}}}); REPLACE(ssh_channel_request_exec, ([&cmd](ssh_channel, const char* raw_cmd) { EXPECT_THAT(raw_cmd, Not(StartsWith("'cd' '"))); From cd743eaecc54e17a676e69f975844a0c037d324b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Thu, 9 Jun 2022 16:30:35 -0300 Subject: [PATCH 34/48] [cli][formatter] Output alias dir mapping boolean. --- src/client/cli/formatter/csv_formatter.cpp | 4 ++-- src/client/cli/formatter/json_formatter.cpp | 1 + src/client/cli/formatter/table_formatter.cpp | 10 +++++++--- src/client/cli/formatter/yaml_formatter.cpp | 1 + 4 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/client/cli/formatter/csv_formatter.cpp b/src/client/cli/formatter/csv_formatter.cpp index 5fe7af3189..2bfbc5dcb7 100644 --- a/src/client/cli/formatter/csv_formatter.cpp +++ b/src/client/cli/formatter/csv_formatter.cpp @@ -116,14 +116,14 @@ std::string mp::CSVFormatter::format(const VersionReply& reply, const std::strin std::string mp::CSVFormatter::format(const mp::AliasDict& aliases) const { fmt::memory_buffer buf; - fmt::format_to(buf, "Alias,Instance,Command\n"); + fmt::format_to(buf, "Alias,Instance,Command,Map working directory\n"); for (const auto& elt : sort_dict(aliases)) { const auto& name = elt.first; const auto& def = elt.second; - fmt::format_to(buf, "{},{},{}\n", name, def.instance, def.command); + fmt::format_to(buf, "{},{},{},{}\n", name, def.instance, def.command, def.map_working_directory); } return fmt::to_string(buf); diff --git a/src/client/cli/formatter/json_formatter.cpp b/src/client/cli/formatter/json_formatter.cpp index 3276a97cd8..ecf2dc0cb6 100644 --- a/src/client/cli/formatter/json_formatter.cpp +++ b/src/client/cli/formatter/json_formatter.cpp @@ -237,6 +237,7 @@ std::string mp::JsonFormatter::format(const mp::AliasDict& aliases) const alias_obj.insert("alias", QString::fromStdString(alias)); alias_obj.insert("instance", QString::fromStdString(def.instance)); alias_obj.insert("command", QString::fromStdString(def.command)); + alias_obj.insert("map-working-directory", def.map_working_directory); aliases_array.append(alias_obj); } diff --git a/src/client/cli/formatter/table_formatter.cpp b/src/client/cli/formatter/table_formatter.cpp index a05041d727..c45b4adb24 100644 --- a/src/client/cli/formatter/table_formatter.cpp +++ b/src/client/cli/formatter/table_formatter.cpp @@ -256,17 +256,21 @@ std::string mp::TableFormatter::format(const mp::AliasDict& aliases) const aliases.cbegin(), aliases.cend(), [](const auto& alias) -> int { return alias.first.length(); }, 7); const auto instance_width = mp::format::column_width( aliases.cbegin(), aliases.cend(), [](const auto& alias) -> int { return alias.second.instance.length(); }, 10); + const auto command_width = mp::format::column_width( + aliases.cbegin(), aliases.cend(), [](const auto& alias) -> int { return alias.second.command.length(); }, 9); - const auto row_format = "{:<{}}{:<{}}{:<}\n"; + const auto row_format = "{:<{}}{:<{}}{:<{}}{:<}\n"; - fmt::format_to(buf, row_format, "Alias", alias_width, "Instance", instance_width, "Command"); + fmt::format_to(buf, row_format, "Alias", alias_width, "Instance", instance_width, "Command", command_width, + "Map working directory"); for (const auto& elt : sort_dict(aliases)) { const auto& name = elt.first; const auto& def = elt.second; - fmt::format_to(buf, row_format, name, alias_width, def.instance, instance_width, def.command); + fmt::format_to(buf, row_format, name, alias_width, def.instance, instance_width, def.command, command_width, + def.map_working_directory); } return fmt::to_string(buf); diff --git a/src/client/cli/formatter/yaml_formatter.cpp b/src/client/cli/formatter/yaml_formatter.cpp index dc92ea91d3..90761a8472 100644 --- a/src/client/cli/formatter/yaml_formatter.cpp +++ b/src/client/cli/formatter/yaml_formatter.cpp @@ -225,6 +225,7 @@ std::string mp::YamlFormatter::format(const mp::AliasDict& aliases) const alias_node["alias"] = alias; alias_node["command"] = def.command; alias_node["instance"] = def.instance; + alias_node["map-working-directory"] = def.map_working_directory; aliases_node.push_back(alias_node); } From 83773840b496552b409487c47763901aa99785f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Thu, 9 Jun 2022 16:31:05 -0300 Subject: [PATCH 35/48] [cli] Use "map-working-dir" in output files. Don't use underscores in order to be consistent. --- src/client/common/alias_dict.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client/common/alias_dict.cpp b/src/client/common/alias_dict.cpp index 7144d86101..a17bfa0274 100644 --- a/src/client/common/alias_dict.cpp +++ b/src/client/common/alias_dict.cpp @@ -142,7 +142,7 @@ void mp::AliasDict::load_dict() auto instance = record["instance"].toString().toStdString(); auto command = record["command"].toString().toStdString(); - auto map_working_directory = record["map_working_directory"].toBool(); // Defaults to false if not present. + auto map_working_directory = record["map-working-directory"].toBool(); // Defaults to false if not present. aliases.emplace(alias, mp::AliasDefinition{instance, command, map_working_directory}); } @@ -156,7 +156,7 @@ void mp::AliasDict::save_dict() QJsonObject json; json.insert("instance", QString::fromStdString(alias.instance)); json.insert("command", QString::fromStdString(alias.command)); - json.insert("map_working_directory", alias.map_working_directory); + json.insert("map-working-directory", alias.map_working_directory); return json; }; From 4472dbd6a550988c3e5ad34b82584e1217800e04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Fri, 10 Jun 2022 09:06:52 -0300 Subject: [PATCH 36/48] [tests][cli] Adapt tests to new formatters. --- tests/test_alias_dict.cpp | 58 +++++++++++++++++++-------------------- tests/test_cli_client.cpp | 37 +++++++++++++------------ 2 files changed, 48 insertions(+), 47 deletions(-) diff --git a/tests/test_alias_dict.cpp b/tests/test_alias_dict.cpp index d985814ef8..c77d385db4 100644 --- a/tests/test_alias_dict.cpp +++ b/tests/test_alias_dict.cpp @@ -125,11 +125,11 @@ TEST_F(AliasDictionary, skips_correctly_broken_entries) typedef std::vector> AliasesVector; -struct WriteReadTeststuite : public AliasDictionary, public WithParamInterface +struct WriteReadTestsuite : public AliasDictionary, public WithParamInterface { }; -TEST_P(WriteReadTeststuite, writes_and_reads_files) +TEST_P(WriteReadTestsuite, writes_and_reads_files) { auto aliases_vector = GetParam(); @@ -153,7 +153,7 @@ TEST_P(WriteReadTeststuite, writes_and_reads_files) ASSERT_EQ(reader.size(), aliases_vector.size()); } -INSTANTIATE_TEST_SUITE_P(AliasDictionary, WriteReadTeststuite, +INSTANTIATE_TEST_SUITE_P(AliasDictionary, WriteReadTestsuite, Values(AliasesVector{}, AliasesVector{{"w", {"fake", "w", true}}}, AliasesVector{{"ipf", {"instance", "ip", true}}}, AliasesVector{{"lsp", {"primary", "ls", true}}, {"llp", {"primary", "ls", true}}})); @@ -234,13 +234,13 @@ TEST_F(AliasDictionary, throws_when_open_alias_file_fails) mpt::match_what(HasSubstr("Error opening file '"))); } -struct FormatterTeststuite +struct FormatterTestsuite : public AliasDictionary, public WithParamInterface> { }; -TEST_P(FormatterTeststuite, table) +TEST_P(FormatterTestsuite, table) { auto [aliases, csv_output, json_output, table_output, yaml_output] = GetParam(); @@ -257,27 +257,32 @@ TEST_P(FormatterTeststuite, table) ASSERT_EQ(mp::YamlFormatter().format(dict), yaml_output); } -std::string csv_head{"Alias,Instance,Command\n"}; +const std::string csv_head{"Alias,Instance,Command,Map working directory\n"}; -INSTANTIATE_TEST_SUITE_P(AliasDictionary, FormatterTeststuite, +INSTANTIATE_TEST_SUITE_P(AliasDictionary, FormatterTestsuite, Values(std::make_tuple(AliasesVector{}, csv_head, "{\n \"aliases\": [\n ]\n}\n", "No aliases defined.\n", "aliases: ~\n"), std::make_tuple(AliasesVector{{"lsp", {"primary", "ls", true}}, {"llp", {"primary", "ls", true}}}, - csv_head + "llp,primary,ls\nlsp,primary,ls\n", + csv_head + "llp,primary,ls,true\nlsp,primary,ls,true\n", "{\n \"aliases\": [\n {\n" " \"alias\": \"llp\",\n" " \"command\": \"ls\",\n" - " \"instance\": \"primary\"\n" + " \"instance\": \"primary\",\n" + " \"map-working-directory\": true\n" " },\n" " {\n" " \"alias\": \"lsp\",\n" " \"command\": \"ls\",\n" - " \"instance\": \"primary\"\n" + " \"instance\": \"primary\",\n" + " \"map-working-directory\": true\n" " }\n ]\n}\n", - "Alias Instance Command\nllp primary ls\nlsp primary ls\n", + "Alias Instance Command Map working directory\n" + "llp primary ls true\n" + "lsp primary ls true\n", "aliases:\n - alias: llp\n command: ls\n instance: primary\n" - " - alias: lsp\n command: ls\n instance: primary\n"))); + " map-working-directory: true\n - alias: lsp\n command: ls\n" + " instance: primary\n map-working-directory: true\n"))); struct RemoveInstanceTestsuite : public AliasDictionary, public WithParamInterface>> @@ -383,25 +388,18 @@ TEST_P(DaemonAliasTestsuite, purge_removes_purged_instance_aliases_and_scripts) INSTANTIATE_TEST_SUITE_P( AliasDictionary, DaemonAliasTestsuite, - Values(std::make_tuple(CmdList{{"delete", "real-zebraphant"}, {"purge"}}, - std::string{"Alias,Instance,Command\nlsp,primary,ls\n"}, std::vector{"lsz"}, - std::vector{}), - std::make_tuple(CmdList{{"delete", "--purge", "real-zebraphant"}}, - std::string{"Alias,Instance,Command\nlsp,primary,ls\n"}, std::vector{"lsz"}, - std::vector{}), + Values(std::make_tuple(CmdList{{"delete", "real-zebraphant"}, {"purge"}}, csv_head + "lsp,primary,ls,true\n", + std::vector{"lsz"}, std::vector{}), + std::make_tuple(CmdList{{"delete", "--purge", "real-zebraphant"}}, csv_head + "lsp,primary,ls,true\n", + std::vector{"lsz"}, std::vector{}), std::make_tuple(CmdList{{"delete", "primary"}, {"delete", "primary", "real-zebraphant", "--purge"}}, - std::string{"Alias,Instance,Command\n"}, std::vector{"lsp", "lsz"}, - std::vector{}), + csv_head, std::vector{"lsp", "lsz"}, std::vector{}), std::make_tuple(CmdList{{"delete", "primary"}, {"delete", "primary", "real-zebraphant", "--purge"}}, - std::string{"Alias,Instance,Command\n"}, std::vector{}, - std::vector{"lsp", "lsz"}), + csv_head, std::vector{}, std::vector{"lsp", "lsz"}), std::make_tuple(CmdList{{"delete", "primary"}, {"delete", "primary", "real-zebraphant", "--purge"}}, - std::string{"Alias,Instance,Command\n"}, std::vector{"lsp"}, - std::vector{"lsz"}), - std::make_tuple(CmdList{{"delete", "real-zebraphant"}, {"purge"}}, - std::string{"Alias,Instance,Command\nlsp,primary,ls\n"}, std::vector{}, - std::vector{"lsz"}), - std::make_tuple(CmdList{{"delete", "real-zebraphant", "primary"}, {"purge"}}, - std::string{"Alias,Instance,Command\n"}, std::vector{}, - std::vector{"lsz", "lsp"}))); + csv_head, std::vector{"lsp"}, std::vector{"lsz"}), + std::make_tuple(CmdList{{"delete", "real-zebraphant"}, {"purge"}}, csv_head + "lsp,primary,ls,true\n", + std::vector{}, std::vector{"lsz"}), + std::make_tuple(CmdList{{"delete", "real-zebraphant", "primary"}, {"purge"}}, csv_head, + std::vector{}, std::vector{"lsz", "lsp"}))); } // namespace diff --git a/tests/test_cli_client.cpp b/tests/test_cli_client.cpp index ec842b5130..dfd6e505d5 100644 --- a/tests/test_cli_client.cpp +++ b/tests/test_cli_client.cpp @@ -338,6 +338,8 @@ auto make_info_function(const std::string& source_path = "", const std::string& typedef std::vector> AliasesVector; +const std::string csv_header{"Alias,Instance,Command,Map working directory\n"}; + // Tests for no postional args given TEST_F(Client, no_command_is_error) { @@ -2811,13 +2813,14 @@ TEST_F(ClientAlias, alias_creates_alias) populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command", true}}}); - EXPECT_EQ(send_command({"alias", "primary:another_command", "another_alias"}), mp::ReturnCode::Ok); + EXPECT_EQ(send_command({"alias", "primary:another_command", "another_alias", "--no-map-working-directory"}), + mp::ReturnCode::Ok); std::stringstream cout_stream; send_command({"aliases", "--format=csv"}, cout_stream); - EXPECT_THAT(cout_stream.str(), "Alias,Instance,Command\nan_alias,an_instance,a_command\nanother_alias," - "primary,another_command\n"); + EXPECT_THAT(cout_stream.str(), csv_header + "an_alias,an_instance,a_command,true\n" + "another_alias,primary,another_command,false\n"); } struct ClientAliasNameSuite : public ClientAlias, @@ -2833,15 +2836,14 @@ TEST_P(ClientAliasNameSuite, creates_correct_default_alias_name) std::vector arguments{"alias"}; arguments.push_back(fmt::format("primary:{}{}", path, command)); + arguments.push_back("--no-map-working-directory"); EXPECT_EQ(send_command(arguments), mp::ReturnCode::Ok); std::stringstream cout_stream; send_command({"aliases", "--format=csv"}, cout_stream); - EXPECT_THAT(cout_stream.str(), fmt::format("Alias,Instance,Command\n" - "{},primary,{}{}\n", - command, path, command)); + EXPECT_THAT(cout_stream.str(), fmt::format(csv_header + "{},primary,{}{},false\n", command, path, command)); } INSTANTIATE_TEST_SUITE_P(ClientAlias, ClientAliasNameSuite, @@ -2861,7 +2863,7 @@ TEST_F(ClientAlias, fails_if_cannot_write_script) std::stringstream cout_stream; send_command({"aliases", "--format=csv"}, cout_stream); - EXPECT_THAT(cout_stream.str(), "Alias,Instance,Command\n"); + EXPECT_THAT(cout_stream.str(), csv_header); } TEST_F(ClientAlias, alias_does_not_overwrite_alias) @@ -2878,7 +2880,7 @@ TEST_F(ClientAlias, alias_does_not_overwrite_alias) std::stringstream cout_stream; send_command({"aliases", "--format=csv"}, cout_stream); - EXPECT_THAT(cout_stream.str(), "Alias,Instance,Command\nan_alias,an_instance,a_command\n"); + EXPECT_THAT(cout_stream.str(), csv_header + "an_alias,an_instance,a_command,true\n"); } struct ArgumentCheckTestsuite @@ -3009,7 +3011,7 @@ TEST_F(ClientAlias, alias_refuses_creation_unexisting_instance) send_command({"aliases", "--format=csv"}, cout_stream); - EXPECT_THAT(cout_stream.str(), "Alias,Instance,Command\nan_alias,an_instance,a_command\n"); + EXPECT_THAT(cout_stream.str(), csv_header + "an_alias,an_instance,a_command,true\n"); } TEST_F(ClientAlias, alias_refuses_creation_rpc_error) @@ -3026,12 +3028,12 @@ TEST_F(ClientAlias, alias_refuses_creation_rpc_error) send_command({"aliases", "--format=csv"}, cout_stream); - EXPECT_THAT(cout_stream.str(), "Alias,Instance,Command\nan_alias,an_instance,a_command\n"); + EXPECT_THAT(cout_stream.str(), csv_header + "an_alias,an_instance,a_command,true\n"); } TEST_F(ClientAlias, unalias_removes_existing_alias) { - populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command", true}}, + populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command", false}}, {"another_alias", {"another_instance", "another_command", true}}}); EXPECT_EQ(send_command({"unalias", "another_alias"}), mp::ReturnCode::Ok); @@ -3039,14 +3041,14 @@ TEST_F(ClientAlias, unalias_removes_existing_alias) std::stringstream cout_stream; send_command({"aliases", "--format=csv"}, cout_stream); - EXPECT_THAT(cout_stream.str(), "Alias,Instance,Command\nan_alias,an_instance,a_command\n"); + EXPECT_THAT(cout_stream.str(), csv_header + "an_alias,an_instance,a_command,false\n"); } TEST_F(ClientAlias, unalias_succeeds_even_if_script_cannot_be_removed) { EXPECT_CALL(*mock_platform, remove_alias_script(_)).Times(1).WillRepeatedly(Throw(std::runtime_error("bbb"))); - populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command", true}}, + populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command", false}}, {"another_alias", {"another_instance", "another_command", true}}}); std::stringstream cerr_stream; @@ -3056,13 +3058,13 @@ TEST_F(ClientAlias, unalias_succeeds_even_if_script_cannot_be_removed) std::stringstream cout_stream; send_command({"aliases", "--format=csv"}, cout_stream); - EXPECT_THAT(cout_stream.str(), "Alias,Instance,Command\nan_alias,an_instance,a_command\n"); + EXPECT_THAT(cout_stream.str(), csv_header + "an_alias,an_instance,a_command,false\n"); } TEST_F(ClientAlias, unalias_does_not_remove_unexisting_alias) { populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command", true}}, - {"another_alias", {"another_instance", "another_command", true}}}); + {"another_alias", {"another_instance", "another_command", false}}}); std::stringstream cerr_stream; EXPECT_EQ(send_command({"unalias", "unexisting_alias"}, trash_stream, cerr_stream), @@ -3072,8 +3074,9 @@ TEST_F(ClientAlias, unalias_does_not_remove_unexisting_alias) std::stringstream cout_stream; send_command({"aliases", "--format=csv"}, cout_stream); - EXPECT_EQ(cout_stream.str(), "Alias,Instance,Command\nan_alias,an_instance,a_command\nanother_alias," - "another_instance,another_command\n"); + EXPECT_EQ(cout_stream.str(), + csv_header + + "an_alias,an_instance,a_command,true\nanother_alias,another_instance,another_command,false\n"); } TEST_F(ClientAlias, too_many_unalias_arguments) From 70f66e2548c67a9771c08627fb1ac65ea069231b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Fri, 10 Jun 2022 10:02:17 -0300 Subject: [PATCH 37/48] [tests][cli] Check no-dir-mapping default is false. It is false to assure backwards-compatibility. --- tests/test_alias_dict.cpp | 33 +++++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/tests/test_alias_dict.cpp b/tests/test_alias_dict.cpp index c77d385db4..f258404b96 100644 --- a/tests/test_alias_dict.cpp +++ b/tests/test_alias_dict.cpp @@ -94,13 +94,15 @@ TEST_F(AliasDictionary, skips_correctly_broken_entries) std::string file_contents{"{\n" " \"alias1\": {\n" " \"command\": \"first_command\",\n" - " \"instance\": \"first_instance\"\n" + " \"instance\": \"first_instance\",\n" + " \"map-working-directory\": true\n" " },\n" " \"empty_entry\": {\n" " },\n" " \"alias2\": {\n" " \"command\": \"second_command\",\n" - " \"instance\": \"second_instance\"\n" + " \"instance\": \"second_instance\",\n" + " \"map-working-directory\": false\n" " }\n" "}\n"}; @@ -116,11 +118,38 @@ TEST_F(AliasDictionary, skips_correctly_broken_entries) ASSERT_TRUE(a1); ASSERT_EQ(a1->instance, "first_instance"); ASSERT_EQ(a1->command, "first_command"); + ASSERT_EQ(a1->map_working_directory, true); auto a2 = dict.get_alias("alias2"); ASSERT_TRUE(a2); ASSERT_EQ(a2->instance, "second_instance"); ASSERT_EQ(a2->command, "second_command"); + ASSERT_EQ(a2->map_working_directory, false); +} + +// In old versions, the file did not contain the `map-working-directory` flag in the JSON, because the +// `--no-map-working-directory` flag were not yet introduced. In case the file was generated by an old version, +// the flag must be `false`, in order to maintain backwards compatibility. +TEST_F(AliasDictionary, mapDirInOldFilesDefaultsToFalse) +{ + std::string file_contents{"{\n" + " \"alias3\": {\n" + " \"command\": \"third_command\",\n" + " \"instance\": \"third_instance\"\n" + " }\n" + "}\n"}; + + mpt::make_file_with_content(QString::fromStdString(db_filename()), file_contents); + + std::stringstream trash_stream; + mpt::StubTerminal trash_term(trash_stream, trash_stream, trash_stream); + mp::AliasDict dict(&trash_term); + + auto a3 = dict.get_alias("alias3"); + ASSERT_TRUE(a3); + ASSERT_EQ(a3->instance, "third_instance"); + ASSERT_EQ(a3->command, "third_command"); + ASSERT_EQ(a3->map_working_directory, false); } typedef std::vector> AliasesVector; From 333ce68b44a7927f361ce7ce0597802925a62bcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Fri, 10 Jun 2022 10:16:32 -0300 Subject: [PATCH 38/48] [bash] Add new `exec`, `alias` completion options. --- completions/bash/multipass | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/completions/bash/multipass b/completions/bash/multipass index 089ec454c8..50cfa94027 100644 --- a/completions/bash/multipass +++ b/completions/bash/multipass @@ -70,16 +70,19 @@ _multipass_complete() { opts="" + local no_map_found=0 local help_found=0 local definition_found=0 local verbose_found=0 for ((i=2; i Date: Fri, 10 Jun 2022 15:05:27 -0300 Subject: [PATCH 39/48] [cli] Use `--no-runtime-information` in alias. The recently introduced hidden option is useful for querying the instances without executing commands on them. --- src/client/cli/cmd/alias.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/client/cli/cmd/alias.cpp b/src/client/cli/cmd/alias.cpp index 9ce35a994d..8e96058086 100644 --- a/src/client/cli/cmd/alias.cpp +++ b/src/client/cli/cmd/alias.cpp @@ -145,6 +145,7 @@ mp::ParseCode cmd::Alias::parse_args(mp::ArgParser* parser) info_request.mutable_instance_names()->add_instance_name(instance); info_request.set_verbosity_level(0); + info_request.set_no_runtime_information(true); auto on_success = [](InfoReply&) { return ReturnCode::Ok; }; From f27a8bfc6c541f4fad92558ac450debb59d67f91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Fri, 10 Jun 2022 15:07:10 -0300 Subject: [PATCH 40/48] [tests][cli] Test `info --no-runtime-information`. --- tests/test_cli_client.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/test_cli_client.cpp b/tests/test_cli_client.cpp index dfd6e505d5..c75aec9006 100644 --- a/tests/test_cli_client.cpp +++ b/tests/test_cli_client.cpp @@ -1378,6 +1378,24 @@ TEST_F(Client, info_cmd_fails_with_names_and_all) EXPECT_THAT(send_command({"info", "--all", "foo", "bar"}), Eq(mp::ReturnCode::CommandLineError)); } +TEST_F(Client, infoCmdDoesNotDefaultToNoRuntimeInformationAndSucceeds) +{ + EXPECT_CALL(mock_daemon, info(_, Property(&mp::InfoRequest::no_runtime_information, IsFalse()), _)); + EXPECT_THAT(send_command({"info", "name1", "name2"}), Eq(mp::ReturnCode::Ok)); +} + +TEST_F(Client, infoCmdSucceedsWithInstanceNamesAndNoRuntimeInformation) +{ + EXPECT_CALL(mock_daemon, info(_, Property(&mp::InfoRequest::no_runtime_information, IsTrue()), _)); + EXPECT_THAT(send_command({"info", "name3", "name4", "--no-runtime-information"}), Eq(mp::ReturnCode::Ok)); +} + +TEST_F(Client, infoCmdSucceedsWithAllAndNoRuntimeInformation) +{ + EXPECT_CALL(mock_daemon, info(_, Property(&mp::InfoRequest::no_runtime_information, IsTrue()), _)); + EXPECT_THAT(send_command({"info", "name5", "--no-runtime-information"}), Eq(mp::ReturnCode::Ok)); +} + // list cli tests TEST_F(Client, list_cmd_ok_no_args) { From 159ab6097c5dd75d8455a50295d36feb07ad34af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Fri, 17 Jun 2022 15:19:08 -0300 Subject: [PATCH 41/48] Address @townsend2010 review. --- include/multipass/file_ops.h | 1 - src/client/cli/cmd/exec.cpp | 2 +- src/utils/file_ops.cpp | 5 ----- tests/test_cli_client.cpp | 7 +++++-- tests/test_daemon.cpp | 2 +- tests/test_ssh_client.cpp | 17 +++++++++++++++++ 6 files changed, 24 insertions(+), 10 deletions(-) diff --git a/include/multipass/file_ops.h b/include/multipass/file_ops.h index 32f175772c..693e2a165d 100644 --- a/include/multipass/file_ops.h +++ b/include/multipass/file_ops.h @@ -39,7 +39,6 @@ class FileOps : public Singleton FileOps(const Singleton::PrivatePass&) noexcept; // QDir operations - virtual QDir current() const; virtual bool isReadable(const QDir& dir) const; virtual bool mkpath(const QDir& dir, const QString& dirName) const; virtual bool rmdir(QDir& dir, const QString& dirName) const; diff --git a/src/client/cli/cmd/exec.cpp b/src/client/cli/cmd/exec.cpp index b542f1f6ae..27681a1e05 100644 --- a/src/client/cli/cmd/exec.cpp +++ b/src/client/cli/cmd/exec.cpp @@ -72,7 +72,7 @@ mp::ReturnCode cmd::Exec::run(mp::ArgParser* parser) if (!parser->isSet(no_dir_mapping_option)) { // The host directory on which the user is executing the command. - QString clean_exec_dir = QDir::cleanPath(MP_FILEOPS.current().canonicalPath()); + QString clean_exec_dir = QDir::cleanPath(QDir::current().canonicalPath()); QStringList split_exec_dir = clean_exec_dir.split('/'); auto on_info_success = [&work_dir, &split_exec_dir](mp::InfoReply& reply) { diff --git a/src/utils/file_ops.cpp b/src/utils/file_ops.cpp index 04611499e0..37805eb664 100644 --- a/src/utils/file_ops.cpp +++ b/src/utils/file_ops.cpp @@ -23,11 +23,6 @@ mp::FileOps::FileOps(const Singleton::PrivatePass& pass) noexcept : Sin { } -QDir mp::FileOps::current() const -{ - return QDir::current(); -} - bool mp::FileOps::isReadable(const QDir& dir) const { return dir.isReadable(); diff --git a/tests/test_cli_client.cpp b/tests/test_cli_client.cpp index c75aec9006..d98d5d8120 100644 --- a/tests/test_cli_client.cpp +++ b/tests/test_cli_client.cpp @@ -3218,7 +3218,7 @@ TEST_F(ClientAlias, execAliasRewritesMountedDir) std::string source_dir{(current_dir.canonicalPath()).toStdString()}; std::string target_dir{"/home/ubuntu/dir"}; - EXPECT_CALL(mock_daemon, info(_, _, _)).Times(AtMost(1)).WillRepeatedly(make_info_function(source_dir, target_dir)); + EXPECT_CALL(mock_daemon, info(_, _, _)).WillOnce(make_info_function(source_dir, target_dir)); populate_db_file(AliasesVector{{alias_name, {instance_name, cmd, true}}}); @@ -3259,7 +3259,10 @@ TEST_P(NotDirRewriteTestsuite, execAliasDoesNotRewriteMountedDir) std::string source_dir{source_qdir.toStdString()}; std::string target_dir{"/home/ubuntu/dir"}; - EXPECT_CALL(mock_daemon, info(_, _, _)).Times(AtMost(1)).WillRepeatedly(make_info_function(source_dir, target_dir)); + if (use_no_map_argument) + EXPECT_CALL(mock_daemon, info(_, _, _)).Times(0); + else + EXPECT_CALL(mock_daemon, info(_, _, _)).WillOnce(make_info_function(source_dir, target_dir)); populate_db_file(AliasesVector{{alias_name, {instance_name, cmd, true}}}); diff --git a/tests/test_daemon.cpp b/tests/test_daemon.cpp index bbfce1cac1..c43bd30237 100644 --- a/tests/test_daemon.cpp +++ b/tests/test_daemon.cpp @@ -150,7 +150,7 @@ TEST_F(Daemon, receives_commands_and_calls_corresponding_slot) EXPECT_CALL(daemon, list(_, _, _)) .WillOnce(Invoke(&daemon, &mpt::MockDaemon::set_promise_value)); EXPECT_CALL(daemon, recover(_, _, _)) - .WillRepeatedly(Invoke(&daemon, &mpt::MockDaemon::set_promise_value)); + .WillOnce(Invoke(&daemon, &mpt::MockDaemon::set_promise_value)); EXPECT_CALL(daemon, start(_, _, _)) .WillOnce(Invoke(&daemon, &mpt::MockDaemon::set_promise_value)); EXPECT_CALL(daemon, stop(_, _, _)) diff --git a/tests/test_ssh_client.cpp b/tests/test_ssh_client.cpp index 3df50da668..4136016ca4 100644 --- a/tests/test_ssh_client.cpp +++ b/tests/test_ssh_client.cpp @@ -1,3 +1,20 @@ +/* + * Copyright (C) 2018-2022 Canonical, Ltd. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 3. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + #include "common.h" #include "disabling_macros.h" #include "fake_key_data.h" From e342a214cb7c42e3605185d7327a15df54eed46f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Mon, 20 Jun 2022 11:36:14 -0300 Subject: [PATCH 42/48] [cli] Improve human-readable formatter output. --- src/client/cli/formatter/csv_formatter.cpp | 5 +++-- src/client/cli/formatter/table_formatter.cpp | 5 +++-- tests/test_alias_dict.cpp | 16 ++++++++-------- tests/test_cli_client.cpp | 20 ++++++++++---------- 4 files changed, 24 insertions(+), 22 deletions(-) diff --git a/src/client/cli/formatter/csv_formatter.cpp b/src/client/cli/formatter/csv_formatter.cpp index 2bfbc5dcb7..914f671635 100644 --- a/src/client/cli/formatter/csv_formatter.cpp +++ b/src/client/cli/formatter/csv_formatter.cpp @@ -116,14 +116,15 @@ std::string mp::CSVFormatter::format(const VersionReply& reply, const std::strin std::string mp::CSVFormatter::format(const mp::AliasDict& aliases) const { fmt::memory_buffer buf; - fmt::format_to(buf, "Alias,Instance,Command,Map working directory\n"); + fmt::format_to(buf, "Alias,Instance,Command,Working directory\n"); for (const auto& elt : sort_dict(aliases)) { const auto& name = elt.first; const auto& def = elt.second; + std::string work_dir = def.map_working_directory ? "map" : "default"; - fmt::format_to(buf, "{},{},{},{}\n", name, def.instance, def.command, def.map_working_directory); + fmt::format_to(buf, "{},{},{},{}\n", name, def.instance, def.command, work_dir); } return fmt::to_string(buf); diff --git a/src/client/cli/formatter/table_formatter.cpp b/src/client/cli/formatter/table_formatter.cpp index c45b4adb24..b825b9a5f1 100644 --- a/src/client/cli/formatter/table_formatter.cpp +++ b/src/client/cli/formatter/table_formatter.cpp @@ -262,15 +262,16 @@ std::string mp::TableFormatter::format(const mp::AliasDict& aliases) const const auto row_format = "{:<{}}{:<{}}{:<{}}{:<}\n"; fmt::format_to(buf, row_format, "Alias", alias_width, "Instance", instance_width, "Command", command_width, - "Map working directory"); + "Working directory"); for (const auto& elt : sort_dict(aliases)) { const auto& name = elt.first; const auto& def = elt.second; + std::string work_dir = def.map_working_directory ? "map" : "default"; fmt::format_to(buf, row_format, name, alias_width, def.instance, instance_width, def.command, command_width, - def.map_working_directory); + work_dir); } return fmt::to_string(buf); diff --git a/tests/test_alias_dict.cpp b/tests/test_alias_dict.cpp index f258404b96..a981e97502 100644 --- a/tests/test_alias_dict.cpp +++ b/tests/test_alias_dict.cpp @@ -286,14 +286,14 @@ TEST_P(FormatterTestsuite, table) ASSERT_EQ(mp::YamlFormatter().format(dict), yaml_output); } -const std::string csv_head{"Alias,Instance,Command,Map working directory\n"}; +const std::string csv_head{"Alias,Instance,Command,Working directory\n"}; INSTANTIATE_TEST_SUITE_P(AliasDictionary, FormatterTestsuite, Values(std::make_tuple(AliasesVector{}, csv_head, "{\n \"aliases\": [\n ]\n}\n", "No aliases defined.\n", "aliases: ~\n"), std::make_tuple(AliasesVector{{"lsp", {"primary", "ls", true}}, {"llp", {"primary", "ls", true}}}, - csv_head + "llp,primary,ls,true\nlsp,primary,ls,true\n", + csv_head + "llp,primary,ls,map\nlsp,primary,ls,map\n", "{\n \"aliases\": [\n {\n" " \"alias\": \"llp\",\n" " \"command\": \"ls\",\n" @@ -306,9 +306,9 @@ INSTANTIATE_TEST_SUITE_P(AliasDictionary, FormatterTestsuite, " \"instance\": \"primary\",\n" " \"map-working-directory\": true\n" " }\n ]\n}\n", - "Alias Instance Command Map working directory\n" - "llp primary ls true\n" - "lsp primary ls true\n", + "Alias Instance Command Working directory\n" + "llp primary ls map\n" + "lsp primary ls map\n", "aliases:\n - alias: llp\n command: ls\n instance: primary\n" " map-working-directory: true\n - alias: lsp\n command: ls\n" " instance: primary\n map-working-directory: true\n"))); @@ -417,9 +417,9 @@ TEST_P(DaemonAliasTestsuite, purge_removes_purged_instance_aliases_and_scripts) INSTANTIATE_TEST_SUITE_P( AliasDictionary, DaemonAliasTestsuite, - Values(std::make_tuple(CmdList{{"delete", "real-zebraphant"}, {"purge"}}, csv_head + "lsp,primary,ls,true\n", + Values(std::make_tuple(CmdList{{"delete", "real-zebraphant"}, {"purge"}}, csv_head + "lsp,primary,ls,map\n", std::vector{"lsz"}, std::vector{}), - std::make_tuple(CmdList{{"delete", "--purge", "real-zebraphant"}}, csv_head + "lsp,primary,ls,true\n", + std::make_tuple(CmdList{{"delete", "--purge", "real-zebraphant"}}, csv_head + "lsp,primary,ls,map\n", std::vector{"lsz"}, std::vector{}), std::make_tuple(CmdList{{"delete", "primary"}, {"delete", "primary", "real-zebraphant", "--purge"}}, csv_head, std::vector{"lsp", "lsz"}, std::vector{}), @@ -427,7 +427,7 @@ INSTANTIATE_TEST_SUITE_P( csv_head, std::vector{}, std::vector{"lsp", "lsz"}), std::make_tuple(CmdList{{"delete", "primary"}, {"delete", "primary", "real-zebraphant", "--purge"}}, csv_head, std::vector{"lsp"}, std::vector{"lsz"}), - std::make_tuple(CmdList{{"delete", "real-zebraphant"}, {"purge"}}, csv_head + "lsp,primary,ls,true\n", + std::make_tuple(CmdList{{"delete", "real-zebraphant"}, {"purge"}}, csv_head + "lsp,primary,ls,map\n", std::vector{}, std::vector{"lsz"}), std::make_tuple(CmdList{{"delete", "real-zebraphant", "primary"}, {"purge"}}, csv_head, std::vector{}, std::vector{"lsz", "lsp"}))); diff --git a/tests/test_cli_client.cpp b/tests/test_cli_client.cpp index d98d5d8120..dc0f4f9252 100644 --- a/tests/test_cli_client.cpp +++ b/tests/test_cli_client.cpp @@ -338,7 +338,7 @@ auto make_info_function(const std::string& source_path = "", const std::string& typedef std::vector> AliasesVector; -const std::string csv_header{"Alias,Instance,Command,Map working directory\n"}; +const std::string csv_header{"Alias,Instance,Command,Working directory\n"}; // Tests for no postional args given TEST_F(Client, no_command_is_error) @@ -2837,8 +2837,8 @@ TEST_F(ClientAlias, alias_creates_alias) std::stringstream cout_stream; send_command({"aliases", "--format=csv"}, cout_stream); - EXPECT_THAT(cout_stream.str(), csv_header + "an_alias,an_instance,a_command,true\n" - "another_alias,primary,another_command,false\n"); + EXPECT_THAT(cout_stream.str(), csv_header + "an_alias,an_instance,a_command,map\n" + "another_alias,primary,another_command,default\n"); } struct ClientAliasNameSuite : public ClientAlias, @@ -2861,7 +2861,7 @@ TEST_P(ClientAliasNameSuite, creates_correct_default_alias_name) std::stringstream cout_stream; send_command({"aliases", "--format=csv"}, cout_stream); - EXPECT_THAT(cout_stream.str(), fmt::format(csv_header + "{},primary,{}{},false\n", command, path, command)); + EXPECT_THAT(cout_stream.str(), fmt::format(csv_header + "{},primary,{}{},default\n", command, path, command)); } INSTANTIATE_TEST_SUITE_P(ClientAlias, ClientAliasNameSuite, @@ -2898,7 +2898,7 @@ TEST_F(ClientAlias, alias_does_not_overwrite_alias) std::stringstream cout_stream; send_command({"aliases", "--format=csv"}, cout_stream); - EXPECT_THAT(cout_stream.str(), csv_header + "an_alias,an_instance,a_command,true\n"); + EXPECT_THAT(cout_stream.str(), csv_header + "an_alias,an_instance,a_command,map\n"); } struct ArgumentCheckTestsuite @@ -3029,7 +3029,7 @@ TEST_F(ClientAlias, alias_refuses_creation_unexisting_instance) send_command({"aliases", "--format=csv"}, cout_stream); - EXPECT_THAT(cout_stream.str(), csv_header + "an_alias,an_instance,a_command,true\n"); + EXPECT_THAT(cout_stream.str(), csv_header + "an_alias,an_instance,a_command,map\n"); } TEST_F(ClientAlias, alias_refuses_creation_rpc_error) @@ -3046,7 +3046,7 @@ TEST_F(ClientAlias, alias_refuses_creation_rpc_error) send_command({"aliases", "--format=csv"}, cout_stream); - EXPECT_THAT(cout_stream.str(), csv_header + "an_alias,an_instance,a_command,true\n"); + EXPECT_THAT(cout_stream.str(), csv_header + "an_alias,an_instance,a_command,map\n"); } TEST_F(ClientAlias, unalias_removes_existing_alias) @@ -3059,7 +3059,7 @@ TEST_F(ClientAlias, unalias_removes_existing_alias) std::stringstream cout_stream; send_command({"aliases", "--format=csv"}, cout_stream); - EXPECT_THAT(cout_stream.str(), csv_header + "an_alias,an_instance,a_command,false\n"); + EXPECT_THAT(cout_stream.str(), csv_header + "an_alias,an_instance,a_command,default\n"); } TEST_F(ClientAlias, unalias_succeeds_even_if_script_cannot_be_removed) @@ -3076,7 +3076,7 @@ TEST_F(ClientAlias, unalias_succeeds_even_if_script_cannot_be_removed) std::stringstream cout_stream; send_command({"aliases", "--format=csv"}, cout_stream); - EXPECT_THAT(cout_stream.str(), csv_header + "an_alias,an_instance,a_command,false\n"); + EXPECT_THAT(cout_stream.str(), csv_header + "an_alias,an_instance,a_command,default\n"); } TEST_F(ClientAlias, unalias_does_not_remove_unexisting_alias) @@ -3094,7 +3094,7 @@ TEST_F(ClientAlias, unalias_does_not_remove_unexisting_alias) EXPECT_EQ(cout_stream.str(), csv_header + - "an_alias,an_instance,a_command,true\nanother_alias,another_instance,another_command,false\n"); + "an_alias,an_instance,a_command,map\nanother_alias,another_instance,another_command,default\n"); } TEST_F(ClientAlias, too_many_unalias_arguments) From dedb765170b260db2534274955e3b86046f87168 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Mon, 20 Jun 2022 11:44:53 -0300 Subject: [PATCH 43/48] [cli] map_working_directory -> working_directory Preparing for moving bool to std::string in AliasDefinition. --- include/multipass/cli/alias_definition.h | 2 +- src/client/cli/cmd/alias.cpp | 4 ++-- src/client/cli/formatter/csv_formatter.cpp | 2 +- src/client/cli/formatter/json_formatter.cpp | 2 +- src/client/cli/formatter/table_formatter.cpp | 2 +- src/client/cli/formatter/yaml_formatter.cpp | 2 +- src/client/common/alias_dict.cpp | 6 +++--- tests/test_alias_dict.cpp | 22 ++++++++++---------- 8 files changed, 21 insertions(+), 21 deletions(-) diff --git a/include/multipass/cli/alias_definition.h b/include/multipass/cli/alias_definition.h index 4eda5c99e1..9d0fae93fa 100644 --- a/include/multipass/cli/alias_definition.h +++ b/include/multipass/cli/alias_definition.h @@ -26,7 +26,7 @@ struct AliasDefinition { std::string instance; std::string command; - bool map_working_directory; + bool working_directory; }; inline bool operator==(const AliasDefinition& a, const AliasDefinition& b) diff --git a/src/client/cli/cmd/alias.cpp b/src/client/cli/cmd/alias.cpp index 8e96058086..ee816d1a84 100644 --- a/src/client/cli/cmd/alias.cpp +++ b/src/client/cli/cmd/alias.cpp @@ -141,7 +141,7 @@ mp::ParseCode cmd::Alias::parse_args(mp::ArgParser* parser) } auto instance = definition.left(colon_pos).toStdString(); - bool map_working_directory(!parser->isSet(no_alias_dir_mapping_option)); + bool working_directory(!parser->isSet(no_alias_dir_mapping_option)); info_request.mutable_instance_names()->add_instance_name(instance); info_request.set_verbosity_level(0); @@ -180,7 +180,7 @@ mp::ParseCode cmd::Alias::parse_args(mp::ArgParser* parser) return ParseCode::CommandLineError; } - alias_definition = AliasDefinition{instance, command, map_working_directory}; + alias_definition = AliasDefinition{instance, command, working_directory}; return ParseCode::Ok; } diff --git a/src/client/cli/formatter/csv_formatter.cpp b/src/client/cli/formatter/csv_formatter.cpp index 914f671635..0b5f46382f 100644 --- a/src/client/cli/formatter/csv_formatter.cpp +++ b/src/client/cli/formatter/csv_formatter.cpp @@ -122,7 +122,7 @@ std::string mp::CSVFormatter::format(const mp::AliasDict& aliases) const { const auto& name = elt.first; const auto& def = elt.second; - std::string work_dir = def.map_working_directory ? "map" : "default"; + std::string work_dir = def.working_directory ? "map" : "default"; fmt::format_to(buf, "{},{},{},{}\n", name, def.instance, def.command, work_dir); } diff --git a/src/client/cli/formatter/json_formatter.cpp b/src/client/cli/formatter/json_formatter.cpp index ecf2dc0cb6..9040335902 100644 --- a/src/client/cli/formatter/json_formatter.cpp +++ b/src/client/cli/formatter/json_formatter.cpp @@ -237,7 +237,7 @@ std::string mp::JsonFormatter::format(const mp::AliasDict& aliases) const alias_obj.insert("alias", QString::fromStdString(alias)); alias_obj.insert("instance", QString::fromStdString(def.instance)); alias_obj.insert("command", QString::fromStdString(def.command)); - alias_obj.insert("map-working-directory", def.map_working_directory); + alias_obj.insert("working-directory", def.working_directory); aliases_array.append(alias_obj); } diff --git a/src/client/cli/formatter/table_formatter.cpp b/src/client/cli/formatter/table_formatter.cpp index b825b9a5f1..18de2d8155 100644 --- a/src/client/cli/formatter/table_formatter.cpp +++ b/src/client/cli/formatter/table_formatter.cpp @@ -268,7 +268,7 @@ std::string mp::TableFormatter::format(const mp::AliasDict& aliases) const { const auto& name = elt.first; const auto& def = elt.second; - std::string work_dir = def.map_working_directory ? "map" : "default"; + std::string work_dir = def.working_directory ? "map" : "default"; fmt::format_to(buf, row_format, name, alias_width, def.instance, instance_width, def.command, command_width, work_dir); diff --git a/src/client/cli/formatter/yaml_formatter.cpp b/src/client/cli/formatter/yaml_formatter.cpp index 90761a8472..03db28c04a 100644 --- a/src/client/cli/formatter/yaml_formatter.cpp +++ b/src/client/cli/formatter/yaml_formatter.cpp @@ -225,7 +225,7 @@ std::string mp::YamlFormatter::format(const mp::AliasDict& aliases) const alias_node["alias"] = alias; alias_node["command"] = def.command; alias_node["instance"] = def.instance; - alias_node["map-working-directory"] = def.map_working_directory; + alias_node["working-directory"] = def.working_directory; aliases_node.push_back(alias_node); } diff --git a/src/client/common/alias_dict.cpp b/src/client/common/alias_dict.cpp index a17bfa0274..dc5cd2fe44 100644 --- a/src/client/common/alias_dict.cpp +++ b/src/client/common/alias_dict.cpp @@ -142,9 +142,9 @@ void mp::AliasDict::load_dict() auto instance = record["instance"].toString().toStdString(); auto command = record["command"].toString().toStdString(); - auto map_working_directory = record["map-working-directory"].toBool(); // Defaults to false if not present. + auto working_directory = record["working-directory"].toBool(); // Defaults to false if not present. - aliases.emplace(alias, mp::AliasDefinition{instance, command, map_working_directory}); + aliases.emplace(alias, mp::AliasDefinition{instance, command, working_directory}); } db_file.close(); @@ -156,7 +156,7 @@ void mp::AliasDict::save_dict() QJsonObject json; json.insert("instance", QString::fromStdString(alias.instance)); json.insert("command", QString::fromStdString(alias.command)); - json.insert("map-working-directory", alias.map_working_directory); + json.insert("working-directory", alias.working_directory); return json; }; diff --git a/tests/test_alias_dict.cpp b/tests/test_alias_dict.cpp index a981e97502..1da01fc9ff 100644 --- a/tests/test_alias_dict.cpp +++ b/tests/test_alias_dict.cpp @@ -95,14 +95,14 @@ TEST_F(AliasDictionary, skips_correctly_broken_entries) " \"alias1\": {\n" " \"command\": \"first_command\",\n" " \"instance\": \"first_instance\",\n" - " \"map-working-directory\": true\n" + " \"working-directory\": true\n" " },\n" " \"empty_entry\": {\n" " },\n" " \"alias2\": {\n" " \"command\": \"second_command\",\n" " \"instance\": \"second_instance\",\n" - " \"map-working-directory\": false\n" + " \"working-directory\": false\n" " }\n" "}\n"}; @@ -118,17 +118,17 @@ TEST_F(AliasDictionary, skips_correctly_broken_entries) ASSERT_TRUE(a1); ASSERT_EQ(a1->instance, "first_instance"); ASSERT_EQ(a1->command, "first_command"); - ASSERT_EQ(a1->map_working_directory, true); + ASSERT_EQ(a1->working_directory, true); auto a2 = dict.get_alias("alias2"); ASSERT_TRUE(a2); ASSERT_EQ(a2->instance, "second_instance"); ASSERT_EQ(a2->command, "second_command"); - ASSERT_EQ(a2->map_working_directory, false); + ASSERT_EQ(a2->working_directory, false); } -// In old versions, the file did not contain the `map-working-directory` flag in the JSON, because the -// `--no-map-working-directory` flag were not yet introduced. In case the file was generated by an old version, +// In old versions, the file did not contain the `working-directory` flag in the JSON, because the +// `--no-working-directory` flag were not yet introduced. In case the file was generated by an old version, // the flag must be `false`, in order to maintain backwards compatibility. TEST_F(AliasDictionary, mapDirInOldFilesDefaultsToFalse) { @@ -149,7 +149,7 @@ TEST_F(AliasDictionary, mapDirInOldFilesDefaultsToFalse) ASSERT_TRUE(a3); ASSERT_EQ(a3->instance, "third_instance"); ASSERT_EQ(a3->command, "third_command"); - ASSERT_EQ(a3->map_working_directory, false); + ASSERT_EQ(a3->working_directory, false); } typedef std::vector> AliasesVector; @@ -298,20 +298,20 @@ INSTANTIATE_TEST_SUITE_P(AliasDictionary, FormatterTestsuite, " \"alias\": \"llp\",\n" " \"command\": \"ls\",\n" " \"instance\": \"primary\",\n" - " \"map-working-directory\": true\n" + " \"working-directory\": true\n" " },\n" " {\n" " \"alias\": \"lsp\",\n" " \"command\": \"ls\",\n" " \"instance\": \"primary\",\n" - " \"map-working-directory\": true\n" + " \"working-directory\": true\n" " }\n ]\n}\n", "Alias Instance Command Working directory\n" "llp primary ls map\n" "lsp primary ls map\n", "aliases:\n - alias: llp\n command: ls\n instance: primary\n" - " map-working-directory: true\n - alias: lsp\n command: ls\n" - " instance: primary\n map-working-directory: true\n"))); + " working-directory: true\n - alias: lsp\n command: ls\n" + " instance: primary\n working-directory: true\n"))); struct RemoveInstanceTestsuite : public AliasDictionary, public WithParamInterface>> From d7a06ee5472156a01f47301c8f95034d3ab0a5a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Tue, 21 Jun 2022 11:30:08 -0300 Subject: [PATCH 44/48] [cli] stringify AliasDefinition::working_directory It was a boolean, we make it a string to allow for more values in the future. --- include/multipass/cli/alias_definition.h | 2 +- src/client/cli/cmd/alias.cpp | 2 +- src/client/cli/formatter/csv_formatter.cpp | 3 +- src/client/cli/formatter/json_formatter.cpp | 2 +- src/client/cli/formatter/table_formatter.cpp | 3 +- src/client/common/alias_dict.cpp | 25 ++++++- tests/linux/test_platform_linux.cpp | 12 +-- tests/test_alias_dict.cpp | 77 +++++++++++++------- tests/test_argparser.cpp | 2 +- tests/test_cli_client.cpp | 34 ++++----- 10 files changed, 102 insertions(+), 60 deletions(-) diff --git a/include/multipass/cli/alias_definition.h b/include/multipass/cli/alias_definition.h index 9d0fae93fa..9cee0d278d 100644 --- a/include/multipass/cli/alias_definition.h +++ b/include/multipass/cli/alias_definition.h @@ -26,7 +26,7 @@ struct AliasDefinition { std::string instance; std::string command; - bool working_directory; + std::string working_directory; }; inline bool operator==(const AliasDefinition& a, const AliasDefinition& b) diff --git a/src/client/cli/cmd/alias.cpp b/src/client/cli/cmd/alias.cpp index ee816d1a84..10597a30d5 100644 --- a/src/client/cli/cmd/alias.cpp +++ b/src/client/cli/cmd/alias.cpp @@ -141,7 +141,7 @@ mp::ParseCode cmd::Alias::parse_args(mp::ArgParser* parser) } auto instance = definition.left(colon_pos).toStdString(); - bool working_directory(!parser->isSet(no_alias_dir_mapping_option)); + std::string working_directory = parser->isSet(no_alias_dir_mapping_option) ? "default" : "map"; info_request.mutable_instance_names()->add_instance_name(instance); info_request.set_verbosity_level(0); diff --git a/src/client/cli/formatter/csv_formatter.cpp b/src/client/cli/formatter/csv_formatter.cpp index 0b5f46382f..71fcd0b1bd 100644 --- a/src/client/cli/formatter/csv_formatter.cpp +++ b/src/client/cli/formatter/csv_formatter.cpp @@ -122,9 +122,8 @@ std::string mp::CSVFormatter::format(const mp::AliasDict& aliases) const { const auto& name = elt.first; const auto& def = elt.second; - std::string work_dir = def.working_directory ? "map" : "default"; - fmt::format_to(buf, "{},{},{},{}\n", name, def.instance, def.command, work_dir); + fmt::format_to(buf, "{},{},{},{}\n", name, def.instance, def.command, def.working_directory); } return fmt::to_string(buf); diff --git a/src/client/cli/formatter/json_formatter.cpp b/src/client/cli/formatter/json_formatter.cpp index 9040335902..bc40f5e893 100644 --- a/src/client/cli/formatter/json_formatter.cpp +++ b/src/client/cli/formatter/json_formatter.cpp @@ -237,7 +237,7 @@ std::string mp::JsonFormatter::format(const mp::AliasDict& aliases) const alias_obj.insert("alias", QString::fromStdString(alias)); alias_obj.insert("instance", QString::fromStdString(def.instance)); alias_obj.insert("command", QString::fromStdString(def.command)); - alias_obj.insert("working-directory", def.working_directory); + alias_obj.insert("working-directory", QString::fromStdString(def.working_directory)); aliases_array.append(alias_obj); } diff --git a/src/client/cli/formatter/table_formatter.cpp b/src/client/cli/formatter/table_formatter.cpp index 18de2d8155..14100e0e33 100644 --- a/src/client/cli/formatter/table_formatter.cpp +++ b/src/client/cli/formatter/table_formatter.cpp @@ -268,10 +268,9 @@ std::string mp::TableFormatter::format(const mp::AliasDict& aliases) const { const auto& name = elt.first; const auto& def = elt.second; - std::string work_dir = def.working_directory ? "map" : "default"; fmt::format_to(buf, row_format, name, alias_width, def.instance, instance_width, def.command, command_width, - work_dir); + def.working_directory); } return fmt::to_string(buf); diff --git a/src/client/common/alias_dict.cpp b/src/client/common/alias_dict.cpp index dc5cd2fe44..2f9da9d0f8 100644 --- a/src/client/common/alias_dict.cpp +++ b/src/client/common/alias_dict.cpp @@ -34,6 +34,15 @@ namespace mp = multipass; namespace mpu = multipass::utils; +namespace +{ +void check_working_directory_string(const std::string& dir) +{ + if (dir != "map" && dir != "default") + throw std::runtime_error(fmt::format("invalid working_directory string \"{}\"", dir)); +} +} // namespace + mp::AliasDict::AliasDict(mp::Terminal* term) : cout(term->cout()), cerr(term->cerr()) { const auto file_name = QStringLiteral("%1_aliases.json").arg(mp::client_name); @@ -142,7 +151,16 @@ void mp::AliasDict::load_dict() auto instance = record["instance"].toString().toStdString(); auto command = record["command"].toString().toStdString(); - auto working_directory = record["working-directory"].toBool(); // Defaults to false if not present. + + auto read_working_directory = record["working-directory"]; + std::string working_directory; + + if (read_working_directory.isString() && !read_working_directory.toString().isEmpty()) + working_directory = read_working_directory.toString().toStdString(); + else + working_directory = "default"; + + check_working_directory_string(working_directory); aliases.emplace(alias, mp::AliasDefinition{instance, command, working_directory}); } @@ -154,9 +172,12 @@ void mp::AliasDict::save_dict() { auto alias_to_json = [](const mp::AliasDefinition& alias) -> QJsonObject { QJsonObject json; + + check_working_directory_string(alias.working_directory); + json.insert("instance", QString::fromStdString(alias.instance)); json.insert("command", QString::fromStdString(alias.command)); - json.insert("working-directory", alias.working_directory); + json.insert("working-directory", QString::fromStdString(alias.working_directory)); return json; }; diff --git a/tests/linux/test_platform_linux.cpp b/tests/linux/test_platform_linux.cpp index b1c2d5636b..5aff4a745f 100644 --- a/tests/linux/test_platform_linux.cpp +++ b/tests/linux/test_platform_linux.cpp @@ -763,7 +763,7 @@ TEST_F(PlatformLinux, create_alias_script_works_unconfined) EXPECT_CALL(mpt::MockStandardPaths::mock_instance(), writableLocation(mp::StandardPaths::AppLocalDataLocation)) .WillOnce(Return(tmp_dir.path())); - EXPECT_NO_THROW(MP_PLATFORM.create_alias_script("alias_name", mp::AliasDefinition{"instance", "command", true})); + EXPECT_NO_THROW(MP_PLATFORM.create_alias_script("alias_name", mp::AliasDefinition{"instance", "command", "map"})); QFile checked_script(tmp_dir.path() + "/bin/alias_name"); checked_script.open(QFile::ReadOnly); @@ -788,7 +788,7 @@ TEST_F(PlatformLinux, create_alias_script_works_confined) qputenv("SNAP_NAME", QByteArray{"multipass"}); qputenv("SNAP_USER_COMMON", tmp_dir.path().toUtf8()); - EXPECT_NO_THROW(MP_PLATFORM.create_alias_script("alias_name", mp::AliasDefinition{"instance", "command", true})); + EXPECT_NO_THROW(MP_PLATFORM.create_alias_script("alias_name", mp::AliasDefinition{"instance", "command", "map"})); QFile checked_script(tmp_dir.path() + "/bin/alias_name"); checked_script.open(QFile::ReadOnly); @@ -817,7 +817,7 @@ TEST_F(PlatformLinux, create_alias_script_overwrites) EXPECT_CALL(*mock_file_ops, setPermissions(_, _)).WillOnce(Return(true)); EXPECT_NO_THROW( - MP_PLATFORM.create_alias_script("alias_name", mp::AliasDefinition{"instance", "other_command", true})); + MP_PLATFORM.create_alias_script("alias_name", mp::AliasDefinition{"instance", "other_command", "map"})); } TEST_F(PlatformLinux, create_alias_script_throws_if_cannot_create_path) @@ -827,7 +827,7 @@ TEST_F(PlatformLinux, create_alias_script_throws_if_cannot_create_path) EXPECT_CALL(*mock_file_ops, mkpath(_, _)).WillOnce(Return(false)); MP_EXPECT_THROW_THAT( - MP_PLATFORM.create_alias_script("alias_name", mp::AliasDefinition{"instance", "command", true}), + MP_PLATFORM.create_alias_script("alias_name", mp::AliasDefinition{"instance", "command", "map"}), std::runtime_error, mpt::match_what(HasSubstr("failed to create dir '"))); } @@ -840,7 +840,7 @@ TEST_F(PlatformLinux, create_alias_script_throws_if_cannot_write_script) EXPECT_CALL(*mock_file_ops, write(_, _, _)).WillOnce(Return(747)); MP_EXPECT_THROW_THAT( - MP_PLATFORM.create_alias_script("alias_name", mp::AliasDefinition{"instance", "command", true}), + MP_PLATFORM.create_alias_script("alias_name", mp::AliasDefinition{"instance", "command", "map"}), std::runtime_error, mpt::match_what(HasSubstr("failed to write to file '"))); } @@ -854,7 +854,7 @@ TEST_F(PlatformLinux, create_alias_script_throws_if_cannot_set_permissions) EXPECT_CALL(*mock_file_ops, setPermissions(_, _)).WillOnce(Return(false)); MP_EXPECT_THROW_THAT( - MP_PLATFORM.create_alias_script("alias_name", mp::AliasDefinition{"instance", "command", true}), + MP_PLATFORM.create_alias_script("alias_name", mp::AliasDefinition{"instance", "command", "map"}), std::runtime_error, mpt::match_what(HasSubstr("cannot set permissions to alias script '"))); } diff --git a/tests/test_alias_dict.cpp b/tests/test_alias_dict.cpp index 1da01fc9ff..6570ca3add 100644 --- a/tests/test_alias_dict.cpp +++ b/tests/test_alias_dict.cpp @@ -95,14 +95,14 @@ TEST_F(AliasDictionary, skips_correctly_broken_entries) " \"alias1\": {\n" " \"command\": \"first_command\",\n" " \"instance\": \"first_instance\",\n" - " \"working-directory\": true\n" + " \"working-directory\": \"map\"\n" " },\n" " \"empty_entry\": {\n" " },\n" " \"alias2\": {\n" " \"command\": \"second_command\",\n" " \"instance\": \"second_instance\",\n" - " \"working-directory\": false\n" + " \"working-directory\": \"default\"\n" " }\n" "}\n"}; @@ -118,19 +118,19 @@ TEST_F(AliasDictionary, skips_correctly_broken_entries) ASSERT_TRUE(a1); ASSERT_EQ(a1->instance, "first_instance"); ASSERT_EQ(a1->command, "first_command"); - ASSERT_EQ(a1->working_directory, true); + ASSERT_EQ(a1->working_directory, "map"); auto a2 = dict.get_alias("alias2"); ASSERT_TRUE(a2); ASSERT_EQ(a2->instance, "second_instance"); ASSERT_EQ(a2->command, "second_command"); - ASSERT_EQ(a2->working_directory, false); + ASSERT_EQ(a2->working_directory, "default"); } // In old versions, the file did not contain the `working-directory` flag in the JSON, because the // `--no-working-directory` flag were not yet introduced. In case the file was generated by an old version, // the flag must be `false`, in order to maintain backwards compatibility. -TEST_F(AliasDictionary, mapDirInOldFilesDefaultsToFalse) +TEST_F(AliasDictionary, mapDirMissingTraslatesToDefault) { std::string file_contents{"{\n" " \"alias3\": {\n" @@ -149,7 +149,30 @@ TEST_F(AliasDictionary, mapDirInOldFilesDefaultsToFalse) ASSERT_TRUE(a3); ASSERT_EQ(a3->instance, "third_instance"); ASSERT_EQ(a3->command, "third_command"); - ASSERT_EQ(a3->working_directory, false); + ASSERT_EQ(a3->working_directory, "default"); +} + +TEST_F(AliasDictionary, mapDirEmptyStringTraslatesToDefault) +{ + std::string file_contents{"{\n" + " \"alias4\": {\n" + " \"command\": \"fourth_command\",\n" + " \"instance\": \"fourth_instance\",\n" + " \"working-directory\": \"\"\n" + " }\n" + "}\n"}; + + mpt::make_file_with_content(QString::fromStdString(db_filename()), file_contents); + + std::stringstream trash_stream; + mpt::StubTerminal trash_term(trash_stream, trash_stream, trash_stream); + mp::AliasDict dict(&trash_term); + + auto a4 = dict.get_alias("alias4"); + ASSERT_TRUE(a4); + ASSERT_EQ(a4->instance, "fourth_instance"); + ASSERT_EQ(a4->command, "fourth_command"); + ASSERT_EQ(a4->working_directory, "default"); } typedef std::vector> AliasesVector; @@ -183,9 +206,9 @@ TEST_P(WriteReadTestsuite, writes_and_reads_files) } INSTANTIATE_TEST_SUITE_P(AliasDictionary, WriteReadTestsuite, - Values(AliasesVector{}, AliasesVector{{"w", {"fake", "w", true}}}, - AliasesVector{{"ipf", {"instance", "ip", true}}}, - AliasesVector{{"lsp", {"primary", "ls", true}}, {"llp", {"primary", "ls", true}}})); + Values(AliasesVector{}, AliasesVector{{"w", {"fake", "w", "map"}}}, + AliasesVector{{"ipf", {"instance", "ip", "map"}}}, + AliasesVector{{"lsp", {"primary", "ls", "map"}}, {"llp", {"primary", "ls", "map"}}})); TEST_F(AliasDictionary, correctly_removes_alias) { @@ -193,7 +216,7 @@ TEST_F(AliasDictionary, correctly_removes_alias) mpt::StubTerminal trash_term(trash_stream, trash_stream, trash_stream); mp::AliasDict dict(&trash_term); - dict.add_alias("alias", mp::AliasDefinition{"instance", "command", true}); + dict.add_alias("alias", mp::AliasDefinition{"instance", "command", "map"}); ASSERT_FALSE(dict.empty()); ASSERT_TRUE(dict.remove_alias("alias")); @@ -206,7 +229,7 @@ TEST_F(AliasDictionary, works_when_removing_unexisting_alias) mpt::StubTerminal trash_term(trash_stream, trash_stream, trash_stream); mp::AliasDict dict(&trash_term); - dict.add_alias("alias", mp::AliasDefinition{"instance", "command", true}); + dict.add_alias("alias", mp::AliasDefinition{"instance", "command", "map"}); ASSERT_FALSE(dict.empty()); ASSERT_FALSE(dict.remove_alias("unexisting")); @@ -220,7 +243,7 @@ TEST_F(AliasDictionary, correctly_gets_alias) mp::AliasDict dict(&trash_term); std::string alias_name{"alias"}; - mp::AliasDefinition alias_def{"instance", "command", true}; + mp::AliasDefinition alias_def{"instance", "command", "map"}; dict.add_alias(alias_name, alias_def); ASSERT_FALSE(dict.empty()); @@ -241,12 +264,12 @@ TEST_F(AliasDictionary, get_unexisting_alias_returns_nullopt) TEST_F(AliasDictionary, creates_backup_db) { - populate_db_file(AliasesVector{{"some_alias", {"some_instance", "some_command", true}}}); + populate_db_file(AliasesVector{{"some_alias", {"some_instance", "some_command", "map"}}}); QString bak_filename = QString::fromStdString(db_filename() + ".bak"); ASSERT_FALSE(QFile::exists(bak_filename)); - populate_db_file(AliasesVector{{"another_alias", {"an_instance", "a_command", true}}}); + populate_db_file(AliasesVector{{"another_alias", {"an_instance", "a_command", "map"}}}); ASSERT_TRUE(QFile::exists(bak_filename)); } @@ -291,27 +314,27 @@ const std::string csv_head{"Alias,Instance,Command,Working directory\n"}; INSTANTIATE_TEST_SUITE_P(AliasDictionary, FormatterTestsuite, Values(std::make_tuple(AliasesVector{}, csv_head, "{\n \"aliases\": [\n ]\n}\n", "No aliases defined.\n", "aliases: ~\n"), - std::make_tuple(AliasesVector{{"lsp", {"primary", "ls", true}}, - {"llp", {"primary", "ls", true}}}, + std::make_tuple(AliasesVector{{"lsp", {"primary", "ls", "map"}}, + {"llp", {"primary", "ls", "map"}}}, csv_head + "llp,primary,ls,map\nlsp,primary,ls,map\n", "{\n \"aliases\": [\n {\n" " \"alias\": \"llp\",\n" " \"command\": \"ls\",\n" " \"instance\": \"primary\",\n" - " \"working-directory\": true\n" + " \"working-directory\": \"map\"\n" " },\n" " {\n" " \"alias\": \"lsp\",\n" " \"command\": \"ls\",\n" " \"instance\": \"primary\",\n" - " \"working-directory\": true\n" + " \"working-directory\": \"map\"\n" " }\n ]\n}\n", "Alias Instance Command Working directory\n" "llp primary ls map\n" "lsp primary ls map\n", "aliases:\n - alias: llp\n command: ls\n instance: primary\n" - " working-directory: true\n - alias: lsp\n command: ls\n" - " instance: primary\n working-directory: true\n"))); + " working-directory: map\n - alias: lsp\n command: ls\n" + " instance: primary\n working-directory: map\n"))); struct RemoveInstanceTestsuite : public AliasDictionary, public WithParamInterface>> @@ -338,13 +361,13 @@ TEST_P(RemoveInstanceTestsuite, removes_instance_aliases) INSTANTIATE_TEST_SUITE_P( AliasDictionary, RemoveInstanceTestsuite, - Values(std::make_pair(AliasesVector{{"some_alias", {"instance_to_remove", "some_command", true}}, - {"other_alias", {"other_instance", "other_command", true}}, - {"another_alias", {"instance_to_remove", "another_command", true}}, - {"yet_another_alias", {"yet_another_instance", "yet_another_command", true}}}, + Values(std::make_pair(AliasesVector{{"some_alias", {"instance_to_remove", "some_command", "map"}}, + {"other_alias", {"other_instance", "other_command", "map"}}, + {"another_alias", {"instance_to_remove", "another_command", "map"}}, + {"yet_another_alias", {"yet_another_instance", "yet_another_command", "map"}}}, std::vector{"other_alias", "yet_another_alias"}), - std::make_pair(AliasesVector{{"alias", {"instance", "command", true}}}, std::vector{"alias"}), - std::make_pair(AliasesVector{{"alias", {"instance_to_remove", "command", true}}}, + std::make_pair(AliasesVector{{"alias", {"instance", "command", "map"}}}, std::vector{"alias"}), + std::make_pair(AliasesVector{{"alias", {"instance_to_remove", "command", "map"}}}, std::vector{}))); typedef std::vector> CmdList; @@ -381,7 +404,7 @@ TEST_P(DaemonAliasTestsuite, purge_removes_purged_instance_aliases_and_scripts) std::string json_contents = make_instance_json(mp::nullopt, {}, {"primary"}); - AliasesVector fake_aliases{{"lsp", {"primary", "ls", true}}, {"lsz", {"real-zebraphant", "ls", true}}}; + AliasesVector fake_aliases{{"lsp", {"primary", "ls", "map"}}, {"lsz", {"real-zebraphant", "ls", "map"}}}; populate_db_file(fake_aliases); diff --git a/tests/test_argparser.cpp b/tests/test_argparser.cpp index 21e0800ba3..80138a1217 100644 --- a/tests/test_argparser.cpp +++ b/tests/test_argparser.cpp @@ -77,7 +77,7 @@ TEST_P(TestAliasArguments, test_alias_arguments) const auto& [pre, post] = GetParam(); - populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command", true}}}); + populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command", "map"}}}); mp::AliasDict alias_dict(&term); auto parser = mp::ArgParser{pre, cmds, oss, oss}; diff --git a/tests/test_cli_client.cpp b/tests/test_cli_client.cpp index dc0f4f9252..314f3e4950 100644 --- a/tests/test_cli_client.cpp +++ b/tests/test_cli_client.cpp @@ -2829,7 +2829,7 @@ TEST_F(ClientAlias, alias_creates_alias) { EXPECT_CALL(mock_daemon, info(_, _, _)).Times(AtMost(1)).WillRepeatedly(make_info_function()); - populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command", true}}}); + populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command", "map"}}}); EXPECT_EQ(send_command({"alias", "primary:another_command", "another_alias", "--no-map-working-directory"}), mp::ReturnCode::Ok); @@ -2888,7 +2888,7 @@ TEST_F(ClientAlias, alias_does_not_overwrite_alias) { EXPECT_CALL(mock_daemon, info(_, _, _)).Times(AtMost(1)).WillRepeatedly(make_info_function()); - populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command", true}}}); + populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command", "map"}}}); std::stringstream cerr_stream; EXPECT_EQ(send_command({"alias", "primary:another_command", "an_alias"}, trash_stream, cerr_stream), @@ -2973,7 +2973,7 @@ TEST_F(ClientAlias, too_many_aliases_arguments) TEST_F(ClientAlias, execute_existing_alias) { - populate_db_file(AliasesVector{{"some_alias", {"some_instance", "some_command", true}}}); + populate_db_file(AliasesVector{{"some_alias", {"some_instance", "some_command", "map"}}}); EXPECT_CALL(mock_daemon, info(_, _, _)).WillOnce(make_info_function()); EXPECT_CALL(mock_daemon, ssh_info(_, _, _)); @@ -2983,7 +2983,7 @@ TEST_F(ClientAlias, execute_existing_alias) TEST_F(ClientAlias, execute_unexisting_alias) { - populate_db_file(AliasesVector{{"some_alias", {"some_instance", "some_command", true}}}); + populate_db_file(AliasesVector{{"some_alias", {"some_instance", "some_command", "map"}}}); EXPECT_CALL(mock_daemon, ssh_info(_, _, _)).Times(0); @@ -2994,7 +2994,7 @@ TEST_F(ClientAlias, execute_unexisting_alias) TEST_F(ClientAlias, execute_alias_with_arguments) { - populate_db_file(AliasesVector{{"some_alias", {"some_instance", "some_command", true}}}); + populate_db_file(AliasesVector{{"some_alias", {"some_instance", "some_command", "map"}}}); EXPECT_CALL(mock_daemon, info(_, _, _)).WillOnce(make_info_function()); EXPECT_CALL(mock_daemon, ssh_info(_, _, _)); @@ -3004,7 +3004,7 @@ TEST_F(ClientAlias, execute_alias_with_arguments) TEST_F(ClientAlias, fails_executing_alias_without_separator) { - populate_db_file(AliasesVector{{"some_alias", {"some_instance", "some_command", true}}}); + populate_db_file(AliasesVector{{"some_alias", {"some_instance", "some_command", "map"}}}); EXPECT_CALL(mock_daemon, ssh_info(_, _, _)).Times(0); @@ -3019,7 +3019,7 @@ TEST_F(ClientAlias, alias_refuses_creation_unexisting_instance) { EXPECT_CALL(mock_daemon, info(_, _, _)).Times(AtMost(1)).WillRepeatedly(make_info_function()); - populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command", true}}}); + populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command", "map"}}}); std::stringstream cout_stream, cerr_stream; send_command({"alias", "foo:another_command", "another_alias"}, cout_stream, cerr_stream); @@ -3036,7 +3036,7 @@ TEST_F(ClientAlias, alias_refuses_creation_rpc_error) { EXPECT_CALL(mock_daemon, info(_, _, _)).WillOnce(Return(grpc::Status{grpc::StatusCode::NOT_FOUND, "msg"})); - populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command", true}}}); + populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command", "map"}}}); std::stringstream cout_stream, cerr_stream; send_command({"alias", "foo:another_command", "another_alias"}, cout_stream, cerr_stream); @@ -3051,8 +3051,8 @@ TEST_F(ClientAlias, alias_refuses_creation_rpc_error) TEST_F(ClientAlias, unalias_removes_existing_alias) { - populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command", false}}, - {"another_alias", {"another_instance", "another_command", true}}}); + populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command", "default"}}, + {"another_alias", {"another_instance", "another_command", "map"}}}); EXPECT_EQ(send_command({"unalias", "another_alias"}), mp::ReturnCode::Ok); @@ -3066,8 +3066,8 @@ TEST_F(ClientAlias, unalias_succeeds_even_if_script_cannot_be_removed) { EXPECT_CALL(*mock_platform, remove_alias_script(_)).Times(1).WillRepeatedly(Throw(std::runtime_error("bbb"))); - populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command", false}}, - {"another_alias", {"another_instance", "another_command", true}}}); + populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command", "map"}}, + {"another_alias", {"another_instance", "another_command", "default"}}}); std::stringstream cerr_stream; EXPECT_EQ(send_command({"unalias", "another_alias"}, trash_stream, cerr_stream), mp::ReturnCode::Ok); @@ -3076,13 +3076,13 @@ TEST_F(ClientAlias, unalias_succeeds_even_if_script_cannot_be_removed) std::stringstream cout_stream; send_command({"aliases", "--format=csv"}, cout_stream); - EXPECT_THAT(cout_stream.str(), csv_header + "an_alias,an_instance,a_command,default\n"); + EXPECT_THAT(cout_stream.str(), csv_header + "an_alias,an_instance,a_command,map\n"); } TEST_F(ClientAlias, unalias_does_not_remove_unexisting_alias) { - populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command", true}}, - {"another_alias", {"another_instance", "another_command", false}}}); + populate_db_file(AliasesVector{{"an_alias", {"an_instance", "a_command", "map"}}, + {"another_alias", {"another_instance", "another_command", "default"}}}); std::stringstream cerr_stream; EXPECT_EQ(send_command({"unalias", "unexisting_alias"}, trash_stream, cerr_stream), @@ -3220,7 +3220,7 @@ TEST_F(ClientAlias, execAliasRewritesMountedDir) EXPECT_CALL(mock_daemon, info(_, _, _)).WillOnce(make_info_function(source_dir, target_dir)); - populate_db_file(AliasesVector{{alias_name, {instance_name, cmd, true}}}); + populate_db_file(AliasesVector{{alias_name, {instance_name, cmd, "map"}}}); REPLACE(ssh_channel_request_exec, ([&target_dir, &cmd](ssh_channel, const char* raw_cmd) { EXPECT_THAT(raw_cmd, StartsWith("'cd' '" + target_dir + "/'")); @@ -3264,7 +3264,7 @@ TEST_P(NotDirRewriteTestsuite, execAliasDoesNotRewriteMountedDir) else EXPECT_CALL(mock_daemon, info(_, _, _)).WillOnce(make_info_function(source_dir, target_dir)); - populate_db_file(AliasesVector{{alias_name, {instance_name, cmd, true}}}); + populate_db_file(AliasesVector{{alias_name, {instance_name, cmd, "map"}}}); REPLACE(ssh_channel_request_exec, ([&cmd](ssh_channel, const char* raw_cmd) { EXPECT_THAT(raw_cmd, Not(StartsWith("'cd' '"))); From cb1ed4f0606d0195035d71dab8b5b8a94304368f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Tue, 21 Jun 2022 12:27:16 -0300 Subject: [PATCH 45/48] [tests][cli] Cover missing function. --- tests/test_alias_dict.cpp | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/test_alias_dict.cpp b/tests/test_alias_dict.cpp index 6570ca3add..aa27cb7899 100644 --- a/tests/test_alias_dict.cpp +++ b/tests/test_alias_dict.cpp @@ -175,6 +175,25 @@ TEST_F(AliasDictionary, mapDirEmptyStringTraslatesToDefault) ASSERT_EQ(a4->working_directory, "default"); } +TEST_F(AliasDictionary, mapDirWrongThrows) +{ + std::string file_contents{"{\n" + " \"alias5\": {\n" + " \"command\": \"fifth_command\",\n" + " \"instance\": \"fifth_instance\",\n" + " \"working-directory\": \"wrong string\"\n" + " }\n" + "}\n"}; + + mpt::make_file_with_content(QString::fromStdString(db_filename()), file_contents); + + std::stringstream trash_stream; + mpt::StubTerminal trash_term(trash_stream, trash_stream, trash_stream); + + MP_ASSERT_THROW_THAT(mp::AliasDict dict(&trash_term), std::runtime_error, + mpt::match_what(HasSubstr("invalid working_directory string \"wrong string\""))); +} + typedef std::vector> AliasesVector; struct WriteReadTestsuite : public AliasDictionary, public WithParamInterface From 58f9b1d53b22c78e23213c7066a066b7d8e386cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Tue, 21 Jun 2022 15:22:54 -0300 Subject: [PATCH 46/48] Address second round of @townsend2010 review. --- include/multipass/ssh/ssh_client.h | 2 +- src/client/cli/cmd/alias.cpp | 2 +- src/client/cli/cmd/exec.cpp | 5 ++--- src/ssh/ssh_client.cpp | 8 ++++---- 4 files changed, 8 insertions(+), 9 deletions(-) diff --git a/include/multipass/ssh/ssh_client.h b/include/multipass/ssh/ssh_client.h index c8280c06e7..bb48d6aec0 100644 --- a/include/multipass/ssh/ssh_client.h +++ b/include/multipass/ssh/ssh_client.h @@ -43,7 +43,7 @@ class SSHClient SSHClient(SSHSessionUPtr ssh_session, ConsoleCreator console_creator); int exec(const std::vector& args); - int exec(const std::vector>& argss); + int exec(const std::vector>& args_list); void connect(); private: diff --git a/src/client/cli/cmd/alias.cpp b/src/client/cli/cmd/alias.cpp index 10597a30d5..8839298dd6 100644 --- a/src/client/cli/cmd/alias.cpp +++ b/src/client/cli/cmd/alias.cpp @@ -141,7 +141,7 @@ mp::ParseCode cmd::Alias::parse_args(mp::ArgParser* parser) } auto instance = definition.left(colon_pos).toStdString(); - std::string working_directory = parser->isSet(no_alias_dir_mapping_option) ? "default" : "map"; + auto working_directory = parser->isSet(no_alias_dir_mapping_option) ? "default" : "map"; info_request.mutable_instance_names()->add_instance_name(instance); info_request.set_verbosity_level(0); diff --git a/src/client/cli/cmd/exec.cpp b/src/client/cli/cmd/exec.cpp index 27681a1e05..6bee49dd73 100644 --- a/src/client/cli/cmd/exec.cpp +++ b/src/client/cli/cmd/exec.cpp @@ -19,7 +19,6 @@ #include "common_cli.h" #include -#include #include namespace mp = multipass; @@ -53,13 +52,13 @@ mp::ReturnCode cmd::Exec::run(mp::ArgParser* parser) return parser->returnCodeFrom(ret); } - std::string instance_name = ssh_info_request.instance_name(0); + auto instance_name = ssh_info_request.instance_name(0); std::vector args; for (int i = 1; i < parser->positionalArguments().size(); ++i) args.push_back(parser->positionalArguments().at(i).toStdString()); - std::optional work_dir; + mp::optional work_dir; if (parser->isSet(work_dir_option_name)) { // If the user asked for a working directory, prepend the appropriate `cd`. diff --git a/src/ssh/ssh_client.cpp b/src/ssh/ssh_client.cpp index f4a3ff59bb..2a4ac5192a 100644 --- a/src/ssh/ssh_client.cpp +++ b/src/ssh/ssh_client.cpp @@ -59,15 +59,15 @@ int mp::SSHClient::exec(const std::vector& args) return exec_string(utils::to_cmd(args, mp::utils::QuoteType::quote_every_arg)); } -int mp::SSHClient::exec(const std::vector>& argss) +int mp::SSHClient::exec(const std::vector>& args_list) { std::string cmd_line; - if (argss.size()) + if (args_list.size()) { - auto args_it = argss.begin(); + auto args_it = args_list.begin(); cmd_line = utils::to_cmd(*args_it++, mp::utils::QuoteType::quote_every_arg); - for (; args_it != argss.end(); ++args_it) + for (; args_it != args_list.end(); ++args_it) cmd_line += "&&" + utils::to_cmd(*args_it, mp::utils::QuoteType::quote_every_arg); } From 690286162d3990909b5b2970dd65d1f11bc90cfd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Tue, 21 Jun 2022 15:24:56 -0300 Subject: [PATCH 47/48] [cli] Improve help for the `aliases` command. --- src/client/cli/cmd/aliases.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/client/cli/cmd/aliases.cpp b/src/client/cli/cmd/aliases.cpp index aab3154649..81da60a77f 100644 --- a/src/client/cli/cmd/aliases.cpp +++ b/src/client/cli/cmd/aliases.cpp @@ -56,7 +56,10 @@ QString cmd::Aliases::description() const mp::ParseCode cmd::Aliases::parse_args(mp::ArgParser* parser) { QCommandLineOption formatOption( - "format", "Output list in the requested format.\nValid formats are: table (default), json, csv and yaml", + "format", + "Output list in the requested format. Valid formats are: table (default), json, csv and yaml. " + "The output working directory states whether the alias runs in the instance's default directory " + "or the alias running directory should try to be mapped to a mounted one.\n", "format", "table"); parser->addOption(formatOption); From 5b7ca57b8c1ea1939b115b98276af535fca99805 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Tue, 21 Jun 2022 15:51:43 -0300 Subject: [PATCH 48/48] [cli] Use auto in is_dir_mounted(). --- src/client/cli/cmd/exec.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client/cli/cmd/exec.cpp b/src/client/cli/cmd/exec.cpp index 6bee49dd73..dddfee7303 100644 --- a/src/client/cli/cmd/exec.cpp +++ b/src/client/cli/cmd/exec.cpp @@ -29,9 +29,9 @@ namespace const QString work_dir_option_name{"working-directory"}; const QString no_dir_mapping_option{"no-map-working-directory"}; -bool is_dir_mounted(const QStringList& split_current_dir, const QStringList& split_source_dir) +auto is_dir_mounted(const QStringList& split_current_dir, const QStringList& split_source_dir) { - int source_dir_size = split_source_dir.size(); + auto source_dir_size = split_source_dir.size(); if (split_current_dir.size() < source_dir_size) return false;