From 87cda46a402b73ea1122ae3c8889bc1155eb532c Mon Sep 17 00:00:00 2001 From: KB-perByte Date: Mon, 25 Mar 2024 19:42:56 +0530 Subject: [PATCH] update tests and config code --- .../network/ios/config/acls/acls.py | 167 +++++----- .../network/ios/rm_templates/acls.py | 6 +- .../ios_acls/tests/cli/remarks_states.yaml | 133 ++++++++ .../targets/ios_acls/vars/main.yaml | 285 ++++++++++++++++++ .../unit/modules/network/ios/test_ios_acls.py | 236 +++++++++++---- 5 files changed, 693 insertions(+), 134 deletions(-) create mode 100644 tests/integration/targets/ios_acls/tests/cli/remarks_states.yaml diff --git a/plugins/module_utils/network/ios/config/acls/acls.py b/plugins/module_utils/network/ios/config/acls/acls.py index 7e784ebb1..a08272170 100644 --- a/plugins/module_utils/network/ios/config/acls/acls.py +++ b/plugins/module_utils/network/ios/config/acls/acls.py @@ -24,7 +24,9 @@ dict_merge, ) -from ansible_collections.cisco.ios.plugins.module_utils.network.ios.facts.facts import Facts +from ansible_collections.cisco.ios.plugins.module_utils.network.ios.facts.facts import ( + Facts, +) from ansible_collections.cisco.ios.plugins.module_utils.network.ios.rm_templates.acls import ( AclsTemplate, ) @@ -83,7 +85,9 @@ def generate_commands(self): wplists = wvalue.get("acls", {}) hplists = hvalue.get("acls", {}) hvalue["acls"] = { - k: v for k, v in iteritems(hplists) if k in wplists or not wplists + k: v + for k, v in iteritems(hplists) + if k in wplists or not wplists } # remove superfluous config for overridden and deleted @@ -118,7 +122,9 @@ def rearrange_cmds(aces): hplists = have.get("acls", {}) 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") + 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( @@ -169,20 +175,22 @@ def pop_remark(r_entry, afi): else: return {} + # case 1 - loop on want and compare with have data here for wseq, wentry in iteritems(want): hentry = have.pop(wseq, {}) rem_hentry, rem_wentry = {}, {} - if hentry: + if hentry: # if there is have information with same sequence + # the protocol options are processed here hentry = self.sanitize_protocol_options(wentry, hentry) - if hentry != wentry: # will let in if ace is same but remarks is not same - if hentry: + if hentry != wentry: # if want and have is different + if hentry: # separate remarks from have in an ace entry rem_hentry["remarks"] = pop_remark(hentry, afi) - if wentry: + if wentry: # separate remarks from want in an ace entry rem_wentry["remarks"] = pop_remark(wentry, afi) - if hentry: + if hentry: # have aces processing starts here if self.state == "merged": self._module.fail_json( msg="Cannot update existing sequence {0} of ACLs {1} with state merged." @@ -190,80 +198,78 @@ def pop_remark(r_entry, afi): hentry.get("sequence", ""), name, ), - ) - else: # other action states - if rem_hentry.get("remarks"): # remove remark if not in want - for k_hrems, hrems in rem_hentry.get("remarks").items(): - if k_hrems not in rem_wentry.get("remarks", {}).keys(): - if self.state in ["replaced", "overridden"]: - self.addcmd( - { - "remarks": hrems, - "sequence": hentry.get("sequence", ""), - }, - "remarks_no_data", - negate=True, - ) - break - else: - self.addcmd( - { - "remarks": hrems, - "sequence": hentry.get("sequence", ""), - }, - "remarks", - negate=True, - ) - # remove ace if not in want - # we might think why not update it directly, - # if we try to update without negating the entry appliance - # reports % Duplicate sequence number - if hentry != wentry: - self.addcmd(add_afi(hentry, afi), "aces", negate=True) - # once an ace is negated intentionally emptying out have so that - # the remarks are repopulated, as the remarks and ace behavior is sticky - # if an ace is taken out all the remarks is removed automatically. - rem_hentry["remarks"] = {} + ) # if merged then don't update anything and fail + + # i.e if not merged + if rem_hentry.get("remarks") != rem_wentry.get("remarks"): + self.addcmd( + { + "sequence": hentry.get("sequence", None), + }, + "remarks_no_data", + negate=True, + ) # remove all remarks for a ace if want and have don't match + # as if we randomly add aces we cannot maintain order we have to + # add all of them again, for that ace + rem_hentry["remarks"] = {} + # and me empty our have as we would add back + # all our remarks for that ace anyways + + # remove ace if not in want + # we might think why not update it directly, + # if we try to update without negating the entry appliance + # reports % Duplicate sequence number + if hentry != wentry: + self.addcmd(add_afi(hentry, afi), "aces", negate=True) + # once an ace is negated intentionally emptying out have so that + # the remarks are repopulated, as the remarks and ace behavior is sticky + # if an ace is taken out all the remarks is removed automatically. + rem_hentry["remarks"] = {} if rem_wentry.get("remarks"): # add remark if not in have + if rem_hentry.get("remarks"): + self.addcmd( + { + "sequence": hentry.get("sequence", None), + }, + "remarks_no_data", + negate=True, + ) # but delete all remarks before to protect order for k_wrems, wrems in rem_wentry.get("remarks").items(): - if k_wrems not in rem_hentry.get("remarks", {}).keys(): - self.addcmd( - { - "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": wrems, + "sequence": wentry.get("sequence", ""), + }, "remarks", - negate=True, ) # add ace if not in have if hentry != wentry: - self.addcmd(add_afi(wentry, afi), "aces") - - # remove remaining entries from have aces list + if len(wentry) == 1 and wentry.get( + "sequence" + ): # if the ace entry just has sequence then do nothing + continue + else: # add normal ace entries from want + self.addcmd(add_afi(wentry, afi), "aces") + + # case 2 - loop over remaining have and remove them for hseq in have.values(): - if hseq.get("remarks"): # remove remarks that are extra in have - for krems, rems in hseq.get("remarks").items(): - self.addcmd( - {"remarks": rems, "sequence": hseq.get("sequence", "")}, - "remarks", - negate=True, - ) + if hseq.get("remarks"): # remove all remarks in that + self.addcmd( + { + "sequence": hseq.get("sequence", None), + }, + "remarks_no_data", + negate=True, + ) hseq.pop("remarks") + # deal with the rest of ace entry self.addcmd( add_afi(hseq, afi), "aces", negate=True, - ) # deal with the rest of ace entry + ) def sanitize_protocol_options(self, wace, hace): """handles protocol and protocol options as optional attribute""" @@ -320,20 +326,25 @@ def list_to_dict(self, param): .get("range") ): for k, v in ( - ace.get("destination", {}).get("port_protocol", {}).items() + ace.get("destination", {}) + .get("port_protocol", {}) + .items() ): - ace["destination"]["port_protocol"][ - k - ] = self.port_protocl_no_to_protocol(v) + ace["destination"]["port_protocol"][k] = ( + self.port_protocl_no_to_protocol(v) + ) if acl.get("acl_type") == "standard": for ks in list(ace.keys()): - if ks not in [ - "sequence", - "grant", - "source", - "remarks", - "log", - ]: # failing for mutually exclusive standard acl key + if ( + ks + not in [ + "sequence", + "grant", + "source", + "remarks", + "log", + ] + ): # failing for mutually exclusive standard acl key self._module.fail_json( "Unsupported attribute for standard ACL - {0}.".format( ks, diff --git a/plugins/module_utils/network/ios/rm_templates/acls.py b/plugins/module_utils/network/ios/rm_templates/acls.py index 819961e0d..8ccaec0e8 100644 --- a/plugins/module_utils/network/ios/rm_templates/acls.py +++ b/plugins/module_utils/network/ios/rm_templates/acls.py @@ -23,9 +23,9 @@ def remarks_with_sequence(remarks_data): - cmd = "remark " + cmd = "remark" if remarks_data.get("remarks"): - cmd += remarks_data.get("remarks") + cmd += " " + remarks_data.get("remarks") if remarks_data.get("sequence"): cmd = to_text(remarks_data.get("sequence")) + " " + cmd return cmd @@ -206,7 +206,7 @@ def __init__(self, lines=None): r"""(?P^\d+)\s*remark\s(?P.+)$""", re.VERBOSE, ), - "setval": "{{ sequence }} remark", + "setval": remarks_with_sequence, "result": { "acls": { "{{ acl_name|d() }}": { diff --git a/tests/integration/targets/ios_acls/tests/cli/remarks_states.yaml b/tests/integration/targets/ios_acls/tests/cli/remarks_states.yaml new file mode 100644 index 000000000..68fd361bd --- /dev/null +++ b/tests/integration/targets/ios_acls/tests/cli/remarks_states.yaml @@ -0,0 +1,133 @@ +--- +- ansible.builtin.debug: + msg: START ios_acls round trip integration tests on connection={{ ansible_connection }} + +- ansible.builtin.include_tasks: _remove_config.yaml + +- ansible.builtin.include_tasks: _populate_config.yaml + +- block: + - name: Apply the configuration with remarks + register: base_config + cisco.ios.ios_acls: + config: + - afi: ipv4 + acls: + - name: TEST + acl_type: extended + aces: + - sequence: 10 + remarks: + - "FIRST REMARK BEFORE SEQUENCE 10" + - "============" + - "REMARKS FOR SEQUENCE 10 NO FOLLOWING ACE" + - sequence: 20 + remarks: + - "FIRST REMARK BEFORE SEQUENCE 20" + - "============" + - "ALLOW HOST FROM SEQUENCE 20" + grant: permit + protocol: ip + source: + host: 1.1.1.1 + destination: + any: true + - sequence: 30 + remarks: + - "FIRST REMARK BEFORE SEQUENCE 30" + - "============" + - "ALLOW HOST FROM SEQUENCE 30" + grant: permit + protocol: ip + source: + host: 2.2.2.2 + destination: + any: true + - sequence: 40 + remarks: + - "FIRST REMARK BEFORE SEQUENCE 40" + - "============" + - "ALLOW NEW HOST FROM SEQUENCE 40" + grant: permit + protocol: ip + source: + host: 3.3.3.3 + destination: + any: true + - remarks: + - "Remark not specific to sequence" + - "============" + - "End Remarks" + state: merged + + - ansible.builtin.assert: + that: + - base_config.changed == true + - base_config.commands|symmetric_difference(remarks_check.commands) == [] + - base_config.before|symmetric_difference(remarks_check.before) == [] + - base_config.after|symmetric_difference(remarks_check.after) == [] + + - name: Apply enhanced configuration + register: base_config_overridden + cisco.ios.ios_acls: + config: + - afi: ipv4 + acls: + - name: TEST + acl_type: extended + aces: + - sequence: 10 + remarks: + - "FIRST REMARK BEFORE SEQUENCE 10" + - "============" + - "REMARKS FOR SEQUENCE 10 NO FOLLOWING ACE" + grant: permit + protocol: ip + source: + host: 1.1.1.1 + destination: + any: true + - sequence: 20 + remarks: + - "FIRST REMARK BEFORE SEQUENCE 20" + - "============" + - "ALLOW HOST FROM SEQUENCE 20" + grant: permit + protocol: ip + source: + host: 192.168.0.1 + destination: + any: true + - sequence: 30 + remarks: + - "FIRST REMARK BEFORE SEQUENCE 30" + - "============" + - "ALLOW HOST FROM SEQUENCE 30 updated" + grant: permit + protocol: ip + source: + host: 2.2.2.2 + destination: + any: true + - sequence: 40 + remarks: + - "FIRST REMARK BEFORE SEQUENCE 40" + - "============" + - "ALLOW NEW HOST FROM SEQUENCE 40" + grant: permit + protocol: ip + source: + host: 3.3.3.3 + destination: + any: true + - remarks: + - "Remark not specific to sequence" + - "============" + - "End Remarks updated" + state: overridden + + - ansible.builtin.assert: + that: + - base_config_overridden.commands|symmetric_difference(remarks_check_override.commands) == [] + always: + - ansible.builtin.include_tasks: _remove_config.yaml diff --git a/tests/integration/targets/ios_acls/vars/main.yaml b/tests/integration/targets/ios_acls/vars/main.yaml index 58bf37004..3188b297d 100644 --- a/tests/integration/targets/ios_acls/vars/main.yaml +++ b/tests/integration/targets/ios_acls/vars/main.yaml @@ -396,3 +396,288 @@ rtt: - no ip access-list extended 150 - ipv6 access-list R1_TRAFFIC - deny tcp any eq www any eq telnet ack dscp af11 sequence 10 +remarks_check: + commands: + - ip access-list extended TEST + - 10 remark FIRST REMARK BEFORE SEQUENCE 10 + - 10 remark ============ + - 10 remark REMARKS FOR SEQUENCE 10 NO FOLLOWING ACE + - 20 remark FIRST REMARK BEFORE SEQUENCE 20 + - 20 remark ============ + - 20 remark ALLOW HOST FROM SEQUENCE 20 + - 20 permit ip host 1.1.1.1 any + - 30 remark FIRST REMARK BEFORE SEQUENCE 30 + - 30 remark ============ + - 30 remark ALLOW HOST FROM SEQUENCE 30 + - 30 permit ip host 2.2.2.2 any + - 40 remark FIRST REMARK BEFORE SEQUENCE 40 + - 40 remark ============ + - 40 remark ALLOW NEW HOST FROM SEQUENCE 40 + - 40 permit ip host 3.3.3.3 any + - remark Remark not specific to sequence + - remark ============ + - remark End Remarks + before: + - acls: + - aces: + - destination: + address: 192.0.3.0 + wildcard_bits: 0.0.0.255 + dscp: ef + grant: deny + protocol: icmp + protocol_options: + icmp: + echo: true + sequence: 10 + source: + address: 192.0.2.0 + wildcard_bits: 0.0.0.255 + ttl: + eq: 10 + acl_type: extended + name: "110" + - aces: + - destination: + address: 198.51.101.0 + port_protocol: + eq: telnet + wildcard_bits: 0.0.0.255 + grant: deny + protocol: tcp + protocol_options: + tcp: + ack: true + sequence: 10 + source: + address: 198.51.100.0 + wildcard_bits: 0.0.0.255 + tos: + service_value: 12 + - destination: + address: 192.0.4.0 + port_protocol: + eq: www + wildcard_bits: 0.0.0.255 + dscp: ef + grant: deny + protocol: tcp + protocol_options: + tcp: + ack: true + sequence: 20 + source: + address: 192.0.3.0 + wildcard_bits: 0.0.0.255 + ttl: + lt: 20 + acl_type: extended + name: "123" + - aces: + - destination: + address: 192.0.3.0 + port_protocol: + eq: www + wildcard_bits: 0.0.0.255 + grant: deny + option: + traceroute: true + protocol: tcp + protocol_options: + tcp: + fin: true + sequence: 10 + source: + address: 192.0.2.0 + wildcard_bits: 0.0.0.255 + ttl: + eq: 10 + acl_type: extended + name: test_acl + afi: ipv4 + - acls: + - aces: + - destination: + any: true + port_protocol: + eq: telnet + dscp: af11 + grant: deny + protocol: tcp + protocol_options: + tcp: + ack: true + sequence: 10 + source: + any: true + port_protocol: + eq: www + name: R1_TRAFFIC + afi: ipv6 + after: + - acls: + - aces: + - destination: + address: 192.0.3.0 + wildcard_bits: 0.0.0.255 + dscp: ef + grant: deny + protocol: icmp + protocol_options: + icmp: + echo: true + sequence: 10 + source: + address: 192.0.2.0 + wildcard_bits: 0.0.0.255 + ttl: + eq: 10 + acl_type: extended + name: "110" + - aces: + - destination: + address: 198.51.101.0 + port_protocol: + eq: telnet + wildcard_bits: 0.0.0.255 + grant: deny + protocol: tcp + protocol_options: + tcp: + ack: true + sequence: 10 + source: + address: 198.51.100.0 + wildcard_bits: 0.0.0.255 + tos: + service_value: 12 + - destination: + address: 192.0.4.0 + port_protocol: + eq: www + wildcard_bits: 0.0.0.255 + dscp: ef + grant: deny + protocol: tcp + protocol_options: + tcp: + ack: true + sequence: 20 + source: + address: 192.0.3.0 + wildcard_bits: 0.0.0.255 + ttl: + lt: 20 + acl_type: extended + name: "123" + - aces: + - destination: + any: true + grant: permit + protocol: ip + remarks: + - FIRST REMARK BEFORE SEQUENCE 20 + - ============ + - ALLOW HOST FROM SEQUENCE 20 + sequence: 20 + source: + host: 1.1.1.1 + - destination: + any: true + grant: permit + protocol: ip + remarks: + - FIRST REMARK BEFORE SEQUENCE 30 + - ============ + - ALLOW HOST FROM SEQUENCE 30 + sequence: 30 + source: + host: 2.2.2.2 + - destination: + any: true + grant: permit + protocol: ip + remarks: + - FIRST REMARK BEFORE SEQUENCE 40 + - ============ + - ALLOW NEW HOST FROM SEQUENCE 40 + sequence: 40 + source: + host: 3.3.3.3 + - remarks: + - FIRST REMARK BEFORE SEQUENCE 10 + - ============ + - REMARKS FOR SEQUENCE 10 NO FOLLOWING ACE + sequence: 10 + - remarks: + - Remark not specific to sequence + - ============ + - End Remarks + acl_type: extended + name: TEST + - aces: + - destination: + address: 192.0.3.0 + port_protocol: + eq: www + wildcard_bits: 0.0.0.255 + grant: deny + option: + traceroute: true + protocol: tcp + protocol_options: + tcp: + fin: true + sequence: 10 + source: + address: 192.0.2.0 + wildcard_bits: 0.0.0.255 + ttl: + eq: 10 + acl_type: extended + name: test_acl + afi: ipv4 + - acls: + - aces: + - destination: + any: true + port_protocol: + eq: telnet + dscp: af11 + grant: deny + protocol: tcp + protocol_options: + tcp: + ack: true + sequence: 10 + source: + any: true + port_protocol: + eq: www + name: R1_TRAFFIC + afi: ipv6 +remarks_check_override: + commands: + - no ipv6 access-list R1_TRAFFIC + - ip access-list extended TEST + - no 10 + - no 20 permit ip host 1.1.1.1 any + - no 30 remark + - no remark + - 10 remark FIRST REMARK BEFORE SEQUENCE 10 + - 10 remark ============ + - 10 remark REMARKS FOR SEQUENCE 10 NO FOLLOWING ACE + - 10 permit ip host 1.1.1.1 any + - 20 remark FIRST REMARK BEFORE SEQUENCE 20 + - 20 remark ============ + - 20 remark ALLOW HOST FROM SEQUENCE 20 + - 20 permit ip host 192.168.0.1 any + - 30 remark FIRST REMARK BEFORE SEQUENCE 30 + - 30 remark ============ + - 30 remark ALLOW HOST FROM SEQUENCE 30 updated + - remark Remark not specific to sequence + - remark ============ + - remark End Remarks updated + - no ip access-list extended 110 + - no ip access-list extended 123 + - no ip access-list extended test_acl diff --git a/tests/unit/modules/network/ios/test_ios_acls.py b/tests/unit/modules/network/ios/test_ios_acls.py index 600be646b..7283f02ab 100644 --- a/tests/unit/modules/network/ios/test_ios_acls.py +++ b/tests/unit/modules/network/ios/test_ios_acls.py @@ -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." @@ -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() @@ -444,64 +448,58 @@ def test_ios_acls_merged_remarks_positional(self): ) result = self.execute_module(changed=True) commands = [ - "ip access-list extended mytest", - "remark I am a test ace", - "remark I am right after the test ace", - "remark I third the test ace", - "100 permit ip host 100.100.100.100 any", - "remark I am the next test ace", - "remark I am the next ace to the next ace", - "110 permit ip host 10.40.150.0 any", - "remark I am the peace ace", - "remark Peace out", "ip access-list extended 199", "10 permit ip 10.40.150.0 0.0.0.255 any", "20 permit ip any 10.40.150.0 0.0.0.255", + "ip access-list standard 42", + "10 permit 10.182.250.0 0.0.0.255", + "ip access-list extended empty_ip_ex_acl", + "remark empty remark 1", + "remark empty remark 2", + "remark empty remark never ends", "ip access-list extended NET-MGMT-VTY", "10 permit tcp 10.57.66.243 0.0.0.7 any eq 22", "20 permit tcp host 10.160.114.111 any eq 22", "30 permit tcp host 10.160.115.22 any eq 22", "40 deny ip any any log", - "ip access-list extended empty_ip_ex_acl", - "remark empty remark 1", - "remark empty remark 2", - "remark empty remark never ends", + "ip access-list extended mytest", + "100 remark I am a test ace", + "100 remark I am right after the test ace", + "100 remark I third the test ace", + "100 permit ip host 100.100.100.100 any", + "110 remark I am the next test ace", + "110 remark I am the next ace to the next ace", + "110 permit ip host 10.40.150.0 any", + "remark I am the peace ace", + "remark Peace out", "ip access-list extended TEST", - "remark FIRST REMARK BEFORE LINE 10", - "remark ============", - "remark ALLOW HOST FROM BUILDING 10", + "10 remark FIRST REMARK BEFORE LINE 10", + "10 remark ============", + "10 remark ALLOW HOST FROM BUILDING 10", "10 permit ip host 1.1.1.1 any", - "remark FIRST REMARK BEFORE LINE 20", - "remark ============", - "remark ALLOW HOST FROM BUILDING 20", + "20 remark FIRST REMARK BEFORE LINE 20", + "20 remark ============", + "20 remark ALLOW HOST FROM BUILDING 20", "20 permit ip host 2.2.2.2 any", - "remark FIRST REMARK BEFORE LINE 30", - "remark ============", - "remark ALLOW NEW HOST FROM BUILDING 10", + "30 remark FIRST REMARK BEFORE LINE 30", + "30 remark ============", + "30 remark ALLOW NEW HOST FROM BUILDING 10", "30 permit ip host 3.3.3.3 any", "remark FIRST REMARK AT END OF ACL", "remark SECOND REMARK AT END OF ACL", - "ip access-list standard 42", - "10 permit 10.182.250.0 0.0.0.255", "ipv6 access-list R1_TRAFFIC", "permit ipv6 2001:ABAD:BEEF:1221::/64 any sequence 10", "deny tcp host 2001:ABAD:BEEF:2345::1 host 2001:ABAD:BEEF:1212::1 eq www sequence 20", "ipv6 access-list empty_ipv6_acl", - "remark empty remark 1", - " sequence 10", - "remark empty remark 2", - " sequence 20", - "remark empty remark never ends", - " sequence 30", + "10 remark empty remark 1", + "20 remark empty remark 2", + "30 remark empty remark never ends", "ipv6 access-list ipv6_acl", - "remark I am a ipv6 ace", - " sequence 10", - "remark I am test", - " sequence 20", + "10 remark I am a ipv6 ace", + "20 remark I am test", "permit tcp any any sequence 30", "permit udp any any sequence 40", - "remark I am new set of ipv6 ace", - " sequence 50", + "50 remark I am new set of ipv6 ace", "permit icmp any any sequence 60", ] self.assertEqual(sorted(result["commands"]), sorted(commands)) @@ -650,17 +648,16 @@ def test_ios_acls_replaced(self): "ip access-list extended replace_acl", "deny tcp 198.51.100.0 0.0.0.255 198.51.101.0 0.0.0.255 eq telnet ack tos min-monetary-cost", "ip access-list standard test_acl", - "no remark remark check 1", - "no remark some random remark 2", + "no remark", "remark Another remark here", "ip access-list standard testRobustReplace", - "no 20 remark Remarks for 20", + "no 20 remark", "no 20 permit 0.0.0.0 255.0.0.0", - "no 30 remark Remarks for 30", + "no 30 remark", "no 30 permit 172.16.0.0 0.15.255.255", - "no 40 remark Remarks for 40", + "no 40 remark", "no 40 permit 192.0.2.0 0.0.0.255", - "no 50 remark Remarks for 50", + "no 50 remark", "no 50 permit 198.51.100.0 0.0.0.255", ] self.assertEqual(sorted(result["commands"]), sorted(commands)) @@ -888,8 +885,7 @@ def test_ios_acls_replaced_changetype(self): "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", + "no remark", "remark Another remark here", ] self.assertEqual(sorted(result["commands"]), sorted(commands)) @@ -1193,9 +1189,9 @@ def test_ios_acls_rendered(self): ) commands = [ "ip access-list extended 110", + "10 remark check for remark", + "10 remark remark for acl 110", "10 deny tcp 192.0.2.0 0.0.0.255 192.0.3.0 0.0.0.255 eq www syn dscp ef ttl eq 10", - "remark check for remark", - "remark remark for acl 110", ] result = self.execute_module(changed=False) self.assertEqual(sorted(result["rendered"]), sorted(commands)) @@ -1438,9 +1434,8 @@ def test_ios_acls_overridden_option(self): "permit 433 198.51.101.0 0.0.0.255 198.51.102.0 0.0.0.255 eq telnet log check tos max-throughput", "permit 198.51.105.0 0.0.0.255 198.51.106.0 0.0.0.255 time-range 20 tos max-throughput", "ip access-list standard test_acl", + "no remark", "remark Another remark here", - "no remark remark check 1", - "no remark some random remark 2", ] self.assertEqual(sorted(result["commands"]), sorted(commands)) @@ -2072,8 +2067,7 @@ def test_ios_acls_rendered_muiltioption(self): "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 + "10 remark standalone remarks", "ip access-list standard 2", "30 permit host 172.16.1.11", "20 permit host 172.16.1.10", @@ -2139,9 +2133,145 @@ def test_ios_acls_overridden_sticky_remarks(self): result = self.execute_module(changed=True) commands = [ "ip access-list standard test123", + "no 10 remark", "no 10 permit host 8.8.8.8", "10 remark TEST", "10 remark TEST 2", "10 permit 8.8.128.0 0.0.0.63", ] self.assertEqual(sorted(result["commands"]), sorted(commands)) + + def test_ios_acls_overridden_remarks_complex(self): + self.execute_show_command.return_value = dedent( + """\ + ip access-list extended TEST + 10 remark FIRST REMARK BEFORE SEQUENCE 10 + 10 remark ============ + 10 remark REMARKS FOR SEQUENCE 10 NO FOLLOWING ACE + 20 remark FIRST REMARK BEFORE SEQUENCE 20 + 20 remark ============ + 20 remark ALLOW HOST FROM SEQUENCE 20 + 20 permit ip host 1.1.1.1 any + 30 remark FIRST REMARK BEFORE SEQUENCE 30 + 30 remark ============ + 30 remark ALLOW HOST FROM SEQUENCE 30 + 30 permit ip host 2.2.2.2 any + 40 remark FIRST REMARK BEFORE SEQUENCE 40 + 40 remark ============ + 40 remark ALLOW NEW HOST FROM SEQUENCE 40 + 40 permit ip host 3.3.3.3 any + remark Remark not specific to sequence + remark ============ + remark End Remarks + ip access-list extended test_acl + 10 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 + 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 + ip access-list extended 123 + 10 deny tcp 198.51.100.0 0.0.0.255 198.51.101.0 0.0.0.255 eq telnet ack tos 12 + 20 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 + sequence 10 deny tcp any eq www any eq telnet ack dscp af11 + """, + ) + self.execute_show_command_name.return_value = dedent("") + set_module_args( + dict( + config=[ + { + "afi": "ipv4", + "acls": [ + { + "name": "TEST", + "acl_type": "extended", + "aces": [ + { + "sequence": 10, + "remarks": [ + "FIRST REMARK BEFORE SEQUENCE 10", + "============", + "REMARKS FOR SEQUENCE 10 NO FOLLOWING ACE", + ], + "grant": "permit", + "protocol": "ip", + "source": {"host": "1.1.1.1"}, + "destination": {"any": True}, + }, + { + "sequence": 20, + "remarks": [ + "FIRST REMARK BEFORE SEQUENCE 20", + "============", + "ALLOW HOST FROM SEQUENCE 20", + ], + "grant": "permit", + "protocol": "ip", + "source": {"host": "192.168.0.1"}, + "destination": {"any": True}, + }, + { + "sequence": 30, + "remarks": [ + "FIRST REMARK BEFORE SEQUENCE 30", + "============", + "ALLOW HOST FROM SEQUENCE 30 updated", + ], + "grant": "permit", + "protocol": "ip", + "source": {"host": "2.2.2.2"}, + "destination": {"any": True}, + }, + { + "sequence": 40, + "remarks": [ + "FIRST REMARK BEFORE SEQUENCE 40", + "============", + "ALLOW NEW HOST FROM SEQUENCE 40", + ], + "grant": "permit", + "protocol": "ip", + "source": {"host": "3.3.3.3"}, + "destination": {"any": True}, + }, + { + "remarks": [ + "Remark not specific to sequence", + "============", + "End Remarks 1", + ] + }, + ], + } + ], + } + ], + state="overridden", + ), + ) + result = self.execute_module(changed=True) + commands = [ + "no ipv6 access-list R1_TRAFFIC", + "ip access-list extended TEST", + "no 10", # removes all remarks and ace entry for sequence 10 + "no 20 permit ip host 1.1.1.1 any", # removing the ace automatically removes the remarks + "no 30 remark", # just remove remarks for sequence 30 + "no remark", # remove all remarks at end of acl, that has no sequence + "10 remark FIRST REMARK BEFORE SEQUENCE 10", + "10 remark ============", + "10 remark REMARKS FOR SEQUENCE 10 NO FOLLOWING ACE", + "10 permit ip host 1.1.1.1 any", + "20 remark FIRST REMARK BEFORE SEQUENCE 20", + "20 remark ============", + "20 remark ALLOW HOST FROM SEQUENCE 20", + "20 permit ip host 192.168.0.1 any", + "30 remark FIRST REMARK BEFORE SEQUENCE 30", + "30 remark ============", + "30 remark ALLOW HOST FROM SEQUENCE 30 updated", + "remark Remark not specific to sequence", + "remark ============", + "remark End Remarks 1", + "no ip access-list extended 110", + "no ip access-list extended 123", + "no ip access-list extended test_acl", + ] + self.assertEqual(sorted(result["commands"]), sorted(commands))