From e67925463c45d34f2741be35aa90d1bdbe0b32fc Mon Sep 17 00:00:00 2001 From: Allen Robel Date: Wed, 15 Jul 2026 08:30:31 -1000 Subject: [PATCH 1/3] Normalize loopback ip to bare IPv4 (ND 4.2.1 rejects CIDR on the wire) Fixes #401. On develop, LoopbackPolicyModel.ip is typed IPv4CIDR and the field description/tests use /32 CIDR values, but live ND 4.2.1 400s a loopback create when ip is CIDR (e.g. 10.99.99.250/32) and accepts only the bare host form (HTTP 207). The intLoopbackTemplate declares ip as format: ipv4 (bare), consistent with the wire behavior. Add IPv4Host to models/types.py (BeforeValidator normalizing IPv4Interface to its bare .ip form) and switch LoopbackPolicyModel.ip from IPv4CIDR to IPv4Host, so bare or CIDR input is accepted but always normalized to bare before it reaches the wire. This keeps existing /32-style playbooks working and preserves idempotency, since ND always returns the bare address on read. ipv6 (IPv6CIDR) is unchanged - not verified against ND in this probe. develop's integration tasks already use bare ip values, so they need no changes. Updated the loopback model unit tests that expected the CIDR ip form to expect the normalized bare form and the new "is not a valid IPv4 address" error message. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01DYUaS5GPgMkSwN3Rzpt4DT --- .../models/interfaces/loopback_interface.py | 4 +- plugins/module_utils/models/types.py | 29 ++++++++++ .../models/test_loopback_interface.py | 54 ++++++++++--------- 3 files changed, 59 insertions(+), 28 deletions(-) diff --git a/plugins/module_utils/models/interfaces/loopback_interface.py b/plugins/module_utils/models/interfaces/loopback_interface.py index 6d8181508..563cd43c4 100644 --- a/plugins/module_utils/models/interfaces/loopback_interface.py +++ b/plugins/module_utils/models/interfaces/loopback_interface.py @@ -34,7 +34,7 @@ ) from ansible_collections.cisco.nd.plugins.module_utils.models.base import NDBaseModel from ansible_collections.cisco.nd.plugins.module_utils.models.nested import NDNestedModel -from ansible_collections.cisco.nd.plugins.module_utils.models.types import AsciiDescription, IPv4CIDR, IPv6CIDR +from ansible_collections.cisco.nd.plugins.module_utils.models.types import AsciiDescription, IPv4Host, IPv6CIDR class LoopbackPolicyModel(NDNestedModel): @@ -49,7 +49,7 @@ class LoopbackPolicyModel(NDNestedModel): """ admin_state: bool | None = Field(default=None, alias="adminState", description="Enable or disable the interface") - ip: IPv4CIDR = Field(default=None, alias="ip", description="Loopback IPv4 address in CIDR notation (e.g. 10.1.1.1/32)") + ip: IPv4Host = Field(default=None, alias="ip", description="Loopback IPv4 address (bare host form, e.g. 10.1.1.1; CIDR input is accepted and normalized)") ipv6: IPv6CIDR = Field(default=None, alias="ipv6", description="Loopback IPv6 address in CIDR notation") vrf: str | None = Field(default=None, alias="vrfInterface", min_length=1, max_length=32, description="Interface VRF name") route_map_tag: str | None = Field(default=None, alias="routeMapTag", description="Route-Map tag associated with interface IP") diff --git a/plugins/module_utils/models/types.py b/plugins/module_utils/models/types.py index d72c81f9f..1fac76381 100644 --- a/plugins/module_utils/models/types.py +++ b/plugins/module_utils/models/types.py @@ -17,6 +17,8 @@ UTF-8 input. Catching this client-side gives users a clear error instead of a confusing server fault. - `IPv4CIDR` — `str | None` validated as an IPv4 interface address in CIDR notation (e.g. `10.1.1.1/32`). - `IPv6CIDR` — `str | None` validated as an IPv6 interface address in CIDR notation (e.g. `2001:db8::1/128`). +- `IPv4Host` — `str | None` that accepts bare (`10.1.1.1`) or CIDR (`10.1.1.1/32`) input and normalizes to the bare + host address. Use where ND rejects CIDR notation on the wire (e.g. the loopback `ip` field). """ from __future__ import annotations @@ -116,6 +118,33 @@ def validate_ipv6_cidr(value: str | None) -> str | None: IPv6CIDR = Annotated[Optional[str], BeforeValidator(validate_ipv6_cidr)] """IPv6 interface address in CIDR notation (`str | None`). Layer with `Field(...)` for alias/description as usual.""" + +def validate_ipv4_host(value: str | None) -> str | None: + """ + # Summary + + Normalize an IPv4 interface address to its bare host form (strip any prefix). Accepts bare (`10.1.1.1`) or CIDR + (`10.1.1.1/32`) and returns the bare address, because ND's loopback `ip` field rejects CIDR notation. + + ## Raises + + ### ValueError + + - If `value` is not a valid IPv4 address or interface. + """ + if value is None: + return value + try: + return str(ipaddress.IPv4Interface(value).ip) + except (ipaddress.AddressValueError, ipaddress.NetmaskValueError, ValueError) as err: + raise ValueError(f"'{value}' is not a valid IPv4 address") from err + + +# See AsciiDescription comment above for why Optional[str] is used at runtime instead of `str | None`. +IPv4Host = Annotated[Optional[str], BeforeValidator(validate_ipv4_host)] +"""IPv4 host address (`str | None`). Accepts bare or CIDR input; normalizes to the bare host form (no prefix). +Layer with `Field(...)` for alias/description as usual.""" + # Fabric name rules (verified against ND 4.2.x GUI): # - Allowed chars: a-z, A-Z, 0-9, _, - # - Must start AND end with an alphanumeric (no leading/trailing _ or -) diff --git a/tests/unit/module_utils/models/test_loopback_interface.py b/tests/unit/module_utils/models/test_loopback_interface.py index 27ddcc514..0a2c5fc29 100644 --- a/tests/unit/module_utils/models/test_loopback_interface.py +++ b/tests/unit/module_utils/models/test_loopback_interface.py @@ -45,7 +45,7 @@ def does_not_raise(): "networkOSType": "nx-os", "policy": { "adminState": True, - "ip": "10.1.1.1/32", + "ip": "10.1.1.1", "vrfInterface": "management", "policyType": "loopback", "routeMapTag": "12345", @@ -65,7 +65,7 @@ def does_not_raise(): "network_os_type": "nx-os", "policy": { "admin_state": True, - "ip": "10.1.1.1/32", + "ip": "10.1.1.1", "vrf": "management", "policy_type": "loopback", "route_map_tag": "12345", @@ -127,14 +127,14 @@ def test_loopback_interface_00020(): with does_not_raise(): instance = LoopbackPolicyModel( admin_state=True, - ip="10.1.1.1/32", + ip="10.1.1.1", vrf="management", policy_type="loopback", route_map_tag="100", description="test", ) assert instance.admin_state is True - assert instance.ip == "10.1.1.1/32" + assert instance.ip == "10.1.1.1" assert instance.vrf == "management" assert instance.policy_type == "loopback" assert instance.route_map_tag == "100" @@ -159,13 +159,13 @@ def test_loopback_interface_00030(): with does_not_raise(): instance = LoopbackPolicyModel( adminState=False, - ip="10.2.2.2/32", + ip="10.2.2.2", vrfInterface="default", policyType="loopback", routeMapTag="200", ) assert instance.admin_state is False - assert instance.ip == "10.2.2.2/32" + assert instance.ip == "10.2.2.2" assert instance.vrf == "default" assert instance.policy_type == "loopback" assert instance.route_map_tag == "200" @@ -463,40 +463,42 @@ def test_loopback_interface_00080(): """ # Summary - Verify `ip` accepts a valid IPv4 address in CIDR notation. + Verify `ip` accepts a valid IPv4 address in CIDR notation and normalizes it to the bare host form (ND rejects + CIDR notation on the wire). ## Test - Construct with ip="10.1.1.1/32" - - Value is accepted + - Value is normalized to "10.1.1.1" (bare, no prefix) ## Classes and Methods - - LoopbackPolicyModel.ip (IPv4CIDR Annotated type) + - LoopbackPolicyModel.ip (IPv4Host Annotated type) """ with does_not_raise(): instance = LoopbackPolicyModel(ip="10.1.1.1/32") - assert instance.ip == "10.1.1.1/32" + assert instance.ip == "10.1.1.1" def test_loopback_interface_00081(): """ # Summary - Verify `ip` accepts a valid IPv4 address with a non-/32 prefix. + Verify `ip` accepts a valid IPv4 address with a non-/32 prefix and normalizes it to the bare host form (the + prefix is not preserved; ND's loopback `ip` field always wants a bare host address). ## Test - Construct with ip="10.1.1.0/24" - - Value is accepted + - Value is normalized to "10.1.1.0" (bare, no prefix) ## Classes and Methods - - LoopbackPolicyModel.ip (IPv4CIDR Annotated type) + - LoopbackPolicyModel.ip (IPv4Host Annotated type) """ with does_not_raise(): instance = LoopbackPolicyModel(ip="10.1.1.0/24") - assert instance.ip == "10.1.1.0/24" + assert instance.ip == "10.1.1.0" def test_loopback_interface_00082(): @@ -512,9 +514,9 @@ def test_loopback_interface_00082(): ## Classes and Methods - - LoopbackPolicyModel.ip (IPv4CIDR Annotated type) + - LoopbackPolicyModel.ip (IPv4Host Annotated type) """ - with pytest.raises(ValidationError, match="Invalid IPv4 address"): + with pytest.raises(ValidationError, match="is not a valid IPv4 address"): LoopbackPolicyModel(ip="999.999.999.999/32") @@ -531,7 +533,7 @@ def test_loopback_interface_00083(): ## Classes and Methods - - LoopbackPolicyModel.ip (IPv4CIDR Annotated type) + - LoopbackPolicyModel.ip (IPv4Host Annotated type) """ with does_not_raise(): instance = LoopbackPolicyModel(ip="10.1.1.1") @@ -551,9 +553,9 @@ def test_loopback_interface_00084(): ## Classes and Methods - - LoopbackPolicyModel.ip (IPv4CIDR Annotated type) + - LoopbackPolicyModel.ip (IPv4Host Annotated type) """ - with pytest.raises(ValidationError, match="Invalid IPv4 address"): + with pytest.raises(ValidationError, match="is not a valid IPv4 address"): LoopbackPolicyModel(ip="not-an-ip") @@ -570,9 +572,9 @@ def test_loopback_interface_00085(): ## Classes and Methods - - LoopbackPolicyModel.ip (IPv4CIDR Annotated type) + - LoopbackPolicyModel.ip (IPv4Host Annotated type) """ - with pytest.raises(ValidationError, match="Invalid IPv4 address"): + with pytest.raises(ValidationError, match="is not a valid IPv4 address"): LoopbackPolicyModel(ip="2001:db8::1/128") @@ -803,7 +805,7 @@ def test_loopback_interface_00160(): ), ) assert instance.mode == "managed" - assert instance.network_os.policy.ip == "10.1.1.1/32" + assert instance.network_os.policy.ip == "10.1.1.1" assert instance.network_os.policy.admin_state is True @@ -1123,7 +1125,7 @@ def test_loopback_interface_00400(): assert instance.interface_name == "loopback0" assert instance.interface_type == "loopback" assert instance.config_data.mode == "managed" - assert instance.config_data.network_os.policy.ip == "10.1.1.1/32" + assert instance.config_data.network_os.policy.ip == "10.1.1.1" assert instance.config_data.network_os.policy.vrf == "management" assert instance.config_data.network_os.policy.admin_state is True @@ -1217,7 +1219,7 @@ def test_loopback_interface_00450(): assert instance.interface_name == "loopback0" assert instance.interface_type == "loopback" assert instance.config_data.mode == "managed" - assert instance.config_data.network_os.policy.ip == "10.1.1.1/32" + assert instance.config_data.network_os.policy.ip == "10.1.1.1" assert instance.config_data.network_os.policy.vrf == "management" @@ -1463,7 +1465,7 @@ def test_loopback_interface_00650(): instance = LoopbackInterfaceModel.from_config(config_base) other = LoopbackInterfaceModel.from_config(config_other) instance.merge(other) - assert instance.config_data.network_os.policy.ip == "10.1.1.1/32" + assert instance.config_data.network_os.policy.ip == "10.1.1.1" def test_loopback_interface_00660(): @@ -1484,7 +1486,7 @@ def test_loopback_interface_00660(): instance = LoopbackInterfaceModel.from_config(copy.deepcopy(SAMPLE_ANSIBLE_CONFIG)) other = LoopbackInterfaceModel(switch_ip="192.168.1.1", interface_name="loopback0") instance.merge(other) - assert instance.config_data.network_os.policy.ip == "10.1.1.1/32" + assert instance.config_data.network_os.policy.ip == "10.1.1.1" def test_loopback_interface_00670(): From bc02821d4621054fab52e203b4f1998c49af8215 Mon Sep 17 00:00:00 2001 From: Allen Robel Date: Wed, 15 Jul 2026 08:56:31 -1000 Subject: [PATCH 2/3] Integration: add CIDR-ip normalization/idempotency test for loopback (#401) Add a merged-state task to nd_interface_loopback integration that supplies the loopback ip in CIDR form (10.100.104.1/32) and asserts (a) the create succeeds with the ip normalized to bare (10.100.104.1) in the after-state and (b) a re-apply of the same CIDR ip is idempotent against ND's bare wire form. This covers the end-to-end wire behavior and idempotency loop that the model unit tests cannot: live ND 4.2.1 400s a CIDR ip and stores/returns bare, so without normalization the create fails and merged-state would perpetually report changed. Note: this suite is not run against a live fabric in CI, so the task is a manual/lab regression guard - it must be verified on a live integration run. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01DYUaS5GPgMkSwN3Rzpt4DT --- .../nd_interface_loopback/tasks/merged.yaml | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/integration/targets/nd_interface_loopback/tasks/merged.yaml b/tests/integration/targets/nd_interface_loopback/tasks/merged.yaml index 9793fc36d..6bc49768f 100644 --- a/tests/integration/targets/nd_interface_loopback/tasks/merged.yaml +++ b/tests/integration/targets/nd_interface_loopback/tasks/merged.yaml @@ -141,3 +141,51 @@ - switch_ip: "{{ test_switch_ip }}" interface_name: loopback103 state: deleted + +# --- MERGED WITH CIDR-format ip --- +# Regression guard for #401: live ND 4.2.1 rejects a CIDR ip on a loopback create (400) and stores/returns the bare +# host form. The model normalizes CIDR input to bare before the wire, so a CIDR ip must (a) create successfully with a +# bare ip in the after-state and (b) be idempotent on re-apply against ND's bare wire form. + +- name: "MERGED CIDR: Create loopback104 with a CIDR-format ip (normal mode)" + cisco.nd.nd_interface_loopback: &merge_lb104_cidr + output_level: "{{ nd_info.output_level }}" + fabric_name: "{{ test_fabric_name }}" + config: + - switch_ip: "{{ test_switch_ip }}" + interface_name: loopback104 + config_data: + network_os: + policy: + admin_state: true + ip: "10.100.104.1/32" + description: "CIDR normalization test loopback104" + state: merged + register: nm_merged_cidr + +- name: "MERGED CIDR: Verify create succeeded and the ip was normalized to bare in the after state" + vars: + lb104_after: "{{ nm_merged_cidr.after | selectattr('interface_name', 'equalto', 'loopback104') | first }}" + ansible.builtin.assert: + that: + - nm_merged_cidr is changed + - lb104_after.config_data.network_os.policy.ip == "10.100.104.1" + +- name: "MERGED CIDR: Re-apply the same CIDR ip for idempotency" + cisco.nd.nd_interface_loopback: *merge_lb104_cidr + register: nm_merged_cidr_idem + +- name: "MERGED CIDR: Verify idempotency (CIDR input normalizes to ND's bare wire form)" + ansible.builtin.assert: + that: + - nm_merged_cidr_idem is not changed + +# Clean up the CIDR normalization test interface +- name: "CLEANUP: Remove loopback104" + cisco.nd.nd_interface_loopback: + output_level: "{{ nd_info.output_level }}" + fabric_name: "{{ test_fabric_name }}" + config: + - switch_ip: "{{ test_switch_ip }}" + interface_name: loopback104 + state: deleted From ce2e08bb33f3d75f02069e8499fd6f05af6d31c6 Mon Sep 17 00:00:00 2001 From: Allen Robel Date: Wed, 15 Jul 2026 09:12:46 -1000 Subject: [PATCH 3/3] Fix test_00083 docstring to match accept-and-normalize behavior Addresses Copilot review on #402: the test_loopback_interface_00083 Summary said `ip` "rejects a bare IPv4 address without prefix length" while the test asserts the bare address is accepted and returned in bare host form. Update the Summary and Test bullet to describe the actual IPv4Host behavior (bare input accepted, returned unchanged as the bare host form). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01DYUaS5GPgMkSwN3Rzpt4DT --- tests/unit/module_utils/models/test_loopback_interface.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/module_utils/models/test_loopback_interface.py b/tests/unit/module_utils/models/test_loopback_interface.py index 0a2c5fc29..6f92453bf 100644 --- a/tests/unit/module_utils/models/test_loopback_interface.py +++ b/tests/unit/module_utils/models/test_loopback_interface.py @@ -524,12 +524,12 @@ def test_loopback_interface_00083(): """ # Summary - Verify `ip` rejects a bare IPv4 address without prefix length. + Verify `ip` accepts a bare IPv4 address (no prefix) and returns it unchanged in the bare host form. ## Test - Construct with ip="10.1.1.1" (no CIDR prefix) - - Value is accepted (ipaddress.IPv4Interface accepts bare addresses, defaulting to /32) + - Value is accepted and returned as the bare host form "10.1.1.1" ## Classes and Methods