meraki_ssid - Improve change reporting (ansible#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
raphaelm · Jun 12, 2019 · 17c475b · 17c475b
1 parent 1fa7bfc
commit 17c475b
Show file tree

Hide file tree

Showing 3 changed files with 212 additions and 37 deletions.
diff --git a/changelogs/fragments/51561-meraki_ssid-fix.yml 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."
diff --git a/lib/ansible/modules/network/meraki/meraki_ssid.py b/lib/ansible/modules/network/meraki/meraki_ssid.py
@@ -110,6 +110,7 @@
             secret:
                 description:
                 - RADIUS password.
+                - Setting password is not idempotent.
                 type: str
     radius_coa_enabled:
         description:
@@ -145,6 +146,7 @@
             secret:
                 description:
                 - RADIUS password.
+                - Setting password is not idempotent.
                 type: str
     ip_assignment_mode:
         description:
@@ -158,18 +160,23 @@
     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
@@ -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: