From 5e7c29d97e2f6a924841a8147d69cfde50055642 Mon Sep 17 00:00:00 2001
From: Rafael Castillo <rcastill@redhat.com>
Date: Fri, 5 Aug 2022 19:21:09 -0700
Subject: [PATCH] Update identity_group for 2.0.0

Replace calls to the sdk cloud layer with proxy layer calls where
appropriate.

Ensure module return values are converted into dict.

General refactoring to bring module more in line with collection
conventions.

Expand tests to assert idempotency and presence of return values.

Rename test role to identity_group to match module name.

Change-Id: I06fe28f77431bb151d85c8d9cd924a1634d85d98
---
 .zuul.yaml                                |   3 +-
 ci/roles/group/defaults/main.yml          |   1 -
 ci/roles/group/tasks/main.yml             |  19 ---
 ci/roles/identity_group/defaults/main.yml |   7 ++
 ci/roles/identity_group/tasks/main.yml    | 138 ++++++++++++++++++++++
 ci/run-collection.yml                     |   2 +-
 plugins/modules/identity_group.py         |  84 +++++++------
 7 files changed, 192 insertions(+), 62 deletions(-)
 delete mode 100644 ci/roles/group/defaults/main.yml
 delete mode 100644 ci/roles/group/tasks/main.yml
 create mode 100644 ci/roles/identity_group/defaults/main.yml
 create mode 100644 ci/roles/identity_group/tasks/main.yml

diff --git a/.zuul.yaml b/.zuul.yaml
index 8a52314..c29417b 100644
--- a/.zuul.yaml
+++ b/.zuul.yaml
@@ -71,11 +71,10 @@
         dns_zone_info
         endpoint
         floating_ip_info
-        group
         host_aggregate
         identity_domain_info
+        identity_group
         identity_group_info
-        identity_role
         identity_user
         identity_user_info
         identity_role
diff --git a/ci/roles/group/defaults/main.yml b/ci/roles/group/defaults/main.yml
deleted file mode 100644
index 361c011..0000000
--- a/ci/roles/group/defaults/main.yml
+++ /dev/null
@@ -1 +0,0 @@
-group_name: ansible_group
diff --git a/ci/roles/group/tasks/main.yml b/ci/roles/group/tasks/main.yml
deleted file mode 100644
index 7735573..0000000
--- a/ci/roles/group/tasks/main.yml
+++ /dev/null
@@ -1,19 +0,0 @@
----
-- name: Create group
-  openstack.cloud.identity_group:
-     cloud: "{{ cloud }}"
-     state: present
-     name: "{{ group_name }}"
-
-- name: Update group
-  openstack.cloud.identity_group:
-     cloud: "{{ cloud }}"
-     state: present
-     name: "{{ group_name }}"
-     description: "updated description"
-
-- name: Delete group
-  openstack.cloud.identity_group:
-     cloud: "{{ cloud }}"
-     state: absent
-     name: "{{ group_name }}"
diff --git a/ci/roles/identity_group/defaults/main.yml b/ci/roles/identity_group/defaults/main.yml
new file mode 100644
index 0000000..9b6ad24
--- /dev/null
+++ b/ci/roles/identity_group/defaults/main.yml
@@ -0,0 +1,7 @@
+group_name: ansible_group
+domain_name: ansible_domain
+expected_fields:
+  - description
+  - domain_id
+  - id
+  - name
diff --git a/ci/roles/identity_group/tasks/main.yml b/ci/roles/identity_group/tasks/main.yml
new file mode 100644
index 0000000..0c19f43
--- /dev/null
+++ b/ci/roles/identity_group/tasks/main.yml
@@ -0,0 +1,138 @@
+---
+- name: Create domain
+  openstack.cloud.identity_domain:
+     cloud: "{{ cloud }}"
+     state: present
+     name: "{{ domain_name }}"
+  register: domain
+
+- name: Create group
+  openstack.cloud.identity_group:
+     cloud: "{{ cloud }}"
+     state: present
+     name: "{{ group_name }}"
+  register: group_result
+
+- name: Assert changed
+  assert:
+    that: group_result is changed
+
+- name: Assert returned fields
+  assert:
+    that: item in group_result.group
+  loop: "{{ expected_fields }}"
+
+- name: Create group again
+  openstack.cloud.identity_group:
+     cloud: "{{ cloud }}"
+     state: present
+     name: "{{ group_name }}"
+  register: group_result
+
+- name: Assert not changed
+  assert:
+    that: group_result is not changed
+
+- name: Update group
+  openstack.cloud.identity_group:
+     cloud: "{{ cloud }}"
+     state: present
+     name: "{{ group_name }}"
+     description: "updated description"
+  register: group_result
+
+- name: Assert changed
+  assert:
+    that:
+      - group_result is changed
+      - group_result.group.description == "updated description"
+
+- name: Create group in specific domain
+  openstack.cloud.identity_group:
+     cloud: "{{ cloud }}"
+     state: present
+     name: "{{ group_name }}"
+     domain_id: "{{ domain.id }}"
+  register: group_result
+
+- name: Assert results
+  assert:
+    that:
+      - group_result is changed
+      - group_result.group.domain_id == domain.id
+
+- name: Create group in specific domain again
+  openstack.cloud.identity_group:
+     cloud: "{{ cloud }}"
+     state: present
+     name: "{{ group_name }}"
+     domain_id: "{{ domain.id }}"
+  register: group_result
+
+- name: Assert not changed
+  assert:
+    that: group_result is not changed
+
+- name: Delete ambiguous domain
+  openstack.cloud.identity_group:
+     cloud: "{{ cloud }}"
+     state: absent
+     name: "{{ group_name }}"
+  ignore_errors: true
+  register: group_result
+
+- name: Assert failed
+  assert:
+    that: group_result is failed
+
+- name: Delete group in specific domain
+  openstack.cloud.identity_group:
+     cloud: "{{ cloud }}"
+     state: absent
+     name: "{{ group_name }}"
+     domain_id: "{{ domain.id }}"
+  register: group_result
+
+- name: Assert changed
+  assert:
+    that: group_result is changed
+
+- name: Delete group in specific domain again
+  openstack.cloud.identity_group:
+     cloud: "{{ cloud }}"
+     state: absent
+     name: "{{ group_name }}"
+     domain_id: "{{ domain.id }}"
+  register: group_result
+
+- name: Assert not changed
+  assert:
+    that: group_result is not changed
+
+- name: Delete group
+  openstack.cloud.identity_group:
+     cloud: "{{ cloud }}"
+     state: absent
+     name: "{{ group_name }}"
+  register: group_result
+
+- name: Assert changed
+  assert:
+    that: group_result is changed
+
+- name: Delete group again
+  openstack.cloud.identity_group:
+     cloud: "{{ cloud }}"
+     state: absent
+     name: "{{ group_name }}"
+  register: group_result
+
+- name: Assert not changed
+  assert:
+    that: group_result is not changed
+
+- name: Delete domain
+  openstack.cloud.identity_domain:
+    cloud: "{{ cloud }}"
+    state: absent
+    name: "{{ domain_name }}"
diff --git a/ci/run-collection.yml b/ci/run-collection.yml
index 3d5246f..48edfbe 100644
--- a/ci/run-collection.yml
+++ b/ci/run-collection.yml
@@ -12,7 +12,6 @@
     - role: object_container
       tags: object_container
       when: sdk_version is version(0.18, '>=')
-    - { role: group, tags: group }
     - role: dns
       tags: dns
       when: sdk_version is version(0.28, '>=')
@@ -20,6 +19,7 @@
     - { role: floating_ip_info, tags: floating_ip_info }
     - { role: host_aggregate, tags: host_aggregate }
     - { role: identity_domain_info, tags: identity_domain_info }
+    - { role: identity_group, tags: identity_group }
     - { role: identity_group_info, tags: identity_group_info }
     - { role: identity_user, tags: identity_user }
     - { role: identity_user_info, tags: identity_user_info }
