[ignore] Fixed the update logic to clear bfd config on ndo_l3out_node…

…_group_policy module

plugins/module_utils/template.py

@@ -227,9 +227,9 @@ def get_l3out_object(self, uuid=None, name=None, fail_module=False):
 
     def get_l3out_node_group(self, name, l3out_object, fail_module=False):
         """
-        Get the L3Out Node/Interface Group Policy by name.
-        :param name: Name of the L3Out Node/Interface Group Policy to search for -> Str
-        :param l3out_object: L3Out object to search Node/Interface Group Policy -> Dict
+        Get the L3Out Node Group Policy by name.
+        :param name: Name of the L3Out Node Group Policy to search for -> Str
+        :param l3out_object: L3Out object to search Node Group Policy -> Dict
         :param fail_module: When match is not found fail the ansible module -> Bool
         :return: Dict | None | List[Dict] | List[]: The processed result which could be:
                  When the UUID | Name is existing in the search list -> Dict
@@ -239,5 +239,5 @@ def get_l3out_node_group(self, name, l3out_object, fail_module=False):
         """
         existing_l3out_node_groups = l3out_object.get("nodeGroups", [])
         if name:  # Query a specific object
-            return self.get_object_by_key_value_pairs("L3Out Node/Interface Group Policy", existing_l3out_node_groups, [KVPair("name", name)], fail_module)
+            return self.get_object_by_key_value_pairs("L3Out Node Group Policy", existing_l3out_node_groups, [KVPair("name", name)], fail_module)
         return existing_l3out_node_groups  # Query all objects

plugins/modules/ndo_l3out_node_group_policy.py

@@ -46,16 +46,19 @@
   node_routing_policy:
     description:
     - The name of the L3Out Node Routing Policy.
+    - Providing an empty string will remove the O(node_routing_policy="") from L3Out Node Group Policy.
     type: str
   bfd_multi_hop_authentication:
     description:
     - The Bidirectional Forwarding Detection (BFD) multi-hop authentication of the L3Out Node Group Policy.
     - To enable the O(bfd_multi_hop_authentication) BGP routing protocol must be configured on the L3Out.
+    - To clear the BFD auth configuration, provide the values O(bfd_multi_hop_authentication=""), O(bfd_multi_hop_key=""), and O(bfd_multi_hop_key_id=0).
     type: str
-    choices: [ enabled, disabled ]
+    choices: [ enabled, disabled, "" ]
   bfd_multi_hop_key_id:
     description:
     - The BFD multi-hop key ID of the L3Out Node Group Policy.
+    - Providing 0 will remove the O(bfd_multi_hop_key_id=0) from L3Out Node Group Policy.
     type: int
   bfd_multi_hop_key:
     description:
@@ -176,9 +179,9 @@ def main():
         name=dict(type="str", aliases=["l3out_node_group_policy"]),
         description=dict(type="str"),
         node_routing_policy=dict(type="str"),
-        bfd_multi_hop_authentication=dict(type="str", choices=["enabled", "disabled"]),
+        bfd_multi_hop_authentication=dict(type="str", choices=["enabled", "disabled", ""]),
         bfd_multi_hop_key_id=dict(type="int"),
-        bfd_multi_hop_key=dict(type="str", no_log=False),
+        bfd_multi_hop_key=dict(type="str", no_log=True),
         target_dscp=dict(type="str", choices=list(TARGET_DSCP_MAP)),
         state=dict(type="str", default="query", choices=["absent", "query", "present"]),
     )
@@ -235,6 +238,9 @@ def main():
 
         if mso.existing:
             proposed_payload = copy.deepcopy(mso.existing)
+            if proposed_payload.get("bfdMultiHop", {}).get("key", {}).get("ref"):
+                proposed_payload["bfdMultiHop"]["key"].pop("ref", None)
+                mso.existing["bfdMultiHop"]["key"].pop("ref", None)
 
             if description is not None and mso.existing.get("description") != description:
                 ops.append(dict(op="replace", path=node_group_policy_path + "/description", value=description))
