nodes: strip launcher-managed mgmt IP from saved vrnetlab configs

nodes/vr_node.go

@@ -126,6 +126,134 @@ func (vr *VRNode) CheckInterfaceName() error {
 	return nil
 }
 
+// mgmtIPTokenRE builds a regexp matching the given management IP address as a
+// standalone token, so that e.g. 172.20.20.2 does not also match 172.20.20.20.
+// The address is bounded on both sides by any character that is not part of an
+// IPv4/IPv6 literal (hex digit, dot or colon).
+func mgmtIPTokenRE(ip string) *regexp.Regexp {
+	const bound = `[^0-9A-Fa-f:.]`
+	return regexp.MustCompile(`(?:^|` + bound + `)` + regexp.QuoteMeta(ip) + `(?:` + bound + `|$)`)
+}
+
+// addressAssignmentRE guards the mgmt-IP filter to lines that actually assign an
+// address (interface `ip address` / `ipv4 address` / Junos `address x/y;` /
+// RouterOS `add address=`), so we never drop unrelated lines that merely
+// reference the mgmt IP (e.g. `snmp-server host`, `ntp server`, a BGP neighbor).
+var addressAssignmentRE = regexp.MustCompile(`(?i)address`)
+
+// descriptionRE excludes interface/other descriptions from the filter: a
+// `description "... 172.20.20.2 ..."` can carry both the mgmt IP and the word
+// "address", but is never an address assignment. A real address line never
+// contains the word "description".
+var descriptionRE = regexp.MustCompile(`(?i)description`)
+
+// mgmtConfigFilterPlatforms is the set of scrapli platform names for which it is
+// safe to strip the management address out of a saved config.
+//
+// Stripping is safe only when the node re-establishes its management address on
+// deploy *independently* of the saved config -- i.e. a vrnetlab launcher that
+// configures the management interface itself, from the container's IP, on every
+// boot. Otherwise removing the address from a saved config that is later reused
+// verbatim as a startup-config would leave the node with no management address
+// at all. This is exactly the set of kinds whose launchers do that and are
+// fixed launcher-side in the companion vrnetlab change: Cisco
+// IOS-XR/IOS-XE/NX-OS/ASA and Juniper Junos.
+//
+// Deliberately excluded even though their address syntax would match the filter:
+// kinds whose launcher applies the startup-config verbatim with no management
+// stanza of its own -- notably cisco_ios (vios) and the dell_emc/ftosv OS10
+// launcher -- because for those, stripping the saved management address is the
+// regression this change exists to prevent, inverted. Also excluded until their
+// launcher's management behavior is verified: arista_eos, huawei_vrp,
+// ipinfusion_ocnos, and aruba_aoscx (whose "ip static" syntax the filter's
+// "address" guard would not match anyway).
+//
+// Any platform not listed here is left untouched (no-op), so unverified kinds
+// never regress.
+var mgmtConfigFilterPlatforms = map[string]bool{
+	"cisco_asa":     true,
+	"cisco_iosxe":   true,
+	"cisco_iosxr":   true,
+	"cisco_nxos":    true,
+	"juniper_junos": true,
+}
+
+// FilterMgmtIPConfigLines removes the node's clab-assigned management IP
+// address(es) from a saved device configuration, structurally.
+//
+// VM-based (vrnetlab) nodes have their management interface configured by the
+// vrnetlab launcher from the container's *actual* IP on every boot. Persisting
+// that address into the saved startup-config pins a now-fixed IP that is
+// re-applied verbatim on the next deploy; if the container is later assigned a
+// different management IP (e.g. a partial `--node-filter` deploy, or docker
+// IPAM handing out a different address), the router comes up with mgmt on the
+// stale IP while clab/DNS expect the new one -- "healthy" but unreachable. By
+// dropping the launcher-managed address at save time we keep the launcher as the
+// single source of truth for management addressing.
+//
+// Device CLIs are stateful/hierarchical, so we do not blindly delete matching
+// lines (that would unbalance a Junos "address x/y { ... }" block or similar).
+// Instead, for the line carrying the mgmt IP:
+//   - if it opens a hierarchy block ("{"), the whole *balanced* block is removed
+//     so braces and children stay consistent (Junos with address options);
+//   - otherwise it is a leaf statement (IOS-XR "ipv4 address x", IOS "ip address
+//     x", Junos "address x/y;", RouterOS "add address=x/y ...") and only that
+//     line is removed, leaving the enclosing interface/context header intact so
+//     the launcher can re-add the address on the next boot.
+//
+// This is vendor-agnostic: it keys off the assigned IP rather than each vendor's
+// management-interface name, while still respecting config structure.
+func FilterMgmtIPConfigLines(config, mgmtV4, mgmtV6 string) string {
+	var res []*regexp.Regexp
+	for _, ip := range []string{mgmtV4, mgmtV6} {
+		if ip != "" {
+			res = append(res, mgmtIPTokenRE(ip))
+		}
+	}
+	if len(res) == 0 {
+		return config
+	}
+
+	// A line is a removal candidate only if it both carries a mgmt IP token and
+	// looks like an address assignment (guard against unrelated references), and
+	// is not a description (which may legitimately contain the mgmt IP and the
+	// word "address").
+	matches := func(s string) bool {
+		if !addressAssignmentRE.MatchString(s) || descriptionRE.MatchString(s) {
+			return false
+		}
+		for _, re := range res {
+			if re.MatchString(s) {
+				return true
+			}
+		}
+		return false
+	}
+
+	lines := strings.Split(config, "\n")
+	out := make([]string, 0, len(lines))
+	for i := 0; i < len(lines); {
+		line := lines[i]
+		if !matches(line) {
+			out = append(out, line)
+			i++
+			continue
+		}
+
+		// The mgmt IP is on this line. Decide leaf vs. block by net brace depth.
+		depth := strings.Count(line, "{") - strings.Count(line, "}")
+		if depth <= 0 {
+			i++ // leaf statement: drop just this line
+			continue
+		}
+		// Block opener: consume lines until the braces it opened are balanced.
+		for i++; i < len(lines) && depth > 0; i++ {
+			depth += strings.Count(lines[i], "{") - strings.Count(lines[i], "}")
+		}
+	}
+	return strings.Join(out, "\n")
+}
+
 func (n *VRNode) SaveConfig(_ context.Context) (*SaveConfigResult, error) {
 	config, err := clabnetconf.GetConfig(n.Cfg.LongName,
 		n.Cfg.Credentials.Username,
@@ -136,6 +264,14 @@ func (n *VRNode) SaveConfig(_ context.Context) (*SaveConfigResult, error) {
 		return nil, err
 	}
 
+	// Do not persist the launcher-managed management address; it is re-applied
+	// from the container's actual IP on every boot and would otherwise pin a
+	// stale mgmt IP on redeploy (see FilterMgmtIPConfigLines). Only for platforms
+	// whose config syntax we handle safely; others are left untouched.
+	if mgmtConfigFilterPlatforms[n.ScrapliPlatformName] {
+		config = FilterMgmtIPConfigLines(config, n.Cfg.MgmtIPv4Address, n.Cfg.MgmtIPv6Address)
+	}
+
 	// Save config to mounted labdir startup config path
 	configPath := filepath.Join(n.Cfg.LabDir, n.ConfigDirName, n.StartupCfgFName)
 	err = os.WriteFile(

nodes/vr_node_mgmtfilter_test.go

@@ -0,0 +1,102 @@
+// Copyright 2020 Nokia
+// Licensed under the BSD 3-Clause License.
+// SPDX-License-Identifier: BSD-3-Clause
+
+package nodes
+
+import "testing"
+
+func TestFilterMgmtIPConfigLines(t *testing.T) {
+	tests := map[string]struct {
+		config string
+		v4     string
+		v6     string
+		want   string
+	}{
+		"ios-xr-leaf": {
+			config: "interface MgmtEth0/RP0/CPU0/0\n ipv4 address 172.20.20.2 255.255.255.0\n!\n" +
+				"interface Loopback0\n ipv4 address 10.0.0.1 255.255.255.255\n!",
+			v4: "172.20.20.2",
+			// interface header kept, only the address leaf dropped
+			want: "interface MgmtEth0/RP0/CPU0/0\n!\n" +
+				"interface Loopback0\n ipv4 address 10.0.0.1 255.255.255.255\n!",
+		},
+		"ios-leaf": {
+			config: "interface GigabitEthernet1\n ip address 172.20.20.2 255.255.255.0\n!",
+			v4:     "172.20.20.2",
+			want:   "interface GigabitEthernet1\n!",
+		},
+		"nxos-slash": {
+			config: "interface mgmt0\n  ip address 172.20.20.2/24\n",
+			v4:     "172.20.20.2",
+			want:   "interface mgmt0\n",
+		},
+		"junos-leaf": {
+			config: "interfaces {\n    fxp0 {\n        unit 0 {\n            family inet {\n" +
+				"                address 172.20.20.2/24;\n            }\n        }\n    }\n}",
+			v4: "172.20.20.2",
+			want: "interfaces {\n    fxp0 {\n        unit 0 {\n            family inet {\n" +
+				"            }\n        }\n    }\n}",
+		},
+		"junos-address-block": {
+			// address that OPENS a block: the whole balanced block must go,
+			// or braces would be left unbalanced (the bug in naive line-delete).
+			config: "        family inet {\n            address 172.20.20.2/24 {\n" +
+				"                primary;\n                preferred;\n            }\n        }",
+			v4:   "172.20.20.2",
+			want: "        family inet {\n        }",
+		},
+		"routeros-setstyle": {
+			config: "/ip address\nadd address=172.20.20.2/24 interface=ether1\n" +
+				"add address=10.1.1.1/24 interface=ether2",
+			v4:   "172.20.20.2",
+			want: "/ip address\nadd address=10.1.1.1/24 interface=ether2",
+		},
+		"boundary-no-false-positive": {
+			// stripping .2 must NOT drop .20 or .200
+			config: "ip address 172.20.20.2 255.255.255.0\nip address 172.20.20.20 255.255.255.0\n" +
+				"ip address 172.20.20.200 255.255.255.0",
+			v4:   "172.20.20.2",
+			want: "ip address 172.20.20.20 255.255.255.0\nip address 172.20.20.200 255.255.255.0",
+		},
+		"address-guard-keeps-non-assignment": {
+			// the mgmt IP appears on non-address lines: those must be KEPT
+			config: "ip address 172.20.20.2 255.255.255.0\nsnmp-server host 172.20.20.2\n" +
+				"ntp server 172.20.20.2",
+			v4:   "172.20.20.2",
+			want: "snmp-server host 172.20.20.2\nntp server 172.20.20.2",
+		},
+		"description-with-mgmt-ip-and-address-word-kept": {
+			// a description carrying both the mgmt IP and the word "address"
+			// must NOT be stripped; only the real address line is.
+			config: " description \"mgmt address 172.20.20.2 primary\"\n" +
+				" ip address 172.20.20.2 255.255.255.0",
+			v4:   "172.20.20.2",
+			want: " description \"mgmt address 172.20.20.2 primary\"",
+		},
+		"ipv6": {
+			config: " ipv6 address 3fff:172:20:20::2/64\n ipv6 address 2001:db8::1/64",
+			v6:     "3fff:172:20:20::2",
+			want:   " ipv6 address 2001:db8::1/64",
+		},
+		"dual-stack": {
+			config: " ipv4 address 172.20.20.2 255.255.255.0\n ipv6 address 3fff::2/64\n description keep",
+			v4:     "172.20.20.2",
+			v6:     "3fff::2",
+			want:   " description keep",
+		},
+		"no-mgmt-ip-set": {
+			config: "ip address 172.20.20.2 255.255.255.0",
+			want:   "ip address 172.20.20.2 255.255.255.0",
+		},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			got := FilterMgmtIPConfigLines(tc.config, tc.v4, tc.v6)
+			if got != tc.want {
+				t.Errorf("FilterMgmtIPConfigLines()\n got: %q\nwant: %q", got, tc.want)
+			}
+		})
+	}
+}

nodes/vr_ros/vr-ros.go

@@ -137,8 +137,11 @@ func (n *vrRos) SaveConfig(_ context.Context) (*clabnodes.SaveConfigResult, erro
 		return nil, fmt.Errorf("received empty configuration from %s", n.Cfg.LongName)
 	}
 
-	// Filter out ether1 management interface IP address configuration
-	filtered_config := n.filterManagementInterfaceConfig(config_content)
+	// Drop the launcher-managed management address so a saved config never pins
+	// a stale mgmt IP that breaks reachability on redeploy (shared with all
+	// other vrnetlab kinds via VRNode).
+	filtered_config := clabnodes.FilterMgmtIPConfigLines(
+		config_content, n.Cfg.MgmtIPv4Address, n.Cfg.MgmtIPv6Address)
 
 	// Save config to mounted labdir startup config path
 	configPath := filepath.Join(n.Cfg.LabDir, n.ConfigDirName, n.StartupCfgFName)
@@ -158,31 +161,3 @@ func (n *vrRos) SaveConfig(_ context.Context) (*clabnodes.SaveConfigResult, erro
 		ConfigPath: configPath,
 	}, nil
 }
-
-// filterManagementInterfaceConfig removes ether1 (management interface) IP address configuration
-// from the exported RouterOS configuration to avoid including containerlab management IP settings.
-func (n *vrRos) filterManagementInterfaceConfig(config string) string {
-	lines := strings.Split(config, "\n")
-	var filteredLines []string
-
-	for _, line := range lines {
-		// Skip lines related to ether1 IP address configuration
-		if strings.Contains(line, "/ip address") {
-			// Mark that we're in the IP address section
-			filteredLines = append(filteredLines, line)
-			continue
-		}
-
-		// Skip IP address entries for ether1 interface
-		if strings.Contains(line, "interface=ether1") &&
-			(strings.Contains(line, "add address=") || strings.Contains(line, "add ")) {
-			// Skip this line as it's ether1 IP configuration
-			continue
-		}
-
-		// Keep all other lines
-		filteredLines = append(filteredLines, line)
-	}
-
-	return strings.Join(filteredLines, "\n")
-}