diff --git a/nodes/vr_node.go b/nodes/vr_node.go index 14aa1758d0..f47b148b0f 100644 --- a/nodes/vr_node.go +++ b/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( diff --git a/nodes/vr_node_mgmtfilter_test.go b/nodes/vr_node_mgmtfilter_test.go new file mode 100644 index 0000000000..373be8d2ae --- /dev/null +++ b/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) + } + }) + } +} diff --git a/nodes/vr_ros/vr-ros.go b/nodes/vr_ros/vr-ros.go index 08bb971288..3ed48c2747 100644 --- a/nodes/vr_ros/vr-ros.go +++ b/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") -}