[minor change] Add support for annotation in aci_rest module (#437)

plugins/modules/aci_rest.py

@@ -3,6 +3,7 @@
 
 # Copyright: (c) 2017, Dag Wieers (@dagwieers) <[email protected]>
 # Copyright: (c) 2020, Cindy Zhao (@cizhao) <[email protected]>
+# Copyright: (c) 2023, Samita Bhattacharjee (@samitab) <[email protected]>
 # GNU General Public License v3.0+ (see LICENSE or https://www.gnu.org/licenses/gpl-3.0.txt)
 
 from __future__ import absolute_import, division, print_function
@@ -62,6 +63,7 @@
     default: false
 extends_documentation_fragment:
 - cisco.aci.aci
+- cisco.aci.annotation
 
 notes:
 - Certain payloads are known not to be idempotent, so be careful when constructing payloads,
@@ -73,6 +75,7 @@
 - XML payloads require the C(lxml) and C(xmljson) python libraries. For JSON payloads nothing special is needed.
 - If you do not have any attributes, it may be necessary to add the "attributes" key with an empty dictionnary "{}" for value
   as the APIC does expect the entry to precede any children.
+- Annotation set directly in c(src) or C(content) will take precedent over the C(annotation) parameter.
 seealso:
 - module: cisco.aci.aci_tenant
 - name: Cisco APIC REST API Configuration Guide
@@ -81,6 +84,7 @@
 author:
 - Dag Wieers (@dagwieers)
 - Cindy Zhao (@cizhao)
+- Samita Bhattacharjee (@samitab)
 """
 
 EXAMPLES = r"""
@@ -284,8 +288,11 @@
     HAS_YAML = False
 
 from ansible.module_utils.basic import AnsibleModule
-from ansible_collections.cisco.aci.plugins.module_utils.aci import ACIModule, aci_argument_spec
+from ansible_collections.cisco.aci.plugins.module_utils.aci import ACIModule, aci_argument_spec, aci_annotation_spec
 from ansible.module_utils._text import to_text
+from ansible_collections.cisco.aci.plugins.module_utils.annotation_unsupported import (
+    ANNOTATION_UNSUPPORTED,
+)
 
 
 def update_qsl(url, params):
@@ -303,6 +310,33 @@
         return url + "?" + "&".join(["%s=%s" % (k, v) for k, v in params.items()])
 
 
+def add_annotation(annotation, payload):
+    """Add annotation to payload only if it has not already been added"""
+    if annotation and isinstance(payload, dict):
+        for key, val in payload.items():
+            if key in ANNOTATION_UNSUPPORTED:
+                continue
+            att = val.get("attributes", {})
+            if "annotation" not in att.keys():
+                att["annotation"] = annotation
+            # Recursively add annotation to children
+            children = val.get("children", None)
+            if children:
+                for child in children:
+                    add_annotation(annotation, child)
+
+
+def add_annotation_xml(annotation, tree):
+    """Add annotation to payload xml only if it has not already been added"""
+    if annotation:
+        for element in tree.iter():
+            if element.tag in ANNOTATION_UNSUPPORTED:
+                continue
+            ann = element.get("annotation")
+            if ann is None:
+                element.set("annotation", annotation)
+
+
 class ACIRESTModule(ACIModule):
     def changed(self, d):
         """Check ACI response for changes"""
@@ -335,6 +369,7 @@
 
 def main():
     argument_spec = aci_argument_spec()
+    argument_spec.update(aci_annotation_spec())
     argument_spec.update(
         path=dict(type="str", required=True, aliases=["uri"]),
         method=dict(type="str", default="get", choices=["delete", "get", "post"], aliases=["action"]),
@@ -353,6 +388,7 @@
     path = module.params.get("path")
     src = module.params.get("src")
     rsp_subtree_preserve = module.params.get("rsp_subtree_preserve")
+    annotation = module.params.get("annotation")
 
     # Report missing file
     file_exists = False
@@ -388,21 +424,27 @@
     if rest_type == "json":
         if content and isinstance(content, dict):
             # Validate inline YAML/JSON
+            add_annotation(annotation, payload)
             payload = json.dumps(payload)
         elif payload and isinstance(payload, str) and HAS_YAML:
             try:
                 # Validate YAML/JSON string
-                payload = json.dumps(yaml.safe_load(payload))
+                payload = yaml.safe_load(payload)
+                add_annotation(annotation, payload)
+                payload = json.dumps(payload)
             except Exception as e:
                 module.fail_json(msg="Failed to parse provided JSON/YAML payload: {0}".format(to_text(e)), exception=to_text(e), payload=payload)
     elif rest_type == "xml" and HAS_LXML_ETREE:
         if content and isinstance(content, dict) and HAS_XMLJSON_COBRA:
             # Validate inline YAML/JSON
+            add_annotation(annotation, payload)
             payload = etree.tostring(cobra.etree(payload)[0], encoding="unicode")
         elif payload and isinstance(payload, str):
             try:
                 # Validate XML string
-                payload = etree.tostring(etree.fromstring(payload), encoding="unicode")
+                payload = etree.fromstring(payload)
+                add_annotation_xml(annotation, payload)
+                payload = etree.tostring(payload, encoding="unicode")
             except Exception as e:
                 module.fail_json(msg="Failed to parse provided XML payload: {0}".format(to_text(e)), payload=payload)
 

tests/integration/targets/aci_rest/tasks/error_handling.yml

@@ -114,7 +114,7 @@
     that:
     - error_on_input_validation is failed
     - error_on_input_validation.method == 'POST'
-    - "error_on_input_validation.msg == 'APIC Error 801: property descr of tn-ansible_test failed validation for value \\'This is an [invalid] description\\''"
+    - "error_on_input_validation.msg is ansible.builtin.regex('APIC Error 801: property descr of.*tn-ansible_test failed validation for value \\'This is an \\[invalid\\] description\\'')"
     - 'error_on_input_validation.response == "HTTP Error 400: Bad Request"'
     - error_on_input_validation.status == 400
     - "'current' not in error_on_input_validation"

tests/integration/targets/aci_rest/tasks/json_inline.yml

@@ -58,6 +58,7 @@
     that:
     - cm_add_tenant is changed
     - cm_add_tenant.proposed.fvTenant.attributes.name == "ansible_test"
+    - cm_add_tenant.proposed.fvTenant.attributes.annotation == "orchestrator:ansible"
     - cm_verify_checkmode_tenant.current == []
 
 - name: Add tenant (normal mode)
@@ -72,6 +73,7 @@
   ansible.builtin.assert:
     that:
     - nm_add_tenant is changed
+    - nm_add_tenant.imdata.0.fvTenant.attributes.annotation == "orchestrator:ansible"
     - nm_add_tenant_again is not changed
 
 # CHANGE TENANT
@@ -179,3 +181,160 @@
   ansible.builtin.assert:
     that:
     - nm_query_non_tenant is not changed
+
+# VERIFY ANNOTATION SUPPORT
+- name: Add tenant with annotation option
+  cisco.aci.aci_rest:
+    host: '{{ aci_hostname }}'
+    username: '{{ aci_username }}'
+    password: '{{ aci_password }}'
+    validate_certs: '{{ aci_validate_certs | default(false) }}'
+    use_ssl: '{{ aci_use_ssl | default(true) }}'
+    use_proxy: '{{ aci_use_proxy | default(true) }}'
+    output_level: '{{ aci_output_level | default("info") }}'
+    path: /api/mo/uni.json
+    method: post
+    annotation: test:inoption
+    content:
+      {
+          "fvTenant": {
+              "attributes": {
+                  "descr": "Ansible test tenant",
+                  "name": "ansible_test"
+              }
+          }
+      }
+  register: nm_add_tenant_annotation_option
+
+- name: Add tenant with annotation in content
+  cisco.aci.aci_rest:
+    host: '{{ aci_hostname }}'
+    username: '{{ aci_username }}'
+    password: '{{ aci_password }}'
+    validate_certs: '{{ aci_validate_certs | default(false) }}'
+    use_ssl: '{{ aci_use_ssl | default(true) }}'
+    use_proxy: '{{ aci_use_proxy | default(true) }}'
+    output_level: '{{ aci_output_level | default("info") }}'
+    path: /api/mo/uni.json
+    method: post
+    content:
+      {
+          "fvTenant": {
+              "attributes": {
+                  "descr": "Ansible test tenant",
+                  "name": "ansible_test",
+                  "annotation": "test:incontent"
+              }
+          }
+      }
+  register: nm_add_tenant_annotation_content
+
+- name: Add tenant with annotation in content and option
+  cisco.aci.aci_rest:
+    host: '{{ aci_hostname }}'
+    username: '{{ aci_username }}'
+    password: '{{ aci_password }}'
+    validate_certs: '{{ aci_validate_certs | default(false) }}'
+    use_ssl: '{{ aci_use_ssl | default(true) }}'
+    use_proxy: '{{ aci_use_proxy | default(true) }}'
+    output_level: '{{ aci_output_level | default("info") }}'
+    path: /api/mo/uni.json
+    method: post
+    annotation: test:inoption
+    content:
+      {
+          "fvTenant": {
+              "attributes": {
+                  "descr": "Ansible test tenant",
+                  "name": "ansible_test",
+                  "annotation": "test:optionincontent"
+              }
+          }
+      }
+  register: nm_add_tenant_annotation_option_content
+
+- name: Add tag to tenant with annotation unsupported
+  cisco.aci.aci_rest:
+    host: '{{ aci_hostname }}'
+    username: '{{ aci_username }}'
+    password: '{{ aci_password }}'
+    validate_certs: '{{ aci_validate_certs | default(false) }}'
+    use_ssl: '{{ aci_use_ssl | default(true) }}'
+    use_proxy: '{{ aci_use_proxy | default(true) }}'
+    output_level: '{{ aci_output_level | default("info") }}'
+    path: /api/mo/uni/tn-ansible_test/tagKey-foo.json
+    method: post
+    annotation: test:inoption
+    content:
+      {
+          "tagTag": {
+              "attributes": {
+                  "value": "bar"
+              }
+          }
+      }
+  register: nm_add_tag_no_annotation
+
+- name: Remove tenant
+  cisco.aci.aci_rest: *tenant_absent
+
+- name: Add tenant with children objects including annotation
+  cisco.aci.aci_rest:
+    host: '{{ aci_hostname }}'
+    username: '{{ aci_username }}'
+    password: '{{ aci_password }}'
+    validate_certs: '{{ aci_validate_certs | default(false) }}'
+    use_ssl: '{{ aci_use_ssl | default(true) }}'
+    use_proxy: '{{ aci_use_proxy | default(true) }}'
+    output_level: '{{ aci_output_level | default("info") }}'
+    path: /api/mo/uni.json
+    method: post
+    annotation: test:inoption
+    content:
+      {
+          "fvTenant": {
+              "attributes": {
+                  "descr": "Ansible test tenant",
+                  "name": "ansible_test"
+              },
+              "children": [
+                {
+                  "fvCtx": {
+                    "attributes": {
+                      "name": "VRF1"
+                    }
+                  }
+                },
+                {
+                  "fvAp": {
+                    "attributes": {
+                      "name": "Application1"
+                    },
+                    "children": [
+                      {
+                        "fvAEPg": {
+                          "attributes": {
+                            "name": "WebTier",
+                            "annotation": "test:inchild"
+                          }
+                        }
+                      }
+                    ]
+                  }
+                }
+              ]
+          }
+      }
+  register: nm_add_tenant_annotation_children
+
+- name: Verify annotation support
+  assert:
+    that:
+    - nm_add_tenant_annotation_option.imdata.0.fvTenant.attributes.annotation == "test:inoption"
+    - nm_add_tenant_annotation_content.imdata.0.fvTenant.attributes.annotation == "test:incontent"
+    - nm_add_tenant_annotation_option_content.imdata.0.fvTenant.attributes.annotation == "test:optionincontent"
+    - nm_add_tag_no_annotation.imdata.0.tagTag.attributes.annotation is undefined
+    - nm_add_tenant_annotation_children.imdata.0.fvTenant.attributes.annotation == "test:inoption"
+    - nm_add_tenant_annotation_children.imdata.0.fvTenant.children.0.fvAp.attributes.annotation == "test:inoption"
+    - nm_add_tenant_annotation_children.imdata.0.fvTenant.children.0.fvAp.children.0.fvAEPg.attributes.annotation == "test:inchild"
+    - nm_add_tenant_annotation_children.imdata.0.fvTenant.children.2.fvCtx.attributes.annotation == "test:inoption"