Refactored identity_role{,_info} modules

Change-Id: If8230eb8b41b5461e1eaa470569030e8a888015b

ci/roles/identity_role/defaults/main.yml

@@ -1,4 +1,3 @@
-role_name: ansible_keystone_role
 expected_fields:
   - description
   - domain_id

ci/roles/identity_role/tasks/main.yml

@@ -1,83 +1,88 @@
 ---
-- name: Cleanup before tests
+- name: Create identity role
-  block:
-    - openstack.cloud.identity_role:
-        cloud: "{{ cloud }}"
-        state: absent
-        name: "{{ role_name }}"
-
-- block:
-  - name: Delete unexistent role
-    openstack.cloud.identity_role:
-      cloud: "{{ cloud }}"
-      state: absent
-      name: "{{ role_name }}"
-    register: role
-  - name: Assert role didn't change
-    assert:
-      that: role is not changed
-
-- block:
-  - name: Create keystone role
   openstack.cloud.identity_role:
     cloud: "{{ cloud }}"
     state: present
-      name: "{{ role_name }}"
+    name: ansible_role
+    description: "ansible role"
   register: role
-  - name: Try to get role
+
+- name: Assert return values of identity_role module
+  assert:
+    that:
+      - role.role.name == 'ansible_role'
+      - role.role.description == "ansible role"
+      # allow new fields to be introduced but prevent fields from being removed
+      - expected_fields|difference(role.role.keys())|length == 0
+
+- name: Try to get role
   openstack.cloud.identity_role_info:
      cloud: "{{ cloud }}"
-       name: "{{ role_name }}"
+     name: ansible_role
   register: roles
-  - name: Assert role found
+
+- name: Assert role found
   assert:
     that:
       - roles.roles | length == 1
-  - name: Assert role changed
+      - roles.roles.0.name == 'ansible_role'
-    assert:
-      that: role is changed
-  - name: Assert return fields
-    assert:
-      that: item in role['role']
-    loop: "{{ expected_fields }}"
-  - name: Assert return value
-    assert:
-      that: role['role']['name'] == role_name
-  - name: Assert retrieved values
-    assert:
-      that: roles.roles[0].name == role_name
 
-- block:
+- name: Fetch all roles
-  - name: Create existing keystone role
+  openstack.cloud.identity_role_info:
+    cloud: "{{ cloud }}"
+  register: roles
+
+- name: Assert return values of identity_role_info module
+  assert:
+    that:
+      - roles.roles | length > 0
+      # allow new fields to be introduced but prevent fields from being removed
+      - expected_fields|difference(roles.roles.0.keys())|length == 0
+
+- name: Create identity role again
   openstack.cloud.identity_role:
     cloud: "{{ cloud }}"
     state: present
-      name: "{{ role_name }}"
+    name: ansible_role
+    description: "ansible role"
   register: role
-  - name: Assert role not changed
-    assert:
-      that: role is not changed
-  - name: Assert return fields
-    assert:
-      that: item in role['role']
-    loop: "{{ expected_fields }}"
 
-- block:
+- name: Assert role did not change
-  - name: Delete keystone role
+  assert:
+    that:
+      - role is not changed
+
+- name: Delete identity role
   openstack.cloud.identity_role:
     cloud: "{{ cloud }}"
     state: absent
-      name: "{{ role_name }}"
+    name: ansible_role
   register: role
-  - name: Assert role changed
+
+- name: Assert role changed
   assert:
-      that: role is changed
+    that:
-  - name: Try to get role
+      - role is changed
+
+- name: Try to get role
   openstack.cloud.identity_role_info:
      cloud: "{{ cloud }}"
-       name: "{{ role_name }}"
+     name: ansible_role
   register: roles
-  - name: Assert no role found
+
+- name: Assert no role found
   assert:
     that:
       - roles.roles | length == 0
+
+- name: Delete role again
+  openstack.cloud.identity_role:
+    cloud: "{{ cloud }}"
+    state: absent
+    name: ansible_role
+  register: role
+
+- name: Assert role did not change
+  assert:
+    that:
+      - role is not changed

ci/roles/identity_role_info/tasks/main.yml

