Merge #2190

2190: [firewall] Fix detection of nftables support in kernel r=Saviq a=townsend2010 Fixes #2183 Co-authored-by: Chris Townsend <christopher.townsend@canonical.com>
canonical · Aug 5, 2021 · 6582d9f · 6582d9f
2 parents a130207 + 7524f1b
commit 6582d9f
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 10 deletions.
diff --git a/src/platform/backends/qemu/firewall_config.cpp b/src/platform/backends/qemu/firewall_config.cpp
@@ -299,7 +299,7 @@ QString detect_firewall()
     QString firewall_exec;
     try
     {
-        firewall_exec = (is_firewall_in_use(nftables) || (!is_firewall_in_use(iptables) && kernel_supports_nftables()))
+        firewall_exec = kernel_supports_nftables() && (is_firewall_in_use(nftables) || !is_firewall_in_use(iptables))
                             ? nftables
                             : iptables;
     }

diff --git a/src/platform/backends/qemu/qemu_virtual_machine_factory.cpp b/src/platform/backends/qemu/qemu_virtual_machine_factory.cpp
@@ -51,18 +51,14 @@ auto generate_tap_device_name(const std::string& vm_name)
 
 void create_virtual_switch(const std::string& subnet, const QString& bridge_name)
 {
-    const QString dummy_name{bridge_name + "-dummy"};
-
     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);
 
         mp::utils::run_cmd_for_status("ip",
-                                      {"link", "add", dummy_name, "address", mac_address.c_str(), "type", "dummy"});
-        mp::utils::run_cmd_for_status("ip", {"link", "add", bridge_name, "type", "bridge"});
-        mp::utils::run_cmd_for_status("ip", {"link", "set", dummy_name, "master", bridge_name});
+                                      {"link", "add", bridge_name, "address", mac_address.c_str(), "type", "bridge"});
         mp::utils::run_cmd_for_status(
             "ip", {"address", "add", cidr.c_str(), "dev", bridge_name, "broadcast", broadcast.c_str()});
         mp::utils::run_cmd_for_status("ip", {"link", "set", bridge_name, "up"});
@@ -71,12 +67,9 @@ void create_virtual_switch(const std::string& subnet, const QString& bridge_name
 
 void delete_virtual_switch(const QString& bridge_name)
 {
-    const QString dummy_name{bridge_name + "-dummy"};
-
     if (mp::utils::run_cmd_for_status("ip", {"addr", "show", bridge_name}))
     {
         mp::utils::run_cmd_for_status("ip", {"link", "delete", bridge_name});
-        mp::utils::run_cmd_for_status("ip", {"link", "delete", dummy_name});
     }
 }
 

diff --git a/tests/qemu/test_firewall_config.cpp b/tests/qemu/test_firewall_config.cpp
@@ -223,12 +223,18 @@ INSTANTIATE_TEST_SUITE_P(FirewallConfig, FirewallToUseTestSuite,
 TEST_P(KernelCheckTestSuite, usesIptablesAndLogsWithBadKernelInfo)
 {
     auto [kernel, msg] = GetParam();
+    bool nftables_called{false};
 
-    mpt::MockProcessFactory::Callback firewall_callback = [](mpt::MockProcess* process) {
+    mpt::MockProcessFactory::Callback firewall_callback = [&nftables_called](mpt::MockProcess* process)
+    {
         if (process->program() == "iptables-legacy" && process->arguments().contains("--list-rules"))
         {
             EXPECT_CALL(*process, read_all_standard_output()).WillOnce(Return(QByteArray()));
         }
+        else if (process->program() == "iptables-nft")
+        {
+            nftables_called = true;
+        }
     };
 
     auto factory = mpt::MockProcessFactory::Inject();
@@ -242,6 +248,8 @@ TEST_P(KernelCheckTestSuite, usesIptablesAndLogsWithBadKernelInfo)
     logger_scope.mock_logger->expect_log(mpl::Level::warning, msg);
 
     mp::FirewallConfig firewall_config{goodbr0, subnet};
+
+    EXPECT_FALSE(nftables_called);
 }
 
 INSTANTIATE_TEST_SUITE_P(FirewallConfig, KernelCheckTestSuite,