Refactored {group,role}_assignment modules

Change-Id: I6ec79eb203d0f68661b54bc89a194c366b3574c6

ci/roles/group_assignment/tasks/main.yml

@@ -0,0 +1,68 @@
+---
+- name: Create user
+  openstack.cloud.identity_user:
+    cloud: "{{ cloud }}"
+    state: present
+    name: ansible_user
+    password: secret
+    email: ansible.user@nowhere.net
+    domain: default
+    default_project: demo
+
+- name: Assign user to nonadmins group
+  openstack.cloud.group_assignment:
+    cloud: "{{ cloud }}"
+    state: present
+    user: ansible_user
+    group: nonadmins
+  register: group_assignment
+
+- name: Assert group assignment
+  assert:
+    that:
+      - group_assignment is changed
+
+- name: Assign user to nonadmins group again
+  openstack.cloud.group_assignment:
+    cloud: "{{ cloud }}"
+    state: present
+    user: ansible_user
+    group: nonadmins
+  register: group_assignment
+
+- name: Assert group assignment
+  assert:
+    that:
+      - group_assignment is not changed
+
+- name: Remove user from nonadmins group
+  openstack.cloud.group_assignment:
+    cloud: "{{ cloud }}"
+    state: absent
+    user: ansible_user
+    group: nonadmins
+  register: group_assignment
+
+- name: Assert group assignment
+  assert:
+    that:
+      - group_assignment is changed
+
+- name: Remove user from nonadmins group again
+  openstack.cloud.group_assignment:
+    cloud: "{{ cloud }}"
+    state: absent
+    user: ansible_user
+    group: nonadmins
+  register: group_assignment
+
+- name: Assert group assignment
+  assert:
+    that:
+      - group_assignment is not changed
+
+- name: Delete user
+  openstack.cloud.identity_user:
+    cloud: "{{ cloud }}"
+    state: absent
+    name: ansible_user

ci/roles/role_assignment/tasks/main.yml

@@ -15,6 +15,12 @@
     project: ansible_project
     role: admin
     user: admin
+  register: role_assignment
+
+- name: Assert role assignment
+  assert:
+    that:
+      - role_assignment is changed
 
 - name: Grant an admin role on the user admin in the project ansible_project again
   openstack.cloud.role_assignment:
@@ -23,12 +29,12 @@
     project: ansible_project
     role: admin
     user: admin
-  register: grant_again
+  register: role_assignment
 
-- name: Ensure grant again doesn't change anything
+- name: Ensure grant again did not change anything
   assert:
     that:
-      - not grant_again.changed
+      - role_assignment is not changed
 
 - name: Revoke the admin role on the user admin in the project ansible_project
   openstack.cloud.role_assignment:
@@ -44,3 +50,124 @@
      cloud: "{{ cloud }}"
      state: absent
      name: ansible_project