@@ -245,41 +251,50 @@ def main():
                     dict(op="replace", path=node_group_policy_path + "/nodeRoutingPolicyRef", value=l3out_node_routing_policy_object.details.get("uuid"))
                 )
                 proposed_payload["nodeRoutingPolicyRef"] = l3out_node_routing_policy_object.details.get("uuid")
-            elif node_routing_policy == "" and mso.existing.get("nodeRoutingPolicyRef"):
-                ops.append(dict(op="remove", path=node_group_policy_path + "/nodeRoutingPolicyRef"))
-                proposed_payload.pop("nodeRoutingPolicyRef", None)
 
-            if (bfd_multi_hop_authentication or bfd_multi_hop_key or bfd_multi_hop_key_id) and not mso.existing.get("bfdMultiHop"):
-                ops.append(dict(op="replace", path=node_group_policy_path + "/bfdMultiHop", value=dict()))
-                proposed_payload["bfdMultiHop"] = dict()
-
-            if bfd_multi_hop_authentication is not None and mso.existing.get("bfdMultiHop", {}).get("authEnabled") is not (
-                True if bfd_multi_hop_authentication == "enabled" else False
-            ):
-                ops.append(
-                    dict(
-                        op="replace",
-                        path=node_group_policy_path + "/bfdMultiHop/authEnabled",
-                        value=True if bfd_multi_hop_authentication == "enabled" else False,
+            elif node_routing_policy == "" and mso.existing.get(
+                "nodeRoutingPolicyRef"
+            ):  # Clear the node routing policy when node_routing_policy is empty string
+                ops.append(dict(op="replace", path=node_group_policy_path + "/nodeRoutingPolicyRef", value=node_routing_policy))
+                proposed_payload["nodeRoutingPolicyRef"] = node_routing_policy
+
+            if bfd_multi_hop_authentication or bfd_multi_hop_key or bfd_multi_hop_key_id:
+                if not mso.existing.get("bfdMultiHop"):
+                    ops.append(dict(op="replace", path=node_group_policy_path + "/bfdMultiHop", value=dict()))
+                    proposed_payload["bfdMultiHop"] = dict()
+
+                # Ignore when bfd_multi_hop_authentication is None or empty string
+                if bfd_multi_hop_authentication and mso.existing.get("bfdMultiHop", {}).get("authEnabled") is not (
+                    True if bfd_multi_hop_authentication == "enabled" else False
+                ):
+                    ops.append(
+                        dict(
+                            op="replace",
+                            path=node_group_policy_path + "/bfdMultiHop/authEnabled",
+                            value=True if bfd_multi_hop_authentication == "enabled" else False,
+                        )
                     )
-                )
-                proposed_payload["bfdMultiHop"]["authEnabled"] = True if bfd_multi_hop_authentication == "enabled" else False
+                    proposed_payload["bfdMultiHop"]["authEnabled"] = True if bfd_multi_hop_authentication == "enabled" else False
 
-            if bfd_multi_hop_key_id is not None and mso.existing.get("bfdMultiHop", {}).get("keyID") != bfd_multi_hop_key_id:
-                ops.append(dict(op="replace", path=node_group_policy_path + "/bfdMultiHop/keyID", value=bfd_multi_hop_key_id))
-                proposed_payload["bfdMultiHop"]["keyID"] = bfd_multi_hop_key_id
+                if bfd_multi_hop_key_id is not None and mso.existing.get("bfdMultiHop", {}).get("keyID") != bfd_multi_hop_key_id:
+                    ops.append(dict(op="replace", path=node_group_policy_path + "/bfdMultiHop/keyID", value=bfd_multi_hop_key_id))
+                    proposed_payload["bfdMultiHop"]["keyID"] = bfd_multi_hop_key_id
 
-            if bfd_multi_hop_key is not None:
-                ops.append(dict(op="replace", path=node_group_policy_path + "/bfdMultiHop/key", value=dict()))
-                ops.append(dict(op="replace", path=node_group_policy_path + "/bfdMultiHop/key/value", value=bfd_multi_hop_key))
-                proposed_payload["bfdMultiHop"]["key"] = dict(value=bfd_multi_hop_key)
+                if bfd_multi_hop_key is not None:
+                    ops.append(dict(op="replace", path=node_group_policy_path + "/bfdMultiHop/key", value=dict()))
+                    ops.append(dict(op="replace", path=node_group_policy_path + "/bfdMultiHop/key/value", value=bfd_multi_hop_key))
+                    proposed_payload["bfdMultiHop"]["key"] = dict(value=bfd_multi_hop_key)
+
+            # Clear the complete BFD multi-hop configuration when all parameters are empty
+            elif bfd_multi_hop_authentication == "" and bfd_multi_hop_key == "" and bfd_multi_hop_key_id == 0 and mso.existing.get("bfdMultiHop"):
+                ops.append(dict(op="remove", path=node_group_policy_path + "/bfdMultiHop"))
+                proposed_payload.pop("bfdMultiHop")
 
             if target_dscp is not None and mso.existing.get("targetDscp") != target_dscp:
                 ops.append(dict(op="replace", path=node_group_policy_path + "/targetDscp", value=target_dscp))
                 proposed_payload["targetDscp"] = target_dscp
 
             mso.sanitize(proposed_payload, collate=True)
-
         else:
             payload = dict(name=name)
 
@@ -291,7 +306,7 @@ def main():
 
             bfd_multi_hop = dict()
 
-            if bfd_multi_hop_authentication is not None:
+            if bfd_multi_hop_authentication:
                 bfd_multi_hop["authEnabled"] = True if bfd_multi_hop_authentication == "enabled" else False
 
             if bfd_multi_hop_key_id:
@@ -321,6 +336,8 @@ def main():
         l3out_node_group = mso_template.get_l3out_node_group(name, l3out_object.details)
         if l3out_node_group:
             mso.existing = l3out_node_group.details  # When the state is present
+            if mso.existing.get("bfdMultiHop", {}).get("key", {}).get("ref"):
+                mso.existing["bfdMultiHop"]["key"].pop("ref")
         else:
             mso.existing = {}  # When the state is absent
     elif module.check_mode and state != "query":  # When the state is present/absent with check mode

tests/integration/targets/ndo_l3out_node_group_policy/tasks/main.yml

@@ -270,7 +270,7 @@
         that:
           - cm_update_node_group_policy_1 is changed
           - cm_update_node_group_policy_1.current.bfdMultiHop.keyID == 1
-          - cm_update_node_group_policy_1.current.bfdMultiHop.key.value == "TestKey"
+          - cm_update_node_group_policy_1.current.bfdMultiHop.key.value is defined
           - cm_update_node_group_policy_1.current.description == "Test description"
           - cm_update_node_group_policy_1.current.name == "node_group_policy_1"
           - cm_update_node_group_policy_1.current.nodeRoutingPolicyRef != ""
@@ -298,18 +298,17 @@
           - nm_update_node_group_policy_1.previous.nodeRoutingPolicyRef == ""
           - nm_update_node_group_policy_1.previous.targetDscp == "unspecified"
 
-    - name: Update L3Out node group policy object with normal mode again - task status should be changed because of the auth key ref change
+    - name: Update L3Out node group policy object with normal mode again
       cisco.mso.ndo_l3out_node_group_policy:
         <<: *nm_update_node_group_policy_1
       register: nm_update_node_group_policy_1_again
 
     - name: Assertion check for update L3Out node group policy object with normal mode again
       ansible.builtin.assert:
         that:
-          - nm_update_node_group_policy_1_again is changed
+          - nm_update_node_group_policy_1_again is not changed
           - nm_update_node_group_policy_1_again.current.bfdMultiHop.authEnabled == true
           - nm_update_node_group_policy_1_again.current.bfdMultiHop.keyID == 1
-          - nm_update_node_group_policy_1_again.current.bfdMultiHop.key.ref != nm_update_node_group_policy_1_again.previous.bfdMultiHop.key.ref
           - nm_update_node_group_policy_1_again.current.description == "Test description"
           - nm_update_node_group_policy_1_again.current.name == "node_group_policy_1"
           - nm_update_node_group_policy_1_again.current.nodeRoutingPolicyRef != ""
@@ -473,7 +472,6 @@
         that:
           - add_ngp_3_auth_disabled is changed
           - add_ngp_3_auth_disabled.current.bfdMultiHop.authEnabled == true
-          - add_ngp_3_auth_disabled.current.bfdMultiHop.key.ref is defined
           - add_ngp_3_auth_disabled.current.bfdMultiHop.keyID == 2
           - add_ngp_3_auth_disabled.current.name == "node_group_policy_30"
           - add_ngp_3_auth_disabled.current.nodeRoutingPolicyRef != ""
@@ -500,7 +498,6 @@
           - add_ngp_3_auth_disabled is changed
           - add_ngp_3_auth_disabled.current.bfdMultiHop.authEnabled == false
           - add_ngp_3_auth_disabled.current.bfdMultiHop.keyID == 2
-          - add_ngp_3_auth_disabled.current.bfdMultiHop.key.ref is not defined
           - add_ngp_3_auth_disabled.current.name == "node_group_policy_3"
           - add_ngp_3_auth_disabled.current.nodeRoutingPolicyRef != ""
           - add_ngp_3_auth_disabled.current.targetDscp == "af12"
@@ -578,6 +575,61 @@
           - add_ngp_4_auth_disabled.current.targetDscp == "af12"
           - add_ngp_4_auth_disabled.previous == {}
 
+    - name: Add bfd_multi_hop_key_id to node_group_policy_4 when bfd_multi_hop_authentication is disabled
+      cisco.mso.ndo_l3out_node_group_policy:
+        <<: *mso_info
+        template: '{{ ansible_l3out_template | default("ansible_test") }}'
+        l3out: "l3out_2"
+        name: "node_group_policy_4"
+        bfd_multi_hop_key_id: 10
+        state: present
+      register: add_key_id_ngp_4_auth_disabled
+
+    - name: Assertion check for add bfd_multi_hop_key_id to node_group_policy_4 when bfd_multi_hop_authentication is disabled
+      ansible.builtin.assert:
+        that:
+          - add_key_id_ngp_4_auth_disabled is changed
+          - add_key_id_ngp_4_auth_disabled.current.bfdMultiHop.authEnabled == false
+          - add_key_id_ngp_4_auth_disabled.current.bfdMultiHop.keyID == 10
+          - add_key_id_ngp_4_auth_disabled.current.name == "node_group_policy_4"
+          - add_key_id_ngp_4_auth_disabled.current.nodeRoutingPolicyRef != ""
+          - add_key_id_ngp_4_auth_disabled.current.targetDscp == "af12"
+          - add_key_id_ngp_4_auth_disabled.previous.bfdMultiHop.authEnabled == false
+          - add_key_id_ngp_4_auth_disabled.previous.name == "node_group_policy_4"
+          - add_key_id_ngp_4_auth_disabled.previous.nodeRoutingPolicyRef != ""
+          - add_key_id_ngp_4_auth_disabled.previous.bfdMultiHop.keyID is not defined
+          - add_key_id_ngp_4_auth_disabled.previous.targetDscp == "af12"
+
+    - name: Remove bfd_multi_hop_key_id from node_group_policy_4 when bfd_multi_hop_authentication is disabled
+      cisco.mso.ndo_l3out_node_group_policy:
+        <<: *mso_info
+        template: '{{ ansible_l3out_template | default("ansible_test") }}'
+        l3out: "l3out_2"
+        name: "node_group_policy_4"
+        bfd_multi_hop_authentication: disabled
+        bfd_multi_hop_key_id: 0
+        state: present
+        output_level: debug
+      register: clear_key_id_ngp_4_auth_disabled
+
+    - name: Assertion check for remove bfd_multi_hop_key_id from node_group_policy_4 when bfd_multi_hop_authentication is disabled
+      ansible.builtin.assert:
+        that:
+          - clear_key_id_ngp_4_auth_disabled is changed
+          - clear_key_id_ngp_4_auth_disabled.current.bfdMultiHop.keyID is not defined
+          - clear_key_id_ngp_4_auth_disabled.current.bfdMultiHop.authEnabled == false
+          - clear_key_id_ngp_4_auth_disabled.current.name == "node_group_policy_4"
+          - clear_key_id_ngp_4_auth_disabled.current.nodeRoutingPolicyRef != ""
+          - clear_key_id_ngp_4_auth_disabled.current.targetDscp == "af12"
+          - clear_key_id_ngp_4_auth_disabled.previous.bfdMultiHop.authEnabled == false
+          - clear_key_id_ngp_4_auth_disabled.previous.bfdMultiHop.keyID == 10
+          - clear_key_id_ngp_4_auth_disabled.previous.name == "node_group_policy_4"
+          - clear_key_id_ngp_4_auth_disabled.previous.nodeRoutingPolicyRef != ""
+          - clear_key_id_ngp_4_auth_disabled.previous.targetDscp == "af12"
+          - clear_key_id_ngp_4_auth_disabled.proposed.bfdMultiHop.authEnabled == false
+          - clear_key_id_ngp_4_auth_disabled.proposed.bfdMultiHop.keyID == 0
+          - clear_key_id_ngp_4_auth_disabled.proposed.name == "node_group_policy_4"
+
     - name: Create node_group_policy_5 without bfd config
       cisco.mso.ndo_l3out_node_group_policy:
         <<: *mso_info
@@ -657,7 +709,6 @@
         that:
           - update_ngp_5_bfd_config is changed
           - update_ngp_5_bfd_config.current.bfdMultiHop.authEnabled == true
-          - update_ngp_5_bfd_config.current.bfdMultiHop.key.ref is defined
           - update_ngp_5_bfd_config.current.bfdMultiHop.keyID == 3
           - update_ngp_5_bfd_config.current.name == "node_group_policy_5"
           - update_ngp_5_bfd_config.current.nodeRoutingPolicyRef != ""
@@ -668,6 +719,31 @@
           - update_ngp_5_bfd_config.previous.nodeRoutingPolicyRef != ""
           - update_ngp_5_bfd_config.previous.targetDscp == "unspecified"
 
+    - name: Clear complete bfd and node_routing_policy config from node_group_policy_5
+      cisco.mso.ndo_l3out_node_group_policy:
+        <<: *mso_info
+        template: '{{ ansible_l3out_template | default("ansible_test") }}'
+        l3out: "l3out_2"
+        name: "node_group_policy_5"
+        node_routing_policy: ""
+        bfd_multi_hop_authentication: ""
+        bfd_multi_hop_key_id: 0
+        bfd_multi_hop_key: ""
+        state: present
+      register: clear_ngp_5_bfd_config
+
+    - name: Assertion check for clear complete bfd and node_routing_policy config from node_group_policy_5
+      ansible.builtin.assert:
+        that:
+          - clear_ngp_5_bfd_config is changed
+          - clear_ngp_5_bfd_config.current.name == "node_group_policy_5"
+          - clear_ngp_5_bfd_config.current.nodeRoutingPolicyRef == ""
+          - clear_ngp_5_bfd_config.current.targetDscp == "unspecified"
+          - clear_ngp_5_bfd_config.previous.name == "node_group_policy_5"
+          - clear_ngp_5_bfd_config.previous.nodeRoutingPolicyRef != ""
+          - clear_ngp_5_bfd_config.previous.targetDscp == "unspecified"
+          - clear_ngp_5_bfd_config.previous.bfdMultiHop is defined
+
     - name: Remove node_group_policy_2 with check mode
       cisco.mso.ndo_l3out_node_group_policy: &cm_rm_node_group_policy_2
         <<: *add_node_group_policy_2