Merge pull request #749 from rjeffman/ipauser_fix_peserved_idempotence_issue

ipauser: Fix idempotence issue when using 'preserved'.

.github/workflows/ansible-test.yml

@@ -0,0 +1,17 @@
+---
+name: ansible-test sanity
+on:
+  - push
+  - pull_request
+jobs:
+  ansible_test:
+    name: Verify ansible-test sanity
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+        with:
+          fetch-depth: 0
+      - name: Install virtualenv using pip
+        run: pip install virtualenv
+      - name: Run ansible-test
+        run: bash tests/sanity/sanity.sh

.github/workflows/docs.yml

@@ -12,9 +12,11 @@ jobs:
       - uses: actions/setup-python@v2
         with:
           python-version: '3.x'
-      - name: Run ansible-doc-test
+      - name: Install Ansible 2.9
         run: |
           python -m pip install "ansible < 2.10"
+      - name: Run ansible-doc-test
+        run: |
           ANSIBLE_LIBRARY="." ANSIBLE_DOC_FRAGMENT_PLUGINS="." python utils/ansible-doc-test -v roles plugins
 
   check_docs_2_11:
@@ -25,9 +27,27 @@ jobs:
       - uses: actions/setup-python@v2
         with:
           python-version: '3.x'
-      - name: Run ansible-doc-test
+      - name: Install Ansible 2.11
         run: |
           python -m pip install "ansible-core >=2.11,<2.12"
+      - name: Run ansible-doc-test
+        run: |
+          ANSIBLE_LIBRARY="." ANSIBLE_DOC_FRAGMENT_PLUGINS="." python utils/ansible-doc-test -v roles plugins
+
+  check_docs_2_12:
+    name: Check Ansible Documentation with ansible-core 2.12.
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions/setup-python@v2
+        with:
+          python-version: '3.x'
+      - name: Install Ansible 2.12
+        run: |
+          python -m pip install "ansible-core >=2.12,<2.13"
+      - name: Run ansible-doc-test
+        run: |
+          python -m pip install "ansible-core >=2.12,<2.13"
           ANSIBLE_LIBRARY="." ANSIBLE_DOC_FRAGMENT_PLUGINS="." python utils/ansible-doc-test -v roles plugins
 
   check_docs_latest:
@@ -38,7 +58,9 @@ jobs:
       - uses: actions/setup-python@v2
         with:
           python-version: '3.x'
-      - name: Run ansible-doc-test
+      - name: Install Ansible-latest
         run: |
           python -m pip install ansible
+      - name: Run ansible-doc-test
+        run: |
           ANSIBLE_LIBRARY="." ANSIBLE_DOC_FRAGMENT_PLUGINS="." python utils/ansible-doc-test -v roles plugins

.github/workflows/lint.yml

@@ -13,15 +13,9 @@ jobs:
         with:
           python-version: "3.x"
       - name: Run ansible-lint
-        uses: ansible/ansible-lint-action@master
-        with:
-          targets: |
-            tests/*.yml
-            tests/*/*.yml
-            tests/*/*/*.yml
-            playbooks/*.yml
-            playbooks/*/*.yml
-            roles/*/*/*.yml
+        run: |
+          pip install ansible-core==2.11.6 ansible-lint
+          find playbooks roles tests -name '*.yml' ! -name "env_*" ! -name "tasks_*" -exec ansible-lint --force-color {} \+
         env:
           ANSIBLE_MODULE_UTILS: plugins/module_utils
           ANSIBLE_LIBRARY: plugins/modules
@@ -75,7 +69,7 @@ jobs:
       - name: Run pylint
         run: |
             pip install pylint==2.10.2
-            pylint plugins --disable=import-error
+            pylint plugins roles --disable=import-error
 
   shellcheck:
     name: Shellcheck

.github/workflows/readme.yml

@@ -0,0 +1,16 @@
+---
+name: readme test
+on:
+  - push
+  - pull_request
+jobs:
+  ansible_test:
+    name: Verify readme
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - name: Run readme test
+        run: |
+          error=0
+          for i in roles/ipa*/README.md README-*.md; do grep -q $i README.md && echo "OK: $i" || { echo -e "\033[31;1mERROR: ${i} missing\033[0m"; error=1; } done
+          exit $error

.pre-commit-config.yaml

@@ -1,7 +1,7 @@
 ---
 repos:
 - repo: https://github.com/ansible/ansible-lint.git
-  rev: v5.1.2
+  rev: v5.3.2
   hooks:
   - id: ansible-lint
     always_run: false
@@ -38,8 +38,10 @@ repos:
     entry: utils/ansible-doc-test
     # args: ['-v', 'roles', 'plugins']
     files: ^.*.py$
-- repo: https://github.com/koalaman/shellcheck-precommit
-  rev: v0.8.0
+- repo: local
   hooks:
   - id: shellcheck
-    args: ["--severity=warning"]  # Only show errors and warnings
+    name: ShellCheck
+    language: system
+    entry: shellcheck
+    files: \.sh$

README-automember.md

@@ -262,9 +262,6 @@ Example playbook to ensure all orphan automember hostgroup rules are removed:
 Variables
 ---------
 
-ipaautomember
--------
-
 Variable | Description | Required
 -------- | ----------- | --------
 `ipaadmin_principal` | The admin principal is a string and defaults to `admin` | no

README-automountkey.md

@@ -0,0 +1,112 @@
+Automountkey module
+=====================
+
+Description
+-----------
+
+The automountkey module allows management of keys within an automount map.
+
+It is desgined to follow the IPA api as closely as possible while ensuring ease of use.
+
+
+Features
+--------
+* Automount key management
+
+Supported FreeIPA Versions
+--------------------------
+
+FreeIPA versions 4.4.0 and up are supported by the ipaautomountkey module.
+
+Requirements
+------------
+**Controller**
+* Ansible version: 2.8+
+
+**Node**
+* Supported FreeIPA version (see above)
+
+
+Usage
+=====
+
+Example inventory file
+
+```ini
+[ipaserver]
+ipaserver.test.local
+```
+
+
+Example playbook to ensure presence of an automount key:
+
+```yaml
+---
+- name: Playbook to manage automount key
+  hosts: ipaserver
+
+  tasks:
+  - name: ensure automount key TestKey is present
+    ipaautomountkey:
+      ipaadmin_password: SomeADMINpassword
+      location: TestLocation
+      mapname: TestMap
+      key: TestKey
+      info: 192.168.122.1:/exports
+      state: present
+```
+
+Example playbook to rename an automount map:
+
+```yaml
+---
+- name: Playbook to add an automount map
+  hosts: ipaserver
+
+  tasks:
+  - name: ensure aumount key TestKey is renamed to NewKeyName
+    ipaautomountkey:
+      ipaadmin_password: password01
+      automountlocationcn: TestLocation
+      automountmapname: TestMap
+      automountkey: TestKey
+      newname: NewKeyName
+      state: renamed
+```
+
+Example playbook to ensure an automount key is absent:
+
+```yaml
+---
+- name: Playbook to manage an automount key
+  hosts: ipaserver
+
+  tasks:
+  - name: ensure automount key TestKey is absent
+    ipaautomountkey:
+      ipaadmin_password: SomeADMINpassword
+      location: TestLocation
+      mapname: TestMap
+      key: TestKey
+      state: absent
+```
+
+
+Variables
+=========
+
+Variable | Description | Required
+-------- | ----------- | --------
+`ipaadmin_principal` | The admin principal is a string and defaults to `admin` | no
+`ipaadmin_password` | The admin password is a string and is required if there is no admin ticket available on the node | no
+`location` \| `automountlocationcn` \| `automountlocation` | Location name. | yes
+`mapname` \|  `map` \| `automountmapname` \| `automountmap` | Map the key belongs to | yes
+`key` \| `name` \| `automountkey` | Automount key to manage | yes
+`rename` \| `new_name` \| `newautomountkey` | the name to change the key to if state is `renamed` | yes when state is `renamed`
+`info` \| `information` \| `automountinformation` | Mount information for the key | yes when state is `present`
+`state` | The state to ensure. It can be one of `present`, `absent` or `renamed`, default: `present`. | no
+
+Authors
+=======
+
+Chris Procter

README-automountlocation.md

@@ -97,9 +97,6 @@ Example playbook to ensure absence of an automount location:
 Variables
 =========
 
-ipaautomountlocation
--------
-
 Variable | Description | Required
 -------- | ----------- | --------
 `ipaadmin_principal` | The admin principal is a string and defaults to `admin` | no

README-automountmap.md

@@ -0,0 +1,96 @@
+Automountmap module
+=====================
+
+Description
+-----------
+
+The automountmap module allows the addition and removal of maps within automount locations.
+
+It is desgined to follow the IPA api as closely as possible while ensuring ease of use.
+
+
+Features
+--------
+* Automount map management
+
+Supported FreeIPA Versions
+--------------------------
+
+FreeIPA versions 4.4.0 and up are supported by the ipaautomountmap module.
+
+Requirements
+------------
+**Controller**
+* Ansible version: 2.8+
+
+**Node**
+* Supported FreeIPA version (see above)
+
+
+Usage
+=====
+
+Example inventory file
+
+```ini
+[ipaserver]
+ipaserver.test.local
+```
+
+Example playbook to ensure presence of an automount map:
+
+```yaml
+---
+- name: Playbook to add an automount map
+  hosts: ipaserver
+  become: no
+
+  tasks:
+  - name: ensure map named auto.DMZ in location DMZ is created
+    ipaautomountmap:
+      ipaadmin_password: SomeADMINpassword
+      name: auto.DMZ
+      location: DMZ
+      desc: "this is a map for servers in the DMZ"
+```
+
+Example playbook to ensure auto.DMZi is absent:
+
+```yaml
+---
+- name: Playbook to remove an automount map
+  hosts: ipaserver
+  become: no
+
+  tasks:
+  - name: ensure map auto.DMZ has been removed
+    ipaautomountmap:
+      ipaadmin_password: SomeADMINpassword
+      name: auto.DMZ
+      location: DMZ
+      state: absent
+```
+
+
+Variables
+=========
+
+Variable | Description | Required
+-------- | ----------- | --------
+`ipaadmin_principal` | The admin principal is a string and defaults to `admin` | no
+`ipaadmin_password` | The admin password is a string and is required if there is no admin ticket available on the node | no
+`name` \| `mapname` \| `map` \| `automountmapname` | Name of the map to manage | yes
+`location` \| `automountlocation` \| `automountlocationcn` | Location name. | yes
+`desc` \| `description` | Description of the map | yes
+`state` | The state to ensure. It can be one of `present`, or `absent`, default: `present`. | no
+
+
+Notes
+=====
+
+Creation of indirect mount points are not supported.
+
+Authors
+=======
+
+Chris Procter

README-config.md

@@ -82,9 +82,6 @@ Example playbook to read config options:
 Variables
 =========
 
-ipauser
--------
-
 **General Variables:**
 
 Variable | Description | Required

README-delegation.md

@@ -135,9 +135,6 @@ Example playbook to make sure delegation "basic manager attributes" is absent:
 Variables
 ---------
 
-ipadelegation
--------
-
 Variable | Description | Required
 -------- | ----------- | --------
 `ipaadmin_principal` | The admin principal is a string and defaults to `admin` | no

README-dnsconfig.md

@@ -71,6 +71,7 @@ Example playbook to ensure a global forwarder, with a custom port, is absent:
       forwarders:
           - ip_address: 2001:4860:4860::8888
             port: 53
+      action: member
       state: absent
 ```
 
@@ -119,9 +120,6 @@ Example playbook to disallow synchronization of forward (A, AAAA) and reverse (P
 Variables
 =========
 
-ipadnsconfig
-------------
-
 Variable | Description | Required
 -------- | ----------- | --------
 `ipaadmin_principal` | The admin principal is a string and defaults to `admin` | no
@@ -133,7 +131,8 @@ Variable | Description | Required
   | `port` - The custom port that should be used on this server. | no
 `forward_policy` | The global forwarding policy. It can be one of `only`, `first`, or `none`.  | no
 `allow_sync_ptr` | Allow synchronization of forward (A, AAAA) and reverse (PTR) records (bool). | yes
-`state` | The state to ensure. It can be one of `present` or `absent`, default: `present`. | yes
+`action` | Work on dnsconfig or member level. It can be one of `member` or `dnsconfig` and defaults to `dnsconfig`. Only `forwarders` can be managed with `action: member`. | no
+`state` | The state to ensure. It can be one of `present` or `absent`, default: `present`. `absent` can only be used with `action: member` and `forwarders`. | yes
 
 
 Authors

README-dnsforwardzone.md

@@ -100,9 +100,6 @@ Example playbook to ensure presence of a forwardzone to ipa DNS:
 Variables
 =========
 
-ipagroup
--------
-
 Variable | Description | Required
 -------- | ----------- | --------
 `ipaadmin_principal` | The admin principal is a string and defaults to `admin` | no

README-dnsrecord.md

@@ -242,9 +242,6 @@ Example playbook to ensure multiple DNS records are absent:
 Variables
 =========
 
-ipadnsrecord
-------------
-
 Variable | Description | Required
 -------- | ----------- | --------
 `ipaadmin_principal` | The admin principal is a string and defaults to `admin` | no

README-dnszone.md

@@ -195,9 +195,6 @@ Example playbook to create a zone for reverse DNS lookup, from an IP address, gi
 Variables
 =========
 
-ipadnszone
-----------
-
 Variable | Description | Required
 -------- | ----------- | --------
 `ipaadmin_principal` | The admin principal is a string and defaults to `admin` | no
@@ -233,9 +230,6 @@ Variable | Description | Required
 Return Values
 =============
 
-ipadnszone
-----------
-
 Variable | Description | Returned When
 -------- | ----------- | -------------
 `dnszone` | DNS Zone dict with zone name infered from `name_from_ip`. <br>Options: |  If `state` is `present`, `name_from_ip` is used, and a zone was created.

README-group.md

@@ -147,9 +147,6 @@ Example playbook to remove groups:
 Variables
 =========
 
-ipagroup
--------
-
 Variable | Description | Required
 -------- | ----------- | --------
 `ipaadmin_principal` | The admin principal is a string and defaults to `admin` | no

README-hbacrule.md

@@ -129,9 +129,6 @@ Example playbook to make sure HBAC Rule login is absent:
 Variables
 =========
 
-ipahbacrule
----------------
-
 Variable | Description | Required
 -------- | ----------- | --------
 `ipaadmin_principal` | The admin principal is a string and defaults to `admin` | no

README-hbacsvc.md

@@ -91,9 +91,6 @@ Example playbook to make sure HBAC Services for http and tftp are absent
 Variables
 =========
 
-ipahbacsvc
-----------
-
 Variable | Description | Required
 -------- | ----------- | --------
 `ipaadmin_principal` | The admin principal is a string and defaults to `admin` | no

README-hbacsvcgroup.md

@@ -129,9 +129,6 @@ Example playbook to make sure HBAC Service Group login is absent:
 Variables
 =========
 
-ipahbacsvcgroup
----------------
-
 Variable | Description | Required
 -------- | ----------- | --------
 `ipaadmin_principal` | The admin principal is a string and defaults to `admin` | no

README-host.md

@@ -313,9 +313,6 @@ Example playbook to ensure a host is absent:
 Variables
 =========
 
-ipahost
--------
-
 Variable | Description | Required
 -------- | ----------- | --------
 `ipaadmin_principal` | The admin principal is a string and defaults to `admin` | no
@@ -370,9 +367,6 @@ Variable | Description | Required
 Return Values
 =============
 
-ipahost
--------
-
 There are only return values if one or more random passwords have been generated.
 
 Variable | Description | Returned When

README-hostgroup.md

@@ -143,9 +143,6 @@ Example playbook to make sure host-group databases is absent:
 Variables
 =========
 
-ipahostgroup
--------
-
 Variable | Description | Required
 -------- | ----------- | --------
 `ipaadmin_principal` | The admin principal is a string and defaults to `admin` | no

README-location.md

@@ -74,9 +74,6 @@ Example playbook to make sure location "my_location1" is absent:
 Variables
 ---------
 
-ipalocation
--------
-
 Variable | Description | Required
 -------- | ----------- | --------
 `ipaadmin_principal` | The admin principal is a string and defaults to `admin` | no

README-permission.md

@@ -154,9 +154,6 @@ Example playbook to make sure permission "MyPermission" is renamed to "MyNewPerm
 Variables
 ---------
 
-ipapermission
--------
-
 Variable | Description | Required
 -------- | ----------- | --------
 `ipaadmin_principal` | The admin principal is a string and defaults to `admin` | no

README-privilege.md

@@ -126,9 +126,6 @@ Example playbook to make sure privilege "DNS Special Privilege" is absent:
 Variables
 ---------
 
-ipaprivilege
-------------
-
 Variable | Description | Required
 -------- | ----------- | --------
 `ipaadmin_principal` | The admin principal is a string and defaults to `admin`. | no

README-pwpolicy.md

@@ -91,9 +91,6 @@ Example playbook to ensure maxlife is set to 49 in global policy:
 Variables
 =========
 
-ipapwpolicy
--------
-
 Variable | Description | Required
 -------- | ----------- | --------
 `ipaadmin_principal` | The admin principal is a string and defaults to `admin` | no

README-role.md

@@ -238,9 +238,6 @@ Example playbook to ensure that different members are not associated with a role
 Variables
 ---------
 
-iparole
--------
-
 Variable | Description | Required
 -------- | ----------- | --------
 `ipaadmin_principal` | The admin principal is a string and defaults to `admin` | no

README-selfservice.md

@@ -131,9 +131,6 @@ Example playbook to make sure selfservice "Users can manage their own name detai
 Variables
 ---------
 
-ipaselfservice
--------
-
 Variable | Description | Required
 -------- | ----------- | --------
 `ipaadmin_principal` | The admin principal is a string and defaults to `admin` | no

README-server.md

@@ -242,9 +242,6 @@ This task will always report a change.
 Variables
 ---------
 
-ipaserver
--------
-
 Variable | Description | Required
 -------- | ----------- | --------
 `ipaadmin_principal` | The admin principal is a string and defaults to `admin` | no

README-service.md

@@ -285,8 +285,6 @@ Example playbook to allow users, groups, hosts or hostgroups to retrieve a keyta
 Variables
 ---------
 
-ipaservice
-
 Variable | Description | Required
 -------- | ----------- | --------
 `ipaadmin_principal` | The admin principal is a string and defaults to `admin` | no

README-sudocmd.md

@@ -76,9 +76,6 @@ Example playbook to make sure sudocmd is absent:
 Variables
 =========
 
-ipasudocmd
--------
-
 Variable | Description | Required
 -------- | ----------- | --------
 `ipaadmin_principal` | The admin principal is a string and defaults to `admin` | no

README-sudocmdgroup.md

@@ -116,9 +116,6 @@ Example playbook to make sure sudocmdgroup is absent:
 Variables
 =========
 
-ipasudocmdgroup
--------
-
 Variable | Description | Required
 -------- | ----------- | --------
 `ipaadmin_principal` | The admin principal is a string and defaults to `admin` | no

README-sudorule.md

@@ -113,9 +113,6 @@ Example playbook to make sure Sudo Rule is absent:
 Variables
 =========
 
-ipasudorule
----------------
-
 Variable | Description | Required
 -------- | ----------- | --------
 `ipaadmin_principal` | The admin principal is a string and defaults to `admin` | no

README-trust.md

@@ -94,9 +94,6 @@ This will only delete the ipa-side of the trust and it does NOT delete the id-ra
 Variables
 =========
 
-ipatrust
--------
-
 Variable | Description | Required
 -------- | ----------- | --------
 `ipaadmin_principal` | The admin principal is a string and defaults to `admin` | no

README-user.md

@@ -356,9 +356,6 @@ Example playbook to ensure users are absent:
 Variables
 =========
 
-ipauser
--------
-
 **General Variables:**
 
 Variable | Description | Required
@@ -432,9 +429,6 @@ Variable | Description | Required
 Return Values
 =============
 
-ipauser
--------
-
 There are only return values if one or more random passwords have been generated.
 
 Variable | Description | Returned When

README-vault.md

@@ -210,9 +210,6 @@ Example playbook to make sure vault is absent:
 Variables
 =========
 
-ipavault
--------
-
 Variable | Description | Required
 -------- | ----------- | --------
 `ipaadmin_principal` | The admin principal is a string and defaults to `admin` | no
@@ -250,9 +247,6 @@ Variable | Description | Required
 Return Values
 =============
 
-ipavault
---------
-
 There is only a return value if `state` is `retrieved`.
 
 Variable | Description | Returned When

README.md

@@ -13,6 +13,9 @@ Features
 * Repair mode for clients
 * Backup and restore, also to and from controller
 * Modules for automembership rule management
+* Modules for automount key management
+* Modules for automount location management
+* Modules for automount map management
 * Modules for config management
 * Modules for delegation management
 * Modules for dns config management
@@ -62,7 +65,6 @@ Requirements
 **Controller**
 * Ansible version: 2.8+ (ansible-freeipa is an Ansible Collection)
 * /usr/bin/kinit is required on the controller if a one time password (OTP) is used
-* python3-gssapi is required on the controller if a one time password (OTP) is used with keytab to install the client.
 
 **Node**
 * Supported FreeIPA version (see above)
@@ -282,7 +284,8 @@ ipaserver_domain=test.local
 ipaserver_realm=TEST.LOCAL
 ```
 
