Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ios_acls] add fix to standalone remarks #1044

Merged
merged 8 commits into from
Mar 27, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 46 additions & 10 deletions plugins/module_utils/network/ios/facts/acls/acls.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@

from ansible.module_utils._text import to_text
from ansible.module_utils.six import iteritems
from ansible_collections.ansible.netcommon.plugins.module_utils.network.common import utils
from ansible_collections.ansible.netcommon.plugins.module_utils.network.common import (
utils,
)
from ansible_collections.ansible.netcommon.plugins.module_utils.network.common.rm_base.network_template import (
NetworkTemplate,
)
Expand Down Expand Up @@ -105,7 +107,9 @@ def populate_facts(self, connection, ansible_facts, data=None):

if namedata:
# parse just names to update empty acls
templateObjName = NetworkTemplate(lines=namedata.splitlines(), tmplt=AclsTemplate())
templateObjName = NetworkTemplate(
lines=namedata.splitlines(), tmplt=AclsTemplate()
)
raw_acl_names = templateObjName.parse()
raw_acls = self.populate_empty_acls(raw_acls, raw_acl_names)

Expand All @@ -114,7 +118,10 @@ def populate_facts(self, connection, ansible_facts, data=None):

if raw_acls.get("acls"):
for k, v in iteritems(raw_acls.get("acls")):
if v.get("afi") == "ipv4" and v.get("acl_type") in ["standard", "extended"]:
if v.get("afi") == "ipv4" and v.get("acl_type") in [
"standard",
"extended",
]:
del v["afi"]
temp_v4.append(v)
elif v.get("afi") == "ipv6":
Expand Down Expand Up @@ -142,10 +149,14 @@ def factor_source_dest(ace, typ):
def process_protocol_options(each):
for each_ace in each.get("aces"):
if each.get("acl_type") == "standard":
if len(each_ace.get("source", {})) == 1 and each_ace.get("source", {}).get(
if len(each_ace.get("source", {})) == 1 and each_ace.get(
"source", {}
).get(
"address",
):
each_ace["source"]["host"] = each_ace["source"].pop("address")
each_ace["source"]["host"] = each_ace["source"].pop(
"address"
)
if each_ace.get("source", {}).get("address"):
addr = each_ace.get("source", {}).get("address")
if addr[-1] == ",":
Expand All @@ -159,7 +170,9 @@ def process_protocol_options(each):
if each_ace.get("icmp_igmp_tcp_protocol"):
each_ace["protocol_options"] = {
each_ace["protocol"]: {
each_ace.pop("icmp_igmp_tcp_protocol").replace("-", "_"): True,
each_ace.pop("icmp_igmp_tcp_protocol").replace(
"-", "_"
): True,
},
}
if each_ace.get("protocol_number"):
Expand All @@ -172,13 +185,21 @@ def collect_remarks(aces):
ace_entry = []
ace_rem = []
rem = {}
# every remarks is one list item which has a sequence number
# every ace remark is preserved and ordered
# at the end of each sequence it is flushed to a ace entry
for i in aces:
# i here denotes an ace, which would be populated with remarks entries
if i.get("is_remark_for"):
if not rem.get(i.get("is_remark_for")):
rem[i.get("is_remark_for")] = {"remarks": []}
rem[i.get("is_remark_for")]["remarks"].append(i.get("the_remark"))
rem[i.get("is_remark_for")]["remarks"].append(
i.get("the_remark")
)
else:
rem[i.get("is_remark_for")]["remarks"].append(i.get("the_remark"))
rem[i.get("is_remark_for")]["remarks"].append(
i.get("the_remark")
)
else:
if rem:
if rem.get(i.get("sequence")):
Expand All @@ -187,12 +208,27 @@ def collect_remarks(aces):
ace_entry.append(i)

if rem: # pending remarks
pending_rem = rem.get("remark")
ace_entry.append({"remarks": pending_rem.get("remarks")})
for pending_rem_seq, pending_rem_val in rem.items():
# there can be ace entry with just a remarks and no ace actually
# 10 remarks I am a remarks
# 20 ..... so onn
if pending_rem_seq != "remark":
ace_entry.append(
{
"sequence": pending_rem_seq,
"remarks": pending_rem_val.get("remarks"),
}
)
else:
# this handles the classic set of remarks at the end, which is not tied to
# any sequence number
pending_rem = rem.get("remark", {})
ace_entry.append({"remarks": pending_rem.get("remarks")})
return ace_entry

for each in temp_v4:
if each.get("aces"):
# handling remarks for each ace entry
each["aces"] = collect_remarks(each.get("aces"))
process_protocol_options(each)

Expand Down
4 changes: 3 additions & 1 deletion plugins/modules/ios_acls.py
Original file line number Diff line number Diff line change
Expand Up @@ -2784,7 +2784,9 @@
from ansible_collections.cisco.ios.plugins.module_utils.network.ios.argspec.acls.acls import (
AclsArgs,
)
from ansible_collections.cisco.ios.plugins.module_utils.network.ios.config.acls.acls import Acls
from ansible_collections.cisco.ios.plugins.module_utils.network.ios.config.acls.acls import (
Acls,
)


def main():
Expand Down
67 changes: 63 additions & 4 deletions tests/unit/modules/network/ios/test_ios_acls.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ def setUp(self):
"ansible_collections.ansible.netcommon.plugins.module_utils.network.common.rm_base.resource_module_base."
"get_resource_connection",
)
self.get_resource_connection_facts = self.mock_get_resource_connection_facts.start()
self.get_resource_connection_facts = (
self.mock_get_resource_connection_facts.start()
)

