diff --git a/changelogs/fragments/ipsubnet_performance.yaml b/changelogs/fragments/ipsubnet_performance.yaml new file mode 100644 index 0000000..f60ebbc --- /dev/null +++ b/changelogs/fragments/ipsubnet_performance.yaml @@ -0,0 +1,14 @@ +--- +bugfixes: + - >- + ipsubnet - interacting with large subnets could cause performance + constraints. the result would be the system would appear to hang while + it built out a list of all possible subnets or stepped through all + possible subnets one at a time. when sending a prefix that is a supernet + of the passed in network the behavior wasn't consistent. this now returns + an AnsibleFilterError in that scenario across all python releases. + (https://github.com/ansible-collections/ansible.utils/issues/132) +minor_changes: + - >- + ipsubnet - the index parameter should only ever be an integer if it is + provided. this changes the argument type from str to int. diff --git a/docs/ansible.utils.ipsubnet_filter.rst b/docs/ansible.utils.ipsubnet_filter.rst index 12422ca..c73244f 100644 --- a/docs/ansible.utils.ipsubnet_filter.rst +++ b/docs/ansible.utils.ipsubnet_filter.rst @@ -40,7 +40,7 @@ Parameters index
- string + integer
diff --git a/plugins/filter/ipsubnet.py b/plugins/filter/ipsubnet.py index f127a36..85bedb4 100644 --- a/plugins/filter/ipsubnet.py +++ b/plugins/filter/ipsubnet.py @@ -24,6 +24,9 @@ from ansible_collections.ansible.utils.plugins.plugin_utils.base.ipaddr_utils im __metaclass__ = type +from ansible.module_utils._text import to_text + + try: from jinja2.filters import pass_environment except ImportError: @@ -69,7 +72,7 @@ DOCUMENTATION = """ description: | The second argument of the ipsubnet() filter is an index number; by specifying it you can get a new subnet with the specified index. - type: str + type: int notes: """ @@ -254,71 +257,88 @@ def _ipsubnet(*args, **kwargs): return ipsubnet(**updated_data) -def ipsubnet(value, query="", index="x"): +def ipsubnet(value, query="", index=None): """Manipulate IPv4/IPv6 subnets""" - try: - vtype = ipaddr(value, "type") - if vtype == "address": - v = ipaddr(value, "cidr") - elif vtype == "network": - v = ipaddr(value, "subnet") - - value = netaddr.IPNetwork(v) - except Exception: + vtype = ipaddr(value, "type") + if not vtype: return False - query_string = str(query) + elif vtype == "address": + v = ipaddr(value, "cidr") + elif vtype == "network": + v = ipaddr(value, "subnet") + value = netaddr.IPNetwork(v).cidr + vtype = ipaddr(value, "type") + if not query: - return str(value) + return to_text(value) - elif query_string.isdigit(): - vsize = ipaddr(v, "size") + vtotalbits = 128 if value.version == 6 else 32 + if query.isdigit(): query = int(query) + if query < 0 or query > vtotalbits: + return False - try: - float(index) - index = int(index) + if index is None: + if vtype == "address": + return to_text(value.supernet(query)[0]) + elif vtype == "network": + if query - value.prefixlen < 0: + msg = "Requested subnet size of {0} is invalid".format( + to_text(query), + ) + raise AnsibleFilterError(msg) + return to_text(2 ** (query - value.prefixlen)) - if vsize > 1: - try: - return str(list(value.subnet(query))[index]) - except Exception: - return False - - elif vsize == 1: - try: - return str(value.supernet(query)[index]) - except Exception: - return False - - except Exception: - if vsize > 1: - try: - return str(len(list(value.subnet(query)))) - except Exception: - return False - - elif vsize == 1: - try: - return str(value.supernet(query)[0]) - except Exception: - return False - - elif query_string: + index = int(index) + if vtype == "address": + if index > vtotalbits + 1 - index or index < query - vtotalbits: + return False + return to_text(value.supernet(query)[index]) + elif vtype == "network": + if index < 0: + index = index + 2 ** (query - value.prefixlen) + return to_text( + netaddr.IPNetwork( + to_text( + netaddr.IPAddress( + value.network.value + (index << vtotalbits - query), + ), + ) + + "/" + + to_text(query), + ), + ) + else: vtype = ipaddr(query, "type") if vtype == "address": v = ipaddr(query, "cidr") elif vtype == "network": v = ipaddr(query, "subnet") else: - msg = "You must pass a valid subnet or IP address; {0} is invalid".format(query_string) + msg = "You must pass a valid subnet or IP address; {0} is invalid".format( + to_text(query), + ) raise AnsibleFilterError(msg) query = netaddr.IPNetwork(v) - for i, subnet in enumerate(query.subnet(value.prefixlen), 1): - if subnet == value: - return str(i) + if ( + value.value >> vtotalbits - query.prefixlen + == query.value >> vtotalbits - query.prefixlen + ): + return to_text( + ( + ( + value.value + & 2 ** (value.prefixlen - query.prefixlen) - 1 + << (vtotalbits - value.prefixlen) + ) + >> (vtotalbits - value.prefixlen) + ) + + 1, + ) msg = "{0} is not in the subnet {1}".format(value.cidr, query.cidr) raise AnsibleFilterError(msg) + return False diff --git a/tests/integration/requirements.txt b/tests/integration/requirements.txt index 279789b..118f11d 100644 --- a/tests/integration/requirements.txt +++ b/tests/integration/requirements.txt @@ -1,3 +1,4 @@ jsonschema ; python_version >= '2.7' +netaddr ttp textfsm diff --git a/tests/integration/targets/utils_ipaddr_filter/tasks/ipsubnet.yaml b/tests/integration/targets/utils_ipaddr_filter/tasks/ipsubnet.yaml index a134c38..76740d3 100644 --- a/tests/integration/targets/utils_ipaddr_filter/tasks/ipsubnet.yaml +++ b/tests/integration/targets/utils_ipaddr_filter/tasks/ipsubnet.yaml @@ -62,3 +62,19 @@ - name: Assert result for ipsubnet. assert: that: "{{ result8 == '36870' }}" + +- name: Small subnets from a large supernet should return quickly + ansible.builtin.set_fact: + result9: "{{ '2001:db8::/56' | ansible.utils.ipsubnet(120, 0) }}" + +- name: Assert result for ipsubnet. + assert: + that: result9 == '2001:db8::/120' + +- name: Small subnets from a large supernet should return quickly at index + ansible.builtin.set_fact: + result10: "{{ '2001:db8::/56' | ansible.utils.ipsubnet(120, 4) }}" + +- name: Assert result for ipsubnet. + assert: + that: result10 == '2001:db8::400/120' diff --git a/tests/unit/plugins/filter/test_ipaddr.py b/tests/unit/plugins/filter/test_ipaddr.py index a310244..72783c1 100644 --- a/tests/unit/plugins/filter/test_ipaddr.py +++ b/tests/unit/plugins/filter/test_ipaddr.py @@ -11,7 +11,6 @@ from __future__ import absolute_import, division, print_function __metaclass__ = type -import sys import unittest import pytest @@ -498,7 +497,6 @@ class TestIpFilter(unittest.TestCase): def test_ipsubnet(self): test_cases = ( (("1.1.1.1/24", "30"), "64"), - (("1.1.1.1/25", "24"), "0"), (("1.12.1.34/32", "1.12.1.34/24"), "35"), (("192.168.50.0/24", "192.168.0.0/16"), "51"), (("192.168.144.5", "192.168.0.0/16"), "36870"), @@ -528,15 +526,13 @@ class TestIpFilter(unittest.TestCase): self._test_ipsubnet(args, res) def _test_ipsubnet(self, ipsubnet_args, expected_result): - if ( - ipsubnet_args == ("1.1.1.1/25", "24") - and expected_result == "0" - and sys.version_info >= (3, 7) - ): - return # fails in netaddr on Python 3.7+ - self.assertEqual(ipsubnet(*ipsubnet_args), expected_result) + expected = "Requested subnet size of 24 is invalid" + with self.assertRaises(AnsibleFilterError) as exc: + ipsubnet("1.1.1.1/25", "24") + self.assertEqual(exc.exception.message, expected) + with self.assertRaisesRegexp( AnsibleFilterError, "You must pass a valid subnet or IP address; invalid_subnet is invalid", diff --git a/tests/unit/plugins/filter/test_ipsubnet.py b/tests/unit/plugins/filter/test_ipsubnet.py index 6774d35..b790f60 100644 --- a/tests/unit/plugins/filter/test_ipsubnet.py +++ b/tests/unit/plugins/filter/test_ipsubnet.py @@ -73,6 +73,54 @@ class TestIpSubnet(unittest.TestCase): result = _ipsubnet(*args) self.assertEqual(result, "192.168.144.4/31") + def test_ipvsubnet_filter_lots_of_subnets_v4_bottom1(self): + """Use the subnets call to see if it slows way down with v4""" + args = ["", "0.0.0.0/1", "31", -1] + result = _ipsubnet(*args) + self.assertEqual(result, "127.255.255.254/31") + + def test_ipvsubnet_filter_lots_of_subnets_v4_bottom2(self): + """Use the subnets call to see if it slows way down with v4""" + args = ["", "1.0.0.0/1", "3", -1] + result = _ipsubnet(*args) + self.assertEqual(result, "96.0.0.0/3") + + def test_ipvsubnet_filter_lots_of_subnets_v4_top1(self): + """Use the subnets call to see if it slows way down with v4""" + args = ["", "128.0.0.0/1", "31", -1] + result = _ipsubnet(*args) + self.assertEqual(result, "255.255.255.254/31") + + def test_ipvsubnet_filter_lots_of_subnets_v4_top2(self): + """Use the subnets call to see if it slows way down with v4""" + args = ["", "130.0.0.0/1", "31", -1] + result = _ipsubnet(*args) + self.assertEqual(result, "255.255.255.254/31") + + def test_ipvsubnet_filter_lots_of_subnets_v6_bottom1(self): + """Use the subnets call to see if it slows way down with v6""" + args = ["", "8000::/1", "127", -1] + result = _ipsubnet(*args) + self.assertEqual(result, "ffff:ffff:ffff:ffff:ffff:ffff:ffff:fffe/127") + + def test_ipvsubnet_filter_lots_of_subnets_v6_bottom2(self): + """Use the subnets call to see if it slows way down with v6""" + args = ["", "9000::/1", "127", -1] + result = _ipsubnet(*args) + self.assertEqual(result, "ffff:ffff:ffff:ffff:ffff:ffff:ffff:fffe/127") + + def test_ipvsubnet_filter_lots_of_subnets_v6_top1(self): + """Use the subnets call to see if it slows way down with v6""" + args = ["", "::/1", "127", -1] + result = _ipsubnet(*args) + self.assertEqual(result, "7fff:ffff:ffff:ffff:ffff:ffff:ffff:fffe/127") + + def test_ipvsubnet_filter_lots_of_subnets_v6_top2(self): + """Use the subnets call to see if it slows way down with v6""" + args = ["", "1000::/1", "127", -1] + result = _ipsubnet(*args) + self.assertEqual(result, "7fff:ffff:ffff:ffff:ffff:ffff:ffff:fffe/127") + def test_ipvsubnet_filter_rank_address_in_subnet(self): """The rank of the IP in the subnet (the IP is the 36870nth /32 of the subnet)""" args = ["", address, subnet]