diff --git a/plugins/modules/identity_group.py b/plugins/modules/identity_group.py
index 1242b8c..0e4cc6f 100644
--- a/plugins/modules/identity_group.py
+++ b/plugins/modules/identity_group.py
@@ -13,11 +13,6 @@ description:
     - Manage OpenStack Identity Groups. Groups can be created, deleted or
       updated. Only the I(description) value can be updated.
 options:
-   name:
-     description:
-        - Group name
-     required: true
-     type: str
    description:
      description:
         - Group description
@@ -26,6 +21,11 @@ options:
      description:
         - Domain id to create the group in if the cloud supports domains.
      type: str
+   name:
+     description:
+        - Group name
+     required: true
+     type: str
    state:
      description:
        - Should the resource be present or absent.
@@ -68,16 +68,8 @@ RETURN = '''
 group:
     description: Dictionary describing the group.
     returned: On success when I(state) is 'present'.
-    type: complex
+    type: dict
     contains:
-        id:
-            description: Unique group ID
-            type: str
-            sample: "ee6156ff04c645f481a6738311aea0b0"
-        name:
-            description: Group name
-            type: str
-            sample: "demo"
         description:
             description: Group description
             type: str
@@ -86,6 +78,14 @@ group:
             description: Domain for the group
             type: str
             sample: "default"
+        id:
+            description: Unique group ID
+            type: str
+            sample: "ee6156ff04c645f481a6738311aea0b0"
+        name:
+            description: Group name
+            type: str
+            sample: "demo"
 '''
 
 from ansible_collections.openstack.cloud.plugins.module_utils.openstack import OpenStackModule
@@ -103,51 +103,57 @@ class IdentityGroupModule(OpenStackModule):
         supports_check_mode=True
     )
 
-    def _system_state_change(self, state, description, group):
+    def _system_state_change(self, state, group):
         if state == 'present' and not group:
             return True
-        if state == 'present' and description is not None and group.description != description:
+        if state == 'present' and self._build_update(group):
             return True
         if state == 'absent' and group:
             return True
         return False
 
+    def _build_update(self, group):
+        update = {}
+        desc = self.params['description']
+        if desc is not None and desc != group.description:
+            update['description'] = desc
+        return update
+
     def run(self):
-        name = self.params.get('name')
-        description = self.params.get('description')
-        state = self.params.get('state')
+        name = self.params['name']
+        description = self.params['description']
+        state = self.params['state']
+        domain_id = self.params['domain_id']
 
-        domain_id = self.params.pop('domain_id')
+        group_filters = {}
+        if domain_id is not None:
+            group_filters['domain_id'] = domain_id
 
-        if domain_id:
-            group = self.conn.get_group(name, filters={'domain_id': domain_id})
-        else:
-            group = self.conn.get_group(name)
+        group = self.conn.identity.find_group(name, **group_filters)
 
         if self.ansible.check_mode:
-            self.exit_json(changed=self._system_state_change(state, description, group))
+            self.exit_json(changed=self._system_state_change(state, group))
 
+        changed = False
         if state == 'present':
             if group is None:
-                group = self.conn.create_group(
-                    name=name, description=description, domain=domain_id)
+                kwargs = dict(description=description, domain_id=domain_id)
+                kwargs = {k: v for k, v in kwargs.items() if v is not None}
+                group = self.conn.identity.create_group(
+                    name=name, **kwargs)
                 changed = True
             else:
-                if description is not None and group.description != description:
-                    group = self.conn.update_group(
-                        group.id, description=description)
+                update = self._build_update(group)
+                if update:
+                    group = self.conn.identity.update_group(group, **update)
                     changed = True
-                else:
-                    changed = False
+            group = group.to_dict(computed=False)
             self.exit_json(changed=changed, group=group)
 
-        elif state == 'absent':
-            if group is None:
-                changed = False
-            else:
-                self.conn.delete_group(group.id)
-                changed = True
-            self.exit_json(changed=changed)
+        elif state == 'absent' and group is not None:
+            self.conn.identity.delete_group(group)
+            changed = True
+        self.exit_json(changed=changed)
 
 
 def main():
-- 
GitLab