Package: release.debian.org Severity: normal User: release.debian....@packages.debian.org Usertags: unblock X-Debbugs-Cc: sl...@ubuntu.com
Please unblock package netplan.io I missed the hard freeze deadline by one day (after the 10 days soft freeze delay). netplan.io is a key package, thus we need a manual unblock. Thanks. -- Lukas [ Reason ] A regression was found in upstream Netplan v0.106 (as uploaded into testing via 0.106-1). It was fixed and merged upstream, incl. adding additional integration tests (autopkgtests) covering this regression. See: https://github.com/canonical/netplan/pull/331 [ Impact ] If the fix is not applied/unblocked, Netplan's DBus Config() API will be non-functional. See: https://netplan.readthedocs.io/en/stable/netplan-dbus/ [ Tests ] - We've added additional autopkgtests covering this regression explicitly, which are passing in unstable: * tests/integration/dbus.py * tests/integration/regressions.py - We've run snapd's "spread test" integration test suite, which originally found the issue and confirmed it's now passing: https://bugs.launchpad.net/netplan/+bug/1997467/comments/38 - We've run a manual test case: $ busctl call io.netplan.Netplan /io/netplan/Netplan io.netplan.Netplan Config $ busctl call io.netplan.Netplan /io/netplan/Netplan/config/XXX_CONFIG_ID io.netplan.Netplan.Config Set ss "network.bridges.br54.dhcp4=false" "90-test-config" $ busctl call io.netplan.Netplan /io/netplan/Netplan/config/XXX_CONFIG_ID io.netplan.Netplan.Config Get => Make sure the io.netplan.Netplan.Config.Get() DBus call returns some YAML output (not the empty string)! [ Risks ] - netplan.io is a key package, as a dependency of debian-cloud-images-packages. - The code fix is trivial: improve path building, by using g_build_path() instead of g_strjoin() + propagating the correct return value on failure. [ Checklist ] [x] all changes are documented in the d/changelog [x] I reviewed all changes and I approve them [x] attach debdiff against the package in testing [ Other info ] - Discussion in bug report: https://pad.lv/1997467 (comment #29 ++) - Upstream fixes: https://github.com/canonical/netplan/pull/331 unblock netplan.io/0.106-2
diff -Nru netplan.io-0.106/debian/changelog netplan.io-0.106/debian/changelog --- netplan.io-0.106/debian/changelog 2023-02-09 12:09:04.000000000 +0100 +++ netplan.io-0.106/debian/changelog 2023-03-02 17:40:56.000000000 +0100 @@ -1,3 +1,10 @@ +netplan.io (0.106-2) unstable; urgency=medium + + * Fix DBus .Config/.Get APIs using upstream commits (PR#331) (LP: #1997467) + * Enable additional 'dbus' autopkgtests to check the regressed cases + + -- Lukas Märdian <l...@slyon.de> Thu, 02 Mar 2023 17:40:56 +0100 + netplan.io (0.106-1) unstable; urgency=medium * Merge new upstream release 0.106 (from 0.106-0ubuntu1) diff -Nru netplan.io-0.106/debian/patches/lp1997467/0001-dbus-Build-the-copy-path-correctly.patch netplan.io-0.106/debian/patches/lp1997467/0001-dbus-Build-the-copy-path-correctly.patch --- netplan.io-0.106/debian/patches/lp1997467/0001-dbus-Build-the-copy-path-correctly.patch 1970-01-01 01:00:00.000000000 +0100 +++ netplan.io-0.106/debian/patches/lp1997467/0001-dbus-Build-the-copy-path-correctly.patch 2023-03-02 17:39:28.000000000 +0100 @@ -0,0 +1,23 @@ +From: Danilo Egea Gondolfo <danilo.egea.gondo...@canonical.com> +Date: Tue, 28 Feb 2023 17:23:37 +0000 +Subject: dbus: Build the copy path correctly + +Dbus Config()/Get() were not working due to a missing / in the +destination path. +--- + src/dbus.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/dbus.c b/src/dbus.c +index 7f633e6..510ac7d 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 diff -Nru netplan.io-0.106/debian/patches/lp1997467/0002-dbus-Use-the-error-set-by-_copy_yaml_state.patch netplan.io-0.106/debian/patches/lp1997467/0002-dbus-Use-the-error-set-by-_copy_yaml_state.patch --- netplan.io-0.106/debian/patches/lp1997467/0002-dbus-Use-the-error-set-by-_copy_yaml_state.patch 1970-01-01 01:00:00.000000000 +0100 +++ netplan.io-0.106/debian/patches/lp1997467/0002-dbus-Use-the-error-set-by-_copy_yaml_state.patch 2023-03-02 17:39:28.000000000 +0100 @@ -0,0 +1,75 @@ +From: Danilo Egea Gondolfo <danilo.egea.gondo...@canonical.com> +Date: Thu, 2 Mar 2023 13:42:54 +0000 +Subject: dbus: Use the error set by _copy_yaml_state() + +_copy_yaml_state() is setting an error when g_copy_file fails but it +wasn't used by any callers. +--- + src/dbus.c | 19 ++++++++++++------- + 1 file changed, 12 insertions(+), 7 deletions(-) + +diff --git a/src/dbus.c b/src/dbus.c +index 510ac7d..a5125fc 100644 +--- a/src/dbus.c ++++ b/src/dbus.c +@@ -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); + } diff -Nru netplan.io-0.106/debian/patches/lp1997467/0003-tests-Add-some-integration-tests-for-DBus.patch netplan.io-0.106/debian/patches/lp1997467/0003-tests-Add-some-integration-tests-for-DBus.patch --- netplan.io-0.106/debian/patches/lp1997467/0003-tests-Add-some-integration-tests-for-DBus.patch 1970-01-01 01:00:00.000000000 +0100 +++ netplan.io-0.106/debian/patches/lp1997467/0003-tests-Add-some-integration-tests-for-DBus.patch 2023-03-02 17:39:28.000000000 +0100 @@ -0,0 +1,306 @@ +From: Danilo Egea Gondolfo <danilo.egea.gondo...@canonical.com> +Date: Wed, 1 Mar 2023 15:13:53 +0000 +Subject: tests: Add some integration tests for DBus + +This tests can be executed as autopkgtests. They cover netplan get, set +and apply. + +There is a copy of test_dbus_config_get() in the file +integration/regressions.py that can be dropped once we adapt the +netplan.io package to run the new test set. +--- + tests/integration/dbus.py | 214 +++++++++++++++++++++++++++++++++++++++ + tests/integration/regressions.py | 54 ++++++++++ + 2 files changed, 268 insertions(+) + create mode 100644 tests/integration/dbus.py + +diff --git a/tests/integration/dbus.py b/tests/integration/dbus.py +new file mode 100644 +index 0000000..336ac0c +--- /dev/null ++++ b/tests/integration/dbus.py +@@ -0,0 +1,214 @@ ++#!/usr/bin/python3 ++# ++# Integration tests for netplan-dbus ++# ++# These need to be run in a VM and do change the system ++# configuration. ++# ++# Copyright (C) 2018-2023 Canonical, Ltd. ++# Author: Mathieu Trudel-Lapierre <mathieu.trudel-lapie...@canonical.com> ++# Author: Lukas Märdian <sl...@ubuntu.com> ++# Author: Danilo Egea Gondolfo <danilo.egea.gondo...@canonical.com> ++# ++# 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 <http://www.gnu.org/licenses/>. ++ ++import json ++import os ++import signal ++import sys ++import subprocess ++import unittest ++ ++from base import IntegrationTestsBase ++ ++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' ++ ] ++ ++BUSCTL_CONFIG_APPLY = [ ++ 'busctl', ++ '-j', ++ 'call', ++ '--system', ++ 'io.netplan.Netplan', ++ 'PLACEHOLDER', ++ 'io.netplan.Netplan.Config', ++ 'Apply' ++ ] ++ ++ ++class _CommonTests(): ++ ++ def setUp(self): ++ super().setUp() ++ ++ # If netplan-dbus is already running let's terminate it to ++ # be sure the process is not from a binary from an old package ++ # (before the installation of the one being tested) ++ 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) ++ ++ def test_dbus_config_get(self): ++ NETPLAN_YAML = '''network: ++ version: 2 ++ ethernets: ++ %(nic)s: ++ dhcp4: true ++''' ++ ++ 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") ++ ++ def test_dbus_config_set(self): ++ BUSCTL_CONFIG_SET = [ ++ 'busctl', ++ '-j', ++ 'call', ++ '--system', ++ 'io.netplan.Netplan', ++ 'PLACEHOLDER', ++ 'io.netplan.Netplan.Config', ++ 'Set', ++ 'ss', ++ 'ethernets.%(nic)s.dhcp4=false' % {'nic': self.dev_e_client}, ++ '', ++ ] ++ ++ NETPLAN_YAML_BEFORE = '''network: ++ version: 2 ++ ethernets: ++ %(nic)s: ++ dhcp4: true ++''' ++ ++ NETPLAN_YAML_AFTER = '''network: ++ version: 2 ++ ethernets: ++ %(nic)s: ++ dhcp4: false ++''' ++ with open(self.config, 'w') as f: ++ f.write(NETPLAN_YAML_BEFORE % {'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 ++ BUSCTL_CONFIG_SET[5] = config_path ++ ++ # Changing the configuration ++ out = subprocess.run(BUSCTL_CONFIG_SET, capture_output=True, universal_newlines=True) ++ self.assertEqual(out.returncode, 0, msg=f"Busctl Set() failed with error: {out.stderr}") ++ ++ out_dict = json.loads(out.stdout) ++ self.assertEqual(out_dict.get('data')[0], True, msg="Set command failed") ++ ++ # Retrieving the configuration ++ 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_YAML_AFTER % {'nic': self.dev_e_client}, ++ netplan_data, msg="The final YAML is different than expected") ++ ++ def test_dbus_config_apply(self): ++ NETPLAN_YAML = '''network: ++ version: 2 ++ bridges: ++ br1234: ++ dhcp4: false ++''' ++ ++ self.addCleanup(subprocess.call, ['ip', 'link', 'delete', 'br1234'], stderr=subprocess.DEVNULL) ++ ++ with open(self.config, 'w') as f: ++ f.write(NETPLAN_YAML) ++ ++ 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_APPLY[5] = config_path ++ ++ # Applying the configuration ++ out = subprocess.run(BUSCTL_CONFIG_APPLY, capture_output=True, universal_newlines=True) ++ self.assertEqual(out.returncode, 0, msg=f"Busctl Apply() failed with error: {out.stderr}") ++ ++ out_dict = json.loads(out.stdout) ++ self.assertEqual(out_dict.get('data')[0], True, msg="Apply command failed") ++ ++ self.assert_iface('br1234') ++ ++ ++class TestNetworkd(IntegrationTestsBase, _CommonTests): ++ pass ++ ++ ++unittest.main(testRunner=unittest.TextTestRunner(stream=sys.stdout, verbosity=2)) +diff --git a/tests/integration/regressions.py b/tests/integration/regressions.py +index b1fe708..71bc532 100644 +--- a/tests/integration/regressions.py ++++ b/tests/integration/regressions.py +@@ -21,6 +21,7 @@ + # 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 +@@ -145,4 +146,57 @@ class TestNetworkManager(IntegrationTestsBase, _CommonTests): + backend = 'NetworkManager' + + ++class TestDbus(IntegrationTestsBase): ++ # This test can be dropped when tests/integration/dbus.py is ++ # integrated as an autopkgtest in the Debian package ++ 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)) diff -Nru netplan.io-0.106/debian/patches/series netplan.io-0.106/debian/patches/series --- netplan.io-0.106/debian/patches/series 2023-02-09 11:10:55.000000000 +0100 +++ netplan.io-0.106/debian/patches/series 2023-03-02 17:39:28.000000000 +0100 @@ -0,0 +1,3 @@ +lp1997467/0001-dbus-Build-the-copy-path-correctly.patch +lp1997467/0002-dbus-Use-the-error-set-by-_copy_yaml_state.patch +lp1997467/0003-tests-Add-some-integration-tests-for-DBus.patch diff -Nru netplan.io-0.106/debian/tests/control netplan.io-0.106/debian/tests/control --- netplan.io-0.106/debian/tests/control 2023-02-09 12:09:04.000000000 +0100 +++ netplan.io-0.106/debian/tests/control 2023-03-02 17:39:28.000000000 +0100 @@ -142,6 +142,20 @@ Restrictions: allow-stderr, needs-root, isolation-container, breaks-testbed Features: test-name=regressions +Test-Command: ./debian/tests/prep-testbed.sh && python3 tests/integration/run.py --test=dbus +Tests-Directory: tests/integration +Depends: @, + systemd-resolved | systemd (<< 251.3-2~), + network-manager, + hostapd, + wpasupplicant, + dnsmasq-base, + libnm0, + python3-gi, + gir1.2-nm-1.0, +Restrictions: allow-stderr, needs-root, isolation-container +Features: test-name=dbus + Test-Command: ./debian/tests/prep-testbed.sh && ./debian/tests/autostart.sh Depends: @, systemd-resolved | systemd (<< 251.3-2~),