meraki_network - Parameter change for combined network type (#49160)

* Added support for types parameter
- Parameter is used to specify multiple network types

* Fix documentation

* Apply suggestions from code review

Co-Authored-By: kbreit <kevin.breit@kevinbreit.net>

* Reworked type parameter to be a list so types isn't needed

* Re-add tags documentation

* Fix documentation around compatibility

* Convert tags to list from string

* Add changelog fragment

changelogs/fragments/49160-meraki_network-combined-type-change.yml

@@ -0,0 +1,2 @@
+minor_changes:
+  - "meraki_network - type parameter no longer accepts combined. Instead, the network types should be specified in a list."

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

@@ -1,7 +1,7 @@
 #!/usr/bin/python
 # -*- coding: utf-8 -*-
 
-# Copyright: (c) 2018, Kevin Breit (@kbreit) <kevin.breit@kevinbreit.net>
+# Copyright: (c) 2018, 2019 Kevin Breit (@kbreit) <kevin.breit@kevinbreit.net>
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
 from __future__ import absolute_import, division, print_function
@@ -28,12 +28,12 @@ options:
     state:
         description:
         - Create or modify an organization.
-        choices: [absent, present, query]
+        choices: [ absent, present, query ]
         default: present
     net_name:
         description:
         - Name of a network.
-        aliases: [name, network]
+        aliases: [ name, network ]
     net_id:
         description:
         - ID number of a network.
@@ -47,11 +47,17 @@ options:
         description:
         - Type of network device network manages.
         - Required when creating a network.
-        choices: [appliance, combined, switch, wireless]
-        aliases: [net_type]
+        - As of Ansible 2.8, C(combined) type is no longer accepted.
+        - As of Ansible 2.8, changes to this parameter are no longer idempotent.
+        choices: [ appliance, switch, wireless ]
+        aliases: [ net_type ]
+        type: list
     tags:
+        type: list
         description:
-        - Comma delimited list of tags to assign to network.
+        - List of tags to assign to network.
+        - C(tags) name conflicts with the tags parameter in Ansible. Indentation problems may cause unexpected behaviors.
+        - Ansible 2.8 converts this to a list from a comma separated list.
     timezone:
         description:
         - Timezone associated to network.
@@ -92,6 +98,18 @@ EXAMPLES = r'''
     timezone: America/Chicago
     tags: production, chicago
   delegate_to: localhost
+- name: Create combined network named MyNet in the YourOrg organization
+  meraki_network:
+    auth_key: abc12345
+    state: present
+    org_name: YourOrg
+    net_name: MyNet
+    type:
+      - switch
+      - appliance
+    timezone: America/Chicago
+    tags: production, chicago
+  delegate_to: localhost
 '''
 
 RETURN = r'''
@@ -152,16 +170,18 @@ def is_net_valid(meraki, net_name, data):
 
 
 def construct_tags(tags):
-    ''' Assumes tags are a comma separated list '''
-    if tags is not None:
-        tags = tags.replace(' ', '')
-        tags = tags.split(',')
-        tag_list = str()
-        for t in tags:
-            tag_list = tag_list + " " + t
-        tag_list = tag_list + " "
-        return tag_list
-    return None
+    formatted_tags = ' '.join(tags)
+    return ' {0} '.format(formatted_tags)  # Meraki needs space padding
+
+
+def list_to_string(data):
+    new_string = str()
+    for i, item in enumerate(data):
+        if i == len(new_string) - 1:
+            new_string += i
+        else:
+            new_string = "{0}{1} ".format(new_string, item)
+    return new_string.strip()
 
 
 def main():
@@ -172,8 +192,8 @@ def main():
     argument_spec = meraki_argument_spec()
     argument_spec.update(
         net_id=dict(type='str'),
-        type=dict(type='str', choices=['wireless', 'switch', 'appliance', 'combined'], aliases=['net_type']),
-        tags=dict(type='str'),
+        type=dict(type='list', choices=['wireless', 'switch', 'appliance'], aliases=['net_type']),
+        tags=dict(type='list'),
         timezone=dict(type='str'),
         net_name=dict(type='str', aliases=['name', 'network']),
         state=dict(type='str', choices=['present', 'query', 'absent'], default='present'),
@@ -219,9 +239,7 @@ def main():
         if meraki.params['net_name']:
             payload['name'] = meraki.params['net_name']
         if meraki.params['type']:
-            payload['type'] = meraki.params['type']
-            if meraki.params['type'] == 'combined':
-                payload['type'] = 'switch wireless appliance'
+            payload['type'] = list_to_string(meraki.params['type'])
         if meraki.params['tags']:
             payload['tags'] = construct_tags(meraki.params['tags'])
         if meraki.params['timezone']:
@@ -277,6 +295,19 @@ def main():
                 if meraki.status == 200:
                     meraki.result['data'] = r
                     meraki.result['changed'] = True
+            else:
+                net = meraki.get_net(meraki.params['org_name'], meraki.params['net_name'], data=nets)
+                # meraki.fail_json(msg="HERE", net=net, payload=payload)
+                if meraki.is_update_required(net, payload):
+                    path = meraki.construct_path('update',
+                                                 net_id=meraki.get_net_id(net_name=meraki.params['net_name'], data=nets)
+                                                 )
+                    r = meraki.request(path,
+                                       method='PUT',
+                                       payload=json.dumps(payload))
+                    if meraki.status == 200:
+                        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:
             net_id = meraki.get_net_id(net_name=meraki.params['net_name'],

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

@@ -83,9 +83,11 @@
     meraki_network:
       auth_key: '{{ auth_key }}'
       state: present
-      org_name: '{{test_org_name}}'
+      org_name: '{{ test_org_name }}'
       net_name: IntTestNetworkCombined
-      type: combined
+      type:
+        - appliance
+        - switch
       timezone: America/Chicago
       disable_my_meraki: yes
     delegate_to: localhost
@@ -124,7 +126,9 @@
       net_name: IntTestNetworkTags
       type: switch
       timezone: America/Chicago
-      tags: first_tag, second_tag
+      tags: 
+        - first_tag
+        - second_tag
     delegate_to: localhost
     register: create_net_tags
 
@@ -139,7 +143,10 @@
       net_name: IntTestNetworkTags
       type: switch
       timezone: America/Chicago
-      tags: first_tag, second_tag, third_tag
+      tags: 
+        - first_tag
+        - second_tag
+        - third_tag
     delegate_to: localhost
     register: create_net_modified
 
@@ -151,7 +158,10 @@
       net_name: IntTestNetworkTags
       type: switch
       timezone: America/Chicago
-      tags: first_tag, second_tag, third_tag
+      tags: 
+        - first_tag
+        - second_tag
+        - third_tag
     delegate_to: localhost
     register: create_net_modified_idempotent
 
@@ -257,4 +267,4 @@
 
   - assert:
       that:
-        - 'delete_all_no_org.msg == "org_name or org_id parameters are required"'
+        - 'delete_all_no_org.msg == "org_name or org_id parameters are required"'