Fix Netplan deletion of interfaces in shared files

This PR fixes interface deletion when multiple interfaces share a Netplan YAML file.

Each deletion looks up the interface’s current position, so earlier deletions cannot leave stale line numbers.

Unrelated interfaces and global settings are preserved. Empty device sections are removed.

Live regression tests and nine native Netplan generation scenarios passed on Ubuntu.

net/netplan-lib.pl

@@ -354,13 +354,32 @@ if ($iface->{'virtual'} ne '') {
 	&save_interface($parent, $boot);
 	}
 else {
-	# Delete all the lines
+	# Find current lines because earlier deletions may have shifted them.
 	&lock_file($iface->{'file'});
+	&unflush_file_lines($iface->{'file'});
+	my $yaml = &read_yaml_file($iface->{'file'});
+	my @network = grep { $_->{'name'} eq 'network' } @$yaml;
+	my @groups = grep { $_->{'name'} eq 'ethernets' ||
+			   $_->{'name'} eq 'bridges' }
+			 map { @{$_->{'members'}} } @network;
+	my $entry;
+	foreach my $g (@groups) {
+		my ($e) = grep { $_->{'name'} eq $iface->{'fullname'} }
+				@{$g->{'members'}};
+		next if (!$e);
+		# Remove the group as well when deleting its last interface.
+		$entry = @{$g->{'members'}} == 1 ? $g : $e;
+		last;
+		}
+	$entry || &error("No interface named $iface->{'fullname'} exists");
+
+	# Delete the interface without changing neighboring definitions.
 	my $lref = &read_file_lines($iface->{'file'});
-	splice(@$lref, $iface->{'line'},
-	       $iface->{'eline'} - $iface->{'line'} + 1);
+	splice(@$lref, $entry->{'line'},
+	       $entry->{'eline'} - $entry->{'line'} + 1);
 	&flush_file_lines($iface->{'file'});
 	if (&is_yaml_empty($iface->{'file'})) {
+		# Discard the file only when no settings or interfaces remain.
 		&unlink_file($iface->{'file'});
 		}
 	&unlock_file($iface->{'file'});
@@ -368,22 +387,22 @@ else {
 }
 
 # is_yaml_empty(file)
-# Return 1 if a YAML file contains only network and ethernets line, with no
-# other interfaces
+# Returns 1 if only the version and empty Ethernet or bridge groups remain
 sub is_yaml_empty
 {
 my ($file) = @_;
 my $yaml = &read_yaml_file($file);
 return 1 if (!$yaml);
-my @rest = grep { $_->{'name'} ne 'network' } @$yaml;
-return 0 if (@rest);
 foreach my $n (@$yaml) {
-	my @rest = grep { $_->{'name'} ne 'ethernets' &&
-			  $_->{'name'} ne 'bridges' }
-			@{$network->{'members'}};
-	return 0 if (@rest);
-	foreach my $ens (@{$network->{'members'}}) {
-		return 0 if (@{$ens->{'members'}});
+	# Preserve unknown directives and inline network settings.
+	return 0 if ($n->{'name'} ne 'network' ||
+		    ($n->{'value'} ne '' && $n->{'value'} ne '{}'));
+	# Only the version and empty device groups are safe to discard.
+	foreach my $ens (@{$n->{'members'}}) {
+		next if ($ens->{'name'} eq 'version');
+		return 0 if (($ens->{'name'} ne 'ethernets' &&
+			     $ens->{'name'} ne 'bridges') || @{$ens->{'members'}} ||
+			($ens->{'value'} ne '' && $ens->{'value'} ne '{}'));
 		}
 	}
 return 1;

t/net-netplan-vm.t

@@ -0,0 +1,123 @@
+#!/usr/bin/perl
+# Run with WEBMIN_NETPLAN_VM_TEST=1 on a Linux debug VM with Netplan installed.
+use strict;
+use warnings;
+no warnings 'once';
+use Test::More;
+use File::Path qw(make_path);
+use File::Temp qw(tempdir);
+use FindBin;
+use lib "$FindBin::Bin/../net";
+
+plan skip_all => 'Set WEBMIN_NETPLAN_VM_TEST=1 on a Linux debug VM'
+    unless ($ENV{'WEBMIN_NETPLAN_VM_TEST'} || '') eq '1';
+$^O eq 'linux' or die 'Linux VM required';
+require "$FindBin::Bin/../web-lib-funcs.pl";
+require "$FindBin::Bin/../net/netplan-lib.pl";
+local %main::gconfig = ();
+local $main::readonly_mode_cache = 0;
+
+# Load real address helpers without initializing the VM's Webmin configuration.
+my $net_lib = main::read_file_contents("$FindBin::Bin/../net/net-lib.pl");
+foreach my $name (qw(prefix_to_mask mask_to_prefix log2)) {
+    my ($code) = $net_lib =~ /^(sub \Q$name\E\n\{.*?^\})/ms;
+    $code or die "Cannot find $name";
+    eval "no strict; $code";
+    die $@ if $@;
+}
+
+# Keep realistic addresses, DNS, routes, and comments in the surviving port.
+my $header = "network:\n  version: 2\n";
+my $global = $header."  renderer: networkd\n";
+my $ethernet = <<'YAML';
+  ethernets:
+    eth0:
+      # Preserve the port's static settings.
+      addresses: [192.0.2.10/24, '2001:db8::10/64']
+      nameservers:
+        addresses: [192.0.2.53]
+        search: [example.test]
+      routes:
+        - to: default
+          via: 192.0.2.1
+    eth1:
+      dhcp4: true
+YAML
+my $bridge0 = "    br0:\n      interfaces: [eth0]\n";
+my $bridge1 = "    br1:\n      interfaces: [eth1]\n";
+my $standalone = "    br0:\n      interfaces: []\n      dhcp4: true\n";
+my $other_groups = <<'YAML';
+  vlans:
+    vlan10:
+      id: 10
+      link: eth0
+  bonds:
+    bond0:
+      interfaces: [eth1]
+      parameters:
+        mode: active-backup
+YAML
+my $inline = "  ethernets: {eth0: {dhcp4: true}}\n";
+my $bulk = $global."  ethernets: {}\n  bridges:\n".
+    "    br0:\n      dhcp4: false\n    br1:\n      dhcp4: true\n";
+
+# Each case supplies valid input, deletion order, and exact expected contents.
+foreach my $case (
+    ['shared bridge', $header.$ethernet."  bridges:\n".$bridge0,
+        ['br0'], $header.$ethernet],
+    ['remaining bridge', $header.$ethernet."  bridges:\n".$bridge0.$bridge1,
+        ['br0'], $header.$ethernet."  bridges:\n".$bridge1],
+    ['last Ethernet', $header."  ethernets:\n    eth0: {}\n",
+        ['eth0'], undef],
+    ['last bridge', $header."  bridges:\n".$standalone,
+        ['br0'], undef],
+    ['VLAN and bond settings', $header.$ethernet.$other_groups.
+        "  bridges:\n".$standalone, ['br0'], $header.$ethernet.$other_groups],
+    ['inline Ethernet mapping', $header.$inline."  bridges:\n".$bridge0,
+        ['br0'], $header.$inline],
+    ['bulk deletion preserves existing empty groups', $bulk, ['br1', 'br0'],
+        $global."  ethernets: {}\n"],
+    ['bulk deletion follows display order',
+        $global."  ethernets:\n    eth0: {}\n  bridges:\n".$standalone,
+        ['eth0', 'br0'], $global],
+    ['virtual IPv4 address', $header."  ethernets:\n    eth0:\n".
+        "      addresses: [192.0.2.10/24, 192.0.2.11/24]\n",
+        ['eth0:0'], $header."  ethernets:\n    eth0:\n".
+        "      addresses: ['192.0.2.10/24']\n"],
+) {
+    subtest $case->[0] => sub {
+        # Generate only beneath this temporary root, without applying networking.
+        my $root = tempdir(CLEANUP => 1);
+        local $main::netplan_dir = "$root/etc/netplan";
+        make_path($main::netplan_dir);
+        my $file = "$main::netplan_dir/shared.yaml";
+        my $control = "$main::netplan_dir/00-renderer.yaml";
+        main::write_file_contents($file, $case->[1]);
+        main::write_file_contents($control, $global);
+        chmod(0600, $file, $control) == 2 or die "chmod: $!";
+        is(system('netplan', 'generate', '--root-dir', $root), 0,
+            'original configuration passes Netplan');
+
+        # Match the UI's reuse of one interface list throughout bulk deletion.
+        my @boot = main::boot_interfaces();
+        foreach my $name (@{$case->[2]}) {
+            my ($iface) = grep { $_->{'fullname'} eq $name } @boot;
+            $iface or die "Fixture interface $name not found";
+            main::delete_interface($iface);
+        }
+        if (defined($case->[3])) {
+            # Surviving settings and comments must retain their exact contents.
+            is(main::read_file_contents($file), $case->[3],
+                'remaining configuration is unchanged');
+        }
+        else {
+            # No interface or global setting remains in this file.
+            ok(!-e $file, 'empty file is removed');
+        }
+        is(main::read_file_contents($control), $global,
+            'another YAML file is untouched');
+        is(system('netplan', 'generate', '--root-dir', $root), 0,
+            'resulting configuration passes Netplan');
+    };
+}
+done_testing();

t/net-netplan.t

@@ -0,0 +1,65 @@
+#!/usr/bin/perl
+# Exercise Netplan deletion using temporary files only, without host configuration.
+use strict;
+use warnings;
+no warnings 'once';
+use Test::More;
+use File::Temp qw(tempdir);
+use FindBin;
+use lib "$FindBin::Bin/../net";
+require "$FindBin::Bin/../web-lib-funcs.pl";
+require "$FindBin::Bin/../net/netplan-lib.pl";
+local %main::gconfig = ();
+local $main::readonly_mode_cache = 0;
+my $root = tempdir(CLEANUP => 1);
+my $file = "$root/shared.yaml";
+local $main::netplan_dir = $root;
+
+# Empty detection must retain real interface entries and global settings.
+foreach my $case (
+    ["network:\n  version: 2\n  ethernets:\n  bridges:\n", 1, 'empty interface groups'],
+    ["network:\n  version: 2\n  ethernets:\n    eth0: {}\n", 0, 'empty Ethernet definition'],
+    ["network:\n  version: 2\n  bridges:\n    br1:\n      dhcp4: true\n", 0, 'another bridge'],
+    ["network:\n  version: 2\n  renderer: networkd\n", 0, 'global renderer'],
+    ["network:\n  version: 2\n  vlans:\n    vlan10:\n      id: 10\n      link: eth0\n", 0, 'VLAN settings']) {
+    main::write_file_contents($file, $case->[0]);
+    main::unflush_file_lines($file);
+    is(main::is_yaml_empty($file), $case->[1], "$case->[2] handled correctly");
+}
+
+# Deleting the last bridge must retain its port and unrelated YAML byte for byte.
+my $remaining = "network:\n  version: 2\n  ethernets:\n    eth0:\n      dhcp4: true\n".
+    "    eth1:\n      dhcp4: false\n";
+my $original = $remaining."  bridges:\n    br0:\n      interfaces: [eth0]\n";
+main::write_file_contents($file, $original);
+main::unflush_file_lines($file);
+main::delete_interface((grep { $_->{'fullname'} eq 'br0' } main::boot_interfaces())[0]);
+ok(-f $file, 'shared YAML survives bridge deletion');
+is(main::read_file_contents($file), $remaining, 'empty group removed, Ethernet settings untouched');
+
+# Removing one bridge must preserve the group and the remaining bridge.
+my $second = "    br1:\n      interfaces: [eth1]\n";
+main::write_file_contents($file, $original.$second);
+main::unflush_file_lines($file);
+main::delete_interface((grep { $_->{'fullname'} eq 'br0' } main::boot_interfaces())[0]);
+is(main::read_file_contents($file), $remaining."  bridges:\n".$second,
+    'another bridge and its group remain unchanged');
+
+# A bridge-only file is removed once its last interface is gone.
+main::write_file_contents($file, "network:\n  version: 2\n  bridges:\n    br0:\n      dhcp4: true\n");
+main::unflush_file_lines($file);
+main::delete_interface((grep { $_->{'fullname'} eq 'br0' } main::boot_interfaces())[0]);
+ok(!-e $file, 'empty bridge-only file is removed');
+
+# Bulk deletion reuses interface details after earlier deletions shift lines.
+my $global = "network:\n  version: 2\n  renderer: networkd\n";
+main::write_file_contents($file, $global."  ethernets: {}\n  bridges:\n".
+    "    br0:\n      dhcp4: false\n    br1:\n      dhcp4: true\n");
+main::unflush_file_lines($file);
+my @boot = main::boot_interfaces();
+foreach my $iface (@boot) {
+    main::delete_interface($iface);
+}
+is(main::read_file_contents($file), $global."  ethernets: {}\n",
+    'bulk deletion handles line shifts and preserves unrelated settings');
+done_testing();