+
+- name: Create domain
+  openstack.cloud.identity_domain:
+     cloud: "{{ cloud }}"
+     state: present
+     name: ansible_domain
+  register: domain
+
+- name: Create group in default domain
+  openstack.cloud.identity_group:
+     cloud: "{{ cloud }}"
+     state: present
+     name: ansible_group
+     domain_id: default
+
+- name: Create group in specific domain
+  openstack.cloud.identity_group:
+     cloud: "{{ cloud }}"
+     state: present
+     name: ansible_group
+     domain_id: "{{ domain.domain.id }}"
+
+- name: Create user in default domain
+  openstack.cloud.identity_user:
+     cloud: "{{ cloud }}"
+     state: present
+     name: ansible_user
+     domain: default
+
+- name: Create user in specific domain
+  openstack.cloud.identity_user:
+     cloud: "{{ cloud }}"
+     state: present
+     name: ansible_user
+     domain: "{{ domain.domain.id }}"
+
+- name: Assign role to group in default domain
+  openstack.cloud.role_assignment:
+    cloud: "{{ cloud }}"
+    role: anotherrole
+    group: ansible_group
+    domain: default
+  register: role_assignment
+
+- name: Assert role assignment
+  assert:
+    that:
+      - role_assignment is changed
+
+- name: Assign role to group in specific domain
+  openstack.cloud.role_assignment:
+    cloud: "{{ cloud }}"
+    role: anotherrole
+    group: ansible_group
+    domain: "{{ domain.domain.id }}"
+  register: role_assignment
+
+- name: Assert role assignment
+  assert:
+    that:
+      - role_assignment is changed
+
+- name: Assign role to user in default domain
+  openstack.cloud.role_assignment:
+    cloud: "{{ cloud }}"
+    role: anotherrole
+    user: ansible_user
+    domain: default
+  register: role_assignment
+
+- name: Assert role assignment
+  assert:
+    that:
+      - role_assignment is changed
+
+- name: Assign role to user in specific domain
+  openstack.cloud.role_assignment:
+    cloud: "{{ cloud }}"
+    role: anotherrole
+    user: ansible_user
+    domain: "{{ domain.domain.id }}"
+  register: role_assignment
+
+- name: Assert role assignment
+  assert:
+    that:
+      - role_assignment is changed
+
+- name: Delete group in default domain
+  openstack.cloud.identity_group:
+     cloud: "{{ cloud }}"
+     state: absent
+     name: ansible_group
+     domain_id: default
+
+- name: Delete group in specific domain
+  openstack.cloud.identity_group:
+     cloud: "{{ cloud }}"
+     state: absent
+     name: ansible_group
+     domain_id: "{{ domain.domain.id }}"
+
+- name: Delete user in default domain
+  openstack.cloud.identity_user:
+     cloud: "{{ cloud }}"
+     state: absent
+     name: ansible_user
+     domain: default
+
+- name: Delete user in specific domain
+  openstack.cloud.identity_user:
+     cloud: "{{ cloud }}"
+     state: absent
+     name: ansible_user
+     domain: "{{ domain.domain.id }}"
+
+- name: Delete domain
+  openstack.cloud.identity_domain:
+     cloud: "{{ cloud }}"
+     state: absent
+     name: ansible_domain

ci/roles/user_group/tasks/main.yml

@@ -1,31 +0,0 @@
----
-- name: Create user
-  openstack.cloud.identity_user:
-     cloud: "{{ cloud }}"
-     state: present
-     name: ansible_user
-     password: secret
-     email: ansible.user@nowhere.net
-     domain: default
-     default_project: demo
-  register: user
-
-- name: Assign user to nonadmins group
-  openstack.cloud.group_assignment:
-     cloud: "{{ cloud }}"
-     state: present
-     user: ansible_user
-     group: nonadmins
-
-- name: Remove user from nonadmins group
-  openstack.cloud.group_assignment:
-     cloud: "{{ cloud }}"
-     state: absent
-     user: ansible_user
-     group: nonadmins
-
-- name: Delete user
-  openstack.cloud.identity_user:
-     cloud: "{{ cloud }}"
-     state: absent
-     name: ansible_user

ci/roles/user_role/defaults/main.yaml

@@ -1,4 +0,0 @@
-domain_name: ansible_domain
-user_name: ansible_user
-group_name: ansible_group
-keystone_role_name: anotherrole

ci/roles/user_role/tasks/main.yaml

