From cb91676ddb939ff297aba3f0253d2be7f0e9c64b Mon Sep 17 00:00:00 2001 From: trevor-shoe Date: Mon, 22 Sep 2025 08:42:18 -0400 Subject: [PATCH 01/28] [network] Subnet class API --- include/multipass/subnet.h | 59 ++++++++++++++++++++++++++ src/network/CMakeLists.txt | 1 + src/network/subnet.cpp | 84 ++++++++++++++++++++++++++++++++++++++ tests/CMakeLists.txt | 1 + tests/test_subnet.cpp | 23 +++++++++++ 5 files changed, 168 insertions(+) create mode 100644 include/multipass/subnet.h create mode 100644 src/network/subnet.cpp create mode 100644 tests/test_subnet.cpp diff --git a/include/multipass/subnet.h b/include/multipass/subnet.h new file mode 100644 index 0000000000..8d43c59c7e --- /dev/null +++ b/include/multipass/subnet.h @@ -0,0 +1,59 @@ +/* + * Copyright (C) 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 . + * + */ + +#pragma once + +#include +#include + +#include "ip_address.h" + +#define MP_SUBNET_UTILS multipass::SubnetUtils::instance(); + +namespace multipass +{ +class Subnet +{ +public: + Subnet(IPAddress ip, uint8_t cidr); + Subnet(const std::string& cidr_string); + + [[nodiscard]] IPAddress get_min_address() const; + [[nodiscard]] IPAddress get_max_address() const; + [[nodiscard]] uint32_t get_address_count() const; + + [[nodiscard]] IPAddress get_identifier() const; + [[nodiscard]] uint8_t get_CIDR() const; + [[nodiscard]] IPAddress get_subnet_mask() const; + + // uses CIDR notation + [[nodiscard]] std::string as_string() const; + +private: + IPAddress identifier; + uint8_t cidr; +}; + +struct SubnetUtils : Singleton +{ + using Singleton::Singleton; + + [[nodiscard]] virtual Subnet generate_random_subnet(IPAddress start, IPAddress end, uint8_t cidr) const; + [[nodiscard]] virtual Subnet get_subnet(const Path& network_dir, const QString& bridge_name) const; +}; +} // namespace multipass + diff --git a/src/network/CMakeLists.txt b/src/network/CMakeLists.txt index c0e2654da3..d0a4fdd075 100644 --- a/src/network/CMakeLists.txt +++ b/src/network/CMakeLists.txt @@ -15,6 +15,7 @@ set(CMAKE_AUTOMOC ON) add_library(network STATIC + subnet.cpp url_downloader.cpp) add_library(ip_address STATIC diff --git a/src/network/subnet.cpp b/src/network/subnet.cpp new file mode 100644 index 0000000000..e97296f4cc --- /dev/null +++ b/src/network/subnet.cpp @@ -0,0 +1,84 @@ +/* + * Copyright (C) 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 + +namespace mp = multipass; + +namespace +{ +mp::Subnet parse(const std::string& cidr_string) +{ + return mp::Subnet(mp::IPAddress(""), 0); +} + +mp::IPAddress apply_mask(mp::IPAddress ip, uint8_t cidr) +{ + return mp::IPAddress{""}; +} +} + +mp::Subnet::Subnet(IPAddress ip, uint8_t cidr) : identifier(apply_mask(ip, cidr)), cidr(cidr) +{ +} + +mp::Subnet::Subnet(const std::string& cidr_string) : Subnet(parse(cidr_string)) +{ +} + +mp::IPAddress mp::Subnet::get_min_address() const +{ + return mp::IPAddress{""}; +} + +mp::IPAddress mp::Subnet::get_max_address() const +{ + return mp::IPAddress{""}; +} + +uint32_t mp::Subnet::get_address_count() const +{ + return 0; +} + +mp::IPAddress mp::Subnet::get_identifier() const +{ + return mp::IPAddress{""}; +} + +uint8_t mp::Subnet::get_CIDR() const +{ + return 0; +} + +// uses CIDR notation +std::string mp::Subnet::as_string() const +{ + return ""; +} + +mp::Subnet mp::SubnetUtils::generate_random_subnet(IPAddress start, + IPAddress end, + uint8_t cidr) const +{ + return mp::Subnet{""}; +} + +mp::Subnet mp::SubnetUtils::get_subnet(const mp::Path& network_dir, const QString& bridge_name) const +{ + return mp::Subnet{""}; +} diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index bc008147d4..e546deeb9b 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -115,6 +115,7 @@ add_executable(multipass_tests test_sshfsmount.cpp test_sshfs_mount_handler.cpp test_ssl_cert_provider.cpp + test_subnet.cpp test_timer.cpp test_top_catch_all.cpp test_ubuntu_image_host.cpp diff --git a/tests/test_subnet.cpp b/tests/test_subnet.cpp new file mode 100644 index 0000000000..f752b7d700 --- /dev/null +++ b/tests/test_subnet.cpp @@ -0,0 +1,23 @@ +/* + * Copyright (C) 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 + +namespace mp = multipass; +using namespace testing; From f87733d45ca13e5816f25911f6bdce22aa0903e9 Mon Sep 17 00:00:00 2001 From: trevor-shoe Date: Mon, 22 Sep 2025 09:57:19 -0400 Subject: [PATCH 02/28] [network] Add Subnet class tests --- src/network/subnet.cpp | 5 ++ tests/test_subnet.cpp | 144 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 149 insertions(+) diff --git a/src/network/subnet.cpp b/src/network/subnet.cpp index e97296f4cc..ef7cad4d42 100644 --- a/src/network/subnet.cpp +++ b/src/network/subnet.cpp @@ -65,6 +65,11 @@ uint8_t mp::Subnet::get_CIDR() const return 0; } +mp::IPAddress mp::Subnet::get_subnet_mask() const +{ + return mp::IPAddress{""}; +} + // uses CIDR notation std::string mp::Subnet::as_string() const { diff --git a/tests/test_subnet.cpp b/tests/test_subnet.cpp index f752b7d700..1f4d7c2795 100644 --- a/tests/test_subnet.cpp +++ b/tests/test_subnet.cpp @@ -20,4 +20,148 @@ #include namespace mp = multipass; +namespace mpt = mp::test; using namespace testing; + +TEST(Subnet, canInitializeFromIpCidrPair) +{ + mp::Subnet subnet{mp::IPAddress("192.168.0.0"), 24}; + + EXPECT_EQ(subnet.get_identifier(), mp::IPAddress("192.168.0.0")); + EXPECT_EQ(subnet.get_CIDR(), 24); +} + +TEST(Subnet, canInitializeFromString) +{ + mp::Subnet subnet{"192.168.0.0/24"}; + + EXPECT_EQ(subnet.get_identifier(), mp::IPAddress("192.168.0.0")); + EXPECT_EQ(subnet.get_CIDR(), 24); +} + +TEST(Subet, throwsOnInvalidIP) +{ + EXPECT_THROW(mp::Subnet{""}, std::invalid_argument); + EXPECT_THROW(mp::Subnet{"thisisnotanipithinkbuticouldbewrong"}, std::invalid_argument); + + EXPECT_THROW(mp::Subnet{"192.168/16"}, std::invalid_argument); + EXPECT_THROW(mp::Subnet{"/24"}, std::invalid_argument); + EXPECT_THROW(mp::Subnet{"/"}, std::invalid_argument); + + MP_EXPECT_THROW_THAT(mp::Subnet{"192.168.XXX.XXX/16"}, + std::invalid_argument, + mpt::match_what("invalid IP octet")); +} + +TEST(Subnet, throwsOnLargeCIDR) +{ + static constexpr auto what = "CIDR value must be non-negative and less than 31"; + + // valid but not supported + MP_EXPECT_THROW_THAT(mp::Subnet{"192.168.0.0/31"}, + std::invalid_argument, + mpt::match_what(what)); + + // valid but not supported + MP_EXPECT_THROW_THAT(mp::Subnet{"192.168.0.0/32"}, + std::invalid_argument, + mpt::match_what(what)); + + // boundary case + MP_EXPECT_THROW_THAT(mp::Subnet{"192.168.0.0/33"}, + std::invalid_argument, + mpt::match_what(what)); + + // at 8 bit limit + MP_EXPECT_THROW_THAT(mp::Subnet{"192.168.0.0/255"}, + std::invalid_argument, + mpt::match_what(what)); + + // above 8 bit limit + MP_EXPECT_THROW_THAT(mp::Subnet{"192.168.0.0/895231337"}, + std::invalid_argument, + mpt::match_what(what)); + + // extreme case + MP_EXPECT_THROW_THAT(mp::Subnet{"192.168.0.0/895231337890712387952378952359871235987169601436"}, + std::invalid_argument, + mpt::match_what(what)); +} + +TEST(Subnet, throwsOnNegativeCIDR) +{ + static constexpr auto what = "CIDR value must be non-negative and less than 31"; + + MP_EXPECT_THROW_THAT(mp::Subnet{"192.168.0.0/-24"}, + std::invalid_argument, + mpt::match_what(what)); +} + +TEST(Subnet, givesCorrectRange) +{ + mp::Subnet subnet{"192.168.0.0/24"}; + EXPECT_EQ(subnet.get_identifier(), mp::IPAddress{"192.168.0.0"}); + EXPECT_EQ(subnet.get_min_address(), mp::IPAddress{"192.168.0.1"}); + EXPECT_EQ(subnet.get_max_address(), mp::IPAddress{"192.168.0.254"}); + EXPECT_EQ(subnet.get_address_count(), 254); + + subnet = mp::Subnet{"121.212.1.152/11"}; + EXPECT_EQ(subnet.get_identifier(), mp::IPAddress{"121.192.0.0"}); + EXPECT_EQ(subnet.get_min_address(), mp::IPAddress{"121.192.0.1"}); + EXPECT_EQ(subnet.get_max_address(), mp::IPAddress{"121,223.255.254"}); + EXPECT_EQ(subnet.get_address_count(), 2097150); + + subnet = mp::Subnet{"0.0.0.0/0"}; + EXPECT_EQ(subnet.get_identifier(), mp::IPAddress{"0.0.0.0"}); + EXPECT_EQ(subnet.get_min_address(), mp::IPAddress{"0.0.0.1"}); + EXPECT_EQ(subnet.get_max_address(), mp::IPAddress{"255,255.255.254"}); + EXPECT_EQ(subnet.get_address_count(), 4294967294); +} + +TEST(Subnet, convertsToMaskedIP) +{ + mp::Subnet subnet{"192.168.255.52/24"}; + EXPECT_EQ(subnet.get_identifier(), mp::IPAddress{"192.168.255.0"}); + + subnet = mp::Subnet{"255.168.1.152/8"}; + EXPECT_EQ(subnet.get_identifier(), mp::IPAddress{"255.0.0.0"}); + + subnet = mp::Subnet{"192.168.1.152/0"}; + EXPECT_EQ(subnet.get_identifier(), mp::IPAddress{"0.0.0.0"}); + + subnet = mp::Subnet{"255.212.1.152/13"}; + EXPECT_EQ(subnet.get_identifier(), mp::IPAddress{"255.208.0.0"}); +} + +TEST(Subnet, getSubnetMaskReturnsSubnetMask) +{ + mp::Subnet subnet{"192.168.0.1/24"}; + EXPECT_EQ(subnet.get_subnet_mask(), mp::IPAddress("255.255.255.0")); + + subnet = mp::Subnet{"192.168.0.1/21"}; + EXPECT_EQ(subnet.get_subnet_mask(), mp::IPAddress("255.255.248.0")); + + subnet = mp::Subnet{"192.168.0.1/16"}; + EXPECT_EQ(subnet.get_subnet_mask(), mp::IPAddress("255.255.0.0")); + + subnet = mp::Subnet{"192.168.0.1/9"}; + EXPECT_EQ(subnet.get_subnet_mask(), mp::IPAddress("255.128.0.0")); + + subnet = mp::Subnet{"192.168.0.1/4"}; + EXPECT_EQ(subnet.get_subnet_mask(), mp::IPAddress("240.0.0.0")); + + subnet = mp::Subnet{"192.168.0.1/0"}; + EXPECT_EQ(subnet.get_subnet_mask(), mp::IPAddress("0.0.0.0")); +} + +TEST(Subnet, canConvertToString) +{ + mp::Subnet subnet{"192.168.0.1/24"}; + EXPECT_EQ(subnet.as_string(), "192.168.0.0/24"); + + subnet = mp::Subnet{"255.0.255.0/8"}; + EXPECT_EQ(subnet.as_string(), "255.0.0.0/8"); + + subnet = mp::Subnet{"255.0.255.0/0"}; + EXPECT_EQ(subnet.as_string(), "0.0.0.0/8"); +} From 3b0d639b920fcc9b0e32f510d678bfd4f0d76d97 Mon Sep 17 00:00:00 2001 From: trevor-shoe Date: Mon, 22 Sep 2025 11:46:33 -0400 Subject: [PATCH 03/28] [network] Implement Subnet class --- src/network/subnet.cpp | 81 ++++++++++++++++++++++++++++++++++++------ tests/test_subnet.cpp | 18 +++++----- 2 files changed, 80 insertions(+), 19 deletions(-) diff --git a/src/network/subnet.cpp b/src/network/subnet.cpp index ef7cad4d42..4056fefe6c 100644 --- a/src/network/subnet.cpp +++ b/src/network/subnet.cpp @@ -17,23 +17,83 @@ #include +#include + +#include +#include + namespace mp = multipass; namespace { +constexpr auto large_CIDR_err_fmt = "CIDR value must be non-negative and less than 31: {}"; + mp::Subnet parse(const std::string& cidr_string) +try +{ + if (auto i = cidr_string.find('/'); i != std::string::npos) + { + mp::IPAddress addr{cidr_string.substr(0, i)}; + + auto cidr = std::stoul(cidr_string.substr(i + 1)); + if (cidr >= 31) + throw std::invalid_argument(fmt::format(large_CIDR_err_fmt, cidr)); + + return mp::Subnet(addr, cidr); + } + throw std::invalid_argument(fmt::format("CIDR address {} does not contain '/' seperator", cidr_string)); +} +catch (const std::out_of_range& e) { - return mp::Subnet(mp::IPAddress(""), 0); + throw std::invalid_argument(fmt::format(large_CIDR_err_fmt, e.what())); } -mp::IPAddress apply_mask(mp::IPAddress ip, uint8_t cidr) +[[nodiscard]] mp::IPAddress get_subnet_mask(uint8_t cidr) { - return mp::IPAddress{""}; + using Octects = decltype(mp::IPAddress::octets); + static constexpr auto value_size = sizeof(Octects::value_type) * 8; + + Octects octets{}; + std::fill(octets.begin(), octets.end(), std::numeric_limits::max()); + + if (cidr > value_size * octets.size()) + throw std::out_of_range("CIDR too large for address space"); + + const uint8_t start_octet = cidr / value_size; + const uint8_t remain = cidr % value_size; + + for (size_t i = start_octet; i < octets.size(); ++i) + { + octets[i] = 0; + } + + for (size_t i = 0; i < remain; ++ i) + { + octets[start_octet] >>= 1; + octets[start_octet] |= 1 << (value_size - 1); + } + + return mp::IPAddress{octets}; } + +[[nodiscard]] mp::IPAddress apply_mask(mp::IPAddress ip, uint8_t cidr) +{ + const auto mask = get_subnet_mask(cidr); + for (size_t i = 0; i < ip.octets.size(); ++i) + { + ip.octets[i] &= mask.octets[i]; + } + return ip; } +} // namespace mp::Subnet::Subnet(IPAddress ip, uint8_t cidr) : identifier(apply_mask(ip, cidr)), cidr(cidr) { + if (cidr >= 31) + { + throw std::invalid_argument( + fmt::format("CIDR value must be non-negative and less than 31: {}", cidr)); + } } mp::Subnet::Subnet(const std::string& cidr_string) : Subnet(parse(cidr_string)) @@ -42,38 +102,39 @@ mp::Subnet::Subnet(const std::string& cidr_string) : Subnet(parse(cidr_string)) mp::IPAddress mp::Subnet::get_min_address() const { - return mp::IPAddress{""}; + return identifier + 1; } mp::IPAddress mp::Subnet::get_max_address() const { - return mp::IPAddress{""}; + // identifier + 2^(32 - cidr) - 1 - 1 + return identifier + ((1ull << (32ull - cidr)) - 2ull); } uint32_t mp::Subnet::get_address_count() const { - return 0; + return get_max_address().as_uint32() - get_min_address().as_uint32() + 1; } mp::IPAddress mp::Subnet::get_identifier() const { - return mp::IPAddress{""}; + return identifier; } uint8_t mp::Subnet::get_CIDR() const { - return 0; + return cidr; } mp::IPAddress mp::Subnet::get_subnet_mask() const { - return mp::IPAddress{""}; + return ::get_subnet_mask(cidr); } // uses CIDR notation std::string mp::Subnet::as_string() const { - return ""; + return fmt::format("{}/{}", identifier.as_string(), cidr); } mp::Subnet mp::SubnetUtils::generate_random_subnet(IPAddress start, diff --git a/tests/test_subnet.cpp b/tests/test_subnet.cpp index 1f4d7c2795..115c4da3c8 100644 --- a/tests/test_subnet.cpp +++ b/tests/test_subnet.cpp @@ -50,7 +50,7 @@ TEST(Subet, throwsOnInvalidIP) MP_EXPECT_THROW_THAT(mp::Subnet{"192.168.XXX.XXX/16"}, std::invalid_argument, - mpt::match_what("invalid IP octet")); + mpt::match_what(HasSubstr("invalid IP octet"))); } TEST(Subnet, throwsOnLargeCIDR) @@ -60,32 +60,32 @@ TEST(Subnet, throwsOnLargeCIDR) // valid but not supported MP_EXPECT_THROW_THAT(mp::Subnet{"192.168.0.0/31"}, std::invalid_argument, - mpt::match_what(what)); + mpt::match_what(HasSubstr(what))); // valid but not supported MP_EXPECT_THROW_THAT(mp::Subnet{"192.168.0.0/32"}, std::invalid_argument, - mpt::match_what(what)); + mpt::match_what(HasSubstr(what))); // boundary case MP_EXPECT_THROW_THAT(mp::Subnet{"192.168.0.0/33"}, std::invalid_argument, - mpt::match_what(what)); + mpt::match_what(HasSubstr(what))); // at 8 bit limit MP_EXPECT_THROW_THAT(mp::Subnet{"192.168.0.0/255"}, std::invalid_argument, - mpt::match_what(what)); + mpt::match_what(HasSubstr(what))); // above 8 bit limit MP_EXPECT_THROW_THAT(mp::Subnet{"192.168.0.0/895231337"}, std::invalid_argument, - mpt::match_what(what)); + mpt::match_what(HasSubstr(what))); // extreme case MP_EXPECT_THROW_THAT(mp::Subnet{"192.168.0.0/895231337890712387952378952359871235987169601436"}, std::invalid_argument, - mpt::match_what(what)); + mpt::match_what(HasSubstr(what))); } TEST(Subnet, throwsOnNegativeCIDR) @@ -94,7 +94,7 @@ TEST(Subnet, throwsOnNegativeCIDR) MP_EXPECT_THROW_THAT(mp::Subnet{"192.168.0.0/-24"}, std::invalid_argument, - mpt::match_what(what)); + mpt::match_what(HasSubstr(what))); } TEST(Subnet, givesCorrectRange) @@ -163,5 +163,5 @@ TEST(Subnet, canConvertToString) EXPECT_EQ(subnet.as_string(), "255.0.0.0/8"); subnet = mp::Subnet{"255.0.255.0/0"}; - EXPECT_EQ(subnet.as_string(), "0.0.0.0/8"); + EXPECT_EQ(subnet.as_string(), "0.0.0.0/0"); } From b6681b424796e6ad2223e527f7c526b0ab7bd57e Mon Sep 17 00:00:00 2001 From: trevor-shoe Date: Tue, 23 Sep 2025 04:54:29 -0400 Subject: [PATCH 04/28] [util] Move int rng to MP_UTILS this is to make testing possible and consistent. --- include/multipass/subnet.h | 2 +- include/multipass/utils.h | 2 ++ .../backends/shared/linux/backend_utils.cpp | 9 ++------ src/utils/json_utils.cpp | 4 +--- src/utils/utils.cpp | 23 +++++++++++++++---- tests/test_subnet.cpp | 11 +++++++++ 6 files changed, 35 insertions(+), 16 deletions(-) diff --git a/include/multipass/subnet.h b/include/multipass/subnet.h index 8d43c59c7e..27fd4a4ad1 100644 --- a/include/multipass/subnet.h +++ b/include/multipass/subnet.h @@ -22,7 +22,7 @@ #include "ip_address.h" -#define MP_SUBNET_UTILS multipass::SubnetUtils::instance(); +#define MP_SUBNET_UTILS multipass::SubnetUtils::instance() namespace multipass { diff --git a/include/multipass/utils.h b/include/multipass/utils.h index b8586f4ecf..6a224a1b9f 100644 --- a/include/multipass/utils.h +++ b/include/multipass/utils.h @@ -257,6 +257,8 @@ class Utils : public Singleton virtual Path default_mount_target(const Path& source) const; virtual Path normalize_mount_target(Path target_mount_path) const; virtual bool invalid_target_path(const Path& target_path) const; // needs normalized input path + + virtual int random_int(int a, int b) const; }; } // namespace multipass diff --git a/src/platform/backends/shared/linux/backend_utils.cpp b/src/platform/backends/shared/linux/backend_utils.cpp index b359f71709..470de1a1e5 100644 --- a/src/platform/backends/shared/linux/backend_utils.cpp +++ b/src/platform/backends/shared/linux/backend_utils.cpp @@ -55,12 +55,6 @@ Q_DECLARE_METATYPE(VariantMapMap) // for DBus namespace { -std::default_random_engine gen = [] { - // seed the rng with the time at initialization - auto seed = std::chrono::system_clock::now().time_since_epoch().count(); - return std::default_random_engine(seed); -}(); -std::uniform_int_distribution dist{0, 255}; const auto nm_bus_name = QStringLiteral("org.freedesktop.NetworkManager"); const auto nm_root_obj = QStringLiteral("/org/freedesktop/NetworkManager"); const auto nm_root_ifc = QStringLiteral("org.freedesktop.NetworkManager"); @@ -201,7 +195,8 @@ std::string generate_random_subnet() // TODO don't rely on pure randomness for (auto i = 0; i < 100; ++i) { - auto subnet = fmt::format("10.{}.{}", dist(gen), dist(gen)); + auto subnet = + fmt::format("10.{}.{}", MP_UTILS.random_int(0, 255), MP_UTILS.random_int(0, 255)); if (subnet_used_locally(subnet)) continue; diff --git a/src/utils/json_utils.cpp b/src/utils/json_utils.cpp index fc2139f20e..32bd632cfb 100644 --- a/src/utils/json_utils.cpp +++ b/src/utils/json_utils.cpp @@ -95,9 +95,7 @@ void mp::JsonUtils::write_json(const QJsonObject& root, QString file_name) const { auto get_jitter_amount = [] { constexpr static auto kMaxJitter = 25; - thread_local std::mt19937 rng{std::random_device{}()}; - thread_local std::uniform_int_distribution jit(0, kMaxJitter); - return jit(rng); + return MP_UTILS.random_int(0, kMaxJitter); }; // Delay with jitter + backoff. A typical series produced diff --git a/src/utils/utils.cpp b/src/utils/utils.cpp index 36fffd19bf..d446b8702b 100644 --- a/src/utils/utils.cpp +++ b/src/utils/utils.cpp @@ -242,11 +242,8 @@ std::vector mp::utils::split(const std::string& string, const std:: std::string mp::utils::generate_mac_address() { - std::default_random_engine gen; - std::uniform_int_distribution dist{0, 255}; - - gen.seed(std::chrono::system_clock::now().time_since_epoch().count()); - std::array octets{{dist(gen), dist(gen), dist(gen)}}; + std::array octets{ + {MP_UTILS.random_int(0, 255), MP_UTILS.random_int(0, 255), MP_UTILS.random_int(0, 255)}}; return fmt::format("52:54:00:{:02x}:{:02x}:{:02x}", octets[0], octets[1], octets[2]); } @@ -626,6 +623,22 @@ bool mp::Utils::invalid_target_path(const QString& target_path) const return matcher.match(target_path).hasMatch(); } +int mp::Utils::random_int(int a, int b) const +{ + static std::default_random_engine gen = [] { + // seed the rng with the time at first call + auto seed = std::chrono::system_clock::now().time_since_epoch().count(); + return std::default_random_engine(seed); + }(); + + if (a > b) // avoid undefined behavior, this can only happen by programmer error + throw std::logic_error(fmt::format("random range [{}, {}] is invalid", a, b)); + + std::uniform_int_distribution dist{a, b}; + + return dist(gen); +} + auto mp::utils::find_bridge_with(const std::vector& networks, const std::string& target_network, const std::string& bridge_type) diff --git a/tests/test_subnet.cpp b/tests/test_subnet.cpp index 115c4da3c8..ecef3dc303 100644 --- a/tests/test_subnet.cpp +++ b/tests/test_subnet.cpp @@ -16,6 +16,7 @@ */ #include "common.h" +#include "mock_utils.h" #include @@ -165,3 +166,13 @@ TEST(Subnet, canConvertToString) subnet = mp::Subnet{"255.0.255.0/0"}; EXPECT_EQ(subnet.as_string(), "0.0.0.0/0"); } + +TEST(SubnetUtils, generateRandomSubnetTriviallyWorks) +{ + mp::IPAddress ip{"10.1.2.0"}; + + mp::Subnet subnet = MP_SUBNET_UTILS.generate_random_subnet(ip, ip, 24); + + EXPECT_EQ(subnet.get_identifier(), ip); + EXPECT_EQ(subnet.get_CIDR(), 24); +} From d41ecd7204fafbb891823bb4c0a6a3e537abe7b2 Mon Sep 17 00:00:00 2001 From: trevor-shoe Date: Tue, 23 Sep 2025 06:05:20 -0400 Subject: [PATCH 05/28] [network] Add subnet contains --- include/multipass/subnet.h | 18 ++-- include/multipass/utils.h | 2 +- src/network/subnet.cpp | 40 +++++--- src/utils/utils.cpp | 4 +- tests/mock_utils.h | 1 + tests/test_subnet.cpp | 185 +++++++++++++++++++++++++++++++------ 6 files changed, 199 insertions(+), 51 deletions(-) diff --git a/include/multipass/subnet.h b/include/multipass/subnet.h index 27fd4a4ad1..44d61d171a 100644 --- a/include/multipass/subnet.h +++ b/include/multipass/subnet.h @@ -32,19 +32,23 @@ class Subnet Subnet(IPAddress ip, uint8_t cidr); Subnet(const std::string& cidr_string); - [[nodiscard]] IPAddress get_min_address() const; - [[nodiscard]] IPAddress get_max_address() const; - [[nodiscard]] uint32_t get_address_count() const; + [[nodiscard]] IPAddress min_address() const; + [[nodiscard]] IPAddress max_address() const; + [[nodiscard]] uint32_t address_count() const; - [[nodiscard]] IPAddress get_identifier() const; - [[nodiscard]] uint8_t get_CIDR() const; - [[nodiscard]] IPAddress get_subnet_mask() const; + [[nodiscard]] IPAddress identifier() const; + [[nodiscard]] uint8_t CIDR() const; + [[nodiscard]] IPAddress subnet_mask() const; // uses CIDR notation [[nodiscard]] std::string as_string() const; + // Subnets are either disjoint or the smaller is a subset of the larger + [[nodiscard]] bool contains(Subnet other) const; + [[nodiscard]] bool contains(IPAddress ip) const; + private: - IPAddress identifier; + IPAddress id; uint8_t cidr; }; diff --git a/include/multipass/utils.h b/include/multipass/utils.h index 6a224a1b9f..af9ac7a30e 100644 --- a/include/multipass/utils.h +++ b/include/multipass/utils.h @@ -258,7 +258,7 @@ class Utils : public Singleton virtual Path normalize_mount_target(Path target_mount_path) const; virtual bool invalid_target_path(const Path& target_path) const; // needs normalized input path - virtual int random_int(int a, int b) const; + virtual intmax_t random_int(intmax_t a, intmax_t b) const; }; } // namespace multipass diff --git a/src/network/subnet.cpp b/src/network/subnet.cpp index 4056fefe6c..2dc6b7cb39 100644 --- a/src/network/subnet.cpp +++ b/src/network/subnet.cpp @@ -87,7 +87,7 @@ catch (const std::out_of_range& e) } } // namespace -mp::Subnet::Subnet(IPAddress ip, uint8_t cidr) : identifier(apply_mask(ip, cidr)), cidr(cidr) +mp::Subnet::Subnet(IPAddress ip, uint8_t cidr) : id(apply_mask(ip, cidr)), cidr(cidr) { if (cidr >= 31) { @@ -100,33 +100,33 @@ mp::Subnet::Subnet(const std::string& cidr_string) : Subnet(parse(cidr_string)) { } -mp::IPAddress mp::Subnet::get_min_address() const +mp::IPAddress mp::Subnet::min_address() const { - return identifier + 1; + return id + 1; } -mp::IPAddress mp::Subnet::get_max_address() const +mp::IPAddress mp::Subnet::max_address() const { // identifier + 2^(32 - cidr) - 1 - 1 - return identifier + ((1ull << (32ull - cidr)) - 2ull); + return id + ((1ull << (32ull - cidr)) - 2ull); } -uint32_t mp::Subnet::get_address_count() const +uint32_t mp::Subnet::address_count() const { - return get_max_address().as_uint32() - get_min_address().as_uint32() + 1; + return max_address().as_uint32() - min_address().as_uint32() + 1; } -mp::IPAddress mp::Subnet::get_identifier() const +mp::IPAddress mp::Subnet::identifier() const { - return identifier; + return id; } -uint8_t mp::Subnet::get_CIDR() const +uint8_t mp::Subnet::CIDR() const { return cidr; } -mp::IPAddress mp::Subnet::get_subnet_mask() const +mp::IPAddress mp::Subnet::subnet_mask() const { return ::get_subnet_mask(cidr); } @@ -134,7 +134,23 @@ mp::IPAddress mp::Subnet::get_subnet_mask() const // uses CIDR notation std::string mp::Subnet::as_string() const { - return fmt::format("{}/{}", identifier.as_string(), cidr); + return fmt::format("{}/{}", id.as_string(), cidr); +} + +// due to how subnets work overlap does not need consideration +bool mp::Subnet::contains(Subnet other) const +{ + // can't possibly contain a larger subnet + if (other.CIDR() < CIDR()) + return false; + + return contains(other.identifier()); +} + +bool mp::Subnet::contains(IPAddress ip) const +{ + // since get_max_address doesn't include the broadcast address add 1 to it. + return identifier() <= ip && (max_address() + 1) >= ip; } mp::Subnet mp::SubnetUtils::generate_random_subnet(IPAddress start, diff --git a/src/utils/utils.cpp b/src/utils/utils.cpp index d446b8702b..3ce8a3471e 100644 --- a/src/utils/utils.cpp +++ b/src/utils/utils.cpp @@ -623,7 +623,7 @@ bool mp::Utils::invalid_target_path(const QString& target_path) const return matcher.match(target_path).hasMatch(); } -int mp::Utils::random_int(int a, int b) const +intmax_t mp::Utils::random_int(intmax_t a, intmax_t b) const { static std::default_random_engine gen = [] { // seed the rng with the time at first call @@ -634,7 +634,7 @@ int mp::Utils::random_int(int a, int b) const if (a > b) // avoid undefined behavior, this can only happen by programmer error throw std::logic_error(fmt::format("random range [{}, {}] is invalid", a, b)); - std::uniform_int_distribution dist{a, b}; + std::uniform_int_distribution dist{a, b}; return dist(gen); } diff --git a/tests/mock_utils.h b/tests/mock_utils.h index 338a9341d1..e5033ce23e 100644 --- a/tests/mock_utils.h +++ b/tests/mock_utils.h @@ -64,6 +64,7 @@ class MockUtils : public Utils MOCK_METHOD(void, sleep_for, (const std::chrono::milliseconds&), (const, override)); MOCK_METHOD(bool, is_ipv4_valid, (const std::string& ipv4), (const, override)); MOCK_METHOD(Path, default_mount_target, (const Path& source), (const, override)); + MOCK_METHOD(intmax_t, random_int, (intmax_t a, intmax_t b), (const, override)); MP_MOCK_SINGLETON_BOILERPLATE(MockUtils, Utils); }; diff --git a/tests/test_subnet.cpp b/tests/test_subnet.cpp index ecef3dc303..99df494ba3 100644 --- a/tests/test_subnet.cpp +++ b/tests/test_subnet.cpp @@ -16,6 +16,7 @@ */ #include "common.h" +#include "mock_platform.h" #include "mock_utils.h" #include @@ -28,16 +29,16 @@ TEST(Subnet, canInitializeFromIpCidrPair) { mp::Subnet subnet{mp::IPAddress("192.168.0.0"), 24}; - EXPECT_EQ(subnet.get_identifier(), mp::IPAddress("192.168.0.0")); - EXPECT_EQ(subnet.get_CIDR(), 24); + EXPECT_EQ(subnet.identifier(), mp::IPAddress("192.168.0.0")); + EXPECT_EQ(subnet.CIDR(), 24); } TEST(Subnet, canInitializeFromString) { mp::Subnet subnet{"192.168.0.0/24"}; - EXPECT_EQ(subnet.get_identifier(), mp::IPAddress("192.168.0.0")); - EXPECT_EQ(subnet.get_CIDR(), 24); + EXPECT_EQ(subnet.identifier(), mp::IPAddress("192.168.0.0")); + EXPECT_EQ(subnet.CIDR(), 24); } TEST(Subet, throwsOnInvalidIP) @@ -101,58 +102,58 @@ TEST(Subnet, throwsOnNegativeCIDR) TEST(Subnet, givesCorrectRange) { mp::Subnet subnet{"192.168.0.0/24"}; - EXPECT_EQ(subnet.get_identifier(), mp::IPAddress{"192.168.0.0"}); - EXPECT_EQ(subnet.get_min_address(), mp::IPAddress{"192.168.0.1"}); - EXPECT_EQ(subnet.get_max_address(), mp::IPAddress{"192.168.0.254"}); - EXPECT_EQ(subnet.get_address_count(), 254); + EXPECT_EQ(subnet.identifier(), mp::IPAddress{"192.168.0.0"}); + EXPECT_EQ(subnet.min_address(), mp::IPAddress{"192.168.0.1"}); + EXPECT_EQ(subnet.max_address(), mp::IPAddress{"192.168.0.254"}); + EXPECT_EQ(subnet.address_count(), 254); subnet = mp::Subnet{"121.212.1.152/11"}; - EXPECT_EQ(subnet.get_identifier(), mp::IPAddress{"121.192.0.0"}); - EXPECT_EQ(subnet.get_min_address(), mp::IPAddress{"121.192.0.1"}); - EXPECT_EQ(subnet.get_max_address(), mp::IPAddress{"121,223.255.254"}); - EXPECT_EQ(subnet.get_address_count(), 2097150); + EXPECT_EQ(subnet.identifier(), mp::IPAddress{"121.192.0.0"}); + EXPECT_EQ(subnet.min_address(), mp::IPAddress{"121.192.0.1"}); + EXPECT_EQ(subnet.max_address(), mp::IPAddress{"121,223.255.254"}); + EXPECT_EQ(subnet.address_count(), 2097150); subnet = mp::Subnet{"0.0.0.0/0"}; - EXPECT_EQ(subnet.get_identifier(), mp::IPAddress{"0.0.0.0"}); - EXPECT_EQ(subnet.get_min_address(), mp::IPAddress{"0.0.0.1"}); - EXPECT_EQ(subnet.get_max_address(), mp::IPAddress{"255,255.255.254"}); - EXPECT_EQ(subnet.get_address_count(), 4294967294); + EXPECT_EQ(subnet.identifier(), mp::IPAddress{"0.0.0.0"}); + EXPECT_EQ(subnet.min_address(), mp::IPAddress{"0.0.0.1"}); + EXPECT_EQ(subnet.max_address(), mp::IPAddress{"255,255.255.254"}); + EXPECT_EQ(subnet.address_count(), 4294967294); } TEST(Subnet, convertsToMaskedIP) { mp::Subnet subnet{"192.168.255.52/24"}; - EXPECT_EQ(subnet.get_identifier(), mp::IPAddress{"192.168.255.0"}); + EXPECT_EQ(subnet.identifier(), mp::IPAddress{"192.168.255.0"}); subnet = mp::Subnet{"255.168.1.152/8"}; - EXPECT_EQ(subnet.get_identifier(), mp::IPAddress{"255.0.0.0"}); + EXPECT_EQ(subnet.identifier(), mp::IPAddress{"255.0.0.0"}); subnet = mp::Subnet{"192.168.1.152/0"}; - EXPECT_EQ(subnet.get_identifier(), mp::IPAddress{"0.0.0.0"}); + EXPECT_EQ(subnet.identifier(), mp::IPAddress{"0.0.0.0"}); subnet = mp::Subnet{"255.212.1.152/13"}; - EXPECT_EQ(subnet.get_identifier(), mp::IPAddress{"255.208.0.0"}); + EXPECT_EQ(subnet.identifier(), mp::IPAddress{"255.208.0.0"}); } TEST(Subnet, getSubnetMaskReturnsSubnetMask) { mp::Subnet subnet{"192.168.0.1/24"}; - EXPECT_EQ(subnet.get_subnet_mask(), mp::IPAddress("255.255.255.0")); + EXPECT_EQ(subnet.subnet_mask(), mp::IPAddress("255.255.255.0")); subnet = mp::Subnet{"192.168.0.1/21"}; - EXPECT_EQ(subnet.get_subnet_mask(), mp::IPAddress("255.255.248.0")); + EXPECT_EQ(subnet.subnet_mask(), mp::IPAddress("255.255.248.0")); subnet = mp::Subnet{"192.168.0.1/16"}; - EXPECT_EQ(subnet.get_subnet_mask(), mp::IPAddress("255.255.0.0")); + EXPECT_EQ(subnet.subnet_mask(), mp::IPAddress("255.255.0.0")); subnet = mp::Subnet{"192.168.0.1/9"}; - EXPECT_EQ(subnet.get_subnet_mask(), mp::IPAddress("255.128.0.0")); + EXPECT_EQ(subnet.subnet_mask(), mp::IPAddress("255.128.0.0")); subnet = mp::Subnet{"192.168.0.1/4"}; - EXPECT_EQ(subnet.get_subnet_mask(), mp::IPAddress("240.0.0.0")); + EXPECT_EQ(subnet.subnet_mask(), mp::IPAddress("240.0.0.0")); subnet = mp::Subnet{"192.168.0.1/0"}; - EXPECT_EQ(subnet.get_subnet_mask(), mp::IPAddress("0.0.0.0")); + EXPECT_EQ(subnet.subnet_mask(), mp::IPAddress("0.0.0.0")); } TEST(Subnet, canConvertToString) @@ -167,12 +168,138 @@ TEST(Subnet, canConvertToString) EXPECT_EQ(subnet.as_string(), "0.0.0.0/0"); } -TEST(SubnetUtils, generateRandomSubnetTriviallyWorks) +TEST(Subnet, containsWorksOnContainedSubnets) +{ + mp::Subnet container{"192.168.0.0/16"}; + + // self + EXPECT_TRUE(container.contains(container)); + + // bounds + mp::Subnet subnet{"192.168.0.0/17"}; + EXPECT_TRUE(container.contains(subnet)); + + subnet = mp::Subnet{"192.168.128.0/17"}; + EXPECT_TRUE(container.contains(subnet)); + + // sanity cases + subnet = mp::Subnet{"192.168.72.0/24"}; + EXPECT_TRUE(container.contains(subnet)); + + subnet = mp::Subnet{"192.168.123.220/30"}; + EXPECT_TRUE(container.contains(subnet)); +} + +TEST(Subnet, containsWorksOnUnContainedSubnets) +{ + mp::Subnet subnet{"172.17.0.0/16"}; + + // boundary (subset) + mp::Subnet container{"172.17.0.0/15"}; + EXPECT_FALSE(subnet.contains(container)); + + // boundaries (disjoint) + container = mp::Subnet{"172.16.0.0/16"}; + EXPECT_FALSE(subnet.contains(container)); + + container = mp::Subnet{"172.18.0.0/16"}; + EXPECT_FALSE(subnet.contains(container)); + + // disjoint + container = mp::Subnet{"192.168.1.0/24"}; + EXPECT_FALSE(subnet.contains(container)); + + container = mp::Subnet{"172.1.0.0/16"}; + EXPECT_FALSE(subnet.contains(container)); + + // subset + container = mp::Subnet{"0.0.0.0/0"}; + EXPECT_FALSE(subnet.contains(container)); + + container = mp::Subnet{"172.0.0.0/8"}; + EXPECT_FALSE(subnet.contains(container)); +} + +TEST(Subnet, containsWorksOnContainedIps) +{ + mp::Subnet subnet{"10.0.0.0/8"}; + + // boundaries + mp::IPAddress ip{"10.0.0.0"}; + EXPECT_TRUE(subnet.contains(ip)); + + ip = mp::IPAddress{"10.255.255.255"}; + EXPECT_TRUE(subnet.contains(ip)); + + // sanity + ip = mp::IPAddress{"10.1.2.3"}; + EXPECT_TRUE(subnet.contains(ip)); + + ip = mp::IPAddress{"10.168.172.192"}; + EXPECT_TRUE(subnet.contains(ip)); +} + +TEST(Subnet, containsWorksOnUnContainedIps) +{ + mp::Subnet subnet{"192.168.66.0/24"}; + + // boundaries + mp::IPAddress ip{"192.168.67.0"}; + EXPECT_FALSE(subnet.contains(ip)); + + ip = mp::IPAddress{"192.168.65.255"}; + EXPECT_FALSE(subnet.contains(ip)); + + // sanity + ip = mp::IPAddress{"0.0.0.0"}; + EXPECT_FALSE(subnet.contains(ip)); + + ip = mp::IPAddress{"255.255.255.255"}; + EXPECT_FALSE(subnet.contains(ip)); + + ip = mp::IPAddress{"192.168.1.72"}; + EXPECT_FALSE(subnet.contains(ip)); +} + +struct SubnetUtils : public Test +{ + SubnetUtils() + { + + } + + mpt::MockUtils::GuardedMock mock_utils_injection{mpt::MockUtils::inject()}; + mpt::MockUtils* mock_utils = mock_utils_injection.first; + + mpt::MockPlatform::GuardedMock mock_platform_injection = mpt::MockPlatform::inject(); + mpt::MockPlatform* mock_platform = mock_platform_injection.first; +}; + +TEST_F(SubnetUtils, generateRandomSubnetTriviallyWorks) { mp::IPAddress ip{"10.1.2.0"}; mp::Subnet subnet = MP_SUBNET_UTILS.generate_random_subnet(ip, ip, 24); - EXPECT_EQ(subnet.get_identifier(), ip); - EXPECT_EQ(subnet.get_CIDR(), 24); + // @TODO mock backend calls + + EXPECT_EQ(subnet.identifier(), ip); + EXPECT_EQ(subnet.CIDR(), 24); +} + +TEST_F(SubnetUtils, generateRandomSubnetRespectsRange) +{ + mp::IPAddress a{"10.1.2.0"}, b{"11.3.4.0"}; + + auto [mock_utils, guard] = mpt::MockUtils::inject(); + + EXPECT_CALL(*mock_utils, random_int(a.as_uint32(), b.as_uint32())) + .WillOnce(Return(a.as_uint32())); + + // @TODO mock backend calls + + auto subnet = MP_SUBNET_UTILS.generate_random_subnet(a, b, 24); + + EXPECT_EQ(subnet.identifier(), a); + EXPECT_EQ(subnet.CIDR(), 24); } From 18c227386ebe983086d7f0c659d67540bf50cd3a Mon Sep 17 00:00:00 2001 From: trevor-shoe Date: Tue, 23 Sep 2025 09:23:50 -0400 Subject: [PATCH 06/28] [network] Implement generate random subnet --- include/multipass/subnet.h | 3 +- src/network/subnet.cpp | 33 ++++++++++++++----- tests/test_subnet.cpp | 67 +++++++++++++++++++++++++++++++------- 3 files changed, 83 insertions(+), 20 deletions(-) diff --git a/include/multipass/subnet.h b/include/multipass/subnet.h index 44d61d171a..4e4977f2b8 100644 --- a/include/multipass/subnet.h +++ b/include/multipass/subnet.h @@ -56,7 +56,8 @@ struct SubnetUtils : Singleton { using Singleton::Singleton; - [[nodiscard]] virtual Subnet generate_random_subnet(IPAddress start, IPAddress end, uint8_t cidr) const; + [[nodiscard]] virtual Subnet generate_random_subnet(uint8_t cidr = 24, + Subnet range = Subnet{"10.0.0.0/8"}) const; [[nodiscard]] virtual Subnet get_subnet(const Path& network_dir, const QString& bridge_name) const; }; } // namespace multipass diff --git a/src/network/subnet.cpp b/src/network/subnet.cpp index 2dc6b7cb39..1e1fe605fc 100644 --- a/src/network/subnet.cpp +++ b/src/network/subnet.cpp @@ -16,6 +16,7 @@ */ #include +#include #include @@ -67,10 +68,12 @@ catch (const std::out_of_range& e) octets[i] = 0; } - for (size_t i = 0; i < remain; ++ i) + if (remain != 0) { - octets[start_octet] >>= 1; - octets[start_octet] |= 1 << (value_size - 1); + assert(start_octet < octets.size()); // sanity + + // remain = 5, 1 << (8 - 5) = 00001000 -> 00000111 -> 11111000 + octets[start_octet] = ~((1u << (value_size - remain)) - 1u); } return mp::IPAddress{octets}; @@ -137,7 +140,6 @@ std::string mp::Subnet::as_string() const return fmt::format("{}/{}", id.as_string(), cidr); } -// due to how subnets work overlap does not need consideration bool mp::Subnet::contains(Subnet other) const { // can't possibly contain a larger subnet @@ -153,11 +155,26 @@ bool mp::Subnet::contains(IPAddress ip) const return identifier() <= ip && (max_address() + 1) >= ip; } -mp::Subnet mp::SubnetUtils::generate_random_subnet(IPAddress start, - IPAddress end, - uint8_t cidr) const +mp::Subnet mp::SubnetUtils::generate_random_subnet(uint8_t cidr, Subnet range) const { - return mp::Subnet{""}; + if (cidr >= 31) + throw std::invalid_argument(fmt::format(large_CIDR_err_fmt, cidr)); + + if (cidr < range.CIDR()) + throw std::logic_error(fmt::format("A subnet with cidr {} cannot be contained by {}", cidr, range.as_string())); + + // ex. 2^(24 - 16) = 256, [192.168.0.0/24, 192.168.255.0/24] + const size_t possibleSubnets = std::size_t{1} << (cidr - range.CIDR()); + + // narrowing conversion, possibleSubnets is guarenteed to be < 2^31 (4 bytes is safe) + static_assert(sizeof(decltype(MP_UTILS.random_int(0, possibleSubnets))) >= 4); + + const auto subnet = static_cast(MP_UTILS.random_int(0, possibleSubnets - 1)); + + // ex. 192.168.0.0 + (4 * 2^(32 - 24)) = 192.168.0.0 + 1024 = 192.168.4.0 + mp::IPAddress id = range.identifier() + (subnet * (std::size_t{1} << (32 - cidr))); + + return mp::Subnet{id, cidr}; } mp::Subnet mp::SubnetUtils::get_subnet(const mp::Path& network_dir, const QString& bridge_name) const diff --git a/tests/test_subnet.cpp b/tests/test_subnet.cpp index 99df494ba3..43fc9a5bc5 100644 --- a/tests/test_subnet.cpp +++ b/tests/test_subnet.cpp @@ -277,29 +277,74 @@ struct SubnetUtils : public Test TEST_F(SubnetUtils, generateRandomSubnetTriviallyWorks) { - mp::IPAddress ip{"10.1.2.0"}; + const mp::Subnet range{"10.1.2.0/24"}; - mp::Subnet subnet = MP_SUBNET_UTILS.generate_random_subnet(ip, ip, 24); + EXPECT_CALL(*mock_utils, random_int(_, _)).WillOnce(Invoke([](auto a, auto b){ + EXPECT_EQ(a, b); + return a; + })); - // @TODO mock backend calls + mp::Subnet subnet = MP_SUBNET_UTILS.generate_random_subnet(24, range); - EXPECT_EQ(subnet.identifier(), ip); + EXPECT_EQ(subnet.identifier(), range.identifier()); EXPECT_EQ(subnet.CIDR(), 24); } +TEST_F(SubnetUtils, generateRandomSubnetFailsOnSmallRange) +{ + mp::Subnet range{"192.168.1.0/16"}; + + EXPECT_THROW(std::ignore = MP_SUBNET_UTILS.generate_random_subnet(15, range), std::logic_error); + EXPECT_THROW(std::ignore = MP_SUBNET_UTILS.generate_random_subnet(0, range), std::logic_error); +} + +TEST_F(SubnetUtils, generateRandomSubnetFailsOnBadCIDR) +{ + mp::Subnet range{"0.0.0.0/0"}; + + EXPECT_THROW(std::ignore = MP_SUBNET_UTILS.generate_random_subnet(31, range), std::invalid_argument); + EXPECT_THROW(std::ignore = MP_SUBNET_UTILS.generate_random_subnet(32, range), + std::invalid_argument); + EXPECT_THROW(std::ignore = MP_SUBNET_UTILS.generate_random_subnet(33, range), + std::invalid_argument); + EXPECT_THROW(std::ignore = MP_SUBNET_UTILS.generate_random_subnet(255, range), + std::invalid_argument); +} + TEST_F(SubnetUtils, generateRandomSubnetRespectsRange) { - mp::IPAddress a{"10.1.2.0"}, b{"11.3.4.0"}; + mp::Subnet range("192.168.0.0/16"); auto [mock_utils, guard] = mpt::MockUtils::inject(); - EXPECT_CALL(*mock_utils, random_int(a.as_uint32(), b.as_uint32())) - .WillOnce(Return(a.as_uint32())); + EXPECT_CALL(*mock_utils, random_int(_, _)) + .WillOnce(ReturnArg<0>()) + .WillOnce(ReturnArg<1>()); - // @TODO mock backend calls + auto subnetLow = MP_SUBNET_UTILS.generate_random_subnet(24, range); + auto subnetHigh = MP_SUBNET_UTILS.generate_random_subnet(24, range); - auto subnet = MP_SUBNET_UTILS.generate_random_subnet(a, b, 24); + EXPECT_EQ(subnetLow.identifier(), mp::IPAddress{"192.168.0.0"}); + EXPECT_EQ(subnetLow.CIDR(), 24); - EXPECT_EQ(subnet.identifier(), a); - EXPECT_EQ(subnet.CIDR(), 24); + EXPECT_EQ(subnetHigh.identifier(), mp::IPAddress{"192.168.255.0"}); + EXPECT_EQ(subnetHigh.CIDR(), 24); +} + +TEST_F(SubnetUtils, generateRandomSubnetWorksAtUpperExtreme) +{ + mp::Subnet range("0.0.0.0/0"); + + EXPECT_CALL(*mock_utils, random_int(_, _)) + .WillOnce(ReturnArg<0>()) + .WillOnce(ReturnArg<1>()); + + auto subnetLow = MP_SUBNET_UTILS.generate_random_subnet(30, range); + auto subnetHigh = MP_SUBNET_UTILS.generate_random_subnet(30, range); + + EXPECT_EQ(subnetLow.identifier(), mp::IPAddress{"0.0.0.0"}); + EXPECT_EQ(subnetLow.CIDR(), 30); + + EXPECT_EQ(subnetHigh.identifier(), mp::IPAddress{"255.255.255.252"}); + EXPECT_EQ(subnetHigh.CIDR(), 30); } From 2fa5365baa91311a56c88f91eb09d11c27bf36c5 Mon Sep 17 00:00:00 2001 From: trevor-shoe Date: Wed, 24 Sep 2025 08:03:34 -0400 Subject: [PATCH 07/28] [network] Make local subnet check robust previously this missed cases of larger subnets --- include/multipass/platform.h | 3 + .../backends/shared/linux/backend_utils.cpp | 13 ---- src/platform/platform_unix.cpp | 41 +++++++++++ src/platform/platform_win.cpp | 12 ++++ tests/test_alias_dict.cpp | 3 + tests/test_daemon.cpp | 12 ++++ tests/unix/test_platform_unix.cpp | 71 +++++++++++++++++++ 7 files changed, 142 insertions(+), 13 deletions(-) diff --git a/include/multipass/platform.h b/include/multipass/platform.h index 820a79efe3..4c953b1838 100644 --- a/include/multipass/platform.h +++ b/include/multipass/platform.h @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -77,6 +78,8 @@ class Platform : public Singleton virtual QString default_driver() const; virtual QString default_privileged_mounts() const; [[nodiscard]] virtual std::string bridge_nomenclature() const; + [[nodiscard]] virtual bool can_reach_gateway(IPAddress ip) const; + [[nodiscard]] virtual bool subnet_used_locally(Subnet subnet) const; virtual int get_cpus() const; virtual long long get_total_ram() const; diff --git a/src/platform/backends/shared/linux/backend_utils.cpp b/src/platform/backends/shared/linux/backend_utils.cpp index 470de1a1e5..2da30cb3fd 100644 --- a/src/platform/backends/shared/linux/backend_utils.cpp +++ b/src/platform/backends/shared/linux/backend_utils.cpp @@ -63,19 +63,6 @@ const auto nm_settings_ifc = QStringLiteral("org.freedesktop.NetworkManager.Sett const auto nm_connection_ifc = QStringLiteral("org.freedesktop.NetworkManager.Settings.Connection"); constexpr auto max_bridge_name_len = 15; // maximum number of characters in a bridge name -bool subnet_used_locally(const std::string& subnet) -{ - // CLI equivalent: ip -4 route show | grep -q ${SUBNET} - const auto output = - QString::fromStdString(MP_UTILS.run_cmd_for_output("ip", {"-4", "route", "show"})); - return output.contains(QString::fromStdString(subnet)); -} - -bool can_reach_gateway(const std::string& ip) -{ - return MP_UTILS.run_cmd_for_status("ping", {"-n", "-q", ip.c_str(), "-c", "1", "-W", "1"}); -} - auto virtual_switch_subnet(const QString& bridge_name) { // CLI equivalent: ip -4 route show | grep ${BRIDGE_NAME} | cut -d ' ' -f1 | cut -d '.' -f1-3 diff --git a/src/platform/platform_unix.cpp b/src/platform/platform_unix.cpp index 3bb2f6e0e5..ddaff6bd88 100644 --- a/src/platform/platform_unix.cpp +++ b/src/platform/platform_unix.cpp @@ -270,3 +270,44 @@ long long mp::platform::Platform::get_total_ram() const { return static_cast(sysconf(_SC_PHYS_PAGES)) * sysconf(_SC_PAGESIZE); } + +bool mp::platform::Platform::can_reach_gateway(mp::IPAddress ip) const +{ + const auto ipstr = ip.as_string(); + return MP_UTILS.run_cmd_for_status("ping", {"-n", "-q", ipstr.c_str(), "-c", "1", "-W", "1"}); +} + +bool mp::platform::Platform::subnet_used_locally(mp::Subnet subnet) const +{ + // validation of the ip and cidr happen later, otherwise this regex would be massive. + static const QRegularExpression subnet_regex(R"(^((?:[0-9][0-9]?[0-9]?\.){3}[0-9][0-9]?[0-9]?\/[0-9][0-9]?))"); + + const auto output = + QString::fromStdString(MP_UTILS.run_cmd_for_output("ip", {"-4", "route", "show"})); + + QRegularExpressionMatchIterator i = subnet_regex.globalMatch(output); + + while (i.hasNext()) + { + QRegularExpressionMatch match = i.next(); + + try + { + mp::Subnet found_net{match.captured(1).toStdString()}; + + // check for overlap + if (found_net.contains(subnet) || subnet.contains(found_net)) + { + return true; + } + } + catch (const std::invalid_argument& e) + { + mpl::log( + mpl::Level::warning, + "network", + fmt::format("invalid subnet from ip command: {}", e.message())); + } + } + return false; +} diff --git a/src/platform/platform_win.cpp b/src/platform/platform_win.cpp index babe1d4f07..9519229efc 100644 --- a/src/platform/platform_win.cpp +++ b/src/platform/platform_win.cpp @@ -629,6 +629,18 @@ std::string mp::platform::Platform::bridge_nomenclature() const return "switch"; } +bool mp::platform::Platform::can_reach_gateway(mp::IPAddress ip) const +{ + // ping + throw mp::NotImplementedOnThisBackendException{"AZs @TODO"}; +} + +bool mp::platform::Platform::subnet_used_locally(mp::Subnet subnet) const +{ + // Get-NetAdapter | Get-NetIPAddress | Format-Table IPAddress,PrefixLength + throw mp::NotImplementedOnThisBackendException{"AZs @TODO"}; +} + QString mp::platform::Platform::daemon_config_home() const // temporary { auto ret = systemprofile_app_data_path(); diff --git a/tests/test_alias_dict.cpp b/tests/test_alias_dict.cpp index 8d1f96ffca..2d2b2bda2a 100644 --- a/tests/test_alias_dict.cpp +++ b/tests/test_alias_dict.cpp @@ -649,6 +649,9 @@ TEST_P(DaemonAliasTestsuite, purgeRemovesPurgedInstanceAliasesAndScripts) const auto [mock_utils, guard] = mpt::MockUtils::inject(); EXPECT_CALL(*mock_utils, contents_of(_)).WillRepeatedly(Return(mpt::root_cert)); + // generate_unused_mac_address works by randomly guessing + MP_DELEGATE_MOCK_CALLS_ON_BASE(*mock_utils, random_int, mp::Utils); + config_builder.vault = std::move(mock_image_vault); auto mock_factory = use_a_mock_vm_factory(); diff --git a/tests/test_daemon.cpp b/tests/test_daemon.cpp index 133c64a263..4642a45c8c 100644 --- a/tests/test_daemon.cpp +++ b/tests/test_daemon.cpp @@ -883,6 +883,9 @@ TEST_P(DaemonCreateLaunchPollinateDataTestSuite, addsPollinateUserAgentToCloudIn TEST_P(LaunchWithNoExtraNetworkCloudInit, noExtraNetworkCloudInit) { + // generate_unused_mac_address works by randomly guessing + MP_DELEGATE_MOCK_CALLS_ON_BASE(mock_utils, random_int, mp::Utils); + mpt::MockVirtualMachineFactory* mock_factory = use_a_mock_vm_factory(); mp::Daemon daemon{config_builder.build()}; @@ -926,6 +929,9 @@ INSTANTIATE_TEST_SUITE_P( TEST_P(LaunchWithBridges, createsNetworkCloudInitIso) { + // generate_unused_mac_address works by randomly guessing + MP_DELEGATE_MOCK_CALLS_ON_BASE(mock_utils, random_int, mp::Utils); + mpt::MockVirtualMachineFactory* mock_factory = use_a_mock_vm_factory(); mp::Daemon daemon{config_builder.build()}; @@ -1667,6 +1673,9 @@ TEST_F(Daemon, doesNotHoldOnToRepeatedMacAddressesWhenLoading) TEST_F(Daemon, doesNotHoldOnToMacsWhenLoadingFails) { + // generate_unused_mac_address works by randomly guessing + MP_DELEGATE_MOCK_CALLS_ON_BASE(mock_utils, random_int, mp::Utils); + std::string mac1{"52:54:00:73:76:28"}, mac2{"52:54:00:bd:19:41"}; std::vector extra_interfaces{mp::NetworkInterface{"eth0", mac2, true}}; @@ -1725,6 +1734,9 @@ TEST_F(Daemon, releasesMacsWhenLaunchFails) TEST_F(Daemon, releasesMacsOfPurgedInstancesButKeepsTheRest) { + // generate_unused_mac_address works by randomly guessing + MP_DELEGATE_MOCK_CALLS_ON_BASE(mock_utils, random_int, mp::Utils); + auto mock_factory = use_a_mock_vm_factory(); mp::Daemon daemon{config_builder.build()}; diff --git a/tests/unix/test_platform_unix.cpp b/tests/unix/test_platform_unix.cpp index b897df0514..eb8c912b6f 100644 --- a/tests/unix/test_platform_unix.cpp +++ b/tests/unix/test_platform_unix.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include "mock_libc_functions.h" @@ -334,3 +335,73 @@ TEST_F(TestPlatformUnix, quitWatchdogSignalsItselfAsynchronously) std::nullopt); EXPECT_GE(times.load(std::memory_order_acquire), 10); } + +TEST_F(TestPlatformUnix, canReachGatewayRunsPingWithIP) +{ + const mp::IPAddress testIP{"192.168.0.1"}; + const auto testIPstr = testIP.as_string(); + + auto [mock_utils, guard] = mpt::MockUtils::inject(); + + EXPECT_CALL(*mock_utils, run_cmd_for_status(QString("ping"), Contains(StrEq(testIPstr.c_str())), _)).WillOnce(Return(true)).WillOnce(Return(false)); + + EXPECT_TRUE(MP_PLATFORM.can_reach_gateway(testIP)); + EXPECT_FALSE(MP_PLATFORM.can_reach_gateway(testIP)); +} + +TEST_F(TestPlatformUnix, subnetUsedLocallyDetectsUnused) +{ + const mp::IPAddress testSubnet{"192.168.1.0/24"}; + + auto [mock_utils, guard] = mpt::MockUtils::inject(); + + EXPECT_CALL(*mock_utils, run_cmd_for_output(QString("ip"), _, _)).WillOnce(Return(R"( +default via 192.168.0.0 dev wlo1 proto dhcp src 192.168.0.1 metric 600 +10.20.30.0/24 dev lxdbr0 proto kernel scope link src 10.20.30.1 +10.192.168.0/24 dev mpqemubr0 proto kernel scop link src 10.192.168.1 linkdown +10.255.19.0/24 dev mpbr0 proto kernel scope link src 10.255.19.1 +172.172.0.0/16 dev docker0 proto kernel scope link src 172.172.0.1 linkdown +192.168.0.0/24 dev wlo1 proto kernel scope link src 192.168.0.1 metric 600 +192.168.123.0/24 dev virbr0 proto kernel scope link src 192.168.123.1 linkdown +)")); + + EXPECT_FALSE(MP_PLATFORM.subnet_used_locally(testIP)); +} + +TEST_F(TestPlatformUnix, subnetUsedLocallyDetectsOverlapping) +{ + const mp::IPAddress testSubnet{"172.172.1.0/24"}; + + auto [mock_utils, guard] = mpt::MockUtils::inject(); + + EXPECT_CALL(*mock_utils, run_cmd_for_output(QString("ip"), _, _)).WillOnce(Return(R"( +default via 192.168.0.0 dev wlo1 proto dhcp src 192.168.0.1 metric 600 +10.20.30.0/24 dev lxdbr0 proto kernel scope link src 10.20.30.1 +10.192.168.0/24 dev mpqemubr0 proto kernel scop link src 10.192.168.1 linkdown +10.255.19.0/24 dev mpbr0 proto kernel scope link src 10.255.19.1 +172.172.0.0/16 dev docker0 proto kernel scope link src 172.172.0.1 linkdown +192.168.0.0/24 dev wlo1 proto kernel scope link src 192.168.0.1 metric 600 +192.168.123.0/24 dev virbr0 proto kernel scope link src 192.168.123.1 linkdown +)")); + + EXPECT_TRUE(MP_PLATFORM.subnet_used_locally(testIP)); +} + +TEST_F(TestPlatformUnix, subnetUsedLocallyDetectsConflicting) +{ + const mp::IPAddress testSubnet{"10.20.30.0/24"}; + + auto [mock_utils, guard] = mpt::MockUtils::inject(); + + EXPECT_CALL(*mock_utils, run_cmd_for_output(QString("ip"), _, _)).WillOnce(Return(R"( +default via 192.168.0.0 dev wlo1 proto dhcp src 192.168.0.1 metric 600 +10.20.30.0/24 dev lxdbr0 proto kernel scope link src 10.20.30.1 +10.192.168.0/24 dev mpqemubr0 proto kernel scop link src 10.192.168.1 linkdown +10.255.19.0/24 dev mpbr0 proto kernel scope link src 10.255.19.1 +172.172.0.0/16 dev docker0 proto kernel scope link src 172.172.0.1 linkdown +192.168.0.0/24 dev wlo1 proto kernel scope link src 192.168.0.1 metric 600 +192.168.123.0/24 dev virbr0 proto kernel scope link src 192.168.123.1 linkdown +)")); + + EXPECT_TRUE(MP_PLATFORM.subnet_used_locally(testIP)); +} From 637b2c3cee693747f170b038e20e3a4d6dd0c7b8 Mon Sep 17 00:00:00 2001 From: trevor-shoe Date: Thu, 25 Sep 2025 05:56:55 -0400 Subject: [PATCH 08/28] [network] Replace existing subnet code Signed-off-by: trevor-shoe --- include/multipass/platform.h | 1 + include/multipass/subnet.h | 2 + src/network/subnet.cpp | 85 +++++++++++++++---- .../qemu/linux/dnsmasq_process_spec.cpp | 10 +-- .../qemu/linux/dnsmasq_process_spec.h | 4 +- .../backends/qemu/linux/dnsmasq_server.cpp | 6 +- .../backends/qemu/linux/dnsmasq_server.h | 7 +- .../backends/qemu/linux/firewall_config.cpp | 6 +- .../backends/qemu/linux/firewall_config.h | 5 +- .../qemu/linux/qemu_platform_detail.h | 16 ++-- .../qemu/linux/qemu_platform_detail_linux.cpp | 22 ++--- .../backends/shared/linux/backend_utils.cpp | 65 +------------- .../backends/shared/linux/backend_utils.h | 1 - src/platform/platform_unix.cpp | 32 ++++++- src/platform/platform_win.cpp | 6 ++ src/utils/utils.cpp | 2 +- tests/linux/test_backend_utils.cpp | 59 ------------- tests/linux/test_platform_linux.cpp | 2 +- tests/mock_backend_utils.h | 1 - tests/mock_platform.h | 3 + tests/mock_subnet_utils.h | 36 ++++++++ tests/qemu/linux/mock_dnsmasq_server.h | 2 +- tests/qemu/linux/mock_firewall_config.h | 2 +- .../qemu/linux/test_dnsmasq_process_spec.cpp | 2 +- tests/qemu/linux/test_dnsmasq_server.cpp | 19 ++--- tests/qemu/linux/test_firewall_config.cpp | 14 +-- .../qemu/linux/test_qemu_platform_detail.cpp | 29 ++++--- tests/test_subnet.cpp | 69 ++++++++++++++- tests/unix/test_platform_unix.cpp | 14 +-- 29 files changed, 299 insertions(+), 223 deletions(-) create mode 100644 tests/mock_subnet_utils.h diff --git a/include/multipass/platform.h b/include/multipass/platform.h index 4c953b1838..0b5c3cbce3 100644 --- a/include/multipass/platform.h +++ b/include/multipass/platform.h @@ -80,6 +80,7 @@ class Platform : public Singleton [[nodiscard]] virtual std::string bridge_nomenclature() const; [[nodiscard]] virtual bool can_reach_gateway(IPAddress ip) const; [[nodiscard]] virtual bool subnet_used_locally(Subnet subnet) const; + [[nodiscard]] virtual std::optional virtual_switch_subnet(const QString& bridge_name) const; virtual int get_cpus() const; virtual long long get_total_ram() const; diff --git a/include/multipass/subnet.h b/include/multipass/subnet.h index 4e4977f2b8..822c72b7a1 100644 --- a/include/multipass/subnet.h +++ b/include/multipass/subnet.h @@ -47,6 +47,8 @@ class Subnet [[nodiscard]] bool contains(Subnet other) const; [[nodiscard]] bool contains(IPAddress ip) const; + [[nodiscard]] bool operator==(const Subnet& other) const; + [[nodiscard]] bool operator<(const Subnet& other) const; private: IPAddress id; uint8_t cidr; diff --git a/src/network/subnet.cpp b/src/network/subnet.cpp index 1e1fe605fc..8231cf8bac 100644 --- a/src/network/subnet.cpp +++ b/src/network/subnet.cpp @@ -15,6 +15,9 @@ * */ +#include "multipass/platform.h" + +#include #include #include @@ -88,6 +91,28 @@ catch (const std::out_of_range& e) } return ip; } + +mp::Subnet generate_random_subnet(uint8_t cidr, mp::Subnet range) +{ + if (cidr >= 31) + throw std::invalid_argument(fmt::format(large_CIDR_err_fmt, cidr)); + + if (cidr < range.CIDR()) + throw std::logic_error(fmt::format("A subnet with cidr {} cannot be contained by {}", cidr, range.as_string())); + + // ex. 2^(24 - 16) = 256, [192.168.0.0/24, 192.168.255.0/24] + const size_t possibleSubnets = std::size_t{1} << (cidr - range.CIDR()); + + // narrowing conversion, possibleSubnets is guaranteed to be < 2^31 (4 bytes is safe) + static_assert(sizeof(decltype(MP_UTILS.random_int(0, possibleSubnets))) >= 4); + + const auto subnet = static_cast(MP_UTILS.random_int(0, possibleSubnets - 1)); + + // ex. 192.168.0.0 + (4 * 2^(32 - 24)) = 192.168.0.0 + 1024 = 192.168.4.0 + mp::IPAddress id = range.identifier() + (subnet * (std::size_t{1} << (32 - cidr))); + + return mp::Subnet{id, cidr}; +} } // namespace mp::Subnet::Subnet(IPAddress ip, uint8_t cidr) : id(apply_mask(ip, cidr)), cidr(cidr) @@ -155,29 +180,59 @@ bool mp::Subnet::contains(IPAddress ip) const return identifier() <= ip && (max_address() + 1) >= ip; } -mp::Subnet mp::SubnetUtils::generate_random_subnet(uint8_t cidr, Subnet range) const +bool mp::Subnet::operator==(const Subnet& other) const { - if (cidr >= 31) - throw std::invalid_argument(fmt::format(large_CIDR_err_fmt, cidr)); - - if (cidr < range.CIDR()) - throw std::logic_error(fmt::format("A subnet with cidr {} cannot be contained by {}", cidr, range.as_string())); + return id == other.id && cidr == other.cidr; +} - // ex. 2^(24 - 16) = 256, [192.168.0.0/24, 192.168.255.0/24] - const size_t possibleSubnets = std::size_t{1} << (cidr - range.CIDR()); +bool mp::Subnet::operator<(const Subnet& other) const +{ + // note cidr comparison is flipped, smaller is bigger + return id < other.id || (id == other.id && cidr > other.cidr); +} - // narrowing conversion, possibleSubnets is guarenteed to be < 2^31 (4 bytes is safe) - static_assert(sizeof(decltype(MP_UTILS.random_int(0, possibleSubnets))) >= 4); - const auto subnet = static_cast(MP_UTILS.random_int(0, possibleSubnets - 1)); +mp::Subnet mp::SubnetUtils::generate_random_subnet(uint8_t cidr, Subnet range) const +{ + // @TODO don't rely on pure randomness + for (auto i = 0; i < 100; ++i) + { + const auto subnet = ::generate_random_subnet(cidr, range); + if (MP_PLATFORM.subnet_used_locally(subnet)) + continue; - // ex. 192.168.0.0 + (4 * 2^(32 - 24)) = 192.168.0.0 + 1024 = 192.168.4.0 - mp::IPAddress id = range.identifier() + (subnet * (std::size_t{1} << (32 - cidr))); + if (MP_PLATFORM.can_reach_gateway(subnet.min_address())) + continue; - return mp::Subnet{id, cidr}; + if (MP_PLATFORM.can_reach_gateway(subnet.max_address())) + continue; + + return subnet; + } + + throw std::runtime_error("Could not determine a subnet for networking."); } mp::Subnet mp::SubnetUtils::get_subnet(const mp::Path& network_dir, const QString& bridge_name) const { - return mp::Subnet{""}; + if (auto subnet = MP_PLATFORM.virtual_switch_subnet(bridge_name)) + return *subnet; + + QFile subnet_file{network_dir + "/multipass_subnet_" + bridge_name}; + MP_FILEOPS.open(subnet_file, QIODevice::ReadWrite | QIODevice::Text); + if (MP_FILEOPS.size(subnet_file) > 0) + { + const auto content = MP_FILEOPS.read_all(subnet_file).trimmed().toStdString(); + if (content.find('/') != std::string::npos) + { + return Subnet{content}; + } + // assume CIDR of 24 is missing (for backwards compatability) + return Subnet{IPAddress{content}, 24}; + } + + auto new_subnet = MP_SUBNET_UTILS.generate_random_subnet(); + const auto subnet_str = new_subnet.as_string(); + MP_FILEOPS.write(subnet_file, subnet_str.data(), subnet_str.size()); + return new_subnet; } diff --git a/src/platform/backends/qemu/linux/dnsmasq_process_spec.cpp b/src/platform/backends/qemu/linux/dnsmasq_process_spec.cpp index 2f95cd0cdd..e6c0cd206e 100644 --- a/src/platform/backends/qemu/linux/dnsmasq_process_spec.cpp +++ b/src/platform/backends/qemu/linux/dnsmasq_process_spec.cpp @@ -26,14 +26,14 @@ namespace mpu = multipass::utils; namespace { -[[nodiscard]] QStringList make_dnsmasq_subnet_args(const mp::SubnetList& subnets) +[[nodiscard]] QStringList make_dnsmasq_subnet_args(const mp::BridgeSubnetList& subnets) { QStringList out{}; for (const auto& [bridge_name, subnet] : subnets) { - const auto bridge_addr = mp::IPAddress{fmt::format("{}.1", subnet)}; - const auto start_ip = mp::IPAddress{fmt::format("{}.2", subnet)}; - const auto end_ip = mp::IPAddress{fmt::format("{}.254", subnet)}; + const auto bridge_addr = subnet.min_address(); + const auto start_ip = bridge_addr + 1; + const auto end_ip = subnet.max_address(); out << QString("--interface=%1").arg(bridge_name) << QString("--listen-address=%1").arg(QString::fromStdString(bridge_addr.as_string())) @@ -48,7 +48,7 @@ namespace } // namespace mp::DNSMasqProcessSpec::DNSMasqProcessSpec(const mp::Path& data_dir, - const SubnetList& subnets, + const BridgeSubnetList& subnets, const QString& conf_file_path) : data_dir(data_dir), subnets(subnets), conf_file_path{conf_file_path} { diff --git a/src/platform/backends/qemu/linux/dnsmasq_process_spec.h b/src/platform/backends/qemu/linux/dnsmasq_process_spec.h index e37d336622..83cc95a7d3 100644 --- a/src/platform/backends/qemu/linux/dnsmasq_process_spec.h +++ b/src/platform/backends/qemu/linux/dnsmasq_process_spec.h @@ -31,7 +31,7 @@ class DNSMasqProcessSpec : public ProcessSpec { public: explicit DNSMasqProcessSpec(const Path& data_dir, - const SubnetList& subnets, + const BridgeSubnetList& subnets, const QString& conf_file_path); QString program() const override; @@ -42,7 +42,7 @@ class DNSMasqProcessSpec : public ProcessSpec private: const Path data_dir; - const SubnetList subnets; + const BridgeSubnetList subnets; const QString conf_file_path; }; diff --git a/src/platform/backends/qemu/linux/dnsmasq_server.cpp b/src/platform/backends/qemu/linux/dnsmasq_server.cpp index f9ce890e33..606233b8ed 100644 --- a/src/platform/backends/qemu/linux/dnsmasq_server.cpp +++ b/src/platform/backends/qemu/linux/dnsmasq_server.cpp @@ -37,7 +37,7 @@ namespace constexpr auto immediate_wait = 100; // period to wait for immediate dnsmasq failures, in ms auto make_dnsmasq_process(const mp::Path& data_dir, - const mp::SubnetList& subnets, + const mp::BridgeSubnetList& subnets, const QString& conf_file_path) { auto process_spec = std::make_unique(data_dir, subnets, conf_file_path); @@ -45,7 +45,7 @@ auto make_dnsmasq_process(const mp::Path& data_dir, } } // namespace -mp::DNSMasqServer::DNSMasqServer(const Path& data_dir, const SubnetList& subnets) +mp::DNSMasqServer::DNSMasqServer(const Path& data_dir, const BridgeSubnetList& subnets) : data_dir{data_dir}, conf_file{QDir(data_dir).absoluteFilePath("dnsmasq-XXXXXX.conf")} { conf_file.open(); @@ -205,7 +205,7 @@ void mp::DNSMasqServer::start_dnsmasq() mp::DNSMasqServer::UPtr mp::DNSMasqServerFactory::make_dnsmasq_server( const mp::Path& network_dir, - const SubnetList& subnets) const + const BridgeSubnetList& subnets) const { return std::make_unique(network_dir, subnets); } diff --git a/src/platform/backends/qemu/linux/dnsmasq_server.h b/src/platform/backends/qemu/linux/dnsmasq_server.h index b3b58e20eb..d59196a749 100644 --- a/src/platform/backends/qemu/linux/dnsmasq_server.h +++ b/src/platform/backends/qemu/linux/dnsmasq_server.h @@ -21,6 +21,7 @@ #include #include #include +#include #include @@ -32,14 +33,14 @@ namespace multipass { class Process; -using SubnetList = std::vector>; +using BridgeSubnetList = std::vector>; class DNSMasqServer : private DisabledCopyMove { public: using UPtr = std::unique_ptr; - DNSMasqServer(const Path& data_dir, const SubnetList& subnets); + DNSMasqServer(const Path& data_dir, const BridgeSubnetList& subnets); virtual ~DNSMasqServer(); // inherited by mock for testing virtual std::optional get_ip_for(const std::string& hw_addr); @@ -67,6 +68,6 @@ class DNSMasqServerFactory : public Singleton : Singleton::Singleton{pass} {}; virtual DNSMasqServer::UPtr make_dnsmasq_server(const Path& network_dir, - const SubnetList& subnets) const; + const BridgeSubnetList& subnets) const; }; } // namespace multipass diff --git a/src/platform/backends/qemu/linux/firewall_config.cpp b/src/platform/backends/qemu/linux/firewall_config.cpp index 212ac6a430..f77dc4ebfd 100644 --- a/src/platform/backends/qemu/linux/firewall_config.cpp +++ b/src/platform/backends/qemu/linux/firewall_config.cpp @@ -392,10 +392,10 @@ QString detect_firewall() } } // namespace -mp::FirewallConfig::FirewallConfig(const QString& bridge_name, const std::string& subnet) +mp::FirewallConfig::FirewallConfig(const QString& bridge_name, const mp::Subnet& subnet) : firewall{detect_firewall()}, bridge_name{bridge_name}, - cidr{QString("%1.0/24").arg(QString::fromStdString(subnet))}, + cidr{QString::fromStdString(subnet.as_string())}, comment{multipass_firewall_comment(bridge_name)} { try @@ -437,7 +437,7 @@ void mp::FirewallConfig::clear_all_firewall_rules() mp::FirewallConfig::UPtr mp::FirewallConfigFactory::make_firewall_config( const QString& bridge_name, - const std::string& subnet) const + const mp::Subnet& subnet) const { return std::make_unique(bridge_name, subnet); } diff --git a/src/platform/backends/qemu/linux/firewall_config.h b/src/platform/backends/qemu/linux/firewall_config.h index eec65a1d5c..a8f4042506 100644 --- a/src/platform/backends/qemu/linux/firewall_config.h +++ b/src/platform/backends/qemu/linux/firewall_config.h @@ -18,6 +18,7 @@ #pragma once #include +#include #include @@ -30,7 +31,7 @@ class FirewallConfig public: using UPtr = std::unique_ptr; - FirewallConfig(const QString& bridge_name, const std::string& subnet); + FirewallConfig(const QString& bridge_name, const Subnet& subnet); virtual ~FirewallConfig(); virtual void verify_firewall_rules(); @@ -59,6 +60,6 @@ class FirewallConfigFactory : public Singleton : Singleton::Singleton{pass} {}; virtual FirewallConfig::UPtr make_firewall_config(const QString& bridge_name, - const std::string& subnet) const; + const Subnet& subnet) const; }; } // namespace multipass diff --git a/src/platform/backends/qemu/linux/qemu_platform_detail.h b/src/platform/backends/qemu/linux/qemu_platform_detail.h index a4d571d6ea..905378b75f 100644 --- a/src/platform/backends/qemu/linux/qemu_platform_detail.h +++ b/src/platform/backends/qemu/linux/qemu_platform_detail.h @@ -48,23 +48,23 @@ class QemuPlatformDetail : public QemuPlatform private: // explicitly naming DisabledCopyMove since the private one derived from QemuPlatform takes // precedence in lookup - struct Subnet : private multipass::DisabledCopyMove + struct BridgeSubnet : private multipass::DisabledCopyMove { const QString bridge_name; - const std::string subnet; + const Subnet subnet; FirewallConfig::UPtr firewall_config; - Subnet(const Path& network_dir, const std::string& name); - ~Subnet(); + BridgeSubnet(const Path& network_dir, const std::string& name); + ~BridgeSubnet(); }; - using Subnets = std::unordered_map; + using BridgeSubnets = std::unordered_map; - [[nodiscard]] static Subnets get_subnets(const Path& network_dir); + [[nodiscard]] static BridgeSubnets get_subnets(const Path& network_dir); - [[nodiscard]] static SubnetList get_subnets_list(const Subnets&); + [[nodiscard]] static BridgeSubnetList get_subnets_list(const BridgeSubnets&); const Path network_dir; - const Subnets subnets; + const BridgeSubnets subnets; DNSMasqServer::UPtr dnsmasq_server; std::unordered_map> name_to_net_device_map; diff --git a/src/platform/backends/qemu/linux/qemu_platform_detail_linux.cpp b/src/platform/backends/qemu/linux/qemu_platform_detail_linux.cpp index 0bddab00b7..e28f35add6 100644 --- a/src/platform/backends/qemu/linux/qemu_platform_detail_linux.cpp +++ b/src/platform/backends/qemu/linux/qemu_platform_detail_linux.cpp @@ -66,13 +66,13 @@ void remove_tap_device(const QString& tap_device_name) } } -void create_virtual_switch(const std::string& subnet, const QString& bridge_name) +void create_virtual_switch(const mp::Subnet& subnet, const QString& bridge_name) { if (!MP_UTILS.run_cmd_for_status("ip", {"addr", "show", bridge_name})) { const auto mac_address = mp::utils::generate_mac_address(); - const auto cidr = fmt::format("{}.1/24", subnet); - const auto broadcast = fmt::format("{}.255", subnet); + const auto cidr = subnet.as_string(); + const auto broadcast = (subnet.max_address() + 1).as_string(); MP_UTILS.run_cmd_for_status( "ip", @@ -100,7 +100,7 @@ void set_ip_forward() } } -mp::DNSMasqServer::UPtr init_nat_network(const mp::Path& network_dir, const mp::SubnetList& subnets) +mp::DNSMasqServer::UPtr init_nat_network(const mp::Path& network_dir, const mp::BridgeSubnetList& subnets) { set_ip_forward(); return MP_DNSMASQ_SERVER_FACTORY.make_dnsmasq_server(network_dir, subnets); @@ -115,23 +115,23 @@ void delete_virtual_switch(const QString& bridge_name) } } // namespace -mp::QemuPlatformDetail::Subnet::Subnet(const Path& network_dir, const std::string& name) +mp::QemuPlatformDetail::BridgeSubnet::BridgeSubnet(const Path& network_dir, const std::string& name) : bridge_name{multipass_bridge_name.arg(name.c_str())}, - subnet{MP_BACKEND.get_subnet(network_dir, bridge_name)}, + subnet{MP_SUBNET_UTILS.get_subnet(network_dir, bridge_name)}, firewall_config{MP_FIREWALL_CONFIG_FACTORY.make_firewall_config(bridge_name, subnet)} { create_virtual_switch(subnet, bridge_name); } -mp::QemuPlatformDetail::Subnet::~Subnet() +mp::QemuPlatformDetail::BridgeSubnet::~BridgeSubnet() { delete_virtual_switch(bridge_name); } -[[nodiscard]] mp::QemuPlatformDetail::Subnets mp::QemuPlatformDetail::get_subnets( +[[nodiscard]] mp::QemuPlatformDetail::BridgeSubnets mp::QemuPlatformDetail::get_subnets( const Path& network_dir) { - Subnets subnets{}; + BridgeSubnets subnets{}; subnets.reserve(default_zone_names.size()); for (const auto& zone : default_zone_names) @@ -144,9 +144,9 @@ mp::QemuPlatformDetail::Subnet::~Subnet() return subnets; } -[[nodiscard]] mp::SubnetList mp::QemuPlatformDetail::get_subnets_list(const Subnets& subnets) +[[nodiscard]] mp::BridgeSubnetList mp::QemuPlatformDetail::get_subnets_list(const BridgeSubnets& subnets) { - SubnetList out{}; + BridgeSubnetList out{}; out.reserve(subnets.size()); for (const auto& [_, subnet] : subnets) diff --git a/src/platform/backends/shared/linux/backend_utils.cpp b/src/platform/backends/shared/linux/backend_utils.cpp index 2da30cb3fd..71b67809b0 100644 --- a/src/platform/backends/shared/linux/backend_utils.cpp +++ b/src/platform/backends/shared/linux/backend_utils.cpp @@ -17,6 +17,7 @@ #include "backend_utils.h" #include "dbus_wrappers.h" +#include "multipass/subnet.h" #include "process_factory.h" #include @@ -63,32 +64,6 @@ const auto nm_settings_ifc = QStringLiteral("org.freedesktop.NetworkManager.Sett const auto nm_connection_ifc = QStringLiteral("org.freedesktop.NetworkManager.Settings.Connection"); constexpr auto max_bridge_name_len = 15; // maximum number of characters in a bridge name -auto virtual_switch_subnet(const QString& bridge_name) -{ - // CLI equivalent: ip -4 route show | grep ${BRIDGE_NAME} | cut -d ' ' -f1 | cut -d '.' -f1-3 - QString subnet; - - const auto output = - QString::fromStdString(MP_UTILS.run_cmd_for_output("ip", {"-4", "route", "show"})) - .split('\n'); - for (const auto& line : output) - { - if (line.contains(bridge_name)) - { - subnet = line.section('.', 0, 2); - break; - } - } - - if (subnet.isNull()) - { - mpl::info("daemon", - "Unable to determine subnet for the {} subnet", - qUtf8Printable(bridge_name)); - } - return subnet.toStdString(); -} - const mpdbus::DBusConnection& get_checked_system_bus() { const auto& system_bus = mpdbus::DBusProvider::instance().get_system_bus(); @@ -177,28 +152,6 @@ auto make_bridge_rollback_guard(std::string_view log_category, }); } -std::string generate_random_subnet() -{ - // TODO don't rely on pure randomness - for (auto i = 0; i < 100; ++i) - { - auto subnet = - fmt::format("10.{}.{}", MP_UTILS.random_int(0, 255), MP_UTILS.random_int(0, 255)); - if (subnet_used_locally(subnet)) - continue; - - if (can_reach_gateway(fmt::format("{}.1", subnet))) - continue; - - if (can_reach_gateway(fmt::format("{}.254", subnet))) - continue; - - return subnet; - } - - throw std::runtime_error("Could not determine a subnet for networking."); -} - } // namespace // @precondition no bridge exists for this interface @@ -257,22 +210,6 @@ std::string mp::Backend::create_bridge_with(const std::string& interface) return ret; } -std::string mp::Backend::get_subnet(const mp::Path& network_dir, const QString& bridge_name) const -{ - auto subnet = virtual_switch_subnet(bridge_name); - if (!subnet.empty()) - return subnet; - - QFile subnet_file{network_dir + "/multipass_subnet_" + bridge_name}; - MP_FILEOPS.open(subnet_file, QIODevice::ReadWrite | QIODevice::Text); - if (MP_FILEOPS.size(subnet_file) > 0) - return MP_FILEOPS.read_all(subnet_file).trimmed().toStdString(); - - auto new_subnet = generate_random_subnet(); - MP_FILEOPS.write(subnet_file, new_subnet.data(), new_subnet.length()); - return new_subnet; -} - void mp::Backend::check_for_kvm_support() { QFile kvm_device{"/dev/kvm"}; diff --git a/src/platform/backends/shared/linux/backend_utils.h b/src/platform/backends/shared/linux/backend_utils.h index 64ebf0f176..9eb89ee569 100644 --- a/src/platform/backends/shared/linux/backend_utils.h +++ b/src/platform/backends/shared/linux/backend_utils.h @@ -51,7 +51,6 @@ class Backend : public Singleton using Singleton::Singleton; virtual std::string create_bridge_with(const std::string& interface); - virtual std::string get_subnet(const Path& network_dir, const QString& bridge_name) const; // For detecting KVM virtual void check_for_kvm_support(); diff --git a/src/platform/platform_unix.cpp b/src/platform/platform_unix.cpp index ddaff6bd88..9cef983c4a 100644 --- a/src/platform/platform_unix.cpp +++ b/src/platform/platform_unix.cpp @@ -277,10 +277,38 @@ bool mp::platform::Platform::can_reach_gateway(mp::IPAddress ip) const return MP_UTILS.run_cmd_for_status("ping", {"-n", "-q", ipstr.c_str(), "-c", "1", "-W", "1"}); } +std::optional mp::platform::Platform::virtual_switch_subnet(const QString& bridge_name) const +{ + // CLI equivalent: ip -4 route show | grep ${BRIDGE_NAME} | cut -d ' ' -f1 | cut -d '.' -f1-3 + QString subnet; + + const auto output = + QString::fromStdString(MP_UTILS.run_cmd_for_output("ip", {"-4", "route", "show"})) + .split('\n'); + for (const auto& line : output) + { + if (line.contains(bridge_name)) + { + subnet = line.section('.', 0, 2); + break; + } + } + + if (subnet.isNull()) + { + mpl::log(mpl::Level::info, + "daemon", + fmt::format("Unable to determine subnet for the {} subnet", + qUtf8Printable(bridge_name))); + return std::nullopt; + } + return subnet.toStdString(); +} + bool mp::platform::Platform::subnet_used_locally(mp::Subnet subnet) const { // validation of the ip and cidr happen later, otherwise this regex would be massive. - static const QRegularExpression subnet_regex(R"(^((?:[0-9][0-9]?[0-9]?\.){3}[0-9][0-9]?[0-9]?\/[0-9][0-9]?))"); + static const QRegularExpression subnet_regex(R"(((?:[0-9][0-9]?[0-9]?\.){3}[0-9][0-9]?[0-9]?\/[0-9][0-9]?))"); const auto output = QString::fromStdString(MP_UTILS.run_cmd_for_output("ip", {"-4", "route", "show"})); @@ -306,7 +334,7 @@ bool mp::platform::Platform::subnet_used_locally(mp::Subnet subnet) const mpl::log( mpl::Level::warning, "network", - fmt::format("invalid subnet from ip command: {}", e.message())); + fmt::format("invalid subnet from ip command: {}", e.what())); } } return false; diff --git a/src/platform/platform_win.cpp b/src/platform/platform_win.cpp index 9519229efc..ef43a5c40f 100644 --- a/src/platform/platform_win.cpp +++ b/src/platform/platform_win.cpp @@ -641,6 +641,12 @@ bool mp::platform::Platform::subnet_used_locally(mp::Subnet subnet) const throw mp::NotImplementedOnThisBackendException{"AZs @TODO"}; } + +std::optional mp::platform::Platform::virtual_switch_subnet(const QString& bridge_name) const +{ + throw mp::NotImplementedOnThisBackendException{"AZs @TODO"}; +} + QString mp::platform::Platform::daemon_config_home() const // temporary { auto ret = systemprofile_app_data_path(); diff --git a/src/utils/utils.cpp b/src/utils/utils.cpp index 3ce8a3471e..eca0e03292 100644 --- a/src/utils/utils.cpp +++ b/src/utils/utils.cpp @@ -242,7 +242,7 @@ std::vector mp::utils::split(const std::string& string, const std:: std::string mp::utils::generate_mac_address() { - std::array octets{ + std::array octets{ {MP_UTILS.random_int(0, 255), MP_UTILS.random_int(0, 255), MP_UTILS.random_int(0, 255)}}; return fmt::format("52:54:00:{:02x}:{:02x}:{:02x}", octets[0], octets[1], octets[2]); } diff --git a/tests/linux/test_backend_utils.cpp b/tests/linux/test_backend_utils.cpp index f0c98ca975..60ffefb576 100644 --- a/tests/linux/test_backend_utils.cpp +++ b/tests/linux/test_backend_utils.cpp @@ -513,62 +513,3 @@ TEST(LinuxBackendUtils, linuxSyscallsReturnExpectedValues) EXPECT_EQ(MP_LINUX_SYSCALLS.close(null_fd), 0); } - -TEST(LinuxBackendUtils, getSubnetBridgeExistsReturnsExpectedData) -{ - const std::string test_subnet{"10.102.12"}; - const QString bridge_name{"test-bridge"}; - auto [mock_utils, guard] = mpt::MockUtils::inject(); - - EXPECT_CALL(*mock_utils, - run_cmd_for_output(QString("ip"), QStringList({"-4", "route", "show"}), _)) - .WillOnce( - Return(fmt::format("{}.0 dev {} proto kernel scope link", test_subnet, bridge_name))); - - EXPECT_EQ(MP_BACKEND.get_subnet("foo", bridge_name), test_subnet); -} - -TEST(LinuxBackendUtils, getSubnetInFileReturnsExpectedData) -{ - const std::string test_subnet{"10.102.12"}; - const QString bridge_name{"test-bridge"}; - auto [mock_utils, utils_guard] = mpt::MockUtils::inject(); - auto [mock_file_ops, file_ops_guard] = mpt::MockFileOps::inject(); - - EXPECT_CALL(*mock_utils, - run_cmd_for_output(QString("ip"), QStringList({"-4", "route", "show"}), _)) - .WillOnce(Return("")); - - EXPECT_CALL(*mock_file_ops, open(_, _)).WillOnce(Return(true)); - EXPECT_CALL(*mock_file_ops, size(_)).WillOnce(Return(1)); - EXPECT_CALL(*mock_file_ops, read_all(_)) - .WillOnce(Return(QByteArray::fromStdString(test_subnet))); - - EXPECT_EQ(MP_BACKEND.get_subnet("foo", bridge_name), test_subnet); -} - -TEST(LinuxBackendUtils, getSubnetNotInFileWritesNewSubnetReturnsExpectedData) -{ - const QString bridge_name{"test-bridge"}; - std::string generated_subnet; - auto [mock_utils, utils_guard] = mpt::MockUtils::inject(); - auto [mock_file_ops, file_ops_guard] = mpt::MockFileOps::inject(); - - EXPECT_CALL(*mock_utils, - run_cmd_for_output(QString("ip"), QStringList({"-4", "route", "show"}), _)) - .WillOnce(Return("")) - .WillOnce(Return("0.0.0.0")); - EXPECT_CALL(*mock_utils, run_cmd_for_status(QString("ping"), _, _)) - .WillRepeatedly(Return(false)); - - EXPECT_CALL(*mock_file_ops, open(_, _)).WillOnce(Return(true)); - EXPECT_CALL(*mock_file_ops, size(_)).WillOnce(Return(0)); - EXPECT_CALL(*mock_file_ops, write(A(), _, _)) - .WillOnce([&generated_subnet](auto&, auto data, auto) { - generated_subnet = std::string(data); - - return generated_subnet.length(); - }); - - EXPECT_EQ(MP_BACKEND.get_subnet("foo", bridge_name), generated_subnet); -} diff --git a/tests/linux/test_platform_linux.cpp b/tests/linux/test_platform_linux.cpp index 6f8bc08b11..62befb59e8 100644 --- a/tests/linux/test_platform_linux.cpp +++ b/tests/linux/test_platform_linux.cpp @@ -69,7 +69,7 @@ struct PlatformLinux : public mpt::TestWithMockedBinPath mpt::StubAvailabilityZoneManager az_manager{}; decltype(mp::platform::vm_backend("", az_manager)) factory_ptr; - EXPECT_NO_THROW(factory_ptr = mp::platform::vm_backend(backend_path, az_manager);); + EXPECT_NO_THROW(factory_ptr = mp::platform::vm_backend(backend_path, az_manager)); EXPECT_TRUE(dynamic_cast(factory_ptr.get())); } diff --git a/tests/mock_backend_utils.h b/tests/mock_backend_utils.h index 849009fee7..16acfe664b 100644 --- a/tests/mock_backend_utils.h +++ b/tests/mock_backend_utils.h @@ -30,7 +30,6 @@ class MockBackend : public Backend using Backend::Backend; MOCK_METHOD(std::string, create_bridge_with, (const std::string&), (override)); - MOCK_METHOD(std::string, get_subnet, (const Path&, const QString&), (const, override)); MOCK_METHOD(void, check_for_kvm_support, (), (override)); MOCK_METHOD(void, check_if_kvm_is_in_use, (), (override)); diff --git a/tests/mock_platform.h b/tests/mock_platform.h index 5499d0e7c9..4f2766b347 100644 --- a/tests/mock_platform.h +++ b/tests/mock_platform.h @@ -66,6 +66,9 @@ class MockPlatform : public platform::Platform MOCK_METHOD(QString, default_privileged_mounts, (), (const, override)); MOCK_METHOD(QString, get_username, (), (const, override)); MOCK_METHOD(std::string, bridge_nomenclature, (), (const, override)); + MOCK_METHOD(bool, can_reach_gateway, (IPAddress), (const, override)); + MOCK_METHOD(bool, subnet_used_locally, (Subnet), (const, override)); + MOCK_METHOD(std::optional, virtual_switch_subnet, (const QString&), (const, override)); MOCK_METHOD(std::filesystem::path, get_root_cert_dir, (), (const, override)); MP_MOCK_SINGLETON_BOILERPLATE(MockPlatform, Platform); diff --git a/tests/mock_subnet_utils.h b/tests/mock_subnet_utils.h new file mode 100644 index 0000000000..50c24b0247 --- /dev/null +++ b/tests/mock_subnet_utils.h @@ -0,0 +1,36 @@ +/* +* Copyright (C) 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 . + * + */ + +#pragma once + +#include "mock_singleton_helpers.h" + +#include + +namespace multipass::test +{ +struct MockSubnetUtils : public SubnetUtils +{ + using SubnetUtils::SubnetUtils; + + MOCK_METHOD(Subnet, generate_random_subnet, (uint8_t cidr, Subnet range), (const, override)); + MOCK_METHOD(Subnet, get_subnet, (const Path& network_dir, const QString& bridge_name), (const, override)); + + MP_MOCK_SINGLETON_BOILERPLATE(MockSubnetUtils, SubnetUtils); +}; +} // namespace multipass::test + diff --git a/tests/qemu/linux/mock_dnsmasq_server.h b/tests/qemu/linux/mock_dnsmasq_server.h index 337dd90ea5..233c8f7416 100644 --- a/tests/qemu/linux/mock_dnsmasq_server.h +++ b/tests/qemu/linux/mock_dnsmasq_server.h @@ -41,7 +41,7 @@ struct MockDNSMasqServerFactory : public DNSMasqServerFactory MOCK_METHOD(DNSMasqServer::UPtr, make_dnsmasq_server, - (const Path&, (const SubnetList&)), + (const Path&, (const BridgeSubnetList&)), (const, override)); MP_MOCK_SINGLETON_BOILERPLATE(MockDNSMasqServerFactory, DNSMasqServerFactory); diff --git a/tests/qemu/linux/mock_firewall_config.h b/tests/qemu/linux/mock_firewall_config.h index 6d0db6eff3..b0deb73fbc 100644 --- a/tests/qemu/linux/mock_firewall_config.h +++ b/tests/qemu/linux/mock_firewall_config.h @@ -39,7 +39,7 @@ struct MockFirewallConfigFactory : public FirewallConfigFactory MOCK_METHOD(FirewallConfig::UPtr, make_firewall_config, - (const QString&, const std::string&), + (const QString&, const Subnet&), (const, override)); MP_MOCK_SINGLETON_BOILERPLATE(MockFirewallConfigFactory, FirewallConfigFactory); diff --git a/tests/qemu/linux/test_dnsmasq_process_spec.cpp b/tests/qemu/linux/test_dnsmasq_process_spec.cpp index 8d6d18aa0e..afc3c0e72c 100644 --- a/tests/qemu/linux/test_dnsmasq_process_spec.cpp +++ b/tests/qemu/linux/test_dnsmasq_process_spec.cpp @@ -32,7 +32,7 @@ using namespace testing; struct TestDnsmasqProcessSpec : public Test { const QString data_dir{"/data"}; - const mp::SubnetList subnets{{"bridgey", "1.2.3"}}; + const mp::BridgeSubnetList subnets{{"bridgey", mp::Subnet{"1.2.3.0/24"}}}; const QString conf_file_path{"/path/to/file.conf"}; }; diff --git a/tests/qemu/linux/test_dnsmasq_server.cpp b/tests/qemu/linux/test_dnsmasq_server.cpp index f2af4b82b0..e1a8c17054 100644 --- a/tests/qemu/linux/test_dnsmasq_server.cpp +++ b/tests/qemu/linux/test_dnsmasq_server.cpp @@ -75,7 +75,7 @@ struct DNSMasqServer : public mpt::TestWithMockedBinPath fmt::format( "0 {} {} dummy_name 00:01:02:03:04:05:06:07:08:09:0a:0b:0c:0d:0e:0f:10:11:12", expected_hw_addr, - expected_ip)); + expected_ip.as_string())); } void make_lease_entry() @@ -89,17 +89,14 @@ struct DNSMasqServer : public mpt::TestWithMockedBinPath std::shared_ptr logger = std::make_shared(); const QString dummy_bridge{"dummy-bridge"}; - const std::string default_subnet{"192.168.64"}; - const std::string error_subnet{ - "0.0.0"}; // This forces the mock dnsmasq process to exit with error + const mp::Subnet default_subnet{"192.168.64.0/24"}; + const mp::Subnet error_subnet{ + "0.0.0.0/24"}; // This forces the mock dnsmasq process to exit with error const std::string hw_addr{"00:01:02:03:04:05"}; - const std::string expected_ip{"10.177.224.22"}; - const std::string lease_entry = - "0 "s + hw_addr + " "s + expected_ip + - " dummy_name 00:01:02:03:04:05:06:07:08:09:0a:0b:0c:0d:0e:0f:10:11:12"; + const mp::IPAddress expected_ip{"10.177.224.22"}; - [[nodiscard]] static mp::SubnetList make_subnets(const QString& bridge, - const std::string& subnet) + [[nodiscard]] static mp::BridgeSubnetList make_subnets(const QString& bridge, + const mp::Subnet& subnet) { return {{bridge, subnet}}; } @@ -204,7 +201,7 @@ TEST_F(DNSMasqServer, releaseMacCrashesLogsFailure) EXPECT_THAT(logger->logged_lines, Contains(fmt::format("failed to release ip addr {} with mac {}: Crashed", - expected_ip, + expected_ip.as_string(), crash_hw_addr))); } diff --git a/tests/qemu/linux/test_firewall_config.cpp b/tests/qemu/linux/test_firewall_config.cpp index 4c5f31ef67..2a7bd9d426 100644 --- a/tests/qemu/linux/test_firewall_config.cpp +++ b/tests/qemu/linux/test_firewall_config.cpp @@ -45,7 +45,7 @@ struct FirewallConfig : public Test const QString goodbr0{QStringLiteral("goodbr0")}; const QString evilbr0{QStringLiteral("evilbr0")}; - const std::string subnet{"192.168.2"}; + const mp::Subnet subnet{"192.168.2.0/24"}; mpt::MockLogger::Scope logger_scope = mpt::MockLogger::inject(); }; @@ -130,10 +130,10 @@ TEST_F(FirewallConfig, firewallErrorThrowsOnVerify) TEST_F(FirewallConfig, dtorDeletesKnownRules) { const QByteArray base_rule{ - fmt::format("POSTROUTING -s {}.0/24 ! -d {}.0/24 -m comment --comment \"generated for " + fmt::format("POSTROUTING -s {} ! -d {} -m comment --comment \"generated for " "Multipass network {}\" -j MASQUERADE", - subnet, - subnet, + subnet.as_string(), + subnet.as_string(), goodbr0) .data()}; const QByteArray full_rule{"-A " + base_rule}; @@ -165,10 +165,10 @@ TEST_F(FirewallConfig, dtorDeletesKnownRules) TEST_F(FirewallConfig, dtorDeleteErrorLogsErrorAndContinues) { const QByteArray base_rule{ - fmt::format("POSTROUTING -s {}.0/24 ! -d {}.0/24 -m comment --comment \"generated for " + fmt::format("POSTROUTING -s {} ! -d {} -m comment --comment \"generated for " "Multipass network {}\" -j MASQUERADE", - subnet, - subnet, + subnet.as_string(), + subnet.as_string(), goodbr0) .data()}; const QByteArray full_rule{"-A " + base_rule}; diff --git a/tests/qemu/linux/test_qemu_platform_detail.cpp b/tests/qemu/linux/test_qemu_platform_detail.cpp index ba9bf5a498..8081f44a40 100644 --- a/tests/qemu/linux/test_qemu_platform_detail.cpp +++ b/tests/qemu/linux/test_qemu_platform_detail.cpp @@ -23,6 +23,7 @@ #include "tests/mock_file_ops.h" #include "tests/mock_logger.h" #include "tests/mock_process_factory.h" +#include "tests/mock_subnet_utils.h" #include "tests/mock_utils.h" #include "tests/temp_dir.h" @@ -50,13 +51,13 @@ struct QemuPlatformDetail : public Test for (const auto& vswitch : switches) { - EXPECT_CALL(*mock_backend, get_subnet(_, vswitch.bridge_name)) + EXPECT_CALL(*mock_subnet_utils, get_subnet(_, vswitch.bridge_name)) .WillOnce([subnet = vswitch.subnet](auto...) { return subnet; }); EXPECT_CALL(*mock_firewall_config_factory, make_firewall_config(vswitch.bridge_name, vswitch.subnet)) .WillOnce( - [this, &vswitch](auto...) { return std::move(vswitch.mock_firewall_config); }); + [&vswitch](auto...) { return std::move(vswitch.mock_firewall_config); }); EXPECT_CALL( *mock_utils, @@ -79,13 +80,13 @@ struct QemuPlatformDetail : public Test { QString bridge_name; std::string hw_addr; - std::string subnet; + mp::Subnet subnet; std::string name; mutable std::unique_ptr mock_firewall_config; Switch(const QString& bridge_name, const std::string& hw_addr, - const std::string& subnet, + const mp::Subnet& subnet, const std::string& name) : bridge_name(bridge_name), hw_addr(hw_addr), @@ -101,9 +102,9 @@ struct QemuPlatformDetail : public Test } }; const std::vector switches{ - {"mpqemubrzone1", "52:54:00:6f:29:7e", "192.168.64", "foo"}, - {"mpqemubrzone2", "52:54:00:6f:29:7f", "192.168.96", "bar"}, - {"mpqemubrzone3", "52:54:00:6f:29:80", "192.168.128", "baz"}}; + {"mpqemubrzone1", "52:54:00:6f:29:7e", mp::Subnet{"192.168.64.0/24"}, "foo"}, + {"mpqemubrzone2", "52:54:00:6f:29:7f", mp::Subnet{"192.168.96.0/24"}, "bar"}, + {"mpqemubrzone3", "52:54:00:6f:29:80", mp::Subnet{"192.168.128.0/24"}, "baz"}}; mpt::TempDir data_dir; @@ -112,6 +113,9 @@ struct QemuPlatformDetail : public Test mpt::MockUtils::GuardedMock utils_attr{mpt::MockUtils::inject()}; mpt::MockUtils* mock_utils = utils_attr.first; + mpt::MockSubnetUtils::GuardedMock subnet_utils_attr{mpt::MockSubnetUtils::inject()}; + mpt::MockSubnetUtils* mock_subnet_utils = subnet_utils_attr.first; + mpt::MockBackend::GuardedMock backend_attr{mpt::MockBackend::inject()}; mpt::MockBackend* mock_backend = backend_attr.first; @@ -135,7 +139,8 @@ TEST_F(QemuPlatformDetail, ctorSetsUpExpectedVirtualSwitches) { for (const auto& vswitch : switches) { - const QString qstring_subnet{QString::fromStdString(vswitch.subnet)}; + const auto subnet{vswitch.subnet.as_string()}; + const auto broadcast = (vswitch.subnet.max_address() + 1).as_string(); EXPECT_CALL(*mock_utils, run_cmd_for_status(QString("ip"), @@ -152,11 +157,11 @@ TEST_F(QemuPlatformDetail, ctorSetsUpExpectedVirtualSwitches) run_cmd_for_status(QString("ip"), ElementsAre(QString("address"), QString("add"), - QString("%1.1/24").arg(qstring_subnet), + QString::fromStdString(subnet), QString("dev"), vswitch.bridge_name, "broadcast", - QString("%1.255").arg(qstring_subnet)), + QString::fromStdString(broadcast)), _)) .WillOnce(Return(true)); @@ -176,7 +181,7 @@ TEST_F(QemuPlatformDetail, getIpForReturnsExpectedInfo) { for (const auto& vswitch : switches) { - const mp::IPAddress ip_address{fmt::format("{}.5", vswitch.subnet)}; + const mp::IPAddress ip_address{vswitch.subnet.min_address() + 4}; EXPECT_CALL(*mock_dnsmasq_server, get_ip_for(vswitch.hw_addr)) .WillOnce([ip = ip_address](auto...) { return ip; }); } @@ -185,7 +190,7 @@ TEST_F(QemuPlatformDetail, getIpForReturnsExpectedInfo) for (const auto& vswitch : switches) { - const mp::IPAddress ip_address{fmt::format("{}.5", vswitch.subnet)}; + const mp::IPAddress ip_address{vswitch.subnet.min_address() + 4}; auto addr = qemu_platform_detail.get_ip_for(vswitch.hw_addr); ASSERT_TRUE(addr.has_value()); diff --git a/tests/test_subnet.cpp b/tests/test_subnet.cpp index 43fc9a5bc5..9d38e278a4 100644 --- a/tests/test_subnet.cpp +++ b/tests/test_subnet.cpp @@ -16,6 +16,7 @@ */ #include "common.h" +#include "mock_file_ops.h" #include "mock_platform.h" #include "mock_utils.h" @@ -265,13 +266,15 @@ struct SubnetUtils : public Test { SubnetUtils() { - + ON_CALL(*mock_platform, subnet_used_locally).WillByDefault(Return(false)); + ON_CALL(*mock_platform, can_reach_gateway).WillByDefault(Return(false)); + ON_CALL(*mock_platform, virtual_switch_subnet).WillByDefault(Return(std::nullopt)); } mpt::MockUtils::GuardedMock mock_utils_injection{mpt::MockUtils::inject()}; mpt::MockUtils* mock_utils = mock_utils_injection.first; - mpt::MockPlatform::GuardedMock mock_platform_injection = mpt::MockPlatform::inject(); + mpt::MockPlatform::GuardedMock mock_platform_injection = mpt::MockPlatform::inject(); mpt::MockPlatform* mock_platform = mock_platform_injection.first; }; @@ -348,3 +351,65 @@ TEST_F(SubnetUtils, generateRandomSubnetWorksAtUpperExtreme) EXPECT_EQ(subnetHigh.identifier(), mp::IPAddress{"255.255.255.252"}); EXPECT_EQ(subnetHigh.CIDR(), 30); } + +TEST_F(SubnetUtils, generateRandomSubnetGivesUp) +{ + mp::Subnet range("0.0.0.0/0"); + + EXPECT_CALL(*mock_utils, random_int(_, _)) + .WillRepeatedly(ReturnArg<0>()); + + EXPECT_CALL(*mock_platform, subnet_used_locally) + .WillRepeatedly(Return(true)); + + MP_EXPECT_THROW_THAT(std::ignore = MP_SUBNET_UTILS.generate_random_subnet(24, range), + std::runtime_error, mpt::match_what(HasSubstr("subnet"))); +} + +TEST_F(SubnetUtils, getSubnetBridgeExistsReturnsExpectedData) +{ + const mp::Subnet test_subnet{"10.102.12.0/24"}; + const QString bridge_name{"test-bridge"}; + auto [mock_utils, guard] = mpt::MockUtils::inject(); + + EXPECT_CALL(*mock_platform, virtual_switch_subnet(bridge_name)) + .WillOnce(Return(test_subnet)); + + EXPECT_EQ(MP_SUBNET_UTILS.get_subnet("foo", bridge_name).as_string(), test_subnet.as_string()); +} + +TEST_F(SubnetUtils, getSubnetInFileReturnsExpectedData) +{ + const mp::Subnet test_subnet{"10.102.12.0/24"}; + const QString bridge_name{"test-bridge"}; + auto [mock_utils, utils_guard] = mpt::MockUtils::inject(); + auto [mock_file_ops, file_ops_guard] = mpt::MockFileOps::inject(); + + EXPECT_CALL(*mock_file_ops, open(_, _)).WillOnce(Return(true)); + EXPECT_CALL(*mock_file_ops, size(_)).WillOnce(Return(1)); + EXPECT_CALL(*mock_file_ops, read_all(_)) + .WillOnce(Return(QByteArray::fromStdString(test_subnet.as_string()))); + + EXPECT_EQ(MP_SUBNET_UTILS.get_subnet("foo", bridge_name).as_string(), test_subnet.as_string()); +} + +TEST_F(SubnetUtils, getSubnetNotInFileWritesNewSubnetReturnsExpectedData) +{ + const QString bridge_name{"test-bridge"}; + std::string generated_subnet; + auto [mock_utils, utils_guard] = mpt::MockUtils::inject(); + auto [mock_file_ops, file_ops_guard] = mpt::MockFileOps::inject(); + + EXPECT_CALL(*mock_utils, random_int); + + EXPECT_CALL(*mock_file_ops, open(_, _)).WillOnce(Return(true)); + EXPECT_CALL(*mock_file_ops, size(_)).WillOnce(Return(0)); + EXPECT_CALL(*mock_file_ops, write(A(), _, _)) + .WillOnce([&generated_subnet](auto&, auto data, auto) { + generated_subnet = std::string(data); + + return generated_subnet.length(); + }); + + EXPECT_EQ(MP_SUBNET_UTILS.get_subnet("foo", bridge_name).as_string(), generated_subnet); +} diff --git a/tests/unix/test_platform_unix.cpp b/tests/unix/test_platform_unix.cpp index eb8c912b6f..20da6707e8 100644 --- a/tests/unix/test_platform_unix.cpp +++ b/tests/unix/test_platform_unix.cpp @@ -343,7 +343,7 @@ TEST_F(TestPlatformUnix, canReachGatewayRunsPingWithIP) auto [mock_utils, guard] = mpt::MockUtils::inject(); - EXPECT_CALL(*mock_utils, run_cmd_for_status(QString("ping"), Contains(StrEq(testIPstr.c_str())), _)).WillOnce(Return(true)).WillOnce(Return(false)); + EXPECT_CALL(*mock_utils, run_cmd_for_status(QString("ping"), Contains(QString::fromStdString(testIPstr)), _)).WillOnce(Return(true)).WillOnce(Return(false)); EXPECT_TRUE(MP_PLATFORM.can_reach_gateway(testIP)); EXPECT_FALSE(MP_PLATFORM.can_reach_gateway(testIP)); @@ -351,7 +351,7 @@ TEST_F(TestPlatformUnix, canReachGatewayRunsPingWithIP) TEST_F(TestPlatformUnix, subnetUsedLocallyDetectsUnused) { - const mp::IPAddress testSubnet{"192.168.1.0/24"}; + const mp::Subnet testSubnet{"192.168.1.0/24"}; auto [mock_utils, guard] = mpt::MockUtils::inject(); @@ -365,12 +365,12 @@ default via 192.168.0.0 dev wlo1 proto dhcp src 192.168.0.1 metric 600 192.168.123.0/24 dev virbr0 proto kernel scope link src 192.168.123.1 linkdown )")); - EXPECT_FALSE(MP_PLATFORM.subnet_used_locally(testIP)); + EXPECT_FALSE(MP_PLATFORM.subnet_used_locally(testSubnet)); } TEST_F(TestPlatformUnix, subnetUsedLocallyDetectsOverlapping) { - const mp::IPAddress testSubnet{"172.172.1.0/24"}; + const mp::Subnet testSubnet{"172.172.1.0/24"}; auto [mock_utils, guard] = mpt::MockUtils::inject(); @@ -384,12 +384,12 @@ default via 192.168.0.0 dev wlo1 proto dhcp src 192.168.0.1 metric 600 192.168.123.0/24 dev virbr0 proto kernel scope link src 192.168.123.1 linkdown )")); - EXPECT_TRUE(MP_PLATFORM.subnet_used_locally(testIP)); + EXPECT_TRUE(MP_PLATFORM.subnet_used_locally(testSubnet)); } TEST_F(TestPlatformUnix, subnetUsedLocallyDetectsConflicting) { - const mp::IPAddress testSubnet{"10.20.30.0/24"}; + const mp::Subnet testSubnet{"10.20.30.0/24"}; auto [mock_utils, guard] = mpt::MockUtils::inject(); @@ -403,5 +403,5 @@ default via 192.168.0.0 dev wlo1 proto dhcp src 192.168.0.1 metric 600 192.168.123.0/24 dev virbr0 proto kernel scope link src 192.168.123.1 linkdown )")); - EXPECT_TRUE(MP_PLATFORM.subnet_used_locally(testIP)); + EXPECT_TRUE(MP_PLATFORM.subnet_used_locally(testSubnet)); } From a60f38b6689e97c4c234e7b3605701c1745780bf Mon Sep 17 00:00:00 2001 From: trevor-shoe Date: Fri, 26 Sep 2025 05:53:26 -0400 Subject: [PATCH 09/28] [firewall] Update Linux firewall subnet usage Signed-off-by: trevor-shoe --- .../backends/qemu/linux/firewall_config.cpp | 35 ++++++++++--------- .../backends/qemu/linux/firewall_config.h | 17 +++++---- tests/qemu/linux/mock_firewall_config.h | 2 -- tests/qemu/linux/test_firewall_config.cpp | 14 ++++---- 4 files changed, 37 insertions(+), 31 deletions(-) diff --git a/src/platform/backends/qemu/linux/firewall_config.cpp b/src/platform/backends/qemu/linux/firewall_config.cpp index f77dc4ebfd..d01cdb1f63 100644 --- a/src/platform/backends/qemu/linux/firewall_config.cpp +++ b/src/platform/backends/qemu/linux/firewall_config.cpp @@ -175,9 +175,11 @@ auto get_firewall_rules(const QString& firewall, const QString& table) void set_firewall_rules(const QString& firewall, const QString& bridge_name, - const QString& cidr, + const mp::Subnet& cidr, const QString& comment) { + const QString cidr_str = QString::fromStdString(cidr.as_string()); + const QStringList comment_option{match, QStringLiteral("comment"), QStringLiteral("--comment"), @@ -232,14 +234,14 @@ void set_firewall_rules(const QString& firewall, nat, POSTROUTING, QStringList() - << source << cidr << destination << QStringLiteral("224.0.0.0/24") << jump + << source << cidr_str << destination << QStringLiteral("224.0.0.0/24") << jump << RETURN << comment_option); add_firewall_rule(firewall, nat, POSTROUTING, QStringList() - << source << cidr << destination << QStringLiteral("255.255.255.255/32") + << source << cidr_str << destination << QStringLiteral("255.255.255.255/32") << jump << RETURN << comment_option); // Masquerade all packets going from VMs to the LAN/Internet @@ -247,27 +249,27 @@ void set_firewall_rules(const QString& firewall, nat, POSTROUTING, QStringList() - << source << cidr << negate << destination << cidr << protocol << tcp + << source << cidr_str << negate << destination << cidr_str << protocol << tcp << jump << MASQUERADE << to_ports << port_range << comment_option); add_firewall_rule(firewall, nat, POSTROUTING, QStringList() - << source << cidr << negate << destination << cidr << protocol << udp + << source << cidr_str << negate << destination << cidr_str << protocol << udp << jump << MASQUERADE << to_ports << port_range << comment_option); add_firewall_rule(firewall, nat, POSTROUTING, - QStringList() << source << cidr << negate << destination << cidr << jump + QStringList() << source << cidr_str << negate << destination << cidr_str << jump << MASQUERADE << comment_option); // Allow established traffic to the private subnet add_firewall_rule(firewall, filter, FORWARD, - QStringList() << destination << cidr << out_interface << bridge_name << match + QStringList() << destination << cidr_str << out_interface << bridge_name << match << QStringLiteral("conntrack") << QStringLiteral("--ctstate") << QStringLiteral("RELATED,ESTABLISHED") << jump << ACCEPT << comment_option); @@ -276,7 +278,7 @@ void set_firewall_rules(const QString& firewall, add_firewall_rule(firewall, filter, FORWARD, - QStringList() << source << cidr << in_interface << bridge_name << jump + QStringList() << source << cidr_str << in_interface << bridge_name << jump << ACCEPT << comment_option); // Allow traffic between virtual machines @@ -305,14 +307,15 @@ void set_firewall_rules(const QString& firewall, void clear_firewall_rules_for(const QString& firewall, const QString& table, const QString& bridge_name, - const QString& cidr, + const mp::Subnet& cidr, const QString& comment) { + const QString cidr_str = QString::fromStdString(cidr.as_string()); auto rules = QString::fromUtf8(get_firewall_rules(firewall, table)); for (auto& rule : rules.split('\n')) { - if (rule.contains(comment) || rule.contains(bridge_name) || rule.contains(cidr)) + if (rule.contains(comment) || rule.contains(bridge_name) || rule.contains(cidr_str)) { // Remove the policy type since delete doesn't use that rule.remove(0, 3); @@ -392,10 +395,10 @@ QString detect_firewall() } } // namespace -mp::FirewallConfig::FirewallConfig(const QString& bridge_name, const mp::Subnet& subnet) +mp::BasicFirewallConfig::BasicFirewallConfig(const QString& bridge_name, const mp::Subnet& subnet) : firewall{detect_firewall()}, bridge_name{bridge_name}, - cidr{QString::fromStdString(subnet.as_string())}, + cidr{subnet}, comment{multipass_firewall_comment(bridge_name)} { try @@ -411,7 +414,7 @@ mp::FirewallConfig::FirewallConfig(const QString& bridge_name, const mp::Subnet& } } -mp::FirewallConfig::~FirewallConfig() +mp::BasicFirewallConfig::~BasicFirewallConfig() { if (!firewall.isEmpty()) { @@ -419,7 +422,7 @@ mp::FirewallConfig::~FirewallConfig() } } -void mp::FirewallConfig::verify_firewall_rules() +void mp::BasicFirewallConfig::verify_firewall_rules() { if (firewall_error) { @@ -427,7 +430,7 @@ void mp::FirewallConfig::verify_firewall_rules() } } -void mp::FirewallConfig::clear_all_firewall_rules() +void mp::BasicFirewallConfig::clear_all_firewall_rules() { for (const auto& table : firewall_tables) { @@ -439,5 +442,5 @@ mp::FirewallConfig::UPtr mp::FirewallConfigFactory::make_firewall_config( const QString& bridge_name, const mp::Subnet& subnet) const { - return std::make_unique(bridge_name, subnet); + return std::make_unique(bridge_name, subnet); } diff --git a/src/platform/backends/qemu/linux/firewall_config.h b/src/platform/backends/qemu/linux/firewall_config.h index a8f4042506..9cf2664626 100644 --- a/src/platform/backends/qemu/linux/firewall_config.h +++ b/src/platform/backends/qemu/linux/firewall_config.h @@ -31,20 +31,25 @@ class FirewallConfig public: using UPtr = std::unique_ptr; - FirewallConfig(const QString& bridge_name, const Subnet& subnet); - virtual ~FirewallConfig(); + virtual ~FirewallConfig() = default; - virtual void verify_firewall_rules(); + virtual void verify_firewall_rules() = 0; +}; + +class BasicFirewallConfig final : public FirewallConfig +{ +public: + BasicFirewallConfig(const QString& bridge_name, const Subnet& subnet); + ~BasicFirewallConfig() override; -protected: - FirewallConfig() = default; // for testing + void verify_firewall_rules() override; private: void clear_all_firewall_rules(); const QString firewall; const QString bridge_name; - const QString cidr; + const Subnet cidr; const QString comment; bool firewall_error{false}; diff --git a/tests/qemu/linux/mock_firewall_config.h b/tests/qemu/linux/mock_firewall_config.h index b0deb73fbc..424510c195 100644 --- a/tests/qemu/linux/mock_firewall_config.h +++ b/tests/qemu/linux/mock_firewall_config.h @@ -28,8 +28,6 @@ namespace test { struct MockFirewallConfig : public FirewallConfig { - using FirewallConfig::FirewallConfig; - MOCK_METHOD(void, verify_firewall_rules, (), (override)); }; diff --git a/tests/qemu/linux/test_firewall_config.cpp b/tests/qemu/linux/test_firewall_config.cpp index 2a7bd9d426..14f8214bb4 100644 --- a/tests/qemu/linux/test_firewall_config.cpp +++ b/tests/qemu/linux/test_firewall_config.cpp @@ -81,7 +81,7 @@ TEST_F(FirewallConfig, iptablesNftErrorLogsWarningUsesIptablesLegacyByDefault) logger_scope.mock_logger->expect_log(mpl::Level::warning, fmt::format("Failure: {}", error_msg)); - mp::FirewallConfig firewall_config{goodbr0, subnet}; + mp::BasicFirewallConfig firewall_config{goodbr0, subnet}; } TEST_F(FirewallConfig, firewallVerifyNoErrorDoesNotThrow) @@ -98,7 +98,7 @@ TEST_F(FirewallConfig, firewallVerifyNoErrorDoesNotThrow) auto factory = mpt::MockProcessFactory::Inject(); factory->register_callback(firewall_callback); - mp::FirewallConfig firewall_config{goodbr0, subnet}; + mp::BasicFirewallConfig firewall_config{goodbr0, subnet}; EXPECT_NO_THROW(firewall_config.verify_firewall_rules()); } @@ -120,7 +120,7 @@ TEST_F(FirewallConfig, firewallErrorThrowsOnVerify) auto factory = mpt::MockProcessFactory::Inject(); factory->register_callback(firewall_callback); - mp::FirewallConfig firewall_config{evilbr0, subnet}; + mp::BasicFirewallConfig firewall_config{evilbr0, subnet}; MP_EXPECT_THROW_THAT(firewall_config.verify_firewall_rules(), std::runtime_error, @@ -156,7 +156,7 @@ TEST_F(FirewallConfig, dtorDeletesKnownRules) factory->register_callback(firewall_callback); { - mp::FirewallConfig firewall_config{goodbr0, subnet}; + mp::BasicFirewallConfig firewall_config{goodbr0, subnet}; } EXPECT_TRUE(delete_called); @@ -198,7 +198,7 @@ TEST_F(FirewallConfig, dtorDeleteErrorLogsErrorAndContinues) logger_scope.mock_logger->expect_log(mpl::Level::error, msg.toStdString(), AnyNumber()); { - mp::FirewallConfig firewall_config{goodbr0, subnet}; + mp::BasicFirewallConfig firewall_config{goodbr0, subnet}; } } @@ -224,7 +224,7 @@ TEST_P(FirewallToUseTestSuite, usesExpectedFirewall) logger_scope.mock_logger->screen_logs(mpl::Level::info); logger_scope.mock_logger->expect_log(mpl::Level::info, std::get<0>(param)); - mp::FirewallConfig firewall_config{goodbr0, subnet}; + mp::BasicFirewallConfig firewall_config{goodbr0, subnet}; } INSTANTIATE_TEST_SUITE_P(FirewallConfig, @@ -262,7 +262,7 @@ TEST_P(KernelCheckTestSuite, usesIptablesAndLogsWithBadKernelInfo) logger_scope.mock_logger->expect_log(mpl::Level::info, "iptables-legacy"); logger_scope.mock_logger->expect_log(mpl::Level::warning, msg); - mp::FirewallConfig firewall_config{goodbr0, subnet}; + mp::BasicFirewallConfig firewall_config{goodbr0, subnet}; EXPECT_FALSE(nftables_called); } From 7ba9f3893976a337c08edd7aae6243aca6eb78c6 Mon Sep 17 00:00:00 2001 From: trevor-shoe Date: Fri, 26 Sep 2025 05:54:51 -0400 Subject: [PATCH 10/28] [network] Implement virtual_switch_subnet on Linux Signed-off-by: trevor-shoe --- src/platform/platform_linux.cpp | 71 +++++++++++++++++ src/platform/platform_osx.cpp | 17 +++++ src/platform/platform_unix.cpp | 69 ----------------- tests/linux/test_platform_linux.cpp | 113 ++++++++++++++++++++++++++++ tests/unix/test_platform_unix.cpp | 58 +------------- 5 files changed, 202 insertions(+), 126 deletions(-) diff --git a/src/platform/platform_linux.cpp b/src/platform/platform_linux.cpp index d2bdba6c81..a661d83f08 100644 --- a/src/platform/platform_linux.cpp +++ b/src/platform/platform_linux.cpp @@ -354,6 +354,77 @@ std::string mp::platform::Platform::bridge_nomenclature() const return br_nomenclature; } +bool mp::platform::Platform::can_reach_gateway(mp::IPAddress ip) const +{ + const auto ipstr = ip.as_string(); + return MP_UTILS.run_cmd_for_status("ping", {"-n", "-q", ipstr.c_str(), "-c", "1", "-W", "1"}); +} + +// validation of the ip and cidr happen later, otherwise this regex would be massive. +static const QRegularExpression subnet_regex(R"(((?:[0-9][0-9]?[0-9]?\.){3}[0-9][0-9]?[0-9]?\/[0-9][0-9]?))"); + +bool mp::platform::Platform::subnet_used_locally(mp::Subnet subnet) const +{ + const auto output = + QString::fromStdString(MP_UTILS.run_cmd_for_output("ip", {"-4", "route", "show"})); + + QRegularExpressionMatchIterator i = subnet_regex.globalMatch(output); + + while (i.hasNext()) + { + QRegularExpressionMatch match = i.next(); + + try + { + mp::Subnet found_net{match.captured(1).toStdString()}; + + // check for overlap + if (found_net.contains(subnet) || subnet.contains(found_net)) + { + return true; + } + } + catch (const std::invalid_argument& e) + { + mpl::log( + mpl::Level::warning, + "network", + fmt::format("invalid subnet from ip command: {}", e.what())); + } + } + return false; +} + +std::optional mp::platform::Platform::virtual_switch_subnet(const QString& bridge_name) const +{ + const auto outputs = + QString::fromStdString(MP_UTILS.run_cmd_for_output("ip", {"-4", "route", "show"})).split('\n'); + + for (const auto& line : outputs) + { + if (line.contains(bridge_name)) + { + QRegularExpressionMatch match = subnet_regex.match(line); + if (!match.hasMatch()) + continue; + + try + { + return mp::Subnet{match.captured(1).toStdString()}; + } + catch (const std::invalid_argument& e) + { + mpl::log( + mpl::Level::warning, + "network", + fmt::format("invalid subnet from ip command: {}", e.what())); + } + } + } + + return std::nullopt; +} + auto mp::platform::detail::get_network_interfaces_from(const QDir& sys_dir) -> std::map { diff --git a/src/platform/platform_osx.cpp b/src/platform/platform_osx.cpp index bce700bb61..a426ece95f 100644 --- a/src/platform/platform_osx.cpp +++ b/src/platform/platform_osx.cpp @@ -267,6 +267,23 @@ std::string mp::platform::Platform::bridge_nomenclature() const return br_nomenclature; } +bool mp::platform::Platform::can_reach_gateway(mp::IPAddress ip) const +{ + const auto ipstr = ip.as_string(); + return MP_UTILS.run_cmd_for_status("ping", {"-n", "-q", ipstr.c_str(), "-c", "1", "-t", "1"}); +} + +bool mp::platform::Platform::subnet_used_locally(mp::Subnet subnet) const +{ + throw mp::NotImplementedOnThisBackendException{"AZs @TODO"}; +} + + +std::optional mp::platform::Platform::virtual_switch_subnet(const QString& bridge_name) const +{ + throw mp::NotImplementedOnThisBackendException{"AZs @TODO"}; +} + QString mp::platform::Platform::daemon_config_home() const // temporary { auto ret = QStringLiteral("/var/root/Library/Preferences/"); diff --git a/src/platform/platform_unix.cpp b/src/platform/platform_unix.cpp index 9cef983c4a..3bb2f6e0e5 100644 --- a/src/platform/platform_unix.cpp +++ b/src/platform/platform_unix.cpp @@ -270,72 +270,3 @@ long long mp::platform::Platform::get_total_ram() const { return static_cast(sysconf(_SC_PHYS_PAGES)) * sysconf(_SC_PAGESIZE); } - -bool mp::platform::Platform::can_reach_gateway(mp::IPAddress ip) const -{ - const auto ipstr = ip.as_string(); - return MP_UTILS.run_cmd_for_status("ping", {"-n", "-q", ipstr.c_str(), "-c", "1", "-W", "1"}); -} - -std::optional mp::platform::Platform::virtual_switch_subnet(const QString& bridge_name) const -{ - // CLI equivalent: ip -4 route show | grep ${BRIDGE_NAME} | cut -d ' ' -f1 | cut -d '.' -f1-3 - QString subnet; - - const auto output = - QString::fromStdString(MP_UTILS.run_cmd_for_output("ip", {"-4", "route", "show"})) - .split('\n'); - for (const auto& line : output) - { - if (line.contains(bridge_name)) - { - subnet = line.section('.', 0, 2); - break; - } - } - - if (subnet.isNull()) - { - mpl::log(mpl::Level::info, - "daemon", - fmt::format("Unable to determine subnet for the {} subnet", - qUtf8Printable(bridge_name))); - return std::nullopt; - } - return subnet.toStdString(); -} - -bool mp::platform::Platform::subnet_used_locally(mp::Subnet subnet) const -{ - // validation of the ip and cidr happen later, otherwise this regex would be massive. - static const QRegularExpression subnet_regex(R"(((?:[0-9][0-9]?[0-9]?\.){3}[0-9][0-9]?[0-9]?\/[0-9][0-9]?))"); - - const auto output = - QString::fromStdString(MP_UTILS.run_cmd_for_output("ip", {"-4", "route", "show"})); - - QRegularExpressionMatchIterator i = subnet_regex.globalMatch(output); - - while (i.hasNext()) - { - QRegularExpressionMatch match = i.next(); - - try - { - mp::Subnet found_net{match.captured(1).toStdString()}; - - // check for overlap - if (found_net.contains(subnet) || subnet.contains(found_net)) - { - return true; - } - } - catch (const std::invalid_argument& e) - { - mpl::log( - mpl::Level::warning, - "network", - fmt::format("invalid subnet from ip command: {}", e.what())); - } - } - return false; -} diff --git a/tests/linux/test_platform_linux.cpp b/tests/linux/test_platform_linux.cpp index 62befb59e8..5687a0d846 100644 --- a/tests/linux/test_platform_linux.cpp +++ b/tests/linux/test_platform_linux.cpp @@ -684,4 +684,117 @@ TEST_F(PlatformLinux, testSnapMultipassCertLocation) EXPECT_NE(snap_location, unconfined_location); EXPECT_EQ(snap_location.filename(), unconfined_location.filename()); } + +TEST_F(PlatformLinux, subnetUsedLocallyDetectsUnused) +{ + const mp::Subnet testSubnet{"192.168.1.0/24"}; + + auto [mock_utils, guard] = mpt::MockUtils::inject(); + + EXPECT_CALL(*mock_utils, run_cmd_for_output(QString("ip"), _, _)).WillOnce(Return(R"( +default via 192.168.0.0 dev wlo1 proto dhcp src 192.168.0.1 metric 600 +10.20.30.0/24 dev lxdbr0 proto kernel scope link src 10.20.30.1 +10.192.168.0/24 dev mpqemubr0 proto kernel scop link src 10.192.168.1 linkdown +10.255.19.0/24 dev mpbr0 proto kernel scope link src 10.255.19.1 +172.172.0.0/16 dev docker0 proto kernel scope link src 172.172.0.1 linkdown +192.168.0.0/24 dev wlo1 proto kernel scope link src 192.168.0.1 metric 600 +192.168.123.0/24 dev virbr0 proto kernel scope link src 192.168.123.1 linkdown +)")); + + EXPECT_FALSE(MP_PLATFORM.subnet_used_locally(testSubnet)); +} + +TEST_F(PlatformLinux, subnetUsedLocallyDetectsOverlapping) +{ + const mp::Subnet testSubnet{"172.172.1.0/24"}; + + auto [mock_utils, guard] = mpt::MockUtils::inject(); + + EXPECT_CALL(*mock_utils, run_cmd_for_output(QString("ip"), _, _)).WillOnce(Return(R"( +default via 192.168.0.0 dev wlo1 proto dhcp src 192.168.0.1 metric 600 +10.20.30.0/24 dev lxdbr0 proto kernel scope link src 10.20.30.1 +10.192.168.0/24 dev mpqemubr0 proto kernel scop link src 10.192.168.1 linkdown +10.255.19.0/24 dev mpbr0 proto kernel scope link src 10.255.19.1 +172.172.0.0/16 dev docker0 proto kernel scope link src 172.172.0.1 linkdown +192.168.0.0/24 dev wlo1 proto kernel scope link src 192.168.0.1 metric 600 +192.168.123.0/24 dev virbr0 proto kernel scope link src 192.168.123.1 linkdown +)")); + + EXPECT_TRUE(MP_PLATFORM.subnet_used_locally(testSubnet)); +} + +TEST_F(PlatformLinux, subnetUsedLocallyDetectsConflicting) +{ + const mp::Subnet testSubnet{"10.20.30.0/24"}; + + auto [mock_utils, guard] = mpt::MockUtils::inject(); + + EXPECT_CALL(*mock_utils, run_cmd_for_output(QString("ip"), _, _)).WillOnce(Return(R"( +default via 192.168.0.0 dev wlo1 proto dhcp src 192.168.0.1 metric 600 +10.20.30.0/24 dev lxdbr0 proto kernel scope link src 10.20.30.1 +10.192.168.0/24 dev mpqemubr0 proto kernel scop link src 10.192.168.1 linkdown +10.255.19.0/24 dev mpbr0 proto kernel scope link src 10.255.19.1 +172.172.0.0/16 dev docker0 proto kernel scope link src 172.172.0.1 linkdown +192.168.0.0/24 dev wlo1 proto kernel scope link src 192.168.0.1 metric 600 +192.168.123.0/24 dev virbr0 proto kernel scope link src 192.168.123.1 linkdown +)")); + + EXPECT_TRUE(MP_PLATFORM.subnet_used_locally(testSubnet)); +} + +TEST_F(PlatformLinux, virtualSwitchSubnetWorks) +{ + auto [mock_utils, guard] = mpt::MockUtils::inject(); + + EXPECT_CALL(*mock_utils, run_cmd_for_output(QString("ip"), _, _)).WillOnce(Return(R"( +default via 192.168.0.0 dev wlo1 proto dhcp src 192.168.0.1 metric 600 +10.20.30.0/24 dev lxdbr0 proto kernel scope link src 10.20.30.1 +10.192.168.0/24 dev mpqemubr0 proto kernel scop link src 10.192.168.1 linkdown +10.255.19.0/24 dev mpbr0 proto kernel scope link src 10.255.19.1 +172.172.0.0/16 dev docker0 proto kernel scope link src 172.172.0.1 linkdown +192.168.0.0/24 dev wlo1 proto kernel scope link src 192.168.0.1 metric 600 +192.168.123.0/24 dev virbr0 proto kernel scope link src 192.168.123.1 linkdown +)")); + + EXPECT_EQ(MP_PLATFORM.virtual_switch_subnet("mpbr0"), mp::Subnet{"10.255.19.0/24"}); +} + +TEST_F(PlatformLinux, virtualSwitchSubnetReturnsNulloptOnMissing) +{ + auto [mock_utils, guard] = mpt::MockUtils::inject(); + + EXPECT_CALL(*mock_utils, run_cmd_for_output(QString("ip"), _, _)).WillOnce(Return(R"( +default via 192.168.0.0 dev wlo1 proto dhcp src 192.168.0.1 metric 600 +10.20.30.0/24 dev lxdbr0 proto kernel scope link src 10.20.30.1 +192.168.123.0/24 dev virbr0 proto kernel scope link src 192.168.123.1 linkdown +)")); + + EXPECT_FALSE(MP_PLATFORM.virtual_switch_subnet("bridgethatdoesnotexist0").has_value()); +} + +TEST_F(PlatformLinux, virtualSwitchSubnetHandlesBadSubnet) +{ + auto [mock_utils, guard] = mpt::MockUtils::inject(); + + EXPECT_CALL(*mock_utils, run_cmd_for_output(QString("ip"), _, _)).WillOnce(Return(R"( +default via 192.168.0.0 dev wlo1 proto dhcp src 192.168.0.1 metric 600 +10.20.30.256/99 dev badbr0 proto kernel scope link src 10.20.30.1 +192.168.123.0/24 dev virbr0 proto kernel scope link src 192.168.123.1 linkdown +)")); + + EXPECT_FALSE(MP_PLATFORM.virtual_switch_subnet("badbr0").has_value()); +} + +TEST_F(PlatformLinux, virtualSwitchSubnetHandlesBadInput) +{ + auto [mock_utils, guard] = mpt::MockUtils::inject(); + + EXPECT_CALL(*mock_utils, run_cmd_for_output(QString("ip"), _, _)).WillOnce(Return(R"( +lorem ipsum dolor sit amet adipscing consectur ignis terra aer aqua perditio ordo +1.2.3.4.5.6.7.8.9.0, metallum machina lux motus. aka. lantern. +public static void main(string[] args); com.something.subdomain.java.library.subfolder.again.again.class +badbr0, mpqemubr0, virbr0, lxdbr0, badbr1, wlo0 +)")); + EXPECT_FALSE(MP_PLATFORM.virtual_switch_subnet("badbr0").has_value()); +} } // namespace diff --git a/tests/unix/test_platform_unix.cpp b/tests/unix/test_platform_unix.cpp index 20da6707e8..c16a2ad5a3 100644 --- a/tests/unix/test_platform_unix.cpp +++ b/tests/unix/test_platform_unix.cpp @@ -338,6 +338,7 @@ TEST_F(TestPlatformUnix, quitWatchdogSignalsItselfAsynchronously) TEST_F(TestPlatformUnix, canReachGatewayRunsPingWithIP) { + // Linux and MacOS both use ping but with different flags const mp::IPAddress testIP{"192.168.0.1"}; const auto testIPstr = testIP.as_string(); @@ -348,60 +349,3 @@ TEST_F(TestPlatformUnix, canReachGatewayRunsPingWithIP) EXPECT_TRUE(MP_PLATFORM.can_reach_gateway(testIP)); EXPECT_FALSE(MP_PLATFORM.can_reach_gateway(testIP)); } - -TEST_F(TestPlatformUnix, subnetUsedLocallyDetectsUnused) -{ - const mp::Subnet testSubnet{"192.168.1.0/24"}; - - auto [mock_utils, guard] = mpt::MockUtils::inject(); - - EXPECT_CALL(*mock_utils, run_cmd_for_output(QString("ip"), _, _)).WillOnce(Return(R"( -default via 192.168.0.0 dev wlo1 proto dhcp src 192.168.0.1 metric 600 -10.20.30.0/24 dev lxdbr0 proto kernel scope link src 10.20.30.1 -10.192.168.0/24 dev mpqemubr0 proto kernel scop link src 10.192.168.1 linkdown -10.255.19.0/24 dev mpbr0 proto kernel scope link src 10.255.19.1 -172.172.0.0/16 dev docker0 proto kernel scope link src 172.172.0.1 linkdown -192.168.0.0/24 dev wlo1 proto kernel scope link src 192.168.0.1 metric 600 -192.168.123.0/24 dev virbr0 proto kernel scope link src 192.168.123.1 linkdown -)")); - - EXPECT_FALSE(MP_PLATFORM.subnet_used_locally(testSubnet)); -} - -TEST_F(TestPlatformUnix, subnetUsedLocallyDetectsOverlapping) -{ - const mp::Subnet testSubnet{"172.172.1.0/24"}; - - auto [mock_utils, guard] = mpt::MockUtils::inject(); - - EXPECT_CALL(*mock_utils, run_cmd_for_output(QString("ip"), _, _)).WillOnce(Return(R"( -default via 192.168.0.0 dev wlo1 proto dhcp src 192.168.0.1 metric 600 -10.20.30.0/24 dev lxdbr0 proto kernel scope link src 10.20.30.1 -10.192.168.0/24 dev mpqemubr0 proto kernel scop link src 10.192.168.1 linkdown -10.255.19.0/24 dev mpbr0 proto kernel scope link src 10.255.19.1 -172.172.0.0/16 dev docker0 proto kernel scope link src 172.172.0.1 linkdown -192.168.0.0/24 dev wlo1 proto kernel scope link src 192.168.0.1 metric 600 -192.168.123.0/24 dev virbr0 proto kernel scope link src 192.168.123.1 linkdown -)")); - - EXPECT_TRUE(MP_PLATFORM.subnet_used_locally(testSubnet)); -} - -TEST_F(TestPlatformUnix, subnetUsedLocallyDetectsConflicting) -{ - const mp::Subnet testSubnet{"10.20.30.0/24"}; - - auto [mock_utils, guard] = mpt::MockUtils::inject(); - - EXPECT_CALL(*mock_utils, run_cmd_for_output(QString("ip"), _, _)).WillOnce(Return(R"( -default via 192.168.0.0 dev wlo1 proto dhcp src 192.168.0.1 metric 600 -10.20.30.0/24 dev lxdbr0 proto kernel scope link src 10.20.30.1 -10.192.168.0/24 dev mpqemubr0 proto kernel scop link src 10.192.168.1 linkdown -10.255.19.0/24 dev mpbr0 proto kernel scope link src 10.255.19.1 -172.172.0.0/16 dev docker0 proto kernel scope link src 172.172.0.1 linkdown -192.168.0.0/24 dev wlo1 proto kernel scope link src 192.168.0.1 metric 600 -192.168.123.0/24 dev virbr0 proto kernel scope link src 192.168.123.1 linkdown -)")); - - EXPECT_TRUE(MP_PLATFORM.subnet_used_locally(testSubnet)); -} From 32d8ddf935842588bda17cf3d9f9c94f4a7c2232 Mon Sep 17 00:00:00 2001 From: trevor-shoe Date: Mon, 29 Sep 2025 06:58:15 -0400 Subject: [PATCH 11/28] [az] Use Subnet for AZs Signed-off-by: trevor-shoe --- include/multipass/availability_zone.h | 3 ++- include/multipass/base_availability_zone.h | 4 ++-- src/platform/backends/shared/base_availability_zone.cpp | 9 ++++----- src/platform/platform_linux.cpp | 2 +- tests/mock_availability_zone.h | 2 +- tests/stub_availability_zone.h | 4 ++-- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/include/multipass/availability_zone.h b/include/multipass/availability_zone.h index bf146a1256..4b6b325174 100644 --- a/include/multipass/availability_zone.h +++ b/include/multipass/availability_zone.h @@ -19,6 +19,7 @@ #define MULTIPASS_AVAILABILITY_ZONE_H #include "disabled_copy_move.h" +#include "subnet.h" #include "virtual_machine.h" #include @@ -34,7 +35,7 @@ class AvailabilityZone : private DisabledCopyMove virtual ~AvailabilityZone() = default; [[nodiscard]] virtual const std::string& get_name() const = 0; - [[nodiscard]] virtual const std::string& get_subnet() const = 0; + [[nodiscard]] virtual const Subnet& get_subnet() const = 0; [[nodiscard]] virtual bool is_available() const = 0; virtual void set_available(bool new_available) = 0; virtual void add_vm(VirtualMachine& vm) = 0; diff --git a/include/multipass/base_availability_zone.h b/include/multipass/base_availability_zone.h index 16d4a851c5..5d11dad378 100644 --- a/include/multipass/base_availability_zone.h +++ b/include/multipass/base_availability_zone.h @@ -33,7 +33,7 @@ class BaseAvailabilityZone : public AvailabilityZone BaseAvailabilityZone(const std::string& name, const std::filesystem::path& az_directory); const std::string& get_name() const override; - const std::string& get_subnet() const override; + const Subnet& get_subnet() const override; bool is_available() const override; void set_available(bool new_available) override; void add_vm(VirtualMachine& vm) override; @@ -48,7 +48,7 @@ class BaseAvailabilityZone : public AvailabilityZone { const std::string name{}; const std::filesystem::path file_path{}; - const std::string subnet{}; + const Subnet subnet; bool available{}; std::vector> vms{}; // we don't have designated initializers, so mutex remains last so it doesn't need to be diff --git a/src/platform/backends/shared/base_availability_zone.cpp b/src/platform/backends/shared/base_availability_zone.cpp index 4c983af15c..e7f91e713a 100644 --- a/src/platform/backends/shared/base_availability_zone.cpp +++ b/src/platform/backends/shared/base_availability_zone.cpp @@ -43,7 +43,7 @@ catch (const std::ios_base::failure& e) return QJsonObject{}; } -[[nodiscard]] std::string deserialize_subnet(const QJsonObject& json, +[[nodiscard]] multipass::Subnet deserialize_subnet(const QJsonObject& json, const multipass::fs::path& file_path, const std::string& name) { @@ -51,8 +51,7 @@ catch (const std::ios_base::failure& e) return json_subnet; mpl::debug(name, "subnet missing from AZ file '{}', using default", file_path); - // TODO GET ACTUAL SUBNET - return {}; + return MP_SUBNET_UTILS.generate_random_subnet(); }; [[nodiscard]] bool deserialize_available(const QJsonObject& json, @@ -95,7 +94,7 @@ const std::string& BaseAvailabilityZone::get_name() const return m.name; } -const std::string& BaseAvailabilityZone::get_subnet() const +const Subnet& BaseAvailabilityZone::get_subnet() const { return m.subnet; } @@ -145,7 +144,7 @@ void BaseAvailabilityZone::serialize() const const std::unique_lock lock{m.mutex}; const QJsonObject json{ - {subnet_key, QString::fromStdString(m.subnet)}, + {subnet_key, QString::fromStdString(m.subnet.as_string())}, {available_key, m.available}, }; diff --git a/src/platform/platform_linux.cpp b/src/platform/platform_linux.cpp index a661d83f08..6b63d349b6 100644 --- a/src/platform/platform_linux.cpp +++ b/src/platform/platform_linux.cpp @@ -361,7 +361,7 @@ bool mp::platform::Platform::can_reach_gateway(mp::IPAddress ip) const } // validation of the ip and cidr happen later, otherwise this regex would be massive. -static const QRegularExpression subnet_regex(R"(((?:[0-9][0-9]?[0-9]?\.){3}[0-9][0-9]?[0-9]?\/[0-9][0-9]?))"); +const QRegularExpression subnet_regex(R"(((?:[0-9][0-9]?[0-9]?\.){3}[0-9][0-9]?[0-9]?\/[0-9][0-9]?))"); bool mp::platform::Platform::subnet_used_locally(mp::Subnet subnet) const { diff --git a/tests/mock_availability_zone.h b/tests/mock_availability_zone.h index 0ef74d7517..647b0df3ab 100644 --- a/tests/mock_availability_zone.h +++ b/tests/mock_availability_zone.h @@ -31,7 +31,7 @@ namespace mp = multipass; struct MockAvailabilityZone : public mp::AvailabilityZone { MOCK_METHOD(const std::string&, get_name, (), (const, override)); - MOCK_METHOD(const std::string&, get_subnet, (), (const, override)); + MOCK_METHOD(const Subnet&, get_subnet, (), (const, override)); MOCK_METHOD(bool, is_available, (), (const, override)); MOCK_METHOD(void, set_available, (bool), (override)); MOCK_METHOD(void, add_vm, (mp::VirtualMachine&), (override)); diff --git a/tests/stub_availability_zone.h b/tests/stub_availability_zone.h index d779d1c8f3..b03cecdc0f 100644 --- a/tests/stub_availability_zone.h +++ b/tests/stub_availability_zone.h @@ -36,9 +36,9 @@ struct StubAvailabilityZone final : public AvailabilityZone return name; } - const std::string& get_subnet() const override + const Subnet& get_subnet() const override { - static std::string subnet{"192.168.123"}; + static Subnet subnet{"192.168.123.0/24"}; return subnet; } From 3d7d3d2b9cae38894046fb0fcd499b98ffb2d2b1 Mon Sep 17 00:00:00 2001 From: trevor-shoe Date: Mon, 29 Sep 2025 10:43:25 -0400 Subject: [PATCH 12/28] [az] Replace remaining string subnets Signed-off-by: trevor-shoe --- include/multipass/availability_zone_manager.h | 4 +- include/multipass/subnet.h | 1 - src/network/subnet.cpp | 24 ------- .../qemu/linux/qemu_platform_detail.h | 18 ++--- .../qemu/linux/qemu_platform_detail_linux.cpp | 44 +++++++------ src/platform/backends/qemu/qemu_platform.h | 3 +- .../qemu/qemu_virtual_machine_factory.cpp | 2 +- tests/mock_subnet_utils.h | 1 - .../qemu/linux/test_qemu_platform_detail.cpp | 51 +++++++++----- tests/qemu/mock_qemu_platform.h | 2 +- tests/qemu/test_qemu_backend.cpp | 66 +++++++++---------- tests/test_subnet.cpp | 48 -------------- 12 files changed, 108 insertions(+), 156 deletions(-) diff --git a/include/multipass/availability_zone_manager.h b/include/multipass/availability_zone_manager.h index 25c951855d..c58cf9f578 100644 --- a/include/multipass/availability_zone_manager.h +++ b/include/multipass/availability_zone_manager.h @@ -33,10 +33,12 @@ class AvailabilityZoneManager : private DisabledCopyMove using UPtr = std::unique_ptr; using ShPtr = std::shared_ptr; + using Zones = std::vector>; + virtual ~AvailabilityZoneManager() = default; virtual AvailabilityZone& get_zone(const std::string& name) = 0; - virtual std::vector> get_zones() = 0; + virtual Zones get_zones() = 0; // this returns a computed zone name, using an algorithm e.g. round-robin // not to be confused with [get_default_zone_name] virtual std::string get_automatic_zone_name() = 0; diff --git a/include/multipass/subnet.h b/include/multipass/subnet.h index 822c72b7a1..7a5e0751b9 100644 --- a/include/multipass/subnet.h +++ b/include/multipass/subnet.h @@ -60,7 +60,6 @@ struct SubnetUtils : Singleton [[nodiscard]] virtual Subnet generate_random_subnet(uint8_t cidr = 24, Subnet range = Subnet{"10.0.0.0/8"}) const; - [[nodiscard]] virtual Subnet get_subnet(const Path& network_dir, const QString& bridge_name) const; }; } // namespace multipass diff --git a/src/network/subnet.cpp b/src/network/subnet.cpp index 8231cf8bac..f2c69ca760 100644 --- a/src/network/subnet.cpp +++ b/src/network/subnet.cpp @@ -212,27 +212,3 @@ mp::Subnet mp::SubnetUtils::generate_random_subnet(uint8_t cidr, Subnet range) c throw std::runtime_error("Could not determine a subnet for networking."); } - -mp::Subnet mp::SubnetUtils::get_subnet(const mp::Path& network_dir, const QString& bridge_name) const -{ - if (auto subnet = MP_PLATFORM.virtual_switch_subnet(bridge_name)) - return *subnet; - - QFile subnet_file{network_dir + "/multipass_subnet_" + bridge_name}; - MP_FILEOPS.open(subnet_file, QIODevice::ReadWrite | QIODevice::Text); - if (MP_FILEOPS.size(subnet_file) > 0) - { - const auto content = MP_FILEOPS.read_all(subnet_file).trimmed().toStdString(); - if (content.find('/') != std::string::npos) - { - return Subnet{content}; - } - // assume CIDR of 24 is missing (for backwards compatability) - return Subnet{IPAddress{content}, 24}; - } - - auto new_subnet = MP_SUBNET_UTILS.generate_random_subnet(); - const auto subnet_str = new_subnet.as_string(); - MP_FILEOPS.write(subnet_file, subnet_str.data(), subnet_str.size()); - return new_subnet; -} diff --git a/src/platform/backends/qemu/linux/qemu_platform_detail.h b/src/platform/backends/qemu/linux/qemu_platform_detail.h index 905378b75f..33cbf785b0 100644 --- a/src/platform/backends/qemu/linux/qemu_platform_detail.h +++ b/src/platform/backends/qemu/linux/qemu_platform_detail.h @@ -22,10 +22,10 @@ #include +#include #include #include -#include namespace multipass { @@ -33,7 +33,7 @@ namespace multipass class QemuPlatformDetail : public QemuPlatform { public: - explicit QemuPlatformDetail(const Path& data_dir); + explicit QemuPlatformDetail(const Path& data_dir, const AvailabilityZoneManager::Zones& zones); virtual ~QemuPlatformDetail(); std::optional get_ip_for(const std::string& hw_addr) override; @@ -48,23 +48,23 @@ class QemuPlatformDetail : public QemuPlatform private: // explicitly naming DisabledCopyMove since the private one derived from QemuPlatform takes // precedence in lookup - struct BridgeSubnet : private multipass::DisabledCopyMove + struct Bridge : private multipass::DisabledCopyMove { const QString bridge_name; const Subnet subnet; FirewallConfig::UPtr firewall_config; - BridgeSubnet(const Path& network_dir, const std::string& name); - ~BridgeSubnet(); + Bridge(const Subnet& subnet, const std::string& name); + ~Bridge(); }; - using BridgeSubnets = std::unordered_map; + using Bridges = std::unordered_map; - [[nodiscard]] static BridgeSubnets get_subnets(const Path& network_dir); + [[nodiscard]] static Bridges get_bridges(const AvailabilityZoneManager::Zones& zones); - [[nodiscard]] static BridgeSubnetList get_subnets_list(const BridgeSubnets&); + [[nodiscard]] static BridgeSubnetList get_bridge_list(const Bridges&); const Path network_dir; - const BridgeSubnets subnets; + const Bridges bridges; DNSMasqServer::UPtr dnsmasq_server; std::unordered_map> name_to_net_device_map; diff --git a/src/platform/backends/qemu/linux/qemu_platform_detail_linux.cpp b/src/platform/backends/qemu/linux/qemu_platform_detail_linux.cpp index e28f35add6..4889ebe199 100644 --- a/src/platform/backends/qemu/linux/qemu_platform_detail_linux.cpp +++ b/src/platform/backends/qemu/linux/qemu_platform_detail_linux.cpp @@ -115,36 +115,38 @@ void delete_virtual_switch(const QString& bridge_name) } } // namespace -mp::QemuPlatformDetail::BridgeSubnet::BridgeSubnet(const Path& network_dir, const std::string& name) +mp::QemuPlatformDetail::Bridge::Bridge(const Subnet& subnet, const std::string& name) : bridge_name{multipass_bridge_name.arg(name.c_str())}, - subnet{MP_SUBNET_UTILS.get_subnet(network_dir, bridge_name)}, + subnet{subnet}, firewall_config{MP_FIREWALL_CONFIG_FACTORY.make_firewall_config(bridge_name, subnet)} { create_virtual_switch(subnet, bridge_name); } -mp::QemuPlatformDetail::BridgeSubnet::~BridgeSubnet() +mp::QemuPlatformDetail::Bridge::~Bridge() { delete_virtual_switch(bridge_name); } -[[nodiscard]] mp::QemuPlatformDetail::BridgeSubnets mp::QemuPlatformDetail::get_subnets( - const Path& network_dir) +[[nodiscard]] mp::QemuPlatformDetail::Bridges mp::QemuPlatformDetail::get_bridges( + const AvailabilityZoneManager::Zones& zones) { - BridgeSubnets subnets{}; - subnets.reserve(default_zone_names.size()); + Bridges bridges{}; + bridges.reserve(zones.size()); - for (const auto& zone : default_zone_names) + for (const auto& zone_ref : zones) { - subnets.emplace(std::piecewise_construct, - std::forward_as_tuple(zone), - std::forward_as_tuple(network_dir, zone)); + const auto& zone = zone_ref.get(); + const auto& name = zone.get_name(); + bridges.emplace(std::piecewise_construct, + std::forward_as_tuple(name), + std::forward_as_tuple(zone.get_subnet(), name)); } - return subnets; + return bridges; } -[[nodiscard]] mp::BridgeSubnetList mp::QemuPlatformDetail::get_subnets_list(const BridgeSubnets& subnets) +[[nodiscard]] mp::BridgeSubnetList mp::QemuPlatformDetail::get_bridge_list(const Bridges& subnets) { BridgeSubnetList out{}; out.reserve(subnets.size()); @@ -157,10 +159,10 @@ mp::QemuPlatformDetail::BridgeSubnet::~BridgeSubnet() return out; } -mp::QemuPlatformDetail::QemuPlatformDetail(const mp::Path& data_dir) +mp::QemuPlatformDetail::QemuPlatformDetail(const mp::Path& data_dir, const AvailabilityZoneManager::Zones& zones) : network_dir{MP_UTILS.make_dir(QDir(data_dir), "network")}, - subnets{get_subnets(network_dir)}, - dnsmasq_server{init_nat_network(network_dir, get_subnets_list(subnets))} + bridges{get_bridges(zones)}, + dnsmasq_server{init_nat_network(network_dir, get_bridge_list(bridges))} { } @@ -197,9 +199,9 @@ void mp::QemuPlatformDetail::platform_health_check() MP_BACKEND.check_if_kvm_is_in_use(); dnsmasq_server->check_dnsmasq_running(); - for (const auto& [_, subnet] : subnets) + for (const auto& [_, bridge] : bridges) { - subnet.firewall_config->verify_firewall_rules(); + bridge.firewall_config->verify_firewall_rules(); } } @@ -207,7 +209,7 @@ QStringList mp::QemuPlatformDetail::vm_platform_args(const VirtualMachineDescrip { // Configure and generate the args for the default network interface auto tap_device_name = generate_tap_device_name(vm_desc.vm_name); - const QString& bridge_name = subnets.at(vm_desc.zone).bridge_name; + const QString& bridge_name = bridges.at(vm_desc.zone).bridge_name; create_tap_device(tap_device_name, bridge_name); name_to_net_device_map.emplace( @@ -258,9 +260,9 @@ QStringList mp::QemuPlatformDetail::vm_platform_args(const VirtualMachineDescrip return opts; } -mp::QemuPlatform::UPtr mp::QemuPlatformFactory::make_qemu_platform(const Path& data_dir) const +mp::QemuPlatform::UPtr mp::QemuPlatformFactory::make_qemu_platform(const Path& data_dir, const mp::AvailabilityZoneManager::Zones& zones) const { - return std::make_unique(data_dir); + return std::make_unique(data_dir, zones); } bool mp::QemuPlatformDetail::is_network_supported(const std::string& network_type) const diff --git a/src/platform/backends/qemu/qemu_platform.h b/src/platform/backends/qemu/qemu_platform.h index 3a747cb298..16270946a3 100644 --- a/src/platform/backends/qemu/qemu_platform.h +++ b/src/platform/backends/qemu/qemu_platform.h @@ -17,6 +17,7 @@ #pragma once +#include #include #include #include @@ -70,6 +71,6 @@ class QemuPlatformFactory : public Singleton QemuPlatformFactory(const Singleton::PrivatePass& pass) noexcept : Singleton::Singleton{pass} {}; - virtual QemuPlatform::UPtr make_qemu_platform(const Path& data_dir) const; + virtual QemuPlatform::UPtr make_qemu_platform(const Path& data_dir, const AvailabilityZoneManager::Zones& zones) const; }; } // namespace multipass diff --git a/src/platform/backends/qemu/qemu_virtual_machine_factory.cpp b/src/platform/backends/qemu/qemu_virtual_machine_factory.cpp index b67a063e65..eda0e59291 100644 --- a/src/platform/backends/qemu/qemu_virtual_machine_factory.cpp +++ b/src/platform/backends/qemu/qemu_virtual_machine_factory.cpp @@ -38,7 +38,7 @@ constexpr auto category = "qemu factory"; mp::QemuVirtualMachineFactory::QemuVirtualMachineFactory(const mp::Path& data_dir, AvailabilityZoneManager& az_manager) - : QemuVirtualMachineFactory{MP_QEMU_PLATFORM_FACTORY.make_qemu_platform(data_dir), + : QemuVirtualMachineFactory{MP_QEMU_PLATFORM_FACTORY.make_qemu_platform(data_dir, az_manager.get_zones()), data_dir, az_manager} { diff --git a/tests/mock_subnet_utils.h b/tests/mock_subnet_utils.h index 50c24b0247..ce1905d949 100644 --- a/tests/mock_subnet_utils.h +++ b/tests/mock_subnet_utils.h @@ -28,7 +28,6 @@ struct MockSubnetUtils : public SubnetUtils using SubnetUtils::SubnetUtils; MOCK_METHOD(Subnet, generate_random_subnet, (uint8_t cidr, Subnet range), (const, override)); - MOCK_METHOD(Subnet, get_subnet, (const Path& network_dir, const QString& bridge_name), (const, override)); MP_MOCK_SINGLETON_BOILERPLATE(MockSubnetUtils, SubnetUtils); }; diff --git a/tests/qemu/linux/test_qemu_platform_detail.cpp b/tests/qemu/linux/test_qemu_platform_detail.cpp index 8081f44a40..fc3c620dcf 100644 --- a/tests/qemu/linux/test_qemu_platform_detail.cpp +++ b/tests/qemu/linux/test_qemu_platform_detail.cpp @@ -19,6 +19,7 @@ #include "mock_firewall_config.h" #include "tests/common.h" +#include "tests/mock_availability_zone.h" #include "tests/mock_backend_utils.h" #include "tests/mock_file_ops.h" #include "tests/mock_logger.h" @@ -51,9 +52,6 @@ struct QemuPlatformDetail : public Test for (const auto& vswitch : switches) { - EXPECT_CALL(*mock_subnet_utils, get_subnet(_, vswitch.bridge_name)) - .WillOnce([subnet = vswitch.subnet](auto...) { return subnet; }); - EXPECT_CALL(*mock_firewall_config_factory, make_firewall_config(vswitch.bridge_name, vswitch.subnet)) .WillOnce( @@ -69,6 +67,18 @@ struct QemuPlatformDetail : public Test .WillOnce(Return(true)); } + static std::string zone1_name = "zone1"; + EXPECT_CALL(mock_zone1, get_name).WillRepeatedly(ReturnRef(zone1_name)); + EXPECT_CALL(mock_zone1, get_subnet).WillRepeatedly(ReturnRef(zone1_subnet)); + + static std::string zone2_name = "zone2"; + EXPECT_CALL(mock_zone2, get_name).WillRepeatedly(ReturnRef(zone2_name)); + EXPECT_CALL(mock_zone2, get_subnet).WillRepeatedly(ReturnRef(zone2_subnet)); + + static std::string zone3_name = "zone3"; + EXPECT_CALL(mock_zone3, get_name).WillRepeatedly(ReturnRef(zone3_name)); + EXPECT_CALL(mock_zone3, get_subnet).WillRepeatedly(ReturnRef(zone3_subnet)); + EXPECT_CALL(*mock_dnsmasq_server_factory, make_dnsmasq_server(_, _)) .WillOnce([this](auto...) { return std::move(mock_dnsmasq_server); }); @@ -101,10 +111,21 @@ struct QemuPlatformDetail : public Test { } }; + + static inline const mp::Subnet zone1_subnet{"192.168.64.0/24"}; + static inline const mp::Subnet zone2_subnet{"192.168.96.0/24"}; + static inline const mp::Subnet zone3_subnet{"192.168.128.0/24"}; const std::vector switches{ - {"mpqemubrzone1", "52:54:00:6f:29:7e", mp::Subnet{"192.168.64.0/24"}, "foo"}, - {"mpqemubrzone2", "52:54:00:6f:29:7f", mp::Subnet{"192.168.96.0/24"}, "bar"}, - {"mpqemubrzone3", "52:54:00:6f:29:80", mp::Subnet{"192.168.128.0/24"}, "baz"}}; + {"mpqemubrzone1", "52:54:00:6f:29:7e", zone1_subnet, "foo"}, + {"mpqemubrzone2", "52:54:00:6f:29:7f", zone2_subnet, "bar"}, + {"mpqemubrzone3", "52:54:00:6f:29:80", zone3_subnet, "baz"}}; + + mpt::MockAvailabilityZone mock_zone1; + mpt::MockAvailabilityZone mock_zone2; + mpt::MockAvailabilityZone mock_zone3; + const multipass::AvailabilityZoneManager::Zones mock_zones{ + mock_zone1, mock_zone2, mock_zone3 + }; mpt::TempDir data_dir; @@ -174,7 +195,7 @@ TEST_F(QemuPlatformDetail, ctorSetsUpExpectedVirtualSwitches) .WillOnce(Return(true)); } - mp::QemuPlatformDetail qemu_platform_detail{data_dir.path()}; + mp::QemuPlatformDetail qemu_platform_detail{data_dir.path(), mock_zones}; } TEST_F(QemuPlatformDetail, getIpForReturnsExpectedInfo) @@ -186,7 +207,7 @@ TEST_F(QemuPlatformDetail, getIpForReturnsExpectedInfo) .WillOnce([ip = ip_address](auto...) { return ip; }); } - mp::QemuPlatformDetail qemu_platform_detail{data_dir.path()}; + mp::QemuPlatformDetail qemu_platform_detail{data_dir.path(), mock_zones}; for (const auto& vswitch : switches) { @@ -225,7 +246,7 @@ TEST_F(QemuPlatformDetail, platformArgsGenerateNetResourcesRemovesWorksAsExpecte return false; }); - mp::QemuPlatformDetail qemu_platform_detail{data_dir.path()}; + mp::QemuPlatformDetail qemu_platform_detail{data_dir.path(), mock_zones}; const auto platform_args = qemu_platform_detail.vm_platform_args(vm_desc); @@ -290,7 +311,7 @@ TEST_F(QemuPlatformDetail, tapDevicesAreRemovedOnDestruction) return false; }); - mp::QemuPlatformDetail qemu_platform_detail{data_dir.path()}; + mp::QemuPlatformDetail qemu_platform_detail{data_dir.path(), mock_zones}; const auto platform_args = qemu_platform_detail.vm_platform_args(vm_desc); @@ -317,7 +338,7 @@ TEST_F(QemuPlatformDetail, platformHealthCheckCallsExpectedMethods) EXPECT_CALL(*vswitch.mock_firewall_config, verify_firewall_rules()).WillOnce(Return()); } - mp::QemuPlatformDetail qemu_platform_detail{data_dir.path()}; + mp::QemuPlatformDetail qemu_platform_detail{data_dir.path(), mock_zones}; qemu_platform_detail.platform_health_check(); } @@ -331,7 +352,7 @@ TEST_F(QemuPlatformDetail, openingIpforwardFileFailureLogsExpectedMessage) EXPECT_CALL(*mock_file_ops, open(_, _)).WillOnce(Return(false)); - mp::QemuPlatformDetail qemu_platform_detail{data_dir.path()}; + mp::QemuPlatformDetail qemu_platform_detail{data_dir.path(), mock_zones}; } TEST_F(QemuPlatformDetail, writingIpforwardFileFailureLogsExpectedMessage) @@ -343,12 +364,12 @@ TEST_F(QemuPlatformDetail, writingIpforwardFileFailureLogsExpectedMessage) EXPECT_CALL(*mock_file_ops, write(_, QByteArray("1"))).WillOnce(Return(-1)); - mp::QemuPlatformDetail qemu_platform_detail{data_dir.path()}; + mp::QemuPlatformDetail qemu_platform_detail{data_dir.path(), mock_zones}; } TEST_F(QemuPlatformDetail, platformCorrectlySetsAuthorization) { - mp::QemuPlatformDetail qemu_platform_detail{data_dir.path()}; + mp::QemuPlatformDetail qemu_platform_detail{data_dir.path(), mock_zones}; std::vector networks{ mp::NetworkInterfaceInfo{"br-en0", "bridge", "", {"en0"}, false}, @@ -368,7 +389,7 @@ TEST_F(QemuPlatformDetail, createBridgeWithCallsExpectedMethods) { EXPECT_CALL(*mock_backend, create_bridge_with("en0")).WillOnce(Return("br-en0")); - mp::QemuPlatformDetail qemu_platform_detail{data_dir.path()}; + mp::QemuPlatformDetail qemu_platform_detail{data_dir.path(), mock_zones}; EXPECT_EQ(qemu_platform_detail.create_bridge_with( mp::NetworkInterfaceInfo{"en0", "ethernet", "", {}, true}), diff --git a/tests/qemu/mock_qemu_platform.h b/tests/qemu/mock_qemu_platform.h index 078bfce9cf..b57fa3e3ff 100644 --- a/tests/qemu/mock_qemu_platform.h +++ b/tests/qemu/mock_qemu_platform.h @@ -51,7 +51,7 @@ struct MockQemuPlatformFactory : public QemuPlatformFactory { using QemuPlatformFactory::QemuPlatformFactory; - MOCK_METHOD(QemuPlatform::UPtr, make_qemu_platform, (const Path&), (const, override)); + MOCK_METHOD(QemuPlatform::UPtr, make_qemu_platform, (const Path&, const AvailabilityZoneManager::Zones&), (const, override)); MP_MOCK_SINGLETON_BOILERPLATE(MockQemuPlatformFactory, QemuPlatformFactory); }; diff --git a/tests/qemu/test_qemu_backend.cpp b/tests/qemu/test_qemu_backend.cpp index c05f44248d..795742e36b 100644 --- a/tests/qemu/test_qemu_backend.cpp +++ b/tests/qemu/test_qemu_backend.cpp @@ -200,7 +200,7 @@ struct QemuBackend : public mpt::TestWithMockedBinPath TEST_F(QemuBackend, createsInOffState) { - EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_, _)).WillOnce([this](auto...) { return std::move(mock_qemu_platform); }); @@ -212,7 +212,7 @@ TEST_F(QemuBackend, createsInOffState) TEST_F(QemuBackend, machineInOffStateHandlesShutdown) { - EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_, _)).WillOnce([this](auto...) { return std::move(mock_qemu_platform); }); @@ -227,7 +227,7 @@ TEST_F(QemuBackend, machineInOffStateHandlesShutdown) TEST_F(QemuBackend, machineStartShutdownSendsMonitoringEvents) { - EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_, _)).WillOnce([this](auto...) { return std::move(mock_qemu_platform); }); @@ -251,7 +251,7 @@ TEST_F(QemuBackend, machineStartShutdownSendsMonitoringEvents) TEST_F(QemuBackend, machineStartSuspendSendsMonitoringEvent) { - EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_, _)).WillOnce([this](auto...) { return std::move(mock_qemu_platform); }); @@ -285,7 +285,7 @@ TEST_F(QemuBackend, throwsWhenShutdownWhileStarting) } }); - EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_, _)).WillOnce([this](auto...) { return std::move(mock_qemu_platform); }); @@ -326,7 +326,7 @@ TEST_F(QemuBackend, throwsOnShutdownTimeout) } }); - EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_, _)).WillOnce([this](auto...) { return std::move(mock_qemu_platform); }); @@ -365,7 +365,7 @@ TEST_F(QemuBackend, includesErrorWhenShutdownWhileStarting) } }); - EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_, _)).WillOnce([this](auto...) { return std::move(mock_qemu_platform); }); @@ -404,7 +404,7 @@ TEST_F(QemuBackend, includesErrorWhenShutdownWhileStarting) TEST_F(QemuBackend, machineUnknownStateProperlyShutsDown) { - EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_, _)).WillOnce([this](auto...) { return std::move(mock_qemu_platform); }); @@ -432,7 +432,7 @@ TEST_F(QemuBackend, suspendedStateNoForceShutdownThrows) { const std::string sub_error_msg{"Cannot shut down suspended instance"}; - EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_, _)).WillOnce([this](auto...) { return std::move(mock_qemu_platform); }); @@ -454,7 +454,7 @@ TEST_F(QemuBackend, suspendingStateNoForceShutdownThrows) const std::string sub_error_msg1{"Cannot shut down instance"}; const std::string sub_error_msg2{"while suspending."}; - EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_, _)).WillOnce([this](auto...) { return std::move(mock_qemu_platform); }); @@ -477,7 +477,7 @@ TEST_F(QemuBackend, startingStateNoForceShutdownThrows) const std::string sub_error_msg1{"Cannot shut down instance"}; const std::string sub_error_msg2{"while starting."}; - EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_, _)).WillOnce([this](auto...) { return std::move(mock_qemu_platform); }); @@ -514,7 +514,7 @@ TEST_F(QemuBackend, forceShutdownKillsProcessAndLogs) } }); - EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_, _)).WillOnce([this](auto...) { return std::move(mock_qemu_platform); }); @@ -544,7 +544,7 @@ TEST_F(QemuBackend, forceShutdownKillsProcessAndLogs) TEST_F(QemuBackend, forceShutdownNoProcessLogs) { - EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_, _)).WillOnce([this](auto...) { return std::move(mock_qemu_platform); }); @@ -565,7 +565,7 @@ TEST_F(QemuBackend, forceShutdownNoProcessLogs) TEST_F(QemuBackend, forceShutdownSuspendDeletesSuspendImageAndOffState) { - EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_, _)).WillOnce([this](auto...) { return std::move(mock_qemu_platform); }); @@ -604,7 +604,7 @@ TEST_F(QemuBackend, forceShutdownSuspendDeletesSuspendImageAndOffState) TEST_F(QemuBackend, forceShutdownSuspendedStateButNoSuspensionSnapshotInImage) { - EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_, _)).WillOnce([this](auto...) { return std::move(mock_qemu_platform); }); @@ -626,7 +626,7 @@ TEST_F(QemuBackend, forceShutdownSuspendedStateButNoSuspensionSnapshotInImage) TEST_F(QemuBackend, forceShutdownRunningStateButWithSuspensionSnapshotInImage) { - EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_, _)).WillOnce([this](auto...) { return std::move(mock_qemu_platform); }); @@ -667,7 +667,7 @@ TEST_F(QemuBackend, forceShutdownRunningStateButWithSuspensionSnapshotInImage) TEST_F(QemuBackend, verifyDnsmasqQemuimgAndQemuProcessesCreated) { - EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_, _)).WillOnce([this](auto...) { return std::move(mock_qemu_platform); }); @@ -694,7 +694,7 @@ TEST_F(QemuBackend, verifyDnsmasqQemuimgAndQemuProcessesCreated) TEST_F(QemuBackend, verifySomeCommonQemuArguments) { - EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_, _)).WillOnce([this](auto...) { return std::move(mock_qemu_platform); }); @@ -727,7 +727,7 @@ TEST_F(QemuBackend, verifySomeCommonQemuArguments) TEST_F(QemuBackend, verifyQemuArgumentsWhenResumingSuspendImage) { - EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_, _)).WillOnce([this](auto...) { return std::move(mock_qemu_platform); }); @@ -755,7 +755,7 @@ TEST_F(QemuBackend, verifyQemuArgumentsWhenResumingSuspendImageUsesMetadata) { constexpr auto machine_type = "k0mPuT0R"; - EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_, _)).WillOnce([this](auto...) { return std::move(mock_qemu_platform); }); @@ -786,7 +786,7 @@ TEST_F(QemuBackend, verifyQemuArgumentsWhenResumingSuspendImageUsesMetadata) TEST_F(QemuBackend, verifyQemuArgumentsFromMetadataAreUsed) { - EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_, _)).WillOnce([this](auto...) { return std::move(mock_qemu_platform); }); @@ -828,7 +828,7 @@ TEST_F(QemuBackend, verifyQemuArgumentsFromMetadataAreUsed) TEST_F(QemuBackend, returnsVersionString) { - EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_, _)).WillOnce([this](auto...) { return std::move(mock_qemu_platform); }); @@ -856,7 +856,7 @@ TEST_F(QemuBackend, returnsVersionString) TEST_F(QemuBackend, returnsVersionStringWhenFailedParsing) { - EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_, _)).WillOnce([this](auto...) { return std::move(mock_qemu_platform); }); @@ -883,7 +883,7 @@ TEST_F(QemuBackend, returnsVersionStringWhenFailedParsing) TEST_F(QemuBackend, returnsVersionStringWhenErrored) { - EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_, _)).WillOnce([this](auto...) { return std::move(mock_qemu_platform); }); @@ -908,7 +908,7 @@ TEST_F(QemuBackend, returnsVersionStringWhenErrored) TEST_F(QemuBackend, returnsVersionStringWhenExecFailed) { - EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_, _)).WillOnce([this](auto...) { return std::move(mock_qemu_platform); }); @@ -1088,7 +1088,7 @@ TEST_F(QemuBackend, networksReturnsSupportedNetworks) { ON_CALL(*mock_qemu_platform, is_network_supported(_)).WillByDefault(Return(true)); - EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_, _)).WillOnce([this](auto...) { return std::move(mock_qemu_platform); }); @@ -1119,7 +1119,7 @@ TEST_F(QemuBackend, removeResourcesForCallsQemuPlatform) EXPECT_EQ(name, test_name); }); - EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_, _)).WillOnce([this](auto...) { return std::move(mock_qemu_platform); }); @@ -1138,7 +1138,7 @@ TEST_F(QemuBackend, hypervisorHealthCheckCallsQemuPlatform) health_check_called = true; }); - EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_, _)).WillOnce([this](auto...) { return std::move(mock_qemu_platform); }); @@ -1162,7 +1162,7 @@ TEST_F(QemuBackend, getBackendDirectoryNameCallsQemuPlatform) return backend_dir_name; }); - EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_, _)).WillOnce([this](auto...) { return std::move(mock_qemu_platform); }); @@ -1176,7 +1176,7 @@ TEST_F(QemuBackend, getBackendDirectoryNameCallsQemuPlatform) TEST_F(QemuBackend, addNetworkInterface) { - EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_, _)).WillOnce([this](auto...) { return std::move(mock_qemu_platform); }); @@ -1191,7 +1191,7 @@ TEST_F(QemuBackend, addNetworkInterface) TEST_F(QemuBackend, createBridgeWithChecksWithQemuPlatform) { - EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_, _)).WillOnce([this](auto...) { return std::move(mock_qemu_platform); }); EXPECT_CALL(*mock_qemu_platform, needs_network_prep()).Times(1).WillRepeatedly(Return(true)); @@ -1204,7 +1204,7 @@ TEST_F(QemuBackend, createBridgeWithChecksWithQemuPlatform) TEST_F(QemuBackend, removeAllSnapshotsFromTheImage) { - EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_, _)).WillOnce([this](auto...) { return std::move(mock_qemu_platform); }); @@ -1255,7 +1255,7 @@ TEST_F(QemuBackend, removeAllSnapshotsFromTheImage) TEST_F(QemuBackend, cloneCopiesRelevantFiles) { - EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_, _)).WillOnce([this](auto...) { return std::move(mock_qemu_platform); }); diff --git a/tests/test_subnet.cpp b/tests/test_subnet.cpp index 9d38e278a4..0141956abb 100644 --- a/tests/test_subnet.cpp +++ b/tests/test_subnet.cpp @@ -365,51 +365,3 @@ TEST_F(SubnetUtils, generateRandomSubnetGivesUp) MP_EXPECT_THROW_THAT(std::ignore = MP_SUBNET_UTILS.generate_random_subnet(24, range), std::runtime_error, mpt::match_what(HasSubstr("subnet"))); } - -TEST_F(SubnetUtils, getSubnetBridgeExistsReturnsExpectedData) -{ - const mp::Subnet test_subnet{"10.102.12.0/24"}; - const QString bridge_name{"test-bridge"}; - auto [mock_utils, guard] = mpt::MockUtils::inject(); - - EXPECT_CALL(*mock_platform, virtual_switch_subnet(bridge_name)) - .WillOnce(Return(test_subnet)); - - EXPECT_EQ(MP_SUBNET_UTILS.get_subnet("foo", bridge_name).as_string(), test_subnet.as_string()); -} - -TEST_F(SubnetUtils, getSubnetInFileReturnsExpectedData) -{ - const mp::Subnet test_subnet{"10.102.12.0/24"}; - const QString bridge_name{"test-bridge"}; - auto [mock_utils, utils_guard] = mpt::MockUtils::inject(); - auto [mock_file_ops, file_ops_guard] = mpt::MockFileOps::inject(); - - EXPECT_CALL(*mock_file_ops, open(_, _)).WillOnce(Return(true)); - EXPECT_CALL(*mock_file_ops, size(_)).WillOnce(Return(1)); - EXPECT_CALL(*mock_file_ops, read_all(_)) - .WillOnce(Return(QByteArray::fromStdString(test_subnet.as_string()))); - - EXPECT_EQ(MP_SUBNET_UTILS.get_subnet("foo", bridge_name).as_string(), test_subnet.as_string()); -} - -TEST_F(SubnetUtils, getSubnetNotInFileWritesNewSubnetReturnsExpectedData) -{ - const QString bridge_name{"test-bridge"}; - std::string generated_subnet; - auto [mock_utils, utils_guard] = mpt::MockUtils::inject(); - auto [mock_file_ops, file_ops_guard] = mpt::MockFileOps::inject(); - - EXPECT_CALL(*mock_utils, random_int); - - EXPECT_CALL(*mock_file_ops, open(_, _)).WillOnce(Return(true)); - EXPECT_CALL(*mock_file_ops, size(_)).WillOnce(Return(0)); - EXPECT_CALL(*mock_file_ops, write(A(), _, _)) - .WillOnce([&generated_subnet](auto&, auto data, auto) { - generated_subnet = std::string(data); - - return generated_subnet.length(); - }); - - EXPECT_EQ(MP_SUBNET_UTILS.get_subnet("foo", bridge_name).as_string(), generated_subnet); -} From 7833d0041909da41ddc8986b3c97ca8d74ccfe3b Mon Sep 17 00:00:00 2001 From: trevor-shoe Date: Mon, 29 Sep 2025 11:11:57 -0400 Subject: [PATCH 13/28] [network] Remove virtual_switch_subnet Signed-off-by: trevor-shoe --- include/multipass/platform.h | 1 - src/platform/platform_linux.cpp | 30 ---------------- src/platform/platform_osx.cpp | 6 ---- src/platform/platform_win.cpp | 6 ---- tests/linux/test_platform_linux.cpp | 56 ----------------------------- tests/mock_platform.h | 1 - tests/test_subnet.cpp | 1 - 7 files changed, 101 deletions(-) diff --git a/include/multipass/platform.h b/include/multipass/platform.h index 0b5c3cbce3..4c953b1838 100644 --- a/include/multipass/platform.h +++ b/include/multipass/platform.h @@ -80,7 +80,6 @@ class Platform : public Singleton [[nodiscard]] virtual std::string bridge_nomenclature() const; [[nodiscard]] virtual bool can_reach_gateway(IPAddress ip) const; [[nodiscard]] virtual bool subnet_used_locally(Subnet subnet) const; - [[nodiscard]] virtual std::optional virtual_switch_subnet(const QString& bridge_name) const; virtual int get_cpus() const; virtual long long get_total_ram() const; diff --git a/src/platform/platform_linux.cpp b/src/platform/platform_linux.cpp index 6b63d349b6..e8857417a7 100644 --- a/src/platform/platform_linux.cpp +++ b/src/platform/platform_linux.cpp @@ -395,36 +395,6 @@ bool mp::platform::Platform::subnet_used_locally(mp::Subnet subnet) const return false; } -std::optional mp::platform::Platform::virtual_switch_subnet(const QString& bridge_name) const -{ - const auto outputs = - QString::fromStdString(MP_UTILS.run_cmd_for_output("ip", {"-4", "route", "show"})).split('\n'); - - for (const auto& line : outputs) - { - if (line.contains(bridge_name)) - { - QRegularExpressionMatch match = subnet_regex.match(line); - if (!match.hasMatch()) - continue; - - try - { - return mp::Subnet{match.captured(1).toStdString()}; - } - catch (const std::invalid_argument& e) - { - mpl::log( - mpl::Level::warning, - "network", - fmt::format("invalid subnet from ip command: {}", e.what())); - } - } - } - - return std::nullopt; -} - auto mp::platform::detail::get_network_interfaces_from(const QDir& sys_dir) -> std::map { diff --git a/src/platform/platform_osx.cpp b/src/platform/platform_osx.cpp index a426ece95f..1c332085e9 100644 --- a/src/platform/platform_osx.cpp +++ b/src/platform/platform_osx.cpp @@ -278,12 +278,6 @@ bool mp::platform::Platform::subnet_used_locally(mp::Subnet subnet) const throw mp::NotImplementedOnThisBackendException{"AZs @TODO"}; } - -std::optional mp::platform::Platform::virtual_switch_subnet(const QString& bridge_name) const -{ - throw mp::NotImplementedOnThisBackendException{"AZs @TODO"}; -} - QString mp::platform::Platform::daemon_config_home() const // temporary { auto ret = QStringLiteral("/var/root/Library/Preferences/"); diff --git a/src/platform/platform_win.cpp b/src/platform/platform_win.cpp index ef43a5c40f..9519229efc 100644 --- a/src/platform/platform_win.cpp +++ b/src/platform/platform_win.cpp @@ -641,12 +641,6 @@ bool mp::platform::Platform::subnet_used_locally(mp::Subnet subnet) const throw mp::NotImplementedOnThisBackendException{"AZs @TODO"}; } - -std::optional mp::platform::Platform::virtual_switch_subnet(const QString& bridge_name) const -{ - throw mp::NotImplementedOnThisBackendException{"AZs @TODO"}; -} - QString mp::platform::Platform::daemon_config_home() const // temporary { auto ret = systemprofile_app_data_path(); diff --git a/tests/linux/test_platform_linux.cpp b/tests/linux/test_platform_linux.cpp index 5687a0d846..78902e0ec5 100644 --- a/tests/linux/test_platform_linux.cpp +++ b/tests/linux/test_platform_linux.cpp @@ -741,60 +741,4 @@ default via 192.168.0.0 dev wlo1 proto dhcp src 192.168.0.1 metric 600 EXPECT_TRUE(MP_PLATFORM.subnet_used_locally(testSubnet)); } - -TEST_F(PlatformLinux, virtualSwitchSubnetWorks) -{ - auto [mock_utils, guard] = mpt::MockUtils::inject(); - - EXPECT_CALL(*mock_utils, run_cmd_for_output(QString("ip"), _, _)).WillOnce(Return(R"( -default via 192.168.0.0 dev wlo1 proto dhcp src 192.168.0.1 metric 600 -10.20.30.0/24 dev lxdbr0 proto kernel scope link src 10.20.30.1 -10.192.168.0/24 dev mpqemubr0 proto kernel scop link src 10.192.168.1 linkdown -10.255.19.0/24 dev mpbr0 proto kernel scope link src 10.255.19.1 -172.172.0.0/16 dev docker0 proto kernel scope link src 172.172.0.1 linkdown -192.168.0.0/24 dev wlo1 proto kernel scope link src 192.168.0.1 metric 600 -192.168.123.0/24 dev virbr0 proto kernel scope link src 192.168.123.1 linkdown -)")); - - EXPECT_EQ(MP_PLATFORM.virtual_switch_subnet("mpbr0"), mp::Subnet{"10.255.19.0/24"}); -} - -TEST_F(PlatformLinux, virtualSwitchSubnetReturnsNulloptOnMissing) -{ - auto [mock_utils, guard] = mpt::MockUtils::inject(); - - EXPECT_CALL(*mock_utils, run_cmd_for_output(QString("ip"), _, _)).WillOnce(Return(R"( -default via 192.168.0.0 dev wlo1 proto dhcp src 192.168.0.1 metric 600 -10.20.30.0/24 dev lxdbr0 proto kernel scope link src 10.20.30.1 -192.168.123.0/24 dev virbr0 proto kernel scope link src 192.168.123.1 linkdown -)")); - - EXPECT_FALSE(MP_PLATFORM.virtual_switch_subnet("bridgethatdoesnotexist0").has_value()); -} - -TEST_F(PlatformLinux, virtualSwitchSubnetHandlesBadSubnet) -{ - auto [mock_utils, guard] = mpt::MockUtils::inject(); - - EXPECT_CALL(*mock_utils, run_cmd_for_output(QString("ip"), _, _)).WillOnce(Return(R"( -default via 192.168.0.0 dev wlo1 proto dhcp src 192.168.0.1 metric 600 -10.20.30.256/99 dev badbr0 proto kernel scope link src 10.20.30.1 -192.168.123.0/24 dev virbr0 proto kernel scope link src 192.168.123.1 linkdown -)")); - - EXPECT_FALSE(MP_PLATFORM.virtual_switch_subnet("badbr0").has_value()); -} - -TEST_F(PlatformLinux, virtualSwitchSubnetHandlesBadInput) -{ - auto [mock_utils, guard] = mpt::MockUtils::inject(); - - EXPECT_CALL(*mock_utils, run_cmd_for_output(QString("ip"), _, _)).WillOnce(Return(R"( -lorem ipsum dolor sit amet adipscing consectur ignis terra aer aqua perditio ordo -1.2.3.4.5.6.7.8.9.0, metallum machina lux motus. aka. lantern. -public static void main(string[] args); com.something.subdomain.java.library.subfolder.again.again.class -badbr0, mpqemubr0, virbr0, lxdbr0, badbr1, wlo0 -)")); - EXPECT_FALSE(MP_PLATFORM.virtual_switch_subnet("badbr0").has_value()); -} } // namespace diff --git a/tests/mock_platform.h b/tests/mock_platform.h index 4f2766b347..1e1504c4ee 100644 --- a/tests/mock_platform.h +++ b/tests/mock_platform.h @@ -68,7 +68,6 @@ class MockPlatform : public platform::Platform MOCK_METHOD(std::string, bridge_nomenclature, (), (const, override)); MOCK_METHOD(bool, can_reach_gateway, (IPAddress), (const, override)); MOCK_METHOD(bool, subnet_used_locally, (Subnet), (const, override)); - MOCK_METHOD(std::optional, virtual_switch_subnet, (const QString&), (const, override)); MOCK_METHOD(std::filesystem::path, get_root_cert_dir, (), (const, override)); MP_MOCK_SINGLETON_BOILERPLATE(MockPlatform, Platform); diff --git a/tests/test_subnet.cpp b/tests/test_subnet.cpp index 0141956abb..620b9f79a1 100644 --- a/tests/test_subnet.cpp +++ b/tests/test_subnet.cpp @@ -268,7 +268,6 @@ struct SubnetUtils : public Test { ON_CALL(*mock_platform, subnet_used_locally).WillByDefault(Return(false)); ON_CALL(*mock_platform, can_reach_gateway).WillByDefault(Return(false)); - ON_CALL(*mock_platform, virtual_switch_subnet).WillByDefault(Return(std::nullopt)); } mpt::MockUtils::GuardedMock mock_utils_injection{mpt::MockUtils::inject()}; From 064261c84f5bf91e08da0cd71e2041d810fd8c0a Mon Sep 17 00:00:00 2001 From: trevor-shoe Date: Tue, 30 Sep 2025 05:57:52 -0400 Subject: [PATCH 14/28] [az] Mock subnet generating in tests Signed-off-by: trevor-shoe --- tests/test_base_availability_zone.cpp | 64 +++++++++++-------- tests/test_base_availability_zone_manager.cpp | 14 ++++ 2 files changed, 52 insertions(+), 26 deletions(-) diff --git a/tests/test_base_availability_zone.cpp b/tests/test_base_availability_zone.cpp index cb436858e3..15ebff7f02 100644 --- a/tests/test_base_availability_zone.cpp +++ b/tests/test_base_availability_zone.cpp @@ -19,6 +19,7 @@ #include "mock_file_ops.h" #include "mock_json_utils.h" #include "mock_logger.h" +#include "mock_subnet_utils.h" #include "mock_virtual_machine.h" #include @@ -41,11 +42,13 @@ struct BaseAvailabilityZoneTest : public Test const std::string az_name{"zone1"}; const mp::fs::path az_dir{"/path/to/zones"}; const mp::fs::path az_file = az_dir / (az_name + ".json"); - const QString az_file_qstr{QString::fromStdU16String(az_file.u16string())}; + const QString az_file_qstr{QString::fromStdU16tring(az_file.u16string())}; + const mp::Subnet az_subnet{"192.168.1.0/24"}; mpt::MockFileOps::GuardedMock mock_file_ops_guard{mpt::MockFileOps::inject()}; mpt::MockJsonUtils::GuardedMock mock_json_utils_guard{mpt::MockJsonUtils::inject()}; mpt::MockLogger::Scope mock_logger{mpt::MockLogger::inject()}; + mpt::MockSubnetUtils::GuardedMock mock_subnet_utils_guard{mpt::MockSubnetUtils::inject()}; }; TEST_F(BaseAvailabilityZoneTest, CreatesDefaultAvailableZone) @@ -58,37 +61,37 @@ TEST_F(BaseAvailabilityZoneTest, CreatesDefaultAvailableZone) EXPECT_CALL(*mock_json_utils_guard.first, write_json(_, QString::fromStdU16String(az_file.u16string()))); + EXPECT_CALL(*mock_subnet_utils_guard.first, generate_random_subnet(_, _)) + .WillOnce(Return(az_subnet)); + mp::BaseAvailabilityZone zone{az_name, az_dir}; EXPECT_EQ(zone.get_name(), az_name); EXPECT_TRUE(zone.is_available()); - // TODO: Subnet generation is not yet implemented - // EXPECT_TRUE(zone.get_subnet().empty()); } -// TODO: Re-implement this test when subnet generation is implemented -// TEST_F(TestBaseAvailabilityZone, loads_existing_zone_file) -// { -// const std::string test_subnet = "10.0.0.0/24"; -// const bool test_available = false; -// -// QJsonObject json{{"subnet", QString::fromStdString(test_subnet)}, {"available", -// test_available}}; -// -// EXPECT_CALL(*mock_json_utils_guard.first, -// read_object_from_file(az_file)).WillOnce(Return(json)); -// -// EXPECT_CALL(*mock_logger.mock_logger, log(_, _, _)).Times(AnyNumber()); -// -// EXPECT_CALL(*mock_json_utils_guard.first, write_json(_, -// QString::fromStdString(az_file.u8string()))); -// -// mp::BaseAvailabilityZone zone{az_name, az_dir}; -// -// EXPECT_EQ(zone.get_name(), az_name); -// EXPECT_EQ(zone.get_subnet(), test_subnet); -// EXPECT_FALSE(zone.is_available()); -// } +TEST_F(BaseAvailabilityZoneTest, loads_existing_zone_file) + { + const mp::Subnet test_subnet{"10.0.0.0/24"}; + const bool test_available = false; + + QJsonObject json{{"subnet", QString::fromStdString(test_subnet.as_string())}, {"available", + test_available}}; + + EXPECT_CALL(*mock_json_utils_guard.first, + read_object_from_file(az_file)).WillOnce(Return(json)); + + EXPECT_CALL(*mock_logger.mock_logger, log(_, _, _)).Times(AnyNumber()); + + EXPECT_CALL(*mock_json_utils_guard.first, write_json(_, + QString::fromStdString(az_file.u8string()))); + + mp::BaseAvailabilityZone zone{az_name, az_dir}; + + EXPECT_EQ(zone.get_name(), az_name); + EXPECT_EQ(zone.get_subnet(), test_subnet); + EXPECT_FALSE(zone.is_available()); + } TEST_F(BaseAvailabilityZoneTest, AddsVmAndUpdatesOnAvailabilityChange) { @@ -103,6 +106,9 @@ TEST_F(BaseAvailabilityZoneTest, AddsVmAndUpdatesOnAvailabilityChange) write_json(_, QString::fromStdU16String(az_file.u16string()))) .Times(2); // Once in constructor, once in set_available + EXPECT_CALL(*mock_subnet_utils_guard.first, generate_random_subnet(_, _)) + .WillOnce(Return(az_subnet)); + mp::BaseAvailabilityZone zone{az_name, az_dir}; const std::string test_vm_name = "test-vm"; @@ -125,6 +131,9 @@ TEST_F(BaseAvailabilityZoneTest, RemovesVmCorrectly) EXPECT_CALL(*mock_json_utils_guard.first, write_json(_, QString::fromStdU16String(az_file.u16string()))); + EXPECT_CALL(*mock_subnet_utils_guard.first, generate_random_subnet(_, _)) + .WillOnce(Return(az_subnet)); + mp::BaseAvailabilityZone zone{az_name, az_dir}; const std::string test_vm_name = "test-vm"; @@ -147,6 +156,9 @@ TEST_F(BaseAvailabilityZoneTest, AvailabilityStateManagement) write_json(_, QString::fromStdU16String(az_file.u16string()))) .Times(2); // Once in constructor, once in set_available + EXPECT_CALL(*mock_subnet_utils_guard.first, generate_random_subnet(_, _)) + .WillOnce(Return(az_subnet)); + mp::BaseAvailabilityZone zone{az_name, az_dir}; const std::string vm1_name = "test-vm1"; diff --git a/tests/test_base_availability_zone_manager.cpp b/tests/test_base_availability_zone_manager.cpp index 1bff4b24bb..8cb33881ed 100644 --- a/tests/test_base_availability_zone_manager.cpp +++ b/tests/test_base_availability_zone_manager.cpp @@ -19,6 +19,7 @@ #include "mock_file_ops.h" #include "mock_json_utils.h" #include "mock_logger.h" +#include "mock_subnet_utils.h" #include #include @@ -46,6 +47,7 @@ struct BaseAvailabilityZoneManagerTest : public Test mpt::MockFileOps::GuardedMock mock_file_ops_guard{mpt::MockFileOps::inject()}; mpt::MockJsonUtils::GuardedMock mock_json_utils_guard{mpt::MockJsonUtils::inject()}; mpt::MockLogger::Scope mock_logger{mpt::MockLogger::inject()}; + mpt::MockSubnetUtils::GuardedMock mock_subnet_utils_guard{mpt::MockSubnetUtils::inject()}; }; TEST_F(BaseAvailabilityZoneManagerTest, CreatesDefaultZones) @@ -67,6 +69,9 @@ TEST_F(BaseAvailabilityZoneManagerTest, CreatesDefaultZones) write_json(_, QString::fromStdU16String(zone_file.u16string()))); } + EXPECT_CALL(*mock_subnet_utils_guard.first, generate_random_subnet(_, _)) + .Times(expected_zone_count).WillRepeatedly(Return(mp::Subnet{"192.168.1.0/24"})); + // Manager file gets written with default zone (once in constructor and once in // get_automatic_zone_name) EXPECT_CALL(*mock_json_utils_guard.first, write_json(_, manager_file_qstr)).Times(2); @@ -101,6 +106,9 @@ TEST_F(BaseAvailabilityZoneManagerTest, UsesZone1WhenAvailable) .Times(AnyNumber()); } + EXPECT_CALL(*mock_subnet_utils_guard.first, generate_random_subnet(_, _)) + .Times(AnyNumber()).WillRepeatedly(Return(mp::Subnet{"192.168.1.0/24"})); + // Manager file will be written multiple times EXPECT_CALL(*mock_json_utils_guard.first, write_json(_, manager_file_qstr)).Times(AnyNumber()); @@ -147,6 +155,9 @@ TEST_F(BaseAvailabilityZoneManagerTest, ThrowsWhenZoneNotFound) .Times(AnyNumber()); } + EXPECT_CALL(*mock_subnet_utils_guard.first, generate_random_subnet(_, _)) + .Times(AnyNumber()).WillRepeatedly(Return(mp::Subnet{"192.168.1.0/24"})); + EXPECT_CALL(*mock_json_utils_guard.first, write_json(_, manager_file_qstr)).Times(AnyNumber()); mp::BaseAvailabilityZoneManager manager{data_dir}; @@ -172,6 +183,9 @@ TEST_F(BaseAvailabilityZoneManagerTest, PrefersZone1ThenZone2ThenZone3) .Times(AnyNumber()); } + EXPECT_CALL(*mock_subnet_utils_guard.first, generate_random_subnet(_, _)) + .Times(AnyNumber()).WillRepeatedly(Return(mp::Subnet{"192.168.1.0/24"})); + EXPECT_CALL(*mock_json_utils_guard.first, write_json(_, manager_file_qstr)).Times(AnyNumber()); mp::BaseAvailabilityZoneManager manager{data_dir}; From bc1a93d32f8f1ca06fc1143846afe315e73deb26 Mon Sep 17 00:00:00 2001 From: trevor-shoe Date: Tue, 30 Sep 2025 06:19:20 -0400 Subject: [PATCH 15/28] [az] Fix Formatting Signed-off-by: trevor-shoe --- include/multipass/subnet.h | 4 +-- src/network/subnet.cpp | 8 +++-- .../backends/qemu/linux/firewall_config.cpp | 36 +++++++++---------- .../qemu/linux/qemu_platform_detail_linux.cpp | 10 ++++-- src/platform/backends/qemu/qemu_platform.h | 3 +- .../qemu/qemu_virtual_machine_factory.cpp | 7 ++-- .../shared/base_availability_zone.cpp | 4 +-- src/platform/platform_linux.cpp | 10 +++--- tests/mock_subnet_utils.h | 5 ++- tests/qemu/linux/test_dnsmasq_server.cpp | 2 +- .../qemu/linux/test_qemu_platform_detail.cpp | 14 +++----- tests/qemu/mock_qemu_platform.h | 5 ++- tests/test_base_availability_zone.cpp | 28 +++++++-------- tests/test_base_availability_zone_manager.cpp | 12 ++++--- tests/test_subnet.cpp | 26 ++++++-------- tests/unix/test_platform_unix.cpp | 5 ++- 16 files changed, 94 insertions(+), 85 deletions(-) diff --git a/include/multipass/subnet.h b/include/multipass/subnet.h index 7a5e0751b9..519c910b80 100644 --- a/include/multipass/subnet.h +++ b/include/multipass/subnet.h @@ -31,7 +31,7 @@ class Subnet public: Subnet(IPAddress ip, uint8_t cidr); Subnet(const std::string& cidr_string); - + [[nodiscard]] IPAddress min_address() const; [[nodiscard]] IPAddress max_address() const; [[nodiscard]] uint32_t address_count() const; @@ -49,6 +49,7 @@ class Subnet [[nodiscard]] bool operator==(const Subnet& other) const; [[nodiscard]] bool operator<(const Subnet& other) const; + private: IPAddress id; uint8_t cidr; @@ -62,4 +63,3 @@ struct SubnetUtils : Singleton Subnet range = Subnet{"10.0.0.0/8"}) const; }; } // namespace multipass - diff --git a/src/network/subnet.cpp b/src/network/subnet.cpp index f2c69ca760..71091a64e7 100644 --- a/src/network/subnet.cpp +++ b/src/network/subnet.cpp @@ -45,7 +45,8 @@ try return mp::Subnet(addr, cidr); } - throw std::invalid_argument(fmt::format("CIDR address {} does not contain '/' seperator", cidr_string)); + throw std::invalid_argument( + fmt::format("CIDR address {} does not contain '/' seperator", cidr_string)); } catch (const std::out_of_range& e) { @@ -98,7 +99,9 @@ mp::Subnet generate_random_subnet(uint8_t cidr, mp::Subnet range) throw std::invalid_argument(fmt::format(large_CIDR_err_fmt, cidr)); if (cidr < range.CIDR()) - throw std::logic_error(fmt::format("A subnet with cidr {} cannot be contained by {}", cidr, range.as_string())); + throw std::logic_error(fmt::format("A subnet with cidr {} cannot be contained by {}", + cidr, + range.as_string())); // ex. 2^(24 - 16) = 256, [192.168.0.0/24, 192.168.255.0/24] const size_t possibleSubnets = std::size_t{1} << (cidr - range.CIDR()); @@ -191,7 +194,6 @@ bool mp::Subnet::operator<(const Subnet& other) const return id < other.id || (id == other.id && cidr > other.cidr); } - mp::Subnet mp::SubnetUtils::generate_random_subnet(uint8_t cidr, Subnet range) const { // @TODO don't rely on pure randomness diff --git a/src/platform/backends/qemu/linux/firewall_config.cpp b/src/platform/backends/qemu/linux/firewall_config.cpp index d01cdb1f63..d90b225ec1 100644 --- a/src/platform/backends/qemu/linux/firewall_config.cpp +++ b/src/platform/backends/qemu/linux/firewall_config.cpp @@ -234,45 +234,45 @@ void set_firewall_rules(const QString& firewall, nat, POSTROUTING, QStringList() - << source << cidr_str << destination << QStringLiteral("224.0.0.0/24") << jump - << RETURN << comment_option); + << source << cidr_str << destination << QStringLiteral("224.0.0.0/24") + << jump << RETURN << comment_option); add_firewall_rule(firewall, nat, POSTROUTING, - QStringList() - << source << cidr_str << destination << QStringLiteral("255.255.255.255/32") - << jump << RETURN << comment_option); + QStringList() << source << cidr_str << destination + << QStringLiteral("255.255.255.255/32") << jump << RETURN + << comment_option); // Masquerade all packets going from VMs to the LAN/Internet add_firewall_rule(firewall, nat, POSTROUTING, QStringList() - << source << cidr_str << negate << destination << cidr_str << protocol << tcp - << jump << MASQUERADE << to_ports << port_range << comment_option); + << source << cidr_str << negate << destination << cidr_str << protocol + << tcp << jump << MASQUERADE << to_ports << port_range << comment_option); add_firewall_rule(firewall, nat, POSTROUTING, QStringList() - << source << cidr_str << negate << destination << cidr_str << protocol << udp - << jump << MASQUERADE << to_ports << port_range << comment_option); + << source << cidr_str << negate << destination << cidr_str << protocol + << udp << jump << MASQUERADE << to_ports << port_range << comment_option); add_firewall_rule(firewall, nat, POSTROUTING, - QStringList() << source << cidr_str << negate << destination << cidr_str << jump - << MASQUERADE << comment_option); + QStringList() << source << cidr_str << negate << destination << cidr_str + << jump << MASQUERADE << comment_option); // Allow established traffic to the private subnet - add_firewall_rule(firewall, - filter, - FORWARD, - QStringList() << destination << cidr_str << out_interface << bridge_name << match - << QStringLiteral("conntrack") << QStringLiteral("--ctstate") - << QStringLiteral("RELATED,ESTABLISHED") << jump << ACCEPT - << comment_option); + add_firewall_rule( + firewall, + filter, + FORWARD, + QStringList() << destination << cidr_str << out_interface << bridge_name << match + << QStringLiteral("conntrack") << QStringLiteral("--ctstate") + << QStringLiteral("RELATED,ESTABLISHED") << jump << ACCEPT << comment_option); // Allow outbound traffic from the private subnet add_firewall_rule(firewall, diff --git a/src/platform/backends/qemu/linux/qemu_platform_detail_linux.cpp b/src/platform/backends/qemu/linux/qemu_platform_detail_linux.cpp index 4889ebe199..f1efd068ab 100644 --- a/src/platform/backends/qemu/linux/qemu_platform_detail_linux.cpp +++ b/src/platform/backends/qemu/linux/qemu_platform_detail_linux.cpp @@ -100,7 +100,8 @@ void set_ip_forward() } } -mp::DNSMasqServer::UPtr init_nat_network(const mp::Path& network_dir, const mp::BridgeSubnetList& subnets) +mp::DNSMasqServer::UPtr init_nat_network(const mp::Path& network_dir, + const mp::BridgeSubnetList& subnets) { set_ip_forward(); return MP_DNSMASQ_SERVER_FACTORY.make_dnsmasq_server(network_dir, subnets); @@ -159,7 +160,8 @@ mp::QemuPlatformDetail::Bridge::~Bridge() return out; } -mp::QemuPlatformDetail::QemuPlatformDetail(const mp::Path& data_dir, const AvailabilityZoneManager::Zones& zones) +mp::QemuPlatformDetail::QemuPlatformDetail(const mp::Path& data_dir, + const AvailabilityZoneManager::Zones& zones) : network_dir{MP_UTILS.make_dir(QDir(data_dir), "network")}, bridges{get_bridges(zones)}, dnsmasq_server{init_nat_network(network_dir, get_bridge_list(bridges))} @@ -260,7 +262,9 @@ QStringList mp::QemuPlatformDetail::vm_platform_args(const VirtualMachineDescrip return opts; } -mp::QemuPlatform::UPtr mp::QemuPlatformFactory::make_qemu_platform(const Path& data_dir, const mp::AvailabilityZoneManager::Zones& zones) const +mp::QemuPlatform::UPtr mp::QemuPlatformFactory::make_qemu_platform( + const Path& data_dir, + const mp::AvailabilityZoneManager::Zones& zones) const { return std::make_unique(data_dir, zones); } diff --git a/src/platform/backends/qemu/qemu_platform.h b/src/platform/backends/qemu/qemu_platform.h index 16270946a3..bf77708571 100644 --- a/src/platform/backends/qemu/qemu_platform.h +++ b/src/platform/backends/qemu/qemu_platform.h @@ -71,6 +71,7 @@ class QemuPlatformFactory : public Singleton QemuPlatformFactory(const Singleton::PrivatePass& pass) noexcept : Singleton::Singleton{pass} {}; - virtual QemuPlatform::UPtr make_qemu_platform(const Path& data_dir, const AvailabilityZoneManager::Zones& zones) const; + virtual QemuPlatform::UPtr + make_qemu_platform(const Path& data_dir, const AvailabilityZoneManager::Zones& zones) const; }; } // namespace multipass diff --git a/src/platform/backends/qemu/qemu_virtual_machine_factory.cpp b/src/platform/backends/qemu/qemu_virtual_machine_factory.cpp index eda0e59291..c16cda5248 100644 --- a/src/platform/backends/qemu/qemu_virtual_machine_factory.cpp +++ b/src/platform/backends/qemu/qemu_virtual_machine_factory.cpp @@ -38,9 +38,10 @@ constexpr auto category = "qemu factory"; mp::QemuVirtualMachineFactory::QemuVirtualMachineFactory(const mp::Path& data_dir, AvailabilityZoneManager& az_manager) - : QemuVirtualMachineFactory{MP_QEMU_PLATFORM_FACTORY.make_qemu_platform(data_dir, az_manager.get_zones()), - data_dir, - az_manager} + : QemuVirtualMachineFactory{ + MP_QEMU_PLATFORM_FACTORY.make_qemu_platform(data_dir, az_manager.get_zones()), + data_dir, + az_manager} { } diff --git a/src/platform/backends/shared/base_availability_zone.cpp b/src/platform/backends/shared/base_availability_zone.cpp index e7f91e713a..c52f93575b 100644 --- a/src/platform/backends/shared/base_availability_zone.cpp +++ b/src/platform/backends/shared/base_availability_zone.cpp @@ -44,8 +44,8 @@ catch (const std::ios_base::failure& e) } [[nodiscard]] multipass::Subnet deserialize_subnet(const QJsonObject& json, - const multipass::fs::path& file_path, - const std::string& name) + const multipass::fs::path& file_path, + const std::string& name) { if (const auto json_subnet = json[subnet_key].toString().toStdString(); !json_subnet.empty()) return json_subnet; diff --git a/src/platform/platform_linux.cpp b/src/platform/platform_linux.cpp index e8857417a7..67ebc8b112 100644 --- a/src/platform/platform_linux.cpp +++ b/src/platform/platform_linux.cpp @@ -361,7 +361,8 @@ bool mp::platform::Platform::can_reach_gateway(mp::IPAddress ip) const } // validation of the ip and cidr happen later, otherwise this regex would be massive. -const QRegularExpression subnet_regex(R"(((?:[0-9][0-9]?[0-9]?\.){3}[0-9][0-9]?[0-9]?\/[0-9][0-9]?))"); +const QRegularExpression subnet_regex( + R"(((?:[0-9][0-9]?[0-9]?\.){3}[0-9][0-9]?[0-9]?\/[0-9][0-9]?))"); bool mp::platform::Platform::subnet_used_locally(mp::Subnet subnet) const { @@ -386,10 +387,9 @@ bool mp::platform::Platform::subnet_used_locally(mp::Subnet subnet) const } catch (const std::invalid_argument& e) { - mpl::log( - mpl::Level::warning, - "network", - fmt::format("invalid subnet from ip command: {}", e.what())); + mpl::log(mpl::Level::warning, + "network", + fmt::format("invalid subnet from ip command: {}", e.what())); } } return false; diff --git a/tests/mock_subnet_utils.h b/tests/mock_subnet_utils.h index ce1905d949..173aa33caf 100644 --- a/tests/mock_subnet_utils.h +++ b/tests/mock_subnet_utils.h @@ -1,5 +1,5 @@ /* -* Copyright (C) Canonical, Ltd. + * Copyright (C) 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 @@ -15,7 +15,7 @@ * */ -#pragma once +#pragma once #include "mock_singleton_helpers.h" @@ -32,4 +32,3 @@ struct MockSubnetUtils : public SubnetUtils MP_MOCK_SINGLETON_BOILERPLATE(MockSubnetUtils, SubnetUtils); }; } // namespace multipass::test - diff --git a/tests/qemu/linux/test_dnsmasq_server.cpp b/tests/qemu/linux/test_dnsmasq_server.cpp index e1a8c17054..f21bcb7d22 100644 --- a/tests/qemu/linux/test_dnsmasq_server.cpp +++ b/tests/qemu/linux/test_dnsmasq_server.cpp @@ -96,7 +96,7 @@ struct DNSMasqServer : public mpt::TestWithMockedBinPath const mp::IPAddress expected_ip{"10.177.224.22"}; [[nodiscard]] static mp::BridgeSubnetList make_subnets(const QString& bridge, - const mp::Subnet& subnet) + const mp::Subnet& subnet) { return {{bridge, subnet}}; } diff --git a/tests/qemu/linux/test_qemu_platform_detail.cpp b/tests/qemu/linux/test_qemu_platform_detail.cpp index fc3c620dcf..2d73574f95 100644 --- a/tests/qemu/linux/test_qemu_platform_detail.cpp +++ b/tests/qemu/linux/test_qemu_platform_detail.cpp @@ -54,8 +54,7 @@ struct QemuPlatformDetail : public Test { EXPECT_CALL(*mock_firewall_config_factory, make_firewall_config(vswitch.bridge_name, vswitch.subnet)) - .WillOnce( - [&vswitch](auto...) { return std::move(vswitch.mock_firewall_config); }); + .WillOnce([&vswitch](auto...) { return std::move(vswitch.mock_firewall_config); }); EXPECT_CALL( *mock_utils, @@ -115,17 +114,14 @@ struct QemuPlatformDetail : public Test static inline const mp::Subnet zone1_subnet{"192.168.64.0/24"}; static inline const mp::Subnet zone2_subnet{"192.168.96.0/24"}; static inline const mp::Subnet zone3_subnet{"192.168.128.0/24"}; - const std::vector switches{ - {"mpqemubrzone1", "52:54:00:6f:29:7e", zone1_subnet, "foo"}, - {"mpqemubrzone2", "52:54:00:6f:29:7f", zone2_subnet, "bar"}, - {"mpqemubrzone3", "52:54:00:6f:29:80", zone3_subnet, "baz"}}; + const std::vector switches{{"mpqemubrzone1", "52:54:00:6f:29:7e", zone1_subnet, "foo"}, + {"mpqemubrzone2", "52:54:00:6f:29:7f", zone2_subnet, "bar"}, + {"mpqemubrzone3", "52:54:00:6f:29:80", zone3_subnet, "baz"}}; mpt::MockAvailabilityZone mock_zone1; mpt::MockAvailabilityZone mock_zone2; mpt::MockAvailabilityZone mock_zone3; - const multipass::AvailabilityZoneManager::Zones mock_zones{ - mock_zone1, mock_zone2, mock_zone3 - }; + const multipass::AvailabilityZoneManager::Zones mock_zones{mock_zone1, mock_zone2, mock_zone3}; mpt::TempDir data_dir; diff --git a/tests/qemu/mock_qemu_platform.h b/tests/qemu/mock_qemu_platform.h index b57fa3e3ff..ac0d101059 100644 --- a/tests/qemu/mock_qemu_platform.h +++ b/tests/qemu/mock_qemu_platform.h @@ -51,7 +51,10 @@ struct MockQemuPlatformFactory : public QemuPlatformFactory { using QemuPlatformFactory::QemuPlatformFactory; - MOCK_METHOD(QemuPlatform::UPtr, make_qemu_platform, (const Path&, const AvailabilityZoneManager::Zones&), (const, override)); + MOCK_METHOD(QemuPlatform::UPtr, + make_qemu_platform, + (const Path&, const AvailabilityZoneManager::Zones&), + (const, override)); MP_MOCK_SINGLETON_BOILERPLATE(MockQemuPlatformFactory, QemuPlatformFactory); }; diff --git a/tests/test_base_availability_zone.cpp b/tests/test_base_availability_zone.cpp index 15ebff7f02..64caa30c7b 100644 --- a/tests/test_base_availability_zone.cpp +++ b/tests/test_base_availability_zone.cpp @@ -71,27 +71,27 @@ TEST_F(BaseAvailabilityZoneTest, CreatesDefaultAvailableZone) } TEST_F(BaseAvailabilityZoneTest, loads_existing_zone_file) - { +{ const mp::Subnet test_subnet{"10.0.0.0/24"}; - const bool test_available = false; + const bool test_available = false; - QJsonObject json{{"subnet", QString::fromStdString(test_subnet.as_string())}, {"available", - test_available}}; + QJsonObject json{{"subnet", QString::fromStdString(test_subnet.as_string())}, + {"available", test_available}}; - EXPECT_CALL(*mock_json_utils_guard.first, - read_object_from_file(az_file)).WillOnce(Return(json)); + EXPECT_CALL(*mock_json_utils_guard.first, read_object_from_file(az_file)) + .WillOnce(Return(json)); - EXPECT_CALL(*mock_logger.mock_logger, log(_, _, _)).Times(AnyNumber()); + EXPECT_CALL(*mock_logger.mock_logger, log(_, _, _)).Times(AnyNumber()); - EXPECT_CALL(*mock_json_utils_guard.first, write_json(_, - QString::fromStdString(az_file.u8string()))); + EXPECT_CALL(*mock_json_utils_guard.first, + write_json(_, QString::fromStdString(az_file.u8string()))); - mp::BaseAvailabilityZone zone{az_name, az_dir}; + mp::BaseAvailabilityZone zone{az_name, az_dir}; - EXPECT_EQ(zone.get_name(), az_name); - EXPECT_EQ(zone.get_subnet(), test_subnet); - EXPECT_FALSE(zone.is_available()); - } + EXPECT_EQ(zone.get_name(), az_name); + EXPECT_EQ(zone.get_subnet(), test_subnet); + EXPECT_FALSE(zone.is_available()); +} TEST_F(BaseAvailabilityZoneTest, AddsVmAndUpdatesOnAvailabilityChange) { diff --git a/tests/test_base_availability_zone_manager.cpp b/tests/test_base_availability_zone_manager.cpp index 8cb33881ed..af8b588b2d 100644 --- a/tests/test_base_availability_zone_manager.cpp +++ b/tests/test_base_availability_zone_manager.cpp @@ -70,7 +70,8 @@ TEST_F(BaseAvailabilityZoneManagerTest, CreatesDefaultZones) } EXPECT_CALL(*mock_subnet_utils_guard.first, generate_random_subnet(_, _)) - .Times(expected_zone_count).WillRepeatedly(Return(mp::Subnet{"192.168.1.0/24"})); + .Times(expected_zone_count) + .WillRepeatedly(Return(mp::Subnet{"192.168.1.0/24"})); // Manager file gets written with default zone (once in constructor and once in // get_automatic_zone_name) @@ -107,7 +108,8 @@ TEST_F(BaseAvailabilityZoneManagerTest, UsesZone1WhenAvailable) } EXPECT_CALL(*mock_subnet_utils_guard.first, generate_random_subnet(_, _)) - .Times(AnyNumber()).WillRepeatedly(Return(mp::Subnet{"192.168.1.0/24"})); + .Times(AnyNumber()) + .WillRepeatedly(Return(mp::Subnet{"192.168.1.0/24"})); // Manager file will be written multiple times EXPECT_CALL(*mock_json_utils_guard.first, write_json(_, manager_file_qstr)).Times(AnyNumber()); @@ -156,7 +158,8 @@ TEST_F(BaseAvailabilityZoneManagerTest, ThrowsWhenZoneNotFound) } EXPECT_CALL(*mock_subnet_utils_guard.first, generate_random_subnet(_, _)) - .Times(AnyNumber()).WillRepeatedly(Return(mp::Subnet{"192.168.1.0/24"})); + .Times(AnyNumber()) + .WillRepeatedly(Return(mp::Subnet{"192.168.1.0/24"})); EXPECT_CALL(*mock_json_utils_guard.first, write_json(_, manager_file_qstr)).Times(AnyNumber()); @@ -184,7 +187,8 @@ TEST_F(BaseAvailabilityZoneManagerTest, PrefersZone1ThenZone2ThenZone3) } EXPECT_CALL(*mock_subnet_utils_guard.first, generate_random_subnet(_, _)) - .Times(AnyNumber()).WillRepeatedly(Return(mp::Subnet{"192.168.1.0/24"})); + .Times(AnyNumber()) + .WillRepeatedly(Return(mp::Subnet{"192.168.1.0/24"})); EXPECT_CALL(*mock_json_utils_guard.first, write_json(_, manager_file_qstr)).Times(AnyNumber()); diff --git a/tests/test_subnet.cpp b/tests/test_subnet.cpp index 620b9f79a1..11ea6323d3 100644 --- a/tests/test_subnet.cpp +++ b/tests/test_subnet.cpp @@ -212,7 +212,7 @@ TEST(Subnet, containsWorksOnUnContainedSubnets) container = mp::Subnet{"172.1.0.0/16"}; EXPECT_FALSE(subnet.contains(container)); - + // subset container = mp::Subnet{"0.0.0.0/0"}; EXPECT_FALSE(subnet.contains(container)); @@ -281,7 +281,7 @@ TEST_F(SubnetUtils, generateRandomSubnetTriviallyWorks) { const mp::Subnet range{"10.1.2.0/24"}; - EXPECT_CALL(*mock_utils, random_int(_, _)).WillOnce(Invoke([](auto a, auto b){ + EXPECT_CALL(*mock_utils, random_int(_, _)).WillOnce(Invoke([](auto a, auto b) { EXPECT_EQ(a, b); return a; })); @@ -304,7 +304,8 @@ TEST_F(SubnetUtils, generateRandomSubnetFailsOnBadCIDR) { mp::Subnet range{"0.0.0.0/0"}; - EXPECT_THROW(std::ignore = MP_SUBNET_UTILS.generate_random_subnet(31, range), std::invalid_argument); + EXPECT_THROW(std::ignore = MP_SUBNET_UTILS.generate_random_subnet(31, range), + std::invalid_argument); EXPECT_THROW(std::ignore = MP_SUBNET_UTILS.generate_random_subnet(32, range), std::invalid_argument); EXPECT_THROW(std::ignore = MP_SUBNET_UTILS.generate_random_subnet(33, range), @@ -316,12 +317,10 @@ TEST_F(SubnetUtils, generateRandomSubnetFailsOnBadCIDR) TEST_F(SubnetUtils, generateRandomSubnetRespectsRange) { mp::Subnet range("192.168.0.0/16"); - + auto [mock_utils, guard] = mpt::MockUtils::inject(); - EXPECT_CALL(*mock_utils, random_int(_, _)) - .WillOnce(ReturnArg<0>()) - .WillOnce(ReturnArg<1>()); + EXPECT_CALL(*mock_utils, random_int(_, _)).WillOnce(ReturnArg<0>()).WillOnce(ReturnArg<1>()); auto subnetLow = MP_SUBNET_UTILS.generate_random_subnet(24, range); auto subnetHigh = MP_SUBNET_UTILS.generate_random_subnet(24, range); @@ -337,9 +336,7 @@ TEST_F(SubnetUtils, generateRandomSubnetWorksAtUpperExtreme) { mp::Subnet range("0.0.0.0/0"); - EXPECT_CALL(*mock_utils, random_int(_, _)) - .WillOnce(ReturnArg<0>()) - .WillOnce(ReturnArg<1>()); + EXPECT_CALL(*mock_utils, random_int(_, _)).WillOnce(ReturnArg<0>()).WillOnce(ReturnArg<1>()); auto subnetLow = MP_SUBNET_UTILS.generate_random_subnet(30, range); auto subnetHigh = MP_SUBNET_UTILS.generate_random_subnet(30, range); @@ -355,12 +352,11 @@ TEST_F(SubnetUtils, generateRandomSubnetGivesUp) { mp::Subnet range("0.0.0.0/0"); - EXPECT_CALL(*mock_utils, random_int(_, _)) - .WillRepeatedly(ReturnArg<0>()); + EXPECT_CALL(*mock_utils, random_int(_, _)).WillRepeatedly(ReturnArg<0>()); - EXPECT_CALL(*mock_platform, subnet_used_locally) - .WillRepeatedly(Return(true)); + EXPECT_CALL(*mock_platform, subnet_used_locally).WillRepeatedly(Return(true)); MP_EXPECT_THROW_THAT(std::ignore = MP_SUBNET_UTILS.generate_random_subnet(24, range), - std::runtime_error, mpt::match_what(HasSubstr("subnet"))); + std::runtime_error, + mpt::match_what(HasSubstr("subnet"))); } diff --git a/tests/unix/test_platform_unix.cpp b/tests/unix/test_platform_unix.cpp index c16a2ad5a3..da4cea0bc2 100644 --- a/tests/unix/test_platform_unix.cpp +++ b/tests/unix/test_platform_unix.cpp @@ -344,7 +344,10 @@ TEST_F(TestPlatformUnix, canReachGatewayRunsPingWithIP) auto [mock_utils, guard] = mpt::MockUtils::inject(); - EXPECT_CALL(*mock_utils, run_cmd_for_status(QString("ping"), Contains(QString::fromStdString(testIPstr)), _)).WillOnce(Return(true)).WillOnce(Return(false)); + EXPECT_CALL(*mock_utils, + run_cmd_for_status(QString("ping"), Contains(QString::fromStdString(testIPstr)), _)) + .WillOnce(Return(true)) + .WillOnce(Return(false)); EXPECT_TRUE(MP_PLATFORM.can_reach_gateway(testIP)); EXPECT_FALSE(MP_PLATFORM.can_reach_gateway(testIP)); From e6e4c21000ac8c12c62058fc5cd51c5eef66c753 Mon Sep 17 00:00:00 2001 From: trevor-shoe Date: Tue, 30 Sep 2025 06:39:11 -0400 Subject: [PATCH 16/28] [az] Fix make_qemu_platform on MacOS --- .../backends/qemu/macos/qemu_platform_detail_macos.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/platform/backends/qemu/macos/qemu_platform_detail_macos.cpp b/src/platform/backends/qemu/macos/qemu_platform_detail_macos.cpp index 1b74899aae..a3baba6d30 100644 --- a/src/platform/backends/qemu/macos/qemu_platform_detail_macos.cpp +++ b/src/platform/backends/qemu/macos/qemu_platform_detail_macos.cpp @@ -115,7 +115,9 @@ void mp::QemuPlatformDetail::set_authorization(std::vector // nothing to do here } -mp::QemuPlatform::UPtr mp::QemuPlatformFactory::make_qemu_platform(const Path& data_dir) const +mp::QemuPlatform::UPtr mp::QemuPlatformFactory::make_qemu_platform( + const Path& data_dir, + const mp::AvailabilityZoneManager::Zones& zones) const { return std::make_unique(); } From 2abb5de19462d07c3f00dafdd22a73c81cef1e9c Mon Sep 17 00:00:00 2001 From: trevor-shoe Date: Tue, 30 Sep 2025 08:01:48 -0400 Subject: [PATCH 17/28] [network] Add C++20 spaceship todos --- include/multipass/ip_address.h | 2 ++ include/multipass/subnet.h | 4 +++- src/network/subnet.cpp | 13 ++++++++----- tests/test_subnet.cpp | 32 ++++++++++++++++++++++++++++++++ 4 files changed, 45 insertions(+), 6 deletions(-) diff --git a/include/multipass/ip_address.h b/include/multipass/ip_address.h index 9ecd215f34..afa75eb459 100644 --- a/include/multipass/ip_address.h +++ b/include/multipass/ip_address.h @@ -35,6 +35,8 @@ struct IPAddress std::string as_string() const; uint32_t as_uint32() const; + // TODO C++20 uncomment then remove other bool operators + // auto operator<=>(const IPAddress& other) const = default; bool operator==(const IPAddress& other) const; bool operator!=(const IPAddress& other) const; bool operator<(const IPAddress& other) const; diff --git a/include/multipass/subnet.h b/include/multipass/subnet.h index 519c910b80..a098f961e9 100644 --- a/include/multipass/subnet.h +++ b/include/multipass/subnet.h @@ -47,8 +47,10 @@ class Subnet [[nodiscard]] bool contains(Subnet other) const; [[nodiscard]] bool contains(IPAddress ip) const; + // TODO C++20 uncomment then remove existing operator== + // [[nodiscard]] std::strong_ordering operator<=>(const Subnet& other) const; + // [[nodiscard]] bool operator==(const Subnet& other) const == default; [[nodiscard]] bool operator==(const Subnet& other) const; - [[nodiscard]] bool operator<(const Subnet& other) const; private: IPAddress id; diff --git a/src/network/subnet.cpp b/src/network/subnet.cpp index 71091a64e7..636f175f55 100644 --- a/src/network/subnet.cpp +++ b/src/network/subnet.cpp @@ -122,8 +122,7 @@ mp::Subnet::Subnet(IPAddress ip, uint8_t cidr) : id(apply_mask(ip, cidr)), cidr( { if (cidr >= 31) { - throw std::invalid_argument( - fmt::format("CIDR value must be non-negative and less than 31: {}", cidr)); + throw std::invalid_argument(fmt::format(large_CIDR_err_fmt, cidr)); } } @@ -188,11 +187,15 @@ bool mp::Subnet::operator==(const Subnet& other) const return id == other.id && cidr == other.cidr; } -bool mp::Subnet::operator<(const Subnet& other) const +/* TODO C++20 uncomment +std::strong_ordering mp::Subnet::operator<=>(const Subnet& other) const { - // note cidr comparison is flipped, smaller is bigger - return id < other.id || (id == other.id && cidr > other.cidr); + const auto ip_res = id <=> other.id; + + // note the cidr operands are purposely flipped + return (ip_res == 0) ? other.cidr <=> cidr : ip_res; } +*/ mp::Subnet mp::SubnetUtils::generate_random_subnet(uint8_t cidr, Subnet range) const { diff --git a/tests/test_subnet.cpp b/tests/test_subnet.cpp index 11ea6323d3..c4ce91e002 100644 --- a/tests/test_subnet.cpp +++ b/tests/test_subnet.cpp @@ -65,6 +65,10 @@ TEST(Subnet, throwsOnLargeCIDR) std::invalid_argument, mpt::match_what(HasSubstr(what))); + MP_EXPECT_THROW_THAT((mp::Subnet{mp::IPAddress{"192.168.0.0"}, 31}), + std::invalid_argument, + mpt::match_what(HasSubstr(what))); + // valid but not supported MP_EXPECT_THROW_THAT(mp::Subnet{"192.168.0.0/32"}, std::invalid_argument, @@ -262,6 +266,34 @@ TEST(Subnet, containsWorksOnUnContainedIps) EXPECT_FALSE(subnet.contains(ip)); } +/* TODO C++20 uncomment +TEST(Subnet, relationalComparisonsWorkAsExpected) +{ + const mp::Subnet low{"0.0.0.0/0"}; + const mp::Subnet middle{"192.168.0.0/16"}; + const mp::Subnet submiddle{middle.identifier(), 24}; + const mp::Subnet high{"255.255.255.0/24"}; + + EXPECT_LT(low, middle); + EXPECT_LT(low, submiddle); + EXPECT_LT(low, high); + EXPECT_LE(low, low); + EXPECT_GE(low, low); + + EXPECT_GT(high, low); + EXPECT_GT(high, submiddle); + EXPECT_GT(high, middle); + + EXPECT_GT(middle, low); + EXPECT_GT(middle, submiddle); + EXPECT_LT(middle, high); + + EXPECT_GT(submiddle, low); + EXPECT_LT(submiddle, middle); + EXPECT_LT(submiddle, high); +} +*/ + struct SubnetUtils : public Test { SubnetUtils() From 380fa6c16256fc1fe9be5a16202303e1ee3c38f8 Mon Sep 17 00:00:00 2001 From: trevor-shoe Date: Tue, 30 Sep 2025 08:10:02 -0400 Subject: [PATCH 18/28] [network] Add missing subnet coverage --- tests/linux/test_platform_linux.cpp | 25 ++++++++++++++++++++++--- tests/test_subnet.cpp | 16 +++++++++++++++- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/tests/linux/test_platform_linux.cpp b/tests/linux/test_platform_linux.cpp index 78902e0ec5..e09333f5de 100644 --- a/tests/linux/test_platform_linux.cpp +++ b/tests/linux/test_platform_linux.cpp @@ -694,7 +694,7 @@ TEST_F(PlatformLinux, subnetUsedLocallyDetectsUnused) EXPECT_CALL(*mock_utils, run_cmd_for_output(QString("ip"), _, _)).WillOnce(Return(R"( default via 192.168.0.0 dev wlo1 proto dhcp src 192.168.0.1 metric 600 10.20.30.0/24 dev lxdbr0 proto kernel scope link src 10.20.30.1 -10.192.168.0/24 dev mpqemubr0 proto kernel scop link src 10.192.168.1 linkdown +10.192.168.0/24 dev mpqemubr0 proto kernel scope link src 10.192.168.1 linkdown 10.255.19.0/24 dev mpbr0 proto kernel scope link src 10.255.19.1 172.172.0.0/16 dev docker0 proto kernel scope link src 172.172.0.1 linkdown 192.168.0.0/24 dev wlo1 proto kernel scope link src 192.168.0.1 metric 600 @@ -713,7 +713,7 @@ TEST_F(PlatformLinux, subnetUsedLocallyDetectsOverlapping) EXPECT_CALL(*mock_utils, run_cmd_for_output(QString("ip"), _, _)).WillOnce(Return(R"( default via 192.168.0.0 dev wlo1 proto dhcp src 192.168.0.1 metric 600 10.20.30.0/24 dev lxdbr0 proto kernel scope link src 10.20.30.1 -10.192.168.0/24 dev mpqemubr0 proto kernel scop link src 10.192.168.1 linkdown +10.192.168.0/24 dev mpqemubr0 proto kernel scope link src 10.192.168.1 linkdown 10.255.19.0/24 dev mpbr0 proto kernel scope link src 10.255.19.1 172.172.0.0/16 dev docker0 proto kernel scope link src 172.172.0.1 linkdown 192.168.0.0/24 dev wlo1 proto kernel scope link src 192.168.0.1 metric 600 @@ -732,7 +732,7 @@ TEST_F(PlatformLinux, subnetUsedLocallyDetectsConflicting) EXPECT_CALL(*mock_utils, run_cmd_for_output(QString("ip"), _, _)).WillOnce(Return(R"( default via 192.168.0.0 dev wlo1 proto dhcp src 192.168.0.1 metric 600 10.20.30.0/24 dev lxdbr0 proto kernel scope link src 10.20.30.1 -10.192.168.0/24 dev mpqemubr0 proto kernel scop link src 10.192.168.1 linkdown +10.192.168.0/24 dev mpqemubr0 proto kernel scope link src 10.192.168.1 linkdown 10.255.19.0/24 dev mpbr0 proto kernel scope link src 10.255.19.1 172.172.0.0/16 dev docker0 proto kernel scope link src 172.172.0.1 linkdown 192.168.0.0/24 dev wlo1 proto kernel scope link src 192.168.0.1 metric 600 @@ -741,4 +741,23 @@ default via 192.168.0.0 dev wlo1 proto dhcp src 192.168.0.1 metric 600 EXPECT_TRUE(MP_PLATFORM.subnet_used_locally(testSubnet)); } + +TEST_F(PlatformLinux, subnetUsedLocallyHandlesBadSubnets) +{ + const mp::Subnet testSubnet{"10.255.19.0/24"}; + + auto [mock_utils, guard] = mpt::MockUtils::inject(); + + EXPECT_CALL(*mock_utils, run_cmd_for_output(QString("ip"), _, _)).WillOnce(Return(R"( +default via 192.168.0.0 dev wlo1 proto dhcp src 192.168.0.1 metric 600 +(incomplete) dev lxdbr0 proto kernel scope link src (unknown) +10.192.168.0/33 dev mpqemubr0 proto kernel scope link src 10.192.168.1 +10.255.19.0/24 dev mpbr0 proto kernel scope link src 10.255.19.1 +172.256.0.0/16 dev docker0 proto kernel scope link src 172.256.0.1 linkdown +192.168.0.256/24 dev wlo1 proto kernel scope link src 192.168.0.1 metric 600 +256.168.123.0/24 dev virbr0 proto kernel scope link src 256.168.123.1 linkdown +)")); + + EXPECT_TRUE(MP_PLATFORM.subnet_used_locally(testSubnet)); +} } // namespace diff --git a/tests/test_subnet.cpp b/tests/test_subnet.cpp index c4ce91e002..f2f65c5a43 100644 --- a/tests/test_subnet.cpp +++ b/tests/test_subnet.cpp @@ -380,7 +380,7 @@ TEST_F(SubnetUtils, generateRandomSubnetWorksAtUpperExtreme) EXPECT_EQ(subnetHigh.CIDR(), 30); } -TEST_F(SubnetUtils, generateRandomSubnetGivesUp) +TEST_F(SubnetUtils, generateRandomSubnetGivesUpUsedLocally) { mp::Subnet range("0.0.0.0/0"); @@ -392,3 +392,17 @@ TEST_F(SubnetUtils, generateRandomSubnetGivesUp) std::runtime_error, mpt::match_what(HasSubstr("subnet"))); } + +TEST_F(SubnetUtils, generateRandomSubnetGivesUpGatewayReached) +{ + mp::Subnet range("0.0.0.0/0"); + + EXPECT_CALL(*mock_utils, random_int(_, _)).WillRepeatedly(ReturnArg<0>()); + + EXPECT_CALL(*mock_platform, subnet_used_locally).WillRepeatedly(Return(false)); + EXPECT_CALL(*mock_platform, can_reach_gateway).WillRepeatedly(Return(true)); + + MP_EXPECT_THROW_THAT(std::ignore = MP_SUBNET_UTILS.generate_random_subnet(24, range), + std::runtime_error, + mpt::match_what(HasSubstr("subnet"))); +} From 66a2f6c42aaf758e1f11bb886e64cff7474eb2fe Mon Sep 17 00:00:00 2001 From: trevor-shoe Date: Tue, 30 Sep 2025 12:27:46 -0400 Subject: [PATCH 19/28] [network] Use warn instead of log --- src/platform/platform_linux.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/platform/platform_linux.cpp b/src/platform/platform_linux.cpp index 67ebc8b112..86afb289cd 100644 --- a/src/platform/platform_linux.cpp +++ b/src/platform/platform_linux.cpp @@ -387,9 +387,7 @@ bool mp::platform::Platform::subnet_used_locally(mp::Subnet subnet) const } catch (const std::invalid_argument& e) { - mpl::log(mpl::Level::warning, - "network", - fmt::format("invalid subnet from ip command: {}", e.what())); + mpl::warn(category, "invalid subnet from ip command: {}", e.what()) } } return false; From 12f324f78d5940eb0502e8a204a13d72f3e2acf7 Mon Sep 17 00:00:00 2001 From: trevor-shoe Date: Wed, 1 Oct 2025 12:45:36 -0400 Subject: [PATCH 20/28] [network] Use strong type for prefix len --- include/multipass/subnet.h | 39 +++++++++++++++++++--- src/network/subnet.cpp | 68 +++++++++++++++++--------------------- tests/mock_subnet_utils.h | 5 ++- tests/test_subnet.cpp | 65 +++++++++++++++--------------------- 4 files changed, 96 insertions(+), 81 deletions(-) diff --git a/include/multipass/subnet.h b/include/multipass/subnet.h index a098f961e9..0eb47e818d 100644 --- a/include/multipass/subnet.h +++ b/include/multipass/subnet.h @@ -17,6 +17,7 @@ #pragma once +#include #include #include @@ -29,7 +30,37 @@ namespace multipass class Subnet { public: - Subnet(IPAddress ip, uint8_t cidr); + struct PrefixLengthOutOfRange final : FormattedExceptionBase + { + template + explicit PrefixLengthOutOfRange(const T& value) + : FormattedExceptionBase{ + "Subnet prefix length must be non-negative and less than 31: {}", + value} + { + } + }; + + class PrefixLength + { + public: + constexpr PrefixLength(uint8_t value) : value(value) + { + if (value >= 31) + throw PrefixLengthOutOfRange{value}; + } + + constexpr operator uint8_t() const noexcept + { + return value; + } + + private: + uint8_t value; + }; + + Subnet(IPAddress ip, PrefixLength prefix_length); + Subnet(const std::string& cidr_string); [[nodiscard]] IPAddress min_address() const; @@ -37,7 +68,7 @@ class Subnet [[nodiscard]] uint32_t address_count() const; [[nodiscard]] IPAddress identifier() const; - [[nodiscard]] uint8_t CIDR() const; + [[nodiscard]] PrefixLength prefix_length() const; [[nodiscard]] IPAddress subnet_mask() const; // uses CIDR notation @@ -54,14 +85,14 @@ class Subnet private: IPAddress id; - uint8_t cidr; + PrefixLength prefix; }; struct SubnetUtils : Singleton { using Singleton::Singleton; - [[nodiscard]] virtual Subnet generate_random_subnet(uint8_t cidr = 24, + [[nodiscard]] virtual Subnet generate_random_subnet(Subnet::PrefixLength prefix = 24, Subnet range = Subnet{"10.0.0.0/8"}) const; }; } // namespace multipass diff --git a/src/network/subnet.cpp b/src/network/subnet.cpp index 636f175f55..53c6610157 100644 --- a/src/network/subnet.cpp +++ b/src/network/subnet.cpp @@ -30,8 +30,6 @@ namespace mp = multipass; namespace { -constexpr auto large_CIDR_err_fmt = "CIDR value must be non-negative and less than 31: {}"; - mp::Subnet parse(const std::string& cidr_string) try { @@ -39,21 +37,21 @@ try { mp::IPAddress addr{cidr_string.substr(0, i)}; - auto cidr = std::stoul(cidr_string.substr(i + 1)); - if (cidr >= 31) - throw std::invalid_argument(fmt::format(large_CIDR_err_fmt, cidr)); + const auto prefix_length = std::stoul(cidr_string.substr(i + 1)); + if (prefix_length >= 31) + throw mp::Subnet::PrefixLengthOutOfRange(prefix_length); - return mp::Subnet(addr, cidr); + return mp::Subnet(addr, prefix_length); } throw std::invalid_argument( - fmt::format("CIDR address {} does not contain '/' seperator", cidr_string)); + fmt::format("CIDR {:?} does not contain '/' seperator", cidr_string)); } catch (const std::out_of_range& e) { - throw std::invalid_argument(fmt::format(large_CIDR_err_fmt, e.what())); + throw mp::Subnet::PrefixLengthOutOfRange(e.what()); } -[[nodiscard]] mp::IPAddress get_subnet_mask(uint8_t cidr) +[[nodiscard]] mp::IPAddress get_subnet_mask(mp::Subnet::PrefixLength prefix_length) { using Octects = decltype(mp::IPAddress::octets); static constexpr auto value_size = sizeof(Octects::value_type) * 8; @@ -61,11 +59,11 @@ catch (const std::out_of_range& e) Octects octets{}; std::fill(octets.begin(), octets.end(), std::numeric_limits::max()); - if (cidr > value_size * octets.size()) - throw std::out_of_range("CIDR too large for address space"); + if (prefix_length > value_size * octets.size()) + throw std::out_of_range("prefix length too large for address space"); - const uint8_t start_octet = cidr / value_size; - const uint8_t remain = cidr % value_size; + const uint8_t start_octet = prefix_length / value_size; + const uint8_t remain = prefix_length % value_size; for (size_t i = start_octet; i < octets.size(); ++i) { @@ -83,9 +81,9 @@ catch (const std::out_of_range& e) return mp::IPAddress{octets}; } -[[nodiscard]] mp::IPAddress apply_mask(mp::IPAddress ip, uint8_t cidr) +[[nodiscard]] mp::IPAddress apply_mask(mp::IPAddress ip, mp::Subnet::PrefixLength prefix_length) { - const auto mask = get_subnet_mask(cidr); + const auto mask = get_subnet_mask(prefix_length); for (size_t i = 0; i < ip.octets.size(); ++i) { ip.octets[i] &= mask.octets[i]; @@ -93,18 +91,15 @@ catch (const std::out_of_range& e) return ip; } -mp::Subnet generate_random_subnet(uint8_t cidr, mp::Subnet range) +mp::Subnet generate_random_subnet(mp::Subnet::PrefixLength prefix_length, mp::Subnet range) { - if (cidr >= 31) - throw std::invalid_argument(fmt::format(large_CIDR_err_fmt, cidr)); - - if (cidr < range.CIDR()) + if (prefix_length < range.prefix_length()) throw std::logic_error(fmt::format("A subnet with cidr {} cannot be contained by {}", - cidr, + uint8_t(prefix_length), range.as_string())); // ex. 2^(24 - 16) = 256, [192.168.0.0/24, 192.168.255.0/24] - const size_t possibleSubnets = std::size_t{1} << (cidr - range.CIDR()); + const size_t possibleSubnets = std::size_t{1} << (prefix_length - range.prefix_length()); // narrowing conversion, possibleSubnets is guaranteed to be < 2^31 (4 bytes is safe) static_assert(sizeof(decltype(MP_UTILS.random_int(0, possibleSubnets))) >= 4); @@ -112,18 +107,15 @@ mp::Subnet generate_random_subnet(uint8_t cidr, mp::Subnet range) const auto subnet = static_cast(MP_UTILS.random_int(0, possibleSubnets - 1)); // ex. 192.168.0.0 + (4 * 2^(32 - 24)) = 192.168.0.0 + 1024 = 192.168.4.0 - mp::IPAddress id = range.identifier() + (subnet * (std::size_t{1} << (32 - cidr))); + mp::IPAddress id = range.identifier() + (subnet * (std::size_t{1} << (32 - prefix_length))); - return mp::Subnet{id, cidr}; + return mp::Subnet{id, prefix_length}; } } // namespace -mp::Subnet::Subnet(IPAddress ip, uint8_t cidr) : id(apply_mask(ip, cidr)), cidr(cidr) +mp::Subnet::Subnet(IPAddress ip, PrefixLength prefix_length) + : id(apply_mask(ip, prefix_length)), prefix(prefix_length) { - if (cidr >= 31) - { - throw std::invalid_argument(fmt::format(large_CIDR_err_fmt, cidr)); - } } mp::Subnet::Subnet(const std::string& cidr_string) : Subnet(parse(cidr_string)) @@ -138,7 +130,7 @@ mp::IPAddress mp::Subnet::min_address() const mp::IPAddress mp::Subnet::max_address() const { // identifier + 2^(32 - cidr) - 1 - 1 - return id + ((1ull << (32ull - cidr)) - 2ull); + return id + ((1ull << (32ull - prefix)) - 2ull); } uint32_t mp::Subnet::address_count() const @@ -151,26 +143,26 @@ mp::IPAddress mp::Subnet::identifier() const return id; } -uint8_t mp::Subnet::CIDR() const +mp::Subnet::PrefixLength mp::Subnet::prefix_length() const { - return cidr; + return prefix; } mp::IPAddress mp::Subnet::subnet_mask() const { - return ::get_subnet_mask(cidr); + return ::get_subnet_mask(prefix); } // uses CIDR notation std::string mp::Subnet::as_string() const { - return fmt::format("{}/{}", id.as_string(), cidr); + return fmt::format("{}/{}", id.as_string(), uint8_t(prefix)); } bool mp::Subnet::contains(Subnet other) const { // can't possibly contain a larger subnet - if (other.CIDR() < CIDR()) + if (other.prefix_length() < prefix) return false; return contains(other.identifier()); @@ -184,7 +176,7 @@ bool mp::Subnet::contains(IPAddress ip) const bool mp::Subnet::operator==(const Subnet& other) const { - return id == other.id && cidr == other.cidr; + return id == other.id && prefix == other.prefix; } /* TODO C++20 uncomment @@ -197,12 +189,12 @@ std::strong_ordering mp::Subnet::operator<=>(const Subnet& other) const } */ -mp::Subnet mp::SubnetUtils::generate_random_subnet(uint8_t cidr, Subnet range) const +mp::Subnet mp::SubnetUtils::generate_random_subnet(Subnet::PrefixLength prefix, Subnet range) const { // @TODO don't rely on pure randomness for (auto i = 0; i < 100; ++i) { - const auto subnet = ::generate_random_subnet(cidr, range); + const auto subnet = ::generate_random_subnet(prefix, range); if (MP_PLATFORM.subnet_used_locally(subnet)) continue; diff --git a/tests/mock_subnet_utils.h b/tests/mock_subnet_utils.h index 173aa33caf..c2b249b583 100644 --- a/tests/mock_subnet_utils.h +++ b/tests/mock_subnet_utils.h @@ -27,7 +27,10 @@ struct MockSubnetUtils : public SubnetUtils { using SubnetUtils::SubnetUtils; - MOCK_METHOD(Subnet, generate_random_subnet, (uint8_t cidr, Subnet range), (const, override)); + MOCK_METHOD(Subnet, + generate_random_subnet, + (Subnet::PrefixLength prefix, Subnet range), + (const, override)); MP_MOCK_SINGLETON_BOILERPLATE(MockSubnetUtils, SubnetUtils); }; diff --git a/tests/test_subnet.cpp b/tests/test_subnet.cpp index f2f65c5a43..afa3a7692c 100644 --- a/tests/test_subnet.cpp +++ b/tests/test_subnet.cpp @@ -31,7 +31,7 @@ TEST(Subnet, canInitializeFromIpCidrPair) mp::Subnet subnet{mp::IPAddress("192.168.0.0"), 24}; EXPECT_EQ(subnet.identifier(), mp::IPAddress("192.168.0.0")); - EXPECT_EQ(subnet.CIDR(), 24); + EXPECT_EQ(subnet.prefix_length(), 24); } TEST(Subnet, canInitializeFromString) @@ -39,7 +39,7 @@ TEST(Subnet, canInitializeFromString) mp::Subnet subnet{"192.168.0.0/24"}; EXPECT_EQ(subnet.identifier(), mp::IPAddress("192.168.0.0")); - EXPECT_EQ(subnet.CIDR(), 24); + EXPECT_EQ(subnet.prefix_length(), 24); } TEST(Subet, throwsOnInvalidIP) @@ -56,52 +56,41 @@ TEST(Subet, throwsOnInvalidIP) mpt::match_what(HasSubstr("invalid IP octet"))); } -TEST(Subnet, throwsOnLargeCIDR) +TEST(Subnet, throwsOnLargePrefixLength) { - static constexpr auto what = "CIDR value must be non-negative and less than 31"; - // valid but not supported MP_EXPECT_THROW_THAT(mp::Subnet{"192.168.0.0/31"}, - std::invalid_argument, - mpt::match_what(HasSubstr(what))); + mp::Subnet::PrefixLengthOutOfRange, + mpt::match_what(HasSubstr("31"))); MP_EXPECT_THROW_THAT((mp::Subnet{mp::IPAddress{"192.168.0.0"}, 31}), - std::invalid_argument, - mpt::match_what(HasSubstr(what))); + mp::Subnet::PrefixLengthOutOfRange, + mpt::match_what(HasSubstr("31"))); // valid but not supported MP_EXPECT_THROW_THAT(mp::Subnet{"192.168.0.0/32"}, - std::invalid_argument, - mpt::match_what(HasSubstr(what))); + mp::Subnet::PrefixLengthOutOfRange, + mpt::match_what(HasSubstr("32"))); // boundary case MP_EXPECT_THROW_THAT(mp::Subnet{"192.168.0.0/33"}, - std::invalid_argument, - mpt::match_what(HasSubstr(what))); + mp::Subnet::PrefixLengthOutOfRange, + mpt::match_what(HasSubstr("33"))); // at 8 bit limit - MP_EXPECT_THROW_THAT(mp::Subnet{"192.168.0.0/255"}, - std::invalid_argument, - mpt::match_what(HasSubstr(what))); + EXPECT_THROW(mp::Subnet{"192.168.0.0/255"}, mp::Subnet::PrefixLengthOutOfRange); // above 8 bit limit - MP_EXPECT_THROW_THAT(mp::Subnet{"192.168.0.0/895231337"}, - std::invalid_argument, - mpt::match_what(HasSubstr(what))); + EXPECT_THROW(mp::Subnet{"192.168.0.0/895231337"}, mp::Subnet::PrefixLengthOutOfRange); // extreme case - MP_EXPECT_THROW_THAT(mp::Subnet{"192.168.0.0/895231337890712387952378952359871235987169601436"}, - std::invalid_argument, - mpt::match_what(HasSubstr(what))); + EXPECT_THROW(mp::Subnet{"192.168.0.0/895231337890712387952378952359871235987169601436"}, + mp::Subnet::PrefixLengthOutOfRange); } -TEST(Subnet, throwsOnNegativeCIDR) +TEST(Subnet, throwsOnNegativePrefixLength) { - static constexpr auto what = "CIDR value must be non-negative and less than 31"; - - MP_EXPECT_THROW_THAT(mp::Subnet{"192.168.0.0/-24"}, - std::invalid_argument, - mpt::match_what(HasSubstr(what))); + EXPECT_THROW(mp::Subnet{"192.168.0.0/-24"}, mp::Subnet::PrefixLengthOutOfRange); } TEST(Subnet, givesCorrectRange) @@ -321,7 +310,7 @@ TEST_F(SubnetUtils, generateRandomSubnetTriviallyWorks) mp::Subnet subnet = MP_SUBNET_UTILS.generate_random_subnet(24, range); EXPECT_EQ(subnet.identifier(), range.identifier()); - EXPECT_EQ(subnet.CIDR(), 24); + EXPECT_EQ(subnet.prefix_length(), 24); } TEST_F(SubnetUtils, generateRandomSubnetFailsOnSmallRange) @@ -332,18 +321,18 @@ TEST_F(SubnetUtils, generateRandomSubnetFailsOnSmallRange) EXPECT_THROW(std::ignore = MP_SUBNET_UTILS.generate_random_subnet(0, range), std::logic_error); } -TEST_F(SubnetUtils, generateRandomSubnetFailsOnBadCIDR) +TEST_F(SubnetUtils, generateRandomSubnetFailsOnBadPrefixLength) { mp::Subnet range{"0.0.0.0/0"}; EXPECT_THROW(std::ignore = MP_SUBNET_UTILS.generate_random_subnet(31, range), - std::invalid_argument); + mp::Subnet::PrefixLengthOutOfRange); EXPECT_THROW(std::ignore = MP_SUBNET_UTILS.generate_random_subnet(32, range), - std::invalid_argument); + mp::Subnet::PrefixLengthOutOfRange); EXPECT_THROW(std::ignore = MP_SUBNET_UTILS.generate_random_subnet(33, range), - std::invalid_argument); + mp::Subnet::PrefixLengthOutOfRange); EXPECT_THROW(std::ignore = MP_SUBNET_UTILS.generate_random_subnet(255, range), - std::invalid_argument); + mp::Subnet::PrefixLengthOutOfRange); } TEST_F(SubnetUtils, generateRandomSubnetRespectsRange) @@ -358,10 +347,10 @@ TEST_F(SubnetUtils, generateRandomSubnetRespectsRange) auto subnetHigh = MP_SUBNET_UTILS.generate_random_subnet(24, range); EXPECT_EQ(subnetLow.identifier(), mp::IPAddress{"192.168.0.0"}); - EXPECT_EQ(subnetLow.CIDR(), 24); + EXPECT_EQ(subnetLow.prefix_length(), 24); EXPECT_EQ(subnetHigh.identifier(), mp::IPAddress{"192.168.255.0"}); - EXPECT_EQ(subnetHigh.CIDR(), 24); + EXPECT_EQ(subnetHigh.prefix_length(), 24); } TEST_F(SubnetUtils, generateRandomSubnetWorksAtUpperExtreme) @@ -374,10 +363,10 @@ TEST_F(SubnetUtils, generateRandomSubnetWorksAtUpperExtreme) auto subnetHigh = MP_SUBNET_UTILS.generate_random_subnet(30, range); EXPECT_EQ(subnetLow.identifier(), mp::IPAddress{"0.0.0.0"}); - EXPECT_EQ(subnetLow.CIDR(), 30); + EXPECT_EQ(subnetLow.prefix_length(), 30); EXPECT_EQ(subnetHigh.identifier(), mp::IPAddress{"255.255.255.252"}); - EXPECT_EQ(subnetHigh.CIDR(), 30); + EXPECT_EQ(subnetHigh.prefix_length(), 30); } TEST_F(SubnetUtils, generateRandomSubnetGivesUpUsedLocally) From 604ac65a66e9a58bd59a1c596b5757263b610e93 Mon Sep 17 00:00:00 2001 From: trevor-shoe Date: Wed, 1 Oct 2025 14:46:43 -0400 Subject: [PATCH 21/28] [network] Add format specialization for subnet --- include/multipass/subnet.h | 38 ++++++++++++++++++- src/network/subnet.cpp | 21 ++++++---- .../backends/qemu/linux/firewall_config.cpp | 4 +- .../qemu/linux/qemu_platform_detail_linux.cpp | 2 +- .../shared/base_availability_zone.cpp | 2 +- src/platform/platform_linux.cpp | 4 +- tests/linux/test_platform_linux.cpp | 2 +- tests/qemu/linux/test_firewall_config.cpp | 8 ++-- .../qemu/linux/test_qemu_platform_detail.cpp | 2 +- tests/test_base_availability_zone.cpp | 2 +- tests/test_subnet.cpp | 6 +-- 11 files changed, 65 insertions(+), 26 deletions(-) diff --git a/include/multipass/subnet.h b/include/multipass/subnet.h index 0eb47e818d..71643a8de1 100644 --- a/include/multipass/subnet.h +++ b/include/multipass/subnet.h @@ -71,8 +71,7 @@ class Subnet [[nodiscard]] PrefixLength prefix_length() const; [[nodiscard]] IPAddress subnet_mask() const; - // uses CIDR notation - [[nodiscard]] std::string as_string() const; + [[nodiscard]] std::string to_cidr() const; // Subnets are either disjoint or the smaller is a subset of the larger [[nodiscard]] bool contains(Subnet other) const; @@ -96,3 +95,38 @@ struct SubnetUtils : Singleton Subnet range = Subnet{"10.0.0.0/8"}) const; }; } // namespace multipass + +namespace fmt +{ +template <> +struct formatter +{ + template + constexpr auto parse(ParseContext& ctx) + { + return ctx.begin(); + } + + template + auto format(const multipass::Subnet& subnet, FormatContext& ctx) const + { + return format_to(ctx.out(), "{}", subnet.to_cidr()); + } +}; + +template <> +struct formatter +{ + template + constexpr auto parse(ParseContext& ctx) + { + return ctx.begin(); + } + + template + auto format(const multipass::Subnet::PrefixLength& prefix, FormatContext& ctx) const + { + return format_to(ctx.out(), "{}", uint8_t(prefix)); + } +}; +} // namespace fmt diff --git a/src/network/subnet.cpp b/src/network/subnet.cpp index 53c6610157..ef38a43c3e 100644 --- a/src/network/subnet.cpp +++ b/src/network/subnet.cpp @@ -46,6 +46,10 @@ try throw std::invalid_argument( fmt::format("CIDR {:?} does not contain '/' seperator", cidr_string)); } +catch (const mp::Subnet::PrefixLengthOutOfRange&) +{ + throw; +} catch (const std::out_of_range& e) { throw mp::Subnet::PrefixLengthOutOfRange(e.what()); @@ -94,9 +98,10 @@ catch (const std::out_of_range& e) mp::Subnet generate_random_subnet(mp::Subnet::PrefixLength prefix_length, mp::Subnet range) { if (prefix_length < range.prefix_length()) - throw std::logic_error(fmt::format("A subnet with cidr {} cannot be contained by {}", - uint8_t(prefix_length), - range.as_string())); + throw std::logic_error( + fmt::format("A subnet with prefix length {} cannot be contained by {}", + prefix_length, + range)); // ex. 2^(24 - 16) = 256, [192.168.0.0/24, 192.168.255.0/24] const size_t possibleSubnets = std::size_t{1} << (prefix_length - range.prefix_length()); @@ -129,7 +134,7 @@ mp::IPAddress mp::Subnet::min_address() const mp::IPAddress mp::Subnet::max_address() const { - // identifier + 2^(32 - cidr) - 1 - 1 + // identifier + 2^(32 - prefix) - 1 - 1 return id + ((1ull << (32ull - prefix)) - 2ull); } @@ -154,9 +159,9 @@ mp::IPAddress mp::Subnet::subnet_mask() const } // uses CIDR notation -std::string mp::Subnet::as_string() const +std::string mp::Subnet::to_cidr() const { - return fmt::format("{}/{}", id.as_string(), uint8_t(prefix)); + return fmt::format("{}/{}", id.as_string(), prefix); } bool mp::Subnet::contains(Subnet other) const @@ -184,8 +189,8 @@ std::strong_ordering mp::Subnet::operator<=>(const Subnet& other) const { const auto ip_res = id <=> other.id; - // note the cidr operands are purposely flipped - return (ip_res == 0) ? other.cidr <=> cidr : ip_res; + // note the prefix_length operands are purposely flipped + return (ip_res == 0) ? other.prefix_length <=> prefix_length : ip_res; } */ diff --git a/src/platform/backends/qemu/linux/firewall_config.cpp b/src/platform/backends/qemu/linux/firewall_config.cpp index d90b225ec1..2acad1a945 100644 --- a/src/platform/backends/qemu/linux/firewall_config.cpp +++ b/src/platform/backends/qemu/linux/firewall_config.cpp @@ -178,7 +178,7 @@ void set_firewall_rules(const QString& firewall, const mp::Subnet& cidr, const QString& comment) { - const QString cidr_str = QString::fromStdString(cidr.as_string()); + const QString cidr_str = QString::fromStdString(cidr.to_cidr()); const QStringList comment_option{match, QStringLiteral("comment"), @@ -310,7 +310,7 @@ void clear_firewall_rules_for(const QString& firewall, const mp::Subnet& cidr, const QString& comment) { - const QString cidr_str = QString::fromStdString(cidr.as_string()); + const QString cidr_str = QString::fromStdString(cidr.to_cidr()); auto rules = QString::fromUtf8(get_firewall_rules(firewall, table)); for (auto& rule : rules.split('\n')) diff --git a/src/platform/backends/qemu/linux/qemu_platform_detail_linux.cpp b/src/platform/backends/qemu/linux/qemu_platform_detail_linux.cpp index f1efd068ab..905309b293 100644 --- a/src/platform/backends/qemu/linux/qemu_platform_detail_linux.cpp +++ b/src/platform/backends/qemu/linux/qemu_platform_detail_linux.cpp @@ -71,7 +71,7 @@ void create_virtual_switch(const mp::Subnet& subnet, const QString& bridge_name) if (!MP_UTILS.run_cmd_for_status("ip", {"addr", "show", bridge_name})) { const auto mac_address = mp::utils::generate_mac_address(); - const auto cidr = subnet.as_string(); + const auto cidr = subnet.to_cidr(); const auto broadcast = (subnet.max_address() + 1).as_string(); MP_UTILS.run_cmd_for_status( diff --git a/src/platform/backends/shared/base_availability_zone.cpp b/src/platform/backends/shared/base_availability_zone.cpp index c52f93575b..c921b79131 100644 --- a/src/platform/backends/shared/base_availability_zone.cpp +++ b/src/platform/backends/shared/base_availability_zone.cpp @@ -144,7 +144,7 @@ void BaseAvailabilityZone::serialize() const const std::unique_lock lock{m.mutex}; const QJsonObject json{ - {subnet_key, QString::fromStdString(m.subnet.as_string())}, + {subnet_key, QString::fromStdString(m.subnet.to_cidr())}, {available_key, m.available}, }; diff --git a/src/platform/platform_linux.cpp b/src/platform/platform_linux.cpp index 86afb289cd..f4d60f59d1 100644 --- a/src/platform/platform_linux.cpp +++ b/src/platform/platform_linux.cpp @@ -385,9 +385,9 @@ bool mp::platform::Platform::subnet_used_locally(mp::Subnet subnet) const return true; } } - catch (const std::invalid_argument& e) + catch (const std::exception& e) { - mpl::warn(category, "invalid subnet from ip command: {}", e.what()) + mpl::warn(category, "invalid subnet from ip command: {}", e.what()); } } return false; diff --git a/tests/linux/test_platform_linux.cpp b/tests/linux/test_platform_linux.cpp index e09333f5de..12e913b04a 100644 --- a/tests/linux/test_platform_linux.cpp +++ b/tests/linux/test_platform_linux.cpp @@ -752,8 +752,8 @@ TEST_F(PlatformLinux, subnetUsedLocallyHandlesBadSubnets) default via 192.168.0.0 dev wlo1 proto dhcp src 192.168.0.1 metric 600 (incomplete) dev lxdbr0 proto kernel scope link src (unknown) 10.192.168.0/33 dev mpqemubr0 proto kernel scope link src 10.192.168.1 +172.256.0.0/16 dev docker0 proto kernel scope link src 172.256.0.1 10.255.19.0/24 dev mpbr0 proto kernel scope link src 10.255.19.1 -172.256.0.0/16 dev docker0 proto kernel scope link src 172.256.0.1 linkdown 192.168.0.256/24 dev wlo1 proto kernel scope link src 192.168.0.1 metric 600 256.168.123.0/24 dev virbr0 proto kernel scope link src 256.168.123.1 linkdown )")); diff --git a/tests/qemu/linux/test_firewall_config.cpp b/tests/qemu/linux/test_firewall_config.cpp index 14f8214bb4..7fe5b34f00 100644 --- a/tests/qemu/linux/test_firewall_config.cpp +++ b/tests/qemu/linux/test_firewall_config.cpp @@ -132,8 +132,8 @@ TEST_F(FirewallConfig, dtorDeletesKnownRules) const QByteArray base_rule{ fmt::format("POSTROUTING -s {} ! -d {} -m comment --comment \"generated for " "Multipass network {}\" -j MASQUERADE", - subnet.as_string(), - subnet.as_string(), + subnet, + subnet, goodbr0) .data()}; const QByteArray full_rule{"-A " + base_rule}; @@ -167,8 +167,8 @@ TEST_F(FirewallConfig, dtorDeleteErrorLogsErrorAndContinues) const QByteArray base_rule{ fmt::format("POSTROUTING -s {} ! -d {} -m comment --comment \"generated for " "Multipass network {}\" -j MASQUERADE", - subnet.as_string(), - subnet.as_string(), + subnet, + subnet, goodbr0) .data()}; const QByteArray full_rule{"-A " + base_rule}; diff --git a/tests/qemu/linux/test_qemu_platform_detail.cpp b/tests/qemu/linux/test_qemu_platform_detail.cpp index 2d73574f95..1e65f5c58d 100644 --- a/tests/qemu/linux/test_qemu_platform_detail.cpp +++ b/tests/qemu/linux/test_qemu_platform_detail.cpp @@ -156,7 +156,7 @@ TEST_F(QemuPlatformDetail, ctorSetsUpExpectedVirtualSwitches) { for (const auto& vswitch : switches) { - const auto subnet{vswitch.subnet.as_string()}; + const auto subnet{vswitch.subnet.to_cidr()}; const auto broadcast = (vswitch.subnet.max_address() + 1).as_string(); EXPECT_CALL(*mock_utils, diff --git a/tests/test_base_availability_zone.cpp b/tests/test_base_availability_zone.cpp index 64caa30c7b..73b836beee 100644 --- a/tests/test_base_availability_zone.cpp +++ b/tests/test_base_availability_zone.cpp @@ -75,7 +75,7 @@ TEST_F(BaseAvailabilityZoneTest, loads_existing_zone_file) const mp::Subnet test_subnet{"10.0.0.0/24"}; const bool test_available = false; - QJsonObject json{{"subnet", QString::fromStdString(test_subnet.as_string())}, + QJsonObject json{{"subnet", QString::fromStdString(test_subnet.to_cidr())}, {"available", test_available}}; EXPECT_CALL(*mock_json_utils_guard.first, read_object_from_file(az_file)) diff --git a/tests/test_subnet.cpp b/tests/test_subnet.cpp index afa3a7692c..c3131f8996 100644 --- a/tests/test_subnet.cpp +++ b/tests/test_subnet.cpp @@ -153,13 +153,13 @@ TEST(Subnet, getSubnetMaskReturnsSubnetMask) TEST(Subnet, canConvertToString) { mp::Subnet subnet{"192.168.0.1/24"}; - EXPECT_EQ(subnet.as_string(), "192.168.0.0/24"); + EXPECT_EQ(subnet.to_cidr(), "192.168.0.0/24"); subnet = mp::Subnet{"255.0.255.0/8"}; - EXPECT_EQ(subnet.as_string(), "255.0.0.0/8"); + EXPECT_EQ(subnet.to_cidr(), "255.0.0.0/8"); subnet = mp::Subnet{"255.0.255.0/0"}; - EXPECT_EQ(subnet.as_string(), "0.0.0.0/0"); + EXPECT_EQ(subnet.to_cidr(), "0.0.0.0/0"); } TEST(Subnet, containsWorksOnContainedSubnets) From 6d06c055fd71ae45ba116e4d92fdf73eb9ed5cb3 Mon Sep 17 00:00:00 2001 From: trevor-shoe Date: Thu, 2 Oct 2025 09:16:30 -0400 Subject: [PATCH 22/28] [network] Rename subnet network address --- include/multipass/subnet.h | 6 +++--- src/network/subnet.cpp | 25 +++++++++++++------------ tests/test_subnet.cpp | 36 ++++++++++++++++++------------------ 3 files changed, 34 insertions(+), 33 deletions(-) diff --git a/include/multipass/subnet.h b/include/multipass/subnet.h index 71643a8de1..16139559e6 100644 --- a/include/multipass/subnet.h +++ b/include/multipass/subnet.h @@ -65,9 +65,9 @@ class Subnet [[nodiscard]] IPAddress min_address() const; [[nodiscard]] IPAddress max_address() const; - [[nodiscard]] uint32_t address_count() const; + [[nodiscard]] uint32_t usable_address_count() const; - [[nodiscard]] IPAddress identifier() const; + [[nodiscard]] IPAddress network_address() const; [[nodiscard]] PrefixLength prefix_length() const; [[nodiscard]] IPAddress subnet_mask() const; @@ -83,7 +83,7 @@ class Subnet [[nodiscard]] bool operator==(const Subnet& other) const; private: - IPAddress id; + IPAddress address; PrefixLength prefix; }; diff --git a/src/network/subnet.cpp b/src/network/subnet.cpp index ef38a43c3e..916718b777 100644 --- a/src/network/subnet.cpp +++ b/src/network/subnet.cpp @@ -112,14 +112,15 @@ mp::Subnet generate_random_subnet(mp::Subnet::PrefixLength prefix_length, mp::Su const auto subnet = static_cast(MP_UTILS.random_int(0, possibleSubnets - 1)); // ex. 192.168.0.0 + (4 * 2^(32 - 24)) = 192.168.0.0 + 1024 = 192.168.4.0 - mp::IPAddress id = range.identifier() + (subnet * (std::size_t{1} << (32 - prefix_length))); + mp::IPAddress id = + range.network_address() + (subnet * (std::size_t{1} << (32 - prefix_length))); return mp::Subnet{id, prefix_length}; } } // namespace mp::Subnet::Subnet(IPAddress ip, PrefixLength prefix_length) - : id(apply_mask(ip, prefix_length)), prefix(prefix_length) + : address(apply_mask(ip, prefix_length)), prefix(prefix_length) { } @@ -129,23 +130,23 @@ mp::Subnet::Subnet(const std::string& cidr_string) : Subnet(parse(cidr_string)) mp::IPAddress mp::Subnet::min_address() const { - return id + 1; + return address + 1; } mp::IPAddress mp::Subnet::max_address() const { // identifier + 2^(32 - prefix) - 1 - 1 - return id + ((1ull << (32ull - prefix)) - 2ull); + return address + ((1ull << (32ull - prefix)) - 2ull); } -uint32_t mp::Subnet::address_count() const +uint32_t mp::Subnet::usable_address_count() const { return max_address().as_uint32() - min_address().as_uint32() + 1; } -mp::IPAddress mp::Subnet::identifier() const +mp::IPAddress mp::Subnet::network_address() const { - return id; + return address; } mp::Subnet::PrefixLength mp::Subnet::prefix_length() const @@ -161,7 +162,7 @@ mp::IPAddress mp::Subnet::subnet_mask() const // uses CIDR notation std::string mp::Subnet::to_cidr() const { - return fmt::format("{}/{}", id.as_string(), prefix); + return fmt::format("{}/{}", address.as_string(), prefix); } bool mp::Subnet::contains(Subnet other) const @@ -170,24 +171,24 @@ bool mp::Subnet::contains(Subnet other) const if (other.prefix_length() < prefix) return false; - return contains(other.identifier()); + return contains(other.network_address()); } bool mp::Subnet::contains(IPAddress ip) const { // since get_max_address doesn't include the broadcast address add 1 to it. - return identifier() <= ip && (max_address() + 1) >= ip; + return address <= ip && (max_address() + 1) >= ip; } bool mp::Subnet::operator==(const Subnet& other) const { - return id == other.id && prefix == other.prefix; + return address == other.address && prefix == other.prefix; } /* TODO C++20 uncomment std::strong_ordering mp::Subnet::operator<=>(const Subnet& other) const { - const auto ip_res = id <=> other.id; + const auto ip_res = address <=> other.address; // note the prefix_length operands are purposely flipped return (ip_res == 0) ? other.prefix_length <=> prefix_length : ip_res; diff --git a/tests/test_subnet.cpp b/tests/test_subnet.cpp index c3131f8996..1afd4fb243 100644 --- a/tests/test_subnet.cpp +++ b/tests/test_subnet.cpp @@ -30,7 +30,7 @@ TEST(Subnet, canInitializeFromIpCidrPair) { mp::Subnet subnet{mp::IPAddress("192.168.0.0"), 24}; - EXPECT_EQ(subnet.identifier(), mp::IPAddress("192.168.0.0")); + EXPECT_EQ(subnet.network_address(), mp::IPAddress("192.168.0.0")); EXPECT_EQ(subnet.prefix_length(), 24); } @@ -38,7 +38,7 @@ TEST(Subnet, canInitializeFromString) { mp::Subnet subnet{"192.168.0.0/24"}; - EXPECT_EQ(subnet.identifier(), mp::IPAddress("192.168.0.0")); + EXPECT_EQ(subnet.network_address(), mp::IPAddress("192.168.0.0")); EXPECT_EQ(subnet.prefix_length(), 24); } @@ -96,37 +96,37 @@ TEST(Subnet, throwsOnNegativePrefixLength) TEST(Subnet, givesCorrectRange) { mp::Subnet subnet{"192.168.0.0/24"}; - EXPECT_EQ(subnet.identifier(), mp::IPAddress{"192.168.0.0"}); + EXPECT_EQ(subnet.network_address(), mp::IPAddress{"192.168.0.0"}); EXPECT_EQ(subnet.min_address(), mp::IPAddress{"192.168.0.1"}); EXPECT_EQ(subnet.max_address(), mp::IPAddress{"192.168.0.254"}); - EXPECT_EQ(subnet.address_count(), 254); + EXPECT_EQ(subnet.usable_address_count(), 254); subnet = mp::Subnet{"121.212.1.152/11"}; - EXPECT_EQ(subnet.identifier(), mp::IPAddress{"121.192.0.0"}); + EXPECT_EQ(subnet.network_address(), mp::IPAddress{"121.192.0.0"}); EXPECT_EQ(subnet.min_address(), mp::IPAddress{"121.192.0.1"}); EXPECT_EQ(subnet.max_address(), mp::IPAddress{"121,223.255.254"}); - EXPECT_EQ(subnet.address_count(), 2097150); + EXPECT_EQ(subnet.usable_address_count(), 2097150); subnet = mp::Subnet{"0.0.0.0/0"}; - EXPECT_EQ(subnet.identifier(), mp::IPAddress{"0.0.0.0"}); + EXPECT_EQ(subnet.network_address(), mp::IPAddress{"0.0.0.0"}); EXPECT_EQ(subnet.min_address(), mp::IPAddress{"0.0.0.1"}); EXPECT_EQ(subnet.max_address(), mp::IPAddress{"255,255.255.254"}); - EXPECT_EQ(subnet.address_count(), 4294967294); + EXPECT_EQ(subnet.usable_address_count(), 4294967294); } TEST(Subnet, convertsToMaskedIP) { mp::Subnet subnet{"192.168.255.52/24"}; - EXPECT_EQ(subnet.identifier(), mp::IPAddress{"192.168.255.0"}); + EXPECT_EQ(subnet.network_address(), mp::IPAddress{"192.168.255.0"}); subnet = mp::Subnet{"255.168.1.152/8"}; - EXPECT_EQ(subnet.identifier(), mp::IPAddress{"255.0.0.0"}); + EXPECT_EQ(subnet.network_address(), mp::IPAddress{"255.0.0.0"}); subnet = mp::Subnet{"192.168.1.152/0"}; - EXPECT_EQ(subnet.identifier(), mp::IPAddress{"0.0.0.0"}); + EXPECT_EQ(subnet.network_address(), mp::IPAddress{"0.0.0.0"}); subnet = mp::Subnet{"255.212.1.152/13"}; - EXPECT_EQ(subnet.identifier(), mp::IPAddress{"255.208.0.0"}); + EXPECT_EQ(subnet.network_address(), mp::IPAddress{"255.208.0.0"}); } TEST(Subnet, getSubnetMaskReturnsSubnetMask) @@ -260,7 +260,7 @@ TEST(Subnet, relationalComparisonsWorkAsExpected) { const mp::Subnet low{"0.0.0.0/0"}; const mp::Subnet middle{"192.168.0.0/16"}; - const mp::Subnet submiddle{middle.identifier(), 24}; + const mp::Subnet submiddle{middle.network_address(), 24}; const mp::Subnet high{"255.255.255.0/24"}; EXPECT_LT(low, middle); @@ -309,7 +309,7 @@ TEST_F(SubnetUtils, generateRandomSubnetTriviallyWorks) mp::Subnet subnet = MP_SUBNET_UTILS.generate_random_subnet(24, range); - EXPECT_EQ(subnet.identifier(), range.identifier()); + EXPECT_EQ(subnet.network_address(), range.network_address()); EXPECT_EQ(subnet.prefix_length(), 24); } @@ -346,10 +346,10 @@ TEST_F(SubnetUtils, generateRandomSubnetRespectsRange) auto subnetLow = MP_SUBNET_UTILS.generate_random_subnet(24, range); auto subnetHigh = MP_SUBNET_UTILS.generate_random_subnet(24, range); - EXPECT_EQ(subnetLow.identifier(), mp::IPAddress{"192.168.0.0"}); + EXPECT_EQ(subnetLow.network_address(), mp::IPAddress{"192.168.0.0"}); EXPECT_EQ(subnetLow.prefix_length(), 24); - EXPECT_EQ(subnetHigh.identifier(), mp::IPAddress{"192.168.255.0"}); + EXPECT_EQ(subnetHigh.network_address(), mp::IPAddress{"192.168.255.0"}); EXPECT_EQ(subnetHigh.prefix_length(), 24); } @@ -362,10 +362,10 @@ TEST_F(SubnetUtils, generateRandomSubnetWorksAtUpperExtreme) auto subnetLow = MP_SUBNET_UTILS.generate_random_subnet(30, range); auto subnetHigh = MP_SUBNET_UTILS.generate_random_subnet(30, range); - EXPECT_EQ(subnetLow.identifier(), mp::IPAddress{"0.0.0.0"}); + EXPECT_EQ(subnetLow.network_address(), mp::IPAddress{"0.0.0.0"}); EXPECT_EQ(subnetLow.prefix_length(), 30); - EXPECT_EQ(subnetHigh.identifier(), mp::IPAddress{"255.255.255.252"}); + EXPECT_EQ(subnetHigh.network_address(), mp::IPAddress{"255.255.255.252"}); EXPECT_EQ(subnetHigh.prefix_length(), 30); } From b933e4b7aef8f45e95f6f0307fb2a4904256ca74 Mon Sep 17 00:00:00 2001 From: trevor-shoe Date: Thu, 2 Oct 2025 09:22:02 -0400 Subject: [PATCH 23/28] [network] Simplify get_subnet_mask MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Mustafa Kemal Gılor --- src/network/subnet.cpp | 28 ++-------------------------- 1 file changed, 2 insertions(+), 26 deletions(-) diff --git a/src/network/subnet.cpp b/src/network/subnet.cpp index 916718b777..53fd35c849 100644 --- a/src/network/subnet.cpp +++ b/src/network/subnet.cpp @@ -57,32 +57,8 @@ catch (const std::out_of_range& e) [[nodiscard]] mp::IPAddress get_subnet_mask(mp::Subnet::PrefixLength prefix_length) { - using Octects = decltype(mp::IPAddress::octets); - static constexpr auto value_size = sizeof(Octects::value_type) * 8; - - Octects octets{}; - std::fill(octets.begin(), octets.end(), std::numeric_limits::max()); - - if (prefix_length > value_size * octets.size()) - throw std::out_of_range("prefix length too large for address space"); - - const uint8_t start_octet = prefix_length / value_size; - const uint8_t remain = prefix_length % value_size; - - for (size_t i = start_octet; i < octets.size(); ++i) - { - octets[i] = 0; - } - - if (remain != 0) - { - assert(start_octet < octets.size()); // sanity - - // remain = 5, 1 << (8 - 5) = 00001000 -> 00000111 -> 11111000 - octets[start_octet] = ~((1u << (value_size - remain)) - 1u); - } - - return mp::IPAddress{octets}; + const uint32_t mask = (prefix_length == 0) ? 0 : ~uint32_t{0} << (32 - prefix_length); + return mp::IPAddress{mask}; } [[nodiscard]] mp::IPAddress apply_mask(mp::IPAddress ip, mp::Subnet::PrefixLength prefix_length) From 61333433ea94354689a513daca2e55a8764a45f8 Mon Sep 17 00:00:00 2001 From: trevor-shoe Date: Thu, 2 Oct 2025 09:47:49 -0400 Subject: [PATCH 24/28] [util] Use long long for random int --- include/multipass/utils.h | 2 +- src/utils/utils.cpp | 4 ++-- tests/mock_utils.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/multipass/utils.h b/include/multipass/utils.h index af9ac7a30e..7d2cfe5995 100644 --- a/include/multipass/utils.h +++ b/include/multipass/utils.h @@ -258,7 +258,7 @@ class Utils : public Singleton virtual Path normalize_mount_target(Path target_mount_path) const; virtual bool invalid_target_path(const Path& target_path) const; // needs normalized input path - virtual intmax_t random_int(intmax_t a, intmax_t b) const; + virtual long long random_int(long long a, long long b) const; }; } // namespace multipass diff --git a/src/utils/utils.cpp b/src/utils/utils.cpp index eca0e03292..1b99f60c30 100644 --- a/src/utils/utils.cpp +++ b/src/utils/utils.cpp @@ -623,7 +623,7 @@ bool mp::Utils::invalid_target_path(const QString& target_path) const return matcher.match(target_path).hasMatch(); } -intmax_t mp::Utils::random_int(intmax_t a, intmax_t b) const +long long mp::Utils::random_int(long long a, long long b) const { static std::default_random_engine gen = [] { // seed the rng with the time at first call @@ -634,7 +634,7 @@ intmax_t mp::Utils::random_int(intmax_t a, intmax_t b) const if (a > b) // avoid undefined behavior, this can only happen by programmer error throw std::logic_error(fmt::format("random range [{}, {}] is invalid", a, b)); - std::uniform_int_distribution dist{a, b}; + std::uniform_int_distribution dist{a, b}; return dist(gen); } diff --git a/tests/mock_utils.h b/tests/mock_utils.h index e5033ce23e..5423b7f257 100644 --- a/tests/mock_utils.h +++ b/tests/mock_utils.h @@ -64,7 +64,7 @@ class MockUtils : public Utils MOCK_METHOD(void, sleep_for, (const std::chrono::milliseconds&), (const, override)); MOCK_METHOD(bool, is_ipv4_valid, (const std::string& ipv4), (const, override)); MOCK_METHOD(Path, default_mount_target, (const Path& source), (const, override)); - MOCK_METHOD(intmax_t, random_int, (intmax_t a, intmax_t b), (const, override)); + MOCK_METHOD(long long, random_int, (long long a, long long b), (const, override)); MP_MOCK_SINGLETON_BOILERPLATE(MockUtils, Utils); }; From f4333975b78d9fb8345a2d711179e540995f8175 Mon Sep 17 00:00:00 2001 From: trevor-shoe Date: Thu, 2 Oct 2025 10:11:53 -0400 Subject: [PATCH 25/28] [network] Rename random_subnet_from_range --- include/multipass/subnet.h | 5 ++-- src/network/subnet.cpp | 22 +++++++++------ .../shared/base_availability_zone.cpp | 2 +- tests/mock_subnet_utils.h | 2 +- tests/test_base_availability_zone.cpp | 8 +++--- tests/test_base_availability_zone_manager.cpp | 8 +++--- tests/test_subnet.cpp | 28 ++++++++++--------- 7 files changed, 42 insertions(+), 33 deletions(-) diff --git a/include/multipass/subnet.h b/include/multipass/subnet.h index 16139559e6..41f97fa1a4 100644 --- a/include/multipass/subnet.h +++ b/include/multipass/subnet.h @@ -91,8 +91,9 @@ struct SubnetUtils : Singleton { using Singleton::Singleton; - [[nodiscard]] virtual Subnet generate_random_subnet(Subnet::PrefixLength prefix = 24, - Subnet range = Subnet{"10.0.0.0/8"}) const; + [[nodiscard]] virtual Subnet random_subnet_from_range(Subnet::PrefixLength prefix = 24, + Subnet range = Subnet{ + "10.0.0.0/8"}) const; }; } // namespace multipass diff --git a/src/network/subnet.cpp b/src/network/subnet.cpp index 53fd35c849..672e834771 100644 --- a/src/network/subnet.cpp +++ b/src/network/subnet.cpp @@ -71,7 +71,7 @@ catch (const std::out_of_range& e) return ip; } -mp::Subnet generate_random_subnet(mp::Subnet::PrefixLength prefix_length, mp::Subnet range) +mp::Subnet random_subnet_from_range(mp::Subnet::PrefixLength prefix_length, mp::Subnet range) { if (prefix_length < range.prefix_length()) throw std::logic_error( @@ -79,17 +79,20 @@ mp::Subnet generate_random_subnet(mp::Subnet::PrefixLength prefix_length, mp::Su prefix_length, range)); + // a range with prefix /16 has 65536 prefix /32 networks, + // a range with prefix /24 has 256 prefix /32 networks, + // so a prefix /16 network can hold 65536 / 256 = 256 prefix /24 networks. // ex. 2^(24 - 16) = 256, [192.168.0.0/24, 192.168.255.0/24] - const size_t possibleSubnets = std::size_t{1} << (prefix_length - range.prefix_length()); + const size_t possible_subnets = std::size_t{1} << (prefix_length - range.prefix_length()); // narrowing conversion, possibleSubnets is guaranteed to be < 2^31 (4 bytes is safe) - static_assert(sizeof(decltype(MP_UTILS.random_int(0, possibleSubnets))) >= 4); + static_assert(sizeof(decltype(MP_UTILS.random_int(0, possible_subnets))) >= 4); - const auto subnet = static_cast(MP_UTILS.random_int(0, possibleSubnets - 1)); + const auto subnet_block_idx = static_cast(MP_UTILS.random_int(0, possible_subnets - 1)); // ex. 192.168.0.0 + (4 * 2^(32 - 24)) = 192.168.0.0 + 1024 = 192.168.4.0 mp::IPAddress id = - range.network_address() + (subnet * (std::size_t{1} << (32 - prefix_length))); + range.network_address() + (subnet_block_idx * (std::size_t{1} << (32 - prefix_length))); return mp::Subnet{id, prefix_length}; } @@ -111,7 +114,9 @@ mp::IPAddress mp::Subnet::min_address() const mp::IPAddress mp::Subnet::max_address() const { - // identifier + 2^(32 - prefix) - 1 - 1 + // address + 2^(32 - prefix) - 1 - 1 + // address + 2^(32 - prefix) is the next subnet's network address for this prefix length + // subtracting 1 to stay in this subnet and another 1 to exclude the broadcast address return address + ((1ull << (32ull - prefix)) - 2ull); } @@ -171,12 +176,13 @@ std::strong_ordering mp::Subnet::operator<=>(const Subnet& other) const } */ -mp::Subnet mp::SubnetUtils::generate_random_subnet(Subnet::PrefixLength prefix, Subnet range) const +mp::Subnet mp::SubnetUtils::random_subnet_from_range(Subnet::PrefixLength prefix, + Subnet range) const { // @TODO don't rely on pure randomness for (auto i = 0; i < 100; ++i) { - const auto subnet = ::generate_random_subnet(prefix, range); + const auto subnet = ::random_subnet_from_range(prefix, range); if (MP_PLATFORM.subnet_used_locally(subnet)) continue; diff --git a/src/platform/backends/shared/base_availability_zone.cpp b/src/platform/backends/shared/base_availability_zone.cpp index c921b79131..e65287f7af 100644 --- a/src/platform/backends/shared/base_availability_zone.cpp +++ b/src/platform/backends/shared/base_availability_zone.cpp @@ -51,7 +51,7 @@ catch (const std::ios_base::failure& e) return json_subnet; mpl::debug(name, "subnet missing from AZ file '{}', using default", file_path); - return MP_SUBNET_UTILS.generate_random_subnet(); + return MP_SUBNET_UTILS.random_subnet_from_range(); }; [[nodiscard]] bool deserialize_available(const QJsonObject& json, diff --git a/tests/mock_subnet_utils.h b/tests/mock_subnet_utils.h index c2b249b583..a251146f28 100644 --- a/tests/mock_subnet_utils.h +++ b/tests/mock_subnet_utils.h @@ -28,7 +28,7 @@ struct MockSubnetUtils : public SubnetUtils using SubnetUtils::SubnetUtils; MOCK_METHOD(Subnet, - generate_random_subnet, + random_subnet_from_range, (Subnet::PrefixLength prefix, Subnet range), (const, override)); diff --git a/tests/test_base_availability_zone.cpp b/tests/test_base_availability_zone.cpp index 73b836beee..983366bd30 100644 --- a/tests/test_base_availability_zone.cpp +++ b/tests/test_base_availability_zone.cpp @@ -61,7 +61,7 @@ TEST_F(BaseAvailabilityZoneTest, CreatesDefaultAvailableZone) EXPECT_CALL(*mock_json_utils_guard.first, write_json(_, QString::fromStdU16String(az_file.u16string()))); - EXPECT_CALL(*mock_subnet_utils_guard.first, generate_random_subnet(_, _)) + EXPECT_CALL(*mock_subnet_utils_guard.first, random_subnet_from_range(_, _)) .WillOnce(Return(az_subnet)); mp::BaseAvailabilityZone zone{az_name, az_dir}; @@ -106,7 +106,7 @@ TEST_F(BaseAvailabilityZoneTest, AddsVmAndUpdatesOnAvailabilityChange) write_json(_, QString::fromStdU16String(az_file.u16string()))) .Times(2); // Once in constructor, once in set_available - EXPECT_CALL(*mock_subnet_utils_guard.first, generate_random_subnet(_, _)) + EXPECT_CALL(*mock_subnet_utils_guard.first, random_subnet_from_range(_, _)) .WillOnce(Return(az_subnet)); mp::BaseAvailabilityZone zone{az_name, az_dir}; @@ -131,7 +131,7 @@ TEST_F(BaseAvailabilityZoneTest, RemovesVmCorrectly) EXPECT_CALL(*mock_json_utils_guard.first, write_json(_, QString::fromStdU16String(az_file.u16string()))); - EXPECT_CALL(*mock_subnet_utils_guard.first, generate_random_subnet(_, _)) + EXPECT_CALL(*mock_subnet_utils_guard.first, random_subnet_from_range(_, _)) .WillOnce(Return(az_subnet)); mp::BaseAvailabilityZone zone{az_name, az_dir}; @@ -156,7 +156,7 @@ TEST_F(BaseAvailabilityZoneTest, AvailabilityStateManagement) write_json(_, QString::fromStdU16String(az_file.u16string()))) .Times(2); // Once in constructor, once in set_available - EXPECT_CALL(*mock_subnet_utils_guard.first, generate_random_subnet(_, _)) + EXPECT_CALL(*mock_subnet_utils_guard.first, random_subnet_from_range(_, _)) .WillOnce(Return(az_subnet)); mp::BaseAvailabilityZone zone{az_name, az_dir}; diff --git a/tests/test_base_availability_zone_manager.cpp b/tests/test_base_availability_zone_manager.cpp index af8b588b2d..3ad890c53d 100644 --- a/tests/test_base_availability_zone_manager.cpp +++ b/tests/test_base_availability_zone_manager.cpp @@ -69,7 +69,7 @@ TEST_F(BaseAvailabilityZoneManagerTest, CreatesDefaultZones) write_json(_, QString::fromStdU16String(zone_file.u16string()))); } - EXPECT_CALL(*mock_subnet_utils_guard.first, generate_random_subnet(_, _)) + EXPECT_CALL(*mock_subnet_utils_guard.first, random_subnet_from_range(_, _)) .Times(expected_zone_count) .WillRepeatedly(Return(mp::Subnet{"192.168.1.0/24"})); @@ -107,7 +107,7 @@ TEST_F(BaseAvailabilityZoneManagerTest, UsesZone1WhenAvailable) .Times(AnyNumber()); } - EXPECT_CALL(*mock_subnet_utils_guard.first, generate_random_subnet(_, _)) + EXPECT_CALL(*mock_subnet_utils_guard.first, random_subnet_from_range(_, _)) .Times(AnyNumber()) .WillRepeatedly(Return(mp::Subnet{"192.168.1.0/24"})); @@ -157,7 +157,7 @@ TEST_F(BaseAvailabilityZoneManagerTest, ThrowsWhenZoneNotFound) .Times(AnyNumber()); } - EXPECT_CALL(*mock_subnet_utils_guard.first, generate_random_subnet(_, _)) + EXPECT_CALL(*mock_subnet_utils_guard.first, random_subnet_from_range(_, _)) .Times(AnyNumber()) .WillRepeatedly(Return(mp::Subnet{"192.168.1.0/24"})); @@ -186,7 +186,7 @@ TEST_F(BaseAvailabilityZoneManagerTest, PrefersZone1ThenZone2ThenZone3) .Times(AnyNumber()); } - EXPECT_CALL(*mock_subnet_utils_guard.first, generate_random_subnet(_, _)) + EXPECT_CALL(*mock_subnet_utils_guard.first, random_subnet_from_range(_, _)) .Times(AnyNumber()) .WillRepeatedly(Return(mp::Subnet{"192.168.1.0/24"})); diff --git a/tests/test_subnet.cpp b/tests/test_subnet.cpp index 1afd4fb243..deb6d79617 100644 --- a/tests/test_subnet.cpp +++ b/tests/test_subnet.cpp @@ -307,7 +307,7 @@ TEST_F(SubnetUtils, generateRandomSubnetTriviallyWorks) return a; })); - mp::Subnet subnet = MP_SUBNET_UTILS.generate_random_subnet(24, range); + mp::Subnet subnet = MP_SUBNET_UTILS.random_subnet_from_range(24, range); EXPECT_EQ(subnet.network_address(), range.network_address()); EXPECT_EQ(subnet.prefix_length(), 24); @@ -317,21 +317,23 @@ TEST_F(SubnetUtils, generateRandomSubnetFailsOnSmallRange) { mp::Subnet range{"192.168.1.0/16"}; - EXPECT_THROW(std::ignore = MP_SUBNET_UTILS.generate_random_subnet(15, range), std::logic_error); - EXPECT_THROW(std::ignore = MP_SUBNET_UTILS.generate_random_subnet(0, range), std::logic_error); + EXPECT_THROW(std::ignore = MP_SUBNET_UTILS.random_subnet_from_range(15, range), + std::logic_error); + EXPECT_THROW(std::ignore = MP_SUBNET_UTILS.random_subnet_from_range(0, range), + std::logic_error); } TEST_F(SubnetUtils, generateRandomSubnetFailsOnBadPrefixLength) { mp::Subnet range{"0.0.0.0/0"}; - EXPECT_THROW(std::ignore = MP_SUBNET_UTILS.generate_random_subnet(31, range), + EXPECT_THROW(std::ignore = MP_SUBNET_UTILS.random_subnet_from_range(31, range), mp::Subnet::PrefixLengthOutOfRange); - EXPECT_THROW(std::ignore = MP_SUBNET_UTILS.generate_random_subnet(32, range), + EXPECT_THROW(std::ignore = MP_SUBNET_UTILS.random_subnet_from_range(32, range), mp::Subnet::PrefixLengthOutOfRange); - EXPECT_THROW(std::ignore = MP_SUBNET_UTILS.generate_random_subnet(33, range), + EXPECT_THROW(std::ignore = MP_SUBNET_UTILS.random_subnet_from_range(33, range), mp::Subnet::PrefixLengthOutOfRange); - EXPECT_THROW(std::ignore = MP_SUBNET_UTILS.generate_random_subnet(255, range), + EXPECT_THROW(std::ignore = MP_SUBNET_UTILS.random_subnet_from_range(255, range), mp::Subnet::PrefixLengthOutOfRange); } @@ -343,8 +345,8 @@ TEST_F(SubnetUtils, generateRandomSubnetRespectsRange) EXPECT_CALL(*mock_utils, random_int(_, _)).WillOnce(ReturnArg<0>()).WillOnce(ReturnArg<1>()); - auto subnetLow = MP_SUBNET_UTILS.generate_random_subnet(24, range); - auto subnetHigh = MP_SUBNET_UTILS.generate_random_subnet(24, range); + auto subnetLow = MP_SUBNET_UTILS.random_subnet_from_range(24, range); + auto subnetHigh = MP_SUBNET_UTILS.random_subnet_from_range(24, range); EXPECT_EQ(subnetLow.network_address(), mp::IPAddress{"192.168.0.0"}); EXPECT_EQ(subnetLow.prefix_length(), 24); @@ -359,8 +361,8 @@ TEST_F(SubnetUtils, generateRandomSubnetWorksAtUpperExtreme) EXPECT_CALL(*mock_utils, random_int(_, _)).WillOnce(ReturnArg<0>()).WillOnce(ReturnArg<1>()); - auto subnetLow = MP_SUBNET_UTILS.generate_random_subnet(30, range); - auto subnetHigh = MP_SUBNET_UTILS.generate_random_subnet(30, range); + auto subnetLow = MP_SUBNET_UTILS.random_subnet_from_range(30, range); + auto subnetHigh = MP_SUBNET_UTILS.random_subnet_from_range(30, range); EXPECT_EQ(subnetLow.network_address(), mp::IPAddress{"0.0.0.0"}); EXPECT_EQ(subnetLow.prefix_length(), 30); @@ -377,7 +379,7 @@ TEST_F(SubnetUtils, generateRandomSubnetGivesUpUsedLocally) EXPECT_CALL(*mock_platform, subnet_used_locally).WillRepeatedly(Return(true)); - MP_EXPECT_THROW_THAT(std::ignore = MP_SUBNET_UTILS.generate_random_subnet(24, range), + MP_EXPECT_THROW_THAT(std::ignore = MP_SUBNET_UTILS.random_subnet_from_range(24, range), std::runtime_error, mpt::match_what(HasSubstr("subnet"))); } @@ -391,7 +393,7 @@ TEST_F(SubnetUtils, generateRandomSubnetGivesUpGatewayReached) EXPECT_CALL(*mock_platform, subnet_used_locally).WillRepeatedly(Return(false)); EXPECT_CALL(*mock_platform, can_reach_gateway).WillRepeatedly(Return(true)); - MP_EXPECT_THROW_THAT(std::ignore = MP_SUBNET_UTILS.generate_random_subnet(24, range), + MP_EXPECT_THROW_THAT(std::ignore = MP_SUBNET_UTILS.random_subnet_from_range(24, range), std::runtime_error, mpt::match_what(HasSubstr("subnet"))); } From 90fdd281d631c738a5858d8c45312e95ec3e96b8 Mon Sep 17 00:00:00 2001 From: trevor-shoe Date: Wed, 22 Oct 2025 01:52:25 -0400 Subject: [PATCH 26/28] [az] Fixup rebase --- tests/test_base_availability_zone.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_base_availability_zone.cpp b/tests/test_base_availability_zone.cpp index 983366bd30..eb4f4c59de 100644 --- a/tests/test_base_availability_zone.cpp +++ b/tests/test_base_availability_zone.cpp @@ -42,7 +42,7 @@ struct BaseAvailabilityZoneTest : public Test const std::string az_name{"zone1"}; const mp::fs::path az_dir{"/path/to/zones"}; const mp::fs::path az_file = az_dir / (az_name + ".json"); - const QString az_file_qstr{QString::fromStdU16tring(az_file.u16string())}; + const QString az_file_qstr{QString::fromStdU16String(az_file.u16string())}; const mp::Subnet az_subnet{"192.168.1.0/24"}; mpt::MockFileOps::GuardedMock mock_file_ops_guard{mpt::MockFileOps::inject()}; @@ -84,7 +84,7 @@ TEST_F(BaseAvailabilityZoneTest, loads_existing_zone_file) EXPECT_CALL(*mock_logger.mock_logger, log(_, _, _)).Times(AnyNumber()); EXPECT_CALL(*mock_json_utils_guard.first, - write_json(_, QString::fromStdString(az_file.u8string()))); + write_json(_, QString::fromStdU16String(az_file.u16string()))); mp::BaseAvailabilityZone zone{az_name, az_dir}; From 739be7ef59d61d1bc1af86fb0e672b362178a221 Mon Sep 17 00:00:00 2001 From: trevor-shoe Date: Fri, 24 Oct 2025 16:04:32 -0400 Subject: [PATCH 27/28] [network] Use C++20 three-way comparison --- include/multipass/ip_address.h | 10 ++-------- include/multipass/subnet.h | 6 ++---- src/network/ip_address.cpp | 30 +++--------------------------- src/network/subnet.cpp | 9 +-------- tests/test_subnet.cpp | 2 -- 5 files changed, 8 insertions(+), 49 deletions(-) diff --git a/include/multipass/ip_address.h b/include/multipass/ip_address.h index afa75eb459..c018d0788e 100644 --- a/include/multipass/ip_address.h +++ b/include/multipass/ip_address.h @@ -35,14 +35,8 @@ struct IPAddress std::string as_string() const; uint32_t as_uint32() const; - // TODO C++20 uncomment then remove other bool operators - // auto operator<=>(const IPAddress& other) const = default; - bool operator==(const IPAddress& other) const; - bool operator!=(const IPAddress& other) const; - bool operator<(const IPAddress& other) const; - bool operator<=(const IPAddress& other) const; - bool operator>(const IPAddress& other) const; - bool operator>=(const IPAddress& other) const; + std::strong_ordering operator<=>(const IPAddress& other) const; + bool operator==(const IPAddress& other) const = default; IPAddress operator+(int value) const; std::array octets; diff --git a/include/multipass/subnet.h b/include/multipass/subnet.h index 41f97fa1a4..320eec74b2 100644 --- a/include/multipass/subnet.h +++ b/include/multipass/subnet.h @@ -77,10 +77,8 @@ class Subnet [[nodiscard]] bool contains(Subnet other) const; [[nodiscard]] bool contains(IPAddress ip) const; - // TODO C++20 uncomment then remove existing operator== - // [[nodiscard]] std::strong_ordering operator<=>(const Subnet& other) const; - // [[nodiscard]] bool operator==(const Subnet& other) const == default; - [[nodiscard]] bool operator==(const Subnet& other) const; + [[nodiscard]] std::strong_ordering operator<=>(const Subnet& other) const; + [[nodiscard]] bool operator==(const Subnet& other) const = default; private: IPAddress address; diff --git a/src/network/ip_address.cpp b/src/network/ip_address.cpp index d352979a99..1981a53ed6 100644 --- a/src/network/ip_address.cpp +++ b/src/network/ip_address.cpp @@ -88,34 +88,10 @@ uint32_t mp::IPAddress::as_uint32() const return value; } -bool mp::IPAddress::operator==(const IPAddress& other) const +// uint8_t is not required to support <=> by the standard. Appease Apple clang. +std::strong_ordering mp::IPAddress::operator<=>(const IPAddress& other) const { - return octets == other.octets; -} - -bool mp::IPAddress::operator!=(const IPAddress& other) const -{ - return octets != other.octets; -} - -bool mp::IPAddress::operator<(const IPAddress& other) const -{ - return as_uint32() < other.as_uint32(); -} - -bool mp::IPAddress::operator<=(const IPAddress& other) const -{ - return as_uint32() <= other.as_uint32(); -} - -bool mp::IPAddress::operator>(const IPAddress& other) const -{ - return as_uint32() > other.as_uint32(); -} - -bool mp::IPAddress::operator>=(const IPAddress& other) const -{ - return as_uint32() >= other.as_uint32(); + return as_uint32() <=> other.as_uint32(); } mp::IPAddress mp::IPAddress::operator+(int value) const diff --git a/src/network/subnet.cpp b/src/network/subnet.cpp index 672e834771..a010b92dda 100644 --- a/src/network/subnet.cpp +++ b/src/network/subnet.cpp @@ -161,20 +161,13 @@ bool mp::Subnet::contains(IPAddress ip) const return address <= ip && (max_address() + 1) >= ip; } -bool mp::Subnet::operator==(const Subnet& other) const -{ - return address == other.address && prefix == other.prefix; -} - -/* TODO C++20 uncomment std::strong_ordering mp::Subnet::operator<=>(const Subnet& other) const { const auto ip_res = address <=> other.address; // note the prefix_length operands are purposely flipped - return (ip_res == 0) ? other.prefix_length <=> prefix_length : ip_res; + return (ip_res == 0) ? other.prefix_length() <=> prefix_length() : ip_res; } -*/ mp::Subnet mp::SubnetUtils::random_subnet_from_range(Subnet::PrefixLength prefix, Subnet range) const diff --git a/tests/test_subnet.cpp b/tests/test_subnet.cpp index deb6d79617..0a5cdd0091 100644 --- a/tests/test_subnet.cpp +++ b/tests/test_subnet.cpp @@ -255,7 +255,6 @@ TEST(Subnet, containsWorksOnUnContainedIps) EXPECT_FALSE(subnet.contains(ip)); } -/* TODO C++20 uncomment TEST(Subnet, relationalComparisonsWorkAsExpected) { const mp::Subnet low{"0.0.0.0/0"}; @@ -281,7 +280,6 @@ TEST(Subnet, relationalComparisonsWorkAsExpected) EXPECT_LT(submiddle, middle); EXPECT_LT(submiddle, high); } -*/ struct SubnetUtils : public Test { From 65946cf163f57564cb8c3b7548e19b4fa5c0d458 Mon Sep 17 00:00:00 2001 From: trevor-shoe Date: Sun, 26 Oct 2025 18:05:55 -0400 Subject: [PATCH 28/28] [network] Inherit stringview formatter for subnet --- include/multipass/subnet.h | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/include/multipass/subnet.h b/include/multipass/subnet.h index 320eec74b2..7a7b5e3daa 100644 --- a/include/multipass/subnet.h +++ b/include/multipass/subnet.h @@ -98,14 +98,8 @@ struct SubnetUtils : Singleton namespace fmt { template <> -struct formatter +struct formatter : formatter { - template - constexpr auto parse(ParseContext& ctx) - { - return ctx.begin(); - } - template auto format(const multipass::Subnet& subnet, FormatContext& ctx) const { @@ -114,14 +108,8 @@ struct formatter }; template <> -struct formatter +struct formatter : formatter { - template - constexpr auto parse(ParseContext& ctx) - { - return ctx.begin(); - } - template auto format(const multipass::Subnet::PrefixLength& prefix, FormatContext& ctx) const {