-For enhanced security it is possible to use a auto-generated one-time-password (OTP). This will be generated on the controller using the (first) server. It is needed to have the python-gssapi bindings installed on the controller for this.
+For enhanced security it is possible to use a auto-generated one-time-password (OTP). This will be generated on the controller using the (first) server.
+
 To enable the generation of the one-time-password:
 ```yaml
 [ipaclients:vars]
@@ -424,6 +427,9 @@ Modules in plugin/modules
 =========================
 
 * [ipaautomember](README-automember.md)
+* [ipaautomountkey](README-automountkey.md)
+* [ipaautomountlocation](README-automountlocation.md)
+* [ipaautomountmap](README-automountmap.md)
 * [ipaconfig](README-config.md)
 * [ipadelegation](README-delegation.md)
 * [ipadnsconfig](README-dnsconfig.md)
@@ -433,15 +439,15 @@ Modules in plugin/modules
 * [ipagroup](README-group.md)
 * [ipahbacrule](README-hbacrule.md)
 * [ipahbacsvc](README-hbacsvc.md)
-* [ipahbacsvcgroup](README-hbacsvc.md)
+* [ipahbacsvcgroup](README-hbacsvcgroup.md)
 * [ipahost](README-host.md)
 * [ipahostgroup](README-hostgroup.md)
-* [ipalocation](README-ipalocation.md)
-* [ipapermission](README-ipapermission.md)
-* [ipaprivilege](README-ipaprivilege.md)
+* [ipalocation](README-location.md)
+* [ipapermission](README-permission.md)
+* [ipaprivilege](README-privilege.md)
 * [ipapwpolicy](README-pwpolicy.md)
 * [iparole](README-role.md)
-* [ipaselfservice](README-ipaselfservice.md)
+* [ipaselfservice](README-selfservice.md)
 * [ipaserver](README-server.md)
 * [ipaservice](README-service.md)
 * [ipasudocmd](README-sudocmd.md)

galaxy.yml

@@ -16,6 +16,7 @@ readme: "README.md"
 license: "GPL-3.0-or-later"
 
 tags:
+  - "linux"
   - "system"
   - "identity"
   - "ipa"

molecule/centos-9/molecule.yml

@@ -0,0 +1,18 @@
+---
+driver:
+  name: docker
+platforms:
+  - name: centos-9
+    image: quay.io/ansible-freeipa/upstream-tests:centos-9
+    pre_build_image: true
+    hostname: ipaserver.test.local
+    dns_servers:
+      - 127.0.0.1
+    volumes:
+      - /sys/fs/cgroup:/sys/fs/cgroup:ro
+    command: /usr/sbin/init
+    privileged: true
+provisioner:
+  name: ansible
+  playbooks:
+    prepare: ../resources/playbooks/prepare.yml

molecule/resources/playbooks/prepare.yml

@@ -25,3 +25,24 @@
     ansible.builtin.service:
       name: ipa
       state: started
+
+  - name: Wait for krb5dkc to be running
+    ansible.builtin.service_facts:
+    no_log: True
+    register: result
+    until: "'krb5kdc.service' in result.ansible_facts.services and \
+            result.ansible_facts.services['krb5kdc.service'].state == 'running'"
+    retries: 30
+    delay: 5
+
+  - name: Check if TGT is available for admin.
+    ansible.builtin.shell:
+      cmd: echo SomeADMINpassword | kinit -c ansible_freeipa_cache admin
+    register: result
+    until: not result.failed
+    retries: 30
+    delay: 5
+
+  - name: Cleanup TGT.
+    ansible.builtin.shell:
+      cmd: kdestroy -c ansible_freeipa_cache -A

playbooks/automount/automount-map-absent.yaml

@@ -0,0 +1,12 @@
+---
+- name: Automount map absent example
+  hosts: ipaserver
+  become: no
+
+  tasks:
+  - name: ensure map TestMap is absent
+    ipaautomountmap:
+      ipaadmin_password: SomeADMINpassword
+      name: TestMap
+      location: TestLocation
+      state: absent

playbooks/automount/automount-map-present.yaml

@@ -0,0 +1,12 @@
+---
+- name: Automount map present example
+  hosts: ipaserver
+  become: no
+
+  tasks:
+  - name: ensure map TestMap is present
+    ipaautomountmap:
+      ipaadmin_password: SomeADMINpassword
+      name: TestMap
+      location: TestLocation
+      desc: "this is a test map"

playbooks/automount/automountkey-present.yml

@@ -0,0 +1,13 @@
+---
+- name: Playbook to manage an automout key
+  hosts: ipaserver
+
+  tasks:
+  - name: Ensure autmount key is present
+    ipaautomountkey:
+      ipaadmin_password: SomeADMINpassword
+      location: TestLocation
+      mapname: TestMap
+      key: TestKey
+      info: 192.168.122.1:/exports
+      state: present

playbooks/automount/automountkey-renamed.yml

@@ -0,0 +1,13 @@
+---
+- name: Playbook to manage an automount key
+  hosts: ipaserver
+
+  tasks:
+  - name: Ensure aumount key TestKey is renamed to NewKeyName
+    ipaautomountkey:
+      ipaadmin_password: password01
+      automountlocationcn: TestLocation
+      automountmapname: TestMap
+      automountkey: TestKey
+      newname: NewKeyName
+      state: renamed

playbooks/automount/automoutkey-absent.yml

@@ -0,0 +1,12 @@
+---
+- name: Playbook to manage an automount key
+  hosts: ipaserver
+
+  tasks:
+  - name: Ensure autmount key is present
+    ipaautomountkey:
+      ipaadmin_password: SomeADMINpassword
+      location: TestLocation
+      mapname: TestMap
+      key: TestKey
+      state: absent

playbooks/dnsconfig/forwarders-absent.yml

@@ -4,10 +4,11 @@
   become: true
 
   tasks:
-  - name: Set dnsconfig.
+  - name: Set dnsconfig forwarders.
     ipadnsconfig:
       forwarders:
         - ip_address: 8.8.4.4
         - ip_address: 2001:4860:4860::8888
           port: 53
+      action: member
       state: absent

playbooks/dnsconfig/forwarders-present.yml

@@ -0,0 +1,13 @@
+---
+- name: Playbook to handle global DNS configuration
+  hosts: ipaserver
+  become: true
+
+  tasks:
+  - name: Set dnsconfig forwarders.
+    ipadnsconfig:
+      forwarders:
+        - ip_address: 8.8.4.4
+        - ip_address: 2001:4860:4860::8888
+          port: 53
+      action: member

plugins/module_utils/ansible_freeipa_module.py

@@ -875,10 +875,11 @@ else:
             """
             return api_command_no_name(self, command, args)
 
-        @staticmethod
-        def ipa_get_domain():
+        def ipa_get_domain(self):
             """Retrieve IPA API domain."""
-            return api_get_domain()
+            if not hasattr(self, "__ipa_api_domain"):
+                setattr(self, "__ipa_api_domain", api_get_domain())
+            return getattr(self, "__ipa_api_domain")
 
         @staticmethod
         def ipa_get_realm():

plugins/modules/ipaautomountkey.py

@@ -0,0 +1,235 @@
+#!/usr/bin/python
+# -*- coding: utf-8 -*-
+
+# Authors:
+#   Chris Procter <cprocter@redhat.com>
+#
+# Copyright (C) 2021 Red Hat
+# see file 'COPYING' for use and warranty information
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+from __future__ import (absolute_import, division, print_function)
+
+__metaclass__ = type
+
+ANSIBLE_METADATA = {
+    "metadata_version": "1.0",
+    "supported_by": "community",
+    "status": ["preview"],
+}
+
+
+DOCUMENTATION = '''
+---
+module: ipaautomountkey
+author: chris procter
+short_description: Manage FreeIPA autommount map
+description:
+- Add, delete, and modify an IPA automount map
+options:
+  ipaadmin_principal:
+    description: The admin principal
+    default: admin
+  ipaadmin_password:
+    description: The admin password
+    required: False
+  location:
+    description: automount location map is in
+    required: True
+    choices: ["automountlocationcn", "automountlocation"]
+  mapname:
+    description: automount map to be managed
+    choices: ["map", "automountmapname", "automountmap"]
+    required: True
+  key:
+    description: automount key to be managed
+    required: True
+    choices: ["name", "automountkey"]
+  newkey:
+    description: key to change to if state is 'renamed'
+    required: True
+    choices: ["newname", "newautomountkey"]
+  info:
+    description: Mount information for the key
+    required: True
+    choices: ["information", "automountinformation"]
+  state:
+    description: State to ensure
+    required: False
+    default: present
+    choices: ["present", "absent", "renamed"]
+'''
+
+EXAMPLES = '''
+  - name: create key TestKey
+    ipaautomountkey:
+      ipaadmin_password: SomeADMINpassword
+      locationcn: TestLocation
+      mapname: TestMap
+      key: TestKey
+      info: 192.168.122.1:/exports
+      state: present
+
+  - name: ensure key TestKey is absent
+    ipaautomountkey:
+      ipaadmin_password: SomeADMINpassword
+      location: TestLocation
+      mapname: TestMap
+      key: TestKey
+      state: absent
+'''
+
+RETURN = '''
+'''
+
+from ansible.module_utils.ansible_freeipa_module import (
+    IPAAnsibleModule, ipalib_errors
+)
+
+
+class AutomountKey(IPAAnsibleModule):
+
+    def __init__(self, *args, **kwargs):
+        # pylint: disable=super-with-arguments
+        super(AutomountKey, self).__init__(*args, **kwargs)
+        self.commands = []
+
+    def get_key(self, location, mapname, key):
+        try:
+            args = {
+                "automountmapautomountmapname": mapname,
+                "automountkey": key,
+                "all": True,
+            }
+            resp = self.ipa_command("automountkey_show", location, args)
+        except ipalib_errors.NotFound:
+            return None
+        else:
+            return resp.get("result")
+
+    def check_ipa_params(self):
+        invalid = []
+        state = self.params_get("state")
+        if state == "present":
+            invalid = ["rename"]
+            if not self.params_get("info"):
+                self.fail_json(msg="Value required for argument 'info'")
+
+        if state == "rename":
+            invalid = ["info"]
+            if not self.params_get("rename"):
+                self.fail_json(msg="Value required for argument 'renamed'")
+
+        if state == "absent":
+            invalid = ["info", "rename"]
+
+        self.params_fail_used_invalid(invalid, state)
+
+    @staticmethod
+    def get_args(mapname, key, info, rename):
+        _args = {}
+        if mapname:
+            _args["automountmapautomountmapname"] = mapname
+        if key:
+            _args["automountkey"] = key
+        if info:
+            _args["automountinformation"] = info
+        if rename:
+            _args["rename"] = rename
+        return _args
+
+    def define_ipa_commands(self):
+        state = self.params_get("state")
+        location = self.params_get("location")
+        mapname = self.params_get("mapname")
+        key = self.params_get("key")
+        info = self.params_get("info")
+        rename = self.params_get("rename")
+
+        args = self.get_args(mapname, key, info, rename)
+
+        res_find = self.get_key(location, mapname, key)
+
+        if state == "present":
+            if res_find is None:
+                # does not exist and is wanted
+                self.commands.append([location, "automountkey_add", args])
+            else:
+                # exists and is wanted, check for changes
+                if info not in res_find.get("automountinformation"):
+                    self.commands.append([location, "automountkey_mod", args])
+
+        if state == "renamed":
+            if res_find is None:
+                self.fail_json(
+                    msg=(
+                        "Cannot rename inexistent key: '%s', '%s', '%s'"
+                        % (location, mapname, key)
+                    )
+                )
+            self.commands.append([location, "automountkey_mod", args])
+
+        if state == "absent":
+            # if key exists and self.ipa_params.state == "absent":
+            if res_find is not None:
+                self.commands.append([location, "automountkey_del", args])
+
+
+def main():
+    ipa_module = AutomountKey(
+        argument_spec=dict(
+            state=dict(
+                type='str',
+                choices=['present', 'absent', 'renamed'],
+                required=None,
+                default='present',
+            ),
+            location=dict(
+                type="str",
+                aliases=["automountlocationcn", "automountlocation"],
+                required=True,
+            ),
+            rename=dict(
+                type="str",
+                aliases=["new_name", "newautomountkey"],
+                required=False,
+            ),
+            mapname=dict(
+                type="str",
+                aliases=["map", "automountmapname", "automountmap"],
+                required=True,
+            ),
+            key=dict(
+                type="str",
+                aliases=["name", "automountkey"],
+                required=True,
+            ),
+            info=dict(
+                type="str",
+                aliases=["information", "automountinformation"],
+                required=False,
+            ),
+        ),
+    )
+    ipaapi_context = ipa_module.params_get("ipaapi_context")
+    with ipa_module.ipa_connect(context=ipaapi_context):
+        ipa_module.check_ipa_params()
+        ipa_module.define_ipa_commands()
+        changed = ipa_module.execute_ipa_commands(ipa_module.commands)
+    ipa_module.exit_json(changed=changed)
+
+
+if __name__ == "__main__":
+    main()

plugins/modules/ipaautomountmap.py

@@ -0,0 +1,195 @@
+#!/usr/bin/python
+# -*- coding: utf-8 -*-
+# Authors:
+#   Chris Procter <cprocter@redhat.com>
+#
+# Copyright (C) 2021 Red Hat
+# see file 'COPYING' for use and warranty information
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+from __future__ import (absolute_import, division, print_function)
+
+__metaclass__ = type
+
+ANSIBLE_METADATA = {
+    "metadata_version": "1.0",
+    "supported_by": "community",
+    "status": ["preview"],
+}
+
+
+DOCUMENTATION = '''
+---
+module: ipaautomountmap
+author: Chris Procter
+short_description: Manage FreeIPA autommount map
+description:
+- Add, delete, and modify an IPA automount map
+options:
+  ipaadmin_principal:
+    description: The admin principal.
+    default: admin
+  ipaadmin_password:
+    description: The admin password.
+    required: false
+  automountlocation:
+    description: automount location map is anchored to
+    choices: ["location", "automountlocationcn"]
+    required: True
+  name:
+    description: automount map to be managed.
+    choices: ["mapname", "map", "automountmapname"]
+    required: True
+  desc:
+    description: description of automount map.
+    choices: ["description"]
+    required: false
+  state:
+    description: State to ensure
+    required: false
+    default: present
+    choices: ["present", "absent"]
+'''
+
+EXAMPLES = '''
+  - name: ensure map named auto.DMZ in location DMZ is present
+    ipaautomountmap:
+      ipaadmin_password: SomeADMINpassword
+      name: auto.DMZ
+      location: DMZ
+      desc: "this is a map for servers in the DMZ"
+
+  - name: remove a map named auto.DMZ in location DMZ if it exists
+    ipaautomountmap:
+      ipaadmin_password: SomeADMINpassword
+      name: auto.DMZ
+      location: DMZ
+      state: absent
+'''
+
+RETURN = '''
+'''
+
+from ansible.module_utils.ansible_freeipa_module import (
+    IPAAnsibleModule, compare_args_ipa
+)
+
+
+class AutomountMap(IPAAnsibleModule):
+
+    def __init__(self, *args, **kwargs):
+        # pylint: disable=super-with-arguments
+        super(AutomountMap, self).__init__(*args, **kwargs)
+        self.commands = []
+
+    def get_automountmap(self, location, name):
+        try:
+            response = self.ipa_command(
+                "automountmap_show",
+                location,
+                {"automountmapname": name, "all": True}
+            )
+        except Exception:  # pylint: disable=broad-except
+            return None
+        else:
+            return response["result"]
+
+    def check_ipa_params(self):
+        invalid = []
+        name = self.params_get("name")
+        state = self.params_get("state")
+        if state == "present":
+            if len(name) != 1:
+                self.fail_json(msg="Exactly one name must be provided \
+                                for state=present.")
+        if state == "absent":
+            if len(name) == 0:
+                self.fail_json(msg="Argument 'map_type' can not be used with "
+                                   "state 'absent'")
+            invalid = ["desc"]
+
+        self.params_fail_used_invalid(invalid, state)
+
+    def get_args(self, mapname, desc):  # pylint: disable=no-self-use
+        _args = {}
+        if mapname:
+            _args["automountmapname"] = mapname
+        if desc:
+            _args["description"] = desc
+        return _args
+
+    def define_ipa_commands(self):
+        name = self.params_get("name")
+        state = self.params_get("state")
+        location = self.params_get("location")
+        desc = self.params_get("desc")
+
+        for mapname in name:
+            automountmap = self.get_automountmap(location, mapname)
+
+            if state == "present":
+                args = self.get_args(mapname, desc)
+                if automountmap is None:
+                    self.commands.append([location, "automountmap_add", args])
+                else:
+                    if not compare_args_ipa(self, args, automountmap):
+                        self.commands.append(
+                            [location, "automountmap_mod", args]
+                        )
+
+            if state == "absent":
+                if automountmap is not None:
+                    self.commands.append([
+                        location,
+                        "automountmap_del",
+                        {"automountmapname": [mapname]}
+                    ])
+
+
+def main():
+    ipa_module = AutomountMap(
+        argument_spec=dict(
+            state=dict(type='str',
+                       default='present',
+                       choices=['present', 'absent']
+                       ),
+            location=dict(type="str",
+                          aliases=["automountlocation", "automountlocationcn"],
+                          default=None,
+                          required=True
+                          ),
+            name=dict(type="list",
+                      aliases=["mapname", "map", "automountmapname"],
+                      default=None,
+                      required=True
+                      ),
+            desc=dict(type="str",
+                      aliases=["description"],
+                      required=False,
+                      default=None
+                      ),
+        ),
+    )
+    changed = False
+    ipaapi_context = ipa_module.params_get("ipaapi_context")
+    with ipa_module.ipa_connect(context=ipaapi_context):
+        ipa_module.check_ipa_params()
+        ipa_module.define_ipa_commands()
+        changed = ipa_module.execute_ipa_commands(ipa_module.commands)
+    ipa_module.exit_json(changed=changed)
+
+
+if __name__ == "__main__":
+    main()

plugins/modules/ipadnsconfig.py

@@ -59,8 +59,16 @@ options:
       Allow synchronization of forward (A, AAAA) and reverse (PTR) records.
     required: false
     type: bool
+  action:
+    description: |
+      Work on dnsconfig or member level. It can be one of `member` or
+      `dnsconfig`. Only `forwarders` can be managed with `action: member`.
+    default: "dnsconfig"
+    choices: ["member", "dnsconfig"]
   state:
-    description: State to ensure
+    description: |
+      The state to ensure. It can be one of `present` or `absent`.
+      `absent` can only be used with `action: member` and `forwarders`.
     default: present
     choices: ["present", "absent"]
 """
@@ -83,6 +91,7 @@ EXAMPLES = """
       - ip_address: 2001:4860:4860::8888
         port: 53
     state: absent
+    action: member
 
 # Disable PTR record synchronization.
 - ipadnsconfig:
@@ -118,7 +127,7 @@ def find_dnsconfig(module):
     return None
 
 
-def gen_args(module, state, dnsconfig, forwarders, forward_policy,
+def gen_args(module, state, action, dnsconfig, forwarders, forward_policy,
              allow_sync_ptr):
     _args = {}
 
@@ -137,15 +146,20 @@ def gen_args(module, state, dnsconfig, forwarders, forward_policy,
 
         global_forwarders = dnsconfig.get('idnsforwarders', [])
         if state == 'absent':
-            _args['idnsforwarders'] = [
-                fwd for fwd in global_forwarders if fwd not in _forwarders]
-            # When all forwarders should be excluded, use an empty string ('').
-            if not _args['idnsforwarders']:
-                _args['idnsforwarders'] = ['']
+            if action == "member":
+                _args['idnsforwarders'] = [
+                    fwd for fwd in global_forwarders if fwd not in _forwarders]
+                # When all forwarders should be excluded,
+                # use an empty string ('').
+                if not _args['idnsforwarders']:
+                    _args['idnsforwarders'] = ['']
 
         elif state == 'present':
-            _args['idnsforwarders'] = [
-                fwd for fwd in _forwarders if fwd not in global_forwarders]
+            if action == "member":
+                _args['idnsforwarders'] = \
+                    list(set(list(_forwarders) + list(global_forwarders)))
+            else:
+                _args['idnsforwarders'] = _forwarders
             # If no forwarders should be added, remove argument.
             if not _args['idnsforwarders']:
                 del _args['idnsforwarders']
@@ -179,6 +193,8 @@ def main():
             allow_sync_ptr=dict(type='bool', required=False, default=None),
 
             # general
+            action=dict(type="str", default="dnsconfig",
+                        choices=["member", "dnsconfig"]),
             state=dict(type="str", default="present",
                        choices=["present", "absent"]),
         )
@@ -191,11 +207,17 @@ def main():
     forward_policy = ansible_module.params_get('forward_policy')
     allow_sync_ptr = ansible_module.params_get('allow_sync_ptr')
 
+    action = ansible_module.params_get('action')
     state = ansible_module.params_get('state')
 
     # Check parameters.
     invalid = []
+    if state == "present" and action == "member":
+        invalid = ['forward_policy', 'allow_sync_ptr']
     if state == 'absent':
+        if action != "member":
+            ansible_module.fail_json(
+                msg="State 'absent' is only valid with action 'member'.")
         invalid = ['forward_policy', 'allow_sync_ptr']
 
     ansible_module.params_fail_used_invalid(invalid, state)
@@ -208,7 +230,7 @@ def main():
     with ansible_module.ipa_connect():
 
         res_find = find_dnsconfig(ansible_module)
-        args = gen_args(ansible_module, state, res_find, forwarders,
+        args = gen_args(ansible_module, state, action, res_find, forwarders,
                         forward_policy, allow_sync_ptr)
 
         # Execute command only if configuration changes.

plugins/modules/ipagroup.py

@@ -181,6 +181,7 @@ EXAMPLES = """
 RETURN = """
 """
 
+from ansible.module_utils._text import to_text
 from ansible.module_utils.ansible_freeipa_module import \
     IPAAnsibleModule, compare_args_ipa, gen_add_del_lists, \
     gen_add_list, gen_intersection_list
@@ -198,7 +199,14 @@ def find_group(module, name):
         module.fail_json(
             msg="There is more than one group '%s'" % (name))
     elif len(_result["result"]) == 1:
-        return _result["result"][0]
+        _res = _result["result"][0]
+        # The returned services are of type ipapython.kerberos.Principal,
+        # also services are not case sensitive. Therefore services are
+        # converted to lowercase strings to be able to do the comparison.
+        if "member_service" in _res:
+            _res["member_service"] = \
+                [to_text(svc).lower() for svc in _res["member_service"]]
+        return _res
 
     return None
 
@@ -308,7 +316,8 @@ def main():
     nomembers = ansible_module.params_get("nomembers")
     user = ansible_module.params_get("user")
     group = ansible_module.params_get("group")
-    service = ansible_module.params_get("service")
+    # Services are not case sensitive
+    service = ansible_module.params_get_lowercase("service")
     membermanager_user = ansible_module.params_get("membermanager_user")
     membermanager_group = ansible_module.params_get("membermanager_group")
     externalmember = ansible_module.params_get("externalmember")
@@ -376,6 +385,13 @@ def main():
             # Make sure group exists
             res_find = find_group(ansible_module, name)
 
+            user_add, user_del = [], []
+            group_add, group_del = [], []
+            service_add, service_del = [], []
+            externalmember_add, externalmember_del = [], []
+            membermanager_user_add, membermanager_user_del = [], []
+            membermanager_group_add, membermanager_group_del = [], []
+
             # Create command
             if state == "present":
                 # Can't change an existing posix group
@@ -440,45 +456,6 @@ def main():
                          externalmember_del) = gen_add_del_lists(
                             externalmember, res_find.get("member_external"))
 
-                        # setup member args for add/remove members.
-                        add_member_args = {
-                            "user": user_add,
-                            "group": group_add,
-                        }
-                        del_member_args = {
-                            "user": user_del,
-                            "group": group_del,
-                        }
-                        if has_add_member_service:
-                            add_member_args["service"] = service_add
-                            del_member_args["service"] = service_del
-
-                        if is_external_group(res_find):
-                            add_member_args["ipaexternalmember"] = \
-                                externalmember_add
-                            del_member_args["ipaexternalmember"] = \
-                                externalmember_del
-                        elif externalmember or external:
-                            ansible_module.fail_json(
-                                msg="Cannot add external members to a "
-                                    "non-external group."
-                            )
-
-                        # Add members
-                        add_members = any([user_add, group_add,
-                                           service_add, externalmember_add])
-                        if add_members:
-                            commands.append(
-                                [name, "group_add_member", add_member_args]
-                            )
-                        # Remove members
-                        remove_members = any([user_del, group_del,
-                                              service_del, externalmember_del])
-                        if remove_members:
-                            commands.append(
-                                [name, "group_remove_member", del_member_args]
-                            )
-
                     membermanager_user_add, membermanager_user_del = \
                         gen_add_del_lists(
                             membermanager_user,
@@ -491,93 +468,30 @@ def main():
                             res_find.get("membermanager_group")
                         )
 
-                    if has_add_membermanager:
-                        # Add membermanager users and groups
-                        if len(membermanager_user_add) > 0 or \
-                           len(membermanager_group_add) > 0:
-                            commands.append(
-                                [name, "group_add_member_manager",
-                                 {
-                                     "user": membermanager_user_add,
-                                     "group": membermanager_group_add,
-                                 }]
-                            )
-                        # Remove member manager
-                        if len(membermanager_user_del) > 0 or \
-                           len(membermanager_group_del) > 0:
-                            commands.append(
-                                [name, "group_remove_member_manager",
-                                 {
-                                     "user": membermanager_user_del,
-                                     "group": membermanager_group_del,
-                                 }]
-                            )
-
                 elif action == "member":
                     if res_find is None:
                         ansible_module.fail_json(msg="No group '%s'" % name)
 
-                    add_member_args = {
-                        "user": user,
-                        "group": group,
-                    }
-                    if has_add_member_service:
-                        add_member_args["service"] = service
-                    if is_external_group(res_find):
-                        add_member_args["ipaexternalmember"] = externalmember
-                    elif externalmember:
-                        ansible_module.fail_json(
-                            msg="Cannot add external members to a "
-                                "non-external group."
-                        )
-
                     # Reduce add lists for member_user, member_group,
                     # member_service and member_external to new entries
                     # only that are not in res_find.
-                    if user is not None and "member_user" in res_find:
-                        user = gen_add_list(
-                            user, res_find["member_user"])
-                    if group is not None and "member_group" in res_find:
-                        group = gen_add_list(
-                            group, res_find["member_group"])
-                    if service is not None and "member_service" in res_find:
-                        service = gen_add_list(
-                            service, res_find["member_service"])
-                    if externalmember is not None \
-                       and "member_external" in res_find:
-                        externalmember = gen_add_list(
-                            externalmember, res_find["member_external"])
+                    user_add = gen_add_list(
+                        user, res_find.get("member_user"))
+                    group_add = gen_add_list(
+                        group, res_find.get("member_group"))
+                    service_add = gen_add_list(
+                        service, res_find.get("member_service"))
+                    externalmember_add = gen_add_list(
+                        externalmember, res_find.get("member_external"))
 
-                    if any([user, group, service, externalmember]):
-                        commands.append(
-                            [name, "group_add_member", add_member_args]
-                        )
-
-                    if has_add_membermanager:
-                        # Reduce add list for membermanager_user and
-                        # membermanager_group to new entries only that are
-                        # not in res_find.
-                        if membermanager_user is not None \
-                           and "membermanager_user" in res_find:
-                            membermanager_user = gen_add_list(
-                                membermanager_user,
-                                res_find["membermanager_user"])
-                        if membermanager_group is not None \
-                           and "membermanager_group" in res_find:
-                            membermanager_group = gen_add_list(
-                                membermanager_group,
-                                res_find["membermanager_group"])
-
-                        # Add membermanager users and groups
-                        if membermanager_user is not None or \
-                           membermanager_group is not None:
-                            commands.append(
-                                [name, "group_add_member_manager",
-                                 {
-                                     "user": membermanager_user,
-                                     "group": membermanager_group,
-                                 }]
-                            )
+                    membermanager_user_add = gen_add_list(
+                        membermanager_user,
+                        res_find.get("membermanager_user")
+                    )
+                    membermanager_group_add = gen_add_list(
+                        membermanager_group,
+                        res_find.get("membermanager_group")
+                    )
 
             elif state == "absent":
                 if action == "group":
@@ -588,70 +502,91 @@ def main():
                     if res_find is None:
                         ansible_module.fail_json(msg="No group '%s'" % name)
 
-                    del_member_args = {
-                        "user": user,
-                        "group": group,
-                    }
-                    if has_add_member_service:
-                        del_member_args["service"] = service
-                    if is_external_group(res_find):
-                        del_member_args["ipaexternalmember"] = externalmember
-                    elif externalmember:
+                    if not is_external_group(res_find) and externalmember:
                         ansible_module.fail_json(
                             msg="Cannot add external members to a "
                                 "non-external group."
                         )
 
-                    # Reduce del lists of member_user, member_group,
-                    # member_service and member_external to the entries only
-                    # that are in res_find.
-                    if user is not None:
-                        user = gen_intersection_list(
-                            user, res_find.get("member_user"))
-                    if group is not None:
-                        group = gen_intersection_list(
-                            group, res_find.get("member_group"))
-                    if service is not None:
-                        service = gen_intersection_list(
-                            service, res_find.get("member_service"))
-                    if externalmember is not None:
-                        externalmember = gen_intersection_list(
-                            externalmember, res_find.get("member_external"))
-
-                    if any([user, group, service, externalmember]):
-                        commands.append(
-                            [name, "group_remove_member", del_member_args]
-                        )
-
-                    if has_add_membermanager:
-                        # Reduce del lists of membermanager_user and
-                        # membermanager_group to the entries only that are
-                        # in res_find.
-                        if membermanager_user is not None:
-                            membermanager_user = gen_intersection_list(
-                                membermanager_user,
-                                res_find.get("membermanager_user"))
-                        if membermanager_group is not None:
-                            membermanager_group = gen_intersection_list(
-                                membermanager_group,
-                                res_find.get("membermanager_group"))
-
-                        # Remove membermanager users and groups
-                        if membermanager_user is not None or \
-                           membermanager_group is not None:
-                            commands.append(
-                                [name, "group_remove_member_manager",
-                                 {
-                                     "user": membermanager_user,
-                                     "group": membermanager_group,
-                                 }]
-                            )
+                    user_del = gen_intersection_list(
+                        user, res_find.get("member_user"))
+                    group_del = gen_intersection_list(
+                        group, res_find.get("member_group"))
+                    service_del = gen_intersection_list(
+                        service, res_find.get("member_service"))
+                    externalmember_del = gen_intersection_list(
+                        externalmember, res_find.get("member_external"))
 
+                    membermanager_user_del = gen_intersection_list(
+                        membermanager_user, res_find.get("membermanager_user"))
+                    membermanager_group_del = gen_intersection_list(
+                        membermanager_group,
+                        res_find.get("membermanager_group")
+                    )
             else:
                 ansible_module.fail_json(msg="Unkown state '%s'" % state)
 
-        # Execute commands
+            # manage members
+            # setup member args for add/remove members.
+            add_member_args = {
+                "user": user_add,
+                "group": group_add,
+            }
+            del_member_args = {
+                "user": user_del,
+                "group": group_del,
+            }
+            if has_add_member_service:
+                add_member_args["service"] = service_add
+                del_member_args["service"] = service_del
 
+            if is_external_group(res_find):
+                add_member_args["ipaexternalmember"] = \
+                    externalmember_add
+                del_member_args["ipaexternalmember"] = \
+                    externalmember_del
+            elif externalmember or external:
+                ansible_module.fail_json(
+                    msg="Cannot add external members to a "
+                        "non-external group."
+                )
+            # Add members
+            add_members = any([user_add, group_add,
+                               service_add, externalmember_add])
+            if add_members:
+                commands.append(
+                    [name, "group_add_member", add_member_args]
+                )
+            # Remove members
+            remove_members = any([user_del, group_del,
+                                  service_del, externalmember_del])
+            if remove_members:
+                commands.append(
+                    [name, "group_remove_member", del_member_args]
+                )
+
+            # manage membermanager members
+            if has_add_membermanager:
+                # Add membermanager users and groups
+                if any([membermanager_user_add, membermanager_group_add]):
+                    commands.append(
+                        [name, "group_add_member_manager",
+                         {
+                             "user": membermanager_user_add,
+                             "group": membermanager_group_add,
+                         }]
+                    )
+                # Remove member manager
+                if any([membermanager_user_del, membermanager_group_del]):
+                    commands.append(
+                        [name, "group_remove_member_manager",
+                         {
+                             "user": membermanager_user_del,
+                             "group": membermanager_group_del,
+                         }]
+                    )
+
+        # Execute commands
         changed = ansible_module.execute_ipa_commands(
             commands, fail_on_member_errors=True)
 

plugins/modules/ipahbacrule.py

@@ -238,12 +238,12 @@ def main():
     hostcategory = ansible_module.params_get("hostcategory")
     servicecategory = ansible_module.params_get("servicecategory")
     nomembers = ansible_module.params_get("nomembers")
-    host = ansible_module.params_get("host")
-    hostgroup = ansible_module.params_get("hostgroup")
-    hbacsvc = ansible_module.params_get("hbacsvc")
-    hbacsvcgroup = ansible_module.params_get("hbacsvcgroup")
-    user = ansible_module.params_get("user")
-    group = ansible_module.params_get("group")
+    host = ansible_module.params_get_lowercase("host")
+    hostgroup = ansible_module.params_get_lowercase("hostgroup")
+    hbacsvc = ansible_module.params_get_lowercase("hbacsvc")
+    hbacsvcgroup = ansible_module.params_get_lowercase("hbacsvcgroup")
+    user = ansible_module.params_get_lowercase("user")
+    group = ansible_module.params_get_lowercase("group")
     action = ansible_module.params_get("action")
     # state
     state = ansible_module.params_get("state")
@@ -307,7 +307,7 @@ def main():
 
         # Ensure fqdn host names, use default domain for simple names
         if host is not None:
-            _host = [ensure_fqdn(x, default_domain) for x in host]
+            _host = [ensure_fqdn(x, default_domain).lower() for x in host]
             host = _host
 
         commands = []
@@ -316,6 +316,13 @@ def main():
             # Make sure hbacrule exists
             res_find = find_hbacrule(ansible_module, name)
 
+            host_add, host_del = [], []
+            hostgroup_add, hostgroup_del = [], []
+            hbacsvc_add, hbacsvc_del = [], []
+            hbacsvcgroup_add, hbacsvcgroup_del = [], []
+            user_add, user_del = [], []
+            group_add, group_del = [], []
+
             # Create command
             if state == "present":
                 # Generate args
@@ -353,69 +360,30 @@ def main():
                         res_find = {}
 
                     # Generate addition and removal lists
-                    host_add, host_del = gen_add_del_lists(
-                        host, res_find.get("memberhost_host"))
+                    if host:
+                        host_add, host_del = gen_add_del_lists(
+                            host, res_find.get("memberhost_host"))
 
-                    hostgroup_add, hostgroup_del = gen_add_del_lists(
-                        hostgroup, res_find.get("memberhost_hostgroup"))
+                    if hostgroup:
+                        hostgroup_add, hostgroup_del = gen_add_del_lists(
+                            hostgroup, res_find.get("memberhost_hostgroup"))
 
-                    hbacsvc_add, hbacsvc_del = gen_add_del_lists(
-                        hbacsvc, res_find.get("memberservice_hbacsvc"))
+                    if hbacsvc:
+                        hbacsvc_add, hbacsvc_del = gen_add_del_lists(
+                            hbacsvc, res_find.get("memberservice_hbacsvc"))
 
-                    hbacsvcgroup_add, hbacsvcgroup_del = gen_add_del_lists(
-                        hbacsvcgroup,
-                        res_find.get("memberservice_hbacsvcgroup"))
+                    if hbacsvcgroup:
+                        hbacsvcgroup_add, hbacsvcgroup_del = gen_add_del_lists(
+                            hbacsvcgroup,
+                            res_find.get("memberservice_hbacsvcgroup"))
 
-                    user_add, user_del = gen_add_del_lists(
-                        user, res_find.get("memberuser_user"))
+                    if user:
+                        user_add, user_del = gen_add_del_lists(
+                            user, res_find.get("memberuser_user"))
 
-                    group_add, group_del = gen_add_del_lists(
-                        group, res_find.get("memberuser_group"))
-
-                    # Add hosts and hostgroups
-                    if len(host_add) > 0 or len(hostgroup_add) > 0:
-                        commands.append([name, "hbacrule_add_host",
-                                         {
-                                             "host": host_add,
-                                             "hostgroup": hostgroup_add,
-                                         }])
-                    # Remove hosts and hostgroups
-                    if len(host_del) > 0 or len(hostgroup_del) > 0:
-                        commands.append([name, "hbacrule_remove_host",
-                                         {
-                                             "host": host_del,
-                                             "hostgroup": hostgroup_del,
-                                         }])
-
-                    # Add hbacsvcs and hbacsvcgroups
-                    if len(hbacsvc_add) > 0 or len(hbacsvcgroup_add) > 0:
-                        commands.append([name, "hbacrule_add_service",
-                                         {
-                                             "hbacsvc": hbacsvc_add,
-                                             "hbacsvcgroup": hbacsvcgroup_add,
-                                         }])
-                    # Remove hbacsvcs and hbacsvcgroups
-                    if len(hbacsvc_del) > 0 or len(hbacsvcgroup_del) > 0:
-                        commands.append([name, "hbacrule_remove_service",
-                                         {
-                                             "hbacsvc": hbacsvc_del,
-                                             "hbacsvcgroup": hbacsvcgroup_del,
-                                         }])
-
-                    # Add users and groups
-                    if len(user_add) > 0 or len(group_add) > 0:
-                        commands.append([name, "hbacrule_add_user",
-                                         {
-                                             "user": user_add,
-                                             "group": group_add,
-                                         }])
-                    # Remove users and groups
-                    if len(user_del) > 0 or len(group_del) > 0:
-                        commands.append([name, "hbacrule_remove_user",
-                                         {
-                                             "user": user_del,
-                                             "group": group_del,
-                                         }])
+                    if group:
+                        group_add, group_del = gen_add_del_lists(
+                            group, res_find.get("memberuser_group"))
 
                 elif action == "member":
                     if res_find is None:
@@ -424,63 +392,33 @@ def main():
                     # Generate add lists for host, hostgroup and
                     # res_find to only try to add hosts and hostgroups
                     # that not in hbacrule already
-                    if host is not None and \
-                       "memberhost_host" in res_find:
-                        host = gen_add_list(
-                            host, res_find["memberhost_host"])
-                    if hostgroup is not None and \
-                       "memberhost_hostgroup" in res_find:
-                        hostgroup = gen_add_list(
-                            hostgroup, res_find["memberhost_hostgroup"])
-
-                    # Add hosts and hostgroups
-                    if host is not None or hostgroup is not None:
-                        commands.append([name, "hbacrule_add_host",
-                                         {
-                                             "host": host,
-                                             "hostgroup": hostgroup,
-                                         }])
+                    if host:
+                        host_add = gen_add_list(
+                            host, res_find.get("memberhost_host"))
+                    if hostgroup is not None:
+                        hostgroup_add = gen_add_list(
+                            hostgroup, res_find.get("memberhost_hostgroup"))
 
                     # Generate add lists for hbacsvc, hbacsvcgroup and
                     # res_find to only try to add hbacsvcs and hbacsvcgroups
                     # that not in hbacrule already
-                    if hbacsvc is not None and \
-                       "memberservice_hbacsvc" in res_find:
-                        hbacsvc = gen_add_list(
-                            hbacsvc, res_find["memberservice_hbacsvc"])
-                    if hbacsvcgroup is not None and \
-                       "memberservice_hbacsvcgroup" in res_find:
-                        hbacsvcgroup = gen_add_list(
+                    if hbacsvc:
+                        hbacsvc_add = gen_add_list(
+                            hbacsvc, res_find.get("memberservice_hbacsvc"))
+                    if hbacsvcgroup:
+                        hbacsvcgroup_add = gen_add_list(
                             hbacsvcgroup,
-                            res_find["memberservice_hbacsvcgroup"])
-
-                    # Add hbacsvcs and hbacsvcgroups
-                    if hbacsvc is not None or hbacsvcgroup is not None:
-                        commands.append([name, "hbacrule_add_service",
-                                         {
-                                             "hbacsvc": hbacsvc,
-                                             "hbacsvcgroup": hbacsvcgroup,
-                                         }])
+                            res_find.get("memberservice_hbacsvcgroup"))
 
                     # Generate add lists for user, group and
                     # res_find to only try to add users and groups
                     # that not in hbacrule already
-                    if user is not None and \
-                       "memberuser_user" in res_find:
-                        user = gen_add_list(
-                            user, res_find["memberuser_user"])
-                    if group is not None and \
-                       "memberuser_group" in res_find:
-                        group = gen_add_list(
-                            group, res_find["memberuser_group"])
-
-                    # Add users and groups
-                    if user is not None or group is not None:
-                        commands.append([name, "hbacrule_add_user",
-                                         {
-                                             "user": user,
-                                             "group": group,
-                                         }])
+                    if user:
+                        user_add = gen_add_list(
+                            user, res_find.get("memberuser_user"))
+                    if group:
+                        group_add = gen_add_list(
+                            group, res_find.get("memberuser_group"))
 
             elif state == "absent":
                 if action == "hbacrule":
@@ -494,75 +432,39 @@ def main():
                     # Generate intersection lists for host, hostgroup and
                     # res_find to only try to remove hosts and hostgroups
                     # that are in hbacrule
-                    if host is not None:
+                    if host:
                         if "memberhost_host" in res_find:
-                            host = gen_intersection_list(
+                            host_del = gen_intersection_list(
                                 host, res_find["memberhost_host"])
-                        else:
-                            host = None
-                    if hostgroup is not None:
+                    if hostgroup:
                         if "memberhost_hostgroup" in res_find:
-                            hostgroup = gen_intersection_list(
+                            hostgroup_del = gen_intersection_list(
                                 hostgroup, res_find["memberhost_hostgroup"])
-                        else:
-                            hostgroup = None
-
-                    # Remove hosts and hostgroups
-                    if host is not None or hostgroup is not None:
-                        commands.append([name, "hbacrule_remove_host",
-                                         {
-                                             "host": host,
-                                             "hostgroup": hostgroup,
-                                         }])
 
                     # Generate intersection lists for hbacsvc, hbacsvcgroup
                     # and res_find to only try to remove hbacsvcs and
                     # hbacsvcgroups that are in hbacrule
-                    if hbacsvc is not None:
+                    if hbacsvc:
                         if "memberservice_hbacsvc" in res_find:
-                            hbacsvc = gen_intersection_list(
+                            hbacsvc_del = gen_intersection_list(
                                 hbacsvc, res_find["memberservice_hbacsvc"])
-                        else:
-                            hbacsvc = None
-                    if hbacsvcgroup is not None:
+                    if hbacsvcgroup:
                         if "memberservice_hbacsvcgroup" in res_find:
-                            hbacsvcgroup = gen_intersection_list(
+                            hbacsvcgroup_del = gen_intersection_list(
                                 hbacsvcgroup,
                                 res_find["memberservice_hbacsvcgroup"])
-                        else:
-                            hbacsvcgroup = None
-
-                    # Remove hbacsvcs and hbacsvcgroups
-                    if hbacsvc is not None or hbacsvcgroup is not None:
-                        commands.append([name, "hbacrule_remove_service",
-                                         {
-                                             "hbacsvc": hbacsvc,
-                                             "hbacsvcgroup": hbacsvcgroup,
-                                         }])
 
                     # Generate intersection lists for user, group and
                     # res_find to only try to remove users and groups
                     # that are in hbacrule
-                    if user is not None:
+                    if user:
                         if "memberuser_user" in res_find:
-                            user = gen_intersection_list(
+                            user_del = gen_intersection_list(
                                 user, res_find["memberuser_user"])
-                        else:
-                            user = None
-                    if group is not None:
+                    if group:
                         if "memberuser_group" in res_find:
-                            group = gen_intersection_list(
+                            group_del = gen_intersection_list(
                                 group, res_find["memberuser_group"])
-                        else:
-                            group = None
-
-                    # Remove users and groups
-                    if user is not None or group is not None:
-                        commands.append([name, "hbacrule_remove_user",
-                                         {
-                                             "user": user,
-                                             "group": group,
-                                         }])
 
             elif state == "enabled":
                 if res_find is None:
@@ -587,6 +489,53 @@ def main():
             else:
                 ansible_module.fail_json(msg="Unkown state '%s'" % state)
 
+            # Manage HBAC rule members.
+
+            # Add hosts and hostgroups
+            if len(host_add) > 0 or len(hostgroup_add) > 0:
+                commands.append([name, "hbacrule_add_host",
+                                 {
+                                     "host": host_add,
+                                     "hostgroup": hostgroup_add,
+                                 }])
+            # Remove hosts and hostgroups
+            if len(host_del) > 0 or len(hostgroup_del) > 0:
+                commands.append([name, "hbacrule_remove_host",
+                                 {
+                                     "host": host_del,
+                                     "hostgroup": hostgroup_del,
+                                 }])
+
+            # Add hbacsvcs and hbacsvcgroups
+            if len(hbacsvc_add) > 0 or len(hbacsvcgroup_add) > 0:
+                commands.append([name, "hbacrule_add_service",
+                                 {
+                                     "hbacsvc": hbacsvc_add,
+                                     "hbacsvcgroup": hbacsvcgroup_add,
+                                 }])
+            # Remove hbacsvcs and hbacsvcgroups
+            if len(hbacsvc_del) > 0 or len(hbacsvcgroup_del) > 0:
+                commands.append([name, "hbacrule_remove_service",
+                                 {
+                                     "hbacsvc": hbacsvc_del,
+                                     "hbacsvcgroup": hbacsvcgroup_del,
+                                 }])
+
+            # Add users and groups
+            if len(user_add) > 0 or len(group_add) > 0:
+                commands.append([name, "hbacrule_add_user",
+                                 {
+                                     "user": user_add,
+                                     "group": group_add,
+                                 }])
+            # Remove users and groups
+            if len(user_del) > 0 or len(group_del) > 0:
+                commands.append([name, "hbacrule_remove_user",
+                                 {
+                                     "user": user_del,
+                                     "group": group_del,
+                                 }])
+
         # Execute commands
 
         changed = ansible_module.execute_ipa_commands(

plugins/modules/ipahbacsvcgroup.py

@@ -101,7 +101,8 @@ RETURN = """
 """
 
 from ansible.module_utils.ansible_freeipa_module import \
-    IPAAnsibleModule, compare_args_ipa, gen_add_del_lists
+    IPAAnsibleModule, compare_args_ipa, gen_add_del_lists, gen_add_list, \
+    gen_intersection_list
 
 
 def find_hbacsvcgroup(module, name):
@@ -183,7 +184,7 @@ def main():
     # present
     description = ansible_module.params_get("description")
     nomembers = ansible_module.params_get("nomembers")
-    hbacsvc = ansible_module.params_get("hbacsvc")
+    hbacsvc = ansible_module.params_get_lowercase("hbacsvc")
     action = ansible_module.params_get("action")
     # state
     state = ansible_module.params_get("state")
@@ -223,6 +224,8 @@ def main():
             # Make sure hbacsvcgroup exists
             res_find = find_hbacsvcgroup(ansible_module, name)
 
+            hbacsvc_add, hbacsvc_del = [], []
+
             # Create command
             if state == "present":
                 # Generate args
@@ -246,32 +249,20 @@ def main():
                     if not compare_args_ipa(ansible_module, member_args,
                                             res_find):
                         # Generate addition and removal lists
-                        hbacsvc_add, hbacsvc_del = gen_add_del_lists(
-                            hbacsvc, res_find.get("member_hbacsvc"))
+                        if hbacsvc is not None:
+                            hbacsvc_add, hbacsvc_del = gen_add_del_lists(
+                                hbacsvc, res_find.get("member_hbacsvc"))
 
-                        # Add members
-                        if len(hbacsvc_add) > 0:
-                            commands.append([name, "hbacsvcgroup_add_member",
-                                             {
-                                                 "hbacsvc": hbacsvc_add
-                                             }])
-                        # Remove members
-                        if len(hbacsvc_del) > 0:
-                            commands.append([name,
-                                             "hbacsvcgroup_remove_member",
-                                             {
-                                                 "hbacsvc": hbacsvc_del
-                                             }])
                 elif action == "member":
                     if res_find is None:
                         ansible_module.fail_json(
                             msg="No hbacsvcgroup '%s'" % name)
 
                     # Ensure members are present
-                    commands.append([name, "hbacsvcgroup_add_member",
-                                     {
-                                         "hbacsvc": hbacsvc
-                                     }])
+                    if hbacsvc:
+                        hbacsvc_add = gen_add_list(
+                            hbacsvc, res_find.get("member_hbacsvc"))
+
             elif state == "absent":
                 if action == "hbacsvcgroup":
                     if res_find is not None:
@@ -283,15 +274,28 @@ def main():
                             msg="No hbacsvcgroup '%s'" % name)
 
                     # Ensure members are absent
-                    commands.append([name, "hbacsvcgroup_remove_member",
-                                     {
-                                         "hbacsvc": hbacsvc
-                                     }])
+                    if hbacsvc:
+                        hbacsvc_del = gen_intersection_list(
+                            hbacsvc, res_find.get("member_hbacsvc"))
+
             else:
                 ansible_module.fail_json(msg="Unkown state '%s'" % state)
 
-        # Execute commands
+            # Manage members
+            if len(hbacsvc_add) > 0:
+                commands.append([name, "hbacsvcgroup_add_member",
+                                 {
+                                     "hbacsvc": hbacsvc_add
+                                 }])
+            # Remove members
+            if len(hbacsvc_del) > 0:
+                commands.append([name,
+                                 "hbacsvcgroup_remove_member",
+                                 {
+                                     "hbacsvc": hbacsvc_del
+                                 }])
 
+        # Execute commands
         changed = ansible_module.execute_ipa_commands(commands, result_handler)
 
     # Done

plugins/modules/ipahostgroup.py

@@ -139,7 +139,7 @@ RETURN = """
 
 from ansible.module_utils.ansible_freeipa_module import \
     IPAAnsibleModule, compare_args_ipa, gen_add_del_lists, gen_add_list, \
-    gen_intersection_list
+    gen_intersection_list, ensure_fqdn
 
 
 def find_hostgroup(module, name):
@@ -281,6 +281,15 @@ def main():
             ansible_module.fail_json(
                 msg="Renaming hostgroups is not supported by your IPA version")
 
+        # If hosts are given, ensure that the hosts are FQDN and also
+        # lowercase to be able to do a proper comparison to exising hosts
+        # in the hostgroup.
+        # Fixes #666 (ipahostgroup not idempotent and with error)
+        if host is not None:
+            default_domain = ansible_module.ipa_get_domain()
+            host = [ensure_fqdn(_host, default_domain).lower()
+                    for _host in host]
+
         commands = []
 
         for name in names:

plugins/modules/iparole.py

@@ -72,7 +72,7 @@ options:
     required: false
   state:
     description: The state to ensure.
-    choices: ["present", "absent"]
+    choices: ["present", "absent", "renamed"]
     default: present
     required: true
 """
@@ -103,10 +103,10 @@ EXAMPLES = """
 # pylint: disable=no-name-in-module
 from ansible.module_utils._text import to_text
 from ansible.module_utils.ansible_freeipa_module import \
-    IPAAnsibleModule, gen_add_del_lists, compare_args_ipa
+    IPAAnsibleModule, gen_add_del_lists, compare_args_ipa, \
+    gen_intersection_list, ensure_fqdn
 from ansible.module_utils import six
 
-
 if six.PY3:
     unicode = str
 
@@ -145,9 +145,22 @@ def check_parameters(module):
 
     invalid = []
 
-    if state == "present":
+    if state == "renamed":
         if action == "member":
-            invalid.extend(['description', 'rename'])
+            module.fail_json(
+                msg="Invalid action 'member' with state 'renamed'.")
+        invalid = [
+            "description",
+            "user", "group",
+            "host", "hostgroup",
+            "service",
+            "privilege",
+        ]
+
+    if state == "present":
+        invalid = ["rename"]
+        if action == "member":
+            invalid.extend(['description'])
 
     if state == "absent":
         invalid.extend(['description', 'rename'])
@@ -157,28 +170,15 @@ def check_parameters(module):
     module.params_fail_used_invalid(invalid, state, action)
 
 
-def member_intersect(module, attr, memberof, res_find):
-    """Filter member arguments from role found by intersection."""
-    params = module.params_get(attr)
-    if not res_find:
-        return params
-    filtered = []
-    if params:
-        existing = res_find.get(memberof, [])
-        filtered = list(set(params) & set(existing))
-    return filtered
-
-
-def member_difference(module, attr, memberof, res_find):
-    """Filter member arguments from role found by difference."""
-    params = module.params_get(attr)
-    if not res_find:
-        return params
-    filtered = []
-    if params:
-        existing = res_find.get(memberof, [])
-        filtered = list(set(params) - set(existing))
-    return filtered
+def get_member_host_with_fqdn_lowercase(module, mod_member):
+    """Retrieve host members from module, as FQDN, lowercase."""
+    default_domain = module.ipa_get_domain()
+    hosts = module.params_get(mod_member)
+    return (
+        [ensure_fqdn(host, default_domain).lower() for host in hosts]
+        if hosts
+        else hosts
+    )
 
 
 def ensure_absent_state(module, name, action, res_find):
@@ -190,23 +190,41 @@ def ensure_absent_state(module, name, action, res_find):
 
     if action == "member":
 
-        members = member_intersect(
-            module, 'privilege', 'memberof_privilege', res_find)
-        if members:
-            commands.append([name, "role_remove_privilege",
-                             {"privilege": members}])
+        _members = module.params_get_lowercase("privilege")
+        if _members is not None:
+            del_list = gen_intersection_list(
+                _members,
+                result_get_value_lowercase(res_find, "memberof_privilege")
+            )
+            if del_list:
+                commands.append([name, "role_remove_privilege",
+                                 {"privilege": del_list}])
 
         member_args = {}
-        for key in ['user', 'group', 'host', 'hostgroup']:
-            items = member_intersect(
-                module, key, 'member_%s' % key, res_find)
-            if items:
-                member_args[key] = items
+        for key in ['user', 'group', 'hostgroup']:
+            _members = module.params_get_lowercase(key)
+            if _members:
+                del_list = gen_intersection_list(
+                    _members,
+                    result_get_value_lowercase(res_find, "member_%s" % key)
+                )
+                if del_list:
+                    member_args[key] = del_list
 
-        _services = filter_service(module, res_find,
-                                   lambda res, svc: res.startswith(svc))
+        # ensure hosts are FQDN.
+        _members = get_member_host_with_fqdn_lowercase(module, "host")
+        if _members:
+            del_list = gen_intersection_list(
+                _members, res_find.get('member_host'))
+            if del_list:
+                member_args["host"] = del_list
+
+        _services = get_service_param(module, "service")
         if _services:
-            member_args['service'] = _services
+            _existing = result_get_value_lowercase(res_find, "member_service")
+            items = gen_intersection_list(_services.keys(), _existing)
+            if items:
+                member_args["service"] = [_services[key] for key in items]
 
         # Only add remove command if there's at least one member no manage.
         if member_args:
@@ -215,66 +233,112 @@ def ensure_absent_state(module, name, action, res_find):
     return commands
 
 
-def filter_service(module, res_find, predicate):
+def get_service_param(module, key):
     """
-    Filter service based on predicate.
+    Retrieve dict of services, with realm, from the module parameters.
 
-    Compare service name with existing ones matching
-    at least until `@` from principal name.
-
-    Predicate is a callable that accepts the existing service, and the
-    modified service to be compared to.
+    As the services are compared in a case insensitive manner, but
+    are recorded in a case preserving way, a dict mapping the services
+    in lowercase to the provided module parameter is generated, so
+    that dict keys can be used for comparison and the values are used
+    with IPA API.
     """
-    _services = []
-    service = module.params_get('service')
-    if service:
-        existing = [to_text(x) for x in res_find.get('member_service', [])]
-        for svc in service:
-            svc = svc if '@' in svc else ('%s@' % svc)
-            found = [x for x in existing if predicate(x, svc)]
-            _services.extend(found)
+    _services = module.params_get(key)
+    if _services is not None:
+        ipa_realm = module.ipa_get_realm()
+        _services = [
+            to_text(svc) if '@' in svc else ('%s@%s' % (svc, ipa_realm))
+            for svc in _services
+        ]
+        if _services:
+            _services = {svc.lower(): svc for svc in _services}
     return _services
 
 
+def result_get_value_lowercase(res_find, key, default=None):
+    """
+    Retrieve a member of a dictionary converted to lowercase.
+
+    If field data is a string it is returned in lowercase. If
+    field data is a list or tuple, it is assumed that all values
+    are strings and the result is a list of strings in lowercase.
+
+    If 'key' is not found in the dictionary, returns 'default'.
+    """
+    existing = res_find.get(key)
+    if existing is not None:
+        if isinstance(existing, (list, tuple)):
+            existing = [to_text(item).lower() for item in existing]
+        if isinstance(existing, (str, unicode)):
+            existing = existing.lower()
+    else:
+        existing = default
+    return existing
+
+
+def gen_services_add_del_lists(module, mod_member, res_find, res_member):
+    """Generate add/del lists for service principals."""
+    add_list, del_list = None, None
+    _services = get_service_param(module, mod_member)
+    if _services is not None:
+        _existing = result_get_value_lowercase(res_find, res_member)
+        add_list, del_list = gen_add_del_lists(_services.keys(), _existing)
+        if add_list:
+            add_list = [_services[key] for key in add_list]
+        if del_list:
+            del_list = [to_text(item) for item in del_list]
+    return add_list, del_list
+
+
 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(
-        module.params_get("privilege"),
-        res_find.get('memberof_privilege', []))
 
-    if privilege_add:
-        commands.append([name, "role_add_privilege",
-                         {"privilege": privilege_add}])
-    if action == "role" and privilege_del:
-        commands.append([name, "role_remove_privilege",
-                         {"privilege": privilege_del}])
+    _members = module.params_get_lowercase("privilege")
+    if _members:
+        add_list, del_list = gen_add_del_lists(
+            _members,
+            result_get_value_lowercase(res_find, "memberof_privilege")
+        )
+
+        if add_list:
+            commands.append([name, "role_add_privilege",
+                             {"privilege": add_list}])
+        if action == "role" and del_list:
+            commands.append([name, "role_remove_privilege",
+                             {"privilege": del_list}])
 
     add_members = {}
     del_members = {}
 
-    for key in ["user", "group", "host", "hostgroup"]:
-        add_list, del_list = gen_add_del_lists(
-            module.params_get(key),
-            res_find.get('member_%s' % key, [])
-        )
-        if add_list:
-            add_members[key] = add_list
-        if del_list:
-            del_members[key] = [to_text(item) for item in del_list]
+    for key in ["user", "group", "hostgroup"]:
+        _members = module.params_get_lowercase(key)
+        if _members is not None:
+            add_list, del_list = gen_add_del_lists(
+                _members,
+                result_get_value_lowercase(res_find, "member_%s" % key)
+            )
+            if add_list:
+                add_members[key] = add_list
+            if del_list:
+                del_members[key] = del_list
 
-    service = [
-        to_text(svc)
-        if '@' in svc
-        else ('%s@%s' % (svc, module.ipa_get_realm()))
-        for svc in (module.params_get('service') or [])
-    ]
-    existing = [str(svc) for svc in res_find.get('member_service', [])]
-    add_list, del_list = gen_add_del_lists(service, existing)
-    if add_list:
-        add_members['service'] = add_list
-    if del_list:
-        del_members['service'] = [to_text(item) for item in del_list]
+    # ensure hosts are FQDN.
+    _members = get_member_host_with_fqdn_lowercase(module, "host")
+    if _members:
+        add_list, del_list = gen_add_del_lists(
+            _members, res_find.get('member_host'))
+        if add_list:
+            add_members["host"] = add_list
+        if del_list:
+            del_members["host"] = del_list
+
+    (add_services, del_services) = gen_services_add_del_lists(
+        module, "service", res_find, "member_service")
+    if add_services:
+        add_members["service"] = add_services
+    if del_services:
+        del_members["service"] = del_services
 
     if add_members:
         commands.append([name, "role_add_member", add_members])
@@ -285,67 +349,24 @@ def ensure_role_with_members_is_present(module, name, res_find, action):
     return commands
 
 
-def ensure_members_are_present(module, name, res_find):
-    """Define commands to ensure members are present for action `member`."""
-    commands = []
-
-    members = member_difference(
-        module, 'privilege', 'memberof_privilege', res_find)
-    if members:
-        commands.append([name, "role_add_privilege",
-                         {"privilege": members}])
-
-    member_args = {}
-    for key in ['user', 'group', 'host', 'hostgroup']:
-        items = member_difference(
-            module, key, 'member_%s' % key, res_find)
-        if items:
-            member_args[key] = items
-
-    _services = filter_service(module, res_find,
-                               lambda res, svc: not res.startswith(svc))
-    if _services:
-        member_args['service'] = _services
-
-    if member_args:
-        commands.append([name, "role_add_member", member_args])
-
-    return commands
-
-
-# pylint: disable=unused-argument
-def result_handler(module, result, command, name, args, errors):
-    """Process the result of a command, looking for errors."""
-    # Get all errors
-    # All "already a member" and "not a member" failures in the
-    # result are ignored. All others are reported.
-    if "failed" in result and len(result["failed"]) > 0:
-        for item in result["failed"]:
-            failed_item = result["failed"][item]
-            for member_type in failed_item:
-                for member, failure in failed_item[member_type]:
-                    if "already a member" in failure \
-                       or "not a member" in failure:
-                        continue
-                    errors.append("%s: %s %s: %s" % (
-                        command, member_type, member, failure))
-
-
 def role_commands_for_name(module, state, action, name):
     """Define commands for the Role module."""
     commands = []
 
-    rename = module.params_get("rename")
-
     res_find = find_role(module, name)
 
+    if state == "renamed":
+        args = gen_args(module)
+        if res_find is None:
+            module.fail_json(msg="No role '%s'" % name)
+        else:
+            commands.append([name, 'role_mod', args])
+
     if state == "present":
         args = gen_args(module)
 
         if action == "role":
             if res_find is None:
-                if rename is not None:
-                    module.fail_json(msg="Cannot `rename` inexistent role.")
                 commands.append([name, 'role_add', args])
                 res_find = {}
             else:
@@ -391,7 +412,7 @@ def create_module():
             action=dict(type="str", default="role",
                         choices=["role", "member"]),
             state=dict(type="str", default="present",
-                       choices=["present", "absent"]),
+                       choices=["present", "absent", "renamed"]),
         ),
         supports_check_mode=True,
         mutually_exclusive=[],
@@ -426,7 +447,8 @@ def main():
 
         # Execute commands
 
-        changed = ansible_module.execute_ipa_commands(commands, result_handler)
+        changed = ansible_module.execute_ipa_commands(
+            commands, fail_on_member_errors=True)
 
     # Done
     ansible_module.exit_json(changed=changed, **exit_args)

plugins/modules/ipasudorule.py

@@ -188,7 +188,7 @@ RETURN = """
 
 from ansible.module_utils.ansible_freeipa_module import \
     IPAAnsibleModule, compare_args_ipa, gen_add_del_lists, gen_add_list, \
-    gen_intersection_list
+    gen_intersection_list, api_get_domain, ensure_fqdn
 
 
 def find_sudorule(module, name):
@@ -297,17 +297,19 @@ def main():
     hostcategory = ansible_module.params_get("hostcategory")  # noqa
     nomembers = ansible_module.params_get("nomembers")  # noqa
     host = ansible_module.params_get("host")
-    hostgroup = ansible_module.params_get("hostgroup")
-    user = ansible_module.params_get("user")
-    group = ansible_module.params_get("group")
+    hostgroup = ansible_module.params_get_lowercase("hostgroup")
+    user = ansible_module.params_get_lowercase("user")
+    group = ansible_module.params_get_lowercase("group")
     allow_sudocmd = ansible_module.params_get('allow_sudocmd')
-    allow_sudocmdgroup = ansible_module.params_get('allow_sudocmdgroup')
+    allow_sudocmdgroup = \
+        ansible_module.params_get_lowercase('allow_sudocmdgroup')
     deny_sudocmd = ansible_module.params_get('deny_sudocmd')
-    deny_sudocmdgroup = ansible_module.params_get('deny_sudocmdgroup')
+    deny_sudocmdgroup = \
+        ansible_module.params_get_lowercase('deny_sudocmdgroup')
     sudooption = ansible_module.params_get("sudooption")
     order = ansible_module.params_get("order")
-    runasuser = ansible_module.params_get("runasuser")
-    runasgroup = ansible_module.params_get("runasgroup")
+    runasuser = ansible_module.params_get_lowercase("runasuser")
+    runasgroup = ansible_module.params_get_lowercase("runasgroup")
     action = ansible_module.params_get("action")
 
     # state
@@ -374,8 +376,26 @@ def main():
 
     # Connect to IPA API
     with ansible_module.ipa_connect():
+        default_domain = api_get_domain()
+
+        # Ensure host is not short hostname.
+        if host:
+            host = list(
+                {ensure_fqdn(value.lower(), default_domain) for value in host}
+            )
 
         commands = []
+        host_add, host_del = [], []
+        user_add, user_del = [], []
+        group_add, group_del = [], []
+        hostgroup_add, hostgroup_del = [], []
+        allow_cmd_add, allow_cmd_del = [], []
+        allow_cmdgroup_add, allow_cmdgroup_del = [], []
+        deny_cmd_add, deny_cmd_del = [], []
+        deny_cmdgroup_add, deny_cmdgroup_del = [], []
+        sudooption_add, sudooption_del = [], []
+        runasuser_add, runasuser_del = [], []
+        runasgroup_add, runasgroup_del = [], []
 
         for name in names:
             # Make sure sudorule exists
@@ -480,95 +500,11 @@ def main():
                     runasgroup_add, runasgroup_del = gen_add_del_lists(
                         runasgroup,
                         (
-                            res_find.get('ipasudorunas_group', [])
+                            res_find.get('ipasudorunasgroup_group', [])
                             + res_find.get('ipasudorunasextgroup', [])
                         )
                     )
 
-                    # Add hosts and hostgroups
-                    if len(host_add) > 0 or len(hostgroup_add) > 0:
-                        commands.append([name, "sudorule_add_host",
-                                         {
-                                             "host": host_add,
-                                             "hostgroup": hostgroup_add,
-                                         }])
-                    # Remove hosts and hostgroups
-                    if len(host_del) > 0 or len(hostgroup_del) > 0:
-                        commands.append([name, "sudorule_remove_host",
-                                         {
-                                             "host": host_del,
-                                             "hostgroup": hostgroup_del,
-                                         }])
-
-                    # Add users and groups
-                    if len(user_add) > 0 or len(group_add) > 0:
-                        commands.append([name, "sudorule_add_user",
-                                         {
-                                             "user": user_add,
-                                             "group": group_add,
-                                         }])
-                    # Remove users and groups
-                    if len(user_del) > 0 or len(group_del) > 0:
-                        commands.append([name, "sudorule_remove_user",
-                                         {
-                                             "user": user_del,
-                                             "group": group_del,
-                                         }])
-
-                    # Add commands allowed
-                    if len(allow_cmd_add) > 0 or len(allow_cmdgroup_add) > 0:
-                        commands.append([name, "sudorule_add_allow_command",
-                                         {"sudocmd": allow_cmd_add,
-                                          "sudocmdgroup": allow_cmdgroup_add,
-                                          }])
-
-                    if len(allow_cmd_del) > 0 or len(allow_cmdgroup_del) > 0:
-                        commands.append([name, "sudorule_remove_allow_command",
-                                         {"sudocmd": allow_cmd_del,
-                                          "sudocmdgroup": allow_cmdgroup_del
-                                          }])
-
-                    # Add commands denied
-                    if len(deny_cmd_add) > 0 or len(deny_cmdgroup_add) > 0:
-                        commands.append([name, "sudorule_add_deny_command",
-                                         {"sudocmd": deny_cmd_add,
-                                          "sudocmdgroup": deny_cmdgroup_add,
-                                          }])
-
-                    if len(deny_cmd_del) > 0 or len(deny_cmdgroup_del) > 0:
-                        commands.append([name, "sudorule_remove_deny_command",
-                                         {"sudocmd": deny_cmd_del,
-                                          "sudocmdgroup": deny_cmdgroup_del
-                                          }])
-
-                    # Add RunAS Users
-                    if len(runasuser_add) > 0:
-                        commands.append([name, "sudorule_add_runasuser",
-                                         {"user": runasuser_add}])
-                    # Remove RunAS Users
-                    if len(runasuser_del) > 0:
-                        commands.append([name, "sudorule_remove_runasuser",
-                                         {"user": runasuser_del}])
-
-                    # Add RunAS Groups
-                    if len(runasgroup_add) > 0:
-                        commands.append([name, "sudorule_add_runasgroup",
-                                         {"group": runasgroup_add}])
-                    # Remove RunAS Groups
-                    if len(runasgroup_del) > 0:
-                        commands.append([name, "sudorule_remove_runasgroup",
-                                         {"group": runasgroup_del}])
-
-                    # Add sudo options
-                    for sudoopt in sudooption_add:
-                        commands.append([name, "sudorule_add_option",
-                                         {"ipasudoopt": sudoopt}])
-
-                    # Remove sudo options
-                    for sudoopt in sudooption_del:
-                        commands.append([name, "sudorule_remove_option",
-                                         {"ipasudoopt": sudoopt}])
-
                 elif action == "member":
                     if res_find is None:
                         ansible_module.fail_json(msg="No sudorule '%s'" % name)
@@ -578,56 +514,47 @@ def main():
                     # deny_sudocmdgroup, sudooption, runasuser, runasgroup
                     # and res_find to only try to add the items that not in
                     # the sudorule already
-                    if host is not None and \
-                       "memberhost_host" in res_find:
-                        host = gen_add_list(
-                            host, res_find["memberhost_host"])
-                    if hostgroup is not None and \
-                       "memberhost_hostgroup" in res_find:
-                        hostgroup = gen_add_list(
-                            hostgroup, res_find["memberhost_hostgroup"])
-                    if user is not None and \
-                       "memberuser_user" in res_find:
-                        user = gen_add_list(
-                            user, res_find["memberuser_user"])
-                    if group is not None and \
-                       "memberuser_group" in res_find:
-                        group = gen_add_list(
-                            group, res_find["memberuser_group"])
-                    if allow_sudocmd is not None and \
-                       "memberallowcmd_sudocmd" in res_find:
-                        allow_sudocmd = gen_add_list(
-                            allow_sudocmd, res_find["memberallowcmd_sudocmd"])
-                    if allow_sudocmdgroup is not None and \
-                       "memberallowcmd_sudocmdgroup" in res_find:
-                        allow_sudocmdgroup = gen_add_list(
+                    if host is not None:
+                        host_add = gen_add_list(
+                            host, res_find.get("memberhost_host"))
+                    if hostgroup is not None:
+                        hostgroup_add = gen_add_list(
+                            hostgroup, res_find.get("memberhost_hostgroup"))
+                    if user is not None:
+                        user_add = gen_add_list(
+                            user, res_find.get("memberuser_user"))
+                    if group is not None:
+                        group_add = gen_add_list(
+                            group, res_find.get("memberuser_group"))
+                    if allow_sudocmd is not None:
+                        allow_cmd_add = gen_add_list(
+                            allow_sudocmd,
+                            res_find.get("memberallowcmd_sudocmd")
+                        )
+                    if allow_sudocmdgroup is not None:
+                        allow_cmdgroup_add = gen_add_list(
                             allow_sudocmdgroup,
-                            res_find["memberallowcmd_sudocmdgroup"])
-                    if deny_sudocmd is not None and \
-                       "memberdenycmd_sudocmd" in res_find:
-                        deny_sudocmd = gen_add_list(
-                            deny_sudocmd, res_find["memberdenycmd_sudocmd"])
-                    if deny_sudocmdgroup is not None and \
-                       "memberdenycmd_sudocmdgroup" in res_find:
-                        deny_sudocmdgroup = gen_add_list(
+                            res_find.get("memberallowcmd_sudocmdgroup")
+                        )
+                    if deny_sudocmd is not None:
+                        deny_cmd_add = gen_add_list(
+                            deny_sudocmd,
+                            res_find.get("memberdenycmd_sudocmd")
+                        )
+                    if deny_sudocmdgroup is not None:
+                        deny_cmdgroup_add = gen_add_list(
                             deny_sudocmdgroup,
-                            res_find["memberdenycmd_sudocmdgroup"])
-                    if sudooption is not None and \
-                       "ipasudoopt" in res_find:
-                        sudooption = gen_add_list(
-                            sudooption, res_find["ipasudoopt"])
+                            res_find.get("memberdenycmd_sudocmdgroup")
+                        )
+                    if sudooption is not None:
+                        sudooption_add = gen_add_list(
+                            sudooption, res_find.get("ipasudoopt"))
                     # runasuser attribute can be used with both IPA and
                     # non-IPA (external) users, so we need to compare
                     # the provided list against both users and external
                     # users list.
-                    if (
-                        runasuser is not None
-                        and (
-                            "ipasudorunas_user" in res_find
-                            or "ipasudorunasextuser" in res_find
-                        )
-                    ):
-                        runasuser = gen_add_list(
+                    if runasuser is not None:
+                        runasuser_add = gen_add_list(
                             runasuser,
                             (list(res_find.get('ipasudorunas_user', []))
                              + list(res_find.get('ipasudorunasextuser', [])))
@@ -636,69 +563,13 @@ def main():
                     # non-IPA (external) groups, so we need to compare
                     # the provided list against both users and external
                     # groups list.
-                    if (
-                        runasgroup is not None
-                        and (
-                            "ipasudorunasgroup_group" in res_find
-                            or "ipasudorunasextgroup" in res_find
-                        )
-                    ):
-                        runasgroup = gen_add_list(
+                    if runasgroup is not None:
+                        runasgroup_add = gen_add_list(
                             runasgroup,
                             (list(res_find.get("ipasudorunasgroup_group", []))
                              + list(res_find.get("ipasudorunasextgroup", [])))
                         )
 
-                    # Add hosts and hostgroups
-                    if host is not None or hostgroup is not None:
-                        commands.append([name, "sudorule_add_host",
-                                         {
-                                             "host": host,
-                                             "hostgroup": hostgroup,
-                                         }])
-
-                    # Add users and groups
-                    if user is not None or group is not None:
-                        commands.append([name, "sudorule_add_user",
-                                         {
-                                             "user": user,
-                                             "group": group,
-                                         }])
-
-                    # Add commands
-                    if allow_sudocmd is not None \
-                       or allow_sudocmdgroup is not None:
-                        commands.append([name, "sudorule_add_allow_command",
-                                         {"sudocmd": allow_sudocmd,
-                                          "sudocmdgroup": allow_sudocmdgroup,
-                                          }])
-
-                    # Add commands
-                    if deny_sudocmd is not None \
-                       or deny_sudocmdgroup is not None:
-                        commands.append([name, "sudorule_add_deny_command",
-                                         {"sudocmd": deny_sudocmd,
-                                          "sudocmdgroup": deny_sudocmdgroup,
-                                          }])
-
-                    # Add RunAS Users
-                    if runasuser is not None and len(runasuser) > 0:
-                        commands.append([name, "sudorule_add_runasuser",
-                                         {"user": runasuser}])
-
-                    # Add RunAS Groups
-                    if runasgroup is not None and len(runasgroup) > 0:
-                        commands.append([name, "sudorule_add_runasgroup",
-                                         {"group": runasgroup}])
-
-                    # Add options
-                    if sudooption is not None:
-                        existing_opts = res_find.get('ipasudoopt', [])
-                        for sudoopt in sudooption:
-                            if sudoopt not in existing_opts:
-                                commands.append([name, "sudorule_add_option",
-                                                 {"ipasudoopt": sudoopt}])
-
             elif state == "absent":
                 if action == "sudorule":
                     if res_find is not None:
@@ -714,153 +585,70 @@ def main():
                     # and res_find to only try to remove the items that are
                     # in sudorule
                     if host is not None:
-                        if "memberhost_host" in res_find:
-                            host = gen_intersection_list(
-                                host, res_find["memberhost_host"])
-                        else:
-                            host = None
+                        host_del = gen_intersection_list(
+                            host, res_find.get("memberhost_host"))
+
                     if hostgroup is not None:
-                        if "memberhost_hostgroup" in res_find:
-                            hostgroup = gen_intersection_list(
-                                hostgroup, res_find["memberhost_hostgroup"])
-                        else:
-                            hostgroup = None
+                        hostgroup_del = gen_intersection_list(
+                            hostgroup, res_find.get("memberhost_hostgroup"))
+
                     if user is not None:
-                        if "memberuser_user" in res_find:
-                            user = gen_intersection_list(
-                                user, res_find["memberuser_user"])
-                        else:
-                            user = None
+                        user_del = gen_intersection_list(
+                            user, res_find.get("memberuser_user"))
+
                     if group is not None:
-                        if "memberuser_group" in res_find:
-                            group = gen_intersection_list(
-                                group, res_find["memberuser_group"])
-                        else:
-                            group = None
+                        group_del = gen_intersection_list(
+                            group, res_find.get("memberuser_group"))
+
                     if allow_sudocmd is not None:
-                        if "memberallowcmd_sudocmd" in res_find:
-                            allow_sudocmd = gen_intersection_list(
-                                allow_sudocmd,
-                                res_find["memberallowcmd_sudocmd"])
-                        else:
-                            allow_sudocmd = None
+                        allow_cmd_del = gen_intersection_list(
+                            allow_sudocmd,
+                            res_find.get("memberallowcmd_sudocmd")
+                        )
                     if allow_sudocmdgroup is not None:
-                        if "memberallowcmd_sudocmdgroup" in res_find:
-                            allow_sudocmdgroup = gen_intersection_list(
-                                allow_sudocmdgroup,
-                                res_find["memberallowcmd_sudocmdgroup"])
-                        else:
-                            allow_sudocmdgroup = None
+                        allow_cmdgroup_del = gen_intersection_list(
+                            allow_sudocmdgroup,
+                            res_find.get("memberallowcmd_sudocmdgroup")
+                        )
                     if deny_sudocmd is not None:
-                        if "memberdenycmd_sudocmd" in res_find:
-                            deny_sudocmd = gen_intersection_list(
-                                deny_sudocmd,
-                                res_find["memberdenycmd_sudocmd"])
-                        else:
-                            deny_sudocmd = None
+                        deny_cmd_del = gen_intersection_list(
+                            deny_sudocmd,
+                            res_find.get("memberdenycmd_sudocmd")
+                        )
                     if deny_sudocmdgroup is not None:
-                        if "memberdenycmd_sudocmdgroup" in res_find:
-                            deny_sudocmdgroup = gen_intersection_list(
-                                deny_sudocmdgroup,
-                                res_find["memberdenycmd_sudocmdgroup"])
-                        else:
-                            deny_sudocmdgroup = None
+                        deny_cmdgroup_del = gen_intersection_list(
+                            deny_sudocmdgroup,
+                            res_find.get("memberdenycmd_sudocmdgroup")
+                        )
                     if sudooption is not None:
-                        if "ipasudoopt" in res_find:
-                            sudooption = gen_intersection_list(
-                                sudooption, res_find["ipasudoopt"])
-                        else:
-                            sudooption = None
+                        sudooption_del = gen_intersection_list(
+                            sudooption, res_find.get("ipasudoopt"))
                     # runasuser attribute can be used with both IPA and
                     # non-IPA (external) users, so we need to compare
                     # the provided list against both users and external
                     # users list.
                     if runasuser is not None:
-                        if (
-                            "ipasudorunas_user" in res_find
-                            or "ipasudorunasextuser" in res_find
-                        ):
-                            runasuser = gen_intersection_list(
-                                runasuser,
-                                (
-                                    list(res_find.get('ipasudorunas_user', []))
-                                    + list(res_find.get(
-                                        'ipasudorunasextuser', []))
-                                )
+                        runasuser_del = gen_intersection_list(
+                            runasuser,
+                            (
+                                list(res_find.get('ipasudorunas_user', []))
+                                + list(res_find.get('ipasudorunasextuser', []))
                             )
-                        else:
-                            runasuser = None
+                        )
                     # runasgroup attribute can be used with both IPA and
                     # non-IPA (external) groups, so we need to compare
                     # the provided list against both groups and external
                     # groups list.
                     if runasgroup is not None:
-                        if (
-                            "ipasudorunasgroup_group" in res_find
-                            or "ipasudorunasextgroup" in res_find
-                        ):
-                            runasgroup = gen_intersection_list(
-                                runasgroup,
-                                (
-                                    list(res_find.get(
-                                        "ipasudorunasgroup_group", []))
-                                    + list(res_find.get(
-                                        "ipasudorunasextgroup", []))
-                                )
+                        runasgroup_del = gen_intersection_list(
+                            runasgroup,
+                            (
+                                list(res_find.get(
+                                    "ipasudorunasgroup_group", []))
+                                + list(res_find.get(
+                                    "ipasudorunasextgroup", []))
                             )
-                        else:
-                            runasgroup = None
-
-                    # Remove hosts and hostgroups
-                    if host is not None or hostgroup is not None:
-                        commands.append([name, "sudorule_remove_host",
-                                         {
-                                             "host": host,
-                                             "hostgroup": hostgroup,
-                                         }])
-
-                    # Remove users and groups
-                    if user is not None or group is not None:
-                        commands.append([name, "sudorule_remove_user",
-                                         {
-                                             "user": user,
-                                             "group": group,
-                                         }])
-
-                    # Remove allow commands
-                    if allow_sudocmd is not None \
-                       or allow_sudocmdgroup is not None:
-                        commands.append([name, "sudorule_remove_allow_command",
-                                         {"sudocmd": allow_sudocmd,
-                                          "sudocmdgroup": allow_sudocmdgroup
-                                          }])
-
-                    # Remove deny commands
-                    if deny_sudocmd is not None \
-                       or deny_sudocmdgroup is not None:
-                        commands.append([name, "sudorule_remove_deny_command",
-                                         {"sudocmd": deny_sudocmd,
-                                          "sudocmdgroup": deny_sudocmdgroup
-                                          }])
-
-                    # Remove RunAS Users
-                    if runasuser is not None:
-                        commands.append([name, "sudorule_remove_runasuser",
-                                         {"user": runasuser}])
-
-                    # Remove RunAS Groups
-                    if runasgroup is not None:
-                        commands.append([name, "sudorule_remove_runasgroup",
-                                         {"group": runasgroup}])
-
-                    # Remove options
-                    if sudooption is not None:
-                        existing_opts = res_find.get('ipasudoopt', [])
-                        for sudoopt in sudooption:
-                            if sudoopt in existing_opts:
-                                commands.append([name,
-                                                 "sudorule_remove_option",
-                                                 {"ipasudoopt": sudoopt}])
+                        )
 
             elif state == "enabled":
                 if res_find is None:
@@ -885,6 +673,99 @@ def main():
             else:
                 ansible_module.fail_json(msg="Unkown state '%s'" % state)
 
+            # Manage members.
+            # Manage hosts and hostgroups
+            if host_add or hostgroup_add:
+                commands.append([name, "sudorule_add_host",
+                                 {
+                                     "host": host_add,
+                                     "hostgroup": hostgroup_add,
+                                 }])
+            if host_del or hostgroup_del:
+                commands.append([name, "sudorule_remove_host",
+                                 {
+                                     "host": host_del,
+                                     "hostgroup": hostgroup_del,
+                                 }])
+
+            # Manage users and groups
+            if user_add or group_add:
+                commands.append([
+                    name, "sudorule_add_user",
+                    {"user": user_add, "group": group_add}
+                ])
+            if user_del or group_del:
+                commands.append([
+                    name, "sudorule_remove_user",
+                    {"user": user_del, "group": group_del}
+                ])
+
+            # Manage commands allowed
+            if allow_cmd_add or allow_cmdgroup_add:
+                commands.append([
+                    name, "sudorule_add_allow_command",
+                    {
+                        "sudocmd": allow_cmd_add,
+                        "sudocmdgroup": allow_cmdgroup_add,
+                    }
+                ])
+            if allow_cmd_del or allow_cmdgroup_del:
+                commands.append([
+                    name, "sudorule_remove_allow_command",
+                    {
+                        "sudocmd": allow_cmd_del,
+                        "sudocmdgroup": allow_cmdgroup_del
+                    }
+                ])
+            # Manage commands denied
+            if deny_cmd_add or deny_cmdgroup_add:
+                commands.append([
+                    name, "sudorule_add_deny_command",
+                    {
+                        "sudocmd": deny_cmd_add,
+                        "sudocmdgroup": deny_cmdgroup_add,
+                    }
+                ])
+            if deny_cmd_del or deny_cmdgroup_del:
+                commands.append([
+                    name, "sudorule_remove_deny_command",
+                    {
+                        "sudocmd": deny_cmd_del,
+                        "sudocmdgroup": deny_cmdgroup_del
+                    }
+                ])
+            # Manage RunAS users
+            if runasuser_add:
+                commands.append([
+                    name, "sudorule_add_runasuser", {"user": runasuser_add}
+                ])
+            if runasuser_del:
+                commands.append([
+                    name, "sudorule_remove_runasuser", {"user": runasuser_del}
+                ])
+
+            # Manage RunAS Groups
+            if runasgroup_add:
+                commands.append([
+                    name, "sudorule_add_runasgroup", {"group": runasgroup_add}
+                ])
+            if runasgroup_del:
+                commands.append([
+                    name, "sudorule_remove_runasgroup",
+                    {"group": runasgroup_del}
+                ])
+            # Manage sudo options
+            if sudooption_add:
+                for option in sudooption_add:
+                    commands.append([
+                        name, "sudorule_add_option", {"ipasudoopt": option}
+                    ])
+            if sudooption_del:
+                for option in sudooption_del:
+                    commands.append([
+                        name, "sudorule_remove_option", {"ipasudoopt": option}
+                    ])
+
         # Execute commands
 
         changed = ansible_module.execute_ipa_commands(

plugins/modules/ipauser.py

@@ -474,41 +474,31 @@ user:
 
 from ansible.module_utils.ansible_freeipa_module import \
     IPAAnsibleModule, compare_args_ipa, gen_add_del_lists, date_format, \
-    encode_certificate, load_cert_from_str, DN_x500_text, to_text
+    encode_certificate, load_cert_from_str, DN_x500_text, to_text, \
+    ipalib_errors
 from ansible.module_utils import six
 if six.PY3:
     unicode = str
 
 
-def find_user(module, name, preserved=False):
+def find_user(module, name):
     _args = {
         "all": True,
-        "uid": name,
     }
-    if preserved:
-        _args["preserved"] = preserved
 
-    _result = module.ipa_command("user_find", name, _args)
+    try:
+        _result = module.ipa_command("user_show", name, _args).get("result")
+    except ipalib_errors.NotFound:
+        return None
 
-    if len(_result["result"]) > 1:
-        module.fail_json(
-            msg="There is more than one user '%s'" % (name))
-    elif len(_result["result"]) == 1:
-        # Transform each principal to a string
-        _result = _result["result"][0]
-        if "krbprincipalname" in _result \
-           and _result["krbprincipalname"] is not None:
-            _list = []
-            for x in _result["krbprincipalname"]:
-                _list.append(str(x))
-            _result["krbprincipalname"] = _list
-        certs = _result.get("usercertificate")
-        if certs is not None:
-            _result["usercertificate"] = [encode_certificate(x)
-                                          for x in certs]
-        return _result
-
-    return None
+    # Transform each principal to a string
+    _result["krbprincipalname"] = [
+        to_text(x) for x in (_result.get("krbprincipalname") or [])
+    ]
+    _result["usercertificate"] = [
+        encode_certificate(x) for x in (_result.get("usercertificate") or [])
+    ]
+    return _result
 
 
 def gen_args(first, last, fullname, displayname, initials, homedir, shell,
@@ -1085,12 +1075,6 @@ def main():
 
             # Make sure user exists
             res_find = find_user(ansible_module, name)
-            # Also search for preserved user if the user could not be found
-            if res_find is None:
-                res_find_preserved = find_user(ansible_module, name,
-                                               preserved=True)
-            else:
-                res_find_preserved = None
 
             # Create command
             if state == "present":
@@ -1104,10 +1088,6 @@ def main():
                     departmentnumber, employeenumber, employeetype,
                     preferredlanguage, noprivate, nomembers)
 
-                # Also check preserved users
-                if res_find is None and res_find_preserved is not None:
-                    res_find = res_find_preserved
-
                 if action == "user":
                     # Found the user
                     if res_find is not None:
@@ -1310,16 +1290,16 @@ def main():
                                              gen_certmapdata_args(_data)])
 
             elif state == "absent":
-                # Also check preserved users
-                if res_find is None and res_find_preserved is not None:
-                    res_find = res_find_preserved
-
                 if action == "user":
                     if res_find is not None:
                         args = {}
                         if preserve is not None:
                             args["preserve"] = preserve
-                        commands.append([name, "user_del", args])
+                        if (
+                            not res_find.get("preserved", False)
+                            or not args.get("preserve", False)
+                        ):
+                            commands.append([name, "user_del", args])
                 elif action == "member":
                     if res_find is None:
                         ansible_module.fail_json(
@@ -1370,17 +1350,18 @@ def main():
                             commands.append([name, "user_remove_certmapdata",
                                              gen_certmapdata_args(_data)])
             elif state == "undeleted":
-                if res_find_preserved is not None:
-                    commands.append([name, "user_undel", {}])
+                if res_find is not None:
+                    if res_find.get("preserved", False):
+                        commands.append([name, "user_undel", {}])
                 else:
-                    raise ValueError("No preserved user '%s'" % name)
+                    raise ValueError("No user '%s'" % name)
 
             elif state == "enabled":
                 if res_find is not None:
                     if res_find["nsaccountlock"]:
                         commands.append([name, "user_enable", {}])
                 else:
-                    raise ValueError("No disabled user '%s'" % name)
+                    raise ValueError("No user '%s'" % name)
 
             elif state == "disabled":
                 if res_find is not None:
@@ -1392,6 +1373,8 @@ def main():
             elif state == "unlocked":
                 if res_find is not None:
                     commands.append([name, "user_unlock", {}])
+                else:
+                    raise ValueError("No user '%s'" % name)
 
             else:
                 ansible_module.fail_json(msg="Unkown state '%s'" % state)

roles/ipabackup/library/ipabackup_get_backup_dir.py

@@ -61,7 +61,7 @@ from ipaplatform.paths import paths
 
 def main():
     module = AnsibleModule(
-        argument_spec=dict(),
+        argument_spec={},
         supports_check_mode=True,
     )
 

roles/ipaclient/README.md

@@ -33,7 +33,6 @@ Requirements
 **Controller**
 * Ansible version: 2.8+
 * /usr/bin/kinit is required on the controller if a one time password (OTP) is used
-* python3-gssapi is required on the controller if a one time password (OTP) is used with keytab
 
 **Node**
 * Supported FreeIPA version (see above)

roles/ipaclient/action_plugins/ipaclient_get_otp.py

@@ -21,10 +21,6 @@ from __future__ import (absolute_import, division, print_function)
 
 __metaclass__ = type
 
-try:
-    import gssapi
-except ImportError:
-    gssapi = None
 import os
 import shutil
 import subprocess
@@ -45,12 +41,14 @@ def run_cmd(args, stdin=None):
     if stdin:
         p_in = subprocess.PIPE
 
-    p = subprocess.Popen(args, stdin=p_in, stdout=p_out, stderr=p_err,
-                         close_fds=True)
-    __temp, stderr = p.communicate(stdin)
+    # pylint: disable=invalid-name
+    with subprocess.Popen(
+        args, stdin=p_in, stdout=p_out, stderr=p_err, close_fds=True
+    ) as p:
+        __temp, stderr = p.communicate(stdin)
 
-    if p.returncode != 0:
-        raise RuntimeError(stderr)
+        if p.returncode != 0:
+            raise RuntimeError(stderr)
 
 
 def kinit_password(principal, password, ccache_name, config):
@@ -80,22 +78,17 @@ def kinit_keytab(principal, keytab, ccache_name, config):
     It uses the specified config file to kinit and stores the TGT
     in ccache_name.
     """
-    if gssapi is None:
-        raise ImportError("gssapi is not available")
-
+    args = ["/usr/bin/kinit", "-kt", keytab, "-c", ccache_name, principal]
     old_config = os.environ.get('KRB5_CONFIG')
-    os.environ['KRB5_CONFIG'] = config
+    os.environ["KRB5_CONFIG"] = config
+
     try:
-        name = gssapi.Name(principal, gssapi.NameType.kerberos_principal)
-        store = {'ccache': ccache_name,
-                 'client_keytab': keytab}
-        cred = gssapi.Credentials(name=name, store=store, usage='initiate')
-        return cred
+        return run_cmd(args)
     finally:
         if old_config is not None:
-            os.environ['KRB5_CONFIG'] = old_config
+            os.environ["KRB5_CONFIG"] = old_config
         else:
-            os.environ.pop('KRB5_CONFIG', None)
+            os.environ.pop("KRB5_CONFIG", None)
 
 
 KRB5CONF_TEMPLATE = """
@@ -128,8 +121,9 @@ KRB5CONF_TEMPLATE = """
 """
 
 
-class ActionModule(ActionBase):
+class ActionModule(ActionBase):  # pylint: disable=too-few-public-methods
 
+    # pylint: disable=too-many-return-statements
     def run(self, tmp=None, task_vars=None):
         """
         Handle credential cache transfer.
@@ -149,8 +143,9 @@ class ActionModule(ActionBase):
         Then the IPA commands can use this credential cache file.
         """
         if task_vars is None:
-            task_vars = dict()
+            task_vars = {}
 
+        # pylint: disable=super-with-arguments
         result = super(ActionModule, self).run(tmp, task_vars)
         principal = self._task.args.get('principal', None)
         keytab = self._task.args.get('keytab', None)
@@ -168,7 +163,7 @@ class ActionModule(ActionBase):
             return result
 
         data = self._execute_module(module_name='ipaclient_get_facts',
-                                    module_args=dict(), task_vars=task_vars)
+                                    module_args={}, task_vars=task_vars)
 
         try:
             domain = data['ansible_facts']['ipa']['domain']
@@ -195,7 +190,7 @@ class ActionModule(ActionBase):
             ipa_realm=realm,
             ipa_lifetime=lifetime))
 
-        with open(krb5conf_name, 'w') as f:
+        with open(krb5conf_name, 'w') as f:  # pylint: disable=invalid-name
             f.write(content)
 
         if password:

roles/ipaclient/library/ipaclient_api.py

@@ -133,7 +133,9 @@ def main():
 
         # Add CA certs to a temporary NSS database
         try:
+            # pylint: disable=deprecated-method
             argspec = inspect.getargspec(tmp_db.create_db)
+            # pylint: enable=deprecated-method
             if "password_filename" not in argspec.args:
                 tmp_db.create_db()
             else:
@@ -181,7 +183,7 @@ def main():
                 module.warn(
                     "Some capabilities including the ipa command capability "
                     "may not be available")
-            except errors.PublicError as e2:
+            except errors.PublicError as e2:  # pylint: disable=invalid-name
                 module.fail_json(
                     msg="Cannot connect to the IPA server RPC interface: "
                     "%s" % e2)

roles/ipaclient/library/ipaclient_get_facts.py

@@ -56,10 +56,12 @@ def is_ntpd_configured():
     ntpd_conf_section = re.compile(r'^\s*\[ntpd\]\s*$')
 
     try:
+        # pylint: disable=invalid-name
         with open(SERVER_SYSRESTORE_STATE) as f:
             for line in f.readlines():
                 if ntpd_conf_section.match(line):
                     return True
+        # pylint: enable=invalid-name
         return False
     except IOError:
         return False
@@ -71,7 +73,7 @@ def is_dns_configured():
     bind_conf_section = re.compile(r'^\s*dyndb\s+"ipa"\s+"[^"]+"\s+{$')
 
     try:
-        with open(NAMED_CONF) as f:
+        with open(NAMED_CONF) as f:  # pylint: disable=invalid-name
             for line in f.readlines():
                 if bind_conf_section.match(line):
                     return True
@@ -103,7 +105,7 @@ def is_client_configured():
     # and /var/lib/ipa-client/sysrestore/sysrestore.state exists
 
     fstore = sysrestore.FileStore(paths.IPA_CLIENT_SYSRESTORE)
-    return (os.path.isfile(paths.IPA_DEFAULT_CONF) and fstore.has_files())
+    return os.path.isfile(paths.IPA_DEFAULT_CONF) and fstore.has_files()
 
 
 def is_server_configured():
@@ -129,7 +131,9 @@ def get_ipa_conf():
 
 def get_ipa_version():
     try:
+        # pylint: disable=import-outside-toplevel
         from ipapython import version
+        # pylint: enable=import-outside-toplevel
     except ImportError:
         return None
     else:
@@ -156,7 +160,7 @@ def get_ipa_version():
 
 def main():
     module = AnsibleModule(
-        argument_spec=dict(),
+        argument_spec={},
         supports_check_mode=True
     )
 

roles/ipaclient/library/ipaclient_get_otp.py

@@ -146,7 +146,7 @@ def get_host_diff(ipa_host, module_host):
     :return: a dict representing the host attributes to apply
     """
     non_updateable_keys = ['ip_address']
-    data = dict()
+    data = {}
     for key in non_updateable_keys:
         if key in module_host:
             del module_host[key]
@@ -173,7 +173,7 @@ def get_module_host(module):
     :param module: the ansible module
     :returns: a dict representing the host attributes
     """
-    data = dict()
+    data = {}
     certificates = module.params.get('certificates')
     if certificates:
         data['usercertificate'] = certificates
@@ -189,7 +189,7 @@ def get_module_host(module):
     return data
 
 
-def ensure_host_present(module, api, ipahost):
+def ensure_host_present(module, _api, ipahost):
     """
     Ensure host exists in IPA and has the same attributes.
 
@@ -216,13 +216,13 @@ def ensure_host_present(module, api, ipahost):
         # already has Keytab: true, then we need first to run
         # ipa host-disable in order to remove OTP and keytab
         if module.params.get('random') and ipahost['has_keytab'] is True:
-            api.Command.host_disable(fqdn)
+            _api.Command.host_disable(fqdn)
 
-        result = api.Command.host_mod(fqdn, **diffs)
+        result = _api.Command.host_mod(fqdn, **diffs)
         # Save random password as it is not displayed by host-show
         if module.params.get('random'):
             randompassword = result['result']['randompassword']
-        result = api.Command.host_show(fqdn)
+        result = _api.Command.host_show(fqdn)
         if module.params.get('random'):
             result['result']['randompassword'] = randompassword
         module.exit_json(changed=True, host=result['result'])
@@ -236,17 +236,17 @@ def ensure_host_present(module, api, ipahost):
         module_host = get_module_host(module)
         # force creation of host even if there is no DNS record
         module_host["force"] = True
-        result = api.Command.host_add(fqdn, **module_host)
+        result = _api.Command.host_add(fqdn, **module_host)
         # Save random password as it is not displayed by host-show
         if module.params.get('random'):
             randompassword = result['result']['randompassword']
-        result = api.Command.host_show(fqdn)
+        result = _api.Command.host_show(fqdn)
         if module.params.get('random'):
             result['result']['randompassword'] = randompassword
         module.exit_json(changed=True, host=result['result'])
 
 
-def ensure_host_absent(module, api, host):
+def ensure_host_absent(module, _api, host):
     """
     Ensure host does not exist in IPA.
 
@@ -265,7 +265,7 @@ def ensure_host_absent(module, api, host):
 
     fqdn = unicode(module.params.get('fqdn'))
     try:
-        api.Command.host_del(fqdn)
+        _api.Command.host_del(fqdn)
     except Exception as e:
         module.fail_json(msg="Failed to remove host: %s" % e)
 

roles/ipaclient/library/ipaclient_setup_nis.py

@@ -82,7 +82,9 @@ def main():
 
     statestore = sysrestore.StateFile(paths.IPA_CLIENT_SYSRESTORE)
 
+    # pylint: disable=deprecated-method
     argspec = inspect.getargspec(configure_nisdomain)
+    # pylint: enable=deprecated-method
     if "statestore" not in argspec.args:
         # NUM_VERSION < 40500:
         configure_nisdomain(options=options, domain=domain)

roles/ipaclient/library/ipaclient_setup_ntp.py

@@ -113,7 +113,9 @@ def main():
     if sync_time is not None:
         if options.conf_ntp:
             # Attempt to configure and sync time with NTP server (chrony).
+            # pylint: disable=deprecated-method
             argspec = inspect.getargspec(sync_time)
+            # pylint: enable=deprecated-method
             if "options" not in argspec.args:
                 synced_ntp = sync_time(options.ntp_servers, options.ntp_pool,
                                        fstore, statestore)
@@ -149,8 +151,8 @@ def main():
             if options.ntp_servers:
                 ntp_servers = options.ntp_servers
 
-            for s in ntp_servers:
-                synced_ntp = timeconf.synconce_ntp(s, options.debug)
+            for _ntp_server in ntp_servers:
+                synced_ntp = timeconf.synconce_ntp(_ntp_server, options.debug)
                 if synced_ntp:
                     break
 

roles/ipareplica/library/ipareplica_custodia_import_dm_password.py

@@ -199,7 +199,9 @@ def main():
 
         ansible_log.debug("-- CUSTODIA IMPORT DM PASSWORD --")
 
+        # pylint: disable=deprecated-method
         argspec = inspect.getargspec(custodia.import_dm_password)
+        # pylint: enable=deprecated-method
         if "master_host_name" in argspec.args:
             custodia.import_dm_password(config.master_host_name)
         else:

roles/ipareplica/library/ipareplica_setup_certmonger.py

@@ -56,8 +56,7 @@ from ansible.module_utils.ansible_ipa_replica import (
 
 def main():
     ansible_module = AnsibleModule(
-        argument_spec=dict(
-        ),
+        argument_spec={},
         supports_check_mode=True,
     )
 

roles/ipareplica/library/ipareplica_setup_ds.py

@@ -316,7 +316,9 @@ def main():
         ansible_log.debug("-- CONFIGURE DIRSRV --")
         # Configure dirsrv
         with redirect_stdout(ansible_log):
+            # pylint: disable=deprecated-method
             argspec = inspect.getargspec(install_replica_ds)
+            # pylint: enable=deprecated-method
             if "promote" in argspec.args:
                 ds = install_replica_ds(config, options, ca_enabled,
                                         remote_api,
@@ -336,9 +338,13 @@ def main():
                                             ca_file=cafile,
                                             pkcs12_info=dirsrv_pkcs12_info)
 
+        # pylint: disable=deprecated-method
         ansible_log.debug("-- INSTALL DNS RECORDS --")
+        # pylint: enable=deprecated-method
         # Always try to install DNS records
+        # pylint: disable=deprecated-method
         argspec = inspect.getargspec(install_dns_records)
+        # pylint: enable=deprecated-method
         if "fstore" not in argspec.args:
             install_dns_records(config, options, remote_api)
         else:

roles/ipareplica/library/ipareplica_setup_http.py

@@ -202,7 +202,9 @@ def main():
         create_ipa_conf(fstore, config, ca_enabled,
                         master=config.master_host_name)
 
+        # pylint: disable=deprecated-method
         argspec = inspect.getargspec(install_http)
+        # pylint: enable=deprecated-method
         if "promote" in argspec.args:
             install_http(
                 config,

roles/ipareplica/library/ipareplica_setup_krb.py

@@ -161,7 +161,9 @@ def main():
     ansible_log.debug("-- INSTALL_KRB --")
 
     with redirect_stdout(ansible_log):
+        # pylint: disable=deprecated-method
         argspec = inspect.getargspec(install_krb)
+        # pylint: enable=deprecated-method
         if "promote" in argspec.args:
             install_krb(
                 config,

roles/ipareplica/library/ipareplica_test.py

@@ -286,7 +286,9 @@ def main():
     # asks for HTTP certificate in newer ipa versions. In these versions
     # create_ipa_conf has the additional master argument.
     change_master_for_certmonger = False
+    # pylint: disable=deprecated-method
     argspec = inspect.getargspec(create_ipa_conf)
+    # pylint: enable=deprecated-method
     if "master" in argspec.args:
         change_master_for_certmonger = True
 
@@ -418,7 +420,9 @@ def main():
     # check selinux status, http and DS ports, NTP conflicting services
     try:
         with redirect_stdout(ansible_log):
+            # pylint: disable=deprecated-method
             argspec = inspect.getargspec(common_check)
+            # pylint: enable=deprecated-method
             if "skip_mem_check" in argspec.args:
                 common_check(options.no_ntp, options.skip_mem_check,
                              options.setup_ca)

roles/ipareplica/module_utils/ansible_ipa_replica.py

@@ -138,15 +138,15 @@ else:
 
         try:
             from ipaclient.install import timeconf
-            time_service = "chronyd"
-            ntpinstance = None
+            time_service = "chronyd"  # pylint: disable=invalid-name
+            ntpinstance = None  # pylint: disable=invalid-name
         except ImportError:
             try:
                 from ipaclient.install import ntpconf as timeconf
             except ImportError:
                 from ipaclient import ntpconf as timeconf
             from ipaserver.install import ntpinstance
-            time_service = "ntpd"
+            time_service = "ntpd"  # pylint: disable=invalid-name
 
     else:
         # IPA version < 4.6
@@ -162,10 +162,10 @@ else:
             filemode='a', console_format='%(message)s')
 
     @contextlib_contextmanager
-    def redirect_stdout(f):
-        sys.stdout = f
+    def redirect_stdout(stream):
+        sys.stdout = stream
         try:
-            yield f
+            yield stream
         finally:
             sys.stdout = sys.__stdout__
 
@@ -202,7 +202,8 @@ else:
             self.module.debug(msg)
             # self.module.warn(msg)
 
-    class installer_obj(object):
+    # pylint: disable=too-many-instance-attributes, useless-object-inheritance
+    class installer_obj(object):  # pylint: disable=invalid-name
         def __init__(self):
             # CompatServerReplicaInstall
             self.ca_cert_files = None
@@ -244,10 +245,10 @@ else:
         #             (attr, repr(value)))
         #     return value
 
-        def __getattr__(self, attr):
-            logger.info("  --> ADDING missing installer.%s", attr)
-            setattr(self, attr, None)
-            return getattr(self, attr)
+        def __getattr__(self, attrname):
+            logger.info("  --> ADDING missing installer.%s", attrname)
+            setattr(self, attrname, None)
+            return getattr(self, attrname)
 
         # def __setattr__(self, attr, value):
         #    logger.debug("  --> Setting installer.%s to %s" %
@@ -258,6 +259,9 @@ else:
             for name in self.__dict__:
                 yield self, name
 
+    # pylint: enable=too-many-instance-attributes, useless-object-inheritance
+
+    # pylint: disable=attribute-defined-outside-init
     installer = installer_obj()
     options = installer
 
@@ -274,6 +278,7 @@ else:
     # ServerReplicaInstall
     options.subject_base = None
     options.ca_subject = None
+    # pylint: enable=attribute-defined-outside-init
 
     def gen_env_boostrap_finalize_core(etc_ipa, default_config):
         env = Env()
@@ -295,9 +300,12 @@ else:
         # pylint: enable=no-member
         api.finalize()
 
-    def gen_ReplicaConfig():
+    def gen_ReplicaConfig():  # pylint: disable=invalid-name
+        # pylint: disable=too-many-instance-attributes
         class ExtendedReplicaConfig(ReplicaConfig):
+            # pylint: disable=useless-super-delegation
             def __init__(self, top_dir=None):
+                # pylint: disable=super-with-arguments
                 super(ExtendedReplicaConfig, self).__init__(top_dir)
 
             # def __getattribute__(self, attr):
@@ -306,12 +314,13 @@ else:
             #    if attr not in ["__dict__", "knobs"]:
             #        logger.debug("  <== Accessing config.%s (%s)" %
             #                     (attr, repr(value)))
-            #    return value
+            #    return value\
+            # pylint: enable=useless-super-delegation
 
-            def __getattr__(self, attr):
-                logger.info("  ==> ADDING missing config.%s", attr)
-                setattr(self, attr, None)
-                return getattr(self, attr)
+            def __getattr__(self, attrname):
+                logger.info("  ==> ADDING missing config.%s", attrname)
+                setattr(self, attrname, None)
+                return getattr(self, attrname)
 
             # def __setattr__(self, attr, value):
             #   logger.debug("  ==> Setting config.%s to %s" %
@@ -322,7 +331,9 @@ else:
             def knobs(self):
                 for name in self.__dict__:
                     yield self, name
+        # pylint: enable=too-many-instance-attributes
 
+        # pylint: disable=attribute-defined-outside-init
         # config = ReplicaConfig()
         config = ExtendedReplicaConfig()
         config.realm_name = api.env.realm
@@ -338,10 +349,12 @@ else:
         config.basedn = api.env.basedn
         # config.subject_base = options.subject_base
 
+        # pylint: enable=attribute-defined-outside-init
+
         return config
 
     def replica_ds_init_info(ansible_log,
-                             config, options, ca_is_configured, remote_api,
+                             config, options_, ca_is_configured, remote_api,
                              ds_ca_subject, ca_file,
                              promote=False, pkcs12_info=None):
 
@@ -364,7 +377,7 @@ else:
         ca_subject = ds_ca_subject
 
         ds = dsinstance.DsInstance(
-            config_ldif=options.dirsrv_config_file)
+            config_ldif=options_.dirsrv_config_file)
         ds.set_output(ansible_log)
 
         # Source: ipaserver/install/dsinstance.py

roles/ipaserver/library/ipaserver_prepare.py

@@ -312,6 +312,7 @@ def main():
 
         # Create the management framework config file and finalize api
         target_fname = paths.IPA_DEFAULT_CONF
+        # pylint: disable=invalid-name, consider-using-with
         fd = open(target_fname, "w")
         fd.write("[global]\n")
         fd.write("host=%s\n" % options.host_name)
@@ -331,6 +332,7 @@ def main():
             fd.write("ra_plugin=none\n")
         fd.write("mode=production\n")
         fd.close()
+        # pylint: enable=invalid-name, consider-using-with
 
         # Must be readable for everyone
         os.chmod(target_fname, 0o644)
@@ -391,8 +393,8 @@ def main():
                 ansible_log.info("Disabled p11-kit-proxy")
 
     except (RuntimeError, ValueError, ScriptError,
-            ipautil.CalledProcessError) as e:
-        ansible_module.fail_json(msg=str(e))
+            ipautil.CalledProcessError) as err:
+        ansible_module.fail_json(msg=str(err))
 
     ansible_module.exit_json(
         changed=True,

roles/ipaserver/library/ipaserver_setup_ntp.py

@@ -93,7 +93,9 @@ def main():
         # the ipa-server-install --uninstall
         ansible_module.log("Synchronizing time")
 
+        # pylint: disable=deprecated-method
         argspec = inspect.getargspec(sync_time)
+        # pylint: enable=deprecated-method
         if "options" not in argspec.args:
             synced_ntp = sync_time(options.ntp_servers, options.ntp_pool,
                                    fstore, sstore)

roles/ipaserver/library/ipaserver_test.py

@@ -742,7 +742,7 @@ def main():
         validate_admin_password(options.admin_password)
 
     # pkinit is not supported on DL0, don't allow related options
-
+    # pylint: disable=pointless-string-statement
     """
     # replica install: if not options.replica_file is None:
     if (not options._replica_install and \
@@ -755,6 +755,7 @@ def main():
                 "don't use any pkinit-related options.")
         options.no_pkinit = True
     """
+    # pylint: enable=pointless-string-statement
 
     if options.setup_dns:
         if len(options.forwarders) < 1 and not options.no_forwarders and \
@@ -942,7 +943,9 @@ def main():
     else:
         realm_name = options.realm_name.upper()
 
+    # pylint: disable=deprecated-method
     argspec = inspect.getargspec(validate_domain_name)
+    # pylint: enable=deprecated-method
     if "entity" in argspec.args:
         # NUM_VERSION >= 40690:
         try:

roles/ipaserver/module_utils/ansible_ipa_server.py

@@ -105,23 +105,23 @@ else:
         try:
             from ipaclient.install import timeconf
             from ipaclient.install.client import sync_time
-            time_service = "chronyd"
-            ntpinstance = None
+            time_service = "chronyd"  # pylint: disable=invalid-name
+            ntpinstance = None  # pylint: disable=invalid-name
         except ImportError:
             try:
                 from ipaclient.install import ntpconf as timeconf
             except ImportError:
                 from ipaclient import ntpconf as timeconf
             from ipaserver.install import ntpinstance
-            time_service = "ntpd"
-            sync_time = None
+            time_service = "ntpd"  # pylint: disable=invalid-name
+            sync_time = None  # pylint: disable=invalid-name
         from ipaserver.install import (
             adtrust, bindinstance, ca, dns, dsinstance,
             httpinstance, installutils, kra, krbinstance,
             otpdinstance, custodiainstance, replication, service,
             sysupgrade)
-        adtrust_imported = True
-        kra_imported = True
+        adtrust_imported = True  # pylint: disable=invalid-name
+        kra_imported = True  # pylint: disable=invalid-name
         from ipaserver.install.installutils import (
             BadHostError, get_fqdn, get_server_ip_address,
             load_pkcs12, read_password, verify_fqdn,
@@ -158,9 +158,9 @@ else:
 
         try:
             from ipaserver.install import adtrustinstance
-            _server_trust_ad_installed = True
+            _server_trust_ad_installed = True  # pylint: disable=invalid-name
         except ImportError:
-            _server_trust_ad_installed = False
+            _server_trust_ad_installed = False  # pylint: disable=invalid-name
 
         try:
             from ipaclient.install.client import check_ldap_conf
@@ -192,10 +192,10 @@ else:
             filemode='a', console_format='%(message)s')
 
     @contextlib_contextmanager
-    def redirect_stdout(f):
-        sys.stdout = f
+    def redirect_stdout(stream):
+        sys.stdout = stream
         try:
-            yield f
+            yield stream
         finally:
             sys.stdout = sys.__stdout__
 
@@ -232,7 +232,9 @@ else:
             self.module.debug(msg)
             # self.module.warn(msg)
 
-    class options_obj(object):
+    # pylint: disable=too-few-public-methods, useless-object-inheritance
+    # pylint: disable=too-many-instance-attributes
+    class options_obj(object):  # pylint: disable=invalid-name
         def __init__(self):
             self._replica_install = False
             self.dnssec_master = False  # future unknown
@@ -255,9 +257,14 @@ else:
             for name in self.__dict__:
                 yield self, name
 
+    # pylint: enable=too-few-public-methods, useless-object-inheritance
+
+    # pylint: enable=too-many-instance-attributes
     options = options_obj()
     installer = options
 
+    # pylint: disable=attribute-defined-outside-init
+
     # ServerMasterInstall
     options.add_sids = True
     options.add_agents = False
@@ -299,6 +306,9 @@ else:
     options.ignore_topology_disconnect = False
     options.ignore_last_of_role = False
 
+    # pylint: enable=attribute-defined-outside-init
+
+    # pylint: disable=invalid-name
     def api_Backend_ldap2(host_name, setup_ca, connect=False):
         # we are sure we have the configuration file ready.
         cfg = dict(context='installer', confdir=paths.ETC_IPA, in_server=True,
@@ -312,15 +322,17 @@ else:
         if connect:
             api.Backend.ldap2.connect()
 
+    # pylint: enable=invalid-name
+
     def ds_init_info(ansible_log, fstore, domainlevel, dirsrv_config_file,
                      realm_name, host_name, domain_name, dm_password,
                      idstart, idmax, subject_base, ca_subject,
-                     no_hbac_allow, dirsrv_pkcs12_info, no_pkinit):
+                     _no_hbac_allow, dirsrv_pkcs12_info, no_pkinit):
 
         if not options.external_cert_files:
-            ds = dsinstance.DsInstance(fstore=fstore, domainlevel=domainlevel,
-                                       config_ldif=dirsrv_config_file)
-            ds.set_output(ansible_log)
+            _ds = dsinstance.DsInstance(fstore=fstore, domainlevel=domainlevel,
+                                        config_ldif=dirsrv_config_file)
+            _ds.set_output(ansible_log)
 
             if options.dirsrv_cert_files:
                 _dirsrv_pkcs12_info = dirsrv_pkcs12_info
@@ -328,30 +340,30 @@ else:
                 _dirsrv_pkcs12_info = None
 
             with redirect_stdout(ansible_log):
-                ds.init_info(realm_name, host_name, domain_name, dm_password,
-                             subject_base, ca_subject, idstart, idmax,
-                             # hbac_allow=not no_hbac_allow,
-                             _dirsrv_pkcs12_info, setup_pkinit=not no_pkinit)
+                _ds.init_info(realm_name, host_name, domain_name, dm_password,
+                              subject_base, ca_subject, idstart, idmax,
+                              # hbac_allow=not no_hbac_allow,
+                              _dirsrv_pkcs12_info, setup_pkinit=not no_pkinit)
         else:
-            ds = dsinstance.DsInstance(fstore=fstore, domainlevel=domainlevel)
-            ds.set_output(ansible_log)
+            _ds = dsinstance.DsInstance(fstore=fstore, domainlevel=domainlevel)
+            _ds.set_output(ansible_log)
 
             with redirect_stdout(ansible_log):
-                ds.init_info(realm_name, host_name, domain_name, dm_password,
-                             subject_base, ca_subject, 1101, 1100, None,
-                             setup_pkinit=not no_pkinit)
+                _ds.init_info(realm_name, host_name, domain_name, dm_password,
+                              subject_base, ca_subject, 1101, 1100, None,
+                              setup_pkinit=not no_pkinit)
 
-        return ds
+        return _ds
 
     def ansible_module_get_parsed_ip_addresses(ansible_module,
                                                param='ip_addresses'):
         ip_addrs = []
-        for ip in ansible_module.params.get(param):
+        for _ip in ansible_module.params.get(param):
             try:
-                ip_parsed = ipautil.CheckedIPAddress(ip)
-            except Exception as e:
+                ip_parsed = ipautil.CheckedIPAddress(_ip)
+            except Exception as err:
                 ansible_module.fail_json(
-                    msg="Invalid IP Address %s: %s" % (ip, e))
+                    msg="Invalid IP Address %s: %s" % (_ip, err))
             ip_addrs.append(ip_parsed)
         return ip_addrs
 

setup.cfg

@@ -57,14 +57,20 @@ disable =
     fixme
 
 [pylint.BASIC]
-good-names = ex, i, j, k, Run, _, e, x, dn, cn, ip, os, unicode, __metaclass__
+good-names =
+    ex, i, j, k, Run, _, e, x, dn, cn, ip, os, unicode, __metaclass__, ds
 
 [pylint.IMPORTS]
 ignored-modules =
-    ansible.module_utils.ansible_freeipa_module,
+    ansible.errors, ansible.plugins.action,
+    ansible.module_utils, ansible.module_utils.ansible_freeipa_module,
     ipalib, ipalib.config, ipalib.constants, ipalib.krb_utils, ipalib.errors,
     ipapython.ipautil, ipapython.dn, ipapython.version, ipapython.dnsutil,
-    ipaplatform.paths
+    ipapython.ipa_log_manager, ipapython,
+    ipaplatform, ipaplatform.paths, ipaplatform.tasks, ipapython.admintool,
+    ipaserver.install.installutils, ipaserver.install.server.install,
+    ipaserver.install,
+    ipaclient.install.ipachangeconf, ipaclient.install.client
 
 [pylint.REFACTORING]
 max-nested-blocks = 9

site.yml

@@ -1,8 +0,0 @@
----
-- name: Playbook to install IPA clients
-  hosts: ipaclients
-  become: true
-
-  roles:
-  - role: ipaclient
-    state: present

tests/automount/test_automountkey.yml

@@ -0,0 +1,154 @@
+---
+- name: Test automountmap
+  hosts: "{{ ipa_test_host | default('ipaserver') }}"
+  become: no
+  gather_facts: no
+
+  tasks:
+  - name: ensure test location TestLocation is present
+    ipaautomountlocation:
+      ipaadmin_password: SomeADMINpassword
+      name: TestLocation
+
+  - name: ensure test map TestMap is present
+    ipaautomountmap:
+      ipaadmin_password: SomeADMINpassword
+      name: TestMap
+      location: TestLocation
+
+  - name: ensure key NewKeyName is absent
+    ipaautomountkey:
+      ipaadmin_password: SomeADMINpassword
+      location: TestLocation
+      map: TestMap
+      key: NewKeyName
+      state: absent
+
+  - name: ensure key TestKey is absent
+    ipaautomountkey:
+      ipaadmin_password: SomeADMINpassword
+      location: TestLocation
+      map: TestMap
+      key: NewKeyName
+      state: absent
+
+  - block:
+    ### test the key creation, and modification
+    - name: ensure key TestKey is present
+      ipaautomountkey:
+        ipaadmin_password: SomeADMINpassword
+        location: TestLocation
+        map: TestMap
+        key: TestKey
+        info: 192.168.122.1:/exports
+        state: present
+      register: result
+      failed_when: result.failed or not result.changed
+
+    - name: ensure key TestKey is present again
+      ipaautomountkey:
+        ipaadmin_password: SomeADMINpassword
+        location: TestLocation
+        map: TestMap
+        key: TestKey
+        info: 192.168.122.1:/exports
+        state: present
+      register: result
+      failed_when: result.failed or result.changed
+
+    ## modify the key
+    - name: ensure key TestKey information has been updated
+      ipaautomountkey:
+        ipaadmin_password: SomeADMINpassword
+        location: TestLocation
+        map: TestMap
+        key: TestKey
+        info: 192.168.122.1:/nfsshare
+        state: present
+      register: result
+      failed_when: result.failed or not result.changed
+
+    - name: ensure key TestKey information has been updated again
+      ipaautomountkey:
+        ipaadmin_password: SomeADMINpassword
+        location: TestLocation
+        map: TestMap
+        key: TestKey
+        info: 192.168.122.1:/nfsshare
+        state: present
+      register: result
+      failed_when: result.failed or result.changed
+
+    ## modify the name
+    - name: ensure key TestKey has been renamed to NewKeyName
+      ipaautomountkey:
+        ipaadmin_password: SomeADMINpassword
+        location: TestLocation
+        map: TestMap
+        key: TestKey
+        new_name: NewKeyName
+        state: renamed
+      register: result
+      failed_when: result.failed or not result.changed
+
+    - name: ensure key TestKey is absent
+      ipaautomountkey:
+        ipaadmin_password: SomeADMINpassword
+        location: TestLocation
+        map: TestMap
+        key: TestKey
+        state: absent
+      register: result
+      failed_when: result.failed or result.changed
+
+    - name: ensure key NewKeyName is present
+      ipaautomountkey:
+        ipaadmin_password: SomeADMINpassword
+        location: TestLocation
+        map: TestMap
+        key: NewKeyName
+        info: 192.168.122.1:/nfsshare
+        state: present
+      register: result
+      failed_when: result.failed or result.changed
+
+    - name: ensure failure when state is renamed and newname is not set
+      ipaautomountkey:
+        ipaadmin_password: SomeADMINpassword
+        location: TestLocation
+        map: TestMap
+        key: TestKey
+        state: renamed
+      register: result
+      failed_when: not result.failed
+
+    ### cleanup after the tests
+    always:
+    - name: ensure key NewKeyName is absent
+      ipaautomountkey:
+        ipaadmin_password: SomeADMINpassword
+        location: TestLocation
+        map: TestMap
+        key: NewKeyName
+        state: absent
+
+    - name: ensure key TestKey is absent
+      ipaautomountkey:
+        ipaadmin_password: SomeADMINpassword
+        location: TestLocation
+        map: TestMap
+        key: NewKeyName
+        state: absent
+
+    - name: ensure map TestMap is absent
+      ipaautomountmap:
+        ipaadmin_password: SomeADMINpassword
+        name: TestMap
+        location: TestLocation
+        state: absent
+
+    - name: ensure location TestLocation is absent
+      ipaautomountlocation:
+        ipaadmin_password: SomeADMINpassword
+        name: TestLocation
+        state: absent

tests/automount/test_automountkey_client_context.yml

@@ -0,0 +1,41 @@
+---
+- name: Test automountkey
+  hosts: ipaclients, ipaserver
+  become: no
+  gather_facts: no
+
+  tasks:
+  - name: Include FreeIPA facts.
+    include_tasks: ../env_freeipa_facts.yml
+
+  # Test will only be executed if host is not a server.
+  - name: Execute with server context in the client.
+    ipaautomountkey:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: server
+      location: NoLocation
+      map: NoMap
+      key: ThisShouldNotWork
+    register: result
+    failed_when: not (result.failed and result.msg is regex("No module named '*ipaserver'*"))
+    when: ipa_host_is_client
+
+# Import basic module tests, and execute with ipa_context set to 'client'.
+# If ipaclients is set, it will be executed using the client, if not,
+# ipaserver will be used.
+#
+# With this setup, tests can be executed against an IPA client, against
+# an IPA server using "client" context, and ensure that tests are executed
+# in upstream CI.
+
+- name: Test automountlocation using client context, in client host.
+  import_playbook: test_automountkey.yml
+  when: groups['ipaclients']
+  vars:
+    ipa_test_host: ipaclients
+
+- name: Test automountlocation using client context, in server host.
+  import_playbook: test_automountkey.yml
+  when: groups['ipaclients'] is not defined or not groups['ipaclients']
+  vars:
+    ipa_context: client

tests/automount/test_automountmap.yml

@@ -0,0 +1,147 @@
+---
+- name: Test automountmap
+  hosts: "{{ ipa_test_host | default('ipaserver') }}"
+  become: no
+  gather_facts: no
+
+  tasks:
+  # setup environment
+  - name: ensure test maps are absent
+    ipaautomountmap:
+      ipaadmin_password: SomeADMINpassword
+      name:
+        - TestMap01
+        - TestMap02
+      location: TestLocation
+      state: absent
+
+  - name: ensure location TestLocation is absent
+    ipaautomountlocation:
+      ipaadmin_password: SomeADMINpassword
+      name: TestLocation
+      state: absent
+
+  - name: ensure map TestMap is absent
+    ipaautomountmap:
+      ipaadmin_password: SomeADMINpassword
+      name: TestMap
+      location: TestLocation
+      state: absent
+
+  - name: ensure location TestLocation is present
+    ipaautomountlocation:
+      ipaadmin_password: SomeADMINpassword
+      name: TestLocation
+      state: present
+
+  # TESTS
+  - block:
+    - name: ensure map TestMap is present
+      ipaautomountmap:
+        ipaadmin_password: SomeADMINpassword
+        name: TestMap
+        location: TestLocation
+        desc: "this is a test map that should be deleted by the test"
+      register: result
+      failed_when: result.failed or not result.changed
+
+    - name: ensure map TestMap is present again
+      ipaautomountmap:
+        ipaadmin_password: SomeADMINpassword
+        name: TestMap
+        location: TestLocation
+      register: result
+      failed_when: result.failed or result.changed
+
+    - name: ensure map TestMap has a different description
+      ipaautomountmap:
+        ipaadmin_password: SomeADMINpassword
+        name: TestMap
+        location: TestLocation
+        desc: "this is a changed description that should be deleted by the test"
+      register: result
+      failed_when: result.failed or not result.changed
+
+    - name: ensure map TestMap has a different description, again
+      ipaautomountmap:
+        ipaadmin_password: SomeADMINpassword
+        name: TestMap
+        location: TestLocation
+        desc: "this is a changed description that should be deleted by the test"
+      register: result
+      failed_when: result.failed or result.changed
+
+    - name: ensure map TestMap is removed
+      ipaautomountmap:
+        ipaadmin_password: SomeADMINpassword
+        name: TestMap
+        location: TestLocation
+        state: absent
+      register: result
+      failed_when: result.failed or not result.changed
+
+    - name: ensure map TestMap has been removed
+      ipaautomountmap:
+        ipaadmin_password: SomeADMINpassword
+        name: TestMap
+        location: TestLocation
+        state: absent
+      register: result
+      failed_when: result.failed or result.changed
+
+    - name: ensure map TestMap01 is present
+      ipaautomountmap:
+        ipaadmin_password: SomeADMINpassword
+        name: TestMap01
+        location: TestLocation
+        desc: "this is a changed description that should be deleted by the test"
+      register: result
+      failed_when: result.failed or not result.changed
+
+    - name: ensure map TestMap02 is present
+      ipaautomountmap:
+        ipaadmin_password: SomeADMINpassword
+        name: TestMap02
+        location: TestLocation
+        desc: "this is a changed description that should be deleted by the test"
+      register: result
+      failed_when: result.failed or not result.changed
+
+    - name: ensure TestMap01 and TestMap02 are both absent
+      ipaautomountmap:
+        ipaadmin_password: SomeADMINpassword
+        name:
+          - TestMap01
+          - TestMap02
+        location: TestLocation
+        state: absent
+      register: result
+      failed_when: result.failed or not result.changed
+
+    - name: ensure TestMap01 and TestMap02 are both absent again
+      ipaautomountmap:
+        ipaadmin_password: SomeADMINpassword
+        name:
+          - TestMap01
+          - TestMap02
+        location: TestLocation
+        state: absent
+      register: result
+      failed_when: result.failed or result.changed
+
+    # CLEAN UP
+    always:
+    - name: ensure test maps are absent
+      ipaautomountmap:
+        ipaadmin_password: SomeADMINpassword
+        name:
+          - TestMap01
+          - TestMap02
+        location: TestLocation
+        state: absent
+
+    - name: ensure location TestLocation is absent
+      ipaautomountlocation:
+        ipaadmin_password: SomeADMINpassword
+        name: TestLocation
+        state: absent

tests/automount/test_automountmap_client_context.yml

@@ -0,0 +1,40 @@
+---
+- name: Test automountmap
+  hosts: ipaclients, ipaserver
+  become: no
+  gather_facts: no
+
+  tasks:
+  - name: Include FreeIPA facts.
+    include_tasks: ../env_freeipa_facts.yml
+
+  # Test will only be executed if host is not a server.
+  - name: Execute with server context in the client.
+    ipaautomountmap:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: server
+      location: default
+      name: ThisShouldNotWork
+    register: result
+    failed_when: not (result.failed and result.msg is regex("No module named '*ipaserver'*"))
+    when: ipa_host_is_client
+
+# Import basic module tests, and execute with ipa_context set to 'client'.
+# If ipaclients is set, it will be executed using the client, if not,
+# ipaserver will be used.
+#
+# With this setup, tests can be executed against an IPA client, against
+# an IPA server using "client" context, and ensure that tests are executed
+# in upstream CI.
+
+- name: Test automountmap using client context, in client host.
+  import_playbook: test_automountmap.yml
+  when: groups['ipaclients']
+  vars:
+    ipa_test_host: ipaclients
+
+- name: Test automountmap using client context, in server host.
+  import_playbook: test_automountmap.yml
+  when: groups['ipaclients'] is not defined or not groups['ipaclients']
+  vars:
+    ipa_context: client

tests/azure/azure-pipelines.yml

@@ -18,6 +18,17 @@ stages:
       scenario: fedora-latest
       ansible_version: ">=2.9,<2.10"
 
+# CentOS 9
+
+- stage: CentOS9_Ansible_2_9
+  dependsOn: []
+  jobs:
+  - template: templates/group_tests.yml
+    parameters:
+      build_number: $(Build.BuildNumber)
+      scenario: centos-9
+      ansible_version: ">=2.9,<2.10"
+
 # CentOS 8
 
 - stage: CentOS8_Ansible_2_9

tests/azure/nightly.yml

@@ -34,6 +34,15 @@ stages:
       scenario: fedora-latest
       ansible_version: "-core >=2.11,<2.12"
 
+- stage: FedoraLatest_Ansible_Core_2_12
+  dependsOn: []
+  jobs:
+  - template: templates/group_tests.yml
+    parameters:
+      build_number: $(Build.BuildNumber)
+      scenario: fedora-latest
+      ansible_version: "-core >=2.12,<2.13"
+
 - stage: FedoraLatest_Ansible_latest
   dependsOn: []
   jobs:
@@ -43,6 +52,35 @@ stages:
       scenario: fedora-latest
       ansible_version: ""
 
+# CentoOS 9
+
+- stage: CentOS9_Ansible_2_9
+  dependsOn: []
+  jobs:
+  - template: templates/group_tests.yml
+    parameters:
+      build_number: $(Build.BuildNumber)
+      scenario: centos-9
+      ansible_version: ">=2.9,<2.10"
+
+- stage: CentOS9_Ansible_Core_2_11
+  dependsOn: []
+  jobs:
+  - template: templates/group_tests.yml
+    parameters:
+      build_number: $(Build.BuildNumber)
+      scenario: centos-9
+      ansible_version: "-core >=2.11,<2.12"
+
+- stage: CentOS9_Ansible_latest
+  dependsOn: []
+  jobs:
+  - template: templates/group_tests.yml
+    parameters:
+      build_number: $(Build.BuildNumber)
+      scenario: centos-9
+      ansible_version: ""
+
 # CentOS 8
 
 - stage: CentOS8_Ansible_2_9
@@ -63,6 +101,15 @@ stages:
       scenario: centos-8
       ansible_version: "-core >=2.11,<2.12"
 
+- stage: CentOS8_Ansible_Core_2_12
+  dependsOn: []
+  jobs:
+  - template: templates/group_tests.yml
+    parameters:
+      build_number: $(Build.BuildNumber)
+      scenario: centos-8
+      ansible_version: "-core >=2.12,<2.13"
+
 - stage: CentOS8_Ansible_latest
   dependsOn: []
   jobs:
@@ -92,6 +139,15 @@ stages:
       scenario: centos-7
       ansible_version: "-core >=2.11,<2.12"
 
+- stage: CentOS7_Ansible_Core_2_12
+  dependsOn: []
+  jobs:
+  - template: templates/group_tests.yml
+    parameters:
+      build_number: $(Build.BuildNumber)
+      scenario: centos-7
+      ansible_version: "-core >=2.12,<2.13"
+
 - stage: CentOS7_Ansible_latest
   dependsOn: []
   jobs:

tests/azure/templates/group_tests.yml

@@ -38,3 +38,4 @@ jobs:
   parameters:
     build_number: ${{ parameters.build_number }}
     scenario: ${{ parameters.scenario }}
+    ansible_version: ${{ parameters.ansible_version }}

tests/azure/templates/playbook_tests.yml

@@ -14,7 +14,7 @@ parameters:
     default: ""
   - name: python_version
     type: string
-    default: 3.6
+    default: 3.x
   - name: build_number
     type: string
 

tests/azure/templates/pytest_tests.yml

@@ -9,7 +9,7 @@ parameters:
     default: ""
   - name: python_version
     type: string
-    default: 3.6
+    default: 3.x
 
 jobs:
 - job: Test_PyTests

tests/dnsconfig/test_dnsconfig.yml

@@ -17,6 +17,7 @@
         - ip_address: 2001:4860:4860::8888
           port: 53
       state: absent
+      action: member
 
   # Tests.
   - name: Set config to invalid IPv4.
@@ -74,23 +75,72 @@
     register: result
     failed_when: result.changed or result.failed
 
-  - name: Ensure forwarder is absent.
+  - name: Ensure forwarder 8.8.8.8 is absent.
     ipadnsconfig:
       ipaadmin_password: SomeADMINpassword
       ipaapi_context: "{{ ipa_context | default(omit) }}"
       forwarders:
         - ip_address: 8.8.8.8
       state: absent
+      action: member
     register: result
     failed_when: not result.changed or result.failed
 
-  - name: Ensure forwarder is absent, again.
+  - name: Ensure forwarder 8.8.8.8 is absent, again.
     ipadnsconfig:
       ipaadmin_password: SomeADMINpassword
       ipaapi_context: "{{ ipa_context | default(omit) }}"
       forwarders:
         - ip_address: 8.8.8.8
       state: absent
+      action: member
+    register: result
+    failed_when: result.changed or result.failed
+
+  - name: Ensure forwarder 8.8.4.4 is present.
+    ipadnsconfig:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      forwarders:
+        - ip_address: 8.8.4.4
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure forwarder 8.8.8.8 is present.
+    ipadnsconfig:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      forwarders:
+        - ip_address: 8.8.8.8
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure forwarder 8.8.4.4 is present.
+    ipadnsconfig:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      forwarders:
+        - ip_address: 8.8.4.4
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure forwarders are absent.
+    ipadnsconfig:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      forwarders:
+        - ip_address: 8.8.4.4
+        - ip_address: 8.8.8.8
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure forwarders are absent, again.
+    ipadnsconfig:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      forwarders:
+        - ip_address: 8.8.4.4
+        - ip_address: 8.8.8.8
     register: result
     failed_when: result.changed or result.failed
 
@@ -168,10 +218,10 @@
         - ip_address: 2001:4860:4860::8888
           port: 53
       state: absent
+      action: member
     register: result
     failed_when: not result.changed or result.failed
 
-
   - name: Ensure all forwarders are absent, again.
     ipadnsconfig:
       ipaadmin_password: SomeADMINpassword
@@ -182,6 +232,69 @@
         - ip_address: 2001:4860:4860::8888
           port: 53
       state: absent
+      action: member
+    register: result
+    failed_when: result.changed or result.failed
+
+  - name: Ensure forwarder is present.
+    ipadnsconfig:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      forwarders:
+        - ip_address: 8.8.8.8
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure forwarders is not present.
+    ipadnsconfig:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      forwarders:
+        - ip_address: 8.8.4.4
+    check_mode: yes
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure forwarders are present.
+    ipadnsconfig:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      forwarders:
+        - ip_address: 8.8.4.4
+        - ip_address: 8.8.8.8
+      action: member
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure forwarders are present, again.
+    ipadnsconfig:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      forwarders:
+        - ip_address: 8.8.4.4
+        - ip_address: 8.8.8.8
+      action: member
+    register: result
+    failed_when: result.changed or result.failed
+
+  - name: Ensure another forwarder is present.
+    ipadnsconfig:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      forwarders:
+        - ip_address: 8.8.4.4
+      action: member
+    register: result
+    failed_when: result.changed or result.failed
+
+  - name: Ensure forwarders are present.
+    ipadnsconfig:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      forwarders:
+        - ip_address: 8.8.4.4
+        - ip_address: 8.8.8.8
+      action: member
     register: result
     failed_when: result.changed or result.failed
 
@@ -197,3 +310,4 @@
         - ip_address: 2001:4860:4860::8888
           port: 53
       state: absent
+      action: member

tests/env_freeipa_facts.yml

@@ -29,3 +29,15 @@
     ipa_api_version: "{{ ipa_cmd_version.stdout_lines[1] }}"
     ipa_host_is_client: "{{ (output.stdout_lines[-1] == 'CLIENT') | bool }}"
     trust_test_is_supported: no
+
+- block:
+  - name: Get Domain from server name
+    set_fact:
+      ipaserver_domain: "{{ ansible_facts['fqdn'].split('.')[1:] | join ('.') }}"
+    when: "'fqdn' in ansible_facts"
+
+  - name: Set Domain to 'ipa.test' if FQDN could not be retrieved.
+    set_fact:
+      ipaserver_domain: "ipa.test"
+    when: "'fqdn' not in ansible_facts"
+  when: ipaserver_domain is not defined

tests/group/test_group.yml

@@ -5,6 +5,23 @@
   gather_facts: false
 
   tasks:
+  # setup
+  - include_tasks: ../env_freeipa_facts.yml
+
+  # GET DOMAIN AND REALM
+
+  - name: Get Domain from server name
+    set_fact:
+      ipaserver_domain: "{{ ansible_facts['fqdn'].split('.')[1:] | join ('.') }}"
+    when: ipaserver_domain is not defined
+
+  - name: Get Realm from server name
+    set_fact:
+      ipaserver_realm: "{{ ansible_facts['fqdn'].split('.')[1:] | join ('.') | upper }}"
+    when: ipaserver_realm is not defined
+
+  # CLEANUP TEST ITEMS
+
   - name: Ensure users user1, user2 and user3 are absent
     ipauser:
       ipaadmin_password: SomeADMINpassword
@@ -19,6 +36,8 @@
       name: group3,group2,group1
       state: absent
 
+  # CREATE TEST ITEMS
+
   - name: Ensure users user1..user3 are present
     ipauser:
       ipaadmin_password: SomeADMINpassword
@@ -36,6 +55,8 @@
     register: result
     failed_when: not result.changed or result.failed
 
+  # TESTS
+
   - name: Ensure group1 is present
     ipagroup:
       ipaadmin_password: SomeADMINpassword
@@ -119,6 +140,156 @@
     register: result
     failed_when: result.changed or result.failed
 
+    # service
+
+  - block:
+
+    - name: Ensure service "{{ 'HTTP/ipaserver.' + ipaserver_domain + '@' + ipaserver_realm }}" is present in group group1
+      ipagroup:
+        ipaadmin_password: SomeADMINpassword
+        ipaapi_context: "{{ ipa_context | default(omit) }}"
+        name: group1
+        service:
+        - "{{ 'HTTP/ipaserver.' + ipaserver_domain + '@' + ipaserver_realm }}"
+        action: member
+      register: result
+      failed_when: not result.changed or result.failed
+
+    - name: Ensure service "{{ 'HTTP/ipaserver.' + ipaserver_domain + '@' + ipaserver_realm }}" is present in group group1, again
+      ipagroup:
+        ipaadmin_password: SomeADMINpassword
+        ipaapi_context: "{{ ipa_context | default(omit) }}"
+        name: group1
+        service:
+        - "{{ 'HTTP/ipaserver.' + ipaserver_domain + '@' + ipaserver_realm }}"
+        action: member
+      register: result
+      failed_when: result.changed or result.failed
+
+    - name: Ensure service "{{ 'ldap/ipaserver.' + ipaserver_domain + '@' + ipaserver_realm }}" is present in group group1
+      ipagroup:
+        ipaadmin_password: SomeADMINpassword
+        ipaapi_context: "{{ ipa_context | default(omit) }}"
+        name: group1
+        service:
+        - "{{ 'ldap/ipaserver.' + ipaserver_domain + '@' + ipaserver_realm }}"
+        action: member
+      register: result
+      failed_when: not result.changed or result.failed
+
+    - name: Ensure service "{{ 'ldap/ipaserver.' + ipaserver_domain + '@' + ipaserver_realm }}" is present in group group1, again
+      ipagroup:
+        ipaadmin_password: SomeADMINpassword
+        ipaapi_context: "{{ ipa_context | default(omit) }}"
+        name: group1
+        service:
+        - "{{ 'ldap/ipaserver.' + ipaserver_domain + '@' + ipaserver_realm }}"
+        action: member
+      register: result
+      failed_when: result.changed or result.failed
+
+    - name: Ensure service "{{ 'HTTP/ipaserver.' + ipaserver_domain + '@' + ipaserver_realm }}" is absent in group group1
+      ipagroup:
+        ipaadmin_password: SomeADMINpassword
+        ipaapi_context: "{{ ipa_context | default(omit) }}"
+        name: group1
+        service:
+        - "{{ 'HTTP/ipaserver.' + ipaserver_domain + '@' + ipaserver_realm }}"
+        action: member
+        state: absent
+      register: result
+      failed_when: not result.changed or result.failed
+
+    - name: Ensure service "{{ 'HTTP/ipaserver.' + ipaserver_domain + '@' + ipaserver_realm }}" is absent in group group1, again
+      ipagroup:
+        ipaadmin_password: SomeADMINpassword
+        ipaapi_context: "{{ ipa_context | default(omit) }}"
+        name: group1
+        service:
+        - "{{ 'HTTP/ipaserver.' + ipaserver_domain + '@' + ipaserver_realm }}"
+        action: member
+        state: absent
+      register: result
+      failed_when: result.changed or result.failed
+
+    - name: Ensure service "{{ 'ldap/ipaserver.' + ipaserver_domain + '@' + ipaserver_realm }}" is absent in group group1
+      ipagroup:
+        ipaadmin_password: SomeADMINpassword
+        ipaapi_context: "{{ ipa_context | default(omit) }}"
+        name: group1
+        service:
+        - "{{ 'ldap/ipaserver.' + ipaserver_domain + '@' + ipaserver_realm }}"
+        action: member
+        state: absent
+      register: result
+      failed_when: not result.changed or result.failed
+
+    - name: Ensure service "{{ 'ldap/ipaserver.' + ipaserver_domain + '@' + ipaserver_realm }}" is absent in group group1, again
+      ipagroup:
+        ipaadmin_password: SomeADMINpassword
+        ipaapi_context: "{{ ipa_context | default(omit) }}"
+        name: group1
+        service:
+        - "{{ 'ldap/ipaserver.' + ipaserver_domain + '@' + ipaserver_realm }}"
+        action: member
+        state: absent
+      register: result
+      failed_when: result.changed or result.failed
+
+    - name: Ensure services are present in group group1
+      ipagroup:
+        ipaadmin_password: SomeADMINpassword
+        ipaapi_context: "{{ ipa_context | default(omit) }}"
+        name: group1
+        service:
+        - "{{ 'HTTP/ipaserver.' + ipaserver_domain + '@' + ipaserver_realm }}"
+        - "{{ 'ldap/ipaserver.' + ipaserver_domain + '@' + ipaserver_realm }}"
+        action: member
+      register: result
+      failed_when: not result.changed or result.failed
+
+    - name: Ensure services are present in group group1, again
+      ipagroup:
+        ipaadmin_password: SomeADMINpassword
+        ipaapi_context: "{{ ipa_context | default(omit) }}"
+        name: group1
+        service:
+        - "{{ 'http/ipaserver.' + ipaserver_domain + '@' + ipaserver_realm }}"
+        - "{{ 'ldap/ipaserver.' + ipaserver_domain + '@' + ipaserver_realm }}"
+        action: member
+      register: result
+      failed_when: result.changed or result.failed
+
+    - name: Ensure services are absent in group group1
+      ipagroup:
+        ipaadmin_password: SomeADMINpassword
+        ipaapi_context: "{{ ipa_context | default(omit) }}"
+        name: group1
+        service:
+        - "{{ 'HTTP/ipaserver.' + ipaserver_domain + '@' + ipaserver_realm }}"
+        - "{{ 'LDAP/ipaserver.' + ipaserver_domain + '@' + ipaserver_realm }}"
+        action: member
+        state: absent
+      register: result
+      failed_when: not result.changed or result.failed
+
+    - name: Ensure services are absent in group group1, again
+      ipagroup:
+        ipaadmin_password: SomeADMINpassword
+        ipaapi_context: "{{ ipa_context | default(omit) }}"
+        name: group1
+        service:
+        - "{{ 'HTTP/ipaserver.' + ipaserver_domain + '@' + ipaserver_realm }}"
+        - "{{ 'ldap/ipaserver.' + ipaserver_domain + '@' + ipaserver_realm }}"
+        action: member
+        state: absent
+      register: result
+      failed_when: result.changed or result.failed
+
+    when: ipa_version is version('4.7.0', '>=')
+
+    # user
+
   - name: Ensure users user1, user2 and user3 are present in group group1
     ipagroup:
       ipaadmin_password: SomeADMINpassword
@@ -174,6 +345,131 @@
     register: result
     failed_when: result.changed or result.failed
 
+  - name: Ensure groups group3, group2, and group1 are absent
+    ipagroup:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      name: group3,group2,group1
+      state: absent
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure group group1 is present
+    ipagroup:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      name: group1
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure users user1, user2 are present in group group1
+    ipagroup:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      name: group1
+      user:
+      - user1
+      - user2
+      action: member
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure users user1, user2 and user3 are present in group group1
+    ipagroup:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      name: group1
+      user:
+      - user1
+      - user2
+      - user3
+      action: member
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure users user1, user2 are present in group group1, again
+    ipagroup:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      name: group1
+      user:
+      - user1
+      - user2
+      action: member
+    register: result
+    failed_when: result.changed or result.failed
+
+  - name: Ensure users user1, user2 and user3 are present in group group1, again
+    ipagroup:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      name: group1
+      user:
+      - user1
+      - user2
+      - user3
+      action: member
+    register: result
+    failed_when: result.changed or result.failed
+
+  - name: Ensure group group1 is absent
+    ipagroup:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      name: group1
+      state: absent
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure group group1 with users user1, user2 is present
+    ipagroup:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      name: group1
+      user:
+      - user1
+      - user2
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure group group1 with users user1, user2 and user3 is present
+    ipagroup:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      name: group1
+      user:
+      - user1
+      - user2
+      - user3
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure group group1 with users user1, user2 and user3 is present, again
+    ipagroup:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      name: group1
+      user:
+      - user1
+      - user2
+      - user3
+      action: member
+    register: result
+    failed_when: result.changed or result.failed
+
+  - name: Ensure only users user1, user2 are present in group group1
+    ipagroup:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      name: group1
+      user:
+      - user1
+      - user2
+    register: result
+    failed_when: not result.changed or result.failed
+
+  # CLEANUP TEST ITEMS
+
   - name: Ensure group group3, group2 and group1 are absent
     ipagroup:
       ipaadmin_password: SomeADMINpassword

tests/hbacrule/test_hbacrule_member_case_insensitive.yml

@@ -0,0 +1,503 @@
+---
+- name: Test group
+  hosts: "{{ ipa_test_host | default('ipaserver') }}"
+  become: no
+  gather_facts: yes
+
+  vars:
+    user_list:
+      - User1
+      - uSer2
+      - usEr3
+    group_list:
+      - Group1
+      - gRoup2
+      - grOup3
+    host_list:
+      - HoSt01
+      - hOsT02
+    hostgroup_list:
+      - TestHostGroup
+    hbacsvc_list:
+      - Svc1
+      - sVC2
+    hbacsvcgroup_list:
+      - sVCgrOUp1
+
+  tasks:
+  - include_tasks: ../env_freeipa_facts.yml
+
+  - block:
+      # setup
+
+      - name: Ensure test hbacrule is absent
+        ipahbacrule:
+          ipaadmin_password: SomeADMINpassword
+          name: testrule
+          state: absent
+
+      - name: Ensure test users are present
+        ipauser:
+          ipaadmin_password: SomeADMINpassword
+          users:
+          - name: "{{ item }}"
+            first: First
+            last: Last
+        with_items: "{{ user_list }}"
+
+      - name: Ensure test groups are present
+        ipagroup:
+          ipaadmin_password: SomeADMINpassword
+          name: "{{ item }}"
+        with_items: "{{ group_list }}"
+
+      - name: Ensure test hosts are present
+        ipahost:
+          ipaadmin_password: SomeADMINpassword
+          name: "{{ item }}.{{ ipaserver_domain }}"
+          force: yes
+        with_items: "{{ host_list }}"
+
+      - name: Ensure test hostgroups are present
+        ipahostgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: "{{ item }}"
+        with_items: "{{ hostgroup_list }}"
+
+      - name: Ensure test hbac services are present
+        ipahbacsvc:
+          ipaadmin_password: SomeADMINpassword
+          name: "{{ item }}"
+        with_items: "{{ hbacsvc_list }}"
+
+      - name: Ensure test hbac service groups are present
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: "{{ item }}"
+        with_items: "{{ hbacsvcgroup_list }}"
+
+      # Test with action: hbacrule
+
+      - name: Check if hbacrule present with members would trigger changes, mixed case
+        ipahbacrule:
+          ipaadmin_password: SomeADMINpassword
+          name: testrule
+          user:
+          - "{{ user_list[1] }}"
+          - "{{ user_list[2] }}"
+          group:
+          - "{{ group_list[1] }}"
+          - "{{ group_list[2] }}"
+          host:
+          - "{{ host_list[0] }}"
+          - "{{ host_list[1] }}"
+          hostgroup:
+          - "{{ hostgroup_list[0] }}"
+          hbacsvc:
+          - "{{ hbacsvc_list[0] }}"
+          - "{{ hbacsvc_list[1] }}"
+          hbacsvcgroup:
+          - "{{ hbacsvcgroup_list[0] }}"
+        check_mode: yes
+        register: result
+        failed_when: not result.changed or result.failed
+
+      - name: Ensure hbacrule is present with members, mixed case
+        ipahbacrule:
+          ipaadmin_password: SomeADMINpassword
+          name: testrule
+          user:
+          - "{{ user_list[1] }}"
+          - "{{ user_list[2] }}"
+          group:
+          - "{{ group_list[1] }}"
+          - "{{ group_list[2] }}"
+          host:
+          - "{{ host_list[0] }}"
+          - "{{ host_list[1] }}"
+          hostgroup:
+          - "{{ hostgroup_list[0] }}"
+          hbacsvc:
+          - "{{ hbacsvc_list[0] }}"
+          - "{{ hbacsvc_list[1] }}"
+          hbacsvcgroup:
+          - "{{ hbacsvcgroup_list[0] }}"
+        register: result
+        failed_when: not result.changed or result.failed
+
+      - name: Check if hbacrule present with members would not trigger changes, mixed case
+        ipahbacrule:
+          ipaadmin_password: SomeADMINpassword
+          name: testrule
+          user:
+          - "{{ user_list[1] }}"
+          - "{{ user_list[2] }}"
+          group:
+          - "{{ group_list[1] }}"
+          - "{{ group_list[2] }}"
+          host:
+          - "{{ host_list[0] }}"
+          - "{{ host_list[1] }}"
+          hostgroup:
+          - "{{ hostgroup_list[0] }}"
+          hbacsvc:
+          - "{{ hbacsvc_list[0] }}"
+          - "{{ hbacsvc_list[1] }}"
+          hbacsvcgroup:
+          - "{{ hbacsvcgroup_list[0] }}"
+        check_mode: yes
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure hbacrule is present with members, lowercase
+        ipahbacrule:
+          ipaadmin_password: SomeADMINpassword
+          name: testrule
+          user:
+          - "{{ user_list[1] | lower }}"
+          - "{{ user_list[2] | lower }}"
+          group:
+          - "{{ group_list[1] | lower }}"
+          - "{{ group_list[2] | lower }}"
+          host:
+          - "{{ host_list[0] | lower }}"
+          - "{{ host_list[1] | lower }}"
+          hostgroup:
+          - "{{ hostgroup_list[0] | lower }}"
+          hbacsvc:
+          - "{{ hbacsvc_list[0] | lower }}"
+          - "{{ hbacsvc_list[1] | lower }}"
+          hbacsvcgroup:
+          - "{{ hbacsvcgroup_list[0] | lower }}"
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure hbacrule is present with members, upercase
+        ipahbacrule:
+          ipaadmin_password: SomeADMINpassword
+          name: testrule
+          user:
+          - "{{ user_list[1] | upper }}"
+          - "{{ user_list[2] | upper }}"
+          group:
+          - "{{ group_list[1] | upper }}"
+          - "{{ group_list[2] | upper }}"
+          host:
+          - "{{ host_list[0] | upper }}"
+          - "{{ host_list[1] | upper }}"
+          hostgroup:
+          - "{{ hostgroup_list[0] | upper }}"
+          hbacsvc:
+          - "{{ hbacsvc_list[0] | upper }}"
+          - "{{ hbacsvc_list[1] | upper }}"
+          hbacsvcgroup:
+          - "{{ hbacsvcgroup_list[0] | upper  }}"
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure test hbacrule is absent
+        ipahbacrule:
+          ipaadmin_password: SomeADMINpassword
+          name: testrule
+          state: absent
+
+      # Test with action: members
+
+      - name: Ensure test hbacrule is present
+        ipahbacrule:
+          ipaadmin_password: SomeADMINpassword
+          name: testrule
+
+      - name: Check if hbacrule members present would trigger changes, mixed case
+        ipahbacrule:
+          ipaadmin_password: SomeADMINpassword
+          name: testrule
+          user:
+          - "{{ user_list[1] }}"
+          - "{{ user_list[2] }}"
+          group:
+          - "{{ group_list[1] }}"
+          - "{{ group_list[2] }}"
+          host:
+          - "{{ host_list[0] }}"
+          - "{{ host_list[1] }}"
+          hostgroup:
+          - "{{ hostgroup_list[0] }}"
+          hbacsvc:
+          - "{{ hbacsvc_list[0] }}"
+          - "{{ hbacsvc_list[1] }}"
+          hbacsvcgroup:
+          - "{{ hbacsvcgroup_list[0] }}"
+        check_mode: yes
+        register: result
+        failed_when: not result.changed or result.failed
+
+      - name: Ensure hbacrule members present, mixed case
+        ipahbacrule:
+          ipaadmin_password: SomeADMINpassword
+          name: testrule
+          user:
+          - "{{ user_list[1] }}"
+          - "{{ user_list[2] }}"
+          group:
+          - "{{ group_list[1] }}"
+          - "{{ group_list[2] }}"
+          host:
+          - "{{ host_list[0] }}"
+          - "{{ host_list[1] }}"
+          hostgroup:
+          - "{{ hostgroup_list[0] }}"
+          hbacsvc:
+          - "{{ hbacsvc_list[0] }}"
+          - "{{ hbacsvc_list[1] }}"
+          hbacsvcgroup:
+          - "{{ hbacsvcgroup_list[0] }}"
+          action: member
+        register: result
+        failed_when: not result.changed or result.failed
+
+      - name: Check if hbacrule members present would not trigger changes, mixed case
+        ipahbacrule:
+          ipaadmin_password: SomeADMINpassword
+          name: testrule
+          user:
+          - "{{ user_list[1] }}"
+          - "{{ user_list[2] }}"
+          group:
+          - "{{ group_list[1] }}"
+          - "{{ group_list[2] }}"
+          host:
+          - "{{ host_list[0] }}"
+          - "{{ host_list[1] }}"
+          hostgroup:
+          - "{{ hostgroup_list[0] }}"
+          hbacsvc:
+          - "{{ hbacsvc_list[0] }}"
+          - "{{ hbacsvc_list[1] }}"
+          hbacsvcgroup:
+          - "{{ hbacsvcgroup_list[0] }}"
+        check_mode: yes
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure hbacrule members present, lowercase
+        ipahbacrule:
+          ipaadmin_password: SomeADMINpassword
+          name: testrule
+          user:
+          - "{{ user_list[1] | lower }}"
+          - "{{ user_list[2] | lower }}"
+          group:
+          - "{{ group_list[1] | lower }}"
+          - "{{ group_list[2] | lower }}"
+          host:
+          - "{{ host_list[0] | lower }}"
+          - "{{ host_list[1] | lower }}"
+          hostgroup:
+          - "{{ hostgroup_list[0] | lower }}"
+          hbacsvc:
+          - "{{ hbacsvc_list[0] | lower }}"
+          - "{{ hbacsvc_list[1] | lower }}"
+          hbacsvcgroup:
+          - "{{ hbacsvcgroup_list[0] | lower }}"
+          action: member
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure hbacrule members present, upercase
+        ipahbacrule:
+          ipaadmin_password: SomeADMINpassword
+          name: testrule
+          user:
+          - "{{ user_list[1] | upper }}"
+          - "{{ user_list[2] | upper }}"
+          group:
+          - "{{ group_list[1] | upper }}"
+          - "{{ group_list[2] | upper }}"
+          host:
+          - "{{ host_list[0] | upper }}"
+          - "{{ host_list[1] | upper }}"
+          hostgroup:
+          - "{{ hostgroup_list[0] | upper }}"
+          hbacsvc:
+          - "{{ hbacsvc_list[0] | upper }}"
+          - "{{ hbacsvc_list[1] | upper }}"
+          hbacsvcgroup:
+          - "{{ hbacsvcgroup_list[0] | upper }}"
+          action: member
+        register: result
+        failed_when: result.changed or result.failed
+
+      # Test absent members
+
+      - name: Check if hbacrule members absent would trigger change, upercase
+        ipahbacrule:
+          ipaadmin_password: SomeADMINpassword
+          name: testrule
+          user:
+          - "{{ user_list[1] | upper }}"
+          - "{{ user_list[2] | upper }}"
+          group:
+          - "{{ group_list[1] | upper }}"
+          - "{{ group_list[2] | upper }}"
+          host:
+          - "{{ host_list[0] | upper }}"
+          - "{{ host_list[1] | upper }}"
+          hostgroup:
+          - "{{ hostgroup_list[0] | upper }}"
+          hbacsvc:
+          - "{{ hbacsvc_list[0] | upper }}"
+          - "{{ hbacsvc_list[1] | upper }}"
+          hbacsvcgroup:
+          - "{{ hbacsvcgroup_list[0] | upper }}"
+          action: member
+          state: absent
+        check_mode: yes
+        register: result
+        failed_when: not result.changed or result.failed
+
+      - name: Ensure hbacrule members are absent, upercase
+        ipahbacrule:
+          ipaadmin_password: SomeADMINpassword
+          name: testrule
+          user:
+          - "{{ user_list[1] | upper }}"
+          - "{{ user_list[2] | upper }}"
+          group:
+          - "{{ group_list[1] | upper }}"
+          - "{{ group_list[2] | upper }}"
+          host:
+          - "{{ host_list[0] | upper }}"
+          - "{{ host_list[1] | upper }}"
+          hostgroup:
+          - "{{ hostgroup_list[0] | upper }}"
+          hbacsvc:
+          - "{{ hbacsvc_list[0] | upper }}"
+          - "{{ hbacsvc_list[1] | upper }}"
+          hbacsvcgroup:
+          - "{{ hbacsvcgroup_list[0] | upper }}"
+          action: member
+          state: absent
+        register: result
+        failed_when: not result.changed or result.failed
+
+      - name: Check if hbacrule members absent would not trigger change, upercase
+        ipahbacrule:
+          ipaadmin_password: SomeADMINpassword
+          name: testrule
+          user:
+          - "{{ user_list[1] | upper }}"
+          - "{{ user_list[2] | upper }}"
+          group:
+          - "{{ group_list[1] | upper }}"
+          - "{{ group_list[2] | upper }}"
+          host:
+          - "{{ host_list[0] | upper }}"
+          - "{{ host_list[1] | upper }}"
+          hostgroup:
+          - "{{ hostgroup_list[0] | upper }}"
+          hbacsvc:
+          - "{{ hbacsvc_list[0] | upper }}"
+          - "{{ hbacsvc_list[1] | upper }}"
+          hbacsvcgroup:
+          - "{{ hbacsvcgroup_list[0] | upper }}"
+          action: member
+          state: absent
+        check_mode: yes
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure hbacrule members are absent, mixed case
+        ipahbacrule:
+          ipaadmin_password: SomeADMINpassword
+          name: testrule
+          user:
+          - "{{ user_list[1] }}"
+          - "{{ user_list[2] }}"
+          group:
+          - "{{ group_list[1] }}"
+          - "{{ group_list[2] }}"
+          host:
+          - "{{ host_list[0] }}"
+          - "{{ host_list[1] }}"
+          hostgroup:
+          - "{{ hostgroup_list[0] }}"
+          action: member
+          state: absent
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure hbacrule members are absent, lowercase
+        ipahbacrule:
+          ipaadmin_password: SomeADMINpassword
+          name: testrule
+          user:
+          - "{{ user_list[1] | lower }}"
+          - "{{ user_list[2] | lower }}"
+          group:
+          - "{{ group_list[1] | lower }}"
+          - "{{ group_list[2] | lower }}"
+          host:
+          - "{{ host_list[0] | lower }}"
+          - "{{ host_list[1] | lower }}"
+          hostgroup:
+          - "{{ hostgroup_list[0] | lower }}"
+          hbacsvc:
+          - "{{ hbacsvc_list[0] | lower }}"
+          - "{{ hbacsvc_list[1] | lower }}"
+          hbacsvcgroup:
+          - "{{ hbacsvcgroup_list[0] | lower }}"
+          action: member
+          state: absent
+        register: result
+        failed_when: result.changed or result.failed
+
+    always:
+      - name: Ensure test hbacrule is absent
+        ipahbacrule:
+          ipaadmin_password: SomeADMINpassword
+          name: testrule
+          state: absent
+
+      - name: Ensure test users are absent
+        ipauser:
+          ipaadmin_password: SomeADMINpassword
+          users:
+          - name: "{{ item }}"
+          state: absent
+        with_items: "{{ user_list }}"
+
+      - name: Ensure test groups are absent
+        ipagroup:
+          ipaadmin_password: SomeADMINpassword
+          name: "{{ item }}"
+          state: absent
+        with_items: "{{ group_list }}"
+
+      - name: Ensure test hosts are absent
+        ipahost:
+          ipaadmin_password: SomeADMINpassword
+          name: "{{ item }}.{{ ipaserver_domain }}"
+          state: absent
+        with_items: "{{ host_list }}"
+
+      - name: Ensure test hostgroups are absent
+        ipahostgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: "{{ item }}"
+          state: absent
+        with_items: "{{ hostgroup_list }}"
+
+      - name: Ensure test hbac services are absent
+        ipahbacsvc:
+          ipaadmin_password: SomeADMINpassword
+          name: "{{ item }}"
+          state: absent
+        with_items: "{{ hbacsvc_list }}"
+
+      - name: Ensure test hbac service groups are absent
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: "{{ item }}"
+          state: absent
+        with_items: "{{ hbacsvcgroup_list }}"

tests/hbacsvcgroup/test_hbacsvcgroup_member_case_insensitive.yml

@@ -0,0 +1,233 @@
+---
+- name: Test hbacsvcgroup member varying capitalization
+  hosts: "{{ ipa_test_host | default('ipaserver') }}"
+  become: no
+  gather_facts: no
+
+  vars:
+    hbacsvc_list:
+      - sVc1
+      - SvC2
+
+  tasks:
+  - block:
+      - name: Ensure test hbacsvcgroup is absent
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          state: absent
+
+      - name: Ensure test HBAC services are present
+        ipahbacsvc:
+          ipaadmin_password: SomeADMINpassword
+          name: "{{ item }}"
+        with_items: "{{ hbacsvc_list }}"
+
+      - name: Ensure test hbacsvcgroup is present with duplicate hbacsvc
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          hbacsvc:
+          - sVc1
+          - SvC1
+        register: result
+        failed_when: not result.changed or result.failed
+
+      - name: Ensure test hbacsvc is absent from hbacsvcgroup, with duplicate hbacsvc
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          hbacsvc:
+          - sVc1
+          - SvC1
+          action: member
+          state: absent
+        register: result
+        failed_when: not result.changed or result.failed
+
+      - name: Check if test hbacsvc absent, again, from hbacsvcgroup, with duplicate hbacsvc, would trigger changes
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          hbacsvc:
+          - svC1
+          - SVC1
+          action: member
+          state: absent
+        check_mode: yes
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure test hbacsvcgroup is absent
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          state: absent
+        register: result
+        failed_when: not result.changed or result.failed
+
+      - name: Check if hbacsvcgroup with members would trigger changes, mixed case
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          hbacsvc: "{{ hbacsvc_list }}"
+        check_mode: yes
+        register: result
+        failed_when: not result.changed or result.failed
+
+      - name: Ensure hbacsvcgroup is present with members, mixed case
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          hbacsvc: "{{ hbacsvc_list }}"
+        register: result
+        failed_when: not result.changed or result.failed
+
+      - name: Check if hbacsvcgroup with members would not trigger changes, mixed case
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          hbacsvc: "{{ hbacsvc_list }}"
+        check_mode: yes
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure hbacsvcgroup is present with members, lowercase
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          hbacsvc: "{{ hbacsvc_list | lower }}"
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure hbacsvcgroup is present with members, uppercase
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          hbacsvc: "{{ hbacsvc_list | upper }}"
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure test hbacsvcgroup is absent
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          state: absent
+
+      - name: Ensure test hbacsvcgroup is present
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+
+      - name: Check if hbacsvcgroup members would trigger changes, mixed case
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          hbacsvc: "{{ hbacsvc_list }}"
+          action: member
+        check_mode: yes
+        register: result
+        failed_when: not result.changed or result.failed
+
+      - name: Ensure hbacsvcgroup has members, mixed case
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          hbacsvc: "{{ hbacsvc_list }}"
+          action: member
+        register: result
+        failed_when: not result.changed or result.failed
+
+      - name: Check if hbacsvcgroup members would not trigger changes, mixed case
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          hbacsvc: "{{ hbacsvc_list }}"
+          action: member
+        check_mode: yes
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure hbacsvcgroup has members, lowercase
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          hbacsvc: "{{ hbacsvc_list | lower }}"
+          action: member
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure hbacsvcgroup has members, uppercase
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          hbacsvc: "{{ hbacsvc_list | upper }}"
+          action: member
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Check if hbacsvcgroup members absence would trigger changes, uppercase
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          hbacsvc: "{{ hbacsvc_list | upper }}"
+          action: member
+          state: absent
+        check_mode: yes
+        register: result
+        failed_when: not result.changed or result.failed
+
+      - name: Ensure hbacsvcgroup has members absent, uppercase
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          hbacsvc: "{{ hbacsvc_list | upper }}"
+          action: member
+          state: absent
+        register: result
+        failed_when: not result.changed or result.failed
+
+      - name: Check if hbacsvcgroup members absence would not trigger changes, uppercase
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          hbacsvc: "{{ hbacsvc_list | upper }}"
+          action: member
+          state: absent
+        check_mode: yes
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure hbacsvcgroup has members absent, mixed case
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          hbacsvc: "{{ hbacsvc_list }}"
+          action: member
+          state: absent
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure hbacsvcgroup has members absent, lowercase
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          hbacsvc: "{{ hbacsvc_list | lower }}"
+          action: member
+          state: absent
+        register: result
+        failed_when: result.changed or result.failed
+
+    always:
+      - name: Ensure test hbac service group is absent
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          state: absent
+
+      - name: Ensure test hbac services are absent
+        ipahbacsvc:
+          ipaadmin_password: SomeADMINpassword
+          ipaapi_context: "{{ ipa_context | default(omit) }}"
+          name: "{{ hbacsvc_list }}"
+          state: absent

tests/hostgroup/test_hostgroup.yml

@@ -133,6 +133,19 @@
     register: result
     failed_when: result.changed or result.failed
 
+  - name: Ensure hosts db1 and db2 (no FQDN) are member of host-group databases again
+    ipahostgroup:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      name: databases
+      state: present
+      host:
+      - db1
+      - db2
+      action: member
+    register: result
+    failed_when: result.changed or result.failed
+
   - name: Ensure host-group mysql-server is member of host-group databases
     ipahostgroup:
       ipaadmin_password: SomeADMINpassword

tests/role/test_role.yml

@@ -36,6 +36,7 @@
       ipaapi_context: "{{ ipa_context | default(omit) }}"
       name: renamerole
       rename: testrole
+      state: renamed
     register: result
     failed_when: not result.changed or result.failed
 
@@ -47,8 +48,9 @@
       ipaapi_context: "{{ ipa_context | default(omit) }}"
       name: renamerole
       rename: testrole
+      state: renamed
     register: result
-    failed_when: result.changed
+    failed_when: result.changed or (not result.failed and "No role 'renamerole'" not in result.msg)
 
   - name: Ensure role has member has privileges.
     iparole:

tests/role/test_role_member_case_insensitive.yml

@@ -0,0 +1,450 @@
+---
+- name: Test role members
+  hosts: "{{ ipa_test_host | default('ipaserver') }}"
+  become: no
+  gather_facts: no
+
+  vars:
+    user_list:
+      - User1
+      - uSer2
+      - usEr3
+    group_list:
+      - Group1
+      - gRoup2
+      - grOup3
+    host_list:
+      - HoSt01
+      - hOsT02
+    hostgroup_list:
+      - TestHostGroup
+    service_list:
+      - MySVC/host01
+
+  tasks:
+  - include_tasks: ../env_freeipa_facts.yml
+
+  - block:
+      # setup
+
+      - name: Ensure test role is absent
+        iparole:
+          ipaadmin_password: SomeADMINpassword
+          name: testrule
+          state: absent
+
+      - name: Ensure test users are present
+        ipauser:
+          ipaadmin_password: SomeADMINpassword
+          users:
+          - name: "{{ item }}"
+            first: First
+            last: Last
+        with_items: "{{ user_list }}"
+
+      - name: Ensure test groups are present
+        ipagroup:
+          ipaadmin_password: SomeADMINpassword
+          name: "{{ item }}"
+        with_items: "{{ group_list }}"
+
+      - name: Ensure test hosts are present
+        ipahost:
+          ipaadmin_password: SomeADMINpassword
+          name: "{{ item }}.{{ ipaserver_domain }}"
+          ip_address: 192.168.122.101
+          force: yes
+        with_items: "{{ host_list }}"
+
+      - name: Ensure test hostgroups are present
+        ipahostgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: "{{ item }}"
+        with_items: "{{ hostgroup_list }}"
+
+      - name: Ensure test services are present
+        ipaservice:
+          ipaadmin_password: SomeADMINpassword
+          name: "{{ item }}.{{ ipaserver_domain }}"
+        with_items: "{{ service_list }}"
+
+      # Test with action: hbacrule
+
+      - name: Check role present with members would trigger a change, mixed case
+        iparole:
+          ipaadmin_password: SomeADMINpassword
+          name: testrole
+          user:
+          - "{{ user_list[1] }}"
+          - "{{ user_list[2] }}"
+          group:
+          - "{{ group_list[1] }}"
+          - "{{ group_list[2] }}"
+          host:
+          - "{{ host_list[0] }}"
+          - "{{ host_list[1] }}"
+          hostgroup:
+          - "{{ hostgroup_list[0] }}"
+          service:
+          - "{{ service_list[0] }}.{{ ipaserver_domain }}"
+        check_mode: yes
+        register: result
+        failed_when: not result.changed or result.failed
+
+      - name: Ensure role is present with members, mixed case
+        iparole:
+          ipaadmin_password: SomeADMINpassword
+          name: testrole
+          user:
+          - "{{ user_list[1] }}"
+          - "{{ user_list[2] }}"
+          group:
+          - "{{ group_list[1] }}"
+          - "{{ group_list[2] }}"
+          host:
+          - "{{ host_list[0] }}"
+          - "{{ host_list[1] }}"
+          hostgroup:
+          - "{{ hostgroup_list[0] }}"
+          service:
+          - "{{ service_list[0] }}.{{ ipaserver_domain }}"
+        register: result
+        failed_when: not result.changed or result.failed
+
+      - name: Check role present with members would not trigger a change, mixed case
+        iparole:
+          ipaadmin_password: SomeADMINpassword
+          name: testrole
+          user:
+          - "{{ user_list[1] }}"
+          - "{{ user_list[2] }}"
+          group:
+          - "{{ group_list[1] }}"
+          - "{{ group_list[2] }}"
+          host:
+          - "{{ host_list[0] }}"
+          - "{{ host_list[1] }}"
+          hostgroup:
+          - "{{ hostgroup_list[0] }}"
+          service:
+          - "{{ service_list[0] }}.{{ ipaserver_domain }}"
+        check_mode: yes
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure role is present with members, lowercase
+        iparole:
+          ipaadmin_password: SomeADMINpassword
+          name: testrole
+          user:
+          - "{{ user_list[1] | lower }}"
+          - "{{ user_list[2] | lower }}"
+          group:
+          - "{{ group_list[1] | lower }}"
+          - "{{ group_list[2] | lower }}"
+          host:
+          - "{{ host_list[0] | lower }}"
+          - "{{ host_list[1] | lower }}"
+          hostgroup:
+          - "{{ hostgroup_list[0] | lower }}"
+          service:
+          - "{{ (service_list[0] + '.' + ipaserver_domain) | lower }}"
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure role is present with members, upercase
+        iparole:
+          ipaadmin_password: SomeADMINpassword
+          name: testrole
+          user:
+          - "{{ user_list[1] | upper }}"
+          - "{{ user_list[2] | upper }}"
+          group:
+          - "{{ group_list[1] | upper }}"
+          - "{{ group_list[2] | upper }}"
+          host:
+          - "{{ host_list[0] | upper }}"
+          - "{{ host_list[1] | upper }}"
+          hostgroup:
+          - "{{ hostgroup_list[0] | upper }}"
+          service:
+          - "{{ (service_list[0] + '.' + ipaserver_domain) | upper }}"
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure test role is absent
+        iparole:
+          ipaadmin_password: SomeADMINpassword
+          name: testrole
+          state: absent
+
+      # Test with action: members
+
+      - name: Ensure test role is present
+        iparole:
+          ipaadmin_password: SomeADMINpassword
+          name: testrole
+
+      - name: Check role members present would trigger a change, mixed case
+        iparole:
+          ipaadmin_password: SomeADMINpassword
+          name: testrole
+          user:
+          - "{{ user_list[1] }}"
+          - "{{ user_list[2] }}"
+          group:
+          - "{{ group_list[1] }}"
+          - "{{ group_list[2] }}"
+          host:
+          - "{{ host_list[0] }}"
+          - "{{ host_list[1] }}"
+          hostgroup:
+          - "{{ hostgroup_list[0] }}"
+          service:
+          - "{{ service_list[0] }}.{{ ipaserver_domain }}"
+          action: member
+        check_mode: yes
+        register: result
+        failed_when: not result.changed or result.failed
+
+      - name: Ensure role is present with members, mixed case
+        iparole:
+          ipaadmin_password: SomeADMINpassword
+          name: testrole
+          user:
+          - "{{ user_list[1] }}"
+          - "{{ user_list[2] }}"
+          group:
+          - "{{ group_list[1] }}"
+          - "{{ group_list[2] }}"
+          host:
+          - "{{ host_list[0] }}"
+          - "{{ host_list[1] }}"
+          hostgroup:
+          - "{{ hostgroup_list[0] }}"
+          service:
+          - "{{ service_list[0] }}.{{ ipaserver_domain }}"
+          action: member
+        register: result
+        failed_when: not result.changed or result.failed
+
+      - name: Check role members present would  not trigger a change, mixed case
+        iparole:
+          ipaadmin_password: SomeADMINpassword
+          name: testrole
+          user:
+          - "{{ user_list[1] }}"
+          - "{{ user_list[2] }}"
+          group:
+          - "{{ group_list[1] }}"
+          - "{{ group_list[2] }}"
+          host:
+          - "{{ host_list[0] }}"
+          - "{{ host_list[1] }}"
+          hostgroup:
+          - "{{ hostgroup_list[0] }}"
+          service:
+          - "{{ service_list[0] }}.{{ ipaserver_domain }}"
+          action: member
+        check_mode: yes
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure role is present with members, lowercase
+        iparole:
+          ipaadmin_password: SomeADMINpassword
+          name: testrole
+          user:
+          - "{{ user_list[1] | lower }}"
+          - "{{ user_list[2] | lower }}"
+          group:
+          - "{{ group_list[1] | lower }}"
+          - "{{ group_list[2] | lower }}"
+          host:
+          - "{{ host_list[0] | lower }}"
+          - "{{ host_list[1] | lower }}"
+          hostgroup:
+          - "{{ hostgroup_list[0] | lower }}"
+          service:
+          - "{{ (service_list[0] + '.' + ipaserver_domain) | lower }}"
+          action: member
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure role is present with members, upercase
+        iparole:
+          ipaadmin_password: SomeADMINpassword
+          name: testrole
+          user:
+          - "{{ user_list[1] | upper }}"
+          - "{{ user_list[2] | upper }}"
+          group:
+          - "{{ group_list[1] | upper }}"
+          - "{{ group_list[2] | upper }}"
+          host:
+          - "{{ host_list[0] | upper }}"
+          - "{{ host_list[1] | upper }}"
+          hostgroup:
+          - "{{ hostgroup_list[0] | upper }}"
+          service:
+          - "{{ (service_list[0] + '.' + ipaserver_domain) | upper }}"
+          action: member
+        register: result
+        failed_when: result.changed or result.failed
+
+      # # Test absent members
+      - name: Check role members absent would trigger a change, upercase
+        iparole:
+          ipaadmin_password: SomeADMINpassword
+          name: testrole
+          user:
+          - "{{ user_list[1] | upper }}"
+          - "{{ user_list[2] | upper }}"
+          group:
+          - "{{ group_list[1] | upper }}"
+          - "{{ group_list[2] | upper }}"
+          host:
+          - "{{ host_list[0] | upper }}"
+          - "{{ host_list[1] | upper }}"
+          hostgroup:
+          - "{{ hostgroup_list[0] | upper }}"
+          service:
+          - "{{ (service_list[0] + '.' + ipaserver_domain) | upper }}"
+          action: member
+          state: absent
+        check_mode: yes
+        register: result
+        failed_when: not result.changed or result.failed
+
+      - name: Ensure role members are absent, upercase
+        iparole:
+          ipaadmin_password: SomeADMINpassword
+          name: testrole
+          user:
+          - "{{ user_list[1] | upper }}"
+          - "{{ user_list[2] | upper }}"
+          group:
+          - "{{ group_list[1] | upper }}"
+          - "{{ group_list[2] | upper }}"
+          host:
+          - "{{ host_list[0] | upper }}"
+          - "{{ host_list[1] | upper }}"
+          hostgroup:
+          - "{{ hostgroup_list[0] | upper }}"
+          service:
+          - "{{ (service_list[0] + '.' + ipaserver_domain) | upper }}"
+          action: member
+          state: absent
+        register: result
+        failed_when: not result.changed or result.failed
+
+      - name: Check role members absent would not trigger a change, upercase
+        iparole:
+          ipaadmin_password: SomeADMINpassword
+          name: testrole
+          user:
+          - "{{ user_list[1] | upper }}"
+          - "{{ user_list[2] | upper }}"
+          group:
+          - "{{ group_list[1] | upper }}"
+          - "{{ group_list[2] | upper }}"
+          host:
+          - "{{ host_list[0] | upper }}"
+          - "{{ host_list[1] | upper }}"
+          hostgroup:
+          - "{{ hostgroup_list[0] | upper }}"
+          service:
+          - "{{ (service_list[0] + '.' + ipaserver_domain) | upper }}"
+          action: member
+          state: absent
+        check_mode: yes
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure role members are absent, mixed case
+        iparole:
+          ipaadmin_password: SomeADMINpassword
+          name: testrole
+          user:
+          - "{{ user_list[1] }}"
+          - "{{ user_list[2] }}"
+          group:
+          - "{{ group_list[1] }}"
+          - "{{ group_list[2] }}"
+          host:
+          - "{{ host_list[0] }}"
+          - "{{ host_list[1] }}"
+          hostgroup:
+          - "{{ hostgroup_list[0] }}"
+          service:
+          - "{{ service_list[0] }}.{{ ipaserver_domain }}"
+          action: member
+          state: absent
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure role members are absent, lowercase
+        iparole:
+          ipaadmin_password: SomeADMINpassword
+          name: testrole
+          user:
+          - "{{ user_list[1] | lower }}"
+          - "{{ user_list[2] | lower }}"
+          group:
+          - "{{ group_list[1] | lower }}"
+          - "{{ group_list[2] | lower }}"
+          host:
+          - "{{ host_list[0] | lower }}"
+          - "{{ host_list[1] | lower }}"
+          hostgroup:
+          - "{{ hostgroup_list[0] | lower }}"
+          service:
+          - "{{ (service_list[0] + '.' + ipaserver_domain) | lower }}"
+          action: member
+          state: absent
+        register: result
+        failed_when: result.changed or result.failed
+
+    always:
+      - name: Ensure test role is absent
+        iparole:
+          ipaadmin_password: SomeADMINpassword
+          name: testrole
+          state: absent
+
+      - name: Ensure test users are absent
+        ipauser:
+          ipaadmin_password: SomeADMINpassword
+          users:
+          - name: "{{ item }}"
+          state: absent
+        with_items: "{{ user_list }}"
+
+      - name: Ensure test groups are absent
+        ipagroup:
+          ipaadmin_password: SomeADMINpassword
+          name: "{{ item }}"
+          state: absent
+        with_items: "{{ group_list }}"
+
+      - name: Ensure test hosts are absent
+        ipahost:
+          ipaadmin_password: SomeADMINpassword
+          name: "{{ item }}.{{ ipaserver_domain }}"
+          state: absent
+        with_items: "{{ host_list }}"
+
+      - name: Ensure test hostgroups are absent
+        ipahostgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: "{{ item }}"
+          state: absent
+        with_items: "{{ hostgroup_list }}"
+
+      - name: Ensure test services are absent
+        ipaservice:
+          ipaadmin_password: SomeADMINpassword
+          name: "{{ item }}.{{ ipaserver_domain }}"
+          continue: yes
+          state: absent
+        with_items: "{{ service_list }}"

tests/sanity/galaxy-importer.cfg

@@ -0,0 +1,5 @@
+[galaxy-importer]
+RUN_ANSIBLE_TEST = True
+RUN_ANSIBLE_LINT = True
+ANSIBLE_TEST_LOCAL_IMAGE = True
+LOCAL_IMAGE_DOCKER = True

tests/sanity/ignore-2.12.txt

@@ -18,11 +18,15 @@ plugins/modules/ipadnsrecord.py pylint:use-maxsplit-arg
 plugins/modules/ipareplica_enable_ipa.py pylint:ansible-format-automatic-specification
 plugins/modules/ipareplica_prepare.py pylint:ansible-format-automatic-specification
 plugins/modules/ipareplica_test.py pylint:ansible-format-automatic-specification
+plugins/modules/iparole.py compile-2.6!skip
+plugins/modules/iparole.py import-2.6!skip
 plugins/modules/ipaserver_setup_ca.py compile-2.6!skip
 plugins/modules/ipaserver_setup_ca.py import-2.6!skip
 plugins/modules/ipaserver_test.py pylint:ansible-format-automatic-specification
 plugins/modules/ipaservice.py compile-2.6!skip
 plugins/modules/ipaservice.py import-2.6!skip
+plugins/modules/ipasudorule.py compile-2.6!skip
+plugins/modules/ipasudorule.py import-2.6!skip
 plugins/modules/ipavault.py compile-2.6!skip
 plugins/modules/ipavault.py import-2.6!skip
 roles/ipaclient/library/ipaclient_api.py pylint:ansible-format-automatic-specification
@@ -35,6 +39,7 @@ roles/ipaserver/library/ipaserver_test.py pylint:ansible-format-automatic-specif
 roles/ipareplica/module_utils/ansible_ipa_replica.py pylint:ansible-format-automatic-specification
 tests/external-signed-ca-with-automatic-copy/external-ca.sh shebang!skip
 tests/pytests/conftest.py pylint:ansible-format-automatic-specification
+tests/sanity/sanity.sh shebang!skip
 tests/user/users.sh shebang!skip
 tests/user/users_absent.sh shebang!skip
 tests/utils.py pylint:ansible-format-automatic-specification

tests/sanity/sanity.sh

@@ -0,0 +1,36 @@
+#!/bin/bash
+
+VENV=/tmp/ansible-test-venv
+ANSIBLE_COLLECTION=freeipa-ansible_freeipa
+
+virtualenv "$VENV"
+# shellcheck disable=SC1091
+source "$VENV"/bin/activate
+
+python -m pip install --upgrade pip
+pip install galaxy_importer
+
+rm -f "$ANSIBLE_COLLECTION"-*.tar.gz
+rm -f importer_result.json
+
+utils/build-galaxy-release.sh
+
+export GALAXY_IMPORTER_CONFIG=tests/sanity/galaxy-importer.cfg
+
+collection=$(ls -1 "$ANSIBLE_COLLECTION"-*.tar.gz)
+echo "Running: python -m galaxy_importer.main $collection"
+
+error=0
+while read -r line;
+do
+    if [[ $line == ERROR* ]]; then
+        ((error++))
+        echo -e "\033[31;1m${line}\033[0m"
+    else
+        echo "$line"
+    fi
+done < <(python -m galaxy_importer.main "$collection")
+
+rm -rf "$VENV"
+
+exit "$error"

tests/sudorule/test_sudorule.yml

@@ -58,6 +58,7 @@
       name:
           - /sbin/ifconfig
           - /usr/bin/vim
+          - /usr/bin/emacs
       state: present
 
   - name: Ensure sudocmdgroup is available
@@ -68,6 +69,14 @@
       sudocmd: /usr/bin/vim
       state: present
 
+  - name: Ensure sudocmdgroup is available
+    ipasudocmdgroup:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      name: test_sudorule2
+      sudocmd: /usr/bin/emacs
+      state: present
+
   - name: Ensure sudorules are absent
     ipasudorule:
       ipaadmin_password: SomeADMINpassword
@@ -606,6 +615,7 @@
       ipaapi_context: "{{ ipa_context | default(omit) }}"
       name: testrule1
       allow_sudocmdgroup: test_sudorule
+      action: member
       state: present
     register: result
     failed_when: not result.changed or result.failed
@@ -616,6 +626,7 @@
       ipaapi_context: "{{ ipa_context | default(omit) }}"
       name: testrule1
       allow_sudocmdgroup: test_sudorule
+      action: member
       state: present
     register: result
     failed_when: result.changed or result.failed
@@ -648,6 +659,7 @@
       ipaapi_context: "{{ ipa_context | default(omit) }}"
       name: testrule1
       deny_sudocmdgroup: test_sudorule
+      action: member
       state: present
     register: result
     failed_when: not result.changed or result.failed
@@ -658,6 +670,7 @@
       ipaapi_context: "{{ ipa_context | default(omit) }}"
       name: testrule1
       deny_sudocmdgroup: test_sudorule
+      action: member
       state: present
     register: result
     failed_when: result.changed or result.failed
@@ -684,6 +697,114 @@
     register: result
     failed_when: result.changed or result.failed
 
+  - name: Ensure sudorule is present, with `test_sudorule` sudocmdgroup in allow_sudocmdgroup.
+    ipasudorule:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      name: testrule1
+      allow_sudocmdgroup: test_sudorule
+      state: present
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure sudorule is present, with `test_sudorule2` sudocmdgroup in allow_sudocmdgroup.
+    ipasudorule:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      name: testrule1
+      allow_sudocmdgroup: test_sudorule2
+      state: present
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure sudorule is present, with both sudocmdgroup in allow_sudocmdgroup.
+    ipasudorule:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      name: testrule1
+      allow_sudocmdgroup:
+        - test_sudorule
+        - test_sudorule2
+      state: present
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure sudorule is present, with both sudocmdgroup, again.
+    ipasudorule:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      name: testrule1
+      allow_sudocmdgroup:
+        - test_sudorule
+        - test_sudorule2
+      state: present
+    register: result
+    failed_when: result.changed or result.failed
+
+  - name: Ensure sudorule is present, with only `test_sudorule` sudocmdgroup in allow_sudocmdgroup.
+    ipasudorule:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      name: testrule1
+      allow_sudocmdgroup: test_sudorule
+      state: present
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure sudorule is present, with `test_sudorule` sudocmdgroup in deny_sudocmdgroup.
+    ipasudorule:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      name: testrule1
+      deny_sudocmdgroup: test_sudorule
+      state: present
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure sudorule is present, with `test_sudorule2` sudocmdgroup in deny_sudocmdgroup.
+    ipasudorule:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      name: testrule1
+      deny_sudocmdgroup: test_sudorule2
+      state: present
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure sudorule is present, with both sudocmdgroup in deny_sudocmdgroup.
+    ipasudorule:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      name: testrule1
+      deny_sudocmdgroup:
+        - test_sudorule
+        - test_sudorule2
+      state: present
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure sudorule is present, with both sudocmdgroup, again.
+    ipasudorule:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      name: testrule1
+      deny_sudocmdgroup:
+        - test_sudorule
+        - test_sudorule2
+      state: present
+    register: result
+    failed_when: result.changed or result.failed
+
+  - name: Ensure sudorule is present, with only `test_sudorule` sudocmdgroup in deny_sudocmdgroup.
+    ipasudorule:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      name: testrule1
+      deny_sudocmdgroup: test_sudorule
+      state: present
+    register: result
+    failed_when: not result.changed or result.failed
+
   - name: Ensure sudorule is absent
     ipasudorule:
       ipaadmin_password: SomeADMINpassword
@@ -889,7 +1010,9 @@
     ipasudocmdgroup:
       ipaadmin_password: SomeADMINpassword
       ipaapi_context: "{{ ipa_context | default(omit) }}"
-      name: test_sudorule
+      name:
+      - test_sudorule
+      - test_sudorule2
       state: absent
 
   - name: Ensure sudocmds are absent
@@ -899,6 +1022,7 @@
       name:
       - /sbin/ifconfig
       - /usr/bin/vim
+      - /usr/bin/emacs
       state: absent
 
   - name: Ensure sudorules are absent