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

To fix IOS ACLs and Prefix Lists RM to not negate config when using merge state #346

Merged
merged 6 commits into from Jun 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -0,0 +1,3 @@
---
minor_changes:
- To update prefix list and acls merge behaviour and update prefix list description position in model (https://github.com/ansible-collections/cisco.ios/issues/345).
Expand Up @@ -42,6 +42,7 @@ class Prefix_listsArgs(object):
"elements": "dict",
"options": {
"name": {"type": "str"},
"description": {"type": "str"},
"entries": {
"type": "list",
"elements": "dict",
Expand All @@ -50,8 +51,8 @@ class Prefix_listsArgs(object):
"type": "str",
"choices": ["deny", "permit"],
},
"description": {"type": "str"},
"sequence": {"type": "int"},
"description": {"type": "str"},
"prefix": {"type": "str"},
"ge": {"type": "int"},
"le": {"type": "int"},
Expand Down
9 changes: 9 additions & 0 deletions plugins/module_utils/network/ios/config/acls/acls.py
Expand Up @@ -143,6 +143,15 @@ def _compare(self, want, have):
):
have_ace.pop("protocol")
if have_ace and val != have_ace:
if self.state == "merged" and have_ace.get(
"sequence"
) == val.get("sequence"):
self._module.fail_json(
"Cannot update existing sequence {0} of ACLs {1} with state merged.".format(
val.get("sequence"), k
)
+ " Please use state replaced or overridden."
)
self.compare(
parsers=self.parsers,
want=dict(),
Expand Down
159 changes: 98 additions & 61 deletions plugins/module_utils/network/ios/config/prefix_lists/prefix_lists.py
Expand Up @@ -127,12 +127,39 @@ def _compare(self, want, have):
"""
if want != have and self.state != "deleted":
for k, v in iteritems(want["prefix_lists"]):
if have.get("prefix_lists"):
if have.get("prefix_lists") and have["prefix_lists"].get(k):
have_prefix = have["prefix_lists"].pop(k, {})
for key, val in iteritems(v):
have_prefix_param = have_prefix.pop(key, {})
for key, val in iteritems(v.get("entries")):
if have_prefix.get("entries"):
have_prefix_param = have_prefix["entries"].pop(
key, {}
)
else:
have_prefix_param = None
if have_prefix.get("description"):
self.compare(
parsers=self.parsers,
want={
"afi": want["afi"],
"name": k,
"prefix_list": {
"description": v["description"]
},
},
have={
"afi": want["afi"],
"name": k,
"prefix_list": {
"description": have_prefix.pop(
"description"
)
},
},
)
if have_prefix_param and val != have_prefix_param:
if key == "description":
# Code snippet should be removed when Description param is removed from
# entries level as this supports deprecated level of Description
self.compare(
parsers=self.parsers,
want={
Expand All @@ -149,6 +176,17 @@ def _compare(self, want, have):
},
)
else:
if self.state == "merged" and have_prefix_param.get(
"sequence"
) == val.get(
"sequence"
):
self._module.fail_json(
"Cannot update existing sequence {0} of Prefix Lists {1} with state merged.".format(
val.get("sequence"), k
)
+ " Please use state replaced or overridden."
)
self.compare(
parsers=self.parsers,
want=dict(),
Expand All @@ -172,72 +210,67 @@ def _compare(self, want, have):
},
)
elif val and val != have_prefix_param:
if key == "description":
self.compare(
parsers=self.parsers,
want={
"afi": want["afi"],
"name": k,
"prefix_list": {key: val},
},
have=dict(),
)
else:
self.compare(
parsers=self.parsers,
want={
"afi": want["afi"],
"name": k,
"prefix_list": val,
},
have=dict(),
)
if have_prefix and (
self.state == "replaced" or self.state == "overridden"
):
for key, val in iteritems(have_prefix):
if key == "description":
self.compare(
parsers=self.parsers,
want=dict(),
have={
"afi": have["afi"],
"name": k,
"prefix_list": val,
},
)
else:
self.compare(
parsers=self.parsers,
want=dict(),
have={
"afi": have["afi"],
"name": k,
"prefix_list": val,
},
)
elif v:
for key, val in iteritems(v):
if key == "description":
self.compare(
parsers=self.parsers,
want={
"afi": want["afi"],
"name": k,
"prefix_list": {key: val},
"prefix_list": val,
},
have=dict(),
)
else:
if have_prefix and (
self.state == "replaced" or self.state == "overridden"
):
if have_prefix.get("description"):
# Code snippet should be removed when Description param is removed from
# entries level as this supports deprecated level of Description
self.compare(
parsers=self.parsers,
want={
want=dict(),
have={
"afi": want["afi"],
"name": k,
"prefix_list": {
"description": have_prefix[
"description"
]
},
},
)
for key, val in iteritems(have_prefix.get("entries")):
self.compare(
parsers=self.parsers,
want=dict(),
have={
"afi": have["afi"],
"name": k,
"prefix_list": val,
},
have=dict(),
)
elif v:
if v.get("description"):
self.compare(
parsers=self.parsers,
want={
"afi": want["afi"],
"name": k,
"prefix_list": {
"description": v["description"]
},
},
have=dict(),
)
for key, val in iteritems(v.get("entries")):
self.compare(
parsers=self.parsers,
want={
"afi": want["afi"],
"name": k,
"prefix_list": val,
},
have=dict(),
)

def list_to_dict(self, param):
if param:
Expand All @@ -248,11 +281,15 @@ def list_to_dict(self, param):
temp_entries = dict()
if each.get("entries"):
for every in each["entries"]:
if every.get("description"):
temp_entries.update(every)
else:
temp_entries.update(
{str(every["sequence"]): every}
)
temp_prefix_list.update({each["name"]: temp_entries})
temp_entries.update(
{str(every["sequence"]): every}
)
temp_prefix_list.update(
{
each["name"]: {
"description": each.get("description"),
"entries": temp_entries,
}
}
)
val["prefix_lists"] = temp_prefix_list
Expand Up @@ -89,7 +89,14 @@ def populate_facts(self, connection, ansible_facts, data=None):
for each in v["prefix_lists"]:
if not temp_prefix_list.get("name"):
temp_prefix_list["name"] = each["name"]
temp_prefix_list["entries"].append(each["entries"])
if not temp_prefix_list.get("description") and each.get(
"description"
):
temp_prefix_list["description"] = each["description"]
if each["entries"] and not each["entries"].get(
"description"
):
temp_prefix_list["entries"].append(each["entries"])
temp["prefix_lists"].append(temp_prefix_list)
if temp and temp["afi"]:
temp["prefix_lists"] = sorted(
Expand Down
28 changes: 16 additions & 12 deletions plugins/module_utils/network/ios/rm_templates/prefix_lists.py
Expand Up @@ -25,18 +25,19 @@ def _tmplt_set_prefix_lists(config_data):
if config_data.get("afi") == "ipv4":
config_data["afi"] = "ip"
cmd = "{afi} prefix-list {name}".format(**config_data)
if config_data["prefix_list"].get("description"):
cmd += " description {description}".format(
**config_data["prefix_list"]
)
else:
cmd += " seq {sequence} {action} {prefix}".format(
**config_data["prefix_list"]
)
if config_data["prefix_list"].get("ge"):
cmd += " ge {ge}".format(**config_data["prefix_list"])
if config_data["prefix_list"].get("le"):
cmd += " le {le}".format(**config_data["prefix_list"])
if config_data.get("prefix_list"):
if config_data["prefix_list"].get("description"):
cmd += " description {description}".format(
**config_data["prefix_list"]
)
else:
cmd += " seq {sequence} {action} {prefix}".format(
**config_data["prefix_list"]
)
if config_data["prefix_list"].get("ge"):
cmd += " ge {ge}".format(**config_data["prefix_list"])
if config_data["prefix_list"].get("le"):
cmd += " le {le}".format(**config_data["prefix_list"])
return cmd


Expand Down Expand Up @@ -68,7 +69,10 @@ def __init__(self, lines=None):
"prefix_lists": [
{
"name": "{{ name if name is defined }}",
"description": "{{ description.split('description ')[1] if description is defined }}",
"entries": {
# Description at this level is deprecated, should be removed when we plan to remove the
# Description from entries level
"description": "{{ description.split('description ')[1] if description is defined }}",
"sequence": "{{ sequence.split(' ')[1] if sequence is defined }}",
"action": "{{ action if action is defined }}",
Expand Down
34 changes: 33 additions & 1 deletion plugins/modules/ios_acls.py
Expand Up @@ -612,6 +612,10 @@
default: merged
description:
- The state the configuration should be left in
- The states I(merged) is the default state which merges the want and have config, but
for ACL module as the IOS platform doesn't allow update of ACE over an pre-existing ACE
sequence in ACL, same way ACLs resource module will error out for respective scenario
and only addition of new ACE over new sequence will be allowed with merge state.
- The states I(rendered), I(gathered) and I(parsed) does not perform any change
on the device.
- The state I(rendered) will transform the configuration in C(config) option to
Expand All @@ -632,6 +636,33 @@
EXAMPLES = """
# Using merged

# Before state:
# -------------
#
# vios#sh access-lists
# Extended IP access list 100
# 10 deny icmp 192.0.2.0 0.0.0.255 192.0.3.0 0.0.0.255 echo dscp ef ttl eq 10

- name: Merge provided configuration with device configuration
cisco.ios.ios_acls:
config:
- afi: ipv4
acls:
- name: 100
aces:
- sequence: 10
protocol_options:
icmp:
traceroute: true
state: merged

# After state:
# ------------
#
# Play Execution fails, with error:
# Cannot update existing sequence 10 of ACLs 100 with state merged.
# Please use state replaced or overridden.

# Before state:
# -------------
#
Expand Down Expand Up @@ -746,7 +777,6 @@
# - deny 192.168.1.200
# - deny 192.168.2.0 0.0.0.255
# - ip access-list extended 110
# - no 10
# - 10 deny icmp 192.0.2.0 0.0.0.255 192.0.3.0 0.0.0.255 traceroute dscp ef ttl eq 10
# - deny tcp host 198.51.100.0 host 198.51.110.0 eq telnet ack
# - ip access-list extended test
Expand All @@ -764,6 +794,8 @@
# Standard IP access list std_acl
# 10 deny 192.168.1.200
# 20 deny 192.168.2.0, wildcard bits 0.0.0.255
# Extended IP access list 100
# 10 deny icmp 192.0.2.0 0.0.0.255 192.0.3.0 0.0.0.255 echo dscp ef ttl eq 10
# Extended IP access list 110
# 10 deny icmp 192.0.2.0 0.0.0.255 192.0.3.0 0.0.0.255 traceroute dscp ef ttl eq 10
# 20 deny tcp host 198.51.100.0 host 198.51.110.0 eq telnet ack
Expand Down