Merge pull request #511 from t-woerner/ipaclient_otp_rmkeytab_error#7

ipaclient: Do not fail on rmkeytab error #7

.github/workflows/docs.yml

@@ -0,0 +1,16 @@
+---
+name: Verify Ansible documentation.
+on:
+  - push
+  - pull_request
+jobs:
+  check_docs:
+    name: Check Ansible Documentation.
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions/setup-python@v2
+        with:
+          python-version: '3.x'
+      - name: Run ansible-doc-test
+        run: ANSIBLE_LIBRARY="." python utils/ansible-doc-test roles plugins

.github/workflows/lint.yml

@@ -30,4 +30,4 @@ jobs:
         uses: ibiqlik/action-yamllint@v1
 
       - name: Run Python linters
-        uses: rjeffman/python-lint-action@master
+        uses: rjeffman/python-lint-action@v2

.pre-commit-config.yaml

@@ -0,0 +1,31 @@
+---
+repos:
+- repo: https://github.com/ansible/ansible-lint.git
+  rev: v4.3.5
+  hooks:
+  - id: ansible-lint
+    always_run: false
+    pass_filenames: true
+    files: \.(yaml|yml)$
+    entry: env ANSIBLE_LIBRARY=./plugins/modules ANSIBLE_MODULE_UTILS=./plugins/module_utils ansible-lint --force-color
+- repo: https://github.com/adrienverge/yamllint.git
+  rev: v1.25.0
+  hooks:
+  - id: yamllint
+    files: \.(yaml|yml)$
+- repo: https://gitlab.com/pycqa/flake8
+  rev: 3.8.4
+  hooks:
+  - id: flake8
+- repo: https://gitlab.com/pycqa/pydocstyle
+  rev: 5.1.1
+  hooks:
+  - id: pydocstyle
+- repo: local
+  hooks:
+  - id: ansible-doc-test
+    name: Verify Ansible roles and module documentation.
+    language: script
+    entry: utils/ansible-doc-test
+    # args: ['-v', 'roles', 'plugins']
+    files: ^.*.py$

CONTRIBUTING.md

