Merge #2074

2074: [launch] add support for `--bridged` network r=luis4a0,Saviq a=Saviq



Co-authored-by: Michał Sawicz <michal@sawicz.net>

include/multipass/constants.h

@@ -43,6 +43,8 @@ constexpr auto winterm_profile_guid =
 
 constexpr auto petenv_key = "client.primary-name";     // This will eventually be moved to some dynamic settings schema
 constexpr auto driver_key = "local.driver";            // idem
+constexpr auto bridged_interface_key = "local.bridged-network"; // idem
+constexpr auto bridged_network_name = "bridged";
 constexpr auto autostart_key = "client.gui.autostart"; // idem
 constexpr auto winterm_key = "client.apps.windows-terminal.profiles"; // idem
 constexpr auto hotkey_key = "client.gui.hotkey";                      // idem

src/client/cli/cmd/launch.cpp

@@ -206,13 +206,15 @@ mp::ParseCode cmd::Launch::parse_args(mp::ArgParser* parser)
                                      "Add a network interface to the instance, where <spec> is in the "
                                      "\"key=value,key=value\" format, with the following keys available:\n"
                                      "  name: the network to connect to (required), use the networks command for a "
-                                     "list of possible values\n"
+                                     "list of possible values, or use 'bridged' to use the interface configured via "
+                                     "`multipass set local.bridged-network`.\n"
                                      "  mode: auto|manual (default: auto)\n"
                                      "  mac: hardware address (default: random).\n"
                                      "You can also use a shortcut of \"<name>\" to mean \"name=<name>\".",
                                      "spec");
+    QCommandLineOption bridgedOption("bridged", "Adds one `--network bridged` network.");
 
-    parser->addOptions({cpusOption, diskOption, memOption, nameOption, cloudInitOption, networkOption});
+    parser->addOptions({cpusOption, diskOption, memOption, nameOption, cloudInitOption, networkOption, bridgedOption});
 
     mp::cmd::add_timeout(parser);
 
@@ -310,6 +312,11 @@ mp::ParseCode cmd::Launch::parse_args(mp::ArgParser* parser)
         }
     }
 
+    if (parser->isSet(bridgedOption))
+    {
+        request.mutable_network_options()->Add(net_digest(mp::bridged_network_name));
+    }
+
     try
     {
         if (parser->isSet(networkOption))

src/daemon/daemon.cpp

@@ -33,6 +33,7 @@
 #include <multipass/network_interface.h>
 #include <multipass/platform.h>
 #include <multipass/query.h>
+#include <multipass/settings.h>
 #include <multipass/ssh/ssh_session.h>
 #include <multipass/utils.h>
 #include <multipass/version.h>
@@ -367,6 +368,18 @@ std::vector<mp::NetworkInterface> validate_extra_interfaces(const mp::LaunchRequ
 
     for (const auto& net : request->network_options())
     {
+        auto net_id = net.id();
+
+        if (net_id == mp::bridged_network_name)
+        {
+            const auto bridged_id = MP_SETTINGS.get(mp::bridged_interface_key);
+            if (bridged_id == "")
+                throw std::runtime_error(
+                    fmt::format("You have to `multipass set {}=<name>` to use the `--bridged` shortcut.",
+                                mp::bridged_interface_key));
+            net_id = bridged_id.toStdString();
+        }
+
         if (!factory_networks)
         {
             try
@@ -386,12 +399,18 @@ std::vector<mp::NetworkInterface> validate_extra_interfaces(const mp::LaunchRequ
         }
 
         // Check that the id the user specified is valid.
-        auto pred = [net](const mp::NetworkInterfaceInfo& info) { return info.id == net.id(); };
+        auto pred = [net_id](const mp::NetworkInterfaceInfo& info) { return info.id == net_id; };
         auto host_net_it = std::find_if(factory_networks->cbegin(), factory_networks->cend(), pred);
 
         if (host_net_it == factory_networks->cend())
         {
-            mpl::log(mpl::Level::warning, category, fmt::format("Invalid network name \"{}\"", net.id()));
+            if (net.id() == mp::bridged_network_name)
+                throw std::runtime_error(
+                    fmt::format("Invalid network '{}' set as bridged interface, use `multipass set {}=<name>` to "
+                                "correct. See `multipass networks` for valid names.",
+                                net_id, mp::bridged_interface_key));
+
+            mpl::log(mpl::Level::warning, category, fmt::format("Invalid network name \"{}\"", net_id));
             option_errors.add_error_codes(mp::LaunchError::INVALID_NETWORK);
         }
         else if (host_net_it->needs_authorization)
@@ -401,7 +420,7 @@ std::vector<mp::NetworkInterface> validate_extra_interfaces(const mp::LaunchRequ
         if (const auto& mac = QString::fromStdString(net.mac_address()).toLower().toStdString();
             mac.empty() || mpu::valid_mac_address(mac))
             interfaces.push_back(
-                mp::NetworkInterface{net.id(), mac, net.mode() != multipass::LaunchRequest_NetworkOptions_Mode_MANUAL});
+                mp::NetworkInterface{net_id, mac, net.mode() != multipass::LaunchRequest_NetworkOptions_Mode_MANUAL});
         else
         {
             mpl::log(mpl::Level::warning, category, fmt::format("Invalid MAC address \"{}\"", mac));

src/utils/settings.cpp

@@ -53,7 +53,8 @@ std::map<QString, QString> make_defaults()
     auto ret = std::map<QString, QString>{{mp::petenv_key, petenv_name},
                                           {mp::driver_key, mp::platform::default_driver()},
                                           {mp::autostart_key, autostart_default},
-                                          {mp::hotkey_key, default_hotkey()}};
+                                          {mp::hotkey_key, default_hotkey()},
+                                          {mp::bridged_interface_key, ""}};
 
     for(const auto& [k, v] : mp::platform::extra_settings_defaults())
         ret.insert_or_assign(k, v);

tests/test_cli_client.cpp

@@ -722,7 +722,10 @@ INSTANTIATE_TEST_SUITE_P(Client, TestValidNetworkOptions,
                                 std::vector<std::string>{"--network", "name=eth6,mac=01:23:45:67:89:ab"},
                                 std::vector<std::string>{"--network", "name=eth7,mode=manual"},
                                 std::vector<std::string>{"--network", "name=eth8,mode=auto"},
-                                std::vector<std::string>{"--network", "name=eth9", "--network", "name=eth9"}));
+                                std::vector<std::string>{"--network", "name=eth9", "--network", "name=eth9"},
+                                std::vector<std::string>{"--network", "bridged"},
+                                std::vector<std::string>{"--network", "name=bridged"},
+                                std::vector<std::string>{"--bridged"}));
 
 // purge cli tests
 TEST_F(Client, purge_cmd_ok_no_args)
@@ -1650,7 +1653,8 @@ TEST_P(TestBasicGetSetOptions, set_cmd_allows_empty_val)
 }
 
 INSTANTIATE_TEST_SUITE_P(Client, TestBasicGetSetOptions,
-                         Values(mp::petenv_key, mp::driver_key, mp::autostart_key, mp::hotkey_key));
+                         Values(mp::petenv_key, mp::driver_key, mp::autostart_key, mp::hotkey_key,
+                                mp::bridged_interface_key));
 
 TEST_F(Client, get_cmd_fails_with_no_arguments)
 {

tests/test_daemon.cpp

@@ -32,6 +32,7 @@
 #include "mock_logger.h"
 #include "mock_platform.h"
 #include "mock_process_factory.h"
+#include "mock_settings.h"
 #include "mock_utils.h"
 #include "mock_virtual_machine.h"
 #include "mock_vm_image_vault.h"
@@ -107,11 +108,19 @@ struct Daemon : public mpt::DaemonTestFixture
         EXPECT_CALL(*mock_platform, get_workflows_url_override()).WillRepeatedly([] { return QString{}; });
     }
 
