feat: add security_group to scaleway compute resource (#45699)

feat: add more tests

lib/ansible/modules/cloud/scaleway/scaleway_compute.py

@@ -127,6 +127,13 @@ options:
     - Time to wait before every attempt to check the state of the server
     required: false
     default: 3
+
+  security_group:
+    description:
+    - Security group unique identifier
+    - If no value provided, the default security group or current security group will be used
+    required: false
+    version_added: "2.8"
 '''
 
 EXAMPLES = '''
@@ -142,6 +149,19 @@ EXAMPLES = '''
       - test
       - www
 
+- name: Create a server attached to a security group
+  scaleway_compute:
+    name: foobar
+    state: present
+    image: 89ee4018-f8c3-4dc4-a6b5-bca14f985ebe
+    organization: 951df375-e094-4d26-97c1-ba548eeb9c42
+    region: ams1
+    commercial_type: VC1S
+    security_group: 4a31b633-118e-4900-bd52-facf1085fc8d
+    tags:
+      - test
+      - www
+
 - name: Destroy it right after
   scaleway_compute:
     name: foobar
@@ -272,16 +292,19 @@ def public_ip_payload(compute_api, public_ip):
 def create_server(compute_api, server):
     compute_api.module.debug("Starting a create_server")
     target_server = None
-    payload = {"enable_ipv6": server["enable_ipv6"],
+    data = {"enable_ipv6": server["enable_ipv6"],
             "tags": server["tags"],
             "commercial_type": server["commercial_type"],
             "image": server["image"],
             "dynamic_ip_required": server["dynamic_ip_required"],
             "name": server["name"],
-               "organization": server["organization"]}
+            "organization": server["organization"]
+            }
 
-    response = compute_api.post(path="servers",
-                                data=payload)
+    if server["security_group"]:
+        data["security_group"] = server["security_group"]
+
+    response = compute_api.post(path="servers", data=data)
 
     if not response.ok:
         msg = 'Error during server creation: (%s) %s' % (response.status_code, response.json)
@@ -354,7 +377,7 @@ def present_strategy(compute_api, wished_server):
         if compute_api.module.check_mode:
             return changed, {"status": "Server %s attributes would be changed." % target_server["id"]}
 
-        server_change_attributes(compute_api=compute_api, target_server=target_server, wished_server=wished_server)
+        target_server = server_change_attributes(compute_api=compute_api, target_server=target_server, wished_server=wished_server)
 
     return changed, target_server
 
@@ -417,7 +440,7 @@ def running_strategy(compute_api, wished_server):
         if compute_api.module.check_mode:
             return changed, {"status": "Server %s attributes would be changed before running it." % target_server["id"]}
 
-        server_change_attributes(compute_api=compute_api, target_server=target_server, wished_server=wished_server)
+        target_server = server_change_attributes(compute_api=compute_api, target_server=target_server, wished_server=wished_server)
 
     current_state = fetch_state(compute_api=compute_api, server=target_server)
     if current_state not in ("running", "starting"):
@@ -461,7 +484,7 @@ def stop_strategy(compute_api, wished_server):
             return changed, {
                 "status": "Server %s attributes would be changed before stopping it." % target_server["id"]}
 
-        server_change_attributes(compute_api=compute_api, target_server=target_server, wished_server=wished_server)
+        target_server = server_change_attributes(compute_api=compute_api, target_server=target_server, wished_server=wished_server)
 
     wait_to_complete_state_transition(compute_api=compute_api, server=target_server)
 
@@ -508,7 +531,7 @@ def restart_strategy(compute_api, wished_server):
             return changed, {
                 "status": "Server %s attributes would be changed before rebooting it." % target_server["id"]}
 
-        server_change_attributes(compute_api=compute_api, target_server=target_server, wished_server=wished_server)
+        target_server = server_change_attributes(compute_api=compute_api, target_server=target_server, wished_server=wished_server)
 
     changed = True
     if compute_api.module.check_mode:
@@ -564,6 +587,7 @@ PATCH_MUTABLE_SERVER_ATTRIBUTES = (
     "tags",
     "name",
     "dynamic_ip_required",
+    "security_group",
 )
 
 
@@ -575,29 +599,51 @@ def server_attributes_should_be_changed(compute_api, target_server, wished_serve
                       for x in PATCH_MUTABLE_SERVER_ATTRIBUTES
                       if x in target_server and x in wished_server)
     compute_api.module.debug("Debug dict %s" % debug_dict)
-
     try:
-        return any([target_server[x] != wished_server[x]
-                    for x in PATCH_MUTABLE_SERVER_ATTRIBUTES
-                    if x in target_server and x in wished_server])
+        for key in PATCH_MUTABLE_SERVER_ATTRIBUTES:
+            if key in target_server and key in wished_server:
+                # When you are working with dict, only ID matter as we ask user to put only the resource ID in the playbook
+                if isinstance(target_server[key], dict) and wished_server[key] and "id" in target_server[key].keys(
+                ) and target_server[key]["id"] != wished_server[key]:
+                    return True
+                # Handling other structure compare simply the two objects content
+                elif not isinstance(target_server[key], dict) and target_server[key] != wished_server[key]:
+                    return True
+        return False
     except AttributeError:
         compute_api.module.fail_json(msg="Error while checking if attributes should be changed")
 
 
 def server_change_attributes(compute_api, target_server, wished_server):
     compute_api.module.debug("Starting patching server attributes")
-    patch_payload = dict((x, wished_server[x])
-                         for x in PATCH_MUTABLE_SERVER_ATTRIBUTES
-                         if x in wished_server and x in target_server)
+    patch_payload = dict()
+
+    for key in PATCH_MUTABLE_SERVER_ATTRIBUTES:
+        if key in target_server and key in wished_server:
+            # When you are working with dict, only ID matter as we ask user to put only the resource ID in the playbook
+            if isinstance(target_server[key], dict) and "id" in target_server[key] and wished_server[key]:
+                # Setting all key to current value except ID
+                key_dict = dict((x, target_server[key][x]) for x in target_server[key].keys() if x != "id")
+                # Setting ID to the user specified ID
+                key_dict["id"] = wished_server[key]
+                patch_payload[key] = key_dict
+            elif not isinstance(target_server[key], dict):
+                patch_payload[key] = wished_server[key]
+
     response = compute_api.patch(path="servers/%s" % target_server["id"],
                                  data=patch_payload)
     if not response.ok:
         msg = 'Error during server attributes patching: (%s) %s' % (response.status_code, response.json)
         compute_api.module.fail_json(msg=msg)
 
+    try:
+        target_server = response.json["server"]
+    except KeyError:
+        compute_api.module.fail_json(msg="Error in getting the server information from: %s" % response.json)
+
     wait_to_complete_state_transition(compute_api=compute_api, server=target_server)
 
-    return response
+    return target_server
 
 
 def core(module):
@@ -609,7 +655,8 @@ def core(module):
         "commercial_type": module.params["commercial_type"],
         "enable_ipv6": module.params["enable_ipv6"],
         "tags": module.params["tags"],
-        "organization": module.params["organization"]
+        "organization": module.params["organization"],
+        "security_group": module.params["security_group"]
     }
     module.params['api_url'] = SCALEWAY_LOCATION[region]["api_endpoint"]
 
@@ -638,6 +685,7 @@ def main():
         wait=dict(type="bool", default=False),
         wait_timeout=dict(type="int", default=300),
         wait_sleep_time=dict(type="int", default=3),
+        security_group=dict(),
     ))
     module = AnsibleModule(
         argument_spec=argument_spec,

test/legacy/roles/scaleway_compute/tasks/main.yml

@@ -2,3 +2,4 @@
 
 - include_tasks: state.yml
 - include_tasks: ip.yml
+- include_tasks: security_group.yml

test/legacy/roles/scaleway_compute/tasks/security_group.yml

@@ -0,0 +1,147 @@
+- name: Create a scaleway security_group
+  scaleway_security_group:
+    state: present
+    region: '{{ scaleway_region }}'
+    name: test_compute
+    description: test_compute
+    organization: '{{ scaleway_organization }}'
+    stateful: true
+    inbound_default_policy: accept
+    outbound_default_policy: accept
+    organization_default: false
+  register: security_group
+
+- debug: var=security_group
+
+- block:
+  - name: Create a server with security_group (Check)
+    check_mode: yes
+    scaleway_compute:
+      name: '{{ scaleway_name }}'
+      state: present
+      image: '{{ scaleway_image_id }}'
+      organization: '{{ scaleway_organization }}'
+      region: '{{ scaleway_region }}'
+      commercial_type: '{{ scaleway_commerial_type }}'
+      security_group: '{{ security_group.scaleway_security_group.id }}'
+
+    register: server_creation_check_task
+
+  - debug: var=server_creation_check_task
+
+  - assert:
+      that:
+        - server_creation_check_task is success
+        - server_creation_check_task is changed
+
+  - name: Create a server
+    scaleway_compute:
+      name: '{{ scaleway_name }}'
+      state: present
+      image: '{{ scaleway_image_id }}'
+      organization: '{{ scaleway_organization }}'
+      region: '{{ scaleway_region }}'
+      commercial_type: '{{ scaleway_commerial_type }}'
+      security_group: '{{ security_group.scaleway_security_group.id }}'
+      wait: true
+
+    register: server_creation_task
+
+  - debug: var=server_creation_task
+
+  - assert:
+      that:
+        - server_creation_task is success
+        - server_creation_task is changed
+
+  - name: Create a server with security_group (Confirmation)
+    scaleway_compute:
+      name: '{{ scaleway_name }}'
+      state: present
+      image: '{{ scaleway_image_id }}'
+      organization: '{{ scaleway_organization }}'
+      region: '{{ scaleway_region }}'
+      commercial_type: '{{ scaleway_commerial_type }}'
+      security_group: '{{ security_group.scaleway_security_group.id }}'
+      wait: true
+
+    register: server_creation_confirmation_task
+
+  - debug: var=server_creation_confirmation_task
+
+  - assert:
+      that:
+        - server_creation_confirmation_task is success
+        - server_creation_confirmation_task is not changed
+
+  - name: Keep current security_group (Check)
+    check_mode: yes
+    scaleway_compute:
+      name: '{{ scaleway_name }}'
+      state: present
+      image: '{{ scaleway_image_id }}'
+      organization: '{{ scaleway_organization }}'
+      region: '{{ scaleway_region }}'
+      commercial_type: '{{ scaleway_commerial_type }}'
+      security_group: '{{ security_group.scaleway_security_group.id }}'
+      wait: true
+
+    register: server_creation_confirmation_task
+
+  - debug: var=server_creation_confirmation_task
+
+  - assert:
+      that:
+        - server_creation_confirmation_task is success
+        - server_creation_confirmation_task is not changed
+
+  - name: Keep current security_group
+    scaleway_compute:
+      name: '{{ scaleway_name }}'
+      state: present
+      image: '{{ scaleway_image_id }}'
+      organization: '{{ scaleway_organization }}'
+      region: '{{ scaleway_region }}'
+      commercial_type: '{{ scaleway_commerial_type }}'
+      wait: true
+
+    register: server_creation_confirmation_task
+
+  - debug: var=server_creation_confirmation_task
+
+  - assert:
+      that:
+        - server_creation_confirmation_task is success
+        - server_creation_confirmation_task is not changed
+
+  always:
+    - name: Destroy it
+      scaleway_compute:
+        name: '{{ scaleway_name }}'
+        state: absent
+        image: '{{ scaleway_image_id }}'
+        organization: '{{ scaleway_organization }}'
+        region: '{{ scaleway_region }}'
+        commercial_type: '{{ scaleway_commerial_type }}'
+        wait: true
+
+      register: server_destroy_task
+
+    - debug: var=server_destroy_task
+
+    - assert:
+        that:
+          - server_destroy_task is success
+          - server_destroy_task is changed
+
+    - name: Create a scaleway security_group
+      scaleway_security_group:
+        state: absent
+        region: '{{ scaleway_region }}'
+        name: test_compute
+        description: test_compute
+        organization: '{{ scaleway_organization }}'
+        stateful: true
+        inbound_default_policy: accept
+        outbound_default_policy: accept
+        organization_default: false