Bugfix legacy modules, implement idempotency

changelogs/fragments/44-b1-modules-fix.yml

@@ -0,0 +1,7 @@
+bugfixes:
+  - fix previous commit for lint/sanity fixes breaking legacy b1 modules "missing required positional argument 'data'"
+  - fix deleting record returning failed status when record doesn't exist on legacy b1 modules
+  - fix "get" state returning "changed" for legacy b1 modules
+minor_changes:
+  - implement idempotence for b1_a_record and b1_cname_record
+  - implement updating address for b1_a_record, and can_name for b1_cname_record

plugins/module_utils/b1ddi.py

@@ -32,8 +32,10 @@ def __init__(self, baseUrl, token):
         if not HAS_REQUESTS_LIB:
             raise ImportError('Missing required library "requests"') from REQUESTS_LIB_IMP_ERR
 
-    def get(self, endpoint, data):
+    def get(self, endpoint, data=None):
         """GET API request object"""
+        if data is None:
+            data = {}
         try:
             headers = {"Authorization": f"Token {self.token}"}
             url = f"{self.baseUrl}{endpoint}"
@@ -49,8 +51,10 @@ def get(self, endpoint, data):
             meta = {"status": result.status_code, "response": result.json()}
             return (True, False, meta)
 
-    def create(self, endpoint, data, body=True):
+    def create(self, endpoint, data=None, body=True):
         """POST API request object"""
+        if data is None:
+            data = {}
         try:
             headers = {"Authorization": f"Token {self.token}"}
             url = f"{self.baseUrl}{endpoint}"
@@ -62,15 +66,17 @@ def create(self, endpoint, data, body=True):
             raise Exception("API request failed")
 
         if result.status_code in [200, 201, 204]:
-            return (False, False, result.json())
+            return (False, True, result.json())
         elif result.status_code == 401:
             return (True, False, result.content)
         else:
             meta = {"status": result.status_code, "response": result.json()}
             return (True, False, meta)
 
-    def update(self, endpoint, data):
+    def update(self, endpoint, data=None):
         """PATCH API request object"""
+        if data is None:
+            data = {}
         try:
             headers = {"Authorization": f"Token {self.token}"}
             url = f"{self.baseUrl}{endpoint}"
@@ -79,15 +85,17 @@ def update(self, endpoint, data):
             raise Exception("API request failed")
 
         if result.status_code in [200, 201, 204]:
-            return (False, False, result.json())
+            return (False, True, result.json())
         elif result.status_code == 401:
             return (True, False, result.content)
         else:
             meta = {"status": result.status_code, "response": result.json()}
             return (True, False, meta)
 
-    def put(self, endpoint, data):
+    def put(self, endpoint, data=None):
         """PUT API request object"""
+        if data is None:
+            data = {}
         try:
             headers = {"Authorization": f"Token {self.token}"}
             url = f"{self.baseUrl}{endpoint}"
@@ -96,7 +104,7 @@ def put(self, endpoint, data):
             raise Exception("API request failed")
 
         if result.status_code in [200, 201, 204]:
-            return (False, False, result.json())
+            return (False, True, result.json())
         elif result.status_code == 401:
             return (True, False, result.content)
         else:
@@ -116,7 +124,7 @@ def delete(self, endpoint, body=False):
             raise Exception("API request failed")
 
         if result.status_code in [200, 201, 204]:
-            return (False, False, result.json())
+            return (False, True, result.json())
         elif result.status_code == 401:
             return (True, False, result.content)
         else:

plugins/modules/b1_a_record.py

@@ -166,7 +166,7 @@ def get_a_record(data):
         return connector.get("/api/ddi/v1/dns/record")
 
 
-def update_a_record(data):
+def update_a_record_name(data):
     """Updates the existing BloxOne DDI DNS Authoritative Zone object"""
     connector = Request(data["host"], data["api_key"])
     helper = Utilities()
@@ -206,18 +206,37 @@ def update_a_record(data):
     return connector.update(endpoint, payload)
 
 
+def update_a_record(connector, data, reference):
+    """Updates the existing BloxOne DDI DNS Authoritative Zone object"""
+    helper = Utilities()
+    payload = {}
+    old_data = reference[2]["results"][0]
+    ref_id = old_data["id"]
+    if old_data["rdata"]["address"] != data["address"]:
+        payload["rdata"] = {"address": data["address"]}
+    endpoint = f"/api/ddi/v1/{ref_id}"
+    if payload:
+        return connector.update(endpoint, payload)
+    else:
+        return False
+
+
 def create_a_record(data):
     """Creates a new BloxOne DDI DNS Authoritative Zone object"""
     connector = Request(data["host"], data["api_key"])
     helper = Utilities()
     if all(k in data and data[k] is not None for k in ("name", "zone")):
         if "new_name" in data["name"]:
-            return update_a_record(data)
+            return update_a_record_name(data)
         else:
             auth_zone = get_a_record(data)
             payload = {}
             if "results" in auth_zone[2].keys() and len(auth_zone[2]["results"]) > 0:
-                return update_a_record(data)
+                updated = update_a_record(connector, data, auth_zone)
+                if updated:
+                    return updated
+                else:
+                    return auth_zone
             else:
                 zone_endpoint = f"/api/ddi/v1/dns/auth_zone?_filter=fqdn==\"{data['zone']}\""
                 zone = connector.get(zone_endpoint)
@@ -263,9 +282,9 @@ def delete_a_record(data):
             return connector.delete(endpoint)
         else:
             return (
-                True,
                 False,
-                {"status": "400", "response": "Object not found", "data": data},
+                False,
+                {"status": "200", "response": "Object not found", "data": data},
             )
     else:
         return (
@@ -279,7 +298,7 @@ def main():
     """Main entry point for module execution"""
     argument_spec = dict(
         zone=dict(type="str"),
-        api_key=dict(required=True, type="str"),
+        api_key=dict(required=True, type="str", no_log=True),
         host=dict(required=True, type="str"),
         name=dict(type="str"),
         address=dict(type="str"),

plugins/modules/b1_cname_record.py

@@ -163,7 +163,7 @@ def get_cname_record(data):
         return connector.get(endpoint)
 
 
-def update_cname_record(data):
+def update_cname_record_name(data):
     """Updates the existing BloxOne DDI DNS Authoritative Zone object"""
     connector = Request(data["host"], data["api_key"])
     helper = Utilities()
@@ -203,18 +203,37 @@ def update_cname_record(data):
     return connector.update(endpoint, payload)
 
 
+def update_cname_record(connector, data, reference):
+    """Updates the existing BloxOne DDI DNS Authoritative Zone object"""
+    helper = Utilities()
+    payload = {}
+    old_data = reference[2]["results"][0]
+    ref_id = old_data["id"]
+    if old_data["rdata"]["cname"] != data["can_name"]:
+        payload["rdata"] = {"cname": data["can_name"]}
+    endpoint = f"/api/ddi/v1/{ref_id}"
+    if payload:
+        return connector.update(endpoint, payload)
+    else:
+        return False
+
+
 def create_cname_record(data):
     """Creates a new BloxOne DDI DNS Authoritative Zone object"""
     connector = Request(data["host"], data["api_key"])
     helper = Utilities()
     if all(k in data and data[k] is not None for k in ("name", "zone")):
         if "new_name" in data["name"]:
-            return update_cname_record(data)
+            return update_cname_record_name(data)
         else:
             auth_zone = get_cname_record(data)
             payload = {}
             if "results" in auth_zone[2].keys() and len(auth_zone[2]["results"]) > 0:
-                return update_cname_record(data)
+                updated = update_cname_record(connector, data, auth_zone)
+                if updated:
+                    return updated
+                else:
+                    return auth_zone
             else:
                 zone_endpoint = f"/api/ddi/v1/dns/auth_zone?_filter=fqdn==\"{data['zone']}\""
                 zone = connector.get(zone_endpoint)
@@ -260,9 +279,9 @@ def delete_cname_record(data):
             return connector.delete(endpoint)
         else:
             return (
-                True,
                 False,
-                {"status": "400", "response": "Object not found", "data": data},
+                False,
+                {"status": "200", "response": "Object not found", "data": data},
             )
     else:
         return (
@@ -276,7 +295,7 @@ def main():
     """Main entry point for module execution"""
     argument_spec = dict(
         zone=dict(type="str"),
-        api_key=dict(required=True, type="str"),
+        api_key=dict(required=True, type="str", no_log=True),
         host=dict(required=True, type="str"),
         name=dict(type="str"),
         can_name=dict(type="str"),

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

@@ -0,0 +1,109 @@
+---
+# Integration test for b1_a_record
+- name: Create A record
+  infoblox.bloxone.b1_a_record:
+    host: "{{ csp_url }}"
+    api_key: "{{ api_key }}"
+    zone: "{{ test_bloxone_dns_a_record.zone }}"
+    state: present
+    name: "{{ test_bloxone_dns_a_record.name }}"
+    address: "{{ test_bloxone_dns_a_record.ip_address }}"
+  delegate_to: localhost
+  register: create_a_record_01
+
+- name: Create A record (idempotence)
+  infoblox.bloxone.b1_a_record:
+    host: "{{ csp_url }}"
+    api_key: "{{ api_key }}"
+    zone: "{{ test_bloxone_dns_a_record.zone }}"
+    state: present
+    name: "{{ test_bloxone_dns_a_record.name }}"
+    address: "{{ test_bloxone_dns_a_record.ip_address }}"
+  delegate_to: localhost
+  register: create_a_record_02
+
+- name: Assert record was created
+  ansible.builtin.assert:
+    that:
+      - create_a_record_01.meta.result.dns_rdata == test_bloxone_dns_a_record.ip_address
+      - create_a_record_01.meta.result.dns_name_in_zone == test_bloxone_dns_a_record.name
+      - create_a_record_01.meta.result.dns_absolute_zone_name == test_bloxone_dns_a_record.zone + '.'
+      - create_a_record_01.changed
+      - not create_a_record_02.changed
+
+- name: Get A record
+  infoblox.bloxone.b1_a_record:
+    host: "{{ csp_url }}"
+    api_key: "{{ api_key }}"
+    zone: "{{ test_bloxone_dns_a_record.zone }}"
+    state: get
+    name: "{{ test_bloxone_dns_a_record.name }}"
+    address: "{{ test_bloxone_dns_a_record.ip_address }}"
+  delegate_to: localhost
+  register: get_a_record_01
+
+- name: Assert that record exists
+  ansible.builtin.assert:
+    that:
+      - get_a_record_01.meta.results[0].dns_rdata == test_bloxone_dns_a_record.ip_address
+      - get_a_record_01.meta.results[0].dns_name_in_zone == test_bloxone_dns_a_record.name
+      - get_a_record_01.meta.results[0].dns_absolute_zone_name == test_bloxone_dns_a_record.zone + '.'
+      - not get_a_record_01.changed
+
+- name: Update A record IP Address
+  infoblox.bloxone.b1_a_record:
+    host: "{{ csp_url }}"
+    api_key: "{{ api_key }}"
+    zone: "{{ test_bloxone_dns_a_record.zone }}"
+    state: present
+    name: "{{ test_bloxone_dns_a_record.name }}"
+    address: "{{ test_bloxone_dns_a_record.ip_address_update }}"
+  delegate_to: localhost
+  register: update_a_record_01
+
+# Updating name appears to not work at all when expanding variables for the 'name' option, doesn't play nice with json.loads() maybe?
+# Leaving this test in for posterity, since you *CAN* get it to work by hard-coding the record new and old name
+- name: Update A record Record Name
+  tags: never
+  infoblox.bloxone.b1_a_record:
+    host: "{{ csp_url }}"
+    api_key: "{{ api_key }}"
+    zone: "{{ test_bloxone_dns_a_record.zone }}"
+    state: present
+    name: '{"new_name": "", "old_name": ""}'
+    address: "{{ test_bloxone_dns_a_record.ip_address_update }}"
+  delegate_to: localhost
+  register: update_a_record_02
+
+- name: Assert that record exists
+  ansible.builtin.assert:
+    that:
+      - update_a_record_01.meta.result.dns_rdata == test_bloxone_dns_a_record.ip_address_update
+      - update_a_record_01.changed
+
+- name: Delete A record
+  infoblox.bloxone.b1_a_record:
+    host: "{{ csp_url }}"
+    api_key: "{{ api_key }}"
+    zone: "{{ test_bloxone_dns_a_record.zone }}"
+    state: absent
+    name: "{{ test_bloxone_dns_a_record.name }}"
+  delegate_to: localhost
+  register: delete_a_record_01
+
+- name: Delete A record (idempotence)
+  infoblox.bloxone.b1_a_record:
+    host: "{{ csp_url }}"
+    api_key: "{{ api_key }}"
+    zone: "{{ test_bloxone_dns_a_record.zone }}"
+    state: absent
+    name: "{{ test_bloxone_dns_a_record.name }}"
+  delegate_to: localhost
+  register: delete_a_record_02
+
+- name: Assert that record was deleted
+  ansible.builtin.assert:
+    that:
+      - delete_a_record_01.changed
+      - not delete_a_record_02.changed
+...