self.mock_execute_show_command = patch(
"ansible_collections.cisco.ios.plugins.module_utils.network.ios.facts.acls.acls."
Expand All @@ -38,7 +40,9 @@ def setUp(self):
"ansible_collections.cisco.ios.plugins.module_utils.network.ios.facts.acls.acls."
"AclsFacts.get_acl_names",
)
self.execute_show_command_name = self.mock_execute_show_command_name_specific.start()
self.execute_show_command_name = (
self.mock_execute_show_command_name_specific.start()
)

def tearDown(self):
super(TestIosAclsModule, self).tearDown()
Expand Down Expand Up @@ -1638,6 +1642,11 @@ def test_ios_acls_parsed_multioption(self):
dict(
running_config=dedent(
"""\
ip access-list standard 99
10 remark standalone remarks
20 permit 192.15.0.1
30 permit 192.15.0.2
40 permit 192.15.0.3
ip access-list standard 2
30 permit 172.16.1.11
20 permit 172.16.1.10
Expand Down Expand Up @@ -1769,6 +1778,28 @@ def test_ios_acls_parsed_multioption(self):
},
],
},
{
"name": "99",
"acl_type": "standard",
"aces": [
{
"sequence": 20,
"grant": "permit",
"source": {"host": "192.15.0.1"},
},
{
"sequence": 30,
"grant": "permit",
"source": {"host": "192.15.0.2"},
},
{
"sequence": 40,
"grant": "permit",
"source": {"host": "192.15.0.3"},
},
{"sequence": 10, "remarks": ["standalone remarks"]},
],
},
{
"name": "outboundfilters",
"acl_type": "extended",
Expand All @@ -1785,7 +1816,7 @@ def test_ios_acls_parsed_multioption(self):
"address": "172.16.1.0",
"wildcard_bits": "0.0.0.255",
},
},
}
],
},
{
Expand Down Expand Up @@ -1830,7 +1861,7 @@ def test_ios_acls_parsed_multioption(self):
],
},
],
},
}
]
self.assertEqual(parsed_list, result["parsed"])

Expand Down Expand Up @@ -1922,6 +1953,28 @@ def test_ios_acls_rendered_muiltioption(self):
},
],
},
{
"name": "99",
"acl_type": "standard",
"aces": [
{
"sequence": 20,
"grant": "permit",
"source": {"host": "192.15.0.1"},
},
{
"sequence": 30,
"grant": "permit",
"source": {"host": "192.15.0.2"},
},
{
"sequence": 40,
"grant": "permit",
"source": {"host": "192.15.0.3"},
},
{"sequence": 10, "remarks": ["standalone remarks"]},
],
},
{
"name": "2",
"acl_type": "standard",
Expand Down Expand Up @@ -2019,6 +2072,12 @@ def test_ios_acls_rendered_muiltioption(self):
"50 permit ip any 10.1.1.0 0.0.0.255",
"60 permit tcp any host 10.1.1.1 eq telnet",
"70 permit tcp 10.1.1.0 0.0.0.255 172.16.1.0 0.0.0.255 eq telnet time-range EVERYOTHERDAY",
"ip access-list standard 99",
"20 permit host 192.15.0.1",
"30 permit host 192.15.0.2",
"40 permit host 192.15.0.3",
"remark standalone remarks",
"10", # needs fix
KB-perByte marked this conversation as resolved.
Show resolved Hide resolved
"ip access-list standard 2",
"30 permit host 172.16.1.11",
"20 permit host 172.16.1.10",
Expand Down
Loading