Fixed the issue-'For nonexistent object, update operation creates a n…

…ew object' (#129)

* Fixed the issue-'For nonexistent object, update operation creates a new object'
* Fixed sanity issues
* Fixed unit tests failure

plugins/module_utils/api.py

@@ -526,6 +526,7 @@ def get_object_ref(self, module, ib_obj_type, obj_filter, ib_spec):
 
         update = False
         old_name = new_name = None
+        old_ipv4addr_exists = old_text_exists = False
         if ('name' in obj_filter):
             # gets and returns the current object based on name/old_name passed
             try:
@@ -545,8 +546,7 @@ def get_object_ref(self, module, ib_obj_type, obj_filter, ib_spec):
                 if ib_obj:
                     obj_filter['name'] = new_name
                 else:
-                    test_obj_filter['name'] = new_name
-                    ib_obj = self.get_object(ib_obj_type, test_obj_filter, return_fields=list(ib_spec.keys()))
+                    raise Exception("object with name: '%s' is not found" % (old_name))
                 update = True
                 return ib_obj, update, new_name
             if (ib_obj_type == NIOS_HOST_RECORD):
@@ -569,6 +569,7 @@ def get_object_ref(self, module, ib_obj_type, obj_filter, ib_spec):
                 try:
                     ipaddr_obj = check_type_dict(obj_filter['ipv4addr'])
                     ipaddr = ipaddr_obj.get('old_ipv4addr')
+                    old_ipv4addr_exists = True
                 except TypeError:
                     ipaddr = obj_filter['ipv4addr']
                 test_obj_filter['ipv4addr'] = ipaddr
@@ -581,11 +582,13 @@ def get_object_ref(self, module, ib_obj_type, obj_filter, ib_spec):
                         try:
                             text_obj = json.loads(text_obj)
                             txt = text_obj['old_text']
+                            old_text_exists = True
                         except Exception:
                             (result, exc) = safe_eval(text_obj, dict(), include_exceptions=True)
                             if exc is not None:
                                 raise TypeError('unable to evaluate string as dictionary')
                             txt = result['old_text']
+                            old_text_exists = True
                     else:
                         txt = text_obj
                 except TypeError:
@@ -595,16 +598,30 @@ def get_object_ref(self, module, ib_obj_type, obj_filter, ib_spec):
             else:
                 test_obj_filter = obj_filter
             ib_obj = self.get_object(ib_obj_type, test_obj_filter.copy(), return_fields=list(ib_spec.keys()))
+
+            # prevents creation of a new A record with 'new_ipv4addr' when A record with a particular 'old_ipv4addr' is not found
+            if old_ipv4addr_exists and ib_obj is None:
+                raise Exception("A Record with ipv4addr: '%s' is not found" % (ipaddr))
+            # prevents creation of a new TXT record with 'new_text' when TXT record with a particular 'old_text' is not found
+            if old_text_exists and ib_obj is None:
+                raise Exception("TXT Record with text: '%s' is not found" % (txt))
         elif (ib_obj_type == NIOS_A_RECORD):
             # resolves issue where multiple a_records with same name and different IP address
             test_obj_filter = obj_filter
             try:
                 ipaddr_obj = check_type_dict(obj_filter['ipv4addr'])
                 ipaddr = ipaddr_obj.get('old_ipv4addr')
+                old_ipv4addr_exists = True
             except TypeError:
                 ipaddr = obj_filter['ipv4addr']
             test_obj_filter['ipv4addr'] = ipaddr
+            # prevents creation of a new A record with 'new_ipv4addr' when A record with a particular 'old_ipv4addr' is not found
+            if old_ipv4addr_exists and ib_obj is None:
+                raise Exception("A Record with ipv4addr: '%s' is not found" % (ipaddr))
             ib_obj = self.get_object(ib_obj_type, test_obj_filter.copy(), return_fields=list(ib_spec.keys()))
+            # prevents creation of a new A record with 'new_ipv4addr' when A record with a particular 'old_ipv4addr' is not found
+            if old_ipv4addr_exists and ib_obj is None:
+                raise Exception("A Record with ipv4addr: '%s' is not found" % (ipaddr))
         elif (ib_obj_type == NIOS_TXT_RECORD):
             # resolves issue where multiple txt_records with same name and different text
             test_obj_filter = obj_filter
