diff --git a/CHANGELOG.md b/CHANGELOG.md index b0e9ab7c..8015935b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 GitHub Actions dependency updates, with weekly lock-file maintenance, grouped non-major updates, and immediate vulnerability alerts. +### Fixed + +- Collapsed VLAN lines can produce a destructive `no vlan x,y` remediation in Cisco IOS (#264) + --- ## [3.6.0] - 2026-03-26 diff --git a/docs/drivers.md b/docs/drivers.md index 45d9f729..25d34514 100644 --- a/docs/drivers.md +++ b/docs/drivers.md @@ -60,6 +60,7 @@ Cisco IOS is hier_config's primary reference platform and the most thoroughly te - **Negation**: standard `no ` [negation prefix](glossary.md#negation-prefix). Several commands (such as `logging console`) use [`NegationDefaultWithRule`](glossary.md#negation-negate-with) overrides to emit a specific reset form. - **[Sectional exiting](glossary.md#sectional-exiting)**: BGP `peer-policy` and `peer-session` blocks require `exit-peer-policy` and `exit-peer-session` closure tokens. - **Per-line substitutions**: strips `Building configuration…` banners and timestamp headers. +- **VLAN id list splitting**: IOS can render unnamed VLANs collapsed onto a single comma/range line (e.g. `vlan 69,381`, `vlan 10-12`), depending on how the VLANs were created — named VLANs always get their own block, and the grouping shifts as VLANs are named or unnamed. When such a collapsed line is present, a post-load callback splits it into one `vlan ` block each so the VLANs diff block-to-block against an intended config that lists them separately — avoiding a destructive `no vlan 69,381`. Platform enum: `Platform.CISCO_IOS` diff --git a/hier_config/platforms/cisco_ios/driver.py b/hier_config/platforms/cisco_ios/driver.py index 06d93b90..5ae5f4f4 100644 --- a/hier_config/platforms/cisco_ios/driver.py +++ b/hier_config/platforms/cisco_ios/driver.py @@ -43,12 +43,43 @@ def _add_acl_sequence_numbers(config: HConfig) -> None: sequence_number += 10 +def _expand_vlan_id_list(spec: str) -> list[int]: + """Expand a VLAN id spec like ``69,381`` or ``10-12,20`` into a list of ints.""" + ids: list[int] = [] + for part in spec.split(","): + if "-" in part: + low, high = part.split("-") + ids.extend(range(int(low), int(high) + 1)) + else: + ids.append(int(part)) + return ids + + +def _split_vlan_id_lists(config: HConfig) -> None: + """Split ``vlan 69,381`` / ``vlan 10-12`` into a separate ``vlan `` block per VLAN. + + IOS can render unnamed VLANs collapsed onto a single comma/range line + (depending on how they were created; named VLANs always get their own block). + Splitting such a line at load time lets the diff engine match VLANs + block-to-block against an intended config that lists them separately, + avoiding a destructive ``no vlan 69,381``. + """ + for vlan in tuple(config.get_children(re_search=r"^vlan \d[\d,\-]*$")): + spec = vlan.text.split(maxsplit=1)[1] + if not any(separator in spec for separator in (",", "-")): + continue + for vlan_id in _expand_vlan_id_list(spec): + config.add_child(f"vlan {vlan_id}") + vlan.delete() + + class HConfigDriverCiscoIOS(HConfigDriverBase): """Driver for Cisco IOS and IOS-XE. - Includes post-load callbacks that normalise IPv4/IPv6 ACL sequence numbers - and remove IPv4 ACL remarks, ensuring stable diffs across device snapshots. - Also handles BGP template peer-policy/peer-session sectional exiting and + Includes post-load callbacks that normalise IPv4/IPv6 ACL sequence numbers, + remove IPv4 ACL remarks, and split collapsed comma/range VLAN id lists into + one block per VLAN, ensuring stable diffs across device snapshots. Also + handles BGP template peer-policy/peer-session sectional exiting and ``logging console`` negation-with replacement. Platform enum: ``Platform.CISCO_IOS``. """ @@ -196,5 +227,6 @@ def _instantiate_rules() -> HConfigDriverRules: _rm_ipv6_acl_sequence_numbers, _remove_ipv4_acl_remarks, _add_acl_sequence_numbers, + _split_vlan_id_lists, ], ) diff --git a/tests/test_driver_cisco_ios.py b/tests/test_driver_cisco_ios.py index aba54622..7d4ea876 100644 --- a/tests/test_driver_cisco_ios.py +++ b/tests/test_driver_cisco_ios.py @@ -141,3 +141,80 @@ def test_add_acl_sequence_numbers() -> None: assert acl.get_child(equals="10 permit tcp any any eq 443") is not None assert acl.get_child(equals="20 permit tcp any any eq 80") is not None assert acl.get_child(equals="30 deny ip any any") is not None + + +def test_vlan_id_list_split_on_load() -> None: + """The post-load callback expands 'vlan 69,381' into one block per VLAN id.""" + config = get_hconfig(Platform.CISCO_IOS, "vlan 69,381\n") + assert config.get_child(equals="vlan 69") is not None + assert config.get_child(equals="vlan 381") is not None + assert config.get_child(equals="vlan 69,381") is None + + +def test_vlan_id_range_split_on_load() -> None: + """The post-load callback expands ranges into individual VLAN ids.""" + config = get_hconfig(Platform.CISCO_IOS, "vlan 10-12\n") + assert [c.text for c in config.children] == ["vlan 10", "vlan 11", "vlan 12"] + + +def test_single_vlan_not_split_on_load() -> None: + """A single VLAN id is left untouched (no comma/range separator).""" + config = get_hconfig(Platform.CISCO_IOS, "vlan 44\n name servers\n") + vlan = config.get_child(equals="vlan 44") + assert vlan is not None + assert vlan.get_child(equals="name servers") is not None + + +def test_non_vlan_id_line_not_split_on_load() -> None: + """Lines like 'vlan internal allocation policy ...' are not VLAN id lists.""" + text = "vlan internal allocation policy ascending\n" + config = get_hconfig(Platform.CISCO_IOS, text) + assert ( + config.get_child(equals="vlan internal allocation policy ascending") is not None + ) + + +def test_vlan_id_list_rename_is_not_destructive() -> None: + """Renaming one VLAN in a collapsed list must not negate the whole list.""" + running_config = get_hconfig(Platform.CISCO_IOS, "vlan 69,381\n") + generated_config = get_hconfig( + Platform.CISCO_IOS, "vlan 69\n name newname\nvlan 381\n" + ) + remediation = running_config.config_to_get_to(generated_config).dump_simple() + assert remediation == ("vlan 69", " name newname") + assert not any(line.lstrip().startswith("no vlan") for line in remediation) + + +def test_vlan_id_list_naming_middle_vlan_regroups_cleanly() -> None: + """Naming a VLAN regroups IOS commas (69,70,71 -> 69,71 + 70); diff stays surgical.""" + running_config = get_hconfig(Platform.CISCO_IOS, "vlan 69,70,71\n") + generated_config = get_hconfig( + Platform.CISCO_IOS, "vlan 69,71\nvlan 70\n name MIDDLE\n" + ) + remediation = running_config.config_to_get_to(generated_config).dump_simple() + assert remediation == ("vlan 70", " name MIDDLE") + assert not any(line.lstrip().startswith("no vlan") for line in remediation) + + +def test_vlan_id_list_partial_removal_is_surgical() -> None: + """Removing only some VLANs from a collapsed list negates just those ids.""" + running_config = get_hconfig(Platform.CISCO_IOS, "vlan 69,381,400\n") + generated_config = get_hconfig(Platform.CISCO_IOS, "vlan 69\nvlan 381\n") + remediation = running_config.config_to_get_to(generated_config).dump_simple() + assert remediation == ("no vlan 400",) + + +def test_vlan_id_list_pure_add_emits_one_line_per_vlan() -> None: + """Adding a collapsed VLAN list emits one (non-destructive) line per VLAN.""" + running_config = get_hconfig(Platform.CISCO_IOS, "") + generated_config = get_hconfig(Platform.CISCO_IOS, "vlan 10-12,20\n") + remediation = running_config.config_to_get_to(generated_config).dump_simple() + assert remediation == ("vlan 10", "vlan 11", "vlan 12", "vlan 20") + + +def test_vlan_id_list_no_change_is_idempotent() -> None: + """Identical collapsed VLAN lists produce no remediation.""" + running_config = get_hconfig(Platform.CISCO_IOS, "vlan 69,381\n") + generated_config = get_hconfig(Platform.CISCO_IOS, "vlan 69,381\n") + remediation = running_config.config_to_get_to(generated_config).dump_simple() + assert remediation == ()