Skip to content

Commit

Permalink
Some fixes in ios_acls module: (#998)
Browse files Browse the repository at this point in the history 
* Some fixes in ios_acls module:

- Correctly take in count the case where we try to create an ACL with an
  ACL of another type already present.
  I have added the test `test_ios_acls_replaced_changetype` to test it.
- For the ACL without a sequence number, correctly take in count the
  fake ACL number.
  I have also added a test in the unit test test_ios_acls_replaced_idempotent`
  to validate that the test correctly take in count these ACE
- In unit test, the test `test_ios_acls_parsed_matches` was clearly with
  a wrong output format. Fixed it.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* [ios_vlans] remove extra configuration attribute (#1005)

* revert vlan extra parameters

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* add a changelog

* revert vlans test

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* resource segregate implementation

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* documentation update

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* add basic tests

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* minor doc changes

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* check command options

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* add purge

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix unused imports

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* update changelog

* add tests for vlan conf

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* update tests for vlan config purged

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix vlan config

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix tests

* update documentation

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Update docs 2

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* address review comments

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* update doc

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* update test-req

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Release 6.1.0 (#1010)

* - Create integration test for replaced change type
- Fixe one issue in ACL

* Some fixes in ios_acls module:

- Correctly take in count the case where we try to create an ACL with an
  ACL of another type already present.
  I have added the test `test_ios_acls_replaced_changetype` to test it.
- For the ACL without a sequence number, correctly take in count the
  fake ACL number.
  I have also added a test in the unit test test_ios_acls_replaced_idempotent`
  to validate that the test correctly take in count these ACE
- In unit test, the test `test_ios_acls_parsed_matches` was clearly with
  a wrong output format. Fixed it.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* - Create integration test for replaced change type
- Fixe one issue in ACL

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Sagar Paul <[email protected]>
Co-authored-by: Vinay M <[email protected]>
  • Loading branch information
@earendilfr @pre-commit-ci
@KB-perByte @roverflow
4 people authored Feb 14, 2024
1 parent f4084f2 commit 018bf84
Show file tree
Hide file tree
Showing 7 changed files with 196 additions and 27 deletions.
4 changes: 4 additions & 0 deletions changelogs/fragments/ios_acls_changes.yml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
bugfixes:
- ios_acls - take correctly in case where we want to push an ACL from a different type
- ios_acls - correctly match the different line for ACL without sequence number
29 changes: 25 additions & 4 deletions plugins/module_utils/network/ios/config/acls/acls.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,16 @@ def rearrange_cmds(aces):
for wname, wentry in iteritems(wplists):
hentry = hplists.pop(wname, {})
acl_type = wentry["acl_type"] if wentry.get("acl_type") else hentry.get("acl_type")
# If ACLs type is different between existing and wanted ACL, we need first remove it
if acl_type != hentry.get("acl_type", acl_type):
self.commands.append(
"no " + self.acl_name_cmd(wname, afi, hentry.get("acl_type", "")),
)
hentry.pop(
"aces",
{},
) # We remove ACEs because we have previously add a command to suppress completely the ACL

begin = len(self.commands) # to determine the index for acl command
self._compare_aces(
wentry.pop("aces", {}),
Expand Down Expand Up @@ -214,6 +224,16 @@ def pop_remark(r_entry, afi):
{"remarks": wrems, "sequence": hentry.get("sequence", "")},
"remarks",
)
else:
rem_hentry.get("remarks", {}).pop(k_wrems)
# We remove remarks that are not in the wentry for this ACE
for k_hrems, hrems in rem_hentry.get("remarks", {}).items():
self.addcmd(
{"remarks": hrems, "sequence": hentry.get("sequence", "")},
"remarks",
negate=True,
)

# add ace if not in have
if hentry != wentry:
self.addcmd(add_afi(wentry, afi), "aces")
Expand Down Expand Up @@ -262,7 +282,7 @@ def acl_name_cmd(self, name, afi, acl_type):
def list_to_dict(self, param):
"""converts list attributes to dict"""

temp, count = dict(), 0
temp = dict()
if param:
for each in param: # ipv4 and ipv6 acl
temp_acls = {}
Expand All @@ -271,7 +291,7 @@ def list_to_dict(self, param):
temp_aces = {}
if acl.get("aces"):
rem_idx = 0 # remarks if defined in an ace
for ace in acl.get("aces"): # each ace turned to dict
for count, ace in enumerate(acl.get("aces")): # each ace turned to dict
if (
ace.get("destination")
and ace.get("destination", {}).get(
Expand Down Expand Up @@ -311,13 +331,14 @@ def list_to_dict(self, param):
# temp_rem.extend(ace.pop("remarks"))
for remks in ace.get("remarks"):
rem_ace[remks.replace(" ", "_")] = remks
rem_idx += 1
ace["remarks"] = rem_ace

if ace.get("sequence"):
temp_aces.update({ace.get("sequence"): ace})
elif ace.get("remarks"):
temp_aces.update({"__{0}".format(rem_idx): ace})
rem_idx += 1
elif ace:
count += 1
temp_aces.update({"_" + to_text(count): ace})

# if temp_rem: # add remarks to the temp ace
Expand Down
20 changes: 10 additions & 10 deletions plugins/module_utils/network/ios/rm_templates/acls.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ def __init__(self, lines=None):
{
"name": "aces_ipv4_standard",
"getval": re.compile(
r"""(\s*(?P<sequence>\d+))?
r"""^\s*((?P<sequence>\d+))?
(\s(?P<grant>deny|permit))
(\s+(?P<address>((25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)))?
(\s(?P<wildcard>((25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)))?
Expand Down Expand Up @@ -281,17 +281,17 @@ def __init__(self, lines=None):
{
"name": "aces",
"getval": re.compile(
r"""(\s*(?P<sequence>\d+))?
(\s*sequence\s(?P<sequence_ipv6>\d+))?
(\s*(?P<grant>deny|permit))
r"""^\s*((?P<sequence>\d+))?
(\ssequence\s(?P<sequence_ipv6>\d+))?
(\s(?P<grant>deny|permit))
(\sevaluate\s(?P<evaluate>\S+))?
(\s(?P<protocol_num>\d+))?
(\s(?P<protocol_num>\d+)\s)?
(\s*(?P<protocol>ahp|eigrp|esp|gre|icmp|igmp|ipinip|ipv6|ip|nos|ospf|pcp|pim|sctp|tcp|ip|udp))?
((\s(?P<source_any>any))|
(\sobject-group\s(?P<source_obj_grp>\S+))|
(\shost\s(?P<source_host>\S+))|
(\s(?P<ipv6_source_address>\S+/\d+))|
(\s(?P<source_address>(\d{1,3}.\d{1,3}.\d{1,3}.\d{1,3})\s\S+)))?
((\s*(?P<source_any>any))|
(\s*object-group\s(?P<source_obj_grp>\S+))|
(\s*host\s(?P<source_host>\S+))|
(\s*(?P<ipv6_source_address>\S+/\d+))|
(\s*(?P<source_address>(\d{1,3}.\d{1,3}.\d{1,3}.\d{1,3})\s\S+)))?
(\seq\s(?P<seq>(\S+|\d+)))?
(\sgt\s(?P<sgt>(\S+|\d+)))?
(\slt\s(?P<slt>(\S+|\d+)))?
Expand Down
15 changes: 10 additions & 5 deletions tests/integration/targets/ios_acls/tests/cli/_populate_config.yaml
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
---
- name: Populate configuration
vars:
lines:
"ip access-list extended test_acl\ndeny tcp 192.0.2.0 0.0.0.255 192.0.3.0 0.0.0.255 eq www fin option traceroute ttl eq 10\nip access-list extended 110\n\
deny icmp 192.0.2.0 0.0.0.255 192.0.3.0 0.0.0.255 echo dscp ef ttl eq 10\nip access-list extended 123\ndeny tcp 198.51.100.0 0.0.0.255 198.51.101.0 0.0.0.255\
\ eq telnet ack tos 12\ndeny tcp 192.0.3.0 0.0.0.255 192.0.4.0 0.0.0.255 eq www ack dscp ef ttl lt 20\nipv6 access-list R1_TRAFFIC\ndeny tcp any eq www any\
\ eq telnet ack dscp af11\n"
lines: |
ip access-list extended test_acl
deny tcp 192.0.2.0 0.0.0.255 192.0.3.0 0.0.0.255 eq www fin option traceroute ttl eq 10
ip access-list extended 110
deny icmp 192.0.2.0 0.0.0.255 192.0.3.0 0.0.0.255 echo dscp ef ttl eq 10
ip access-list extended 123
deny tcp 198.51.100.0 0.0.0.255 198.51.101.0 0.0.0.255 eq telnet ack tos 12
deny tcp 192.0.3.0 0.0.0.255 192.0.4.0 0.0.0.255 eq www ack dscp ef ttl lt 20
ipv6 access-list R1_TRAFFIC
deny tcp any eq www any eq telnet ack dscp af11
ansible.netcommon.cli_config:
config: "{{ lines }}"
35 changes: 35 additions & 0 deletions tests/integration/targets/ios_acls/tests/cli/replaced.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,5 +87,40 @@
that:
- result.commands|length == 0
- result['changed'] == false

- ansible.builtin.include_tasks: _remove_config.yaml

- ansible.builtin.include_tasks: _populate_config.yaml

- name: Replaces device configuration of ACL with provided configuration
register: result
cisco.ios.ios_acls: &id002
config:
- afi: ipv4
acls:
- name: test_acl
acl_type: standard
aces:
- grant: deny
sequence: 10
source:
address: 198.51.100.0
wildcard_bits: 0.0.0.255
state: replaced

- ansible.builtin.assert:
that:
- result.commands|length == 3
- result.changed == true
- result.commands|symmetric_difference(replaced_changetype.commands) == []

- name: Replaces device configuration of ACL with provided configuration (idempotent)
register: result
cisco.ios.ios_acls: *id002
- name: Assert that task was idempotent
ansible.builtin.assert:
that:
- result.commands|length == 0
- result['changed'] == false
always:
- ansible.builtin.include_tasks: _remove_config.yaml
5 changes: 5 additions & 0 deletions tests/integration/targets/ios_acls/vars/main.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ replaced:
- ip access-list extended 150
- 20 deny tcp 198.51.100.0 0.0.0.255 eq telnet 198.51.110.0 0.0.0.255 eq telnet
syn dscp ef ttl eq 10
replaced_changetype:
commands:
- no ip access-list extended test_acl
- ip access-list standard test_acl
- 10 deny 198.51.100.0 0.0.0.255
overridden:
commands:
- no ipv6 access-list R1_TRAFFIC
Expand Down
115 changes: 107 additions & 8 deletions tests/unit/modules/network/ios/test_ios_acls.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -645,9 +645,21 @@ def test_ios_acls_replaced_idempotent(self):
ip access-list extended test-idem
10 permit ip host 10.153.14.21 any
20 permit ip host 10.153.14.22 any
ip access-list standard test-acl-no-seq
permit 10.0.0.0 0.255.255.255
permit 172.31.16.0 0.0.7.255
""",
)
self.execute_show_command_name.return_value = dedent(
"""\
Standard IP access list test_acl
Standard IP access list test-acl-no-seq
Extended IP access list 110
Extended IP access list test-idem
Extended IP access list test_pre
IPv6 access list R1_TRAFFIC
""",
)
self.execute_show_command_name.return_value = dedent("")
set_module_args(
dict(
config=[
Expand Down Expand Up @@ -734,6 +746,26 @@ def test_ios_acls_replaced_idempotent(self):
},
],
},
{
"name": "test-acl-no-seq",
"acl_type": "standard",
"aces": [
{
"grant": "permit",
"source": {
"address": "10.0.0.0",
"wildcard_bits": "0.255.255.255",
},
},
{
"grant": "permit",
"source": {
"address": "172.31.16.0",
"wildcard_bits": "0.0.7.255",
},
},
],
},
],
},
{
Expand Down Expand Up @@ -768,6 +800,65 @@ def test_ios_acls_replaced_idempotent(self):
result = self.execute_module(changed=False)
self.assertEqual(sorted(result["commands"]), [])

def test_ios_acls_replaced_changetype(self):
self.execute_show_command.return_value = dedent(
"""\
ip access-list extended 110
10 permit tcp 198.51.100.0 0.0.0.255 any eq 22 log (tag = testLog)
20 deny icmp 192.0.2.0 0.0.0.255 192.0.3.0 0.0.0.255 echo dscp ef ttl eq 10
30 deny icmp object-group test_network_og any dscp ef ttl eq 10
ip access-list standard test_acl
remark remark check 1
remark some random remark 2
""",
)
self.execute_show_command_name.return_value = dedent(
"""\
Standard IP access list test_acl
""",
)
set_module_args(
dict(
config=[
dict(
afi="ipv4",
acls=[
dict(
name="110",
acl_type="standard",
aces=[
dict(
grant="deny",
source=dict(
address="198.51.100.0",
wildcard_bits="0.0.0.255",
),
),
],
),
dict(
name="test_acl",
acl_type="standard",
aces=[dict(remarks=["Another remark here"])],
),
],
),
],
state="replaced",
),
)
result = self.execute_module(changed=True)
commands = [
"no ip access-list extended 110",
"ip access-list standard 110",
"deny 198.51.100.0 0.0.0.255",
"ip access-list standard test_acl",
"no remark remark check 1",
"no remark some random remark 2",
"remark Another remark here",
]
self.assertEqual(sorted(result["commands"]), sorted(commands))

def test_ios_acls_overridden(self):
self.execute_show_command.return_value = dedent(
"""\
Expand Down Expand Up @@ -1131,11 +1222,13 @@ def test_ios_acls_parsed(self):
def test_ios_acls_parsed_matches(self):
self.execute_show_command_name.return_value = dedent("")
set_module_args(
dict(
running_config="""ip access-list standard R1_TRAFFIC\n10 permit 10.11.12.13 (2 matches)\n
40 permit 128.0.0.0, wildcard bits 63.255.255.255 (2 matches)\n60 permit 134.107.136.0, wildcard bits 0.0.0.255 (1 match)""",
state="parsed",
),
{
"running_config": """ip access-list standard R1_TRAFFIC
10 permit 10.11.12.13
40 permit 128.0.0.0 63.255.255.255
60 permit 134.107.136.0 0.0.0.255""",
"state": "parsed",
},
)
result = self.execute_module(changed=False)
parsed_list = [
Expand All @@ -1154,12 +1247,18 @@ def test_ios_acls_parsed_matches(self):
{
"sequence": 40,
"grant": "permit",
"protocol_options": {"protocol_number": 128},
"source": {
"address": "128.0.0.0",
"wildcard_bits": "63.255.255.255",
},
},
{
"sequence": 60,
"grant": "permit",
"protocol_options": {"protocol_number": 134},
"source": {
"address": "134.107.136.0",
"wildcard_bits": "0.0.0.255",
},
},
],
},
Expand Down

0 comments on commit 018bf84

Please sign in to comment.