Merge pull request #469 from rjeffman/fix_role_add_privileges

Fix handling members in ipa role.

plugins/modules/iparole.py

@@ -257,7 +257,7 @@ def filter_service(module, res_find, predicate):
     return _services
 
 
-def ensure_role_with_members_is_present(module, name, res_find):
+def ensure_role_with_members_is_present(module, name, res_find, action):
     """Define commands to ensure member are present for action `role`."""
     commands = []
     privilege_add, privilege_del = gen_add_del_lists(
@@ -267,7 +267,7 @@ def ensure_role_with_members_is_present(module, name, res_find):
     if privilege_add:
         commands.append([name, "role_add_privilege",
                          {"privilege": privilege_add}])
-    if privilege_del:
+    if action == "role" and privilege_del:
         commands.append([name, "role_remove_privilege",
                          {"privilege": privilege_del}])
 
@@ -297,7 +297,8 @@ def ensure_role_with_members_is_present(module, name, res_find):
 
     if add_members:
         commands.append([name, "role_add_member", add_members])
-    if del_members:
+    # Only remove members if ensuring role, not acting on members.
+    if action == "role" and del_members:
         commands.append([name, "role_remove_member", del_members])
 
     return commands
@@ -405,7 +406,9 @@ def role_commands_for_name(module, state, action, name):
             if res_find is None:
                 module.fail_json(msg="No role '%s'" % name)
 
-        cmds = ensure_role_with_members_is_present(module, name, res_find)
+        cmds = ensure_role_with_members_is_present(
+            module, name, res_find, action
+        )
         commands.extend(cmds)
 
     if state == "absent" and res_find is not None:

tests/role/env_cleanup.yml

@@ -2,31 +2,42 @@
 - name: Ensure test user is absent.
   ipauser:
     ipaadmin_password: SomeADMINpassword
-    name: user01
+    name:
+    - user01
+    - user02
+    - user03
     state: absent
 
 - name: Ensure test group is absent.
   ipagroup:
     ipaadmin_password: SomeADMINpassword
-    name: group01
+    name:
+    - group01
+    - group02
     state: absent
 
 - name: Ensure test hostgroup is absent.
   ipahostgroup:
     ipaadmin_password: SomeADMINpassword
-    name: hostgroup01
+    name:
+    - hostgroup01
+    - hostgroup02
     state: absent
 
 - name: Ensure test host is absent.
   ipahost:
     ipaadmin_password: SomeADMINpassword
-    name: "{{ host1_fqdn }}"
+    name:
+    - "{{ host1_fqdn }}"
+    - "{{ host2_fqdn }}"
     state: absent
 
 - name: Ensure test service is absent.
   ipaservice:
     ipaadmin_password: SomeADMINpassword
-    name: "service01/{{ host1_fqdn }}"
+    name:
+    - "service01/{{ host1_fqdn }}"
+    - "service02/{{ host2_fqdn }}"
     state: absent
 
 - name: Ensure test roles are absent.

tests/role/env_facts.yml

@@ -1,7 +1,7 @@
 ---
 - name: Get Domain from server name
   set_fact:
-    ipaserver_domain: "{{ ansible_fqdn | join ('.') }}"
+    ipaserver_domain: "{{ ansible_fqdn.split('.')[1:] | join ('.') }}"
   when: ipaserver_domain is not defined
 
 - name: Set fact for realm name
@@ -12,3 +12,4 @@
 - name: Create FQDN for host01
   set_fact:
     host1_fqdn: "host01.{{ ipaserver_domain }}"
+    host2_fqdn: "host02.{{ ipaserver_domain }}"

tests/role/env_setup.yml

@@ -5,30 +5,49 @@
 - name: Ensure test user is present.
   ipauser:
     ipaadmin_password: SomeADMINpassword
-    name: user01
+    users:
-    first: First
+    - name: user01
-    last: Last
+      first: First
+      last: Last
+    - name: user02
+      first: First
+      last: Last
+    - name: user03
+      first: First
+      last: Last
 
 - name: Ensure test group is present.
   ipagroup:
     ipaadmin_password: SomeADMINpassword
-    name: group01
+    name: "{{ item }}"
+  with_items:
+  - group01
+  - group02
 
 - name: Ensure test host is present.
   ipahost:
     ipaadmin_password: SomeADMINpassword
-    name: "{{ host1_fqdn }}"
+    name: "{{ item }}"
     force: yes
+  with_items:
+  - "{{ host1_fqdn }}"
+  - "{{ host2_fqdn }}"
 
 - name: Ensure test hostgroup is present.
   ipahostgroup:
     ipaadmin_password: SomeADMINpassword
-    name: hostgroup01
+    name: "{{ item[0] }}"
     host:
-    - "{{ host1_fqdn }}"
+      - "{{ item[1] }}"
+  with_nested:
+  - [hostgroup01, hostgroup02]
+  - ["{{ host1_fqdn }}", "{{ host2_fqdn }}"]
 
 - name: Ensure test service is present.
   ipaservice:
     ipaadmin_password: SomeADMINpassword
-    name: "service01/{{ host1_fqdn }}"
+    name: "{{ item }}"
     force: yes
+  with_items:
+  - "service01/{{ host1_fqdn }}"
+  - "service02/{{ host2_fqdn }}"

tests/role/test_role_lists_handling.yml

@@ -0,0 +1,259 @@
+---
+- name: Test service member in role module.
+  hosts: ipaserver
+  become: yes
+  gather_facts: yes
+
+  tasks:
+  - name: Set environment facts.
+    import_tasks: env_facts.yml
+
+  - name: Setup environment.
+    import_tasks: env_setup.yml
+
+  - name: Add role.
+    iparole:
+      ipaadmin_password: SomeADMINpassword
+      name: testrole
+      user: user01
+      group: group01
+      hostgroup: hostgroup01
+      host: "{{ host1_fqdn }}"
+      service: "service01/{{ host1_fqdn }}"
+      privilege:
+        - Automember Readers
+        - ADTrust Agents
+    register: result
+    failed_when: result.failed or not result.changed
+
+  # Test fix for https://github.com/freeipa/ansible-freeipa/issues/409
+  - name: Add new privileges to role.
+    iparole:
+      ipaadmin_password: SomeADMINpassword
+      name: testrole
+      privilege:
+        - DNS Servers
+        - Host Administrators
+        - DNS Administrators
+        - Group Administrators
+      action: member
+    register: result
+    failed_when: result.failed or not result.changed
+
+  - name: Verify role privileges.
+    shell:
+      cmd: |
+        echo SomeADMINpassword | kinit -c {{ KRB5CCNAME }} admin
+        KRB5CCNAME={{ KRB5CCNAME }} ipa role-show testrole
+        kdestroy -A -q -c {{ KRB5CCNAME }}
+    register: result
+    failed_when: |
+      result.failed or not (
+        "Automember Readers" in result.stdout
+        and "ADTrust Agents" in result.stdout
+        and "DNS Servers" in result.stdout
+        and "Host Administrators" in result.stdout
+        and "DNS Administrators" in result.stdout
+        and "Group Administrators" in result.stdout
+      )
+    vars:
+      KRB5CCNAME: verify_issue_409
+  # End of test fix for https://github.com/freeipa/ansible-freeipa/issues/409
+
+  # Test fix for https://github.com/freeipa/ansible-freeipa/issues/412
+  - name: Add new user to role.
+    iparole:
+      ipaadmin_password: SomeADMINpassword
+      name: testrole
+      user: user02
+      action: member
+    register: result
+    failed_when: result.failed or not result.changed
+
+  - name: Verify role users.
+    shell:
+      cmd: |
+        echo SomeADMINpassword | kinit -c {{ KRB5CCNAME }} admin
+        KRB5CCNAME={{ KRB5CCNAME }} ipa role-show testrole
+        kdestroy -A -q -c {{ KRB5CCNAME }}
+    register: result
+    failed_when: |
+      result.failed or not (
+        "user01" in result.stdout
+        and "user02" in result.stdout
+      )
+    vars:
+      KRB5CCNAME: verify_issue_412
+
+  - name: Add new group to role.
+    iparole:
+      ipaadmin_password: SomeADMINpassword
+      name: testrole
+      group: group02
+      action: member
+    register: result
+    failed_when: result.failed or not result.changed
+
+  - name: Verify role group.
+    shell:
+      cmd: |
+        echo SomeADMINpassword | kinit -c {{ KRB5CCNAME }} admin
+        KRB5CCNAME={{ KRB5CCNAME }} ipa role-show testrole
+        kdestroy -A -q -c {{ KRB5CCNAME }}
+    register: result
+    failed_when: |
+      result.failed or not (
+        "group01" in result.stdout
+        and "group02" in result.stdout
+      )
+    vars:
+      KRB5CCNAME: verify_issue_412
+
+  - name: Add new host to role.
+    iparole:
+      ipaadmin_password: SomeADMINpassword
+      name: testrole
+      host: "{{ host2_fqdn }}"
+      action: member
+    register: result
+    failed_when: result.failed or not result.changed
+
+  - name: Verify role hosts.
+    shell:
+      cmd: |
+        echo SomeADMINpassword | kinit -c {{ KRB5CCNAME }} admin
+        KRB5CCNAME={{ KRB5CCNAME }} ipa role-show testrole
+        kdestroy -A -q -c {{ KRB5CCNAME }}
+    register: result
+    failed_when: |
+      result.failed or not (
+        host1 in result.stdout
+        and host2 in result.stdout
+      )
+    vars:
+      KRB5CCNAME: verify_issue_412
+      host1: " {{ host1_fqdn }}"
+      host2: " {{ host2_fqdn }}"
+
+  - name: Add new hostgroup to role.
+    iparole:
+      ipaadmin_password: SomeADMINpassword
+      name: testrole
+      hostgroup: hostgroup02
+      action: member
+    register: result
+    failed_when: result.failed or not result.changed
+
+  - name: Verify role hostgroups.
+    shell:
+      cmd: |
+        echo SomeADMINpassword | kinit -c {{ KRB5CCNAME }} admin
+        KRB5CCNAME={{ KRB5CCNAME }} ipa role-show testrole
+        kdestroy -A -q -c {{ KRB5CCNAME }}
+    register: result
+    failed_when: |
+      result.failed or not (
+        " hostgroup01" in result.stdout
+        and " hostgroup02" in result.stdout
+      )
+    vars:
+      KRB5CCNAME: verify_issue_412
+
+  - name: Add new service to role.
+    iparole:
+      ipaadmin_password: SomeADMINpassword
+      name: testrole
+      service: "service02/{{ host2_fqdn }}"
+      action: member
+    register: result
+    failed_when: result.failed or not result.changed
+
+  - name: Verify role services.
+    shell:
+      cmd: |
+        echo SomeADMINpassword | kinit -c {{ KRB5CCNAME }} admin
+        KRB5CCNAME={{ KRB5CCNAME }} ipa role-show testrole
+        kdestroy -A -q -c {{ KRB5CCNAME }}
+    register: result
+    failed_when: |
+      result.failed or not (
+        service1 in result.stdout
+        and service1 in result.stdout
+      )
+    vars:
+      KRB5CCNAME: verify_issue_412
+      service1: "service01/{{ host1_fqdn }}"
+      service2: "service02/{{ host2_fqdn }}"
+  # End of test fix for https://github.com/freeipa/ansible-freeipa/issues/412
+
+  # Test fix for https://github.com/freeipa/ansible-freeipa/issues/413
+  - name: Add new user to role.
+    iparole:
+      ipaadmin_password: SomeADMINpassword
+      name: testrole
+      user: user03
+      action: member
+    register: result
+    failed_when: result.failed or not result.changed
+
+  - name: Verify role services.
+    shell:
+      cmd: |
+        echo SomeADMINpassword | kinit -c {{ KRB5CCNAME }} admin
+        KRB5CCNAME={{ KRB5CCNAME }} ipa role-show testrole
+        kdestroy -A -q -c {{ KRB5CCNAME }}
+    register: result
+    failed_when: |
+      result.failed or not (
+        service1 in result.stdout
+        and service1 in result.stdout
+        and "user03" in result.stdout
+      )
+    vars:
+      KRB5CCNAME: verify_issue_413
+      service1: "service01/{{ host1_fqdn }}"
+      service2: "service02/{{ host2_fqdn }}"
+
+  - name: Remove user from role.
+    iparole:
+      ipaadmin_password: SomeADMINpassword
+      name: testrole
+      user: user03
+      action: member
+      state: absent
+    register: result
+    failed_when: result.failed or not result.changed
+
+  - name: Verify role services.
+    shell:
+      cmd: |
+        echo SomeADMINpassword | kinit -c {{ KRB5CCNAME }} admin
+        KRB5CCNAME={{ KRB5CCNAME }} ipa role-show testrole
+        kdestroy -A -q -c {{ KRB5CCNAME }}
+    register: result
+    failed_when: |
+      result.failed or not (
+        service1 in result.stdout
+        and service1 in result.stdout
+        and "user03" not in result.stdout
+      )
+    vars:
+      KRB5CCNAME: verify_issue_413
+      service1: "service01/{{ host1_fqdn }}"
+      service2: "service02/{{ host2_fqdn }}"
+  # End of test fix for https://github.com/freeipa/ansible-freeipa/issues/413
+
+  # Test fix for https://github.com/freeipa/ansible-freeipa/issues/411
+  - name: Add non-existing user to role.
+    iparole:
+      ipaadmin_password: SomeADMINpassword
+      name: testrole
+      user: nonexisiting_user
+      action: member
+    register: result
+    failed_when: not result.failed
+  # End of test fix for https://github.com/freeipa/ansible-freeipa/issues/411
+
+  # cleanup
+  - name: Cleanup environment.
+    include_tasks: env_cleanup.yml