Fix bugs in Ipaddr filters (#149)

Fix bugs in Ipaddr filters

SUMMARY

ipaddr - Fix input validation issues in ipaddr,ipv4,ipv6,ipwrap filters.
ipaddr - Add valid network for link-local
fixes: #148
fixes: ansible-collections/ansible.netcommon#375
fixes: ansible-collections/ansible.netcommon#350

ISSUE TYPE


Bugfix Pull Request

COMPONENT NAME

ADDITIONAL INFORMATION

Reviewed-by: Nilashish Chakraborty <nilashishchakraborty8@gmail.com>
Reviewed-by: Sagar Paul <sagpaul@redhat.com>

README.md

@@ -10,8 +10,8 @@ The Ansible ``ansible.utils`` collection includes a variety of plugins that aid
 
 This collection has been tested against following Ansible versions: **>=2.9.10**.
 
-For collections that support Ansible 2.9, please ensure you update your `network_os` to use the 
+For collections that support Ansible 2.9, please ensure you update your `network_os` to use the
-fully qualified collection name (for example, `cisco.ios.ios`). 
+fully qualified collection name (for example, `cisco.ios.ios`).
 Plugins and modules within a collection may be tested with only specific Ansible versions.
 A collection may contain metadata that identifies these versions.
 PEP440 is the schema used to describe the versions of Ansible.

changelogs/fragments/bugfix_ipaddr_filter.yaml

@@ -0,0 +1,5 @@
+---
+bugfixes:
+  - Fix issue in ipaddr,ipv4,ipv6,ipwrap filters.(https://github.com/ansible-collections/ansible.utils/issues/148).
+  - ipaddr - Fix issue of breaking ipaddr filter with netcommon 2.6.0(https://github.com/ansible-collections/ansible.netcommon/issues/375).
+  - ipaddr - Add valid network for link-local (https://github.com/ansible-collections/ansible.netcommon/issues/350).

docs/ansible.utils.cidr_merge_filter.rst

@@ -159,7 +159,7 @@ Common return values are documented `here <https://docs.ansible.com/ansible/late
                     <b>data</b>
                     <a class="ansibleOptionLink" href="#return-" title="Permalink to this return value"></a>
                     <div style="font-size: small">
-                      <span style="color: purple">-</span>
+                      <span style="color: purple">raw</span>
                     </div>
                 </td>
                 <td></td>

docs/ansible.utils.ipaddr_filter.rst

@@ -78,8 +78,7 @@ Parameters
                     <b>value</b>
                     <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a>
                     <div style="font-size: small">
-                        <span style="color: purple">list</span>
+                        <span style="color: purple">raw</span>
-                         / <span style="color: purple">elements=string</span>
                          / <span style="color: red">required</span>
                     </div>
                 </td>
@@ -302,13 +301,12 @@ Common return values are documented `here <https://docs.ansible.com/ansible/late
                     <b>data</b>
                     <a class="ansibleOptionLink" href="#return-" title="Permalink to this return value"></a>
                     <div style="font-size: small">
-                      <span style="color: purple">list</span>
+                      <span style="color: purple">raw</span>
-                       / <span style="color: purple">elements=string</span>
                     </div>
                 </td>
                 <td></td>
                 <td>
-                            <div>Returns list with values valid for a particular query.</div>
+                            <div>Returns values valid for a particular query.</div>
                     <br/>
                 </td>
             </tr>

docs/ansible.utils.ipmath_filter.rst

@@ -166,13 +166,12 @@ Common return values are documented `here <https://docs.ansible.com/ansible/late
                     <b>data</b>
                     <a class="ansibleOptionLink" href="#return-" title="Permalink to this return value"></a>
                     <div style="font-size: small">
-                      <span style="color: purple">list</span>
+                      <span style="color: purple">string</span>
-                       / <span style="color: purple">elements=string</span>
                     </div>
                 </td>
                 <td></td>
                 <td>
-                            <div>Returns list with values valid for a particular query.</div>
+                            <div>Returns result of IP math/arithmetic.</div>
                     <br/>
                 </td>
             </tr>

docs/ansible.utils.ipsubnet_filter.rst

@@ -280,13 +280,12 @@ Common return values are documented `here <https://docs.ansible.com/ansible/late
                     <b>data</b>
                     <a class="ansibleOptionLink" href="#return-" title="Permalink to this return value"></a>
                     <div style="font-size: small">
-                      <span style="color: purple">list</span>
+                      <span style="color: purple">raw</span>
-                       / <span style="color: purple">elements=string</span>
                     </div>
                 </td>
                 <td></td>
                 <td>
-                            <div>Returns list with values valid for a particular query.</div>
+                            <div>Returns values valid for a particular query.</div>
                     <br/>
                 </td>
             </tr>

docs/ansible.utils.ipv4_filter.rst

@@ -59,8 +59,7 @@ Parameters
                     <b>value</b>
                     <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a>
                     <div style="font-size: small">
-                        <span style="color: purple">list</span>
+                        <span style="color: purple">raw</span>
-                         / <span style="color: purple">elements=string</span>
                          / <span style="color: red">required</span>
                     </div>
                 </td>
@@ -156,13 +155,12 @@ Common return values are documented `here <https://docs.ansible.com/ansible/late
                     <b>data</b>
                     <a class="ansibleOptionLink" href="#return-" title="Permalink to this return value"></a>
                     <div style="font-size: small">
-                      <span style="color: purple">list</span>
+                      <span style="color: purple">raw</span>
-                       / <span style="color: purple">elements=string</span>
                     </div>
                 </td>
                 <td></td>
                 <td>
-                            <div>Returns list with values valid for a particular query.</div>
+                            <div>Returns values valid for a particular query.</div>
                     <br/>
                 </td>
             </tr>

docs/ansible.utils.ipv6_filter.rst

@@ -59,8 +59,7 @@ Parameters
                     <b>value</b>
                     <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a>
                     <div style="font-size: small">
-                        <span style="color: purple">list</span>
+                        <span style="color: purple">raw</span>
-                         / <span style="color: purple">elements=string</span>
                          / <span style="color: red">required</span>
                     </div>
                 </td>
@@ -172,13 +171,12 @@ Common return values are documented `here <https://docs.ansible.com/ansible/late
                     <b>data</b>
                     <a class="ansibleOptionLink" href="#return-" title="Permalink to this return value"></a>
                     <div style="font-size: small">
-                      <span style="color: purple">list</span>
+                      <span style="color: purple">raw</span>
-                       / <span style="color: purple">elements=string</span>
                     </div>
                 </td>
                 <td></td>
                 <td>
-                            <div>Returns list with values valid for a particular query.</div>
+                            <div>Returns values valid for a particular query.</div>
                     <br/>
                 </td>
             </tr>

docs/ansible.utils.ipwrap_filter.rst

@@ -60,8 +60,7 @@ Parameters
                     <b>value</b>
                     <a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a>
                     <div style="font-size: small">
-                        <span style="color: purple">list</span>
+                        <span style="color: purple">raw</span>
-                         / <span style="color: purple">elements=string</span>
                          / <span style="color: red">required</span>
                     </div>
                 </td>
@@ -170,13 +169,12 @@ Common return values are documented `here <https://docs.ansible.com/ansible/late
                     <b>data</b>
                     <a class="ansibleOptionLink" href="#return-" title="Permalink to this return value"></a>
                     <div style="font-size: small">
-                      <span style="color: purple">list</span>
+                      <span style="color: purple">raw</span>
-                       / <span style="color: purple">elements=string</span>
                     </div>
                 </td>
                 <td></td>
                 <td>
-                            <div>Returns list with values valid for a particular query.</div>
+                            <div>Returns values valid for a particular query.</div>
                     <br/>
                 </td>
             </tr>

docs/ansible.utils.next_nth_usable_filter.rst

@@ -126,13 +126,12 @@ Common return values are documented `here <https://docs.ansible.com/ansible/late
                     <b>data</b>
                     <a class="ansibleOptionLink" href="#return-" title="Permalink to this return value"></a>
                     <div style="font-size: small">
-                      <span style="color: purple">list</span>
+                      <span style="color: purple">string</span>
-                       / <span style="color: purple">elements=string</span>
                     </div>
                 </td>
                 <td></td>
                 <td>
-                            <div>Returns list with values valid for a particular query.</div>
+                            <div>Returns the next nth usable ip within a network described by value.</div>
                     <br/>
                 </td>
             </tr>

docs/ansible.utils.previous_nth_usable_filter.rst

@@ -125,13 +125,12 @@ Common return values are documented `here <https://docs.ansible.com/ansible/late
                     <b>data</b>
                     <a class="ansibleOptionLink" href="#return-" title="Permalink to this return value"></a>
                     <div style="font-size: small">
-                      <span style="color: purple">list</span>
+                      <span style="color: purple">string</span>
-                       / <span style="color: purple">elements=string</span>
                     </div>
                 </td>
                 <td></td>
                 <td>
-                            <div>Returns list with values valid for a particular query.</div>
+                            <div>Returns the previous nth usable ip within a network described by value.</div>
                     <br/>
                 </td>
             </tr>

docs/ansible.utils.slaac_filter.rst

@@ -118,7 +118,7 @@ Common return values are documented `here <https://docs.ansible.com/ansible/late
                 </td>
                 <td></td>
                 <td>
-                            <div>Returns nth host from network</div>
+                            <div>Returns the SLAAC address within a network for a given HW/MAC address.</div>
                     <br/>
                 </td>
             </tr>

plugins/filter/cidr_merge.py

@@ -120,6 +120,7 @@ EXAMPLES = r"""
 
 RETURN = """
   data:
+    type: raw
     description:
       - Returns a minified list of subnets or a single subnet that spans all of the inputs.
 """

plugins/filter/ipaddr.py

@@ -13,6 +13,7 @@ from ansible_collections.ansible.utils.plugins.plugin_utils.base.ipaddr_utils im
     _need_netaddr,
 )
 from ansible.errors import AnsibleFilterError
+from ansible.errors import AnsibleError
 from ansible_collections.ansible.utils.plugins.module_utils.common.argspec_validate import (
     AnsibleArgSpecValidator,
 )
@@ -51,8 +52,7 @@ DOCUMENTATION = """
         value:
             description:
             - list of subnets or individual address or any other values input for ipaddr plugin
-            type: list
+            type: raw
-            elements: str
             required: True
         query:
             description:
@@ -240,10 +240,9 @@ EXAMPLES = r"""
 
 RETURN = """
   data:
-    type: list
+    type: raw
-    elements: str
     description:
-      - Returns list with values valid for a particular query.
+      - Returns values valid for a particular query.
 """
 
 
@@ -253,6 +252,25 @@ def _ipaddr(*args, **kwargs):
     keys = ["value", "query", "version", "alias"]
     data = dict(zip(keys, args[1:]))
     data.update(kwargs)
+    try:
+        if isinstance(data["value"], str):
+            pass
+        elif isinstance(data["value"], list):
+            pass
+        else:
+            raise AnsibleError(
+                "Unrecognized type <{0}> for ipaddr filter <{1}>".format(
+                    type(data["value"]), "value"
+                )
+            )
+
+    except (TypeError, ValueError):
+        raise AnsibleError(
+            "Unrecognized type <{0}> for ipaddr filter <{1}>".format(
+                type(data["value"]), "value"
+            )
+        )
+
     aav = AnsibleArgSpecValidator(
         data=data, schema=DOCUMENTATION, name="ipaddr"
     )

plugins/filter/ipmath.py

@@ -127,10 +127,9 @@ EXAMPLES = r"""
 
 RETURN = """
   data:
-    type: list
+    type: str
-    elements: str
     description:
-      - Returns list with values valid for a particular query.
+      - Returns result of IP math/arithmetic.
 """
 
 

plugins/filter/ipsubnet.py

@@ -231,10 +231,9 @@ tasks:
 
 RETURN = """
   data:
-    type: list
+    type: raw
-    elements: str
     description:
-      - Returns list with values valid for a particular query.
+      - Returns values valid for a particular query.
 """
 
 

plugins/filter/ipv4.py

@@ -13,6 +13,7 @@ from ansible_collections.ansible.utils.plugins.plugin_utils.base.ipaddr_utils im
     _need_netaddr,
 )
 from ansible.errors import AnsibleFilterError
+from ansible.errors import AnsibleError
 from ansible_collections.ansible.utils.plugins.module_utils.common.argspec_validate import (
     AnsibleArgSpecValidator,
 )
@@ -48,8 +49,7 @@ DOCUMENTATION = """
         value:
             description:
             - list of subnets or individual address or any other values input for ipv4 plugin
-            type: list
+            type: raw
-            elements: str
             required: True
         query:
             description:
@@ -118,10 +118,9 @@ EXAMPLES = r"""
 
 RETURN = """
   data:
-    type: list
+    type: raw
-    elements: str
     description:
-      - Returns list with values valid for a particular query.
+      - Returns values valid for a particular query.
 """
 
 
@@ -131,6 +130,24 @@ def _ipv4(*args, **kwargs):
     keys = ["value", "query"]
     data = dict(zip(keys, args[1:]))
     data.update(kwargs)
+    try:
+        if isinstance(data["value"], str):
+            pass
+        elif isinstance(data["value"], list):
+            pass
+        else:
+            raise AnsibleError(
+                "Unrecognized type <{0}> for ipv4 filter <{1}>".format(
+                    type(data["value"]), "value"
+                )
+            )
+
+    except (TypeError, ValueError):
+        raise AnsibleError(
+            "Unrecognized type <{0}> for ipv4 filter <{1}>".format(
+                type(data["value"]), "value"
+            )
+        )
     aav = AnsibleArgSpecValidator(data=data, schema=DOCUMENTATION, name="ipv4")
     valid, errors, updated_data = aav.validate()
     if not valid:

plugins/filter/ipv6.py

@@ -13,6 +13,7 @@ from ansible_collections.ansible.utils.plugins.plugin_utils.base.ipaddr_utils im
     _need_netaddr,
 )
 from ansible.errors import AnsibleFilterError
+from ansible.errors import AnsibleError
 from ansible_collections.ansible.utils.plugins.module_utils.common.argspec_validate import (
     AnsibleArgSpecValidator,
 )
@@ -50,8 +51,7 @@ DOCUMENTATION = """
         value:
             description:
             - list of subnets or individual address or any other values input for ipv6 plugin
-            type: list
+            type: raw
-            elements: str
             required: True
         query:
             description:
@@ -136,10 +136,9 @@ EXAMPLES = r"""
 
 RETURN = """
   data:
-    type: list
+    type: raw
-    elements: str
     description:
-      - Returns list with values valid for a particular query.
+      - Returns values valid for a particular query.
 """
 
 
@@ -149,6 +148,24 @@ def _ipv6(*args, **kwargs):
     keys = ["value", "query"]
     data = dict(zip(keys, args[1:]))
     data.update(kwargs)
+    try:
+        if isinstance(data["value"], str):
+            pass
+        elif isinstance(data["value"], list):
+            pass
+        else:
+            raise AnsibleError(
+                "Unrecognized type <{0}> for ipv6 filter <{1}>".format(
+                    type(data["value"]), "value"
+                )
+            )
+
+    except (TypeError, ValueError):
+        raise AnsibleError(
+            "Unrecognized type <{0}> for ipv6 filter <{1}>".format(
+                type(data["value"]), "value"
+            )
+        )
     aav = AnsibleArgSpecValidator(data=data, schema=DOCUMENTATION, name="ipv6")
     valid, errors, updated_data = aav.validate()
     if not valid:

plugins/filter/ipwrap.py

@@ -14,6 +14,7 @@ from ansible_collections.ansible.utils.plugins.plugin_utils.base.ipaddr_utils im
     _need_netaddr,
 )
 from ansible.errors import AnsibleFilterError
+from ansible.errors import AnsibleError
 from ansible_collections.ansible.utils.plugins.module_utils.common.argspec_validate import (
     AnsibleArgSpecValidator,
 )
@@ -53,8 +54,7 @@ DOCUMENTATION = """
             description:
             - list of subnets or individual address or any other values input. Example. ['192.24.2.1', 'host.fqdn',
               '::1', '192.168.32.0/24', 'fe80::100/10', True, '', '42540766412265424405338506004571095040/64']
-            type: list
+            type: raw
-            elements: str
             required: True
         query:
             description:
@@ -136,10 +136,9 @@ EXAMPLES = r"""
 
 RETURN = """
   data:
-    type: list
+    type: raw
-    elements: str
     description:
-      - Returns list with values valid for a particular query.
+      - Returns values valid for a particular query.
 """
 
 
@@ -149,6 +148,26 @@ def _ipwrap(*args, **kwargs):
     keys = ["value"]
     data = dict(zip(keys, args[1:]))
     data.update(kwargs)
+    try:
+        if isinstance(data["value"], str):
+            pass
+        elif isinstance(data["value"], list):
+            pass
+        elif isinstance(data["value"], bool):
+            pass
+        else:
+            raise AnsibleError(
+                "Unrecognized type <{0}> for ipwrap filter <{1}>".format(
+                    type(data["value"]), "value"
+                )
+            )
+
+    except (TypeError, ValueError):
+        raise AnsibleError(
+            "Unrecognized type <{0}> for ipwrap filter <{1}>".format(
+                type(data["value"]), "value"
+            )
+        )
     aav = AnsibleArgSpecValidator(
         data=data, schema=DOCUMENTATION, name="ipwrap"
     )

plugins/filter/next_nth_usable.py

@@ -91,10 +91,9 @@ EXAMPLES = r"""
 
 RETURN = """
   data:
-    type: list
+    type: str
-    elements: str
     description:
-      - Returns list with values valid for a particular query.
+      - Returns the next nth usable ip within a network described by value.
 """
 
 

plugins/filter/previous_nth_usable.py

@@ -90,10 +90,9 @@ EXAMPLES = r"""
 
 RETURN = """
   data:
-    type: list
+    type: str
-    elements: str
     description:
-      - Returns list with values valid for a particular query.
+      - Returns the previous nth usable ip within a network described by value.
 """
 
 

plugins/filter/slaac.py

@@ -79,7 +79,7 @@ RETURN = """
   data:
     type: str
     description:
-      - Returns nth host from network
+      - Returns the SLAAC address within a network for a given HW/MAC address.
 
 """
 

plugins/plugin_utils/base/ipaddr_utils.py

@@ -199,7 +199,7 @@ def _last_usable_query(v, vtype):
 def _link_local_query(v, value):
     v_ip = netaddr.IPAddress(str(v.ip))
     if v.version == 4:
-        if ipaddr(str(v_ip), "169.254.0.0/24"):
+        if ipaddr(str(v_ip), "169.254.0.0/16"):
             return value
 
     elif v.version == 6:

tests/integration/targets/utils_ipaddr_filter/tasks/ipaddr.yaml

@@ -42,3 +42,19 @@
 - name: Assert result for ipaddr network query
   assert:
     that: "{{ result4 == ipaddr_result4 }}"
+
+- name: ipaddr filter with network/prefix query
+  ansible.builtin.set_fact:
+    result5: "{{ '192.168.0.0/23' | ansible.utils.ipaddr('network/prefix') }}"
+
+- name: Assert result for ipaddr filter with network/prefix query
+  assert:
+    that: "{{ result5 == '192.168.0.0/23' }}"
+
+- name: ipaddr filter with chained filters
+  ansible.builtin.set_fact:
+    result6: "{{ '192.168.255.0' | ansible.utils.ipaddr('network') | ansible.utils.ipmath(123) }}"
+
+- name: Assert result for ipaddr filter with chained filters
+  assert:
+    that: "{{ result6 == '192.168.255.123' }}"

tests/integration/targets/utils_ipaddr_filter/tasks/ipv4.yaml

@@ -33,3 +33,11 @@
 - name: Assert result for ipv4 filter with address query.
   assert:
     that: "{{ result3 == result3_val }}"
+
+- name: ipv4 filter with single value
+  ansible.builtin.set_fact:
+    result4: "{{ '192.24.2.1' | ansible.utils.ipv4('ipv6') }}"
+
+- name: Assert result for ipv4 filter with single value
+  assert:
+    that: "{{ result4 == '::ffff:192.24.2.1/128' }}"

tests/integration/targets/utils_ipaddr_filter/tasks/ipv6.yaml

@@ -33,3 +33,11 @@
 - name: Assert result for ipv6 filter with address query.
   assert:
     that: "{{ result3 == ipv6_result3 }}"
+
+- name: ipv6 filter with single value
+  ansible.builtin.set_fact:
+    result4: "{{ '::ffff:192.168.32.0/120' | ansible.utils.ipv6('ipv4') }}"
+
+- name: Assert result for ipv6 filter with single value
+  assert:
+    that: "{{ result4 == '192.168.32.0/24' }}"

tests/integration/targets/utils_ipaddr_filter/tasks/ipwrap.yaml

@@ -17,3 +17,11 @@
 - name: Assert result for ipwrap.
   assert:
     that: "{{ result1 == ipwrap_result1 }}"
+
+- name: ipwrap filter with single value
+  ansible.builtin.set_fact:
+    result4: "{{ 'fe80::100/10' | ansible.utils.ipwrap }}"
+
+- name: Assert result for ipv6 filter with single value
+  assert:
+    that: "{{ result4 == '[fe80::100]/10' }}"

tests/integration/targets/utils_ipaddr_filter/vars/main.yaml

@@ -39,7 +39,7 @@ ipwrap_result1:
     - "192.168.32.0/24"
     - "[fe80::100]/10"
     - "[2001:db8:32c:faad::]/64"
-    - "True"
+    - True
     
 ipv6_result1:
     - "::ffff:192.168.32.0/120"

tests/unit/plugins/filter/test_ipwrap.py

@@ -35,7 +35,7 @@ VALID_OUTPUT = [
     "192.168.32.0/24",
     "[fe80::100]/10",
     "[2001:db8:32c:faad::]/64",
-    "True",
+    True,
 ]
 
 
@@ -55,4 +55,4 @@ class TestIpWrap(unittest.TestCase):
 
         args = ["", "::1", ""]
         result = _ipwrap(*args)
-        self.assertEqual(result, ["[::1]"])
+        self.assertEqual(result, "[::1]")