@@ -1,96 +0,0 @@
-- name: Create domain
-  openstack.cloud.identity_domain:
-     cloud: "{{ cloud }}"
-     state: present
-     name: "{{ domain_name }}"
-  register: domain
-
-- name: Create group in default domain
-  openstack.cloud.identity_group:
-     cloud: "{{ cloud }}"
-     state: present
-     name: "{{ group_name }}"
-     domain_id: default
-
-- name: Create group in specific domain
-  openstack.cloud.identity_group:
-     cloud: "{{ cloud }}"
-     state: present
-     name: "{{ group_name }}"
-     domain_id: "{{ domain.domain.id }}"
-
-- name: Create user in default domain
-  openstack.cloud.identity_user:
-     cloud: "{{ cloud }}"
-     state: present
-     name: "{{ user_name }}"
-     domain: default
-
-- name: Create user in specific domain
-  openstack.cloud.identity_user:
-     cloud: "{{ cloud }}"
-     state: present
-     name: "{{ user_name }}"
-     domain: "{{ domain.domain.id }}"
-
-- name: Assign role to group in default domain
-  openstack.cloud.role_assignment:
-    cloud: "{{ cloud }}"
-    role: "{{ keystone_role_name }}"
-    group: "{{ group_name }}"
-    domain: default
-
-- name: Assign role to group in specific domain
-  openstack.cloud.role_assignment:
-    cloud: "{{ cloud }}"
-    role: "{{ keystone_role_name }}"
-    group: "{{ group_name }}"
-    domain: "{{ domain.domain.id }}"
-
-- name: Assign role to user in default domain
-  openstack.cloud.role_assignment:
-    cloud: "{{ cloud }}"
-    role: "{{ keystone_role_name }}"
-    user: "{{ user_name }}"
-    domain: default
-
-- name: Assign role to user in specific domain
-  openstack.cloud.role_assignment:
-    cloud: "{{ cloud }}"
-    role: "{{ keystone_role_name }}"
-    user: "{{ user_name }}"
-    domain: "{{ domain.domain.id }}"
-
-- name: Delete group in default domain
-  openstack.cloud.identity_group:
-     cloud: "{{ cloud }}"
-     state: absent
-     name: "{{ group_name }}"
-     domain_id: default
-
-- name: Delete group in specific domain
-  openstack.cloud.identity_group:
-     cloud: "{{ cloud }}"
-     state: absent
-     name: "{{ group_name }}"
-     domain_id: "{{ domain.domain.id }}"
-
-- name: Delete user in default domain
-  openstack.cloud.identity_user:
-     cloud: "{{ cloud }}"
-     state: absent
-     name: "{{ user_name }}"
-     domain: default
-
-- name: Delete user in specific domain
-  openstack.cloud.identity_user:
-     cloud: "{{ cloud }}"
-     state: absent
-     name: "{{ user_name }}"
-     domain: "{{ domain.domain.id }}"
-
-- name: Delete domain
-  openstack.cloud.identity_domain:
-     cloud: "{{ cloud }}"
-     state: absent
-     name: "{{ domain_name }}"

ci/run-collection.yml

@@ -17,6 +17,7 @@
     - { role: endpoint, tags: endpoint }
     - { role: federation_mapping, tags: federation_mapping }
     - { role: floating_ip, tags: floating_ip }
+    - { role: group_assignment, tags: group_assignment }
     - { role: host_aggregate, tags: host_aggregate }
     - { role: identity_domain, tags: identity_domain }
     - { role: identity_group, tags: identity_group }
@@ -50,8 +51,6 @@
     - { role: stack, tags: stack }
     - { role: subnet, tags: subnet }
     - { role: subnet_pool, tags: subnet_pool }
-    - { role: user_group, tags: user_group }
-    - { role: user_role, tags: user_role }
     - { role: volume, tags: volume }
     - { role: volume_backup, tags: volume_backup }
     - { role: volume_snapshot, tags: volume_snapshot }

plugins/modules/group_assignment.py

@@ -4,40 +4,40 @@
 # Copyright (c) 2015 Hewlett-Packard Development Company, L.P.
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
-DOCUMENTATION = '''
+DOCUMENTATION = r'''
 ---
 module: group_assignment
-short_description: Associate OpenStack Identity users and groups
+short_description: Assign OpenStack identity users to groups
 author: OpenStack Ansible SIG
 description:
-    - Add and remove users from groups
+  - Add and remove OpenStack identity (Keystone) users to/from groups.
 options:
-   user:
-     description:
-        - Name or id for the user
-     required: true
-     type: str
   group:
     description:
-        - Name or id for the group.
+      - Name or ID for the group.
     required: true
     type: str
   state:
     description:
-       - Should the user be present or absent in the group
+      - Should the user be present or absent in the group.
     choices: [present, absent]
     default: present
     type: str
+  user:
+    description:
+      - Name or ID for the user.
+    required: true
+    type: str
 extends_documentation_fragment:
