Merge ~danilogondolfo/netplan/+git/ubuntu:jammy_lp1997467_dbus into ~ubuntu-core-dev/netplan/+git/ubuntu:ubuntu-jammy

[Danilo Egea Gondolfo]

[Łukasz Zemczak (sil2100)]

Looking good, will merge and release. Discussed with Danilo already that I didn't see the tests/integration/dbus.py test added as part of these changes. Not strictly required, but more testing is never a bad thing, even if it makes the diff bigger.

Anyway, let's proceed!

review: Approve

diff --git a/debian/changelog b/debian/changelog
index 16e5297..f33f8a8 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,16 @@
+netplan.io (0.105-0ubuntu2~22.04.3) jammy; urgency=medium
+
+  * Fix and improvements for the DBus integration (LP: #1997467)
+    Cherry-picked from upstream: https://github.com/canonical/netplan/pull/331
+    - d/p/lp1997467/0009-dbus-Build-the-copy-path-correctly.patch
+      Properly build the destination path before copying files in the dbus
+      integration and improve error handling
+    - d/p/lp1997467/0010-tests-Add-an-integration-test-for-netplan-dbus.patch
+      Add an integration test to exercise the code path where the issue was
+      addressed.
+
+ -- Danilo Egea Gondolfo <[e-mail]>  Fri, 03 Mar 2023 13:14:22 +0000
+
 netplan.io (0.105-0ubuntu2~22.04.2) jammy; urgency=medium
 
   * d/p/lp1997467: set only specific origin-hint if given (LP: #1997467)
diff --git a/debian/patches/lp1997467/0009-dbus-Build-the-copy-path-correctly.patch b/debian/patches/lp1997467/0009-dbus-Build-the-copy-path-correctly.patch
new file mode 100644
index 0000000..27d9f23
--- /dev/null
+++ b/debian/patches/lp1997467/0009-dbus-Build-the-copy-path-correctly.patch
@@ -0,0 +1,93 @@
+From: Danilo Egea Gondolfo <[e-mail]>
+Subject: dbus: Build the copy path correctly
+
+This patch fixes a bug in the DBus integration where it was silently failing to
+copy the netplan configuration files due to a malformed file path.
+It also improves the error handling in netplan-dbus by propagating the failure to
+the function callers.
+
+This fix is related to https://github.com/canonical/netplan/pull/299
+
+Origin: upstream, https://github.com/canonical/netplan/commit/2ff1daa9fcdb5779bfb88631a0af1104a0722f1e and https://github.com/canonical/netplan/commit/a76739110da77920f73d467e4102df2eb0d1af2c
+Bug-Ubuntu: https://bugs.launchpad.net/netplan/+bug/1997467
+---
+ src/dbus.c | 21 +++++++++++++--------
+ 1 file changed, 13 insertions(+), 8 deletions(-)
+
+diff --git a/src/dbus.c b/src/dbus.c
+index 7f633e6..a5125fc 100644
+--- a/src/dbus.c
++++ b/src/dbus.c
+@@ -141,7 +141,7 @@ _copy_yaml_state(char *src_root, char *dst_root, sd_bus_error *ret_error)
+     gchar *dest_path = NULL;
+     size_t len = strlen(src_root);
+     for (size_t i = 0; i < gl.gl_pathc; ++i) {
+-        dest_path = g_strjoin(NULL, dst_root, (gl.gl_pathv[i])+len, NULL);
++        dest_path = g_build_path(G_DIR_SEPARATOR_S, dst_root, (gl.gl_pathv[i])+len, NULL);
+         source = g_file_new_for_path(gl.gl_pathv[i]);
+         dest = g_file_new_for_path(dest_path);
+         g_file_copy(source, dest, G_FILE_COPY_OVERWRITE
+@@ -223,8 +223,8 @@ _backup_global_state(sd_bus_error *ret_error)
+     }
+ 
+     /* Copy main *.yaml files from /{etc,run,lib}/netplan/ to GLOBAL backup dir */
+-    _copy_yaml_state(NETPLAN_ROOT, path, ret_error);
+-    return 0;
++    r = _copy_yaml_state(NETPLAN_ROOT, path, ret_error);
++    return r;
+ }
+ 
+ /**
+@@ -444,7 +444,8 @@ netplan_try_cancelled_cb(sd_event_source *es, const siginfo_t *si, void* userdat
+         unlink_glob(NETPLAN_ROOT, "/{etc,run,lib}/netplan/*.yaml");
+         /* Restore GLOBAL backup config state to main rootdir */
+         state_dir = g_strdup_printf("%s/run/netplan/config-%s", NETPLAN_ROOT, NETPLAN_GLOBAL_CONFIG);
+-        _copy_yaml_state(state_dir, NETPLAN_ROOT, NULL);
++        r = _copy_yaml_state(state_dir, NETPLAN_ROOT, NULL);
++        if (r < 0) return r;
+ 
+         /* Un-invalidate all other current config objects */
+         if (!g_strcmp0(d->handler_id, d->config_dirty))
+@@ -564,7 +565,8 @@ method_config_apply(sd_bus_message *m, void *userdata, sd_bus_error *ret_error)
+         unlink_glob(NETPLAN_ROOT, "/{etc,run,lib}/netplan/*.yaml");
+         /* Copy current config state to GLOBAL */
+         state_dir = g_strdup_printf("%s/run/netplan/config-%s", NETPLAN_ROOT, d->config_id);
+-        _copy_yaml_state(state_dir, NETPLAN_ROOT, ret_error);
++        r = _copy_yaml_state(state_dir, NETPLAN_ROOT, ret_error);
++        if (r < 0) return r;
+         d->handler_id = g_strdup(d->config_id);
+     }
+ 
+@@ -639,7 +641,8 @@ method_config_try(sd_bus_message *m, void *userdata, sd_bus_error *ret_error)
+ 
+     /* Copy current config *.yaml state to main rootdir (i.e. /etc/netplan/) */
+     state_dir = g_strdup_printf("%s/run/netplan/config-%s", NETPLAN_ROOT, d->config_id);
+-    _copy_yaml_state(state_dir, NETPLAN_ROOT, ret_error);
++    r = _copy_yaml_state(state_dir, NETPLAN_ROOT, ret_error);
++    if (r < 0) return r;
+ 
+     /* Exec try */
+     r = method_try(m, userdata, ret_error);
+@@ -670,7 +673,8 @@ method_config_cancel(sd_bus_message *m, void *userdata, sd_bus_error *ret_error)
+         unlink_glob(NETPLAN_ROOT, "/{etc,run,lib}/netplan/*.yaml");
+         /* Restore GLOBAL backup config state to main rootdir */
+         state_dir = g_strdup_printf("%s/run/netplan/config-%s", NETPLAN_ROOT, NETPLAN_GLOBAL_CONFIG);
+-        _copy_yaml_state(state_dir, NETPLAN_ROOT, ret_error);
++        r = _copy_yaml_state(state_dir, NETPLAN_ROOT, ret_error);
++        if (r < 0) return r;
+ 
+         /* Clear GLOBAL backup and config state */
+         _clear_tmp_state(NETPLAN_GLOBAL_CONFIG, d);
+@@ -754,7 +758,8 @@ method_config(sd_bus_message *m, void *userdata, sd_bus_error *ret_error)
+     }
+ 
+     /* Copy all *.yaml files from /{etc,run,lib}/netplan/ to temp dir */
+-    _copy_yaml_state(NETPLAN_ROOT, path, ret_error);
++    r = _copy_yaml_state(NETPLAN_ROOT, path, ret_error);
++    if (r < 0) return r;
+ 
+     return sd_bus_reply_method_return(m, "o", obj_path);
+ }
+-- 
+2.39.2
+
diff --git a/debian/patches/lp1997467/0010-tests-Add-an-integration-test-for-netplan-dbus.patch b/debian/patches/lp1997467/0010-tests-Add-an-integration-test-for-netplan-dbus.patch
new file mode 100644
index 0000000..3fb9015
--- /dev/null
+++ b/debian/patches/lp1997467/0010-tests-Add-an-integration-test-for-netplan-dbus.patch
@@ -0,0 +1,86 @@
+From: Danilo Egea Gondolfo <[e-mail]>
+Subject: tests: Add an integration test for netplan-dbus
+
+This test exercises the code path where the dbus integration bug was
+addressed.
+
+This fix is related to https://github.com/canonical/netplan/pull/299
+
+Origin: upstream, https://github.com/canonical/netplan/commit/4e89bbf0240d37e662666bec83019cbe3d18edf6
+Bug-Ubuntu: https://bugs.launchpad.net/netplan/+bug/1997467
+---
+ tests/integration/regressions.py | 53 ++++++++++++++++++++++++++++++++
+ 1 file changed, 53 insertions(+)
+
+diff --git a/tests/integration/regressions.py b/tests/integration/regressions.py
+index 1a996b4..6e3afd4 100644
+--- a/tests/integration/regressions.py
++++ b/tests/integration/regressions.py
+@@ -21,6 +21,8 @@
+ # You should have received a copy of the GNU General Public License
+ # along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ 
++import json
++import os
+ import sys
+ import signal
+ import subprocess
+@@ -142,4 +144,55 @@ class TestNetworkManager(IntegrationTestsBase, _CommonTests):
+     backend = 'NetworkManager'
+ 
+ 
++class TestDbus(IntegrationTestsBase):
++    def test_dbus_config_get_lp1997467(self):
++
++        NETPLAN_YAML = '''network:
++  version: 2
++  ethernets:
++    %(nic)s:
++      dhcp4: true
++'''
++        BUSCTL_CONFIG = [
++                'busctl', '-j', 'call', '--system',
++                'io.netplan.Netplan', '/io/netplan/Netplan',
++                'io.netplan.Netplan', 'Config']
++
++        BUSCTL_CONFIG_GET = [
++                'busctl', '-j', 'call', '--system',
++                'io.netplan.Netplan', 'PLACEHOLDER',
++                'io.netplan.Netplan.Config', 'Get']
++
++        # Terminates netplan-dbus if it is running already
++        cmd = ['ps', '-C', 'netplan-dbus', '-o', 'pid=']
++        out = subprocess.run(cmd, capture_output=True, universal_newlines=True)
++        if out.returncode == 0:
++            pid = out.stdout.strip()
++            os.kill(int(pid), signal.SIGTERM)
++
++        with open(self.config, 'w') as f:
++            f.write(NETPLAN_YAML % {'nic': self.dev_e_client})
++
++        out = subprocess.run(BUSCTL_CONFIG, capture_output=True, universal_newlines=True)
++        self.assertEqual(out.returncode, 0, msg=f"Busctl Config() failed with error: {out.stderr}")
++
++        out_dict = json.loads(out.stdout)
++        config_path = out_dict.get('data')[0]
++        self.assertNotEqual(config_path, "", msg="Got an empty response from DBUS")
++
++        # The path has the following format: /io/netplan/Netplan/config/WM6X01
++        BUSCTL_CONFIG_GET[5] = config_path
++
++        # Retrieving the config
++        out = subprocess.run(BUSCTL_CONFIG_GET, capture_output=True, universal_newlines=True)
++        self.assertEqual(out.returncode, 0, msg=f"Busctl Get() failed with error: {out.stderr}")
++
++        out_dict = json.loads(out.stdout)
++        netplan_data = out_dict.get('data')[0]
++
++        self.assertNotEqual(netplan_data, "", msg="Got an empty response from DBUS")
++        self.assertEqual(netplan_data, NETPLAN_YAML % {'nic': self.dev_e_client},
++                         msg="The original YAML is different from the one returned by DBUS")
++
++
+ unittest.main(testRunner=unittest.TextTestRunner(stream=sys.stdout, verbosity=2))
+-- 
+2.39.2
+
diff --git a/debian/patches/series b/debian/patches/series
index 06ed42e..18f0f7f 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -6,3 +6,5 @@ lp1997467/0005-parse-Allow-loading-nullable-origin-hint-overrides-n.patch
 lp1997467/0006-cli-set-fix-origin-hint-handling-LP-1997467.patch
 lp1997467/0007-src-parse-netplan-write-global-renderer-depending-on.patch
 lp1997467/0008-src-parse-plug-memory-leaks-in-nullable-handling.patch
+lp1997467/0009-dbus-Build-the-copy-path-correctly.patch
+lp1997467/0010-tests-Add-an-integration-test-for-netplan-dbus.patch