Skip to content

Commit

Permalink
[acls] fixes aces rendering, overridden and replaced state operations. (
Browse files Browse the repository at this point in the history 
#929)

* acls fixes

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

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

* check tests

* fix parsed

* fix action state bug

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

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

* remove extra test

* add tests

* fix ansible-lint

* add a changelog

* format test

* fix lint

* lint fix lint

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

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

* lgtm

* remove comments

* fix range as port_protocol issue

* bump ansible version

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

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

* the requires_ansible version

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

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

* fix sanity

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
@KB-perByte @pre-commit-ci
KB-perByte and pre-commit-ci[bot] authored Sep 22, 2023
1 parent fefe151 commit 2001633
Show file tree
Hide file tree
Showing 12 changed files with 105 additions and 44 deletions.
5 changes: 5 additions & 0 deletions .ansible-lint
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
profile: production

exclude_paths:
- changelogs/changelog.yaml
12 changes: 12 additions & 0 deletions .github/workflows/lint.yml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
name: ansible-lint
on: # yamllint disable-line rule:truthy
pull_request:
branches: ["main"]
jobs:
build:
name: Ansible Lint
runs-on: ubuntu-latest
steps:
- name: Run ansible-lint
uses: ansible/ansible-lint@main
4 changes: 0 additions & 4 deletions .github/workflows/tests.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ on: # yamllint disable-line rule:truthy


jobs:
ansible-lint:
uses: ansible-network/github_actions/.github/workflows/ansible-lint.yml@main
changelog:
uses: ansible-network/github_actions/.github/workflows/changelog.yml@main
if: github.event_name != 'schedule'
Expand All @@ -32,7 +30,6 @@ jobs:
all_green:
if: ${{ always() && (github.event_name != 'schedule') }}
needs:
- ansible-lint
- changelog
- sanity
- unit-galaxy
Expand All @@ -42,7 +39,6 @@ jobs:
- run: >-
python -c "assert 'failure' not in
set([
'${{ needs.ansible-lint.result }}',
'${{ needs.changelog.result }}',
'${{ needs.sanity.result }}',
'${{ needs.unit-galaxy.result }}',
Expand Down
2 changes: 1 addition & 1 deletion README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ This collection has been tested against Cisco IOS XE Version 17.3 on CML.
<!--start requires_ansible-->
## Ansible version compatibility

This collection has been tested against following Ansible versions: **>=2.9.10**.
This collection has been tested against following Ansible versions: **>=2.13.11**.

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`).
Expand Down
6 changes: 6 additions & 0 deletions changelogs/fragments/ios_acls_fix.yaml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
doc_changes:
- "ios_command - Fix formatting of examples."
bugfixes:
- "ios_acls - Fix standard acls rendering."
- "ios_acls - Fix protocol_options rendering corrects processing of overridden/ replaced state."
2 changes: 1 addition & 1 deletion docs/cisco.ios.ios_command_module.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ Examples
- name: Run show version on remote devices
cisco.ios.ios_command:
commands: show version'
commands: show version
# output-
Expand Down
2 changes: 1 addition & 1 deletion meta/runtime.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,4 +206,4 @@ plugin_routing:
redirect: cisco.ios.ios_vlans
vrf:
redirect: cisco.ios.ios_vrf
requires_ansible: ">=2.9.10"
requires_ansible: ">=2.13.11"
13 changes: 10 additions & 3 deletions plugins/module_utils/network/ios/config/acls/acls.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ def sanitize_protocol_options(self, wace, hace):
list(wace.get("protocol_options"))[0] == hace.get("protocol")
):
hace.pop("protocol")
hace["protocol_options"] = wace.get("protocol_options")
return hace

def acl_name_cmd(self, name, afi, acl_type):
Expand Down Expand Up @@ -226,9 +227,15 @@ def list_to_dict(self, param):
if acl.get("aces"):
temp_rem = [] # remarks if defined in an ace
for ace in acl.get("aces"): # each ace turned to dict
if ace.get("destination") and ace.get("destination", {}).get(
"port_protocol",
{},
if (
ace.get("destination")
and ace.get("destination", {}).get(
"port_protocol",
{},
)
and not ace.get("destination", {})
.get("port_protocol", {})
.get("range")
):
for k, v in (
ace.get("destination", {}).get("port_protocol", {}).items()
Expand Down
55 changes: 33 additions & 22 deletions plugins/module_utils/network/ios/rm_templates/acls.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ def __init__(self, lines=None):
r"""^(ip|ipv6)
(\s(access-list))
(\s(standard|extended))
(\s(?P<acl_name_r>\S+))?
(\s(?P<acl_name_r>\S+))
$""",
re.VERBOSE,
),
Expand All @@ -159,13 +159,12 @@ def __init__(self, lines=None):
"shared": True,
},
{
"name": "_mac_acls_name", #
"name": "_mac_acls_name", # mac acls to be removed
"getval": re.compile(
r"""^(?P<acl_type>Standard|Extended|Reflexive)*
\s*(?P<afi>MAC)*
\s*access
\s*list*
\s*(?P<acl_name>.+)*
r"""^(?P<acl_type>Standard|Extended|Reflexive)
(\s(?P<afi>MAC))
(\saccess\slist)
(\s(?P<acl_name>.+))
$""",
re.VERBOSE,
),
Expand Down Expand Up @@ -204,8 +203,8 @@ def __init__(self, lines=None):
"name": "remarks_type_linear",
"getval": re.compile(
r"""^(access-list)
(\s(?P<acl_name_linear>\S+))?
(\sremark\s(?P<remarks>.+))?
(\s(?P<acl_name_linear>\S+))
(\sremark\s(?P<remarks>.+))
$""",
re.VERBOSE,
),
Expand All @@ -224,7 +223,7 @@ def __init__(self, lines=None):
"getval": re.compile(
r"""\s*(?P<sequence>\d+)*
\s(?P<grant>deny|permit)?
(\s+(?P<address>(?!ahp|eigrp|esp|gre|icmp|igmp|ipv6|ipinip|ip|nos|object-group|ospf|pcp|pim|sctp|tcp|udp)\S+|\S+,))?
(\s+(?P<address>(?!ahp|any|eigrp|esp|gre|icmp|igmp|ipv6|ipinip|ip|nos|object-group|ospf|pcp|pim|sctp|tcp|udp)\S+|\S+,))?
(\s*(?P<any>any))?
(\swildcard\sbits\s(?P<wildcard>\S+))?
(\shost\s(?P<host>\S+))?
Expand Down Expand Up @@ -266,19 +265,27 @@ def __init__(self, lines=None):
(\sobject-group\s(?P<source_obj_grp>\S+))|
(\shost\s(?P<source_host>\S+))|
(\s(?P<source_address>(\d{1,3}.\d{1,3}.\d{1,3}.\d{1,3})\s\S+)))?
(\s(?P<source_port_protocol>(eq|gts|gt|lt|neq)\s(\S+|\d+)))?
(\seq\s(?P<seq>(\S+|\d+)))?
(\sgt\s(?P<sgt>(\S+|\d+)))?
(\slt\s(?P<slt>(\S+|\d+)))?
(\sneq\s(?P<sneq>(\S+|\d+)))?
(\srange\s(?P<srange_start>\d+)\s(?P<srange_end>\d+))?
(\s(?P<dest_any>any))?
(\sobject-group\s(?P<dest_obj_grp>\S+))?
(\shost\s(?P<dest_host>\S+))?
(\s(?P<dest_address>(\d{1,3}.\d{1,3}.\d{1,3}.\d{1,3})\s\S+))?
(\s(?P<dest_port_protocol>(eq|gts|lt|neq)\s(\S+|\d+)))?
(\seq\s(?P<deq>(\S+|\d+)))?
(\sgt\s(?P<dgt>(\S+|\d+)))?
(\slt\s(?P<dlt>(\S+|\d+)))?
(\sneq\s(?P<dneq>(\S+|\d+)))?
(\srange\s(?P<drange_start>\d+)\s(?P<drange_end>\d+))?
(\s(?P<icmp_igmp_tcp_protocol>administratively-prohibited|alternate-address|conversion-error|dod-host-prohibited|dod-net-prohibited|echo-reply|echo|general-parameter-problem|host-isolated|host-precedence-unreachable|host-redirect|host-tos-redirect|host-tos-unreachable|host-unknown|host-unreachable|information-reply|information-request|mask-reply|mask-request|mobile-redirect|net-redirect|net-tos-redirect|net-tos-unreachable|net-unreachable|network-unknown|no-room-for-option|option-missing|packet-too-big|parameter-problem|port-unreachable|precedence-unreachable|protocol-unreachable|reassembly-timeout|redirect|router-advertisement|router-solicitation|source-quench|source-route-failed|time-exceeded|timestamp-reply|timestamp-request|traceroute|ttl-exceeded|unreachable|dvmrp|host-query|mtrace-resp|mtrace-route|pim|trace|v1host-report|v2host-report|v2leave-group|v3host-report|ack|established|fin|psh|rst|syn|urg))?
(\sdscp\s(?P<dscp>\S+))?
(\s(?P<enable_fragments>fragments))?
(\s(?P<log_input>log-input\s\(tag\s=\s\S+\)|log-input))?
(\s(?P<log>log\s\(tag\s=\s\S+\)|log))?
(\slog-input\s\(tag\s=\s(?P<log_input>\S+\)|log-input))?
(\s(?P<log_input_only>log-input))?
(\slog\s\(tag\s=\s(?P<log>\S+\)|log))?
(\s(?P<log_only>log))?
(\soption\s(?P<option>\S+|\d+))?
(\sprecedence\s(?P<precedence>\S+))?
(\stime-range\s(?P<time_range>\S+))?
Expand Down Expand Up @@ -312,8 +319,10 @@ def __init__(self, lines=None):
"host": "{{ source_host }}",
"object_group": "{{ source_obj_grp }}",
"port_protocol": {
"{{ source_port_protocol.split(' ')[0] if source_port_protocol is defined else None }}": "{{\
source_port_protocol.split(' ')[1] if source_port_protocol is defined else None }}",
"eq": "{{ seq }}",
"gt": "{{ sgt }}",
"lt": "{{ slt }}",
"neq": "{{ sneq }}",
"range": {
"start": "{{ srange_start if srange_start is defined else None }}",
"end": "{{ srange_end if srange_end is defined else None }}",
Expand All @@ -326,8 +335,10 @@ def __init__(self, lines=None):
"host": "{{ dest_host }}",
"object_group": "{{ dest_obj_grp }}",
"port_protocol": {
"{{ dest_port_protocol.split(' ')[0] if dest_port_protocol is defined else None }}": "{{\
dest_port_protocol.split(' ')[1] if dest_port_protocol is defined else None }}",
"eq": "{{ deq }}",
"gt": "{{ dgt }}",
"lt": "{{ dlt }}",
"neq": "{{ dneq }}",
"range": {
"start": "{{ drange_start if drange_start is defined else None }}",
"end": "{{ drange_end if drange_end is defined else None }}",
Expand All @@ -337,12 +348,12 @@ def __init__(self, lines=None):
"dscp": "{{ dscp }}",
"enable_fragments": "{{ True if enable_fragments is defined else None }}",
"log": {
"set": "{{ True if log is defined and 'tag' not in log else '' }}",
"user_cookie": "{{ log.split(' ')[-1].split(')')[0] if log is defined and 'tag' in log else '' }}",
"set": "{{ True if log_only is defined or log is defined }}",
"user_cookie": "{{ log.split(')')[0] if log is defined }}",
},
"log_input": {
"set": "{{ True if log_input is defined and 'tag' not in log_input else '' }}",
"user_cookie": "{{ log_input.split(' ')[-1].split(')')[0] if log_input is defined and 'tag' in log_input }}",
"set": "{{ True if log_input_only is defined or log_input is defined }}",
"user_cookie": "{{ log_input.split(')')[0] if log_input is defined }}",
},
"option": {
"{{ option if option is defined else None }}": "{{ True if option is defined else None }}",
Expand Down
2 changes: 1 addition & 1 deletion plugins/modules/ios_command.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@
EXAMPLES = r"""
- name: Run show version on remote devices
cisco.ios.ios_command:
commands: show version'
commands: show version
# output-
Expand Down
1 change: 1 addition & 0 deletions tests/sanity/ignore-2.17.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
plugins/action/ios.py action-plugin-docs # base class for deprecated network platform modules using `connection: local`
45 changes: 34 additions & 11 deletions tests/unit/modules/network/ios/test_ios_acls.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ def test_ios_acls_merged(self):
),
dict(
grant="deny",
protocol_options=dict(tcp=dict(ack="true")),
protocol_options=dict(tcp=dict(ack="True")),
sequence="200",
source=dict(object_group="test_network_og"),
destination=dict(object_group="test_network_og"),
Expand Down Expand Up @@ -234,7 +234,7 @@ def test_ios_acls_replaced(self):
aces=[
dict(
grant="deny",
protocol_options=dict(tcp=dict(ack="true")),
protocol_options=dict(tcp=dict(ack="True")),
source=dict(
address="198.51.100.0",
wildcard_bits="0.0.0.255",
Expand Down Expand Up @@ -282,6 +282,9 @@ def test_ios_acls_replaced_idempotent(self):
deny tcp any eq www any eq telnet ack dscp af11 sequence 10
Extended IP access list test_pre
10 permit ip any any precedence internet
Extended IP access list test-idem
10 permit ip host 10.153.14.21 any
20 permit ip host 10.153.14.22 any
""",
)
set_module_args(
Expand All @@ -303,7 +306,7 @@ def test_ios_acls_replaced_idempotent(self):
"wildcard_bits": "0.0.0.255",
},
"destination": {"any": True, "port_protocol": {"eq": "22"}},
"log": {"user_cookie": "testLog"},
"log": {"set": True, "user_cookie": "testLog"},
},
{
"sequence": 20,
Expand Down Expand Up @@ -333,6 +336,26 @@ def test_ios_acls_replaced_idempotent(self):
],
},
{"name": "test_acl", "acl_type": "standard"},
{
"aces": [
{
"destination": {"any": True},
"grant": "permit",
"protocol_options": {"ip": True},
"sequence": 10,
"source": {"host": "10.153.14.21"},
},
{
"destination": {"any": True},
"grant": "permit",
"protocol_options": {"ip": True},
"sequence": 20,
"source": {"host": "10.153.14.22"},
},
],
"acl_type": "extended",
"name": "test-idem",
},
{
"name": "test_pre",
"acl_type": "extended",
Expand Down Expand Up @@ -401,7 +424,7 @@ def test_ios_acls_overridden(self):
aces=[
dict(
grant="deny",
protocol_options=dict(tcp=dict(syn="true")),
protocol_options=dict(tcp=dict(syn="True")),
source=dict(
address="198.51.100.0",
wildcard_bits="0.0.0.255",
Expand Down Expand Up @@ -468,7 +491,7 @@ def test_ios_acls_overridden_idempotent(self):
"wildcard_bits": "0.0.0.255",
},
"destination": {"any": True, "port_protocol": {"eq": "22"}},
"log": {"user_cookie": "testLog"},
"log": {"set": True, "user_cookie": "testLog"},
},
{
"sequence": 20,
Expand Down Expand Up @@ -570,7 +593,7 @@ def test_ios_acls_deleted_acl_based(self):
aces=[
dict(
grant="deny",
protocol_options=dict(icmp=dict(echo="true")),
protocol_options=dict(icmp=dict(echo="True")),
sequence="10",
source=dict(address="192.0.2.0", wildcard_bits="0.0.0.255"),
destination=dict(
Expand All @@ -592,11 +615,11 @@ def test_ios_acls_deleted_acl_based(self):
aces=[
dict(
grant="deny",
protocol_options=dict(tcp=dict(ack="true")),
protocol_options=dict(tcp=dict(ack="True")),
sequence="10",
source=dict(any="true", port_protocol=dict(eq="www")),
source=dict(any="True", port_protocol=dict(eq="www")),
destination=dict(
any="true",
any="True",
port_protocol=dict(eq="telnet"),
),
dscp="af11",
Expand Down Expand Up @@ -628,7 +651,7 @@ def test_ios_acls_rendered(self):
grant="deny",
sequence="10",
remarks=["check for remark", "remark for acl 110"],
protocol_options=dict(tcp=dict(syn="true")),
protocol_options=dict(tcp=dict(syn="True")),
source=dict(address="192.0.2.0", wildcard_bits="0.0.0.255"),
destination=dict(
address="192.0.3.0",
Expand Down Expand Up @@ -808,7 +831,7 @@ def test_ios_acls_overridden_option(self):
aces=[
dict(
grant="deny",
protocol_options=dict(tcp=dict(ack="true")),
protocol_options=dict(tcp=dict(ack="True")),
source=dict(
address="198.51.100.0",
wildcard_bits="0.0.0.255",
Expand Down

0 comments on commit 2001633

Please sign in to comment.