@@ -1,64 +0,0 @@
-- name: Ensure role does not exist before tests
-  openstack.cloud.identity_role:
-    cloud: "{{ cloud }}"
-    state: absent
-    name: test_role
-
-- name: Get unexistent role
-  openstack.cloud.identity_role_info:
-    cloud: "{{ cloud }}"
-    name: test_role
-  register: roleinfo
-
-- debug:
-    var: roleinfo
-
-- name: Assert that no results were returned
-  assert:
-    that: not roleinfo.roles
-
-- name: Create keystone role
-  openstack.cloud.identity_role:
-    cloud: "{{ cloud }}"
-    state: present
-    name: test_role
-
-- name: Create second role
-  openstack.cloud.identity_role:
-    cloud: "{{ cloud }}"
-    state: present
-    name: test_role2
-
-- name: Get role by name
-  openstack.cloud.identity_role_info:
-    cloud: "{{ cloud }}"
-    name: test_role
-  register: roleinfo
-
-- debug:
-    var: roleinfo
-
-- name: Assert that only one result was returned
-  assert:
-    that: roleinfo.roles | length == 1
-
-- name: Assert that roleinfo has fields
-  assert:
-    that: item in roleinfo.roles[0]
-  loop:
-    - description
-    - domain_id
-    - id
-    - links
-    - name
-
-- name: Post-test cleanup
-  block:
-  - name: Clean up roles
-    openstack.cloud.identity_role:
-      cloud: "{{ cloud }}"
-      state: absent
-      name: "{{ item }}"
-    loop:
-      - test_role
-      - test_role2

ci/run-collection.yml

@@ -23,7 +23,6 @@
     - { role: identity_user, tags: identity_user }
     - { role: identity_user_info, tags: identity_user_info }
     - { role: identity_role, tags: identity_role }
-    - { role: identity_role_info, tags: identity_role_info }
     - { role: image, tags: image }
     - { role: keypair, tags: keypair }
     - role: keystone_idp

plugins/modules/identity_role.py

@@ -4,47 +4,55 @@
 # Copyright (c) 2016 IBM
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
-DOCUMENTATION = '''
+DOCUMENTATION = r'''
 ---
 module: identity_role
-short_description: Manage OpenStack Identity Roles
+short_description: Manage a OpenStack identity (Keystone) role
 author: OpenStack Ansible SIG
 description:
-    - Manage OpenStack Identity Roles.
+  - Create, update or delete a OpenStack identity (Keystone) role.
 options:
+  description:
+    description:
+      - Role description.
+    type: str
+  domain_id:
+    description:
+      - Domain id to create the role in.
+    type: str
   name:
     description:
-        - Role Name
+      - Role name.
     required: true
     type: str
   state:
     description:
       - Should the resource be present or absent.
-     choices: [present, absent]
+    choices: ['present', 'absent']
     default: present
     type: str
 extends_documentation_fragment:
-- openstack.cloud.openstack
+  - openstack.cloud.openstack
 '''
 
-EXAMPLES = '''
+EXAMPLES = r'''
-# Create a role named "demo"
+- name: Create a role named demo
-- openstack.cloud.identity_role:
+  openstack.cloud.identity_role:
     cloud: mycloud
     state: present
     name: demo
 
-# Delete the role named "demo"
+- name: Delete the role named demo
-- openstack.cloud.identity_role:
+  openstack.cloud.identity_role:
     cloud: mycloud
     state: absent
     name: demo
 '''
 
-RETURN = '''
+RETURN = r'''
 role:
-    description: Dictionary describing the role.
+  description: Dictionary describing the identity role.
-    returned: On success when I(state) is 'present'.
+  returned: On success when I(state) is C(present).
   type: dict
   contains:
     description:
@@ -69,10 +77,13 @@ role:
 '''
 
 from ansible_collections.openstack.cloud.plugins.module_utils.openstack import OpenStackModule
+from ansible_collections.openstack.cloud.plugins.module_utils.resource import StateMachine
 
 
 class IdentityRoleModule(OpenStackModule):
     argument_spec = dict(
+        description=dict(),
+        domain_id=dict(),
         name=dict(required=True),
         state=dict(default='present', choices=['absent', 'present']),
     )
@@ -81,33 +92,40 @@ class IdentityRoleModule(OpenStackModule):
         supports_check_mode=True
     )
 
-    def _system_state_change(self, state, role):
+    class _StateMachine(StateMachine):
-        if state == 'present' and not role:
+        def _find(self, attributes, **kwargs):
-            return True
+            kwargs = dict((k, attributes[k])
-        if state == 'absent' and role:
+                          for k in ['domain_id']
-            return True
+                          if k in attributes and attributes[k] is not None)
-        return False
+
+            return self.find_function(attributes['name'], **kwargs)
 
     def run(self):
-        name = self.params.get('name')
+        sm = self._StateMachine(connection=self.conn,
-        state = self.params.get('state')
+                                service_name='identity',
+                                type_name='role',
+                                sdk=self.sdk)
 
-        role = self.conn.identity.find_role(name)
+        kwargs = dict((k, self.params[k])
+                      for k in ['state', 'timeout']
+                      if self.params[k] is not None)
 
-        if self.ansible.check_mode:
+        kwargs['attributes'] = \
-            self.exit_json(changed=self._system_state_change(state, role))
+            dict((k, self.params[k])
+                 for k in ['description', 'domain_id', 'name']
+                 if self.params[k] is not None)
+
+        role, is_changed = sm(check_mode=self.ansible.check_mode,
+                              updateable_attributes=None,
+                              non_updateable_attributes=['domain_id'],
+                              wait=False,
+                              **kwargs)
 
-        changed = False
-        if state == 'present':
         if role is None:
-                role = self.conn.identity.create_role(name=name)
+            self.exit_json(changed=is_changed)
-                changed = True
+        else:
-            role = role.to_dict(computed=False)
+            self.exit_json(changed=is_changed,
-            self.exit_json(changed=changed, role=role)
+                           role=role.to_dict(computed=False))
-        elif state == 'absent' and role is not None:
-            self.conn.identity.delete_role(role['id'])
-            changed = True
-        self.exit_json(changed=changed)
 
 
 def main():

plugins/modules/identity_role_info.py

@@ -4,72 +4,66 @@
 # Copyright (c) 2020, Sagi Shnaidman <sshnaidm@redhat.com>
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
-DOCUMENTATION = '''
+DOCUMENTATION = r'''
 ---
 module: identity_role_info
-short_description: Retrieve information about roles
+short_description: Fetch OpenStack identity (Keystone) roles
 author: OpenStack Ansible SIG
 description:
-  - Get information about identity roles in Openstack
+  - Fetch OpenStack identity (Keystone) roles.
 options:
   domain_id:
     description:
-      - Domain ID which owns the role
+      - Domain ID which owns the role.
     type: str
     required: false
   name:
     description:
-      - Name or ID of the role
+      - Name or ID of the role.
     type: str
     required: false
 extends_documentation_fragment:
   - openstack.cloud.openstack
 '''
 
-RETURN = '''
+RETURN = r'''
 roles:
-  description: List of identity roles
+  description: List of dictionaries describing matching identity roles.
   returned: always
   type: list
   elements: dict
   contains:
-    id:
-      description: Unique ID for the role
-      returned: success
-      type: str
-    name:
-      description: Unique role name, within the owning domain.
-      returned: success
-      type: str
     description:
       description: User-facing description of the role.
-      returned: success
       type: str
     domain_id:
       description: References the domain ID which owns the role.
-      returned: success
+      type: str
+    id:
+      description: Unique ID for the role
       type: str
     links:
       description: The links for the service resources
-      returned: success
       type: dict
+    name:
+      description: Unique role name, within the owning domain.
+      type: str
 '''
 
-EXAMPLES = '''
+EXAMPLES = r'''
-# Retrieve info about all roles
+- name: Retrieve info about all roles
-- openstack.cloud.identity_role_info:
+  openstack.cloud.identity_role_info:
     cloud: mycloud
 
-# Retrieve info about all roles in specific domain
+- name: Retrieve info about all roles in specific domain
-- openstack.cloud.identity_role_info:
+  openstack.cloud.identity_role_info:
     cloud: mycloud
     domain_id: some_domain_id
 
-# Retrieve info about role 'admin'
+- name: Retrieve info about role 'admin'
-- openstack.cloud.identity_role_info:
+  openstack.cloud.identity_role_info:
     cloud: mycloud
     name: admin
-
 '''
 
 from ansible_collections.openstack.cloud.plugins.module_utils.openstack import OpenStackModule
@@ -86,14 +80,17 @@ class IdentityRoleInfoModule(OpenStackModule):
     )
 
     def run(self):
-        params = {
+        kwargs = dict((k, self.params[k])
-            'domain_id': self.params['domain_id'],
+                      for k in ['domain_id']
-            'name_or_id': self.params['name'],
+                      if self.params[k] is not None)
-        }
-        params = {k: v for k, v in params.items() if v is not None}
 
-        roles = [role.to_dict(computed=False) for role in self.conn.search_roles(**params)]
+        name_or_id = self.params['name']
-        self.exit_json(changed=False, roles=roles)
+        if name_or_id is not None:
+            kwargs['name_or_id'] = name_or_id
+
+        self.exit_json(changed=False,
+                       roles=[r.to_dict(computed=False)
+                              for r in self.conn.search_roles(**kwargs)])
 
 
 def main():