-- openstack.cloud.openstack
+  - openstack.cloud.openstack
 '''
 
-EXAMPLES = '''
+EXAMPLES = r'''
-# Add the demo user to the demo group
+- name: Add demo_user user to demo_group group
-- openstack.cloud.group_assignment:
+  openstack.cloud.group_assignment:
     cloud: mycloud
-  user: demo
+    user: demo_user
-  group: demo
+    group: demo_group
 '''
 
 from ansible_collections.openstack.cloud.plugins.module_utils.openstack import OpenStackModule
@@ -45,44 +45,42 @@ from ansible_collections.openstack.cloud.plugins.module_utils.openstack import O
 
 class IdentityGroupAssignment(OpenStackModule):
     argument_spec = dict(
-        user=dict(required=True),
         group=dict(required=True),
         state=dict(default='present', choices=['absent', 'present']),
+        user=dict(required=True),
     )
 
     module_kwargs = dict(
         supports_check_mode=True
     )
 
-    def _system_state_change(self, state, in_group):
-        if state == 'present' and not in_group:
-            return True
-        if state == 'absent' and in_group:
-            return True
-        return False
-
     def run(self):
-        user = self.params['user']
+        user_name_or_id = self.params['user']
-        group = self.params['group']
+        user = self.conn.identity.find_user(user_name_or_id,
+                                            ignore_missing=False)
+
+        group_name_or_id = self.params['group']
+        group = self.conn.identity.find_group(group_name_or_id,
+                                              ignore_missing=False)
+
+        is_user_in_group = \
+            self.conn.identity.check_user_in_group(user, group)
+
         state = self.params['state']
-
-        in_group = self.conn.is_user_in_group(user, group)
-
         if self.ansible.check_mode:
-            self.exit_json(changed=self._system_state_change(state, in_group))
+            self.exit_json(
+                changed=(
+                    (state == 'present' and not is_user_in_group)
+                    or (state == 'absent' and is_user_in_group)))
 
-        changed = False
+        if state == 'present' and not is_user_in_group:
-        if state == 'present':
+            self.conn.identity.add_user_to_group(user, group)
-            if not in_group:
+            self.exit_json(changed=True)
-                self.conn.add_user_to_group(user, group)
+        elif state == 'absent' and is_user_in_group:
-                changed = True
+            self.conn.identity.remove_user_from_group(user, group)
-
+            self.exit_json(changed=True)
-        elif state == 'absent':
+        else:
-            if in_group:
+            self.exit_json(changed=False)
-                self.conn.remove_user_from_group(user, group)
-                changed = True
-
-        self.exit_json(changed=changed)
 
 
 def main():

plugins/modules/role_assignment.py

@@ -4,46 +4,51 @@
 # 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: role_assignment
-short_description: Associate OpenStack Identity users and roles
+short_description: Assign OpenStack identity groups and users to roles
 author: OpenStack Ansible SIG
 description:
   - Grant and revoke roles in either project or domain context for
-      OpenStack Identity Users.
+    OpenStack identity (Keystone) users and groups.
 options:
-   role:
+  domain:
     description:
-        - Name or ID for the role.
+      - Name or ID of the domain to scope the role association to.
-     required: true
+      - Valid only with keystone version 3.
-     type: str
+      - Required if I(project) is not specified.
-   user:
+      - When I(project) is specified, then I(domain) will not be used for
-     description:
+        scoping the role association, only for finding resources.
-        - Name or ID for the user. If I(user) is not specified, then
+      - "When scoping the role association, I(project) has precedence over
-          I(group) is required. Both may not be specified.
+         I(domain) and I(domain) has precedence over I(system): When I(project)
+         is specified, then I(domain) and I(system) are not used for role
+         association. When I(domain) is specified, then I(system) will not be
+         used for role association."
     type: str
   group:
     description:
-        - Name or ID for the group. Valid only with keystone version 3.
+      - Name or ID for the group.
-          If I(group) is not specified, then I(user) is required. Both
+      - Valid only with keystone version 3.
-          may not be specified.
+      - If I(group) is not specified, then I(user) is required. Both may not be
+        specified at the same time.
     type: str
   project:
     description:
       - Name or ID of the project to scope the role association to.