+    void SetUp() override
+    {
+        EXPECT_CALL(mock_settings, get(_))
+            .Times(AnyNumber()); /* Admit get calls beyond explicitly expected in tests. */
+    }
+
     mpt::MockUtils::GuardedMock attr{mpt::MockUtils::inject()};
     NiceMock<mpt::MockUtils>* mock_utils = attr.first;
 
     mpt::MockPlatform::GuardedMock platform_attr{mpt::MockPlatform::inject()};
     mpt::MockPlatform* mock_platform = platform_attr.first;
+    mpt::MockSettings& mock_settings = mpt::MockSettings::mock_instance(); /* although this is shared, expectations are
+                                                                              reset at the end of each test */
 };
 
 TEST_F(Daemon, receives_commands)
@@ -1305,4 +1314,47 @@ INSTANTIATE_TEST_SUITE_P(Daemon, DaemonLaunchTimeoutValueTestSuite,
                          Values(std::make_tuple(0, 600, 600), // client_timeout, workflow_timeout, expected_timeout
                                 std::make_tuple(1000, 600, 1000), std::make_tuple(1000, 0, 1000),
                                 std::make_tuple(0, 0, 300)));
+
+TEST_F(Daemon, launches_with_bridged)
+{
+    mpt::MockVirtualMachineFactory* mock_factory = use_a_mock_vm_factory();
+
+    mp::Daemon daemon{config_builder.build()};
+
+    EXPECT_CALL(*mock_factory, networks());
+    EXPECT_CALL(mock_settings, get(Eq(mp::bridged_interface_key))).WillRepeatedly(Return("eth0"));
+
+    ASSERT_NO_THROW(send_command({"launch", "--network", "bridged"}));
+}
+
+TEST_F(Daemon, refuses_launch_bridged_without_setting)
+{
+    mpt::MockVirtualMachineFactory* mock_factory = use_a_mock_vm_factory();
+
+    mp::Daemon daemon{config_builder.build()};
+
+    EXPECT_CALL(*mock_factory, networks()).Times(0);
+
+    std::stringstream err_stream;
+    send_command({"launch", "--network", "bridged"}, std::cout, err_stream);
+    EXPECT_THAT(err_stream.str(),
+                HasSubstr("You have to `multipass set local.bridged-network=<name>` to use the `--bridged` shortcut."));
+}
+
+TEST_F(Daemon, refuses_launch_with_invalid_bridged_interface)
+{
+    mpt::MockVirtualMachineFactory* mock_factory = use_a_mock_vm_factory();
+
+    mp::Daemon daemon{config_builder.build()};
+
+    EXPECT_CALL(*mock_factory, networks());
+    EXPECT_CALL(mock_settings, get(Eq(mp::bridged_interface_key))).WillRepeatedly(Return("invalid"));
+
+    std::stringstream err_stream;
+    send_command({"launch", "--network", "bridged"}, std::cout, err_stream);
+    EXPECT_THAT(err_stream.str(),
+                HasSubstr("Invalid network 'invalid' set as bridged interface, use `multipass set "
+                          "local.bridged-network=<name>` to correct. See `multipass networks` for valid names."));
+}
+
 } // namespace