From 6dda74d5ccf8bd735033e99cb8c4462f1eb613ab Mon Sep 17 00:00:00 2001 From: Gerry Boland Date: Thu, 21 Feb 2019 16:13:21 +0000 Subject: [PATCH 1/5] Enable passing cloud-init config via stdin --- src/client/cmd/launch.cpp | 25 ++++++++++++++++-- tests/test_cli_client.cpp | 55 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 2 deletions(-) diff --git a/src/client/cmd/launch.cpp b/src/client/cmd/launch.cpp index 4be8707153..04bb57f5c2 100644 --- a/src/client/cmd/launch.cpp +++ b/src/client/cmd/launch.cpp @@ -90,7 +90,8 @@ mp::ParseCode cmd::Launch::parse_args(mp::ArgParser* parser) QCommandLineOption memOption({"m", "mem"}, "Amount of memory to allocate in bytes, or with K, M, G suffix", "mem", "1024"); // In MB's QCommandLineOption nameOption({"n", "name"}, "Name for the instance", "name"); - QCommandLineOption cloudInitOption("cloud-init", "Path to a user-data cloud-init configuration", "file"); + QCommandLineOption cloudInitOption("cloud-init", "Path to a user-data cloud-init configuration, or '-' for stdin", + "file"); parser->addOptions({cpusOption, diskOption, memOption, nameOption, cloudInitOption}); auto status = parser->commandParse(this); @@ -160,7 +161,27 @@ mp::ParseCode cmd::Launch::parse_args(mp::ArgParser* parser) { try { - auto node = YAML::LoadFile(parser->value(cloudInitOption).toStdString()); + YAML::Node node; + const QString& cloudInitFile = parser->value(cloudInitOption); + if (cloudInitFile == "-") + { +#ifdef MULTIPASS_PLATFORM_WINDOWS + _setmode(_fileno(stdin), _O_BINARY); +#endif + QByteArray content; + char arr[1024]; + while (!std::cin.eof()) + { + std::cin.read(arr, sizeof(arr)); + int s = std::cin.gcount(); + content.append(arr, s); + } + node = YAML::Load(content.toStdString()); + } + else + { + node = YAML::LoadFile(cloudInitFile.toStdString()); + } request.set_cloud_init_user_data(YAML::Dump(node)); } catch (const std::exception& e) diff --git a/tests/test_cli_client.cpp b/tests/test_cli_client.cpp index 55fea63fce..4ffa5ac95a 100644 --- a/tests/test_cli_client.cpp +++ b/tests/test_cli_client.cpp @@ -25,6 +25,7 @@ #include #include +#include #include @@ -64,6 +65,21 @@ struct Client : public Test mp::DaemonRpc stub_daemon{server_address, mp::RpcConnectionType::insecure, cert_provider, cert_store}; }; +class CleanerUpper +{ +public: + CleanerUpper(std::function func) : m_func(func) + { + } + ~CleanerUpper() + { + m_func(); + } + +private: + std::function m_func; +}; + // Tests for no postional args given TEST_F(Client, no_command_is_error) { @@ -204,6 +220,45 @@ TEST_F(Client, launch_cmd_custom_image_http_ok) EXPECT_THAT(send_command({"launch", "http://foo"}), Eq(mp::ReturnCode::Ok)); } +TEST_F(Client, launch_cmd_cloudinit_option_with_valid_file_is_ok) +{ + QTemporaryFile tmpfile; // file is auto-deleted when this goes out of scope + tmpfile.open(); + tmpfile.write("password: passw0rd"); // need some YAML + tmpfile.close(); + EXPECT_THAT(send_command({"launch", "--cloud-init", qPrintable(tmpfile.fileName())}), Eq(mp::ReturnCode::Ok)); +} + +TEST_F(Client, launch_cmd_cloudinit_option_with_missing_file) +{ + EXPECT_THAT(send_command({"launch", "--cloud-init", "/definitely/missing-file"}), + Eq(mp::ReturnCode::CommandLineError)); +} + +TEST_F(Client, launch_cmd_cloudinit_option_fails_no_value) +{ + EXPECT_THAT(send_command({"launch", "--cloud-init"}), Eq(mp::ReturnCode::CommandLineError)); +} + +TEST_F(Client, launch_cmd_cloudinit_option_reads_stdin_ok) +{ + QTemporaryFile tmpfile; // file is auto-deleted when this goes out of scope + tmpfile.open(); + tmpfile.write("password: passw0rd"); // need some YAML + tmpfile.close(); + + int stdin_copy = dup(0); // take copy of stdin FD + + freopen(qPrintable(tmpfile.fileName()), "r", stdin); // close stdin FD, reopen backed by tmpfile + + CleanerUpper cleanup([&stdin_copy]() { + dup2(stdin_copy, 0); // restore stdin + close(stdin_copy); + }); + + EXPECT_THAT(send_command({"launch", "--cloud-init", "-"}), Eq(mp::ReturnCode::Ok)); +} + // purge cli tests TEST_F(Client, empty_trash_cmd_ok_no_args) { From 5bee5cbdf7b495529b0e3c123385ef3ba6cf60a0 Mon Sep 17 00:00:00 2001 From: Gerry Boland Date: Fri, 22 Feb 2019 10:05:16 +0000 Subject: [PATCH 2/5] Remove platform specific ifdef --- src/client/cmd/launch.cpp | 4 +--- src/platform/client/client_platform.cpp | 5 +++++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/client/cmd/launch.cpp b/src/client/cmd/launch.cpp index 04bb57f5c2..19d20f0b19 100644 --- a/src/client/cmd/launch.cpp +++ b/src/client/cmd/launch.cpp @@ -165,9 +165,7 @@ mp::ParseCode cmd::Launch::parse_args(mp::ArgParser* parser) const QString& cloudInitFile = parser->value(cloudInitOption); if (cloudInitFile == "-") { -#ifdef MULTIPASS_PLATFORM_WINDOWS - _setmode(_fileno(stdin), _O_BINARY); -#endif + mcp::prepare_stdin_for_read(); QByteArray content; char arr[1024]; while (!std::cin.eof()) diff --git a/src/platform/client/client_platform.cpp b/src/platform/client/client_platform.cpp index dd6b7b7811..8fd93bafd0 100644 --- a/src/platform/client/client_platform.cpp +++ b/src/platform/client/client_platform.cpp @@ -60,3 +60,8 @@ int mcp::getgid() { return ::getgid(); } + +void mcp::prepare_stdin_for_read() +{ + // NO-OP +} From dd3102d0185f302096b7c7bb8d74d46e41d46016 Mon Sep 17 00:00:00 2001 From: Gerry Boland Date: Tue, 26 Feb 2019 11:04:39 +0000 Subject: [PATCH 3/5] Check for tty, move platform specific bits to platform, and mock std::cin better --- src/client/cmd/launch.cpp | 4 ++++ tests/mock_stdcin.h | 48 +++++++++++++++++++++++++++++++++++++++ tests/test_cli_client.cpp | 15 ++---------- 3 files changed, 54 insertions(+), 13 deletions(-) create mode 100644 tests/mock_stdcin.h diff --git a/src/client/cmd/launch.cpp b/src/client/cmd/launch.cpp index 19d20f0b19..adb8f340b6 100644 --- a/src/client/cmd/launch.cpp +++ b/src/client/cmd/launch.cpp @@ -165,6 +165,10 @@ mp::ParseCode cmd::Launch::parse_args(mp::ArgParser* parser) const QString& cloudInitFile = parser->value(cloudInitOption); if (cloudInitFile == "-") { + if (!mcp::is_tty()) + { + throw std::runtime_error("cannot read from stdin without a TTY"); + } mcp::prepare_stdin_for_read(); QByteArray content; char arr[1024]; diff --git a/tests/mock_stdcin.h b/tests/mock_stdcin.h new file mode 100644 index 0000000000..e6450723cc --- /dev/null +++ b/tests/mock_stdcin.h @@ -0,0 +1,48 @@ +/* + * Copyright (C) 2019 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_MOCK_STDCIN_H +#define MULTIPASS_MOCK_STDCIN_H + +#include +#include + +class MockStdCin +{ +public: + MockStdCin(const std::string &s) + { + fake_cin << s; + + // Backup and replace std::cin's streambuffer + cin_backup = std::cin.rdbuf(); + std::streambuf *psbuf = fake_cin.rdbuf(); + std::cin.rdbuf(psbuf); // assign streambuf to cin + } + + ~MockStdCin() + { + // Restore cin's original streanbuffer + std::cin.rdbuf(cin_backup); + } + +private: + std::streambuf *cin_backup; + std::stringstream fake_cin; +}; + +#endif // MULTIPASS_MOCK_STDCIN_H diff --git a/tests/test_cli_client.cpp b/tests/test_cli_client.cpp index 4ffa5ac95a..be3445e5ea 100644 --- a/tests/test_cli_client.cpp +++ b/tests/test_cli_client.cpp @@ -16,6 +16,7 @@ */ #include "path.h" +#include "mock_stdcin.h" #include "stub_cert_store.h" #include "stub_certprovider.h" @@ -242,19 +243,7 @@ TEST_F(Client, launch_cmd_cloudinit_option_fails_no_value) TEST_F(Client, launch_cmd_cloudinit_option_reads_stdin_ok) { - QTemporaryFile tmpfile; // file is auto-deleted when this goes out of scope - tmpfile.open(); - tmpfile.write("password: passw0rd"); // need some YAML - tmpfile.close(); - - int stdin_copy = dup(0); // take copy of stdin FD - - freopen(qPrintable(tmpfile.fileName()), "r", stdin); // close stdin FD, reopen backed by tmpfile - - CleanerUpper cleanup([&stdin_copy]() { - dup2(stdin_copy, 0); // restore stdin - close(stdin_copy); - }); + MockStdCin cin("password: passw0rd"); EXPECT_THAT(send_command({"launch", "--cloud-init", "-"}), Eq(mp::ReturnCode::Ok)); } From b86efe988e3c9ec961926c7968495807cc8a5fc6 Mon Sep 17 00:00:00 2001 From: Gerry Boland Date: Tue, 26 Feb 2019 11:08:38 +0000 Subject: [PATCH 4/5] Remove unneeded CleanerUpper --- tests/test_cli_client.cpp | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/tests/test_cli_client.cpp b/tests/test_cli_client.cpp index be3445e5ea..cbfb87a27c 100644 --- a/tests/test_cli_client.cpp +++ b/tests/test_cli_client.cpp @@ -66,21 +66,6 @@ struct Client : public Test mp::DaemonRpc stub_daemon{server_address, mp::RpcConnectionType::insecure, cert_provider, cert_store}; }; -class CleanerUpper -{ -public: - CleanerUpper(std::function func) : m_func(func) - { - } - ~CleanerUpper() - { - m_func(); - } - -private: - std::function m_func; -}; - // Tests for no postional args given TEST_F(Client, no_command_is_error) { From 5b29daeee254a65f70a635fa6c5df12e039f1bc9 Mon Sep 17 00:00:00 2001 From: Gerry Boland Date: Tue, 26 Feb 2019 11:11:19 +0000 Subject: [PATCH 5/5] Fix formatting, add missing file --- include/multipass/cli/client_platform.h | 1 + tests/mock_stdcin.h | 30 ++++++++++++------------- tests/test_cli_client.cpp | 2 +- 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/include/multipass/cli/client_platform.h b/include/multipass/cli/client_platform.h index 4398054a02..bf5228e976 100644 --- a/include/multipass/cli/client_platform.h +++ b/include/multipass/cli/client_platform.h @@ -33,6 +33,7 @@ void parse_copy_files_entry(const QString& entry, QString& path, QString& instan bool is_tty(); int getuid(); int getgid(); +void prepare_stdin_for_read(); } } } diff --git a/tests/mock_stdcin.h b/tests/mock_stdcin.h index e6450723cc..cb2a2dc8ef 100644 --- a/tests/mock_stdcin.h +++ b/tests/mock_stdcin.h @@ -24,25 +24,25 @@ class MockStdCin { public: - MockStdCin(const std::string &s) - { - fake_cin << s; + MockStdCin(const std::string& s) + { + fake_cin << s; - // Backup and replace std::cin's streambuffer - cin_backup = std::cin.rdbuf(); - std::streambuf *psbuf = fake_cin.rdbuf(); - std::cin.rdbuf(psbuf); // assign streambuf to cin - } + // Backup and replace std::cin's streambuffer + cin_backup = std::cin.rdbuf(); + std::streambuf* psbuf = fake_cin.rdbuf(); + std::cin.rdbuf(psbuf); // assign streambuf to cin + } - ~MockStdCin() - { - // Restore cin's original streanbuffer - std::cin.rdbuf(cin_backup); - } + ~MockStdCin() + { + // Restore cin's original streanbuffer + std::cin.rdbuf(cin_backup); + } private: - std::streambuf *cin_backup; - std::stringstream fake_cin; + std::streambuf* cin_backup; + std::stringstream fake_cin; }; #endif // MULTIPASS_MOCK_STDCIN_H diff --git a/tests/test_cli_client.cpp b/tests/test_cli_client.cpp index cbfb87a27c..7bf411ae8e 100644 --- a/tests/test_cli_client.cpp +++ b/tests/test_cli_client.cpp @@ -15,8 +15,8 @@ * */ -#include "path.h" #include "mock_stdcin.h" +#include "path.h" #include "stub_cert_store.h" #include "stub_certprovider.h"