-          If you are using keystone version 2, then this value is required.
+      - If you are using keystone version 2, then this value is required.
+      - When I(project) is specified, then I(domain) will not be used for
+        scoping the role association, only for finding resources.
+      - "When scoping the role association, I(project) has precedence over
+         I(domain) and I(domain) has precedence over I(system): When I(project)
+         is specified, then I(domain) and I(system) are not used for role
+         association. When I(domain) is specified, then I(system) will not be
+         used for role association."
     type: str
-   domain:
+  role:
     description:
-        - Name or ID of the domain to scope the role association to. Valid only
+      - Name or ID for the role.
-          with keystone version 3, and required if I(project) is not specified.
+    required: true
-     type: str
-   system:
-     description:
-        - Name of system to scope the role association to. Valid only with
-          keystone version 3, and required if I(project) and I(domain)
-          are not specified.
     type: str
   state:
     description:
@@ -51,20 +56,37 @@ options:
     choices: [present, absent]
     default: present
     type: str
+  system:
+    description:
+      - Name of system to scope the role association to.
+      - Valid only with keystone version 3.
+      - Required if I(project) and I(domain) are not specified.
+      - "When scoping the role association, I(project) has precedence over
+         I(domain) and I(domain) has precedence over I(system): When I(project)
+         is specified, then I(domain) and I(system) are not used for role
+         association. When I(domain) is specified, then I(system) will not be
+         used for role association."
+    type: str
+  user:
+    description:
+      - Name or ID for the user.
+      - If I(user) is not specified, then I(group) is required. Both may not be
+        specified at the same time.
+    type: str
 extends_documentation_fragment:
-- openstack.cloud.openstack
+  - openstack.cloud.openstack
 '''
 
-EXAMPLES = '''
+EXAMPLES = r'''
-# Grant an admin role on the user admin in the project project1
+- name: Grant an admin role on the user admin in the project project1
-- openstack.cloud.role_assignment:
+  openstack.cloud.role_assignment:
     cloud: mycloud
     user: admin
     role: admin
     project: project1
 
-# Revoke the admin role from the user barney in the newyork domain
+- name: Revoke the admin role from the user barney in the newyork domain
-- openstack.cloud.role_assignment:
+  openstack.cloud.role_assignment:
     cloud: mycloud
     state: absent
     user: barney
@@ -72,123 +94,95 @@ EXAMPLES = '''
     domain: newyork
 '''
 
-RETURN = '''
-#
-'''
-
 from ansible_collections.openstack.cloud.plugins.module_utils.openstack import OpenStackModule
 
 
 class IdentityRoleAssignmentModule(OpenStackModule):
     argument_spec = dict(
-        role=dict(required=True),
+        domain=dict(),
-        user=dict(),
         group=dict(),
         project=dict(),
-        domain=dict(),
+        role=dict(required=True),
-        system=dict(),
         state=dict(default='present', choices=['absent', 'present']),
+        system=dict(),
+        user=dict(),
     )
 
     module_kwargs = dict(
         required_one_of=[
-            ['user', 'group']
+            ('user', 'group'),
+            ('domain', 'project', 'system'),
         ],
         supports_check_mode=True
     )
 
-    def _system_state_change(self, state, assignment):
-        if state == 'present' and not assignment:
-            return True
-        elif state == 'absent' and assignment:
-            return True
-        return False
-
-    def _build_kwargs(self, user, group, project, domain, system):
-        kwargs = {}
-        if user:
-            kwargs['user'] = user
-        if group:
-            kwargs['group'] = group
-        if project:
-            kwargs['project'] = project
-        if domain:
-            kwargs['domain'] = domain
-        if system:
-            kwargs['system'] = system
-        return kwargs
-
     def run(self):
-        role = self.params.get('role')
-        user = self.params.get('user')
-        group = self.params.get('group')
-        project = self.params.get('project')
-        domain = self.params.get('domain')
-        system = self.params.get('system')
-        state = self.params.get('state')
-
         filters = {}
         find_filters = {}
-        domain_id = None
+        kwargs = {}
 
-        r = self.conn.identity.find_role(role)
+        role_name_or_id = self.params['role']
-        if r is None:
+        role = self.conn.identity.find_role(role_name_or_id,
-            self.fail_json(msg="Role %s is not valid" % role)
+                                            ignore_missing=False)
-        filters['role'] = r['id']
+        filters['role_id'] = role['id']
 
-        if domain:
+        domain_name_or_id = self.params['domain']
-            d = self.conn.identity.find_domain(domain)
+        if domain_name_or_id is not None:
-            if d is None:
+            domain = self.conn.identity.find_domain(
-                self.fail_json(msg="Domain %s is not valid" % domain)
+                domain_name_or_id, ignore_missing=False)
-            domain_id = d['id']
+            filters['scope_domain_id'] = domain['id']
-            find_filters['domain_id'] = domain_id
+            find_filters['domain_id'] = domain['id']
-        if user:
+            kwargs['domain'] = domain['id']
-            u = self.conn.identity.find_user(user, **find_filters)
-            if u is None:
-                self.fail_json(msg="User %s is not valid" % user)
-            filters['user'] = u['id']
 
-        if group:
+        user_name_or_id = self.params['user']
-            g = self.conn.identity.find_group(group, **find_filters)
+        if user_name_or_id is not None:
-            if g is None:
+            user = self.conn.identity.find_user(
-                self.fail_json(msg="Group %s is not valid" % group)
+                user_name_or_id, ignore_missing=False, **find_filters)
-            filters['group'] = g['id']
+            filters['user_id'] = user['id']
-        if project:
+            kwargs['user'] = user['id']
-            p = self.conn.identity.find_project(project, **find_filters)
-            if p is None:
-                self.fail_json(msg="Project %s is not valid" % project)
-            filters['project'] = p['id']
-        if system:
-            # the system role name is the argument. list_role_assignments will
-            # fail if the system role name is not valid
-            filters['system'] = system
 
-        # Keeping the self.conn.list_role_assignments because it calls directly
+        group_name_or_id = self.params['group']
-        # the identity.role_assignments and there are some logics for the
+        if group_name_or_id is not None:
-        # filters that won't worth rewrite here.
+            group = self.conn.identity.find_group(
-        assignment = self.conn.list_role_assignments(filters=filters)
+                group_name_or_id, ignore_missing=False, **find_filters)
+            filters['group_id'] = group['id']
+            kwargs['group'] = group['id']
 
+        system_name = self.params['system']
+        if system_name is not None:
+            # domain has precedence over system
+            if 'scope_domain_id' not in filters:
+                filters['scope.system'] = system_name
+
+            kwargs['system'] = system_name
+
+        project_name_or_id = self.params['project']
+        if project_name_or_id is not None:
+            project = self.conn.identity.find_project(
+                project_name_or_id, ignore_missing=False, **find_filters)
+            filters['scope_project_id'] = project['id']
+            kwargs['project'] = project['id']
+
+            # project has precedence over domain and system
+            filters.pop('scope_domain_id', None)
+            filters.pop('scope.system', None)
+
+        role_assignments = list(self.conn.identity.role_assignments(**filters))
+
+        state = self.params['state']
         if self.ansible.check_mode:
-            self.exit_json(changed=self._system_state_change(state, assignment))
+            self.exit_json(
+                changed=((state == 'present' and not role_assignments)
+                         or (state == 'absent' and role_assignments)))
 
-        changed = False
+        if state == 'present' and not role_assignments:
-
+            self.conn.grant_role(role['id'], **kwargs)
-        # Both grant_role and revoke_role calls directly the proxy layer, and
+            self.exit_json(changed=True)
-        # has some logic that won't worth to rewrite here so keeping it is a
+        elif state == 'absent' and role_assignments:
-        # good idea
+            self.conn.revoke_role(role['id'], **kwargs)
-        if state == 'present':
+            self.exit_json(changed=True)
-            if not assignment:
+        else:
-                kwargs = self._build_kwargs(user, group, project, domain_id, system)
+            self.exit_json(changed=False)
-                self.conn.grant_role(role, **kwargs)
-                changed = True
-
-        elif state == 'absent':
-            if assignment:
-                kwargs = self._build_kwargs(user, group, project, domain_id, system)
-                self.conn.revoke_role(role, **kwargs)
-                changed = True
-
-        self.exit_json(changed=changed)
 
 
 def main():