meraki - Rewrite update requirement check (#48394)

* Rewrite idempotency check
- Check now operates recursively and works on multiple types
- Order of lists matter

* Remove blank line for lint

* Fixed idempotency checks in meraki_ssid
- New sanitize() method for finding keys unique in compared dicts
- Fixed bug in meraki_ssid where SSID specified by number breaks
  - This will require a backport
- Converted ignored_keys from tuple to list

* Made changes required for idempotency

* Add changelog fragment

* Add unidirectional option for testing

* Disable option 1 check

* General fixes for is_update_required testing
- Added commented out debug statements in method
- Fixed ignored_keys modifications

* Remove old commented algorithm

changelogs/fragments/48394-meraki-idempotency-change.yml

@@ -0,0 +1,2 @@
+minor_changes:
+  - "meraki_* - Idempotency check has been rewritten. The new version is more thorough."

lib/ansible/module_utils/network/meraki/meraki.py

@@ -70,6 +70,7 @@ class MerakiModule(object):
         self.original = None
         self.proposed = dict()
         self.merged = None
+        self.ignored_keys = ['id', 'organizationId']
 
         # debug output
         self.filter_string = ''
@@ -84,7 +85,7 @@ class MerakiModule(object):
                          'network': '/organizations/{org_id}/networks',
                          'admins': '/organizations/{org_id}/admins',
                          'configTemplates': '/organizations/{org_id}/configTemplates',
-                         'samlRoles': '/organizations/{org_id}/samlRoles',
+                         'samlymbols': '/organizations/{org_id}/samlRoles',
                          'ssids': '/networks/{net_id}/ssids',
                          'groupPolicies': '/networks/{net_id}/groupPolicies',
                          'staticRoutes': '/networks/{net_id}/staticRoutes',
@@ -128,30 +129,48 @@ class MerakiModule(object):
         else:
             self.params['protocol'] = 'http'
 
-    def is_update_required(self, original, proposed, optional_ignore=None):
+    def sanitize(self, original, proposed):
-        """Compare original and proposed data to see if an update is needed."""
+        """Determine which keys are unique to original"""
-        is_changed = False
+        keys = []
-        ignored_keys = ('id', 'organizationId')
+        for k, v in original.items():
-        if not optional_ignore:
-            optional_ignore = ('')
-
-        # for k, v in original.items():
-        #     try:
-        #         if k not in ignored_keys and k not in optional_ignore:
-        #             if v != proposed[k]:
-        #                 is_changed = True
-        #     except KeyError:
-        #         if v != '':
-        #             is_changed = True
-        for k, v in proposed.items():
             try:
-                if k not in ignored_keys and k not in optional_ignore:
+                if proposed[k] and k not in self.ignored_keys:
-                    if v != original[k]:
+                    pass
-                        is_changed = True
             except KeyError:
-                if v != '':
+                keys.append(k)
-                    is_changed = True
+        return keys
-        return is_changed
+
+    def is_update_required(self, original, proposed, optional_ignore=None):
+        ''' Compare two data-structures '''
+        self.ignored_keys.append('net_id')
+        if optional_ignore is not None:
+            self.ignored_keys = self.ignored_keys + optional_ignore
+
+        if type(original) != type(proposed):
+            # self.fail_json(msg="Types don't match")
+            return True
+        if isinstance(original, list):
+            if len(original) != len(proposed):
+                # self.fail_json(msg="Length of lists don't match")
+                return True
+            for a, b in zip(original, proposed):
+                if self.is_update_required(a, b):
+                    # self.fail_json(msg="List doesn't match", a=a, b=b)
+                    return True
+        elif isinstance(original, dict):
+            for k, v in proposed.items():
+                if k not in self.ignored_keys:
+                    if k in original:
+                        if self.is_update_required(original[k], proposed[k]):
+                            return True
+                    else:
+                        # self.fail_json(msg="Key not in original", k=k)
+                        return True
+        else:
+            if original != proposed:
+                # self.fail_json(msg="Fallback", original=original, proposed=proposed)
+                return True
+        return False
 
     def get_orgs(self):
         """Downloads all organizations for a user."""

lib/ansible/modules/network/meraki/meraki_device.py

@@ -331,9 +331,12 @@ def main():
                 path = meraki.construct_path('get_all', net_id=net_id)
                 devices = meraki.request(path, method='GET')
                 for unit in devices:
+                    try:
                         if unit['name'] == meraki.params['hostname']:
                             device.append(unit)
                             meraki.result['data'] = device
+                    except KeyError:
+                        pass
             elif meraki.params['model']:
                 path = meraki.construct_path('get_all', net_id=net_id)
                 devices = meraki.request(path, method='GET')
@@ -372,6 +375,7 @@ def main():
                 query_path = meraki.construct_path('get_device', net_id=net_id) + meraki.params['serial']
                 device_data = meraki.request(query_path, method='GET')
                 ignore_keys = ['lanIp', 'serial', 'mac', 'model', 'networkId', 'moveMapMarker', 'wan1Ip', 'wan2Ip']
+                # meraki.fail_json(msg="Compare", original=device_data, payload=payload, ignore=ignore_keys)
                 if meraki.is_update_required(device_data, payload, optional_ignore=ignore_keys):
                     path = meraki.construct_path('update', net_id=net_id) + meraki.params['serial']
                     updated_device = []

lib/ansible/modules/network/meraki/meraki_snmp.py

@@ -201,7 +201,7 @@ def set_snmp(meraki, org_id):
         full_compare['v2cEnabled'] = False
     path = meraki.construct_path('create', org_id=org_id)
     snmp = get_snmp(meraki, org_id)
-    ignored_parameters = ('v3AuthPass', 'v3PrivPass', 'hostname', 'port', 'v2CommunityString', 'v3User')
+    ignored_parameters = ['v3AuthPass', 'v3PrivPass', 'hostname', 'port', 'v2CommunityString', 'v3User']
     if meraki.is_update_required(snmp, full_compare, optional_ignore=ignored_parameters):
         r = meraki.request(path,
                            method='PUT',

lib/ansible/modules/network/meraki/meraki_switchport.py

@@ -377,7 +377,7 @@ def main():
         original = meraki.request(query_path, method='GET')
         if meraki.params['type'] == 'trunk':
             proposed['voiceVlan'] = original['voiceVlan']  # API shouldn't include voice VLAN on a trunk port
-        if meraki.is_update_required(original, proposed, optional_ignore=('number')):
+        if meraki.is_update_required(original, proposed, optional_ignore=['number']):
             path = meraki.construct_path('update', custom={'serial': meraki.params['serial'],
                                                            'number': meraki.params['number'],
                                                            })

test/integration/targets/meraki_config_template/tasks/main.yml

@@ -14,7 +14,7 @@
       auth_key: '{{ auth_key }}'
       host: marrrraki.com
       state: query
-      org_name: DevTestOrg
+      org_name: '{{test_org_name}}'
       output_level: debug
     delegate_to: localhost
     register: invalid_domain

test/integration/targets/meraki_device/tasks/main.yml

@@ -1,5 +1,6 @@
 ---
 - block:
+  # This is commented out because a device cannot be unclaimed via API
   # - name: Claim a device into an organization
   #   meraki_device:
   #     auth_key: '{{auth_key}}'