@@ -614,17 +631,22 @@ def get_object_ref(self, module, ib_obj_type, obj_filter, ib_spec):
                     try:
                         text_obj = json.loads(text_obj)
                         txt = text_obj['old_text']
+                        old_text_exists = True
                     except Exception:
                         (result, exc) = safe_eval(text_obj, dict(), include_exceptions=True)
                         if exc is not None:
                             raise TypeError('unable to evaluate string as dictionary')
                         txt = result['old_text']
+                        old_text_exists = True
                 else:
                     txt = text_obj
             except TypeError:
                 txt = obj_filter['text']
             test_obj_filter['text'] = txt
             ib_obj = self.get_object(ib_obj_type, test_obj_filter.copy(), return_fields=list(ib_spec.keys()))
+            # prevents creation of a new TXT record with 'new_text' when TXT record with a particular 'old_text' is not found
+            if old_text_exists and ib_obj is None:
+                raise Exception("TXT Record with text: '%s' is not found" % (txt))
         elif (ib_obj_type == NIOS_ZONE):
             # del key 'restart_if_needed' as nios_zone get_object fails with the key present
             temp = ib_spec['restart_if_needed']

tests/unit/plugins/module_utils/test_api.py

@@ -8,6 +8,7 @@
 
 from ansible_collections.infoblox.nios_modules.tests.unit.compat import unittest
 from ansible_collections.infoblox.nios_modules.tests.unit.compat.mock import patch, MagicMock, Mock
+from ansible.module_utils.common.validation import check_type_dict
 from ansible_collections.infoblox.nios_modules.plugins.module_utils import api
 
 
@@ -22,7 +23,7 @@ def setUp(self):
 
         self.mock_connector = patch('ansible_collections.infoblox.nios_modules.plugins.module_utils.api.get_connector')
         self.mock_connector.start()
-        self.mock_check_type_dict = patch('ansible_collections.infoblox.nios_modules.plugins.module_utils.api.check_type_dict')
+        self.mock_check_type_dict = patch('ansible.module_utils.common.validation.check_type_dict')
         self.mock_check_type_dict_obj = self.mock_check_type_dict.start()
 
     def tearDown(self):
@@ -58,7 +59,7 @@ def test_wapi_no_change(self):
             {
                 "comment": "test comment",
                 "_ref": "networkview/ZG5zLm5ldHdvcmtfdmlldyQw:default/true",
-                "name": self.mock_check_type_dict_obj().__getitem__().lower(),
+                "name": 'default',
                 "extattrs": {}
             }
         ]
@@ -146,7 +147,7 @@ def test_wapi_extattrs_change(self):
 
         kwargs = copy.deepcopy(test_object[0])
         kwargs['extattrs']['Site']['value'] = 'update'
-        kwargs['name'] = self.mock_check_type_dict_obj().__getitem__().lower()
+        kwargs['name'] = 'default'
         del kwargs['_ref']
 
         wapi = self._get_wapi(test_object)
@@ -162,7 +163,7 @@ def test_wapi_extattrs_nochange(self):
         test_object = [{
             "comment": "test comment",
             "_ref": "networkview/ZG5zLm5ldHdvcmtfdmlldyQw:default/true",
-            "name": self.mock_check_type_dict_obj().__getitem__().lower(),
+            "name": "default",
             "extattrs": {'Site': {'value': 'test'}}
         }]
 
@@ -193,7 +194,7 @@ def test_wapi_create(self):
         res = wapi.run('testobject', test_spec)
 
         self.assertTrue(res['changed'])
-        wapi.create_object.assert_called_once_with('testobject', {'name': self.mock_check_type_dict_obj().__getitem__().lower()})
+        wapi.create_object.assert_called_once_with('testobject', {'name': 'ansible'})
 
     def test_wapi_delete(self):
         self.module.params = {'provider': None, 'state': 'absent', 'name': 'ansible',
@@ -243,7 +244,7 @@ def test_wapi_strip_network_view(self):
         kwargs = test_object[0].copy()
         ref = kwargs.pop('_ref')
         kwargs['comment'] = 'updated comment'
-        kwargs['name'] = self.mock_check_type_dict_obj().__getitem__().lower()
+        kwargs['name'] = 'ansible'
         del kwargs['network_view']
         del kwargs['extattrs']
 

tests/unit/plugins/modules/test_nios_a_record.py

@@ -23,6 +23,7 @@
 from ansible_collections.infoblox.nios_modules.plugins.modules import nios_a_record
 from ansible_collections.infoblox.nios_modules.plugins.module_utils import api
 from ansible_collections.infoblox.nios_modules.tests.unit.compat.mock import patch, MagicMock, Mock
+from ansible.module_utils.common.validation import check_type_dict
 from .test_nios_module import TestNiosModule, load_fixture
 
 
@@ -40,7 +41,7 @@ def setUp(self):
         self.mock_wapi_run = patch('ansible_collections.infoblox.nios_modules.plugins.modules.nios_a_record.WapiModule.run')
         self.mock_wapi_run.start()
         self.load_config = self.mock_wapi_run.start()
-        self.mock_check_type_dict = patch('ansible_collections.infoblox.nios_modules.plugins.module_utils.api.check_type_dict')
+        self.mock_check_type_dict = patch('ansible.module_utils.common.validation.check_type_dict')
         self.mock_check_type_dict_obj = self.mock_check_type_dict.start()
 
     def tearDown(self):
@@ -79,7 +80,7 @@ def test_nios_a_record_create(self):
         res = wapi.run('testobject', test_spec)
 
         self.assertTrue(res['changed'])
-        wapi.create_object.assert_called_once_with('testobject', {'name': self.mock_check_type_dict_obj().__getitem__().lower(),
+        wapi.create_object.assert_called_once_with('testobject', {'name': 'a.ansible.com',
                                                                   'ipv4': '192.168.10.1'})
 
     def test_nios_a_record_update_comment(self):

tests/unit/plugins/modules/test_nios_aaaa_record.py

@@ -23,6 +23,7 @@
 from ansible_collections.infoblox.nios_modules.plugins.modules import nios_aaaa_record
 from ansible_collections.infoblox.nios_modules.plugins.module_utils import api
 from ansible_collections.infoblox.nios_modules.tests.unit.compat.mock import patch, MagicMock, Mock
+from ansible.module_utils.common.validation import check_type_dict
 from .test_nios_module import TestNiosModule, load_fixture
 
 
@@ -40,7 +41,7 @@ def setUp(self):
         self.mock_wapi_run = patch('ansible_collections.infoblox.nios_modules.plugins.modules.nios_aaaa_record.WapiModule.run')
         self.mock_wapi_run.start()
         self.load_config = self.mock_wapi_run.start()
-        self.mock_check_type_dict = patch('ansible_collections.infoblox.nios_modules.plugins.module_utils.api.check_type_dict')
+        self.mock_check_type_dict = patch('ansible.module_utils.common.validation.check_type_dict')
         self.mock_check_type_dict_obj = self.mock_check_type_dict.start()
 
     def tearDown(self):
@@ -79,7 +80,7 @@ def test_nios_aaaa_record_create(self):
         res = wapi.run('testobject', test_spec)
 
         self.assertTrue(res['changed'])
-        wapi.create_object.assert_called_once_with('testobject', {'name': self.mock_check_type_dict_obj().__getitem__().lower(),
+        wapi.create_object.assert_called_once_with('testobject', {'name': 'aaaa.ansible.com',
                                                                   'ipv6': '2001:0db8:85a3:0000:0000:8a2e:0370:7334'})
 
     def test_nios_aaaa_record_update_comment(self):

tests/unit/plugins/modules/test_nios_cname_record.py

@@ -23,6 +23,7 @@
 from ansible_collections.infoblox.nios_modules.plugins.modules import nios_cname_record
 from ansible_collections.infoblox.nios_modules.plugins.module_utils import api
 from ansible_collections.infoblox.nios_modules.tests.unit.compat.mock import patch, MagicMock, Mock
+from ansible.module_utils.common.validation import check_type_dict
 from .test_nios_module import TestNiosModule, load_fixture
 
 
@@ -40,7 +41,7 @@ def setUp(self):
         self.mock_wapi_run = patch('ansible_collections.infoblox.nios_modules.plugins.modules.nios_cname_record.WapiModule.run')
         self.mock_wapi_run.start()
         self.load_config = self.mock_wapi_run.start()
-        self.mock_check_type_dict = patch('ansible_collections.infoblox.nios_modules.plugins.module_utils.api.check_type_dict')
+        self.mock_check_type_dict = patch('ansible.module_utils.common.validation.check_type_dict')
         self.mock_check_type_dict_obj = self.mock_check_type_dict.start()
 
     def tearDown(self):
@@ -61,7 +62,7 @@ def load_fixtures(self, commands=None):
         self.exec_command.return_value = (0, load_fixture('nios_result.txt').strip(), None)
         self.load_config.return_value = dict(diff=None, session='session')
 
-    def test_nios_a_record_create(self):
+    def test_nios_cname_record_create(self):
         self.module.params = {'provider': None, 'state': 'present', 'name': 'cname.ansible.com',
                               'canonical': 'realhost.ansible.com', 'comment': None, 'extattrs': None}
 
@@ -79,10 +80,10 @@ def test_nios_a_record_create(self):
         res = wapi.run('testobject', test_spec)
 
         self.assertTrue(res['changed'])
-        wapi.create_object.assert_called_once_with('testobject', {'name': self.mock_check_type_dict_obj().__getitem__().lower(),
+        wapi.create_object.assert_called_once_with('testobject', {'name': 'cname.ansible.com',
                                                                   'canonical': 'realhost.ansible.com'})
 
-    def test_nios_a_record_update_comment(self):
+    def test_nios_cname_record_update_comment(self):
         self.module.params = {'provider': None, 'state': 'present', 'name': 'cname.ansible.com',
                               'canonical': 'realhost.ansible.com', 'comment': 'updated comment', 'extattrs': None}
 
@@ -108,7 +109,7 @@ def test_nios_a_record_update_comment(self):
 
         self.assertTrue(res['changed'])
 
-    def test_nios_a_record_remove(self):
+    def test_nios_cname_record_remove(self):
         self.module.params = {'provider': None, 'state': 'absent', 'name': 'cname.ansible.com',
                               'canonical': 'realhost.ansible.com', 'comment': None, 'extattrs': None}
 

tests/unit/plugins/modules/test_nios_dns_view.py

@@ -23,6 +23,7 @@
 from ansible_collections.infoblox.nios_modules.plugins.modules import nios_dns_view
 from ansible_collections.infoblox.nios_modules.plugins.module_utils import api
 from ansible_collections.infoblox.nios_modules.tests.unit.compat.mock import patch, MagicMock, Mock
+from ansible.module_utils.common.validation import check_type_dict
 from .test_nios_module import TestNiosModule, load_fixture
 
 
@@ -40,7 +41,7 @@ def setUp(self):
         self.mock_wapi_run = patch('ansible_collections.infoblox.nios_modules.plugins.modules.nios_dns_view.WapiModule.run')
         self.mock_wapi_run.start()
         self.load_config = self.mock_wapi_run.start()
-        self.mock_check_type_dict = patch('ansible_collections.infoblox.nios_modules.plugins.module_utils.api.check_type_dict')
+        self.mock_check_type_dict = patch('ansible.module_utils.common.validation.check_type_dict')
         self.mock_check_type_dict_obj = self.mock_check_type_dict.start()
 
     def tearDown(self):
@@ -78,7 +79,7 @@ def test_nios_dns_view_create(self):
         res = wapi.run('testobject', test_spec)
 
         self.assertTrue(res['changed'])
-        wapi.create_object.assert_called_once_with('testobject', {'name': self.mock_check_type_dict_obj().__getitem__().lower()})
+        wapi.create_object.assert_called_once_with('testobject', {'name': 'ansible-dns'})
 
     def test_nios_dns_view_update_comment(self):
         self.module.params = {'provider': None, 'state': 'present', 'name': 'ansible-dns',

tests/unit/plugins/modules/test_nios_host_record.py

@@ -21,6 +21,7 @@
 from ansible_collections.infoblox.nios_modules.plugins.modules import nios_host_record
 from ansible_collections.infoblox.nios_modules.plugins.module_utils import api
 from ansible_collections.infoblox.nios_modules.tests.unit.compat.mock import patch, MagicMock, Mock
+from ansible.module_utils.common.validation import check_type_dict
 from .test_nios_module import TestNiosModule, load_fixture
 
 
@@ -41,7 +42,7 @@ def setUp(self):
         self.mock_wapi_run.start()
 
         self.load_config = self.mock_wapi_run.start()
-        self.mock_check_type_dict = patch('ansible_collections.infoblox.nios_modules.plugins.module_utils.api.check_type_dict')
+        self.mock_check_type_dict = patch('ansible.module_utils.common.validation.check_type_dict')
         self.mock_check_type_dict_obj = self.mock_check_type_dict.start()
 
     def tearDown(self):
@@ -77,7 +78,7 @@ def test_nios_host_record_create(self):
         res = wapi.run('testobject', test_spec)
 
         self.assertTrue(res['changed'])
-        wapi.create_object.assert_called_once_with('testobject', {'name': self.mock_check_type_dict_obj().__getitem__().lower()})
+        wapi.create_object.assert_called_once_with('testobject', {'name': 'ansible'})
 
     def test_nios_host_record_remove(self):
         self.module.params = {'provider': None, 'state': 'absent', 'name': 'ansible',

tests/unit/plugins/modules/test_nios_mx_record.py

@@ -23,6 +23,7 @@
 from ansible_collections.infoblox.nios_modules.plugins.modules import nios_mx_record
 from ansible_collections.infoblox.nios_modules.plugins.module_utils import api
 from ansible_collections.infoblox.nios_modules.tests.unit.compat.mock import patch, MagicMock, Mock
+from ansible.module_utils.common.validation import check_type_dict
 from .test_nios_module import TestNiosModule, load_fixture
 
 
@@ -40,7 +41,7 @@ def setUp(self):
         self.mock_wapi_run = patch('ansible_collections.infoblox.nios_modules.plugins.modules.nios_mx_record.WapiModule.run')
         self.mock_wapi_run.start()
         self.load_config = self.mock_wapi_run.start()
-        self.mock_check_type_dict = patch('ansible_collections.infoblox.nios_modules.plugins.module_utils.api.check_type_dict')
+        self.mock_check_type_dict = patch('ansible.module_utils.common.validation.check_type_dict')
         self.mock_check_type_dict_obj = self.mock_check_type_dict.start()
 
     def tearDown(self):
@@ -80,7 +81,7 @@ def test_nios_mx_record_create(self):
         res = wapi.run('testobject', test_spec)
 
         self.assertTrue(res['changed'])
-        wapi.create_object.assert_called_once_with('testobject', {'name': self.mock_check_type_dict_obj().__getitem__().lower(),
+        wapi.create_object.assert_called_once_with('testobject', {'name': 'ansible.com',
                                                    'mx': 'mailhost.ansible.com', 'preference': 0})
 
     def test_nios_mx_record_update_comment(self):

tests/unit/plugins/modules/test_nios_naptr_record.py

@@ -23,6 +23,7 @@
 from ansible_collections.infoblox.nios_modules.plugins.modules import nios_naptr_record
 from ansible_collections.infoblox.nios_modules.plugins.module_utils import api
 from ansible_collections.infoblox.nios_modules.tests.unit.compat.mock import patch, MagicMock, Mock
+from ansible.module_utils.common.validation import check_type_dict
 from .test_nios_module import TestNiosModule, load_fixture
 
 
@@ -40,7 +41,7 @@ def setUp(self):
         self.mock_wapi_run = patch('ansible_collections.infoblox.nios_modules.plugins.modules.nios_naptr_record.WapiModule.run')
         self.mock_wapi_run.start()
         self.load_config = self.mock_wapi_run.start()
-        self.mock_check_type_dict = patch('ansible_collections.infoblox.nios_modules.plugins.module_utils.api.check_type_dict')
+        self.mock_check_type_dict = patch('ansible.module_utils.common.validation.check_type_dict')
         self.mock_check_type_dict_obj = self.mock_check_type_dict.start()
 
     def tearDown(self):
@@ -82,7 +83,7 @@ def test_nios_naptr_record_create(self):
         res = wapi.run('testobject', test_spec)
 
         self.assertTrue(res['changed'])
-        wapi.create_object.assert_called_once_with('testobject', {'name': self.mock_check_type_dict_obj().__getitem__().lower(),
+        wapi.create_object.assert_called_once_with('testobject', {'name': '*.subscriber-100.ansiblezone.com',
                                                                   'order': '1000', 'preference': '10',
                                                                   'replacement': 'replacement1.network.ansiblezone.com'})