From 7864df8cb9e3250bcd0d4471d4e37f632d49a93e Mon Sep 17 00:00:00 2001 From: Kevin Breit Date: Wed, 29 May 2019 09:18:01 -0500 Subject: [PATCH] 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 --- .../48394-meraki-idempotency-change.yml | 2 + .../module_utils/network/meraki/meraki.py | 65 ++++++++++++------- .../modules/network/meraki/meraki_device.py | 10 ++- .../modules/network/meraki/meraki_snmp.py | 2 +- .../network/meraki/meraki_switchport.py | 2 +- .../meraki_config_template/tasks/main.yml | 2 +- .../targets/meraki_device/tasks/main.yml | 1 + 7 files changed, 55 insertions(+), 29 deletions(-) create mode 100644 changelogs/fragments/48394-meraki-idempotency-change.yml diff --git a/changelogs/fragments/48394-meraki-idempotency-change.yml b/changelogs/fragments/48394-meraki-idempotency-change.yml new file mode 100644 index 00000000000..374568f6531 --- /dev/null +++ b/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." diff --git a/lib/ansible/module_utils/network/meraki/meraki.py b/lib/ansible/module_utils/network/meraki/meraki.py index 5494d5a79ac..443ea91e0d9 100644 --- a/lib/ansible/module_utils/network/meraki/meraki.py +++ b/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): - """Compare original and proposed data to see if an update is needed.""" - is_changed = False - ignored_keys = ('id', 'organizationId') - 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(): + def sanitize(self, original, proposed): + """Determine which keys are unique to original""" + keys = [] + for k, v in original.items(): try: - if k not in ignored_keys and k not in optional_ignore: - if v != original[k]: - is_changed = True + if proposed[k] and k not in self.ignored_keys: + pass except KeyError: - if v != '': - is_changed = True - return is_changed + keys.append(k) + return keys + + 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.""" diff --git a/lib/ansible/modules/network/meraki/meraki_device.py b/lib/ansible/modules/network/meraki/meraki_device.py index 110f62cf830..b64a6a05a84 100644 --- a/lib/ansible/modules/network/meraki/meraki_device.py +++ b/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: - if unit['name'] == meraki.params['hostname']: - device.append(unit) - meraki.result['data'] = device + 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 = [] diff --git a/lib/ansible/modules/network/meraki/meraki_snmp.py b/lib/ansible/modules/network/meraki/meraki_snmp.py index 72e839cb2d3..d57e1da1ec4 100644 --- a/lib/ansible/modules/network/meraki/meraki_snmp.py +++ b/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', diff --git a/lib/ansible/modules/network/meraki/meraki_switchport.py b/lib/ansible/modules/network/meraki/meraki_switchport.py index 3df90eec8ce..cd3aac9ed0a 100644 --- a/lib/ansible/modules/network/meraki/meraki_switchport.py +++ b/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'], }) diff --git a/test/integration/targets/meraki_config_template/tasks/main.yml b/test/integration/targets/meraki_config_template/tasks/main.yml index a6bdbfa764a..dd061e80786 100644 --- a/test/integration/targets/meraki_config_template/tasks/main.yml +++ b/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 diff --git a/test/integration/targets/meraki_device/tasks/main.yml b/test/integration/targets/meraki_device/tasks/main.yml index 7781adf3827..e0a973ab9bc 100644 --- a/test/integration/targets/meraki_device/tasks/main.yml +++ b/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}}'