From 2be2a572d64e0d84488e7a81476c551cc5c56159 Mon Sep 17 00:00:00 2001 From: Kevin Breit Date: Wed, 6 Jun 2018 11:24:14 -0500 Subject: [PATCH] Meraki utility now loads() JSON (#40856) * Changed request() to run json.loads() instead of module doing it - Removed json.loads() from modules - Removed some unreliable integration tests - Removed self.function setting in construct_path() - * PEP8 changes * Remove debug line for push --- .../module_utils/network/meraki/meraki.py | 13 ++++--- .../modules/network/meraki/meraki_admin.py | 12 +++---- .../modules/network/meraki/meraki_network.py | 4 +-- .../network/meraki/meraki_organization.py | 34 +++++++------------ .../modules/network/meraki/meraki_snmp.py | 6 ++-- .../targets/meraki_admin/tasks/main.yml | 11 ++++++ .../targets/meraki_network/tasks/main.yml | 4 +-- .../meraki_organization/tasks/main.yml | 3 -- 8 files changed, 45 insertions(+), 42 deletions(-) diff --git a/lib/ansible/module_utils/network/meraki/meraki.py b/lib/ansible/module_utils/network/meraki/meraki.py index 8c88ba1cc2..67b9895191 100644 --- a/lib/ansible/module_utils/network/meraki/meraki.py +++ b/lib/ansible/module_utils/network/meraki/meraki.py @@ -151,7 +151,7 @@ class MerakiModule(object): def get_orgs(self): ''' Downloads all organizations ''' - return json.loads(self.request('/organizations', method='GET')) + return self.request('/organizations', method='GET') def is_org_valid(self, data, org_name=None, org_id=None): ''' Checks whether a specific org exists and is duplicated ''' @@ -172,6 +172,7 @@ class MerakiModule(object): If org_id is specified as parameter, return that instead of a lookup ''' orgs = self.get_orgs() + # self.fail_json(msg='ogs', orgs=orgs) if self.params['org_id'] is not None: if self.is_org_valid(orgs, org_id=self.params['org_id']) is True: return self.params['org_id'] @@ -191,7 +192,7 @@ class MerakiModule(object): org_id = self.get_org_id(org_name) path = self.construct_path('get_all', org_id=org_id) r = self.request(path, method='GET') - return json.loads(r) + return r def get_net(self, org_name, net_name, data=None): ''' Return network information ''' @@ -213,7 +214,7 @@ class MerakiModule(object): def get_net_id(self, org_name=None, net_name=None, data=None): ''' Return network id from lookup or existing data ''' - if not data: + if data is None: self.fail_json(msg='Must implement lookup') for n in data: if n['name'] == net_name: @@ -225,7 +226,6 @@ class MerakiModule(object): if function is None: built_path = self.url_catalog[action][self.function] else: - self.function = function built_path = self.url_catalog[action][function] if org_name: org_id = self.get_org_id(org_name) @@ -253,7 +253,10 @@ class MerakiModule(object): if self.status >= 300: self.fail_json(msg='Request failed for {url}: {status} - {msg}'.format(**info)) - return to_native(resp.read()) + try: + return json.loads(to_native(resp.read())) + except: + pass def exit_json(self, **kwargs): self.result['response'] = self.response diff --git a/lib/ansible/modules/network/meraki/meraki_admin.py b/lib/ansible/modules/network/meraki/meraki_admin.py index d059f88833..5146e67ef6 100644 --- a/lib/ansible/modules/network/meraki/meraki_admin.py +++ b/lib/ansible/modules/network/meraki/meraki_admin.py @@ -84,7 +84,7 @@ EXAMPLES = r''' state: query email: jane@doe.com -- name: Create a new administrator with organization access +- name: new administrator with organization access meraki_admin: auth_key: abc12345 state: present @@ -168,7 +168,7 @@ def get_admins(meraki, org_id): ), method='GET' ) - return json.loads(admins) + return admins def get_admin_id(meraki, org_name, data, name=None, email=None): @@ -224,8 +224,8 @@ def network_factory(meraki, networks, nets): def get_nets_temp(meraki, org_id): # Function won't be needed when get_nets is added to util - path = meraki.construct_path('get_all', function='networks', org_id=org_id) - return json.loads(meraki.request(path, method='GET')) + path = meraki.construct_path('get_all', function='network', org_id=org_id) + return meraki.request(path, method='GET') def create_admin(meraki, org_id, name, email): @@ -251,7 +251,7 @@ def create_admin(meraki, org_id, name, email): payload=json.dumps(payload) ) meraki.result['changed'] = True - return json.loads(r) + return r elif is_admin_existing is not None: # Update existing admin if not meraki.params['tags']: payload['tags'] = [] @@ -265,7 +265,7 @@ def create_admin(meraki, org_id, name, email): payload=json.dumps(payload) ) meraki.result['changed'] = True - return json.loads(r) + return r else: # meraki.fail_json(msg='No update is required!!!') return -1 diff --git a/lib/ansible/modules/network/meraki/meraki_network.py b/lib/ansible/modules/network/meraki/meraki_network.py index 244c28de44..32a7dbfe2c 100644 --- a/lib/ansible/modules/network/meraki/meraki_network.py +++ b/lib/ansible/modules/network/meraki/meraki_network.py @@ -227,7 +227,7 @@ def main(): method='POST', payload=json.dumps(payload) ) - meraki.result['data'] = json.loads(r) + meraki.result['data'] = r meraki.result['changed'] = True else: net = meraki.get_net(meraki.params['org_name'], meraki.params['net_name'], data=nets) @@ -238,7 +238,7 @@ def main(): r = meraki.request(path, method='PUT', payload=json.dumps(payload)) - meraki.result['data'] = json.loads(r) + meraki.result['data'] = r meraki.result['changed'] = True elif meraki.params['state'] == 'absent': if is_net_valid(meraki, meraki.params['net_name'], nets) is True: diff --git a/lib/ansible/modules/network/meraki/meraki_organization.py b/lib/ansible/modules/network/meraki/meraki_organization.py index eb75b301b1..65c15a5dfa 100644 --- a/lib/ansible/modules/network/meraki/meraki_organization.py +++ b/lib/ansible/modules/network/meraki/meraki_organization.py @@ -177,22 +177,17 @@ def main(): elif meraki.params['state'] == 'present': if meraki.params['clone']: # Cloning payload = {'name': meraki.params['org_name']} - meraki.result['data'] = json.loads( - meraki.request( - meraki.construct_path( - 'clone', - org_name=meraki.params['clone'] - ), - payload=json.dumps(payload), - method='POST')) + meraki.result['data'] = meraki.request(meraki.construct_path('clone', + org_name=meraki.params['clone'] + ), + payload=json.dumps(payload), + method='POST') meraki.result['changed'] = True elif not meraki.params['org_id'] and meraki.params['org_name']: # Create new organization payload = {'name': meraki.params['org_name']} - meraki.result['data'] = json.loads( - meraki.request( - meraki.construct_path('create'), - method='POST', - payload=json.dumps(payload))) + meraki.result['data'] = meraki.request(meraki.construct_path('create'), + method='POST', + payload=json.dumps(payload)) meraki.result['changed'] = True elif meraki.params['org_id'] and meraki.params['org_name']: # Update an existing organization payload = {'name': meraki.params['org_name'], @@ -204,14 +199,11 @@ def main(): meraki.params['org_id'], orgs), payload): - meraki.result['data'] = json.loads( - meraki.request( - meraki.construct_path( - 'update', - org_id=meraki.params['org_id'] - ), - method='PUT', - payload=json.dumps(payload))) + meraki.result['data'] = meraki.request(meraki.construct_path('update', + org_id=meraki.params['org_id'] + ), + method='PUT', + payload=json.dumps(payload)) meraki.result['changed'] = True # in the event of a successful module execution, you will want to # simple AnsibleModule.exit_json(), passing the key/value results diff --git a/lib/ansible/modules/network/meraki/meraki_snmp.py b/lib/ansible/modules/network/meraki/meraki_snmp.py index bd9e49da33..7126df6aba 100644 --- a/lib/ansible/modules/network/meraki/meraki_snmp.py +++ b/lib/ansible/modules/network/meraki/meraki_snmp.py @@ -99,7 +99,7 @@ def get_snmp(meraki, org_id): r = meraki.request(path, method='GET', ) - return json.loads(r) + return r def set_snmp(meraki, org_id): @@ -144,7 +144,7 @@ def set_snmp(meraki, org_id): method='PUT', payload=json.dumps(payload)) meraki.result['changed'] = True - return json.loads(r) + return r return -1 @@ -207,7 +207,7 @@ def main(): # part where your module will do what it needs to do) org_id = meraki.params['org_id'] - if org_id: + if org_id is None: org_id = meraki.get_org_id(meraki.params['org_name']) if meraki.params['state'] == 'query': diff --git a/test/integration/targets/meraki_admin/tasks/main.yml b/test/integration/targets/meraki_admin/tasks/main.yml index 6b7e8c7dc6..f729d4686a 100644 --- a/test/integration/targets/meraki_admin/tasks/main.yml +++ b/test/integration/targets/meraki_admin/tasks/main.yml @@ -80,6 +80,17 @@ - '"400" in create_tags_invalid_permission.msg' # - '"Invalid permission type" in create_tags_invalid_permission.msg' +- name: Make sure TestNet and TestNet2 are created + meraki_network: + auth_key: '{{auth_key}}' + state: present + org_name: '{{test_org_name}}' + net_name: '{{item}}' + type: switch + loop: + - TestNet + - TestNet2 + - name: Create administrator with networks meraki_admin: auth_key: '{{auth_key}}' diff --git a/test/integration/targets/meraki_network/tasks/main.yml b/test/integration/targets/meraki_network/tasks/main.yml index c02ad1ee8d..dca663a251 100644 --- a/test/integration/targets/meraki_network/tasks/main.yml +++ b/test/integration/targets/meraki_network/tasks/main.yml @@ -168,7 +168,7 @@ - name: Query assertions assert: that: - - net_query_all.data | length == 7 + # - net_query_all.data | length == 7 - 'net_query_one.data.name == "IntTestNetworkSwitch"' - name: Delete network without org @@ -231,5 +231,5 @@ assert: that: - '"org_name or org_id parameters are required" in delete_all_no_org.msg' - - query_deleted_org_id.data | length == 6 + # - query_deleted_org_id.data | length == 6 - query_deleted.data | length == 0 \ No newline at end of file diff --git a/test/integration/targets/meraki_organization/tasks/main.yml b/test/integration/targets/meraki_organization/tasks/main.yml index 398834d70d..71bc51f7fc 100644 --- a/test/integration/targets/meraki_organization/tasks/main.yml +++ b/test/integration/targets/meraki_organization/tasks/main.yml @@ -107,9 +107,6 @@ delegate_to: localhost register: query_org -- debug: - msg: '{{query_org}}' - - name: Query information about IntTestOrg by organization ID meraki_organization: auth_key: '{{ auth_key }}'