@@ -0,0 +1,121 @@
+Contributing to ansible-freeipa
+===============================
+
+As part of the [FreeIPA] project, ansible-freeipa follows
+[FreeIPA's Code of Conduct].
+
+
+Reporting bugs or Features
+--------------------------
+
+ansible-freeipa uses [Github issues] for the upstream development, so all RFEs
+and bug reports should be added there.
+
+If you have questions about the usage of ansible-freeipa modules and roles,
+you should also submit an issue, so that anyone that knows an answer can help.
+
+
+Development
+-----------
+
+Contribute code by submitting a [pull request]. All pull requests should be
+created against the `master` branch. If your PR fixes an open issue, please,
+add this information to the commit message, like _"Fix issue #num"_.
+
+Every PR will have to pass some automatic checks and be reviewed by another
+developer(s). Once they are approved, they will be merged.
+
+In your commits, use clear messages that include intent, summary of changes,
+and expected result. Use a template commit message [for modules] and
+[for roles].
+
+Upon review, it is fine to `force push` the changes.
+
+**Preparing the development environment**
+
+There are some useful tools that will help you develop for ansible-freeipa,
+and you should install, at least, the modules in `requirements.txt`. You
+can install the modules with your distribution package manager, or use pip,
+as in the example:
+
+```
+python3 -m pip install --user -r requirements-dev.txt
+```
+
+We recommend using [pre-commit] so that the basic checks that will be executed
+for your PR are executed locally, on your commits. To setup the pre-commit
+hooks, issue the command:
+
+```
+pre-commit install
+```
+
+**Developing new modules**
+
+When developing new modules use the script `utils/new_module`. If the module
+should have `action: member` support, use the flag `-m`.
+
+This script will create the basic structure for the module, the required files
+for tests, playbooks, documentation and source code, all at the appropriate
+places.
+
+
+**Other helpfull tools**
+
+Under directory `utils`, you will find other useful tools, like
+**lint-check.sh**, which will run the Python and YAML linters on your code,
+and **ansible-doc-test** which will verify if the documentation added to the
+roles and modules source code has the right format.
+
+
+Testing
+-------
+
+When testing ansible-freeipa's roles and modules, we aim to check if they
+do what they intend to do, report the results correctly, and if they are
+idempotent (although, sometimes the operation performed is not, like when
+renaming items). To achieve this, we use Ansible playbooks.
+
+The Ansible playbooks test can be found under the [tests] directory. They
+should test the behavior of the module or role, and, if possible, provide
+test cases for all attributes.
+
+There might be some limitation on the testing environment, as some attributes
+or operations are only available in some circumstances, like specific FreeIPA
+versions, or some more elaborate scenarios (for example, requiring a
+configured trust to an AD domain). For these cases, there are some `facts`
+available that will only enable the tests if the testing environment is
+enabled.
+
+The tests run automatically on every pull request, using Fedora, CentOS 7,
+and CentOS 8 environments.
+
+See the document [Running the tests] and also the section `Preparing the
+development environment`, to prepare your environment.
+
+
+Documentation
+-------------
+
+We do our best to provide a correct and complete documentation for the modules
+and roles we provide, but we sometimes miss something that users find it
+important to be documented.
+
+If you think something could be made easier to understand, or found an error
+or omission in the documentation, fixing it will help other users and make
+the experience on using the project much better.
+
+Also, the [playbooks] can be seen as part of the documentation, as they are
+examples of commonly performed tasks.
+
+---
+[FreeIPA]: https://freeipa.org
+[FreeIPA's Code of Conduct]: https://github.com/freeipa/freeipa/blob/master/CODE_OF_CONDUCT.md
+[for modules]: https://github.com/freeipa/ansible-freeipa/pull/357
+[for roles]: https://github.com/freeipa/ansible-freeipa/pull/430
+[Github issues]: https://github.com/freeipa/ansible-freeipa/issues
+[pull request]: https://github.com/freeipa/ansible-freeipa/pulls
+[playbooks]: playbooks
+[pre-commit]: https://pre-commit.com
+[Running the tests]: tests/README.md
+[tests]: tests/

README-dnsforwardzone.md

@@ -47,13 +47,13 @@ Example playbook to ensure presence of a forwardzone to ipa DNS:
   become: true
 
   tasks:
-  - name: ensure presence of forwardzone for DNS requests for example.com to 8.8.8.8
+  - name: ensure presence of forwardzone with a single forwarder DNS server
     ipadnsforwardzone:
       ipaadmin_password: SomeADMINpassword
       state: present
       name: example.com
       forwarders:
-        - 8.8.8.8
+        - ip_address: 8.8.8.8
       forwardpolicy: first
       skip_overlap_check: true
 
@@ -63,14 +63,14 @@ Example playbook to ensure presence of a forwardzone to ipa DNS:
       name: example.com
       state: disabled
 
-  - name: ensure presence of multiple upstream DNS servers for example.com
+  - name: ensure presence of forwardzone with multiple forwarder DNS server
     ipadnsforwardzone:
       ipaadmin_password: SomeADMINpassword
       state: present
       name: example.com
       forwarders:
-        - 8.8.8.8
-        - 4.4.4.4
+        - ip_address: 8.8.8.8
+        - ip_address: 4.4.4.4
 
   - name: ensure presence of another forwarder to any existing ones for example.com
     ipadnsforwardzone:
@@ -78,10 +78,19 @@ Example playbook to ensure presence of a forwardzone to ipa DNS:
       state: present
       name: example.com
       forwarders:
-        - 1.1.1.1
+        - ip_address: 1.1.1.1
       action: member
 
-  - name: ensure the forwarder for example.com does not exists (delete it if needed)
+  - name: ensure presence of forwardzone with single forwarder DNS server on non-stardard port
+    ipadnsforwardzone:
+      ipaadmin_password: SomeADMINpassword
+      state: present
+      name: example.com
+      forwarders:
+        - ip_address: 4.4.4.4
+          port: 8053
+
+  - name: ensure the forward zone is absent
     ipadnsforwardzone:
       ipaadmin_password: SomeADMINpassword
       name: example.com

README-group.md

@@ -109,6 +109,24 @@ Example playbook to add group members to a group:
       - appops
 ```
 
+Example playbook to add members from a trusted realm to an external group:
+
+```yaml
+--
+- name: Playbook to handle groups.
+  hosts: ipaserver
+  became: true
+
+  - name: Create an external group and add members from a trust to it.
+    ipagroup:
+      ipaadmin_password: SomeADMINpassword
+      name: extgroup
+      external: yes
+      externalmember:
+      - WINIPA\\Web Users
+      - WINIPA\\Developers
+```
+
 Example playbook to remove groups:
 
 ```yaml
@@ -148,6 +166,7 @@ Variable | Description | Required
 `service` | List of service name strings assigned to this group. Only usable with IPA versions 4.7 and up. | no
 `membermanager_user` | List of member manager users assigned to this group. Only usable with IPA versions 4.8.4 and up. | no
 `membermanager_group` | List of member manager groups assigned to this group. Only usable with IPA versions 4.8.4 and up. | no
+`externalmember` \| `ipaexternalmember`  \| `external_member`| List of members of a trusted domain in DOM\\name or name@domain form. | no
 `action` | Work on group or member level. It can be on of `member` or `group` and defaults to `group`. | no
 `state` | The state to ensure. It can be one of `present` or `absent`, default: `present`. | yes
 

README-permission.md

@@ -0,0 +1,188 @@
+Permission module
+============
+
+Description
+-----------
+
+The permission module allows to ensure presence and absence of permissions and permission members.
+
+Features
+--------
+
+* Permission management
+
+
+Supported FreeIPA Versions
+--------------------------
+
+FreeIPA versions 4.4.0 and up are supported by the ipapermission 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 make sure permission "MyPermission" is present:
+
+```yaml
+---
+- name: Playbook to handle IPA permissions
+  hosts: ipaserver
+  become: yes
+
+  tasks:
+  - name: Ensure permission MyPermission is present
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      name: MyPermission
+      object_type: host
+      right: all
+```
+
+
+Example playbook to ensure permission "MyPermission" is present with attr carlicense:
+
+```yaml
+---
+- name: Playbook to handle IPA permissions
+  hosts: ipaserver
+  become: yes
+
+  tasks:
+  - name: Ensure permission "MyPermission" is present with attr carlicense
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      name: MyPermission
+      object_type: host
+      right: all
+      attrs:
+      - carlicense
+```
+
+
+Example playbook to ensure attr gecos is present in permission "MyPermission":
+
+```yaml
+---
+- name: Playbook to handle IPA permissions
+  hosts: ipaserver
+  become: yes
+
+  tasks:
+  - name: Ensure attr gecos is present in permission "MyPermission"
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      name: MyPermission
+      attrs:
+      - gecos
+      action: member
+```
+
+
+Example playbook to ensure attr gecos is absent in permission "MyPermission":
+
+```yaml
+---
+- name: Playbook to handle IPA permissions
+  hosts: ipaserver
+  become: yes
+
+  tasks:
+  - name: Ensure attr gecos is present in permission "MyPermission"
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      name: MyPermission
+      attrs:
+      - gecos
+      action: member
+      state: absent
+```
+
+
+Example playbook to make sure permission "MyPermission" is absent:
+
+```yaml
+---
+- name: Playbook to handle IPA permissions
+  hosts: ipaserver
+  become: yes
+
+  tasks:
+  - name: Ensure permission "MyPermission" is absent
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      name: MyPermission
+      state: absent
+```
+
+
+Example playbook to make sure permission "MyPermission" is renamed to "MyNewPermission":
+
+```yaml
+---
+- name: Playbook to handle IPA permissions
+  hosts: ipaserver
+  become: yes
+
+  tasks:
+  - name: Ensure permission "MyPermission" is renamed to "MyNewPermission
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      name: MyPermission
+      rename: MyNewPermission
+      state: renamed
+```
+
+
+Variables
+---------
+
+ipapermission
+-------
+
+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` \| `cn` | The permission name string. | yes
+`right` \| `ipapermright` | Rights to grant. It can be a list of one or more of `read`, `search`, `compare`, `write`, `add`, `delete`, and `all` default: `all` | no
+`attrs` | All attributes to which the permission applies. | no
+`bindtype` \| `ipapermbindruletype` | Bind rule type. It can be one of `permission`, `all`, `self`, or `anonymous` defaults to `permission` for new permissions. Bind rule type `self` can only be used on IPA versions 4.8.7 or up.| no
+`subtree` \| `ipapermlocation` | Subtree to apply permissions to | no
+`filter` \| `extratargetfilter` | Extra target filter | no
+`rawfilter` \| `ipapermtargetfilter` | All target filters | no
+`target` \| `ipapermtarget` | Optional DN to apply the permission to | no
+`targetto` \| `ipapermtargetto` | Optional DN subtree where an entry can be moved to | no
+`targetfrom` \| `ipapermtargetfrom` | Optional DN subtree from where an entry can be moved | no
+`memberof` | Target members of a group (sets memberOf targetfilter) | no
+`targetgroup` | User group to apply permissions to (sets target) | no
+`object_type` | Type of IPA object (sets subtree and objectClass targetfilter) | no
+`no_members` | Suppress processing of membership | no
+`rename` | Rename the permission object | no
+`action` | Work on permission or member level. It can be on of `member` or `permission` and defaults to `permission`. | no
+`state` | The state to ensure. It can be one of `present`, `absent`, or `renamed` default: `present`. | no
+
+The `includedattrs` and `excludedattrs` variables are only usable for managed permisions and are not exposed by the module. Using `attrs` for managed permissions will result in the automatic generation of `includedattrs` and `excludedattrs` in the IPA server.
+
+
+Authors
+=======
+
+Seth Kress

README-vault.md

@@ -130,7 +130,7 @@ Example playbook to make sure vault data is present in a symmetric vault:
       action: member
 ```
 
-Example playbook to retrieve vault data from a symmetric vault:
+When retrieving data from a vault, it is recommended that `no_log: yes` is used, so that sensitive data stored in a vault is not logged by Ansible. The data is returned in a dict `vault`, in the field `data` (e.g. `result.vault.data`). An example playbook to retrieve data from a symmetric vault:
 
 ```yaml
 ---
@@ -139,12 +139,19 @@ Example playbook to retrieve vault data from a symmetric vault:
   become: true
 
   tasks:
-  - ipavault:
+  - name: Retrieve data from vault and register it in 'ipavault'
+    ipavault:
       ipaadmin_password: SomeADMINpassword
       name: symvault
       username: admin
       password: SomeVAULTpassword
       state: retrieved
+    no_log: yes
+    register: ipavault
+
+  - name: Print retrieved data from vault
+    debug:
+      var: ipavault.vault.data
 ```
 
 Example playbook to make sure vault data is absent in a symmetric vault:

README.md

@@ -11,6 +11,10 @@ Features
 * Cluster deployments: Server, replicas and clients in one playbook
 * One-time-password (OTP) support for client installation
 * Repair mode for clients
+* Backup and restore, also to and from controller
+* Modules for config management
+* Modules for delegation management
+* Modules for dns config management
 * Modules for dns forwarder management
 * Modules for dns record management
 * Modules for dns zone management
@@ -20,8 +24,12 @@ Features
 * Modules for hbacsvcgroup management
 * Modules for host management
 * Modules for hostgroup management
+* Modules for location management
+* Modules for permission management
+* Modules for privilege management
 * Modules for pwpolicy management
 * Modules for role management
+* Modules for self service management
 * Modules for service management
 * Modules for sudocmd management
 * Modules for sudocmdgroup management
@@ -146,7 +154,7 @@ ipaserver_domain=test.local
 ipaserver_realm=TEST.LOCAL
 ```
 
-The admin principle is ```admin``` by default. Please set ```ipaadmin_principal``` if you need to change it.
+The admin principal is ```admin``` by default. Please set ```ipaadmin_principal``` if you need to change it.
 
 You can also add more setting here, like for example to enable the DNS server or to set auto-forwarders:
 ```yaml
@@ -408,10 +416,13 @@ Roles
 * [Server](roles/ipaserver/README.md)
 * [Replica](roles/ipareplica/README.md)
 * [Client](roles/ipaclient/README.md)
+* [Backup](roles/ipabackup/README.md)
 
 Modules in plugin/modules
 =========================
 
+* [ipaconfig](README-config.md)
+* [ipadelegation](README-delegation.md)
 * [ipadnsconfig](README-dnsconfig.md)
 * [ipadnsforwardzone](README-dnsforwardzone.md)
 * [ipadnsrecord](README-dnsrecord.md)
@@ -422,8 +433,12 @@ Modules in plugin/modules
 * [ipahbacsvcgroup](README-hbacsvc.md)
 * [ipahost](README-host.md)
 * [ipahostgroup](README-hostgroup.md)
+* [ipalocation](README-ipalocation.md)
+* [ipapermission](README-ipapermission.md)
+* [ipaprivilege](README-ipaprivilege.md)
 * [ipapwpolicy](README-pwpolicy.md)
 * [iparole](README-role.md)
+* [ipaselfservice](README-ipaselfservice.md)
 * [ipaservice](README-service.md)
 * [ipasudocmd](README-sudocmd.md)
 * [ipasudocmdgroup](README-sudocmdgroup.md)

molecule/centos-8-build/molecule.yml

@@ -3,7 +3,7 @@ driver:
   name: docker
 platforms:
   - name: centos-8-build
-    image: centos:8
+    image: "centos:centos8"
     pre_build_image: true
     hostname: ipaserver.test.local
     dns_servers:

molecule/fedora-latest-build/molecule.yml

@@ -3,7 +3,7 @@ driver:
   name: docker
 platforms:
   - name: fedora-latest-build
-    image: fedora-latest
+    image: "fedora:latest"
     dockerfile: Dockerfile
     hostname: ipaserver.test.local
     dns_servers:

playbooks/backup-server-to-controller.yml

@@ -0,0 +1,12 @@
+---
+- name: Playbook to backup IPA server to controller
+  hosts: ipaserver
+  become: true
+
+  vars:
+    ipabackup_to_controller: yes
+    # ipabackup_keep_on_server: yes
+
+  roles:
+  - role: ipabackup
+    state: present

playbooks/backup-server.yml

@@ -0,0 +1,8 @@
+---
+- name: Playbook to backup IPA server
+  hosts: ipaserver
+  become: true
+
+  roles:
+  - role: ipabackup
+    state: present

playbooks/copy-all-backups-from-server.yml

@@ -0,0 +1,12 @@
+---
+- name: Playbook to copy all backups from IPA server
+  hosts: ipaserver
+  become: true
+
+  vars:
+    ipabackup_name: all
+    ipabackup_to_controller: yes
+
+  roles:
+  - role: ipabackup
+    state: copied

playbooks/copy-backup-from-controller.yml

@@ -0,0 +1,12 @@
+---
+- name: Playbook to copy a backup from controller to the IPA server
+  hosts: ipaserver
+  become: true
+
+  vars:
+    ipabackup_name: ipaserver.test.local_ipa-full-2020-10-22-11-11-44
+    ipabackup_from_controller: yes
+
+  roles:
+  - role: ipabackup
+    state: copied

playbooks/copy-backup-from-server.yml

@@ -0,0 +1,12 @@
+---
+- name: Playbook to copy backup from IPA server
+  hosts: ipaserver
+  become: true
+
+  vars:
+    ipabackup_name: ipa-full-2020-10-22-11-11-44
+    ipabackup_to_controller: yes
+
+  roles:
+  - role: ipabackup
+    state: copied

playbooks/permission/permission-absent.yml

@@ -0,0 +1,11 @@
+---
+- name: Permission absent example
+  hosts: ipaserver
+  become: true
+
+  tasks:
+  - name: Ensure permission is absent
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      name: TestPerm1
+      state: absent

playbooks/permission/permission-allow-read-employeenum.yml

@@ -0,0 +1,16 @@
+---
+- name: Permission Allow Read Employee Number Example
+  hosts: ipaserver
+  become: true
+
+  tasks:
+  - name: Ensure permission is present with set of rights to attribute employeenumber
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      name: TestPerm1
+      object_type: user
+      right:
+        - read
+        - search
+        - compare
+      attrs: employeenumber

playbooks/permission/permission-member-absent.yml

@@ -0,0 +1,13 @@
+---
+- name: Permission absent example
+  hosts: ipaserver
+  become: true
+
+  tasks:
+  - name: Ensure permission privilege, "User Administrators", is absent
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      name: TestPerm1
+      privilege: "User Administrators"
+      action: member
+      state: absent

playbooks/permission/permission-member-present.yml

@@ -0,0 +1,12 @@
+---
+- name: Permission member present example
+  hosts: ipaserver
+  become: true
+
+  tasks:
+  - name: Ensure permission is present with "User Administrators" privilege
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      name: TestPerm1
+      privilege: "User Administrators"
+      action: member

playbooks/permission/permission-present.yml

@@ -0,0 +1,12 @@
+---
+- name: Permission present example
+  hosts: ipaserver
+  become: true
+
+  tasks:
+  - name: Ensure permission is present
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      name: TestPerm1
+      object_type: host
+      right: all

playbooks/permission/permission-renamed.yml

@@ -0,0 +1,12 @@
+---
+- name: Permission present example
+  hosts: ipaserver
+  become: true
+
+  tasks:
+  - name: Ensure permission TestPerm1 is renamed to TestPermRenamed
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      name: TestPerm1
+      rename: TestPermRenamed
+      state: renamed

playbooks/remove-all-backups-from-server.yml

@@ -0,0 +1,11 @@
+---
+- name: Playbook to remove all backups from IPA server
+  hosts: ipaserver
+  become: true
+
+  vars:
+    ipabackup_name: all
+
+  roles:
+  - role: ipabackup
+    state: absent

playbooks/remove-backup-from-server.yml

@@ -0,0 +1,11 @@
+---
+- name: Playbook to remove backup from IPA server
+  hosts: ipaserver
+  become: true
+
+  vars:
+    ipabackup_name: ipa-full-2020-10-22-11-11-44
+
+  roles:
+  - role: ipabackup
+    state: absent

playbooks/restore-server-from-controller.yml

@@ -0,0 +1,13 @@
+---
+- name: Playbook to restore IPA server from controller
+  hosts: ipaserver
+  become: true
+
+  vars:
+    ipabackup_name: ipaserver.el83.local_ipa-full-2020-10-22-11-11-44
+    ipabackup_password: SomeDMpassword
+    ipabackup_from_controller: yes
+
+  roles:
+  - role: ipabackup
+    state: restored

playbooks/restore-server.yml

@@ -0,0 +1,12 @@
+---
+- name: Playbook to restore an IPA server
+  hosts: ipaserver
+  become: true
+
+  vars:
+    ipabackup_name: ipa-full-2020-10-22-11-11-44
+    ipabackup_password: SomeDMpassword
+
+  roles:
+  - role: ipabackup
+    state: restored

playbooks/selfservice/selfservice-absent.yml

@@ -1,11 +1,11 @@
 ---
-- name: Delegation absent
+- name: Selfservice absent
   hosts: ipaserver
   become: true
 
   tasks:
-  - name: Ensure delegation "basic manager attributes" is absent
-    ipadelegation:
+  - name: Ensure selfservice "basic manager attributes" is absent
+    ipaselfservice:
       ipaadmin_password: SomeADMINpassword
       name: "basic manager attributes"
       state: absent

playbooks/selfservice/selfservice-member-absent.yml

@@ -1,15 +1,15 @@
 ---
-- name: Delegation member absent
+- name: Selfservice member absent
   hosts: ipaserver
   become: true
 
   tasks:
-  - name: Ensure delegation "basic manager attributes" member attributes employeenumber and employeetype are absent
-    ipadelegation:
+  - name: Ensure selfservice "basic manager attributes" member attributes employeenumber and employeetype are absent
+    ipaselfservice:
       ipaadmin_password: SomeADMINpassword
       name: "basic manager attributes"
       attribute:
-      - employeenumber
-      - employeetype
+      - businesscategory
+      - departmentnumber
       action: member
       state: absent

playbooks/selfservice/selfservice-member-present.yml

@@ -1,11 +1,11 @@
 ---
-- name: Delegation member present
+- name: Selfservice member present
   hosts: ipaserver
   become: true
 
   tasks:
-  - name: Ensure delegation "basic manager attributes" member attribute departmentnumber is present
-    ipadelegation:
+  - name: Ensure selfservice "basic manager attributes" member attribute departmentnumber is present
+    ipaselfservice:
       ipaadmin_password: SomeADMINpassword
       name: "basic manager attributes"
       attribute:

playbooks/selfservice/selfservice-present.yml

@@ -1,11 +1,11 @@
 ---
-- name: Delegation present
+- name: Selfservice present
   hosts: ipaserver
   become: true
 
   tasks:
-  - name: Ensure delegation "basic manager attributes" is present
-    ipadelegation:
+  - name: Ensure selfservice "basic manager attributes" is present
+    ipaselfservice:
       ipaadmin_password: SomeADMINpassword
       name: "basic manager attributes"
       permission: read

playbooks/vault/retrive-data-asymmetric-vault.yml

@@ -13,5 +13,6 @@
         private_key_file: private.pem
         state: retrieved
       register: result
+      no_log: true
     - debug:
        msg: "Data: {{ result.vault.data }}"

playbooks/vault/retrive-data-symmetric-vault.yml

@@ -13,5 +13,6 @@
         password: SomeVAULTpassword
         state: retrieved
       register: result
+      no_log: true
     - debug:
         msg: "{{ result.vault.data }}"

plugins/module_utils/ansible_freeipa_module.py

@@ -22,13 +22,35 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 
+import sys
+import operator
 import os
 import uuid
 import tempfile
 import shutil
+import netaddr
 import gssapi
 from datetime import datetime
 from pprint import pformat
+
+try:
+    from packaging import version
+except ImportError:
+    # If `packaging` not found, split version string for creating version
+    # object. Although it is not PEP 440 compliant, it will work for stable
+    # FreeIPA releases.
+    import re
+
+    class version:
+        @staticmethod
+        def parse(version_str):
+            """
+            Split a version string A.B.C, into a tuple.
+
+            This will not work for `rc`, `dev` or similar version string.
+            """
+            return tuple(re.split("[-_\.]", version_str))  # noqa: W605
+
 from ipalib import api
 from ipalib import errors as ipalib_errors  # noqa
 from ipalib.config import Env
@@ -40,10 +62,12 @@ except ImportError:
     from ipapython.ipautil import kinit_password, kinit_keytab
 from ipapython.ipautil import run
 from ipapython.dn import DN
+from ipapython.version import VERSION
 from ipaplatform.paths import paths
 from ipalib.krb_utils import get_credentials_if_valid
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils._text import to_text
+from ansible.module_utils.common.text.converters import jsonify
 
 try:
     from ipalib.x509 import Encoding
@@ -185,6 +209,26 @@ def api_check_param(command, name):
     return name in api.Command[command].params
 
 
+def api_check_ipa_version(oper, requested_version):
+    """
+    Compare the installed IPA version against a requested version.
+
+    The valid operators are: <, <=, >, >=, ==, !=
+    """
+    oper_map = {
+        "<": operator.lt,
+        "<=": operator.le,
+        ">": operator.gt,
+        ">=": operator.ge,
+        "==": operator.eq,
+        "!=": operator.ne,
+    }
+    operation = oper_map.get(oper)
+    if not(operation):
+        raise NotImplementedError("Invalid operator: %s" % oper)
+    return operation(version.parse(VERSION), version.parse(requested_version))
+
+
 def execute_api_command(module, principal, password, command, name, args):
     """
     Execute an API command.
@@ -370,6 +414,24 @@ def is_valid_port(port):
     return False
 
 
+def is_ip_address(ipaddr):
+    """Test if given IP address is a valid IPv4 or IPv6 address."""
+    try:
+        netaddr.IPAddress(str(ipaddr))
+    except (netaddr.AddrFormatError, ValueError):
+        return False
+    return True
+
+
+def is_ip_network_address(ipaddr):
+    """Test if given IP address is a valid IPv4 or IPv6 address."""
+    try:
+        netaddr.IPNetwork(str(ipaddr))
+    except (netaddr.AddrFormatError, ValueError):
+        return False
+    return True
+
+
 def is_ipv4_addr(ipaddr):
     """Test if given IP address is a valid IPv4 address."""
     try:
@@ -388,6 +450,26 @@ def is_ipv6_addr(ipaddr):
     return True
 
 
+def exit_raw_json(module, **kwargs):
+    """
+    Print the raw parameters in JSON format, without masking.
+
+    Due to Ansible filtering out values in the output that match values
+    in variables which has `no_log` set, if a module need to return user
+    defined dato to the controller, it cannot rely on
+    AnsibleModule.exit_json, as there is a chance that a partial match may
+    occur, masking the data returned.
+
+    This method is a replacement for AnsibleModule.exit_json. It has
+    nearly the same implementation as exit_json, but does not filter
+    data. Beware that this data will be logged by Ansible, and if it
+    contains sensible data, it will be appear in the logs.
+    """
+    module.do_cleanup_files()
+    print(jsonify(kwargs))
+    sys.exit(0)
+
+
 class AnsibleFreeIPAParams(Mapping):
     def __init__(self, ansible_module):
         self.mapping = ansible_module.params

plugins/modules/ipaconfig.py

@@ -428,7 +428,8 @@ def main():
             if params \
                and not compare_args_ipa(ansible_module, params, res_show):
                 changed = True
-                api_command_no_name(ansible_module, "config_mod", params)
+                if not ansible_module.check_mode:
+                    api_command_no_name(ansible_module, "config_mod", params)
 
         else:
             rawresult = api_command_no_name(ansible_module, "config_show", {})

plugins/modules/ipadelegation.py

@@ -310,6 +310,10 @@ def main():
             else:
                 ansible_module.fail_json(msg="Unkown state '%s'" % state)
 
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
         # Execute commands
 
         for name, command, args in commands:

plugins/modules/ipadnsconfig.py

@@ -233,7 +233,8 @@ def main():
         # Execute command only if configuration changes.
         if not compare_args_ipa(ansible_module, args, res_find):
             try:
-                api_command_no_name(ansible_module, 'dnsconfig_mod', args)
+                if not ansible_module.check_mode:
+                    api_command_no_name(ansible_module, 'dnsconfig_mod', args)
                 # If command did not fail, something changed.
                 changed = True
 

plugins/modules/ipadnsforwardzone.py

@@ -89,8 +89,19 @@ EXAMPLES = '''
     state: present
     name: example.com
     forwarders:
-    - 8.8.8.8
-    - 4.4.4.4
+    - ip_address: 8.8.8.8
+    - ip_address: 4.4.4.4
+    forwardpolicy: first
+    skip_overlap_check: true
+
+# Ensure dns zone is present, with forwarder on non-default port
+- ipadnsforwardzone:
+    ipaadmin_password: MyPassword123
+    state: present
+    name: example.com
+    forwarders:
+    - ip_address: 8.8.8.8
+      port: 8053
     forwardpolicy: first
     skip_overlap_check: true
 
@@ -369,6 +380,12 @@ def main():
                         [name, 'dnsforwardzone_remove_permission', {}]
                     )
 
+            # Check mode exit
+            if ansible_module.check_mode:
+                ansible_module.exit_json(changed=len(commands) > 0,
+                                         **exit_args)
+
+            # Execute commands
             for name, command, args in commands:
                 api_command(ansible_module, command, name, args)
                 changed = True

plugins/modules/ipadnsrecord.py

@@ -525,7 +525,6 @@ options:
     aliases: ["uri_record"]
   ip_address:
     description: IP adresses for A ar AAAA.
-    aliases: ["a_ip_address", "aaaa_ip_address"]
     required: false
     type: string
   create_reverse:
@@ -890,6 +889,10 @@ _RECORD_FIELDS = [
     "tlsa_rec", "txt_rec", "uri_rec"
 ]
 
+# The _PART_MAP structure maps ansible-freeipa attributes to their
+# FreeIPA API counterparts. The keys are also used to obtain a list
+# of all supported DNS record attributes.
+
 _PART_MAP = {
     'a_ip_address': 'a_part_ip_address',
     'a_create_reverse': 'a_extra_create_reverse',
@@ -953,6 +956,10 @@ _PART_MAP = {
     "uri_weight": "uri_part_weight"
 }
 
+# _RECORD_PARTS is a structure that maps the attributes that store
+# the DNS record in FreeIPA API to the parts and options available
+# for these records in the API.
+
 _RECORD_PARTS = {
     "arecord": ["a_part_ip_address", "a_extra_create_reverse"],
     "aaaarecord": [
@@ -960,7 +967,7 @@ _RECORD_PARTS = {
     ],
     "a6record": ["a6_part_data"],
     "afsdbrecord": ['afsdb_part_subtype', 'afsdb_part_hostname'],
-    "cert_rec": [
+    "certrecord": [
         'cert_part_type', 'cert_part_key_tag', 'cert_part_algorithm',
         'cert_part_certificate_or_crl'
     ],
@@ -1133,33 +1140,20 @@ def configure_module():
     return ansible_module
 
 
-def find_dnsrecord(module, dnszone, name, **records):
+def find_dnsrecord(module, dnszone, name):
     """Find a DNS record based on its name (idnsname)."""
-    _args = {record: value for record, value in records.items()}
-    _args["all"] = True
-    if name != '@':
-        _args['idnsname'] = to_text(name)
+    _args = {
+        "all": True,
+        "idnsname": to_text(name),
+    }
 
     try:
         _result = api_command(
-            module, "dnsrecord_find", to_text(dnszone), _args)
+            module, "dnsrecord_show", to_text(dnszone), _args)
     except ipalib.errors.NotFound:
         return None
 
-    if len(_result["result"]) > 1 and name != '@':
-        module.fail_json(
-            msg="There is more than one dnsrecord for '%s',"
-                " zone '%s'" % (name, dnszone))
-    else:
-        if len(_result["result"]) == 1:
-            return _result["result"][0]
-        else:
-            for _res in _result["result"]:
-                if 'idnsname' in _res:
-                    for x in _res['idnsname']:
-                        if '@' == to_text(x):
-                            return _res
-    return None
+    return _result["result"]
 
 
 def check_parameters(module, state, zone_name, record):
@@ -1174,10 +1168,21 @@ def check_parameters(module, state, zone_name, record):
             module.fail_json(
                 msg="Record Type '%s' is not supported." % record_type)
 
-    has_record = any(record.get(rec, None) for rec in _RECORD_FIELDS)
+    # has_record is "True" if the playbook has set any of the full record
+    # attributes (*record or *_rec).
+    has_record = any(
+        (rec in record) or (("%sord" % rec) in record)
+        for rec in _RECORD_FIELDS
+    )
 
+    # has_part_record is "True" if the playbook has set any of the
+    # record field attributes.
     has_part_record = any(record.get(rec, None) for rec in _PART_MAP)
 
+    # some attributes in the playbook may have a special meaning,
+    # like "ip_address", which is used for either arecord or aaaarecord,
+    # and has_special is true if any of these attributes is set on
+    # on the playbook.
     special_list = ['ip_address']
     has_special = any(record.get(rec, None) for rec in special_list)
 
@@ -1286,7 +1291,7 @@ def gen_args(entry):
 
     else:
         for field in _RECORD_FIELDS:
-            record_value = entry.get(field, None)
+            record_value = entry.get(field) or entry.get("%sord" % field)
             if record_value is not None:
                 record_type = field.split('_')[0]
                 rec = "{}record".format(record_type.lower())
@@ -1324,7 +1329,17 @@ def define_commands_for_present_state(module, zone_name, entry, res_find):
     name = to_text(entry['name'])
     args = gen_args(entry)
 
-    if res_find is None:
+    existing = find_dnsrecord(module, zone_name, name)
+
+    for record, fields in _RECORD_PARTS.items():
+        part_fields = [f for f in fields if f in args]
+        if part_fields and record in args:
+            record_change_request = True
+            break
+    else:
+        record_change_request = False
+
+    if res_find is None and not record_change_request:
         _commands.append([zone_name, 'dnsrecord_add', args])
     else:
         # Create reverse records for existing records
@@ -1335,8 +1350,6 @@ def define_commands_for_present_state(module, zone_name, entry, res_find):
                     module, zone_name, name, args[record])
                 _commands.extend(cmds)
                 del args['%s_extra_create_reverse' % ipv]
-                if '%s_ip_address' not in args:
-                    del args[record]
         for record, fields in _RECORD_PARTS.items():
             part_fields = [f for f in fields if f in args]
             if part_fields:
@@ -1346,25 +1359,23 @@ def define_commands_for_present_state(module, zone_name, entry, res_find):
                         module.fail_json(msg="Cannot modify multiple records "
                                              "of the same type at once.")
 
-                    existing = find_dnsrecord(module, zone_name, name,
-                                              **{record: args[record][0]})
+                    mod_record = args[record][0]
                     if existing is None:
-                        module.fail_json(msg="``%s` not found." % record)
+                        module.fail_json(msg="`%s` not found." % record)
                     else:
                         # update DNS record
                         _args = {k: args[k] for k in part_fields if k in args}
                         _args["idnsname"] = to_text(args["idnsname"])
-                        _args[record] = res_find[record]
+                        _args[record] = mod_record
                         if 'dns_ttl' in args:
                             _args['dns_ttl'] = args['dns_ttl']
                         _commands.append([zone_name, 'dnsrecord_mod', _args])
                     # remove record from args, as it will not be used again.
                     del args[record]
                 else:
-                    for f in part_fields:
-                        _args = {k: args[k] for k in part_fields}
-                        _args['idnsname'] = name
-                        _commands.append([zone_name, 'dnsrecord_add', _args])
+                    _args = {k: args[k] for k in part_fields if k in args}
+                    _args['idnsname'] = name
+                    _commands.append([zone_name, 'dnsrecord_add', _args])
                 # clean used fields from args
                 for f in part_fields:
                     if f in args:
@@ -1373,9 +1384,11 @@ def define_commands_for_present_state(module, zone_name, entry, res_find):
                 if record in args:
                     add_list = []
                     for value in args[record]:
-                        existing = find_dnsrecord(module, zone_name, name,
-                                                  **{record: value})
-                        if existing is None:
+                        if (
+                            res_find is None
+                            or record not in res_find
+                            or value not in res_find[record]
+                        ):
                             add_list.append(value)
                     if add_list:
                         args[record] = add_list
@@ -1390,7 +1403,6 @@ def define_commands_for_absent_state(module, zone_name, entry, res_find):
     if res_find is None:
         return []
 
-    name = entry['name']
     args = gen_args(entry)
 
     del_all = args.get('del_all', False)
@@ -1404,11 +1416,11 @@ def define_commands_for_absent_state(module, zone_name, entry, res_find):
         delete_records = False
         for record, values in records_to_delete.items():
             del_list = []
-            for value in values:
-                existing = find_dnsrecord(
-                    module, zone_name, name, **{record: value})
-                if existing:
-                    del_list.append(value)
+            if record in res_find:
+                for value in values:
+                    for rec_found in res_find[record]:
+                        if rec_found == value:
+                            del_list.append(value)
             if del_list:
                 args[record] = del_list
                 delete_records = True
@@ -1482,6 +1494,10 @@ def main():
             if cmds:
                 commands.extend(cmds)
 
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
         # Execute commands
         for name, command, args in commands:
             try:

plugins/modules/ipadnszone.py

@@ -210,10 +210,11 @@ dnszone:
 from ipapython.dnsutil import DNSName  # noqa: E402
 from ansible.module_utils.ansible_freeipa_module import (
     FreeIPABaseModule,
-    is_ipv4_addr,
-    is_ipv6_addr,
-    is_valid_port,
+    is_ip_address,
+    is_ip_network_address,
+    is_valid_port
 )  # noqa: E402
+import ipalib.errors
 import netaddr
 import six
 
@@ -251,7 +252,13 @@ class DNSZoneModule(FreeIPABaseModule):
 
     def validate_ips(self, ips, error_msg):
         invalid_ips = [
-            ip for ip in ips if not is_ipv4_addr(ip) or is_ipv6_addr(ip)
+            ip for ip in ips
+            if not any([
+                is_ip_address(ip),
+                is_ip_network_address(ip),
+                ip == "any",
+                ip == "none"
+            ])
         ]
         if any(invalid_ips):
             self.fail_json(msg=error_msg % invalid_ips)
@@ -308,7 +315,7 @@ class DNSZoneModule(FreeIPABaseModule):
             forwarders = []
             for forwarder in self.ipa_params.forwarders:
                 ip_address = forwarder.get("ip_address")
-                if not (is_ipv4_addr(ip_address) or is_ipv6_addr(ip_address)):
+                if not (is_ip_address(ip_address)):
                     self.fail_json(
                         msg="Invalid IP for DNS forwarder: %s" % ip_address
                     )
@@ -404,13 +411,14 @@ class DNSZoneModule(FreeIPABaseModule):
 
     def get_zone(self, zone_name):
         get_zone_args = {"idnsname": zone_name, "all": True}
-        response = self.api_command("dnszone_find", args=get_zone_args)
 
-        zone = None
-        is_zone_active = False
-
-        if response["count"] == 1:
-            zone = response["result"][0]
+        try:
+            response = self.api_command("dnszone_show", args=get_zone_args)
+        except ipalib.errors.NotFound:
+            zone = None
+            is_zone_active = False
+        else:
+            zone = response["result"]
             is_zone_active = zone.get("idnszoneactive") == ["TRUE"]
 
         return zone, is_zone_active
@@ -448,7 +456,10 @@ class DNSZoneModule(FreeIPABaseModule):
             # Look for existing zone in IPA
             zone, is_zone_active = self.get_zone(zone_name)
             args = self.get_ipa_command_args(zone=zone)
-            just_added = False
+            set_serial = self.ipa_params.serial is not None
+
+            if set_serial:
+                del args["idnssoaserial"]
 
             if self.ipa_params.state in ["present", "enabled", "disabled"]:
                 if not zone:
@@ -456,7 +467,7 @@ class DNSZoneModule(FreeIPABaseModule):
                     #   with given args
                     self.add_ipa_command("dnszone_add", zone_name, args)
                     is_zone_active = True
-                    just_added = True
+                    # just_added = True
 
                 else:
                     # Zone already exist so we need to verify if given args
@@ -464,22 +475,24 @@ class DNSZoneModule(FreeIPABaseModule):
                     if self.require_ipa_attrs_change(args, zone):
                         self.add_ipa_command("dnszone_mod", zone_name, args)
 
-                if self.ipa_params.state == "enabled" and not is_zone_active:
-                    self.add_ipa_command("dnszone_enable", zone_name)
+            if self.ipa_params.state == "enabled" and not is_zone_active:
+                self.add_ipa_command("dnszone_enable", zone_name)
 
-                if self.ipa_params.state == "disabled" and is_zone_active:
-                    self.add_ipa_command("dnszone_disable", zone_name)
+            if self.ipa_params.state == "disabled" and is_zone_active:
+                self.add_ipa_command("dnszone_disable", zone_name)
 
-            if self.ipa_params.state == "absent":
-                if zone:
-                    self.add_ipa_command("dnszone_del", zone_name)
+            if self.ipa_params.state == "absent" and zone is not None:
+                self.add_ipa_command("dnszone_del", zone_name)
 
             # Due to a bug in FreeIPA dnszone-add won't set
-            #   SOA Serial. The good news is that dnszone-mod does the job.
-            # See: https://pagure.io/freeipa/issue/8227
-            # Because of that, if the zone was just added with a given serial
-            #   we run mod just after to workaround the bug
-            if just_added and self.ipa_params.serial is not None:
+            # SOA Serial in the creation of a zone, or if
+            # another field is modified along with it.
+            # As a workaround, we set only the SOA serial,
+            # with dnszone-mod, after other changes.
+            # See:
+            #   - https://pagure.io/freeipa/issue/8227
+            #   - https://pagure.io/freeipa/issue/8489
+            if set_serial:
                 args = {
                     "idnssoaserial": self.ipa_params.serial,
                 }

plugins/modules/ipagroup.py

@@ -92,6 +92,12 @@ options:
     - Only usable with IPA versions 4.8.4 and up.
     required: false
     type: list
+  externalmember:
+    description:
+    - List of members of a trusted domain in DOM\\name or name@domain form.
+    required: false
+    type: list
+    ailases: ["ipaexternalmember", "external_member"]
   action:
     description: Work on group or member level
     default: group
@@ -145,7 +151,6 @@ EXAMPLES = """
     - sysops
     - appops
 
-
 # Create a non-POSIX group
 - ipagroup:
     ipaadmin_password: SomeADMINpassword
@@ -158,6 +163,15 @@ EXAMPLES = """
     name: nonposix
     posix: yes
 
+# Create an external group and add members from a trust to it.
+- ipagroup:
+    ipaadmin_password: SomeADMINpassword
+    name: extgroup
+    external: yes
+    externalmember:
+    - WINIPA\\Web Users
+    - WINIPA\\Developers
+
 # Remove goups sysops, appops, ops and nongroup
 - ipagroup:
     ipaadmin_password: SomeADMINpassword
@@ -203,7 +217,7 @@ def gen_args(description, gid, nomembers):
     return _args
 
 
-def gen_member_args(user, group, service):
+def gen_member_args(user, group, service, externalmember):
     _args = {}
     if user is not None:
         _args["member_user"] = user
@@ -211,12 +225,24 @@ def gen_member_args(user, group, service):
         _args["member_group"] = group
     if service is not None:
         _args["member_service"] = service
+    if externalmember is not None:
+        _args["member_external"] = externalmember
 
     return _args
 
 
+def is_external_group(res_find):
+    """Verify if the result group is an external group."""
+    return res_find and 'ipaexternalgroup' in res_find['objectclass']
+
+
+def is_posix_group(res_find):
+    """Verify if the result group is an external group."""
+    return res_find and 'posixgroup' in res_find['objectclass']
+
+
 def check_objectclass_args(module, res_find, nonposix, posix, external):
-    if res_find and 'posixgroup' in res_find['objectclass']:
+    if is_posix_group(res_find):
         if (
             (posix is not None and posix is False)
             or nonposix
@@ -226,7 +252,7 @@ def check_objectclass_args(module, res_find, nonposix, posix, external):
                 msg="Cannot change `POSIX` status of a group "
                     "to `non-POSIX` or `external`.")
     # Can't change an existing external group
-    if res_find and 'ipaexternalgroup' in res_find['objectclass']:
+    if is_external_group(res_find):
         if (
             posix
             or (nonposix is not None and nonposix is False)
@@ -242,10 +268,10 @@ def should_modify_group(module, res_find, args, nonposix, posix, external):
         return True
     if any([posix, nonposix]):
         set_posix = posix or (nonposix is not None and not nonposix)
-        if set_posix and 'posixgroup' not in res_find['objectclass']:
+        if set_posix and not is_posix_group(res_find):
             return True
-    if 'ipaexternalgroup' not in res_find['objectclass'] and external:
-        if 'posixgroup' not in res_find['objectclass']:
+    if not is_external_group(res_find) and external:
+        if not is_posix_group(res_find):
             return True
     return False
 
@@ -272,6 +298,11 @@ def main():
             membermanager_user=dict(required=False, type='list', default=None),
             membermanager_group=dict(required=False, type='list',
                                      default=None),
+            externalmember=dict(required=False, type='list', default=None,
+                                aliases=[
+                                    "ipaexternalmember",
+                                    "external_member"
+                                ]),
             action=dict(type="str", default="group",
                         choices=["member", "group"]),
             # state
@@ -308,6 +339,7 @@ def main():
                                            "membermanager_user")
     membermanager_group = module_params_get(ansible_module,
                                             "membermanager_group")
+    externalmember = module_params_get(ansible_module, "externalmember")
     action = module_params_get(ansible_module, "action")
     # state
     state = module_params_get(ansible_module, "state")
@@ -334,7 +366,7 @@ def main():
         invalid = ["description", "gid", "posix", "nonposix", "external",
                    "nomembers"]
         if action == "group":
-            invalid.extend(["user", "group", "service"])
+            invalid.extend(["user", "group", "service", "externalmember"])
         for x in invalid:
             if vars()[x] is not None:
                 ansible_module.fail_json(
@@ -404,10 +436,19 @@ def main():
                         if external:
                             args['external'] = True
                         commands.append([name, "group_add", args])
-                        # Set res_find to empty dict for next step
+                        # Set res_find dict for next step
                         res_find = {}
 
-                    member_args = gen_member_args(user, group, service)
+                    # if we just created/modified the group, update res_find
+                    res_find.setdefault("objectclass", [])
+                    if external and not is_external_group(res_find):
+                        res_find["objectclass"].append("ipaexternalgroup")
+                    if posix and not is_posix_group(res_find):
+                        res_find["objectclass"].append("posixgroup")
+
+                    member_args = gen_member_args(
+                        user, group, service, externalmember
+                    )
                     if not compare_args_ipa(ansible_module, member_args,
                                             res_find):
                         # Generate addition and removal lists
@@ -420,40 +461,48 @@ def main():
                         service_add, service_del = gen_add_del_lists(
                             service, res_find.get("member_service"))
 
+                        (externalmember_add,
+                         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 members
-                            if len(user_add) > 0 or len(group_add) > 0 or \
-                               len(service_add) > 0:
-                                commands.append([name, "group_add_member",
-                                                 {
-                                                     "user": user_add,
-                                                     "group": group_add,
-                                                     "service": service_add,
-                                                 }])
-                            # Remove members
-                            if len(user_del) > 0 or len(group_del) > 0 or \
-                               len(service_del) > 0:
-                                commands.append([name, "group_remove_member",
-                                                 {
-                                                     "user": user_del,
-                                                     "group": group_del,
-                                                     "service": service_del,
-                                                 }])
-                        else:
-                            # Add members
-                            if len(user_add) > 0 or len(group_add) > 0:
-                                commands.append([name, "group_add_member",
-                                                 {
-                                                     "user": user_add,
-                                                     "group": group_add,
-                                                 }])
-                            # Remove members
-                            if len(user_del) > 0 or len(group_del) > 0:
-                                commands.append([name, "group_remove_member",
-                                                 {
-                                                     "user": user_del,
-                                                     "group": group_del,
-                                                 }])
+                            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(
@@ -492,19 +541,25 @@ def main():
                 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:
-                        commands.append([name, "group_add_member",
-                                         {
-                                             "user": user,
-                                             "group": group,
-                                             "service": service,
-                                         }])
-                    else:
-                        commands.append([name, "group_add_member",
-                                         {
-                                             "user": user,
-                                             "group": group,
-                                         }])
+                        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."
+                        )
+
+                    if any([user, group, service, externalmember]):
+                        commands.append(
+                            [name, "group_add_member", add_member_args]
+                        )
 
                     if has_add_membermanager:
                         # Add membermanager users and groups
@@ -527,19 +582,24 @@ 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:
-                        commands.append([name, "group_remove_member",
-                                         {
-                                             "user": user,
-                                             "group": group,
-                                             "service": service,
-                                         }])
-                    else:
-                        commands.append([name, "group_remove_member",
-                                         {
-                                             "user": user,
-                                             "group": group,
-                                         }])
+                        del_member_args["service"] = service
+                    if is_external_group(res_find):
+                        del_member_args["ipaexternalmember"] = externalmember
+                    elif externalmember:
+                        ansible_module.fail_json(
+                            msg="Cannot add external members to a "
+                                "non-external group."
+                        )
+
+                    if any([user, group, service, externalmember]):
+                        commands.append(
+                            [name, "group_remove_member", del_member_args]
+                        )
 
                     if has_add_membermanager:
                         # Remove membermanager users and groups
@@ -556,6 +616,10 @@ def main():
             else:
                 ansible_module.fail_json(msg="Unkown state '%s'" % state)
 
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
         # Execute commands
 
         for name, command, args in commands:

plugins/modules/ipahbacrule.py

@@ -500,6 +500,10 @@ def main():
             else:
                 ansible_module.fail_json(msg="Unkown state '%s'" % state)
 
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
         # Execute commands
 
         errors = []

plugins/modules/ipahbacsvc.py

@@ -195,6 +195,10 @@ def main():
             else:
                 ansible_module.fail_json(msg="Unkown state '%s'" % state)
 
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
         # Execute commands
 
         for name, command, args in commands:

plugins/modules/ipahbacsvcgroup.py

@@ -300,6 +300,10 @@ def main():
             else:
                 ansible_module.fail_json(msg="Unkown state '%s'" % state)
 
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
         # Execute commands
         errors = []
         for name, command, args in commands:

plugins/modules/ipahost.py

@@ -439,6 +439,12 @@ def find_host(module, name):
 
 
 def find_dnsrecord(module, name):
+    """
+    Search for a DNS record.
+
+    This function may raise ipalib_errors.NotFound in some cases,
+    and it should be handled by the caller.
+    """
     domain_name = name[name.find(".")+1:]
     host_name = name[:name.find(".")]
 
@@ -447,14 +453,8 @@ def find_dnsrecord(module, name):
         "idnsname": to_text(host_name)
     }
 
-    try:
-        _result = api_command(module, "dnsrecord_show", to_text(domain_name),
-                              _args)
-    except ipalib_errors.NotFound as e:
-        msg = str(e)
-        if "record not found" in msg or "zone not found" in msg:
-            return None
-        module.fail_json(msg="dnsrecord_show failed: %s" % msg)
+    _result = api_command(module, "dnsrecord_show", to_text(domain_name),
+                          _args)
 
     return _result["result"]
 
@@ -876,8 +876,11 @@ def main():
                 msg = str(e)
                 dns_not_configured = "DNS is not configured" in msg
                 dns_zone_not_found = "DNS zone not found" in msg
-                if ip_address is None and (
-                    dns_not_configured or dns_zone_not_found
+                dns_res_not_found = "DNS resource record not found" in msg
+                if (
+                    dns_res_not_found
+                    or ip_address is None
+                    and (dns_not_configured or dns_zone_not_found)
                 ):
                     # IP address(es) not given and no DNS support in IPA
                     # -> Ignore failure
@@ -1344,6 +1347,10 @@ def main():
 
         del host_set
 
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
         # Execute commands
 
         errors = []

plugins/modules/ipahostgroup.py

@@ -463,6 +463,10 @@ def main():
             else:
                 ansible_module.fail_json(msg="Unkown state '%s'" % state)
 
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
         # Execute commands
         for name, command, args in commands:
             try:

plugins/modules/ipalocation.py

@@ -190,6 +190,10 @@ def main():
             else:
                 ansible_module.fail_json(msg="Unkown state '%s'" % state)
 
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
         # Execute commands
 
         for name, command, args in commands:

plugins/modules/ipapermission.py

@@ -0,0 +1,494 @@
+#!/usr/bin/python
+# -*- coding: utf-8 -*-
+
+# Authors:
+#   Seth Kress <kresss@gmail.com>
+#
+# Copyright (C) 2020 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/>.
+
+ANSIBLE_METADATA = {
+    "metadata_version": "1.0",
+    "supported_by": "community",
+    "status": ["preview"],
+}
+
+DOCUMENTATION = """
+---
+module: ipapermission
+short description: Manage FreeIPA permission
+description: Manage FreeIPA permission and permission members
+options:
+  ipaadmin_principal:
+    description: The admin principal.
+    default: admin
+  ipaadmin_password:
+    description: The admin password.
+    required: false
+  name:
+    description: The permission name string.
+    required: true
+    aliases: ["cn"]
+  right:
+    description: Rights to grant
+    required: false
+    choices: ["read", "search", "compare", "write", "add", "delete", "all"]
+    type: list
+    aliases: ["ipapermright"]
+  attrs:
+    description: All attributes to which the permission applies
+    required: false
+    type: list
+  bindtype:
+    description: Bind rule type
+    required: false
+    choices: ["permission", "all", "anonymous"]
+    aliases: ["ipapermbindruletype"]
+  subtree:
+    description: Subtree to apply permissions to
+    required: false
+    aliases: ["ipapermlocation"]
+  filter:
+    description: Extra target filter
+    required: false
+    type: list
+    aliases: ["extratargetfilter"]
+  rawfilter:
+    description: All target filters
+    required: false
+    type: list
+    aliases: ["ipapermtargetfilter"]
+  target:
+    description: Optional DN to apply the permission to
+    required: false
+    aliases: ["ipapermtarget"]
+  targetto:
+    description: Optional DN subtree where an entry can be moved to
+    required: false
+    aliases: ["ipapermtargetto"]
+  targetfrom:
+    description: Optional DN subtree from where an entry can be moved
+    required: false
+    aliases: ["ipapermtargetfrom"]
+  memberof:
+    description: Target members of a group (sets memberOf targetfilter)
+    required: false
+    type: list
+  targetgroup:
+    description: User group to apply permissions to (sets target)
+    required: false
+    aliases: ["targetgroup"]
+  object_type:
+    description: Type of IPA object (sets subtree and objectClass targetfilter)
+    required: false
+    aliases: ["type"]
+  no_members:
+    description: Suppress processing of membership
+    required: false
+    type: bool
+  rename:
+    description: Rename the permission object
+    required: false
+  action:
+    description: Work on permission or member privilege level.
+    choices: ["permission", "member"]
+    default: permission
+    required: false
+  state:
+    description: The state to ensure.
+    choices: ["present", "absent", "renamed"]
+    default: present
+    required: true
+"""
+
+EXAMPLES = """
+# Ensure permission NAME is present
+- ipapermission:
+    name: manage-my-hostgroup
+    right: all
+    bindtype: permission
+    object_type: host
+
+# Ensure permission NAME is absent
+- ipapermission:
+    name: "Removed Permission Name"
+    state: absent
+"""
+
+RETURN = """
+"""
+
+
+from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.ansible_freeipa_module import \
+    temp_kinit, temp_kdestroy, valid_creds, api_connect, api_command, \
+    compare_args_ipa, module_params_get, api_check_ipa_version
+import six
+
+if six.PY3:
+    unicode = str
+
+
+def find_permission(module, name):
+    """Find if a permission with the given name already exist."""
+    try:
+        _result = api_command(module, "permission_show", name, {"all": True})
+    except Exception:  # pylint: disable=broad-except
+        # An exception is raised if permission name is not found.
+        return None
+    else:
+        return _result["result"]
+
+
+def gen_args(right, attrs, bindtype, subtree,
+             extra_target_filter, rawfilter, target,
+             targetto, targetfrom, memberof, targetgroup,
+             object_type, no_members, rename):
+    _args = {}
+    if right is not None:
+        _args["ipapermright"] = right
+    if attrs is not None:
+        _args["attrs"] = attrs
+    if bindtype is not None:
+        _args["ipapermbindruletype"] = bindtype
+    if subtree is not None:
+        _args["ipapermlocation"] = subtree
+    if extra_target_filter is not None:
+        _args["extratargetfilter"] = extra_target_filter
+    if rawfilter is not None:
+        _args["ipapermtargetfilter"] = rawfilter
+    if target is not None:
+        _args["ipapermtarget"] = target
+    if targetto is not None:
+        _args["ipapermtargetto"] = targetto
+    if targetfrom is not None:
+        _args["ipapermtargetfrom"] = targetfrom
+    if memberof is not None:
+        _args["memberof"] = memberof
+    if targetgroup is not None:
+        _args["targetgroup"] = targetgroup
+    if object_type is not None:
+        _args["type"] = object_type
+    if no_members is not None:
+        _args["no_members"] = no_members
+    if rename is not None:
+        _args["rename"] = rename
+    return _args
+
+
+def main():
+    ansible_module = AnsibleModule(
+        argument_spec=dict(
+            # general
+            ipaadmin_principal=dict(type="str", default="admin"),
+            ipaadmin_password=dict(type="str", required=False, no_log=True),
+
+            name=dict(type="list", aliases=["cn"],
+                      default=None, required=True),
+            # present
+            right=dict(type="list", aliases=["ipapermright"], default=None,
+                       required=False,
+                       choices=["read", "search", "compare", "write", "add",
+                                "delete", "all"]),
+            attrs=dict(type="list", default=None, required=False),
+            # Note: bindtype has a default of permission for Adds.
+            bindtype=dict(type="str", aliases=["ipapermbindruletype"],
+                          default=None, require=False, choices=["permission",
+                          "all", "anonymous", "self"]),
+            subtree=dict(type="str", aliases=["ipapermlocation"], default=None,
+                         required=False),
+            extra_target_filter=dict(type="list", aliases=["filter",
+                                     "extratargetfilter"], default=None,
+                                     required=False),
+            rawfilter=dict(type="list", aliases=["ipapermtargetfilter"],
+                           default=None, required=False),
+            target=dict(type="str", aliases=["ipapermtarget"], default=None,
+                        required=False),
+            targetto=dict(type="str", aliases=["ipapermtargetto"],
+                          default=None, required=False),
+            targetfrom=dict(type="str", aliases=["ipapermtargetfrom"],
+                            default=None, required=False),
+            memberof=dict(type="list", default=None, required=False),
+            targetgroup=dict(type="str", default=None, required=False),
+            object_type=dict(type="str", aliases=["type"], default=None,
+                             required=False),
+            no_members=dict(type=bool, default=None, require=False),
+            rename=dict(type="str", default=None, required=False),
+
+            action=dict(type="str", default="permission",
+                        choices=["member", "permission"]),
+            # state
+            state=dict(type="str", default="present",
+                       choices=["present", "absent", "renamed"]),
+        ),
+        supports_check_mode=True,
+    )
+
+    ansible_module._ansible_debug = True
+
+    # Get parameters
+
+    # general
+    ipaadmin_principal = module_params_get(ansible_module,
+                                           "ipaadmin_principal")
+    ipaadmin_password = module_params_get(ansible_module, "ipaadmin_password")
+    names = module_params_get(ansible_module, "name")
+
+    # present
+    right = module_params_get(ansible_module, "right")
+    attrs = module_params_get(ansible_module, "attrs")
+    bindtype = module_params_get(ansible_module, "bindtype")
+    subtree = module_params_get(ansible_module, "subtree")
+    extra_target_filter = module_params_get(ansible_module,
+                                            "extra_target_filter")
+    rawfilter = module_params_get(ansible_module, "rawfilter")
+    target = module_params_get(ansible_module, "target")
+    targetto = module_params_get(ansible_module, "targetto")
+    targetfrom = module_params_get(ansible_module, "targetfrom")
+    memberof = module_params_get(ansible_module, "memberof")
+    targetgroup = module_params_get(ansible_module, "targetgroup")
+    object_type = module_params_get(ansible_module, "object_type")
+    no_members = module_params_get(ansible_module, "no_members")
+    rename = module_params_get(ansible_module, "rename")
+    action = module_params_get(ansible_module, "action")
+
+    # state
+    state = module_params_get(ansible_module, "state")
+
+    # Check parameters
+
+    invalid = []
+
+    if state == "present":
+        if len(names) != 1:
+            ansible_module.fail_json(
+                msg="Only one permission can be added at a time.")
+        if action == "member":
+            invalid = ["bindtype", "target", "targetto", "targetfrom",
+                       "subtree", "targetgroup", "object_type", "rename"]
+        else:
+            invalid = ["rename"]
+
+    if state == "renamed":
+        if len(names) != 1:
+            ansible_module.fail_json(
+                msg="Only one permission can be renamed at a time.")
+        if action == "member":
+            ansible_module.fail_json(
+                msg="Member action can not be used with state 'renamed'")
+        invalid = ["right", "attrs", "bindtype", "subtree",
+                   "extra_target_filter", "rawfilter", "target", "targetto",
+                   "targetfrom", "memberof", "targetgroup", "object_type",
+                   "no_members"]
+
+    if state == "absent":
+        if len(names) < 1:
+            ansible_module.fail_json(msg="No name given.")
+        invalid = ["bindtype", "subtree", "target", "targetto",
+                   "targetfrom", "targetgroup", "object_type",
+                   "no_members", "rename"]
+        if action != "member":
+            invalid += ["right", "attrs", "memberof",
+                        "extra_target_filter", "rawfilter"]
+
+    for x in invalid:
+        if vars()[x] is not None:
+            ansible_module.fail_json(
+                msg="Argument '%s' can not be used with action "
+                "'%s' and state '%s'" % (x, action, state))
+
+    if bindtype == "self" and api_check_ipa_version("<", "4.8.7"):
+        ansible_module.fail_json(
+            msg="Bindtype 'self' is not supported by your IPA version.")
+
+    if all([extra_target_filter, rawfilter]):
+        ansible_module.fail_json(
+            msg="Cannot specify target filter and extra target filter "
+                "simultaneously.")
+
+    # Init
+
+    changed = False
+    exit_args = {}
+    ccache_dir = None
+    ccache_name = None
+    try:
+        if not valid_creds(ansible_module, ipaadmin_principal):
+            ccache_dir, ccache_name = temp_kinit(ipaadmin_principal,
+                                                 ipaadmin_password)
+        api_connect()
+
+        commands = []
+        for name in names:
+            # Make sure permission exists
+            res_find = find_permission(ansible_module, name)
+
+            # Create command
+            if state == "present":
+                # Generate args
+                args = gen_args(right, attrs, bindtype, subtree,
+                                extra_target_filter, rawfilter, target,
+                                targetto, targetfrom, memberof, targetgroup,
+                                object_type, no_members, rename)
+
+                if action == "permission":
+                    # Found the permission
+                    if res_find is not None:
+                        # For all settings is args, check if there are
+                        # different settings in the find result.
+                        # If yes: modify
+                        if not compare_args_ipa(ansible_module, args,
+                                                res_find):
+                            commands.append([name, "permission_mod", args])
+                    else:
+                        commands.append([name, "permission_add", args])
+
+                elif action == "member":
+                    if res_find is None:
+                        ansible_module.fail_json(
+                            msg="No permission '%s'" % name)
+
+                    member_attrs = {}
+                    check_members = {
+                        "attrs": attrs,
+                        "memberof": memberof,
+                        "ipapermright": right,
+                        "ipapermtargetfilter": rawfilter,
+                        "extratargetfilter": extra_target_filter,
+                        # subtree member management is currently disabled.
+                        # "ipapermlocation": subtree,
+                    }
+
+                    for _member, _member_change in check_members.items():
+                        if _member_change is not None:
+                            _res_list = res_find[_member]
+                            _new_set = set(_res_list + _member_change)
+                            if _new_set != set(_res_list):
+                                member_attrs[_member] = list(_new_set)
+
+                    if member_attrs:
+                        commands.append([name, "permission_mod", member_attrs])
+
+                else:
+                    ansible_module.fail_json(
+                        msg="Unknown action '%s'" % action)
+
+            elif state == "renamed":
+                if action == "permission":
+                    # Generate args
+                    # Note: Only valid arg for rename is rename.
+                    args = gen_args(right, attrs, bindtype, subtree,
+                                    extra_target_filter, rawfilter, target,
+                                    targetto, targetfrom, memberof,
+                                    targetgroup, object_type, no_members,
+                                    rename)
+
+                    # Found the permission
+                    if res_find is not None:
+                        # For all settings is args, check if there are
+                        # different settings in the find result.
+                        # If yes: modify
+                        if not compare_args_ipa(ansible_module, args,
+                                                res_find):
+                            commands.append([name, "permission_mod", args])
+                    else:
+                        ansible_module.fail_json(
+                            msg="Permission not found, cannot rename")
+                else:
+                    ansible_module.fail_json(
+                        msg="Unknown action '%s'" % action)
+
+            elif state == "absent":
+                if action == "permission":
+                    if res_find is not None:
+                        commands.append([name, "permission_del", {}])
+
+                elif action == "member":
+                    if res_find is None:
+                        ansible_module.fail_json(
+                            msg="No permission '%s'" % name)
+
+                    member_attrs = {}
+                    check_members = {
+                        "attrs": attrs,
+                        "memberof": memberof,
+                        "ipapermright": right,
+                        "ipapermtargetfilter": rawfilter,
+                        "extratargetfilter": extra_target_filter,
+                        # subtree member management is currently disabled.
+                        # "ipapermlocation": subtree,
+                    }
+
+                    for _member, _member_change in check_members.items():
+                        if _member_change is not None:
+                            _res_set = set(res_find[_member])
+                            _new_set = _res_set - set(_member_change)
+                            if _new_set != _res_set:
+                                member_attrs[_member] = list(_new_set)
+
+                    if member_attrs:
+                        commands.append([name, "permission_mod", member_attrs])
+
+            else:
+                ansible_module.fail_json(msg="Unknown state '%s'" % state)
+
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
+        # Execute commands
+
+        for name, command, args in commands:
+            try:
+                result = api_command(ansible_module, command, name,
+                                     args)
+                if "completed" in result:
+                    if result["completed"] > 0:
+                        changed = True
+                else:
+                    changed = True
+            except Exception as e:
+                ansible_module.fail_json(msg="%s: %s: %s" % (command, name,
+                                                             str(e)))
+            # Get all errors
+            # All "already a member" and "not a member" failures in the
+            # result are ignored. All others are reported.
+            errors = []
+            for failed_item in result.get("failed", []):
+                failed = result["failed"][failed_item]
+                for member_type in failed:
+                    for member, failure in failed[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))
+            if len(errors) > 0:
+                ansible_module.fail_json(msg=", ".join(errors))
+
+    except Exception as e:
+        ansible_module.fail_json(msg=str(e))
+
+    finally:
+        temp_kdestroy(ccache_dir, ccache_name)
+
+    # Done
+
+    ansible_module.exit_json(changed=changed, **exit_args)
+
+
+if __name__ == "__main__":
+    main()

plugins/modules/ipaprivilege.py

@@ -312,6 +312,10 @@ def main():
             else:
                 ansible_module.fail_json(msg="Unkown state '%s'" % state)
 
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
         # Execute commands
 
         for name, command, args in commands:

plugins/modules/ipapwpolicy.py

@@ -284,6 +284,10 @@ def main():
             else:
                 ansible_module.fail_json(msg="Unkown state '%s'" % state)
 
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
         # Execute commands
 
         for name, command, args in commands:

plugins/modules/iparole.py

@@ -257,7 +257,7 @@ def filter_service(module, res_find, predicate):
     return _services
 
 
-def ensure_role_with_members_is_present(module, name, res_find):
+def ensure_role_with_members_is_present(module, name, res_find, action):
     """Define commands to ensure member are present for action `role`."""
     commands = []
     privilege_add, privilege_del = gen_add_del_lists(
@@ -267,7 +267,7 @@ def ensure_role_with_members_is_present(module, name, res_find):
     if privilege_add:
         commands.append([name, "role_add_privilege",
                          {"privilege": privilege_add}])
-    if privilege_del:
+    if action == "role" and privilege_del:
         commands.append([name, "role_remove_privilege",
                          {"privilege": privilege_del}])
 
@@ -297,7 +297,8 @@ def ensure_role_with_members_is_present(module, name, res_find):
 
     if add_members:
         commands.append([name, "role_add_member", add_members])
-    if del_members:
+    # Only remove members if ensuring role, not acting on members.
+    if action == "role" and del_members:
         commands.append([name, "role_remove_member", del_members])
 
     return commands
@@ -355,6 +356,11 @@ def process_commands(module, commands):
     errors = []
     exit_args = {}
     changed = False
+
+    # Check mode exit
+    if module.check_mode:
+        return len(commands) > 0, exit_args
+
     for name, command, args in commands:
         try:
             result = api_command(module, command, name, args)
@@ -400,7 +406,9 @@ def role_commands_for_name(module, state, action, name):
             if res_find is None:
                 module.fail_json(msg="No role '%s'" % name)
 
-        cmds = ensure_role_with_members_is_present(module, name, res_find)
+        cmds = ensure_role_with_members_is_present(
+            module, name, res_find, action
+        )
         commands.extend(cmds)
 
     if state == "absent" and res_find is not None:

plugins/modules/ipaselfservice.py

@@ -293,6 +293,10 @@ def main():
             else:
                 ansible_module.fail_json(msg="Unkown state '%s'" % state)
 
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
         # Execute commands
 
         for name, command, args in commands:

plugins/modules/ipaservice.py

@@ -824,6 +824,10 @@ def main():
             else:
                 ansible_module.fail_json(msg="Unkown state '%s'" % state)
 
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
         # Execute commands
         errors = []
         for name, command, args in commands:

plugins/modules/ipasudocmd.py

@@ -182,6 +182,10 @@ def main():
             else:
                 ansible_module.fail_json(msg="Unkown state '%s'" % state)
 
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
         # Execute commands
         for name, command, args in commands:
             try:

plugins/modules/ipasudocmdgroup.py

@@ -50,10 +50,6 @@ options:
     description: Suppress processing of membership attributes
     required: false
     type: bool
-  sudocmdgroup:
-    description: List of sudocmdgroup names assigned to this sudocmdgroup.
-    required: false
-    type: list
   sudocmd:
     description: List of sudocmds assigned to this sudocmdgroup.
     required: false
@@ -113,22 +109,18 @@ from ansible.module_utils.ansible_freeipa_module import temp_kinit, \
     temp_kdestroy, valid_creds, api_connect, api_command, compare_args_ipa, \
     gen_add_del_lists
 
+import ipalib
+
 
 def find_sudocmdgroup(module, name):
-    _args = {
-        "all": True,
-        "cn": to_text(name),
-    }
+    args = {"all": True}
 
-    _result = api_command(module, "sudocmdgroup_find", to_text(name), _args)
-
-    if len(_result["result"]) > 1:
-        module.fail_json(
-            msg="There is more than one sudocmdgroup '%s'" % (name))
-    elif len(_result["result"]) == 1:
-        return _result["result"][0]
-    else:
+    try:
+        _result = api_command(module, "sudocmdgroup_show", to_text(name), args)
+    except ipalib.errors.NotFound:
         return None
+    else:
+        return _result["result"]
 
 
 def gen_args(description, nomembers):
@@ -141,10 +133,10 @@ def gen_args(description, nomembers):
     return _args
 
 
-def gen_member_args(sudocmdgroup):
+def gen_member_args(sudocmd):
     _args = {}
-    if sudocmdgroup is not None:
-        _args["member_sudocmdgroup"] = sudocmdgroup
+    if sudocmd is not None:
+        _args["member_sudocmd"] = sudocmd
 
     return _args
 
@@ -161,7 +153,6 @@ def main():
             # present
             description=dict(type="str", default=None),
             nomembers=dict(required=False, type='bool', default=None),
-            sudocmdgroup=dict(required=False, type='list', default=None),
             sudocmd=dict(required=False, type='list', default=None),
             action=dict(type="str", default="sudocmdgroup",
                         choices=["member", "sudocmdgroup"]),
@@ -184,7 +175,6 @@ def main():
     # present
     description = ansible_module.params.get("description")
     nomembers = ansible_module.params.get("nomembers")
-    sudocmdgroup = ansible_module.params.get("sudocmdgroup")
     sudocmd = ansible_module.params.get("sudocmd")
     action = ansible_module.params.get("action")
     # state
@@ -258,28 +248,28 @@ def main():
                     if not compare_args_ipa(ansible_module, member_args,
                                             res_find):
                         # Generate addition and removal lists
-                        sudocmdgroup_add, sudocmdgroup_del = \
+                        sudocmd_add, sudocmd_del = \
                             gen_add_del_lists(
-                                sudocmdgroup,
-                                res_find.get("member_sudocmdgroup"))
+                                sudocmd,
+                                res_find.get("member_sudocmd"))
 
                         # Add members
-                        if len(sudocmdgroup_add) > 0:
+                        if len(sudocmd_add) > 0:
                             commands.append([name, "sudocmdgroup_add_member",
                                              {
                                                  "sudocmd": [to_text(c)
                                                              for c in
-                                                             sudocmdgroup_add]
+                                                             sudocmd_add]
                                              }
                                              ])
                         # Remove members
-                        if len(sudocmdgroup_del) > 0:
+                        if len(sudocmd_del) > 0:
                             commands.append([name,
                                              "sudocmdgroup_remove_member",
                                              {
                                                  "sudocmd": [to_text(c)
                                                              for c in
-                                                             sudocmdgroup_del]
+                                                             sudocmd_del]
                                              }
                                              ])
                 elif action == "member":
@@ -308,6 +298,10 @@ def main():
             else:
                 ansible_module.fail_json(msg="Unkown state '%s'" % state)
 
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
         # Execute commands
         for name, command, args in commands:
             try:

plugins/modules/ipasudorule.py

@@ -429,16 +429,16 @@ def main():
 
                     # Generate addition and removal lists
                     host_add, host_del = gen_add_del_lists(
-                        host, res_find.get('member_host', []))
+                        host, res_find.get('memberhost_host', []))
 
                     hostgroup_add, hostgroup_del = gen_add_del_lists(
-                        hostgroup, res_find.get('member_hostgroup', []))
+                        hostgroup, res_find.get('memberhost_hostgroup', []))
 
                     user_add, user_del = gen_add_del_lists(
-                        user, res_find.get('member_user', []))
+                        user, res_find.get('memberuser_user', []))
 
                     group_add, group_del = gen_add_del_lists(
-                        group, res_find.get('member_group', []))
+                        group, res_find.get('memberuser_group', []))
 
                     allow_cmd_add, allow_cmd_del = gen_add_del_lists(
                         allow_sudocmd,
@@ -686,6 +686,10 @@ def main():
             else:
                 ansible_module.fail_json(msg="Unkown state '%s'" % state)
 
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
         # Execute commands
 
         errors = []

plugins/modules/ipatopologysegment.py

@@ -326,6 +326,10 @@ def main():
             else:
                 ansible_module.fail_json(msg="Unkown state '%s'" % state)
 
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
         # Execute command
 
         for command, args, _suffix in commands:

plugins/modules/ipatrust.py

@@ -244,7 +244,8 @@ def main():
 
         if state == "absent":
             if res_find is not None:
-                del_trust(ansible_module, realm)
+                if not ansible_module.check_mode:
+                    del_trust(ansible_module, realm)
                 changed = True
         elif res_find is None:
             if admin is None and trust_secret is None:
@@ -256,7 +257,8 @@ def main():
                                 trust_secret, base_id, range_size, range_type,
                                 two_way, external)
 
-                add_trust(ansible_module, realm, args)
+                if not ansible_module.check_mode:
+                    add_trust(ansible_module, realm, args)
                 changed = True
 
     except Exception as e:

plugins/modules/ipauser.py

@@ -1377,6 +1377,10 @@ def main():
 
         del user_set
 
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
         # Execute commands
 
         errors = []

plugins/modules/ipavault.py

@@ -317,10 +317,11 @@ vault:
 import os
 from base64 import b64decode
 from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils._text import to_text
 from ansible.module_utils.ansible_freeipa_module import temp_kinit, \
     temp_kdestroy, valid_creds, api_connect, api_command, \
-    gen_add_del_lists, compare_args_ipa, module_params_get
-from ipalib.errors import EmptyModlist
+    gen_add_del_lists, compare_args_ipa, module_params_get, exit_raw_json
+from ipalib.errors import EmptyModlist, NotFound
 
 
 def find_vault(module, name, username, service, shared):
@@ -351,7 +352,9 @@ def gen_args(description, username, service, shared, vault_type, salt,
              password, password_file, public_key, public_key_file, vault_data,
              datafile_in, datafile_out):
     _args = {}
+    vault_type = vault_type or to_text("symmetric")
 
+    _args['ipavaulttype'] = vault_type
     if description is not None:
         _args['description'] = description
     if username is not None:
@@ -360,27 +363,32 @@ def gen_args(description, username, service, shared, vault_type, salt,
         _args['service'] = service
     if shared is not None:
         _args['shared'] = shared
-    if vault_type is not None:
-        _args['ipavaulttype'] = vault_type
-    if salt is not None:
-        _args['ipavaultsalt'] = salt
-    if public_key is not None:
-        _args['ipavaultpublickey'] = b64decode(public_key.encode('utf-8'))
-    if public_key_file is not None:
-        with open(public_key_file, 'r') as keyfile:
-            keydata = keyfile.read()
-            _args['ipavaultpublickey'] = keydata.strip().encode('utf-8')
+
+    if vault_type == "symmetric":
+        if salt is not None:
+            _args['ipavaultsalt'] = salt
+        _args['ipavaultpublickey'] = None
+
+    elif vault_type == "asymmetric":
+        if public_key is not None:
+            _args['ipavaultpublickey'] = b64decode(public_key.encode('utf-8'))
+        if public_key_file is not None:
+            with open(public_key_file, 'r') as keyfile:
+                keydata = keyfile.read()
+                _args['ipavaultpublickey'] = keydata.strip().encode('utf-8')
+        _args['ipavaultsalt'] = None
+
+    elif vault_type == "standard":
+        _args['ipavaultsalt'] = None
+        _args['ipavaultpublickey'] = None
 
     return _args
 
 
 def gen_member_args(args, users, groups, services):
-    _args = args.copy()
-
-    for arg in ['ipavaulttype', 'description', 'ipavaultpublickey',
-                'ipavaultsalt']:
-        if arg in _args:
-            del _args[arg]
+    remove = ['ipavaulttype', 'description', 'ipavaultpublickey',
+              'ipavaultsalt']
+    _args = {k: v for k, v in args.items() if k not in remove}
 
     if any([users, groups, services]):
         if users is not None:
@@ -395,9 +403,12 @@ def gen_member_args(args, users, groups, services):
     return None
 
 
-def data_storage_args(args, data, password, password_file, private_key,
-                      private_key_file, datafile_in, datafile_out):
-    _args = {}
+def data_storage_args(vault_type, args, data, password, password_file,
+                      private_key, private_key_file, datafile_in,
+                      datafile_out):
+    remove = ['ipavaulttype', 'description', 'ipavaultpublickey',
+              'ipavaultsalt']
+    _args = {k: v for k, v in args.items() if k not in remove}
 
     if 'username' in args:
         _args['username'] = args['username']
@@ -406,15 +417,17 @@ def data_storage_args(args, data, password, password_file, private_key,
     if 'shared' in args:
         _args['shared'] = args['shared']
 
-    if password is not None:
-        _args['password'] = password
-    if password_file is not None:
-        _args['password_file'] = password_file
+    if vault_type is None or vault_type == "symmetric":
+        if password is not None:
+            _args['password'] = password
+        if password_file is not None:
+            _args['password_file'] = password_file
 
-    if private_key is not None:
-        _args['private_key'] = private_key
-    if private_key_file is not None:
-        _args['private_key_file'] = private_key_file
+    if vault_type == "asymmetric":
+        if private_key is not None:
+            _args['private_key'] = private_key
+        if private_key_file is not None:
+            _args['private_key_file'] = private_key_file
 
     if datafile_in is not None:
         _args['in'] = datafile_in
@@ -427,9 +440,6 @@ def data_storage_args(args, data, password, password_file, private_key,
     if datafile_out is not None:
         _args['out'] = datafile_out
 
-    if private_key_file is not None:
-        _args['private_key_file'] = private_key_file
-
     return _args
 
 
@@ -441,7 +451,7 @@ def check_parameters(module, state, action, description, username, service,
                      new_password, new_password_file):
     invalid = []
     if state == "present":
-        invalid = ['private_key', 'private_key_file', 'datafile_out']
+        invalid = ['datafile_out']
 
         if all([password, password_file]) \
            or all([new_password, new_password_file]):
@@ -454,7 +464,7 @@ def check_parameters(module, state, action, description, username, service,
                     "change symmetric vault password.")
 
         if action == "member":
-            invalid.extend(['description'])
+            invalid.extend(['description', 'vault_type'])
 
     elif state == "absent":
         invalid = ['description', 'salt', 'vault_type', 'private_key',
@@ -480,12 +490,6 @@ def check_parameters(module, state, action, description, username, service,
                 msg="Argument '%s' can not be used with state '%s', "
                     "action '%s'" % (arg, state, action))
 
-    for arg in invalid:
-        if vars()[arg] is not None:
-            module.fail_json(
-                msg="Argument '%s' can not be used with state '%s', "
-                    "action '%s'" % (arg, state, action))
-
 
 def check_encryption_params(module, state, action, vault_type, salt,
                             password, password_file, public_key,
@@ -494,6 +498,10 @@ def check_encryption_params(module, state, action, vault_type, salt,
                             new_password, new_password_file, res_find):
     vault_type_invalid = []
 
+    existing_type = None
+    if res_find:
+        existing_type = res_find["ipavaulttype"][0]
+
     if vault_type is None and res_find is not None:
         vault_type = res_find['ipavaulttype']
         if isinstance(vault_type, (tuple, list)):
@@ -536,48 +544,45 @@ def check_encryption_params(module, state, action, vault_type, salt,
                 msg="Assymmetric vault requires public_key "
                     "or public_key_file to store data.")
 
-    for param in vault_type_invalid:
+    valid_fields = []
+    if existing_type == "symmetric":
+        valid_fields = [
+            'password', 'password_file', 'new_password', 'new_password_file',
+            'salt'
+        ]
+    if existing_type == "asymmetric":
+        valid_fields = [
+            'public_key', 'public_key_file', 'private_key', 'private_key_file'
+        ]
+
+    check_fields = [f for f in vault_type_invalid if f not in valid_fields]
+
+    for param in check_fields:
         if vars()[param] is not None:
             module.fail_json(
                 msg="Argument '%s' cannot be used with vault type '%s'" %
                 (param, vault_type or 'symmetric'))
 
 
-def change_password(module, res_find, password, password_file, new_password,
-                    new_password_file):
-    """
-    Change the password of a symmetric vault.
-
-    To change the password of a vault, it is needed to retrieve the stored
-    data with the current password, and store the data again, with the new
-    password, forcing it to override the old one.
-    """
-    # verify parameters.
-    if not any([new_password, new_password_file]):
-        return []
-    if res_find["ipavaulttype"][0] != "symmetric":
-        module.fail_json(msg="Cannot change password of `%s` vault."
-                             % res_find["ipavaulttype"])
-
+def get_stored_data(module, res_find, args):
+    """Retrieve data stored in the vault."""
     # prepare arguments to retrieve data.
     name = res_find["cn"][0]
-    args = {}
-    if password:
-        args["password"] = password
-    if password_file:
-        args["password"] = password_file
-    # retrieve current stored data
-    result = api_command(module, 'vault_retrieve', name, args)
-    args['data'] = result['result']['data']
+    copy_args = []
+    if res_find['ipavaulttype'][0] == "symmetric":
+        copy_args = ["password", "password_file"]
+    if res_find['ipavaulttype'][0] == "asymmetric":
+        copy_args = ["private_key", "private_key_file"]
 
-    # modify arguments to store data with new password.
-    if password:
-        args["password"] = new_password
-    if password_file:
-        args["password"] = new_password_file
-    args["override_password"] = True
-    # return the command to store data with the new password.
-    return [(name, "vault_archive", args)]
+    pwdargs = {arg: args[arg] for arg in copy_args if arg in args}
+
+    # retrieve vault stored data
+    try:
+        result = api_command(module, 'vault_retrieve', name, pwdargs)
+    except NotFound:
+        return None
+
+    return result['result'].get('data')
 
 
 def main():
@@ -595,10 +600,12 @@ def main():
                             default=None, required=False,
                             choices=["standard", "symmetric", "asymmetric"]),
             vault_public_key=dict(type="str", required=False, default=None,
-                                  aliases=['ipavaultpublickey', 'public_key']),
+                                  aliases=['ipavaultpublickey', 'public_key',
+                                           'new_public_key']),
             vault_public_key_file=dict(type="str", required=False,
                                        default=None,
-                                       aliases=['public_key_file']),
+                                       aliases=['public_key_file',
+                                                'new_public_key_file']),
             vault_private_key=dict(
                 type="str", required=False, default=None, no_log=True,
                 aliases=['ipavaultprivatekey', 'private_key']),
@@ -743,6 +750,11 @@ def main():
             res_find = find_vault(
                 ansible_module, name, username, service, shared)
 
+            # Set default vault_type if needed.
+            res_type = res_find.get('ipavaulttype')[0] if res_find else None
+            if vault_type is None:
+                vault_type = res_type if res_find is not None else u"symmetric"
+
             # Generate args
             args = gen_args(description, username, service, shared, vault_type,
                             salt, password, password_file, public_key,
@@ -750,14 +762,6 @@ def main():
                             datafile_out)
             pwdargs = None
 
-            # Set default vault_type if needed.
-            if vault_type is None and vault_data is not None:
-                if res_find is not None:
-                    res_vault_type = res_find.get('ipavaulttype')[0]
-                    args['ipavaulttype'] = vault_type = res_vault_type
-                else:
-                    args['ipavaulttype'] = vault_type = u"symmetric"
-
             # Create command
             if state == "present":
                 # verify data encription args
@@ -767,16 +771,52 @@ def main():
                     private_key_file, vault_data, datafile_in, datafile_out,
                     new_password, new_password_file, res_find)
 
-                # Found the vault
+                change_passwd = any([
+                    new_password, new_password_file,
+                    (private_key or private_key_file) and
+                    (public_key or public_key_file)
+                ])
                 if action == "vault":
+                    # Found the vault
                     if res_find is not None:
-                        # For all settings is args, check if there are
-                        # different settings in the find result.
-                        # If yes: modify
-                        if not compare_args_ipa(ansible_module, args,
-                                                res_find):
-                            commands.append([name, "vault_mod_internal", args])
+                        arg_type = args.get("ipavaulttype")
 
+                        modified = not compare_args_ipa(ansible_module,
+                                                        args, res_find)
+
+                        if arg_type != res_type or change_passwd:
+                            stargs = data_storage_args(
+                                res_type, args, vault_data, password,
+                                password_file, private_key,
+                                private_key_file, datafile_in,
+                                datafile_out)
+                            stored = get_stored_data(
+                                ansible_module, res_find, stargs
+                            )
+                            if stored:
+                                vault_data = \
+                                    (stored or b"").decode("utf-8")
+
+                            remove_attrs = {
+                                "symmetric": ["private_key", "public_key"],
+                                "asymmetric": ["password", "ipavaultsalt"],
+                                "standard": [
+                                    "private_key", "public_key",
+                                    "password", "ipavaultsalt"
+                                ],
+                            }
+                            for attr in remove_attrs.get(arg_type, []):
+                                if attr in args:
+                                    del args[attr]
+
+                            if vault_type == 'symmetric':
+                                if 'ipavaultsalt' not in args:
+                                    args['ipavaultsalt'] = os.urandom(32)
+                            else:
+                                args['ipavaultsalt'] = b''
+
+                        if modified:
+                            commands.append([name, "vault_mod_internal", args])
                     else:
                         if vault_type == 'symmetric' \
                            and 'ipavaultsalt' not in args:
@@ -852,16 +892,22 @@ def main():
                                                      ownerservices)
                         commands.append([name, 'vault_add_owner', owner_args])
 
-                pwdargs = data_storage_args(
-                    args, vault_data, password, password_file, private_key,
-                    private_key_file, datafile_in, datafile_out)
                 if any([vault_data, datafile_in]):
-                    commands.append([name, "vault_archive", pwdargs])
+                    if change_passwd:
+                        pwdargs = data_storage_args(
+                            vault_type, args, vault_data, new_password,
+                            new_password_file, private_key, private_key_file,
+                            datafile_in, datafile_out)
+                    else:
+                        pwdargs = data_storage_args(
+                            vault_type, args, vault_data, password,
+                            password_file, private_key, private_key_file,
+                            datafile_in, datafile_out)
 
-                cmds = change_password(
-                    ansible_module, res_find, password, password_file,
-                    new_password, new_password_file)
-                commands.extend(cmds)
+                    pwdargs['override_password'] = True
+                    pwdargs.pop("private_key", None)
+                    pwdargs.pop("private_key_file", None)
+                    commands.append([name, "vault_archive", pwdargs])
 
             elif state == "retrieved":
                 if res_find is None:
@@ -876,8 +922,9 @@ def main():
                     new_password, new_password_file, res_find)
 
                 pwdargs = data_storage_args(
-                    args, vault_data, password, password_file, private_key,
-                    private_key_file, datafile_in, datafile_out)
+                    res_find["ipavaulttype"][0], args, vault_data, password,
+                    password_file, private_key, private_key_file, datafile_in,
+                    datafile_out)
                 if 'data' in pwdargs:
                     del pwdargs['data']
 
@@ -889,6 +936,10 @@ def main():
 
                 if action == "vault":
                     if res_find is not None:
+                        remove = ['ipavaultsalt', 'ipavaultpublickey']
+                        args = {
+                            k: v for k, v in args.items() if k not in remove
+                        }
                         commands.append([name, "vault_del", args])
 
                 elif action == "member":
@@ -911,6 +962,10 @@ def main():
             else:
                 ansible_module.fail_json(msg="Unknown state '%s'" % state)
 
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
         # Execute commands
 
         errors = []
@@ -965,7 +1020,10 @@ def main():
         temp_kdestroy(ccache_dir, ccache_name)
 
     # Done
-    ansible_module.exit_json(changed=changed, **exit_args)
+
+    # exit_raw_json is a replacement for ansible_module.exit_json that
+    # does not mask the output.
+    exit_raw_json(ansible_module, changed=changed, **exit_args)
 
 
 if __name__ == "__main__":

requirements-dev.txt

@@ -1,2 +1,4 @@
 -r requirements-tests.txt
 ipdb
+pre-commit
+flake8-bugbear

requirements-tests.txt

@@ -2,6 +2,6 @@
 pytest>=2.7
 pytest-sourceorder>=0.5
 pytest-split-tests>=1.0.3
-testinfra>=5.0
+pytest-testinfra>=5.0
 jmespath>=0.9  # needed for the `json_query` filter
 pyyaml>=3

roles/ipabackup/README.md

@@ -0,0 +1,336 @@
+ipabackup role
+==============
+
+Description
+-----------
+
+This role allows to backup an IPA server, to copy a backup from the server to the controller, to copy all backups from the server to the controller, to remove a backup from the server, to remove all backups from the server, to restore an IPA server locally and from the controller and also to copy a backup from the controller to the server.
+
+
+**Note**: The ansible playbooks and role require a configured ansible environment where the ansible nodes are reachable and are properly set up to have an IP address and a working package manager.
+
+
+Features
+--------
+* Server backup
+* Server backup to controller
+* Copy backup from server to controller
+* Copy all backups from server to controller
+* Remove backup from the server
+* Remove all backups from the server
+* Server restore from server local backup.
+* Server restore from controller.
+* Copy a backup from the controller to the server.
+
+
+Supported FreeIPA Versions
+--------------------------
+
+FreeIPA versions 4.5 and up are supported by the backup role.
+
+
+Supported Distributions
+-----------------------
+
+* RHEL/CentOS 7.6+
+* Fedora 26+
+* Ubuntu
+
+
+Requirements
+------------
+
+**Controller**
+* Ansible version: 2.8+
+
+**Node**
+* Supported FreeIPA version (see above)
+* Supported distribution (needed for package installation only, see above)
+
+
+Usage
+=====
+
+Example inventory file with fixed domain and realm, setting up of the DNS server and using forwarders from /etc/resolv.conf:
+
+```ini
+[ipaserver]
+ipaserver.example.com
+```
+
+Example playbook to create a backup on the IPA server locally:
+
+```yaml
+---
+- name: Playbook to backup IPA server
+  hosts: ipaserver
+  become: true
+
+  roles:
+  - role: ipabackup
+    state: present
+```
+
+
+Example playbook to create a backup of the IPA server that is transferred to the controller using the server name as prefix for the backup and removed on the server:
+
+```yaml
+---
+- name: Playbook to backup IPA server to controller
+  hosts: ipaserver
+  become: true
+
+  vars:
+    ipabackup_to_controller: yes
+    # ipabackup_keep_on_server: yes
+
+  roles:
+  - role: ipabackup
+    state: present
+```
+
+
+Example playbook to create a backup of the IPA server that is transferred to the controller using the server name as prefix for the backup and kept on the server:
+
+```yaml
+---
+- name: Playbook to backup IPA server to controller
+  hosts: ipaserver
+  become: true
+
+  vars:
+    ipabackup_to_controller: yes
+    ipabackup_keep_on_server: yes
+
+  roles:
+  - role: ipabackup
+    state: present
+```
+
+
+Copy backup `ipa-full-2020-10-01-10-00-00` from server to controller:
+
+```yaml
+---
+- name: Playbook to copy backup from IPA server
+  hosts: ipaserver
+  become: true
+
+  vars:
+    ipabackup_name: ipa-full-2020-10-01-10-00-00
+    ipabackup_to_controller: yes
+
+  roles:
+  - role: ipabackup
+    state: copied
+```
+
+
+Copy backups `ipa-full-2020-10-01-10-00-00` and `ipa-full-2020-10-02-10-00-00` from server to controller:
+
+```yaml
+---
+- name: Playbook to copy backup from IPA server
+  hosts: ipaserver
+  become: true
+
+  vars:
+    ipabackup_name:
+    - ipa-full-2020-10-01-10-00-00
+    - ipa-full-2020-10-02-10-00-00
+    ipabackup_to_controller: yes
+
+  roles:
+  - role: ipabackup
+    state: copied
+```
+
+
+Copy all backups from server to controller that are following the backup naming scheme:
+
+```yaml
+---
+- name: Playbook to copy all backups from IPA server
+  hosts: ipaserver
+  become: true
+
+  vars:
+    ipabackup_name: all
+    ipabackup_to_controller: yes
+
+  roles:
+  - role: ipabackup
+    state: copied
+```
+
+
+Remove backup `ipa-full-2020-10-01-10-00-00` from server:
+
+```yaml
+---
+- name: Playbook to remove backup from IPA server
+  hosts: ipaserver
+  become: true
+
+  vars:
+    ipabackup_name: ipa-full-2020-10-01-10-00-00
+
+  roles:
+  - role: ipabackup
+    state: absent
+```
+
+
+Remove backups `ipa-full-2020-10-01-10-00-00` and `ipa-full-2020-10-02-10-00-00` from server:
+
+```yaml
+---
+- name: Playbook to remove backup from IPA server
+  hosts: ipaserver
+  become: true
+
+  vars:
+    ipabackup_name:
+    - ipa-full-2020-10-01-10-00-00
+    - ipa-full-2020-10-02-10-00-00
+
+  roles:
+  - role: ipabackup
+    state: absent
+```
+
+
+Remove all backups from server that are following the backup naming scheme:
+
+```yaml
+---
+- name: Playbook to remove all backups from IPA server
+  hosts: ipaserver
+  become: true
+
+  vars:
+    ipabackup_name: all
+
+  roles:
+  - role: ipabackup
+    state: absent
+```
+
+
+Example playbook to restore an IPA server locally:
+
+```yaml
+---
+- name: Playbook to restore an IPA server
+  hosts: ipaserver
+  become: true
+
+  vars:
+    ipabackup_name: ipa-full-2020-10-22-11-11-44
+    ipabackup_password: SomeDMpassword
+
+  roles:
+  - role: ipabackup
+    state: restored
+```
+
+
+Example playbook to restore IPA server from controller:
+
+```yaml
+---
+- name: Playbook to restore IPA server from controller
+  hosts: ipaserver
+  become: true
+
+  vars:
+    ipabackup_name: ipaserver.test.local_ipa-full-2020-10-22-11-11-44
+    ipabackup_password: SomeDMpassword
+    ipabackup_from_controller: yes
+
+  roles:
+  - role: ipabackup
+    state: restored
+```
+
+
+Example playbook to copy a backup from controller to the IPA server:
+
+```yaml
+---
+- name: Playbook to copy a backup from controller to the IPA server
+  hosts: ipaserver
+  become: true
+
+  vars:
+    ipabackup_name: ipaserver.test.local_ipa-full-2020-10-22-11-11-44
+    ipabackup_from_controller: yes
+
+  roles:
+  - role: ipabackup
+    state: copied
+```
+
+
+Playbooks
+=========
+
+The example playbooks to do the backup, copy a backup and also to remove a backup, also to do the restore, copy a backup to the server are part of the repository in the playbooks folder.
+
+```
+backup-server.yml
+backup-server-to-controller.yml
+copy-all-backups-from-server.yml
+copy-backup-from-server.yml
+remove-all-backups-from-server.yml
+remove-backup-from-server.yml
+restore-server.yml
+restore-server-from-controller.yml
+copy-backup-from-controller.yml
+```
+
+Please remember to link or copy the playbooks to the base directory of ansible-freeipa if you want to use the roles within the source archive.
+
+
+Variables
+=========
+
+Base Variables
+--------------
+
+Variable | Description | Required
+-------- | ----------- | --------
+ipabackup_backend | The backend to restore within the instance or instances, str | no
+ipabackup_data | Backup only the data with `state: present` and restore only the data with `state: restored`, bool (default: `no`) | no
+ipabackup_disable_role_check | Perform the backup even if this host does not have all the roles used in the cluster. This is not recommended, bool (default: `no`) | no
+ipabackup_gpg | Encrypt the backup, bool (default: `no`) | no
+ipabackup_gpg_keyring | Full path to the GPG keyring without the file extension, only for GPG 1 and up to IPA 4.6 str | no
+ipabackup_instance | The 389-ds instance to restore (defaults to all found), str | no
+ipabackup_log_file | Log to the given file on server for `state: present` and `state: restored` only, string | no
+ipabackup_logs | Include log files in backup, bool (default: `no`) | no
+ipabackup_no_logs | Do not restore log files from the backup, bool (default: `no`) | no
+ipabackup_online | Perform the LDAP backups online for data only with `state: present` and perform the LDAP restore online for data only with `state: restored`. If `ipabackup_data` is not set it will automatically be enabled.  bool (default: `no`) | no
+ipabackup_password | The diretory manager password needed for restoring a backup with `state: restored`, str | no
+state | `present` to make a new backup, `absent` to remove a backup and `copied` to copy a backup from the server to the controller or from the controller to the server, `restored` to restore a backup. string (default: `present`) | yes
+
+
+Special Variables
+-----------------
+
+Variable | Description | Required
+-------- | ----------- | --------
+ipabackup_name | The IPA backup name(s). Only for removal of server local backup(s) with `state: absent`, to copy server local backup(s) to the controller with `state: copied` and `ipabackup_from_server` set, to copy a backup from the controller to the server with `state: copied` and `ipabackup_from_controller` set or to restore a backup with `state: restored` either locally on the server of from the controller with `ipabackup_from_controller` set. If `all` is used all available backups are copied or removed that are following the backup naming scheme. string list | no
+ipabackup_keep_on_server | Keep local copy of backup on server with `state: present` and `ipabackup_to_controller`, bool (default: `no`) | no
+ipabackup_to_controller | Copy backup to controller, prefixes backup with node name, remove backup on server if `ipabackup_keep_on_server` is not set, bool (default: `no`) | no
+ipabackup_controller_path | Pre existing path on controller to store the backup in with `state: present`, path on the controller to copy the backup from with `state: copied` and `ipabackup_from_controller` set also for the restore with `state: restored` and `ipabackup_from_controller` set. If this is not set, the current working dir is used. string | no
+ipabackup_name_prefix | Set prefix to use for backup directory on controller with `state: present` or `state: copied` and `ipabackup_to_controller` set, The default is the server FQDN, string | no
+ipabackup_from_controller | Copy backup from controller to server, restore if `state: restored`, copy backup to server if `state: copied`, bool (default: `no`) | no
+ipabackup_install_packages | Install needed packages to be able to apply the backup with `state: restored`, bool (default: `yes`) | no
+ipabackup_firewalld_zone | The value defines the firewall zone that will be used with `state: restored`. This needs to be an existing runtime and permanent zone, bool (default: `no`) | no
+ipabackup_setup_firewalld | The value defines if the needed services will automatically be opened in the firewall managed by firewalld with `state: restored`, bool (default: `yes`) | no
+
+
+Authors
+=======
+
+Thomas Woerner

roles/ipabackup/defaults/main.yml

@@ -0,0 +1,16 @@
+---
+# defaults file for ipabackup
+
+ipabackup_gpg: no
+ipabackup_data: no
+ipabackup_logs: no
+ipabackup_online: no
+ipabackup_disable_role_check: no
+ipabackup_no_logs: no
+
+### special ###
+ipabackup_keep_on_server: no
+ipabackup_to_controller: no
+ipabackup_from_controller: no
+ipabackup_install_packages: yes
+ipabackup_setup_firewalld: yes

roles/ipabackup/meta/main.yml

@@ -0,0 +1,20 @@
+dependencies: []
+
+galaxy_info:
+  author: Thomas Woerner
+  description: A role to backup and restore an IPA server
+  company: Red Hat, Inc
+  license: GPLv3
+  min_ansible_version: 2.8
+  platforms:
+  - name: Fedora
+    versions:
+    - all
+  - name: EL
+    versions:
+    - 7
+    - 8
+  galaxy_tags:
+    - identity
+    - ipa
+    - freeipa

roles/ipabackup/tasks/backup.yml

@@ -0,0 +1,39 @@
+---
+# tasks file for ipabackup
+
+- name: Create backup
+  shell: >
+    ipa-backup
+    {{ "--gpg" if ipabackup_gpg | bool else "" }}
+    {{ "--gpg-keyring="+ipabackup_gpg_keyring if ipabackup_gpg_keyring is defined else "" }}
+    {{ "--data" if ipabackup_data | bool else "" }}
+    {{ "--logs" if ipabackup_logs | bool else "" }}
+    {{ "--online" if ipabackup_online | bool else "" }}
+    {{ "--disable-role-check" if ipabackup_disable_role_check | bool else "" }}
+    {{ "--log-file="+ipabackup_log_file if ipabackup_log_file is defined else "" }}
+  register: result_ipabackup
+
+- block:
+  - name: Get ipabackup_item from stderr or stdout output
+    set_fact:
+      ipabackup_item: "{{ item | regex_search('\n.*/([^\n]+)','\\1') | first }}"
+    when: item.find("Backed up to "+ipabackup_dir+"/") > 0
+    with_items:
+    - "{{ result_ipabackup.stderr }}"
+    - "{{ result_ipabackup.stdout }}"
+    loop_control:
+      label: ""
+
+  - name: Fail on missing ipabackup_item
+    fail: msg="Failed to get ipabackup_item"
+    when: ipabackup_item is not defined
+
+  - name: Copy backup to controller
+    include_tasks: "{{ role_path }}/tasks/copy_backup_from_server.yml"
+    when: state|default("present") == "present"
+
+  - name: Remove backup on server
+    include_tasks: "{{ role_path }}/tasks/remove_backup_from_server.yml"
+    when: not ipabackup_keep_on_server
+
+  when: ipabackup_to_controller

roles/ipabackup/tasks/copy_backup_from_server.yml

@@ -0,0 +1,46 @@
+---
+- name: Fail on invalid ipabackup_item
+  fail: msg="ipabackup_item {{ ipabackup_item }} is not valid"
+  when: ipabackup_item is not defined or
+        ipabackup_item | length < 1 or
+        (ipabackup_item.find("ipa-full-") == -1 and
+         ipabackup_item.find("ipa-data-") == -1)
+
+- name: Set controller destination directory
+  set_fact:
+    ipabackup_controller_dir:
+        "{{ ipabackup_controller_path | default(lookup('env','PWD')) }}/{{
+         ipabackup_name_prefix | default(ansible_fqdn) }}_{{
+         ipabackup_item }}/"
+
+- name: Stat backup on server
+  stat:
+    path: "{{ ipabackup_dir }}/{{ ipabackup_item }}"
+  register: result_backup_stat
+
+- name: Fail on missing backup directory
+  fail: msg="Unable to find backup {{ ipabackup_item }}"
+  when: result_backup_stat.stat.isdir is not defined
+
+- name: Get backup files to copy for "{{ ipabackup_item }}"
+  shell:
+    find . -type f | cut -d"/" -f 2
+  args:
+    chdir: "{{ ipabackup_dir }}/{{ ipabackup_item }}"
+  register: result_find_backup_files
+
+- name: Copy server backup files to controller
+  fetch:
+    flat: yes
+    src: "{{ ipabackup_dir }}/{{ ipabackup_item }}/{{ item }}"
+    dest: "{{ ipabackup_controller_dir }}"
+  with_items:
+  - "{{ result_find_backup_files.stdout_lines }}"
+
+- name: Fix file modes for backup on controller
+  file:
+    dest: "{{ ipabackup_controller_dir }}"
+    mode: u=rwX,go=
+    recurse: yes
+  delegate_to: localhost
+  become: no

roles/ipabackup/tasks/copy_backup_to_server.yml

@@ -0,0 +1,43 @@
+---
+- name: Fail on invalid ipabackup_name
+  fail: msg="ipabackup_name {{ ipabackup_name }} is not valid"
+  when: ipabackup_name is not defined or
+        ipabackup_name | length < 1 or
+        (ipabackup_name.find("ipa-full-") == -1 and
+         ipabackup_name.find("ipa-data-") == -1)
+
+- name: Set controller source directory
+  set_fact:
+    ipabackup_controller_dir:
+      "{{ ipabackup_controller_path | default(lookup('env','PWD')) }}"
+
+- name: Set ipabackup_item
+  set_fact:
+    ipabackup_item:
+      "{{ ipabackup_name | regex_search('.*_(ipa-.+)','\\1') | first }}"
+  when: "'_ipa-' in ipabackup_name"
+
+- name: Set ipabackup_item
+  set_fact:
+    ipabackup_item: "{{ ipabackup_name }}"
+  when: "'_ipa-' not in ipabackup_name"
+
+- name: Stat backup to copy
+  stat:
+    path: "{{ ipabackup_controller_dir }}/{{ ipabackup_name }}"
+  register: result_backup_stat
+  delegate_to: localhost
+  become: no
+
+- name: Fail on missing backup to copy
+  fail: msg="Unable to find backup {{ ipabackup_name }}"
+  when: result_backup_stat.stat.isdir is not defined
+
+- name: Copy backup files to server for "{{ ipabackup_item }}"
+  copy:
+    src: "{{ ipabackup_controller_dir }}/{{ ipabackup_name }}/"
+    dest: "{{ ipabackup_dir }}/{{ ipabackup_item }}"
+    owner: root
+    group: root
+    mode: u=rw,go=r
+    directory_mode: u=rwx,go=

roles/ipabackup/tasks/get_ipabackup_dir.yml

@@ -0,0 +1,12 @@
+---
+- name: Get IPA_BACKUP_DIR dir from ipaplatform
+  command: "{{ ansible_playbook_python }}"
+  args:
+    stdin: |
+      from ipaplatform.paths import paths
+      print(paths.IPA_BACKUP_DIR)
+  register: result_ipaplatform_backup_dir
+
+- name: Set IPA backup dir
+  set_fact:
+    ipabackup_dir: "{{ result_ipaplatform_backup_dir.stdout_lines | first }}"

roles/ipabackup/tasks/main.yml

@@ -0,0 +1,138 @@
+---
+# tasks file for ipabackup
+
+- name: Check for empty vars
+  fail: msg="Variable {{ item }} is empty"
+  when: "item in vars and not vars[item]"
+  with_items: "{{ ipabackup_empty_var_checks }}"
+  vars:
+    ipabackup_empty_var_checks:
+    - ipabackup_backend
+    - ipabackup_gpg_keyring
+    - ipabackup_instance
+    - ipabackup_log_file
+    - ipabackup_password
+    - ipabackup_name
+    - ipabackup_controller_path
+    - ipabackup_name_prefix
+    - ipabackup_firewalld_zone
+
+- name: Set ipabackup_data if ipabackup_data is not set but ipabackup_online is
+  set_fact:
+    ipabackup_data: yes
+  when: ipabackup_online | bool and not ipabackup_data | bool
+
+- name: Fail if ipabackup_from_controller and ipabackup_to_controller are set
+  fail: msg="ipabackup_from_controller and ipabackup_to_controller are set"
+  when: ipabackup_from_controller | bool and ipabackup_to_controller | bool
+
+- name: Get ipabackup_dir from IPA installation
+  include_tasks: "{{ role_path }}/tasks/get_ipabackup_dir.yml"
+
+- name: Backup IPA server
+  include_tasks: "{{ role_path }}/tasks/backup.yml"
+  when: state|default("present") == "present"
+
+- name: Fail for given ipabackup_name if state is not copied, restored or absent
+  fail: msg="ipabackup_name is given and state is not copied, restored or absent"
+  when: state is not defined or
+        (state != "copied" and state != "restored" and state != "absent") and
+        ipabackup_name is defined
+
+- name: Fail on missing ipabackup_name
+  fail: msg="ipabackup_name is not set"
+  when: (ipabackup_name is not defined or not ipabackup_name) and
+        state is defined and
+        (state == "copied" or state == "restored" or state == "absent")
+
+- block:
+  - name: Get list of all backups on IPA server
+    shell:
+      find . -name "ipa-full-*" -o -name "ipa-data-*" | cut -d"/" -f 2
+    args:
+      chdir: "{{ ipabackup_dir }}/"
+    register: result_backup_find_backup_files
+
+  - name: Set ipabackup_names using backup list
+    set_fact:
+      ipabackup_names: "{{ result_backup_find_backup_files.stdout_lines }}"
+
+  when: state is defined and
+        ((state == "copied" and ipabackup_to_controller) or
+         state == "absent") and
+        ipabackup_name is defined and ipabackup_name == "all"
+
+- block:
+  - name: Fail on ipabackup_name all
+    fail: msg="ipabackup_name can not be all in this case"
+    when: ipabackup_name is defined and ipabackup_name == "all"
+
+  - name: Set ipabackup_names from ipabackup_name string
+    set_fact:
+      ipabackup_names: ["{{ ipabackup_name }}"]
+    when: ipabackup_name | type_debug != "list"
+
+  - name: Set ipabackup_names from ipabackup_name list
+    set_fact:
+      ipabackup_names: "{{ ipabackup_name }}"
+    when: ipabackup_name | type_debug == "list"
+  when: ipabackup_names is not defined and ipabackup_name is defined
+
+- name: Set empty ipabackup_names if ipabackup_name is not defined
+  set_fact:
+    ipabackup_names: []
+  when: ipabackup_names is not defined and ipabackup_name is not defined
+
+- block:
+  - name: Copy backup from IPA server
+    include_tasks: "{{ role_path }}/tasks/copy_backup_from_server.yml"
+    vars:
+      ipabackup_item: "{{ main_item | basename }}"
+    with_items:
+    - "{{ ipabackup_names }}"
+    loop_control:
+      loop_var: main_item
+    when: state is defined and state == "copied"
+
+  - name: Remove backup from IPA server
+    include_tasks: "{{ role_path }}/tasks/remove_backup_from_server.yml"
+    vars:
+      ipabackup_item: "{{ main_item | basename }}"
+    with_items:
+    - "{{ ipabackup_names }}"
+    loop_control:
+      loop_var: main_item
+    when: state is defined and state == "absent"
+
+  when: state is defined and
+        ((state == "copied" and ipabackup_to_controller) or state == "absent")
+
+# Fail with more than one entry in ipabackup_names for copy to sever and
+# restore.
+
+- name: Fail to copy or restore more than one backup on the server
+  fail: msg="Only one backup can be copied to the server or restored"
+  when: state is defined and (state == "copied" or state == "restored") and
+        ipabackup_from_controller | bool and ipabackup_names | length != 1
+
+# Use only first item in ipabackup_names for copy to server and for restore.
+
+- block:
+  - name: Copy backup to server
+    include_tasks: "{{ role_path }}/tasks/copy_backup_to_server.yml"
+
+  - name: Restore IPA server after copy
+    include_tasks: "{{ role_path }}/tasks/restore.yml"
+    when: state|default("present") == "restored"
+
+  vars:
+    ipabackup_name: "{{ ipabackup_names[0] }}"
+  when: ipabackup_from_controller or
+        (state|default("present") == "copied" and not ipabackup_to_controller)
+
+- name: Restore IPA server
+  include_tasks: "{{ role_path }}/tasks/restore.yml"
+  vars:
+    ipabackup_item: "{{ ipabackup_names[0] | basename }}"
+  when: not ipabackup_from_controller and
+        state|default("present") == "restored"

roles/ipabackup/tasks/remove_backup_from_server.yml

@@ -0,0 +1,5 @@
+---
+- name: Remove backup "{{ ipabackup_item }}"
+  file:
+    path: "{{ ipabackup_dir }}/{{ ipabackup_item }}"
+    state: absent

roles/ipabackup/tasks/restore.yml

@@ -0,0 +1,147 @@
+---
+# tasks file for ipabackup
+
+### VARIABLES
+
+- name: Import variables specific to distribution
+  include_vars: "{{ item }}"
+  with_first_found:
+    - "{{ role_path }}/vars/{{ ansible_distribution }}-{{ ansible_distribution_version }}.yml"
+    - "{{ role_path }}/vars/{{ ansible_distribution }}-{{ ansible_distribution_major_version }}.yml"
+    - "{{ role_path }}/vars/{{ ansible_distribution }}.yml"
+    - "{{ role_path }}/vars/default.yml"
+
+### GET SERVICES FROM BACKUP
+
+- name: Stat backup on server
+  stat:
+    path: "{{ ipabackup_dir }}/{{ ipabackup_item }}"
+  register: result_backup_stat
+
+- name: Fail on missing backup directory
+  fail: msg="Unable to find backup {{ ipabackup_item }}"
+  when: result_backup_stat.stat.isdir is not defined
+
+- name: Stat header file in backup "{{ ipabackup_item }}"
+  stat:
+    path: "{{ ipabackup_dir }}/{{ ipabackup_item }}/header"
+  register: result_backup_header_stat
+
+- name: Fail on missing header file in backup
+  fail: msg="Unable to find backup {{ ipabackup_item }} header file"
+  when: result_backup_header_stat.stat.isreg is not defined
+
+- name: Get services from backup
+  shell: >
+    grep "^services = " "{{ ipabackup_dir }}/{{ ipabackup_item }}/header" | cut -d"=" -f2 | tr -d '[:space:]'
+  register: result_services_grep
+
+- name: Set ipabackup_services
+  set_fact:
+    ipabackup_services: "{{ result_services_grep.stdout.split(',') }}"
+    ipabackup_service_dns: DNS
+    ipabackup_service_adtrust: ADTRUST
+    ipabackup_service_ntp: NTP
+
+### INSTALL PACKAGES
+
+- block:
+  - name: Ensure that IPA server packages are installed
+    package:
+      name: "{{ ipaserver_packages }}"
+      state: present
+
+  - name: Ensure that IPA server packages for dns are installed
+    package:
+      name: "{{ ipaserver_packages_dns }}"
+      state: present
+    when: ipabackup_service_dns in ipabackup_services
+
+  - name: Ensure that IPA server packages for adtrust are installed
+    package:
+      name: "{{ ipaserver_packages_adtrust }}"
+      state: present
+    when: ipabackup_service_adtrust in ipabackup_services
+
+  - name: Ensure that firewalld packages are installed
+    package:
+      name: "{{ ipaserver_packages_firewalld }}"
+      state: present
+    when: ipabackup_setup_firewalld | bool
+
+  when: ipabackup_install_packages | bool
+
+### START FIREWALLD
+
+- block:
+  - name: Ensure that firewalld is running
+    systemd:
+      name: firewalld
+      enabled: yes
+      state: started
+
+  - name: Firewalld - Verify runtime zone "{{ ipabackup_firewalld_zone }}"
+    shell: >
+      firewall-cmd
+      --info-zone="{{ ipabackup_firewalld_zone }}"
+      >/dev/null
+    when: ipabackup_firewalld_zone is defined
+
+  - name: Firewalld - Verify permanent zone "{{ ipabackup_firewalld_zone }}"
+    shell: >
+      firewall-cmd
+      --permanent
+      --info-zone="{{ ipabackup_firewalld_zone }}"
+      >/dev/null
+    when: ipabackup_firewalld_zone is defined
+
+  when: ipabackup_setup_firewalld | bool
+
+### RESTORE
+
+- name: Restore backup
+  no_log: True
+  shell: >
+    ipa-restore
+    {{ ipabackup_item }}
+    --unattended
+    {{ "--password="+ipabackup_password if ipabackup_password is defined else "" }}
+    {{ "--data" if ipabackup_data | bool else "" }}
+    {{ "--online" if ipabackup_online | bool else "" }}
+    {{ "--instance="+ipabackup_instance if ipabackup_instance is defined else "" }}
+    {{ "--backend="+ipabackup_backend if ipabackup_backend is defined else "" }}
+    {{ "--no-logs" if ipabackup_no_logs | bool else "" }}
+    {{ "--log-file="+ipabackup_log_file if ipabackup_log_file is defined else "" }}
+  register: result_iparestore
+  ignore_errors: yes
+
+- name: Report error for restore operation
+  debug:
+    msg: "{{ result_iparestore.stderr }}"
+  when: result_iparestore is failed
+  failed_when: yes
+
+### CONFIGURE FIREWALLD
+
+- name: Configure firewalld
+  command: >
+    firewall-cmd
+    --permanent
+    {{ "--zone="+ipabackup_firewalld_zone if ipabackup_firewalld_zone is defined else "" }}
+    --add-service=freeipa-ldap
+    --add-service=freeipa-ldaps
+    {{ "--add-service=freeipa-trust" if ipabackup_service_adtrust in ipabackup_services else "" }}
+    {{ "--add-service=dns" if ipabackup_service_dns in ipabackup_services else "" }}
+    {{ "--add-service=ntp" if ipabackup_service_ntp in ipabackup_services else "" }}
+  when: ipabackup_setup_firewalld | bool
+
+- name: Configure firewalld runtime
+  command: >
+    firewall-cmd
+    {{ "--zone="+ipabackup_firewalld_zone if ipabackup_firewalld_zone is defined else "" }}
+    --add-service=freeipa-ldap
+    --add-service=freeipa-ldaps
+    {{ "--add-service=freeipa-trust" if ipabackup_service_adtrust in ipabackup_services else "" }}
+    {{ "--add-service=dns" if ipabackup_service_dns in ipabackup_services else "" }}
+    {{ "--add-service=ntp" if ipabackup_service_ntp in ipabackup_services else "" }}
+  when: ipabackup_setup_firewalld | bool

roles/ipabackup/vars/CentOS-7.yml

@@ -0,0 +1,6 @@
+# defaults file for ipaserver
+# vars/rhel.yml
+ipaserver_packages: [ "ipa-server", "libselinux-python" ]
+ipaserver_packages_dns: [ "ipa-server-dns" ]
+ipaserver_packages_adtrust: [ "ipa-server-trust-ad" ]
+ipaserver_packages_firewalld: [ "firewalld" ]

roles/ipabackup/vars/CentOS-8.yml

@@ -0,0 +1 @@
+RedHat-8.yml

roles/ipabackup/vars/Fedora.yml

@@ -0,0 +1,4 @@
+ipaserver_packages: [ "freeipa-server" ]
+ipaserver_packages_dns: [ "freeipa-server-dns" ]
+ipaserver_packages_adtrust: [ "freeipa-server-trust-ad" ]
+ipaserver_packages_firewalld: [ "firewalld" ]

roles/ipabackup/vars/OracleLinux-7.yml

@@ -0,0 +1 @@
+RedHat-7.yml

roles/ipabackup/vars/OracleLinux-8.yml

@@ -0,0 +1 @@
+RedHat-8.yml

roles/ipabackup/vars/RedHat-7.3.yml

@@ -0,0 +1,6 @@
+# defaults file for ipaserver
+# vars/rhel.yml
+ipaserver_packages: [ "ipa-server", "libselinux-python" ]
+ipaserver_packages_dns: [ "ipa-server-dns" ]
+ipaserver_packages_adtrust: [ "ipa-server-trust-ad" ]
+ipaserver_packages_firewalld: [ "firewalld" ]

roles/ipabackup/vars/RedHat-7.yml

@@ -0,0 +1,6 @@
+# defaults file for ipaserver
+# vars/rhel.yml
+ipaserver_packages: [ "ipa-server", "libselinux-python" ]
+ipaserver_packages_dns: [ "ipa-server-dns" ]
+ipaserver_packages_adtrust: [ "ipa-server-trust-ad" ]
+ipaserver_packages_firewalld: [ "firewalld" ]

roles/ipabackup/vars/RedHat-8.yml

@@ -0,0 +1,6 @@
+# defaults file for ipaserver
+# vars/RedHat-8.yml
+ipaserver_packages: [ "@idm:DL1/server" ]
+ipaserver_packages_dns: [ "@idm:DL1/dns" ]
+ipaserver_packages_adtrust: [ "@idm:DL1/adtrust" ]
+ipaserver_packages_firewalld: [ "firewalld" ]

roles/ipabackup/vars/Ubuntu.yml

@@ -0,0 +1,5 @@
+# vars/Ubuntu.yml
+ipaserver_packages: [ "freeipa-server" ]
+ipaserver_packages_dns: [ "freeipa-server-dns" ]
+ipaserver_packages_adtrust: [ "freeipa-server-trust-ad" ]
+ipaserver_packages_firewalld: [ "firewalld" ]

roles/ipabackup/vars/default.yml

@@ -0,0 +1,6 @@
+# defaults file for ipaserver
+# vars/default.yml
+ipaserver_packages: [ "ipa-server" ]
+ipaserver_packages_dns: [ "ipa-server-dns" ]
+ipaserver_packages_adtrust: [ "freeipa-server-trust-ad" ]
+ipaserver_packages_firewalld: [ "firewalld" ]

roles/ipaclient/tasks/install.yml

@@ -181,8 +181,12 @@
     # Do not fail on error codes 3 and 5:
     #   3 - Unable to open keytab
     #   5 - Principal name or realm not found in keytab
+    #   7 - Failed to set cursor, typically when errcode
+    #       would be issued in past
     failed_when: result_ipa_rmkeytab.rc != 0 and
-                 result_ipa_rmkeytab.rc != 3 and result_ipa_rmkeytab.rc != 5
+                 result_ipa_rmkeytab.rc != 3 and
+                 result_ipa_rmkeytab.rc != 5 and
+                 result_ipa_rmkeytab.rc != 7
     when: (ipaclient_use_otp | bool or ipaclient_force_join | bool) and not ipaclient_on_master | bool
 
   - name: Install - Backup and set hostname

roles/ipareplica/README.md

@@ -153,13 +153,15 @@ Variable | Description | Required
 `ipareplica_no_host_dns` | Do not use DNS for hostname lookup during installation. (bool, default: false) | no
 `ipareplica_skip_conncheck` | Skip connection check to remote master. (bool, default: false) | no
 `ipareplica_pki_config_override` | Path to ini file with config overrides. This is only usable with recent FreeIPA versions. (string) | no
+`ipareplica_mem_check` | Checking for minimum required memory for the deployment.  This is only usable with recent FreeIPA versions (4.8.10+) else ignored. (bool, default: yes) | no
 
-Server Vaiables
----------------
+Server Variables
+----------------
 
 Variable | Description | Required
 -------- | ----------- | --------
 `ipadm_password` | The password for the Directory Manager. (string) | mostly
+`ipareplica_hidden_replica` | Install a hidden replica. (bool, default: false) | no
 `ipareplica_setup_adtrust` | Configure AD trust capability. (bool, default: false) | no
 `ipareplica_setup_ca` | Configure a dogtag CA. (bool, default: false) | no
 `ipareplica_setup_kra` | Configure a dogtag KRA. (bool, default: false) | no

roles/ipareplica/defaults/main.yml

@@ -5,6 +5,7 @@
 ipareplica_no_host_dns: no
 ipareplica_skip_conncheck: no
 ipareplica_hidden_replica: no
+ipareplica_mem_check: yes
 ### server ###
 ipareplica_setup_adtrust: no
 ipareplica_setup_ca: no

roles/ipareplica/library/ipareplica_prepare.py

@@ -325,8 +325,6 @@ def main():
         'external_cert_files')
     # options.subject_base = ansible_module.params.get('subject_base')
     # options.ca_subject = ansible_module.params.get('ca_subject')
-    options.no_dnssec_validation = ansible_module.params.get(
-        'no_dnssec_validation')
     # dns
     options.allow_zone_overlap = ansible_module.params.get(
         'allow_zone_overlap')
@@ -338,7 +336,7 @@ def main():
     options.auto_forwarders = ansible_module.params.get('auto_forwarders')
     options.forward_policy = ansible_module.params.get('forward_policy')
     options.no_dnssec_validation = ansible_module.params.get(
-        'no_dnssec_validationdnssec_validation')
+        'no_dnssec_validation')
     # ad trust
     options.enable_compat = ansible_module.params.get('enable_compat')
     options.netbios_name = ansible_module.params.get('netbios_name')

roles/ipareplica/library/ipareplica_setup_dns.py

@@ -143,7 +143,7 @@ def main():
     options.forwarders = ansible_module.params.get('forwarders')
     options.forward_policy = ansible_module.params.get('forward_policy')
     options.no_dnssec_validation = ansible_module.params.get(
-        'no_dnssec_validationdnssec_validation')
+        'no_dnssec_validation')
     # additional
     dns.ip_addresses = ansible_module_get_parsed_ip_addresses(
         ansible_module, 'dns_ip_addresses')

roles/ipareplica/library/ipareplica_test.py

@@ -57,9 +57,15 @@ options:
   hidden_replica:
     description: Install a hidden replica
     required: yes
+  skip_mem_check:
+    description: Skip checking for minimum required memory
+    required: yes
   setup_adtrust:
     description: Configure AD trust capability
     required: yes
+  setup_ca:
+    description: Configure a dogtag CA
+    required: yes
   setup_kra:
     description: Configure a dogtag KRA
     required: yes
@@ -152,8 +158,10 @@ def main():
             hostname=dict(required=False),
             ca_cert_files=dict(required=False, type='list', default=[]),
             hidden_replica=dict(required=False, type='bool', default=False),
+            skip_mem_check=dict(required=False, type='bool', default=False),
             # server
             setup_adtrust=dict(required=False, type='bool', default=False),
+            setup_ca=dict(required=False, type='bool'),
             setup_kra=dict(required=False, type='bool', default=False),
             setup_dns=dict(required=False, type='bool', default=False),
             no_pkinit=dict(required=False, type='bool', default=False),
@@ -196,8 +204,10 @@ def main():
     options.host_name = ansible_module.params.get('hostname')
     options.ca_cert_files = ansible_module.params.get('ca_cert_files')
     options.hidden_replica = ansible_module.params.get('hidden_replica')
+    options.skip_mem_check = ansible_module.params.get('skip_mem_check')
     # server
     options.setup_adtrust = ansible_module.params.get('setup_adtrust')
+    options.setup_ca = ansible_module.params.get('setup_ca')
     options.setup_kra = ansible_module.params.get('setup_kra')
     options.setup_dns = ansible_module.params.get('setup_dns')
     options.no_pkinit = ansible_module.params.get('no_pkinit')
@@ -404,7 +414,12 @@ def main():
     # check selinux status, http and DS ports, NTP conflicting services
     try:
         with redirect_stdout(ansible_log):
-            common_check(options.no_ntp)
+            argspec = inspect.getargspec(common_check)
+            if "skip_mem_check" in argspec.args:
+                common_check(options.no_ntp, options.skip_mem_check,
+                             options.setup_ca)
+            else:
+                common_check(options.no_ntp)
     except Exception as msg:  # ScriptError as msg:
         _msg = str(msg)
         if "server is already configured" in _msg:

roles/ipareplica/tasks/install.yml

@@ -75,8 +75,10 @@
     hostname: "{{ ipareplica_hostname | default(ansible_fqdn) }}"
     ca_cert_files: "{{ ipareplica_ca_cert_files | default([]) }}"
     hidden_replica: "{{ ipareplica_hidden_replica }}"
+    skip_mem_check: "{{ not ipareplica_mem_check }}"
     ### server ###
     setup_adtrust: "{{ ipareplica_setup_adtrust }}"
+    setup_ca: "{{ ipareplica_setup_ca }}"
     setup_kra: "{{ ipareplica_setup_kra }}"
     setup_dns: "{{ ipareplica_setup_dns }}"
     no_pkinit: "{{ ipareplica_no_pkinit }}"

roles/ipaserver/README.md

@@ -205,6 +205,7 @@ Variable | Description | Required
 `ipaserver_realm` | The Kerberos realm of an existing IPA deployment. (string) | no
 `ipaserver_hostname` | Fully qualified name of the server. (string) | no
 `ipaserver_no_host_dns` | Do not use DNS for hostname lookup during installation. (bool, default: false) | no
+`ipaserver_mem_check` | Checking for minimum required memory for the deployment. This is only usable with recent FreeIPA versions (4.8.10+) else ignored. (bool, default: yes) | no
 
 Server Variables
 ----------------

roles/ipaserver/defaults/main.yml

@@ -10,6 +10,7 @@ ipaserver_setup_dns: no
 ipaserver_no_hbac_allow: no
 ipaserver_no_pkinit: no
 ipaserver_no_ui_redirect: no
+ipaserver_mem_check: yes
 ### ssl certificate ###
 ### client ###
 ipaclient_mkhomedir: no

roles/ipaserver/library/ipaserver_test.py

@@ -66,6 +66,9 @@ options:
   pki_config_override:
     description: Path to ini file with config overrides
     required: yes
+  skip_mem_check:
+    description: Skip checking for minimum required memory
+    required: yes
   setup_adtrust:
     description: Configure AD trust capability
     required: yes
@@ -221,7 +224,7 @@ from ansible.module_utils.ansible_ipa_server import (
     read_cache, ca, tasks, check_ldap_conf, timeconf, httpinstance,
     check_dirsrv, ScriptError, get_fqdn, verify_fqdn, BadHostError,
     validate_domain_name, load_pkcs12, IPA_PYTHON_VERSION,
-    encode_certificate
+    encode_certificate, check_available_memory
 )
 
 if six.PY3:
@@ -242,6 +245,7 @@ def main():
             ca_cert_files=dict(required=False, type='list', default=[]),
             no_host_dns=dict(required=False, type='bool', default=False),
             pki_config_override=dict(required=False),
+            skip_mem_check=dict(required=False, type='bool', default=False),
             # server
             setup_adtrust=dict(required=False, type='bool', default=False),
             setup_kra=dict(required=False, type='bool', default=False),
@@ -322,6 +326,7 @@ def main():
     options.no_host_dns = ansible_module.params.get('no_host_dns')
     options.pki_config_override = ansible_module.params.get(
         'pki_config_override')
+    options.skip_mem_check = ansible_module.params.get('skip_mem_check')
     # server
     options.setup_adtrust = ansible_module.params.get('setup_adtrust')
     options.setup_dns = ansible_module.params.get('setup_dns')
@@ -855,8 +860,12 @@ def main():
     if options.ca_subject:
         ca.subject_validator(ca.VALID_SUBJECT_ATTRS, options.ca_subject)
 
-    # IPv6 and SELinux check
+    # Memory check
+    if not options.skip_mem_check and check_available_memory is not None:
+        check_available_memory(ca=options.dirsrv_cert_files and
+                               len(options.dirsrv_cert_files) > 0)
 
+    # IPv6 and SELinux check
     tasks.check_ipv6_stack_enabled()
     tasks.check_selinux_status()
     if check_ldap_conf is not None:

roles/ipaserver/module_utils/ansible_ipa_server.py

@@ -37,7 +37,8 @@ __all__ = ["IPAChangeConf", "certmonger", "sysrestore", "root_logger",
            "validate_dm_password", "read_cache", "write_cache",
            "adtrustinstance", "IPAAPI_USER", "sync_time", "PKIIniLoader",
            "default_subject_base", "default_ca_subject_dn",
-           "check_ldap_conf", "encode_certificate", "decode_certificate"]
+           "check_ldap_conf", "encode_certificate", "decode_certificate",
+           "check_available_memory"]
 
 import sys
 import logging
@@ -139,6 +140,10 @@ if NUM_VERSION >= 40500:
     except ImportError:
         def default_ca_subject_dn(subject_base):
             return DN(('CN', 'Certificate Authority'), subject_base)
+    try:
+        from ipaserver.install.installutils import check_available_memory
+    except ImportError:
+        check_available_memory = None
 
     try:
         from ipaserver.install import adtrustinstance

roles/ipaserver/tasks/copy_external_cert.yml

@@ -2,11 +2,11 @@
   set_fact:
      ipaserver_external_cert_files: []
   when: ipaserver_external_cert_files is undefined
-- name: Install - Copy "{{ item }}" "{{ inventory_hostname }}':/root/'{{ item }}"
+- name: Install - Copy "{{ item }}" "{{ inventory_hostname }}':/root/'{{ item | basename }}"
   copy:
     src: "{{ item }}"
-    dest: "/root/{{ item }}"
+    dest: "/root/{{ item | basename }}"
     force: yes
-- name: Install - Extend ipaserver_external_cert_files with "/root/{{ item }}"
+- name: Install - Extend ipaserver_external_cert_files with "/root/{{ item | basename }}"
   set_fact:
-    ipaserver_external_cert_files: "{{ ipaserver_external_cert_files }} + [ '/root/{{ item }}' ]"
+    ipaserver_external_cert_files: "{{ ipaserver_external_cert_files }} + [ '/root/{{ item | basename }}' ]"

roles/ipaserver/tasks/install.yml

@@ -69,6 +69,7 @@
     ca_cert_files: "{{ ipaserver_ca_cert_files | default(omit) }}"
     no_host_dns: "{{ ipaserver_no_host_dns }}"
     pki_config_override: "{{ ipaserver_pki_config_override | default(omit) }}"
+    skip_mem_check: "{{ not ipaserver_mem_check }}"
     ### server ###
     setup_adtrust: "{{ ipaserver_setup_adtrust }}"
     setup_kra: "{{ ipaserver_setup_kra }}"

tests/README.md

@@ -9,10 +9,10 @@ You will also need to have a remote host with freeipa server installed and confi
 Some other requirements:
 
  * The `controller` must be able to connect to `ipaserver` through ssh using keys.
- * `ipaserver` must be configured with DNS support. See [ipaserver role](../roles/ipaserver/README.md).
  * IPA admin password must be `SomeADMINpassword`.
  * Directory Server admin password must be `SomeDMpassword`.
 
+To provide broader test coverage, `ipaserver` should be configured with DNS and KRA support, and playbook tests are written based on this configuration. Without such support, some tests are expected to fail. Use a different configuration to evaluate those scenarios. See also [ipaserver role](../roles/ipaserver/README.md).
 
 ## Running the tests
 

tests/azure/azure-pipelines.yml

@@ -15,7 +15,7 @@ trigger:
 - master
 
 pool:
-  vmImage: 'ubuntu-18.04'
+  vmImage: 'ubuntu-20.04'
 
 stages:
 - stage: Centos7

tests/azure/build-containers.yml

@@ -11,7 +11,7 @@ schedules:
 trigger: none
 
 pool:
-  vmImage: 'ubuntu-18.04'
+  vmImage: 'ubuntu-20.04'
 
 jobs:
 

tests/azure/templates/build_container.yml

@@ -15,7 +15,7 @@ jobs:
     inputs:
       versionSpec: '3.6'
 
-  - script: python -m pip install --upgrade pip setuptools wheel
+  - script: python -m pip install --upgrade pip setuptools wheel ansible
     displayName: Install tools
 
   - script: pip install molecule[docker]

tests/dnsrecord/test_compatibility_with_ansible_module.yml

@@ -29,26 +29,32 @@
       ipaadmin_password: SomeADMINpassword
       name: host01
       zone_name: testzone.local
-      record_type: 'AAAA'
-      record_value: '::1'
+      del_all: yes
       state: absent
 
-  - name: Ensure that dns record 'vm-001' is absent
+  - name: Ensure that dns records for 'vm-001' are absent
     ipadnsrecord:
       ipaadmin_password: SomeADMINpassword
       name: vm-001
       zone_name: testzone.local
-      record_type: 'AAAA'
-      record_value: '::1'
+      del_all: yes
+      state: absent
+
+  - name: Ensure a PTR record is absent for 'vm-001'
+    ipadnsrecord:
+      ipaadmin_password: SomeADMINpassword
+      name: '1'
+      record_type: 'PTR'
+      record_value: 'vm-001'
+      zone_name: 2.168.192.in-addr.arpa
       state: absent
 
   - name: Ensure a PTR record is absent
     ipadnsrecord:
       ipaadmin_password: SomeADMINpassword
-      name: 5
-      record_type: 'PTR'
-      record_value: 'internal.ipa.testzone.local'
       zone_name: 2.168.192.in-addr.arpa
+      name: "5"
+      del_all: yes
       state: absent
 
   - name: Ensure a TXT record is absent
@@ -79,7 +85,7 @@
       state: absent
 
   # tests
-  - name: Ensure dns record is present
+  - name: Ensure AAAA  dns record is present
     ipadnsrecord:
       ipaadmin_password: SomeADMINpassword
       name: vm-001
@@ -88,9 +94,9 @@
       zone_name: testzone.local
       state: present
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
-  - name: Ensure that dns record exists with a TTL
+  - name: Ensure that AAAA dns record exists with a TTL
     ipadnsrecord:
       ipaadmin_password: SomeADMINpassword
       name: host01
@@ -100,18 +106,52 @@
       zone_name: testzone.local
       state: present
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: Ensure a PTR record is present
     ipadnsrecord:
       ipaadmin_password: SomeADMINpassword
-      name: 5
+      name: '5'
       record_type: 'PTR'
       record_value: 'internal.ipa.testzone.local'
       zone_name: 2.168.192.in-addr.arpa
       state: present
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure A record is present, with reverse
+    ipadnsrecord:
+      ipaadmin_password: SomeADMINpassword
+      name: vm-001
+      record_type: 'A'
+      record_value: '192.168.2.1'
+      create_reverse: yes
+      zone_name: testzone.local
+      state: present
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure A record is present
+    ipadnsrecord:
+      ipaadmin_password: SomeADMINpassword
+      name: vm-001
+      record_type: 'A'
+      record_value: '192.168.2.1'
+      zone_name: testzone.local
+      state: present
+    register: result
+    failed_when: result.changed or result.failed
+
+  - name: Ensure PTR record is present
+    ipadnsrecord:
+      ipaadmin_password: SomeADMINpassword
+      name: '1'
+      record_type: 'PTR'
+      record_value: vm-001.testzone.local
+      zone_name: 2.168.192.in-addr.arpa
+      state: present
+    register: result
+    failed_when: result.changed or result.failed
 
   - name: Ensure a TXT record is present
     ipadnsrecord:
@@ -122,7 +162,7 @@
       zone_name: testzone.local
       state: present
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: Ensure a SRV record is present
     ipadnsrecord:
@@ -133,7 +173,7 @@
       zone_name: testzone.local
       state: present
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: Ensure an MX record is present
     ipadnsrecord:
@@ -144,7 +184,7 @@
       zone_name: testzone.local
       state: present
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: Ensure that dns record is removed
     ipadnsrecord:
@@ -155,7 +195,7 @@
       record_value: '::1'
       state: absent
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
    # cleanup
   - name: Ensure that dns record 'host01' is absent
@@ -167,7 +207,7 @@
       record_value: '::1'
       state: absent
     register: result
-    failed_when: result.changed
+    failed_when: result.changed or result.failed
 
   - name: Ensure that dns record 'vm-001' is absent
     ipadnsrecord:
@@ -178,7 +218,7 @@
       record_value: '::1'
       state: absent
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: Ensure a PTR record is absent
     ipadnsrecord:
@@ -189,7 +229,7 @@
       zone_name: 2.168.192.in-addr.arpa
       state: absent
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: Ensure a TXT record is absent
     ipadnsrecord:
@@ -200,7 +240,7 @@
       zone_name: testzone.local
       state: absent
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: Ensure a SRV record is absent
     ipadnsrecord:
@@ -211,7 +251,7 @@
       zone_name: testzone.local
       state: absent
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: Ensure an MX record is absent
     ipadnsrecord:
@@ -222,7 +262,7 @@
       zone_name: testzone.local
       state: absent
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: Ensure DNS zones to be used are absent.
     ipadnszone:

tests/dnsrecord/test_dnsrecord.yml

@@ -9,6 +9,16 @@
   - name: Setup testing environment.
     include_tasks: env_setup.yml
 
+  - name: Generate self-signed certificates.
+    shell:
+      cmd: |
+        openssl req -x509 -newkey rsa:2048 -days 365 -nodes -keyout "private{{ item }}.key" -out "cert{{ item }}.pem" -subj '/CN=test'
+        openssl x509 -outform der -in "cert{{ item }}.pem" -out "cert{{ item }}.der"
+        base64 "cert{{ item }}.der" -w5000 > "cert{{ item }}.b64"
+    with_items: [1]
+    become: no
+    delegate_to: localhost
+
   # tests
   - name: Ensure that dns record 'host01' is present
     ipadnsrecord:
@@ -472,7 +482,7 @@
       # digest is sha1sum of 'host04."{{ testzone }}"'
       dlv_digest: 08ff468cb25ccd21642989294cc33570da5eb2ba
     register: result
-    failed_when: not result.changed
+    failed_when: result.failed or not result.changed
 
   - name: Ensure that 'host04' DLV record is present, again.
     ipadnsrecord:
@@ -484,27 +494,40 @@
       dlv_digest_type: 1
       dlv_digest: 08ff468cb25ccd21642989294cc33570da5eb2ba
     register: result
-    failed_when: result.changed
+    failed_when: result.failed or result.changed
 
   - name: Ensure that 'host04' DLV record is present, with a different key tag.
     ipadnsrecord:
       ipaadmin_password: SomeADMINpassword
       zone_name: "{{ testzone }}"
       name: host04
-      dlv_key_tag: 54321
+      dlv_key_tag: 4321
       dlv_record: 12345 3 1 08ff468cb25ccd21642989294cc33570da5eb2ba
     register: result
-    failed_when: not result.changed
+    failed_when: result.failed or not result.changed
 
-  - name: Ensure that 'host04' DLV record is present, with a different key tag, again.
+  - name: Ensure that 'host04' DLV second record is present.
+    ipadnsrecord:
+      ipaadmin_password: SomeADMINpassword
+      zone_name: "{{ testzone }}"
+      name: host04
+      dlv_key_tag: 4321
+      dlv_algorithm: 2
+      dlv_digest_type: 2
+      # digest is sha1sum of 'second record'
+      dlv_digest: da39a3ee5e6b4b0d3255bfef95601890afd80709
+    register: result
+    failed_when: result.failed or not result.changed
+
+  - name: Ensure that 'host04' DLV record is changed, in presence of multiple records.
     ipadnsrecord:
       ipaadmin_password: SomeADMINpassword
       zone_name: "{{ testzone }}"
       name: host04
       dlv_key_tag: 54321
-      dlv_record: 12345 3 1 08ff468cb25ccd21642989294cc33570da5eb2ba
+      dlv_record: 4321 3 1 08ff468cb25ccd21642989294cc33570da5eb2ba
     register: result
-    failed_when: result.changed
+    failed_when: result.failed or not result.changed
 
   - name: Ensure that 'host04' DLV record is absent.
     ipadnsrecord:
@@ -514,7 +537,7 @@
       dlv_record: 54321 3 1 08ff468cb25ccd21642989294cc33570da5eb2ba
       state: absent
     register: result
-    failed_when: not result.changed
+    failed_when: result.failed or not result.changed
 
   - name: Ensure that 'host04' DLV record is absent, again.
     ipadnsrecord:
@@ -524,7 +547,17 @@
       dlv_record: 54321 3 1 08ff468cb25ccd21642989294cc33570da5eb2ba
       state: absent
     register: result
-    failed_when: result.changed
+    failed_when: result.failed or result.changed
+
+  - name: Ensure that 'host04' DLV record is absent.
+    ipadnsrecord:
+      ipaadmin_password: SomeADMINpassword
+      zone_name: "{{ testzone }}"
+      name: host04
+      dlv_record: 4321 2 2 da39a3ee5e6b4b0d3255bfef95601890afd80709
+      state: absent
+    register: result
+    failed_when: result.failed or not result.changed
 
   - name: Ensure that dns record 'iron01' is present
     ipadnsrecord:
@@ -615,7 +648,7 @@
       zone_name: "{{ testzone }}"
       name: host04
       afsdb_subtype: 1
-      afsdb_hostname: host04."{{ testzone }}"
+      afsdb_hostname: "host04.{{ testzone }}"
     register: result
     failed_when: not result.changed
 
@@ -625,7 +658,7 @@
       zone_name: "{{ testzone }}"
       name: host04
       afsdb_subtype: 1
-      afsdb_hostname: host04."{{ testzone }}"
+      afsdb_hostname: "host04.{{ testzone }}"
     register: result
     failed_when: result.changed
 
@@ -635,7 +668,7 @@
       zone_name: "{{ testzone }}"
       name: host04
       afsdb_subtype: 2
-      afsdb_rec: 1 host04."{{ testzone }}"
+      afsdb_rec: "1 host04.{{ testzone }}"
     register: result
     failed_when: not result.changed
 
@@ -645,7 +678,7 @@
       zone_name: "{{ testzone }}"
       name: host04
       afsdb_subtype: 2
-      afsdb_rec: 1 host04."{{ testzone }}"
+      afsdb_rec: "1 host04.{{ testzone }}"
     register: result
     failed_when: result.changed
 
@@ -654,7 +687,7 @@
       ipaadmin_password: SomeADMINpassword
       zone_name: "{{ testzone }}"
       name: host04
-      afsdb_rec: 2 host04."{{ testzone }}"
+      afsdb_rec: "2 host04.{{ testzone }}"
       state: absent
     register: result
     failed_when: not result.changed
@@ -664,15 +697,11 @@
       ipaadmin_password: SomeADMINpassword
       zone_name: "{{ testzone }}"
       name: host04
-      afsdb_rec: 2 host04."{{ testzone }}"
+      afsdb_rec: "2 host04.{{ testzone }}"
       state: absent
     register: result
     failed_when: result.changed
 
-  # Certificate created with:
-  # - openssl req -x509 -newkey rsa:512 -days 3650 -nodes -keyout private1.key -out cert1.pem -subj '/CN=test'
-  # - openssl x509 -outform der -in cert1.pem -out cert1.der
-  # - base64 cert1.der -w5000
   - name: Ensure that 'host04' CERT record is present.
     ipadnsrecord:
       ipaadmin_password: SomeADMINpassword
@@ -681,7 +710,7 @@
       cert_type: 1
       cert_key_tag: 1234
       cert_algorithm: 3
-      cert_certificate_or_crl: MIIBdTCCAR+gAwIBAgIUb14+Oug2nPy1fOFF5US+uiJ1LfIwDQYJKoZIhvcNAQELBQAwDzENMAsGA1UEAwwEdGVzdDAeFw0yMDAzMjMxODMzNDNaFw0zMDAzMjExODMzNDNaMA8xDTALBgNVBAMMBHRlc3QwXDANBgkqhkiG9w0BAQEFAANLADBIAkEAv/yGOgQbtUZbiQMjVly7bWuUX1oBGZAkCvumYpvsep3o1eJJ6HlREbLUlJmgibuNsjqE0FyrXueMjsD8D4juWQIDAQABo1MwUTAdBgNVHQ4EFgQUNtEmJqasXgN7Sh/huB5tx0ONblYwHwYDVR0jBBgwFoAUNtEmJqasXgN7Sh/huB5tx0ONblYwDwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAANBAKhPWPK5+pkT9NLLSZm3ASQJcDkU9asrSoc7MsiHIqSUju/YQgjdHgX0ljS8hnlo1scCITW09UXcNRUYFxwEuoQ=
+      cert_certificate_or_crl: "{{ lookup('file', 'cert1.b64') }}"
     register: result
     failed_when: not result.changed
 
@@ -693,7 +722,7 @@
       cert_type: 1
       cert_key_tag: 1234
       cert_algorithm: 3
-      cert_certificate_or_crl: MIIBdTCCAR+gAwIBAgIUb14+Oug2nPy1fOFF5US+uiJ1LfIwDQYJKoZIhvcNAQELBQAwDzENMAsGA1UEAwwEdGVzdDAeFw0yMDAzMjMxODMzNDNaFw0zMDAzMjExODMzNDNaMA8xDTALBgNVBAMMBHRlc3QwXDANBgkqhkiG9w0BAQEFAANLADBIAkEAv/yGOgQbtUZbiQMjVly7bWuUX1oBGZAkCvumYpvsep3o1eJJ6HlREbLUlJmgibuNsjqE0FyrXueMjsD8D4juWQIDAQABo1MwUTAdBgNVHQ4EFgQUNtEmJqasXgN7Sh/huB5tx0ONblYwHwYDVR0jBBgwFoAUNtEmJqasXgN7Sh/huB5tx0ONblYwDwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAANBAKhPWPK5+pkT9NLLSZm3ASQJcDkU9asrSoc7MsiHIqSUju/YQgjdHgX0ljS8hnlo1scCITW09UXcNRUYFxwEuoQ=
+      cert_certificate_or_crl: "{{ lookup('file', 'cert1.b64') }}"
     register: result
     failed_when: result.changed
 
@@ -702,7 +731,7 @@
       ipaadmin_password: SomeADMINpassword
       zone_name: "{{ testzone }}"
       name: host04
-      cert_rec: 1 1234 3 MIIBdTCCAR+gAwIBAgIUb14+Oug2nPy1fOFF5US+uiJ1LfIwDQYJKoZIhvcNAQELBQAwDzENMAsGA1UEAwwEdGVzdDAeFw0yMDAzMjMxODMzNDNaFw0zMDAzMjExODMzNDNaMA8xDTALBgNVBAMMBHRlc3QwXDANBgkqhkiG9w0BAQEFAANLADBIAkEAv/yGOgQbtUZbiQMjVly7bWuUX1oBGZAkCvumYpvsep3o1eJJ6HlREbLUlJmgibuNsjqE0FyrXueMjsD8D4juWQIDAQABo1MwUTAdBgNVHQ4EFgQUNtEmJqasXgN7Sh/huB5tx0ONblYwHwYDVR0jBBgwFoAUNtEmJqasXgN7Sh/huB5tx0ONblYwDwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAANBAKhPWPK5+pkT9NLLSZm3ASQJcDkU9asrSoc7MsiHIqSUju/YQgjdHgX0ljS8hnlo1scCITW09UXcNRUYFxwEuoQ=
+      cert_rec: "1 1234 3 {{ lookup('file', 'cert1.b64') }}"
       state: absent
     register: result
     failed_when: not result.changed
@@ -712,7 +741,7 @@
       ipaadmin_password: SomeADMINpassword
       zone_name: "{{ testzone }}"
       name: host04
-      cert_rec: 1 1234 3 MIIBdTCCAR+gAwIBAgIUb14+Oug2nPy1fOFF5US+uiJ1LfIwDQYJKoZIhvcNAQELBQAwDzENMAsGA1UEAwwEdGVzdDAeFw0yMDAzMjMxODMzNDNaFw0zMDAzMjExODMzNDNaMA8xDTALBgNVBAMMBHRlc3QwXDANBgkqhkiG9w0BAQEFAANLADBIAkEAv/yGOgQbtUZbiQMjVly7bWuUX1oBGZAkCvumYpvsep3o1eJJ6HlREbLUlJmgibuNsjqE0FyrXueMjsD8D4juWQIDAQABo1MwUTAdBgNVHQ4EFgQUNtEmJqasXgN7Sh/huB5tx0ONblYwHwYDVR0jBBgwFoAUNtEmJqasXgN7Sh/huB5tx0ONblYwDwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAANBAKhPWPK5+pkT9NLLSZm3ASQJcDkU9asrSoc7MsiHIqSUju/YQgjdHgX0ljS8hnlo1scCITW09UXcNRUYFxwEuoQ=
+      cert_rec: 1 1234 3 "{{ lookup('file', 'cert1.b64') }}"
       state: absent
     register: result
     failed_when: result.changed
@@ -723,7 +752,7 @@
       zone_name: "{{ testzone }}"
       name: host04
       kx_preference: 10
-      kx_exchanger: keyex."{{ testzone }}"
+      kx_exchanger: "keyex.{{ testzone }}"
     register: result
     failed_when: not result.changed
 
@@ -733,7 +762,7 @@
       zone_name: "{{ testzone }}"
       name: host04
       kx_preference: 10
-      kx_exchanger: keyex."{{ testzone }}"
+      kx_exchanger: "keyex.{{ testzone }}"
     register: result
     failed_when: result.changed
 
@@ -743,7 +772,7 @@
       zone_name: "{{ testzone }}"
       name: host04
       kx_preference: 20
-      kx_rec: 10 keyex."{{ testzone }}"
+      kx_rec: "10 keyex.{{ testzone }}"
     register: result
     failed_when: not result.changed
 
@@ -753,7 +782,7 @@
       zone_name: "{{ testzone }}"
       name: host04
       kx_preference: 20
-      kx_rec: 10 keyex."{{ testzone }}"
+      kx_rec: "10 keyex.{{ testzone }}"
     register: result
     failed_when: result.changed
 
@@ -763,7 +792,7 @@
       zone_name: "{{ testzone }}"
       name: host04
       kx_preference: 20
-      kx_rec: 20 keyex."{{ testzone }}"
+      kx_rec: "20 keyex.{{ testzone }}"
     register: result
     failed_when: result.changed
 
@@ -772,7 +801,7 @@
       ipaadmin_password: SomeADMINpassword
       zone_name: "{{ testzone }}"
       name: host04
-      kx_rec: 20 keyex."{{ testzone }}"
+      kx_rec: "20 keyex.{{ testzone }}"
       state: absent
     register: result
     failed_when: not result.changed
@@ -782,7 +811,7 @@
       ipaadmin_password: SomeADMINpassword
       zone_name: "{{ testzone }}"
       name: host04
-      kx_rec: 20 keyex."{{ testzone }}"
+      kx_rec: "20 keyex.{{ testzone }}"
       state: absent
     register: result
     failed_when: result.changed
@@ -793,7 +822,7 @@
       zone_name: "{{ testzone }}"
       name: host04
       mx_preference: 10
-      mx_exchanger: mail."{{ testzone }}"
+      mx_exchanger: "mail.{{ testzone }}"
     register: result
     failed_when: not result.changed
 
@@ -803,7 +832,7 @@
       zone_name: "{{ testzone }}"
       name: host04
       mx_preference: 10
-      mx_exchanger: mail."{{ testzone }}"
+      mx_exchanger: "mail.{{ testzone }}"
     register: result
     failed_when: result.changed
 
@@ -813,7 +842,7 @@
       zone_name: "{{ testzone }}"
       name: host04
       mx_preference: 20
-      mx_rec: 10 mail."{{ testzone }}"
+      mx_rec: "10 mail.{{ testzone }}"
     register: result
     failed_when: not result.changed
 
@@ -822,7 +851,7 @@
       ipaadmin_password: SomeADMINpassword
       zone_name: "{{ testzone }}"
       name: host04
-      mx_rec: 20 mail."{{ testzone }}"
+      mx_rec: "20 mail.{{ testzone }}"
       state: absent
     register: result
     failed_when: not result.changed
@@ -832,18 +861,7 @@
       ipaadmin_password: SomeADMINpassword
       zone_name: "{{ testzone }}"
       name: host04
-      mx_rec: 20 mail."{{ testzone }}"
-      state: absent
-    register: result
-    failed_when: result.changed
-
-  - name: Ensure that '_sip._udp' service has NAPTR record is absent, again.
-    ipadnsrecord:
-      ipaadmin_password: SomeADMINpassword
-      zone_name: "{{ testzone }}"
-      name: _sip._udp
-      record_type: NAPTR
-      record_value: '100 10 U SIP+D2U !^.*$!sip:customer-service@example.com! .'
+      mx_rec: "20 mail.{{ testzone }}"
       state: absent
     register: result
     failed_when: result.changed
@@ -894,7 +912,7 @@
       zone_name: "{{ testzone }}"
       name: host04
       loc_size: 1.00
-      loc_rec: 52 22 23 N 4 53 32 E -2 0 10000 10
+      loc_rec: 52 22 23.000 N 4 53 32.000 E -2.00 0.00 10000.00 10.00
     register: result
     failed_when: not result.changed
 
@@ -903,7 +921,7 @@
       ipaadmin_password: SomeADMINpassword
       zone_name: "{{ testzone }}"
       name: host04
-      loc_rec: 52 22 23.000 N 4 53 32.000 E -2.00 1.00 10000 10
+      loc_rec: 52 22 23.000 N 4 53 32.000 E -2.00 1.00 10000.00 10.00
       state: absent
     register: result
     failed_when: not result.changed
@@ -913,7 +931,7 @@
       ipaadmin_password: SomeADMINpassword
       zone_name: "{{ testzone }}"
       name: host04
-      loc_rec: 52 22 23.000 N 4 53 32.000 E -2.00 1.00 10000 10
+      loc_rec: 52 22 23.000 N 4 53 32.000 E -2.00 1.00 10000.00 10.00
       state: absent
     register: result
     failed_when: result.changed
@@ -927,10 +945,10 @@
       naptr_preference: 10
       naptr_flags: "U"
       naptr_service: "SIP+D2U"
-      naptr_regexp: "!^.*$!sip:customer-service@example.com!"
+      naptr_regexp: "!^.*$!sip:info@example.com!"
       naptr_replacement: "."
     register: result
-    failed_when: not result.changed
+    failed_when: result.failed or not result.changed
 
   - name: Ensure that '_sip._udp' service has NAPTR record, again.
     ipadnsrecord:
@@ -941,10 +959,10 @@
       naptr_preference: 10
       naptr_flags: "U"
       naptr_service: "SIP+D2U"
-      naptr_regexp: "!^.*$!sip:customer-service@example.com!"
+      naptr_regexp: "!^.*$!sip:info@example.com!"
       naptr_replacement: "."
     register: result
-    failed_when: result.changed
+    failed_when: result.failed or result.changed
 
   - name: Change '_sip._udp' service NAPTR record `preference` to 20.
     ipadnsrecord:
@@ -952,9 +970,43 @@
       zone_name: "{{ testzone }}"
       name: _sip._udp
       naptr_preference: 20
-      naptr_rec: '100 10 U SIP+D2U !^.*$!sip:customer-service@example.com! .'
+      naptr_rec: '100 10 U SIP+D2U !^.*$!sip:info@example.com! .'
     register: result
-    failed_when: not result.changed
+    failed_when: result.failed or not result.changed
+
+  - name: Ensure that '_sip._udp' service has NAPTR record.
+    ipadnsrecord:
+      ipaadmin_password: SomeADMINpassword
+      zone_name: "{{ testzone }}"
+      name: _sip._udp
+      naptr_order: 101
+      naptr_preference: 11
+      naptr_flags: "U"
+      naptr_service: "SIP+D2U"
+      naptr_regexp: "!^.*$!sip:debug@example.com!"
+      naptr_replacement: "."
+
+  - name: Ensure that '_sip._udp' service has NAPTR record.
+    ipadnsrecord:
+      ipaadmin_password: SomeADMINpassword
+      zone_name: "{{ testzone }}"
+      name: _sip._udp
+      naptr_order: 102
+      naptr_preference: 12
+      naptr_flags: "U"
+      naptr_service: "SIP+D2U"
+      naptr_regexp: "!^.*$!sip:prio@example.com!"
+      naptr_replacement: "."
+
+  - name: Change '_sip._udp' service NAPTR record `preference` to 50, when multiple records are present. (BZ 1881436)
+    ipadnsrecord:
+      ipaadmin_password: SomeADMINpassword
+      zone_name: "{{ testzone }}"
+      name: _sip._udp
+      naptr_preference: 50
+      naptr_rec: '100 20 U SIP+D2U !^.*$!sip:info@example.com! .'
+    register: result
+    failed_when: result.failed or not result.changed
 
   - name: Ensure that '_sip._udp' service has NAPTR record is absent.
     ipadnsrecord:
@@ -962,10 +1014,10 @@
       zone_name: "{{ testzone }}"
       name: _sip._udp
       record_type: NAPTR
-      record_value: '100 20 U SIP+D2U !^.*$!sip:customer-service@example.com! .'
+      record_value: '100 50 U SIP+D2U !^.*$!sip:info@example.com! .'
       state: absent
     register: result
-    failed_when: not result.changed
+    failed_when: result.failed or not result.changed
 
   - name: Ensure that '_sip._udp' service has NAPTR record is absent, again.
     ipadnsrecord:
@@ -973,10 +1025,19 @@
       zone_name: "{{ testzone }}"
       name: _sip._udp
       record_type: NAPTR
-      record_value: '100 20 U SIP+D2U !^.*$!sip:customer-service@example.com! .'
+      record_value: '100 50 U SIP+D2U !^.*$!sip:info@example.com! .'
       state: absent
     register: result
-    failed_when: result.changed
+    failed_when: result.failed or result.changed
+
+  - name: Clear NAPTR records.
+    ipadnsrecord:
+      ipaadmin_password: SomeADMINpassword
+      zone_name: "{{ testzone }}"
+      name: _sip._udp
+      del_all: yes
+      state: absent
+
 
   - name: Ensure that '_sip._udp' service has SRV record.
     ipadnsrecord:
@@ -986,7 +1047,7 @@
       srv_priority: 10
       srv_weight: 10
       srv_port: 5060
-      srv_target: sip-server."{{ testzone }}"
+      srv_target: "sip-server.{{ testzone }}"
     register: result
     failed_when: not result.changed
 
@@ -998,7 +1059,7 @@
       srv_priority: 10
       srv_weight: 10
       srv_port: 5060
-      srv_target: sip-server."{{ testzone }}"
+      srv_target: "sip-server.{{ testzone }}"
     register: result
     failed_when: result.changed
 
@@ -1010,8 +1071,8 @@
       srv_priority: 4
       srv_weight: 10
       srv_port: 5060
-      srv_target: sip-server."{{ testzone }}"
-      srv_rec: 10 10 5060 sip-server."{{ testzone }}"
+      srv_target: "sip-server.{{ testzone }}"
+      srv_rec: "10 10 5060 sip-server.{{ testzone }}"
     register: result
     failed_when: not result.changed
 
@@ -1024,7 +1085,7 @@
       srv_weight: 10
       srv_port: 5060
       srv_target: sip-server."{{ testzone }}"
-      srv_rec: 10 10 5060 sip-server."{{ testzone }}"
+      srv_rec: "10 10 5060 sip-server.{{ testzone }}"
     register: result
     failed_when: result.changed
 
@@ -1036,7 +1097,7 @@
       srv_priority: 2
       srv_weight: 20
       srv_port: 5060
-      srv_target: sip-server."{{ testzone }}"
+      srv_target: "sip-server.{{ testzone }}"
     register: result
     failed_when: not result.changed
 
@@ -1048,7 +1109,7 @@
       srv_priority: 2
       srv_weight: 20
       srv_port: 5060
-      srv_target: sip-server."{{ testzone }}"
+      srv_target: "sip-server.{{ testzone }}"
     register: result
     failed_when: result.changed
 
@@ -1057,7 +1118,7 @@
       ipaadmin_password: SomeADMINpassword
       zone_name: "{{ testzone }}"
       name: _sip._udp
-      srv_record: 2 20 5060 sip-server."{{ testzone }}"
+      srv_record: "2 20 5060 sip-server.{{ testzone }}"
       state: absent
     register: result
     failed_when: not result.changed
@@ -1067,7 +1128,7 @@
       ipaadmin_password: SomeADMINpassword
       zone_name: "{{ testzone }}"
       name: _sip._udp
-      srv_record: 2 20 5060 sip-server."{{ testzone }}"
+      srv_record: "2 20 5060 sip-server.{{ testzone }}"
       state: absent
     register: result
     failed_when: result.changed
@@ -1278,7 +1339,7 @@
       name: _ftp._tcp
       uri_priority: 10
       uri_weight: 1
-      uri_target: ftp://ftp.host04."{{ testzone }}"/public
+      uri_target: ftp://ftp.host04.{{ testzone }}/public
     register: result
     failed_when: not result.changed
 
@@ -1289,7 +1350,7 @@
       name: _ftp._tcp
       uri_priority: 10
       uri_weight: 1
-      uri_target: ftp://ftp.host04."{{ testzone }}"/public
+      uri_target: ftp://ftp.host04.{{ testzone }}/public
     register: result
     failed_when: result.changed
 
@@ -1300,13 +1361,13 @@
       name: _ftp._tcp
       uri_priority: 5
       uri_weight: 3
-      uri_rec: 10 1 ftp://ftp.host04."{{ testzone }}"/public
+      uri_rec: 10 1 "ftp://ftp.host04.{{ testzone }}/public"
     register: result
     failed_when: not result.changed
 
   - name: Verify if modification worked.
     ipadnsrecord:
-      uri_rec: 10 1 ftp://ftp.host04."{{ testzone }}"/public
+      uri_rec: 10 1 ftp://ftp.host04.{{ testzone }}/public
       state: absent
     register: result
     failed_when: result.changed
@@ -1319,7 +1380,7 @@
       name: _ftp._tcp
       uri_priority: 5
       uri_weight: 3
-      uri_rec: 5 3 ftp://ftp.host04."{{ testzone }}"/public
+      uri_rec: 5 3 "ftp://ftp.host04.{{ testzone }}/public"
     register: result
     failed_when: result.changed
 
@@ -1328,7 +1389,7 @@
       ipaadmin_password: SomeADMINpassword
       zone_name: "{{ testzone }}"
       name: _ftp._tcp
-      uri_rec: 5 3 ftp://ftp.host04."{{ testzone }}"/public
+      uri_rec: 5 3 "ftp://ftp.host04.{{ testzone }}/public"
       state: absent
     register: result
     failed_when: not result.changed
@@ -1338,7 +1399,7 @@
       ipaadmin_password: SomeADMINpassword
       zone_name: "{{ testzone }}"
       name: _ftp._tcp
-      uri_rec: 5 3 ftp://ftp.host04."{{ testzone }}"/public
+      uri_rec: 5 3 "ftp://ftp.host04.{{ testzone }}/public"
       state: absent
     register: result
     failed_when: result.changed
@@ -1346,3 +1407,12 @@
   # cleanup
   - name: Cleanup test environment.
     include_tasks: env_cleanup.yml
+
+  - name: Remove certificate files.
+    shell:
+      cmd: rm -f "private{{ item }}.key" "cert{{ item }}.pem" "cert{{ item }}.der" "cert{{ item }}.b64"
+    with_items: [1]
+    become: no
+    delegate_to: localhost
+    args:
+      warn: no  # suppres warning for not using the `file` module.