Bug#1032988: unblock: netplan.io/0.106-2
Package: release.debian.org
Severity: normal
User: release.debian.org@packages.debian.org
Usertags: unblock
X-Debbugs-Cc: slyon@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 <luk@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.gondolfo@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.gondolfo@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.gondolfo@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-lapierre@canonical.com>
++# Author: Lukas Märdian <slyon@ubuntu.com>
++# Author: Danilo Egea Gondolfo <danilo.egea.gondolfo@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~),
Reply to: