[ios_acls] enhance remarks behaviour (#941)

* fix remarks behaviour * update remarks facts code * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fix config side code * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * acls first cleanup * half test fix * doc fixes * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * update tests and doc * [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 * add changelog * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * lint issues * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fix edge remarks case * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * update docs * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fix tests * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * use to_text * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
ansible-collections · Oct 27, 2023 · 4365802 · 4365802
1 parent c9c5631
commit 4365802
Show file tree

Hide file tree

Showing 44 changed files with 1,704 additions and 441 deletions.
diff --git a/changelogs/fragments/acl_remaks_fresh.yml b/changelogs/fragments/acl_remaks_fresh.yml
@@ -0,0 +1,4 @@
+---
+minor_changes:
+  - "ios_acls - make remarks ordered and to be applied per ace basis."
+  - "ios_acls - remarks in replaced and overridden state to be negated once per ace."
diff --git a/docs/cisco.ios.ios_acl_interfaces_module.rst b/docs/cisco.ios.ios_acl_interfaces_module.rst
@@ -442,7 +442,7 @@ Examples
     #  ip access-group 123 out
 
     # Using DELETED without any config passed
-    #"(NOTE: This will delete all of configured resource module attributes from each configured interface)"
+    # "(NOTE: This will delete all of configured resource module attributes from each configured interface)"
 
     # Before state:
     # -------------

diff --git a/docs/cisco.ios.ios_acls_module.rst b/docs/cisco.ios.ios_acls_module.rst
diff --git a/docs/cisco.ios.ios_banner_module.rst b/docs/cisco.ios.ios_banner_module.rst
@@ -143,7 +143,7 @@ Examples
     - name: Configure banner from file
       cisco.ios.ios_banner:
         banner: motd
-        text: "{{ lookup('file', './config_partial/raw_banner.cfg') }}" # Use unix formatted text files (LF not CRLF) to avoid idempotency issues.
+        text: "{{ lookup('file', './config_partial/raw_banner.cfg') }}"  # Use unix formatted text files (LF not CRLF) to avoid idempotency issues.
         state: present
 
     - name: Configure the login banner using delimiter

diff --git a/docs/cisco.ios.ios_hostname_module.rst b/docs/cisco.ios.ios_hostname_module.rst
@@ -249,7 +249,7 @@ Examples
     # Before state:
     # -------------
 
-    #router-ios#show running-config | section ^hostname
+    # router-ios#show running-config | section ^hostname
     # hostname RouterTest
 
     # Gathered play:

diff --git a/docs/cisco.ios.ios_lag_interfaces_module.rst b/docs/cisco.ios.ios_lag_interfaces_module.rst
@@ -238,7 +238,7 @@ Examples
           - name: Port-channel20
             members:
               - member: GigabitEthernet0/3
-                mode: on
+                mode: "on"
           - name: Port-channel30
             members:
               - member: GigabitEthernet0/4
@@ -461,7 +461,7 @@ Examples
     #  channel-group 30 mode active
 
     # Using Deleted without any config passed
-    #"(NOTE: This will delete all of configured LLDP module attributes)"
+    # "(NOTE: This will delete all of configured LLDP module attributes)"
 
     #
     # Before state:

diff --git a/docs/cisco.ios.ios_linkagg_module.rst b/docs/cisco.ios.ios_linkagg_module.rst
@@ -275,7 +275,7 @@ Examples
     - name: Create aggregate of linkagg definitions
       cisco.ios.ios_linkagg:
         aggregate:
-          - { group: 3, mode: on, members: [GigabitEthernet0/1] }
+          - { group: 3, mode: "on", members: [GigabitEthernet0/1] }
           - { group: 100, mode: passive, members: [GigabitEthernet0/2] }
 
 

diff --git a/docs/cisco.ios.ios_lldp_global_module.rst b/docs/cisco.ios.ios_lldp_global_module.rst
@@ -440,7 +440,7 @@ Examples
     #  lldp reinit 5
 
     # Using Deleted without any config passed
-    #"(NOTE: This will delete all of configured LLDP module attributes)"
+    # "(NOTE: This will delete all of configured LLDP module attributes)"
 
     # Before state:
     # -------------

diff --git a/docs/cisco.ios.ios_logging_global_module.rst b/docs/cisco.ios.ios_logging_global_module.rst
@@ -2490,7 +2490,7 @@ Examples
     # Before state:
     # -------------
 
-    #router-ios#show running-config | section logging
+    # router-ios#show running-config | section logging
     # logging exception 4099
     # logging message-counter log
     # logging userinfo

diff --git a/docs/cisco.ios.ios_ntp_global_module.rst b/docs/cisco.ios.ios_ntp_global_module.rst
@@ -1884,7 +1884,7 @@ Examples
     # Before state:
     # -------------
 
-    #router-ios#show running-config | section ^ntp
+    # router-ios#show running-config | section ^ntp
     # ntp max-associations 34
     # ntp logging
     # ntp allow mode control 4

diff --git a/docs/cisco.ios.ios_ospfv2_module.rst b/docs/cisco.ios.ios_ospfv2_module.rst
@@ -4633,7 +4633,7 @@ Examples
               passive_interfaces:
                 default: true
                 interface:
-                  set_interface: False
+                  set_interface: false
                   name:
                     - GigabitEthernet0/1
                     - GigabitEthernet0/2

diff --git a/docs/cisco.ios.ios_prefix_lists_module.rst b/docs/cisco.ios.ios_prefix_lists_module.rst
@@ -458,8 +458,6 @@ Examples
           - afi: ipv4
         state: deleted
 
-
-
     # Task Output
     # -------------
     # before:

diff --git a/docs/cisco.ios.ios_service_module.rst b/docs/cisco.ios.ios_service_module.rst
@@ -1295,8 +1295,8 @@ Examples
     #   prompt: true
     #   slave_log: true
 
-    #·After·state:
-    #·------------
+    # After state:
+    # ------------
     #
     # router-ios#show running-config all | section ^service
     # service slave-log

diff --git a/docs/cisco.ios.ios_snmp_server_module.rst b/docs/cisco.ios.ios_snmp_server_module.rst
@@ -9371,7 +9371,7 @@ Examples
     # Before state:
     # -------------
 
-    #router-ios#show running-config | section ^snmp-server
+    # router-ios#show running-config | section ^snmp-server
     # snmp-server engineID remote 172.16.0.12 udp-port 25 AB0C5342FF0F
     # snmp-server user userPaul dev v1 access 24
     # snmp-server group mergedGroup v3 auth

diff --git a/docs/cisco.ios.ios_vlans_module.rst b/docs/cisco.ios.ios_vlans_module.rst
@@ -576,7 +576,7 @@ Examples
     # 1005 trnet 101005     1500  -      -      -        ibm  -        0      0
 
     # Using Deleted without any config passed
-    #"(NOTE: This will delete all of configured vlans attributes)"
+    # "(NOTE: This will delete all of configured vlans attributes)"
 
     # Before state:
     # -------------

diff --git a/plugins/module_utils/network/ios/config/acls/acls.py b/plugins/module_utils/network/ios/config/acls/acls.py
@@ -15,6 +15,7 @@
 
 __metaclass__ = type
 
+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.rm_base.resource_module import (
     ResourceModule,
@@ -149,11 +150,28 @@ def add_afi(entry, afi):
                 entry["afi"] = afi
             return entry
 
+        def pop_remark(r_entry, afi):
+            """Takes out remarks from ace entry as remarks not same
+            does not mean the ace entry to be re-introduced
+            """
+            if r_entry.get("remarks"):
+                return r_entry.pop("remarks")
+            else:
+                return {}
+
         for wseq, wentry in iteritems(want):
             hentry = have.pop(wseq, {})
+            rem_hentry, rem_wentry = {}, {}
+
             if hentry:
                 hentry = self.sanitize_protocol_options(wentry, hentry)
-            if hentry != wentry:
+
+            if hentry != wentry:  # will let in if ace is same but remarks is not same
+                if hentry:
+                    rem_hentry["remarks"] = pop_remark(hentry, afi)
+                if wentry:
+                    rem_wentry["remarks"] = pop_remark(wentry, afi)
+
                 if hentry:
                     if self.state == "merged":
                         self._module.fail_json(
@@ -164,24 +182,51 @@ def add_afi(entry, afi):
                             ),
                         )
                     else:  # other action states
-                        if hentry.get("remarks"):  # remove remark if not in want
-                            for rems in hentry.get("remarks"):
-                                if rems not in wentry.get("remarks", {}):
-                                    self.addcmd({"remarks": rems}, "remarks", negate=True)
-                        else:  # remove ace if not in want
+                        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
+                        if hentry != wentry:
                             self.addcmd(add_afi(hentry, afi), "aces", negate=True)
-                if wentry.get("remarks"):  # add remark if not in have
-                    for rems in wentry.get("remarks"):
-                        if rems not in hentry.get("remarks", {}):
-                            self.addcmd({"remarks": rems}, "remarks")
-                else:  # add ace if not in have
+                if rem_wentry.get("remarks"):  # add remark if not in have
+                    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",
+                            )
+                # add ace if not in have
+                if hentry != wentry:
                     self.addcmd(add_afi(wentry, afi), "aces")
 
         # remove remaining entries from have aces list
         for hseq in have.values():
             if hseq.get("remarks"):  # remove remarks that are extra in have
-                for rems in hseq.get("remarks"):
-                    self.addcmd({"remarks": rems}, "remarks", negate=True)
+                for krems, rems in hseq.get("remarks").items():
+                    self.addcmd(
+                        {"remarks": rems, "sequence": hseq.get("sequence", "")},
+                        "remarks",
+                        negate=True,
+                    )
             else:  # remove extra aces
                 self.addcmd(add_afi(hseq, afi), "aces", negate=True)
 
@@ -225,7 +270,7 @@ def list_to_dict(self, param):
                     for acl in each.get("acls"):  # check each acl for aces
                         temp_aces = {}
                         if acl.get("aces"):
-                            temp_rem = []  # remarks if defined in an ace
+                            rem_idx = 0  # remarks if defined in an ace
                             for ace in acl.get("aces"):  # each ace turned to dict
                                 if (
                                     ace.get("destination")
@@ -258,18 +303,25 @@ def list_to_dict(self, param):
                                                 ),
                                             )
 
-                                if ace.get("remarks"):
-                                    en_name = str(acl.get("name")) + "remark"
-                                    temp_rem.extend(ace.pop("remarks"))
+                                if ace.get(
+                                    "remarks",
+                                ):  # index aces inside of each ace don't cluster them all
+                                    rem_ace = {}
+                                    # en_name = str(acl.get("name")) + "remark"
+                                    # 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:
                                     count += 1
-                                    temp_aces.update({"_" + str(count): ace})
+                                    temp_aces.update({"_" + to_text(count): ace})
 
-                            if temp_rem:  # add remarks to the temp ace
-                                temp_aces.update({en_name: {"remarks": temp_rem}})
+                            # if temp_rem:  # add remarks to the temp ace
+                            #     temp_aces.update({en_name: {"remarks": temp_rem}})
 
                         if acl.get("acl_type"):  # update acl dict with req info
                             temp_acls.update(