From 17c475b101c3e2577eb7a887c54bbfd561cfa979 Mon Sep 17 00:00:00 2001 From: Kevin Breit Date: Wed, 12 Jun 2019 10:06:00 -0500 Subject: [PATCH] meraki_ssid - Improve change reporting (#56201) * Improve change reporting for meraki_ssid - Documentation is more clear about dependencies - Not all change reports are accurate without new algorithm - Improved integration tests * Rename changelog fragment * Enable all tests in integration tests - Fix type merging * Add more integration tests for code coverage * Update URL creation --- .../fragments/51561-meraki_ssid-fix.yml | 3 + .../modules/network/meraki/meraki_ssid.py | 30 ++- .../targets/meraki_ssid/tasks/main.yml | 216 +++++++++++++++--- 3 files changed, 212 insertions(+), 37 deletions(-) create mode 100644 changelogs/fragments/51561-meraki_ssid-fix.yml diff --git a/changelogs/fragments/51561-meraki_ssid-fix.yml b/changelogs/fragments/51561-meraki_ssid-fix.yml new file mode 100644 index 00000000000..ed58fe87b42 --- /dev/null +++ b/changelogs/fragments/51561-meraki_ssid-fix.yml @@ -0,0 +1,3 @@ +bugfixes: +- "meraki_ssid - Provides more accurate change results for some operations." +- "meraki_ssid - Improved documentation about parameter dependencies." \ No newline at end of file diff --git a/lib/ansible/modules/network/meraki/meraki_ssid.py b/lib/ansible/modules/network/meraki/meraki_ssid.py index b965d21e334..9c3c1203e31 100644 --- a/lib/ansible/modules/network/meraki/meraki_ssid.py +++ b/lib/ansible/modules/network/meraki/meraki_ssid.py @@ -110,6 +110,7 @@ options: secret: description: - RADIUS password. + - Setting password is not idempotent. type: str radius_coa_enabled: description: @@ -145,6 +146,7 @@ options: secret: description: - RADIUS password. + - Setting password is not idempotent. type: str ip_assignment_mode: description: @@ -158,18 +160,23 @@ options: use_vlan_tagging: description: - Set whether to use VLAN tagging. + - Requires C(default_vlan_id) to be set. type: bool default_vlan_id: description: - Default VLAN ID. + - Requires C(ip_assignment_mode) to be C(Bridge mode) or C(Layer 3 roaming). type: str vlan_id: description: - ID number of VLAN on SSID. + - Requires C(ip_assignment_mode) to be C(ayer 3 roaming with a concentrator) or C(VPN). type: int ap_tags_vlan_ids: description: - List of VLAN tags. + - Requires C(ip_assignment_mode) to be C(Bridge mode) or C(Layer 3 roaming). + - Requires C(use_vlan_tagging) to be C(True). type: list suboptions: tags: @@ -490,7 +497,7 @@ def main(): meraki.params['follow_redirects'] = 'all' query_urls = {'ssid': '/networks/{net_id}/ssids'} - query_url = {'ssid': '/networks/{net_id}/ssids/'} + query_url = {'ssid': '/networks/{net_id}/ssids/{number}'} update_url = {'ssid': '/networks/{net_id}/ssids/'} meraki.url_catalog['get_all'].update(query_urls) @@ -522,6 +529,9 @@ def main(): if meraki.params['auth_mode'] not in ('open-with-radius', '8021x-radius') or meraki.params['radius_accounting_enabled'] is False: meraki.fail_json(msg='radius_accounting_servers is only allowed when auth_mode is open_with_radius or 8021x-radius and \ radius_accounting_enabled is true') + if meraki.params['use_vlan_tagging'] is True: + if meraki.params['default_vlan_id'] is None: + meraki.fail_json(msg="default_vlan_id is required when use_vlan_tagging is True") # manipulate or modify the state as needed (this is going to be the # part where your module will do what it needs to do) @@ -536,10 +546,10 @@ def main(): if meraki.params['state'] == 'query': if meraki.params['name']: ssid_id = get_ssid_number(meraki.params['name'], get_ssids(meraki, net_id)) - path = meraki.construct_path('get_one', net_id=net_id) + str(ssid_id) + path = meraki.construct_path('get_one', net_id=net_id, custom={'number': ssid_id}) meraki.result['data'] = meraki.request(path, method='GET') - elif meraki.params['number']: - path = meraki.construct_path('get_one', net_id=net_id) + meraki.params['number'] + elif meraki.params['number'] is not None: + path = meraki.construct_path('get_one', net_id=net_id, custom={'number': meraki.params['number']}) meraki.result['data'] = meraki.request(path, method='GET') else: meraki.result['data'] = get_ssids(meraki, net_id) @@ -548,12 +558,21 @@ def main(): for k, v in param_map.items(): if meraki.params[v] is not None: payload[k] = meraki.params[v] + # Short term solution for camelCase/snake_case differences + # Will be addressed later with a method in module utils + if meraki.params['ap_tags_vlan_ids'] is not None: + for i in payload['apTagsAndVlanIds']: + try: + i['vlanId'] = i['vlan_id'] + del i['vlan_id'] + except KeyError: + pass ssids = get_ssids(meraki, net_id) number = meraki.params['number'] if number is None: number = get_ssid_number(meraki.params['name'], ssids) original = ssids[number] - if meraki.is_update_required(original, payload): + if meraki.is_update_required(original, payload, optional_ignore=['secret']): ssid_id = meraki.params['number'] if ssid_id is None: # Name should be used to lookup number ssid_id = get_ssid_number(meraki.params['name'], ssids) @@ -579,7 +598,6 @@ def main(): path = meraki.construct_path('update', net_id=net_id) + str(ssid_id) payload = default_payload payload['name'] = payload['name'] + ' ' + str(ssid_id + 1) - # meraki.fail_json(msg='Payload', payload=payload) result = meraki.request(path, 'PUT', payload=json.dumps(payload)) meraki.result['data'] = result meraki.result['changed'] = True diff --git a/test/integration/targets/meraki_ssid/tasks/main.yml b/test/integration/targets/meraki_ssid/tasks/main.yml index 2fd9c4dc90d..75d2b68cb84 100644 --- a/test/integration/targets/meraki_ssid/tasks/main.yml +++ b/test/integration/targets/meraki_ssid/tasks/main.yml @@ -9,16 +9,16 @@ msg: Please define an API key when: auth_key is not defined - # - name: Use an invalid domain - # meraki_organization: - # auth_key: '{{ auth_key }}' - # host: marrrraki.com - # state: present - # org_name: IntTestOrg - # output_level: debug - # delegate_to: localhost - # register: invalid_domain - # ignore_errors: yes + - name: Use an invalid domain + meraki_organization: + auth_key: '{{ auth_key }}' + host: marrrraki.com + state: present + org_name: IntTestOrg + output_level: debug + delegate_to: localhost + register: invalid_domain + ignore_errors: yes - name: Disable HTTP meraki_organization: @@ -113,6 +113,23 @@ that: - query_one.data.name == 'AnsibleSSID' + - name: Query one SSID with number + meraki_ssid: + auth_key: '{{auth_key}}' + state: query + org_name: '{{test_org_name}}' + net_name: TestNetSSID + number: 1 + delegate_to: localhost + register: query_one_number + + - debug: + msg: '{{query_one_number}}' + + - assert: + that: + - query_one_number.data.name == 'AnsibleSSID' + - name: Disable SSID without specifying number meraki_ssid: auth_key: '{{auth_key}}' @@ -149,46 +166,50 @@ that: - enable_ssid_number.data.enabled == True - - name: Set PSK with wrong mode + - name: Set VLAN arg spec meraki_ssid: auth_key: '{{auth_key}}' state: present org_name: '{{test_org_name}}' net_name: TestNetSSID - name: AnsibleSSID - auth_mode: open - psk: abc1234 + number: 1 + use_vlan_tagging: yes + ip_assignment_mode: Bridge mode + default_vlan_id: 1 + ap_tags_vlan_ids: + - tags: wifi + vlan_id: 2 delegate_to: localhost - register: psk_invalid - ignore_errors: yes + register: set_vlan_arg - - debug: - msg: '{{ psk_invalid }}' + - debug: + var: set_vlan_arg - assert: - that: - - psk_invalid.msg == 'PSK is only allowed when auth_mode is set to psk' + that: set_vlan_arg is changed - - name: Set PSK with invalid encryption mode + - name: Set VLAN arg spec meraki_ssid: auth_key: '{{auth_key}}' state: present org_name: '{{test_org_name}}' net_name: TestNetSSID - name: AnsibleSSID - auth_mode: psk - psk: abc1234 - encryption_mode: eap + number: 1 + use_vlan_tagging: yes + ip_assignment_mode: Bridge mode + default_vlan_id: 1 + ap_tags_vlan_ids: + - tags: wifi + vlan_id: 2 delegate_to: localhost - register: psk_invalid_mode - ignore_errors: yes + register: set_vlan_arg_idempotent - - debug: - msg: '{{ psk_invalid_mode }}' + - debug: + var: set_vlan_arg_idempotent - assert: - that: - - psk_invalid_mode.msg == 'PSK requires encryption_mode be set to wpa' + that: set_vlan_arg_idempotent is not changed + - name: Set PSK meraki_ssid: @@ -212,6 +233,26 @@ - psk.data.encryptionMode == 'wpa' - psk.data.wpaEncryptionMode == 'WPA2 only' + - name: Set PSK with idempotency + meraki_ssid: + auth_key: '{{auth_key}}' + state: present + org_name: '{{test_org_name}}' + net_name: TestNetSSID + name: AnsibleSSID + auth_mode: psk + psk: abc1234567890 + encryption_mode: wpa + delegate_to: localhost + register: psk_idempotent + + - debug: + msg: '{{ psk_idempotent }}' + + - assert: + that: + - psk_idempotent is not changed + - name: Enable click-through splash page meraki_ssid: auth_key: '{{auth_key}}' @@ -251,6 +292,119 @@ - assert: that: - set_radius_server.data.radiusServers.0.host == '192.0.1.200' + + - name: Configure RADIUS servers with idempotency + meraki_ssid: + auth_key: '{{auth_key}}' + state: present + org_name: '{{test_org_name}}' + net_name: TestNetSSID + name: AnsibleSSID + auth_mode: open-with-radius + radius_servers: + - host: 192.0.1.200 + port: 1234 + secret: abc98765 + delegate_to: localhost + register: set_radius_server_idempotent + + - debug: + var: set_radius_server_idempotent + + - assert: + that: + - set_radius_server_idempotent is not changed + + ################# + # Error testing # + ################# + - name: Set PSK with wrong mode + meraki_ssid: + auth_key: '{{auth_key}}' + state: present + org_name: '{{test_org_name}}' + net_name: TestNetSSID + name: AnsibleSSID + auth_mode: open + psk: abc1234 + delegate_to: localhost + register: psk_invalid + ignore_errors: yes + + - debug: + msg: '{{ psk_invalid }}' + + - assert: + that: + - psk_invalid.msg == 'PSK is only allowed when auth_mode is set to psk' + + - name: Set PSK with invalid encryption mode + meraki_ssid: + auth_key: '{{auth_key}}' + state: present + org_name: '{{test_org_name}}' + net_name: TestNetSSID + name: AnsibleSSID + auth_mode: psk + psk: abc1234 + encryption_mode: eap + delegate_to: localhost + register: psk_invalid_mode + ignore_errors: yes + + - debug: + msg: '{{ psk_invalid_mode }}' + + - assert: + that: + - psk_invalid_mode.msg == 'PSK requires encryption_mode be set to wpa' + + - name: Error for PSK and RADIUS servers + meraki_ssid: + auth_key: '{{auth_key}}' + state: present + org_name: '{{test_org_name}}' + net_name: TestNetSSID + name: AnsibleSSID + auth_mode: psk + radius_servers: + - host: 192.0.1.200 + port: 1234 + secret: abc98765 + delegate_to: localhost + register: err_radius_server_psk + ignore_errors: yes + + - debug: + var: err_radius_server_psk + + - assert: + that: + - 'err_radius_server_psk.msg == "radius_servers requires auth_mode to be open-with-radius or 8021x-radius"' + + - name: Set VLAN arg without default VLAN error + meraki_ssid: + auth_key: '{{auth_key}}' + state: present + org_name: '{{test_org_name}}' + net_name: TestNetSSID + number: 1 + use_vlan_tagging: yes + ip_assignment_mode: Bridge mode + ap_tags_vlan_ids: + - tags: wifi + vlan_id: 2 + delegate_to: localhost + register: set_vlan_arg_err + ignore_errors: yes + + - debug: + var: set_vlan_arg_err + + - assert: + that: + - 'set_vlan_arg_err.msg == "default_vlan_id is required when use_vlan_tagging is True"' + always: - name: Delete SSID meraki_ssid: