Merge pull request #596 from freeipa/automember_verify_condition_keys

automember: Verify condition keys

.github/workflows/docs.yml

@@ -0,0 +1,32 @@
+---
+name: Verify Ansible documentation.
+on:
+  - push
+  - pull_request
+jobs:
+  check_docs_29:
+    name: Check Ansible Documentation with Ansible 2.9.
+    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: |
+          python -m pip install "ansible < 2.10"
+          ANSIBLE_LIBRARY="." python utils/ansible-doc-test -v roles plugins
+  
+  check_docs_latest:
+    name: Check Ansible Documentation with latest Ansible.
+    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: |
+          python -m pip install ansible
+          ANSIBLE_LIBRARY="." python utils/ansible-doc-test -v roles plugins
+

.github/workflows/lint.yml

@@ -4,15 +4,14 @@ on:
   - push
   - pull_request
 jobs:
-  linters:
-    name: Run Linters
+  ansible_lint:
+    name: Verify ansible-lint
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v2
       - uses: actions/setup-python@v2
         with:
-          python-version: "3.6"
-
+          python-version: "3.x"
       - name: Run ansible-lint
         uses: ansible/ansible-lint-action@master
         with:
@@ -26,8 +25,52 @@ jobs:
           ANSIBLE_MODULE_UTILS: plugins/module_utils
           ANSIBLE_LIBRARY: plugins/modules
 
+  yamllint:
+    name: Verify yamllint
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions/setup-python@v2
+        with:
+          python-version: "3.x"
       - name: Run yaml-lint
         uses: ibiqlik/action-yamllint@v1
 
-      - name: Run Python linters
-        uses: rjeffman/python-lint-action@master
+  pydocstyle:
+    name: Verify pydocstyle
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions/setup-python@v2
+        with:
+          python-version: "3.x"
+      - name: Run pydocstyle
+        run: |
+            pip install pydocstyle
+            pydocstyle
+
+  flake8:
+    name: Verify flake8
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions/setup-python@v2
+        with:
+          python-version: "3.x"
+      - name: Run flake8
+        run: |
+            pip install flake8
+            flake8
+
+  pylint:
+    name: Verify pylint
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions/setup-python@v2
+        with:
+          python-version: "3.x"
+      - name: Run pylint
+        run: |
+            pip install pylint==2.8.2
+            pylint plugins --disable=import-error

.pre-commit-config.yaml

@@ -0,0 +1,38 @@
+---
+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: https://github.com/pycqa/pylint
+  rev: v2.8.2
+  hooks:
+  - id: pylint
+    args:
+    - --disable=import-error
+    files: \.py$
+- repo: local
+  hooks:
+  - id: ansible-doc-test
+    name: Verify Ansible roles and module documentation.
+    language: python
+    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-automember.md

@@ -0,0 +1,137 @@
+Automember module
+===========
+
+Description
+-----------
+
+The automember module allows to ensure presence or absence of automember rules and manage automember rule conditions.
+
+Features
+--------
+
+* Automember management
+
+
+Supported FreeIPA Versions
+--------------------------
+
+FreeIPA versions 4.4.0 and up are supported by the ipaautomember 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 group automember rule is present with no conditions.
+
+```yaml
+---
+- name: Playbook to ensure a group automember rule is present with no conditions
+  hosts: ipaserver
+  become: yes
+  gather_facts: no
+  tasks:
+    - ipaautomember:
+        ipaadmin_password: SomeADMINpassword
+        name: admins
+        description: "my automember rule"
+        automember_type: group
+```
+
+Example playbook to make sure group automember rule is present with conditions:
+
+```yaml
+---
+- name: Playbook to add a group automember rule with two conditions
+  hosts: ipaserver
+  become: yes
+  gather_facts: no
+  tasks:
+  - ipaautomember:
+      ipaadmin_password: SomeADMINpassword
+      name: admins
+      description: "my automember rule"
+      automember_type: group
+      inclusive:
+        - key: mail
+          expression: '@example.com$'
+      exclusive:
+        - key: uid
+          expression: "1234"
+```
+
+Example playbook to delete a group automember rule:
+
+```yaml
+- name: Playbook to delete a group automember rule
+  hosts: ipaserver
+  become: yes
+  gather_facts: no
+  tasks:
+    - ipaautomember:
+        ipaadmin_password: SomeADMINpassword
+        name: admins
+        description: "my automember rule"
+        automember_type: group
+        state: absent
+```
+
+Example playbook to add an inclusive condition to an existing rule
+
+```yaml
+- name: Playbook to add an inclusive condition to an existing rule
+  hosts: ipaserver
+  become: yes
+  gather_facts: no
+  tasks:
+    - ipaautomember:
+        ipaadmin_password: SomeADMINpassword
+        name: "My domain hosts"
+        description: "my automember condition"
+        automember_tye: hostgroup
+        action: member
+        inclusive:
+          - key: fqdn
+            expression: ".*.mydomain.com"
+```
+
+
+Variables
+---------
+
+ipaautomember
+-------
+
+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` | Automember rule. | yes
+`description` | A description of this auto member rule. | no
+`automember_type` | Grouping to which the rule applies. It can be one of `group`, `hostgroup`. | yes
+`inclusive` | List of dictionaries in the format of `{'key': attribute, 'expression': inclusive_regex}` | no
+`exclusive` | List of dictionaries in the format of `{'key': attribute, 'expression': exclusive_regex}` | no
+`action` | Work on automember or member level. It can be one of `member` or `automember` and defaults to `automember`. | no
+`state` | The state to ensure. It can be one of `present`, `absent`, default: `present`. | no
+
+
+Authors
+=======
+
+Mark Hahl

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-role.md

@@ -248,7 +248,7 @@ Variable | Description | Required
 `name` \| `cn` | The list of role name strings. | yes
 `description` | A description for the role. | no
 `rename` | Rename the role object. | no
-`privileges` | Privileges associated to this role. | no
+`privilege` | Privileges associated to this role. | no
 `user` | List of users to be assigned or not assigned to the role. | no
 `group` | List of groups to be assigned or not assigned to the role. | no
 `host` | List of hosts to be assigned or not assigned to the role. | no

README-server.md

@@ -0,0 +1,248 @@
+Server module
+============
+
+Description
+-----------
+
+The server module allows to ensure presence and absence of servers. The module requires an existing server, the deployment of a new server can not be done with the module.
+
+Features
+--------
+
+* Server management
+
+
+Supported FreeIPA Versions
+--------------------------
+
+FreeIPA versions 4.4.0 and up are supported by the ipaserver 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 server "server.example.com" is present:
+
+```yaml
+---
+- name: Playbook to manage IPA server.
+  hosts: ipaserver
+  become: yes
+
+  tasks:
+  - ipaserver:
+      ipaadmin_password: SomeADMINpassword
+      name: server.example.com
+```
+
+
+Example playbook to make sure server "server.example.com" is present with location mylocation:
+
+```yaml
+---
+- name: Playbook to manage IPA server.
+  hosts: ipaserver
+  become: yes
+
+  tasks:
+  - ipaserver:
+      ipaadmin_password: SomeADMINpassword
+      name: server.example.com
+      location: mylocation
+```
+
+
+Example playbook to make sure server "server.example.com" is present without a location:
+
+```yaml
+---
+- name: Playbook to manage IPA server.
+  hosts: ipaserver
+  become: yes
+
+  tasks:
+  - ipaserver:
+      ipaadmin_password: SomeADMINpassword
+      name: server.example.com
+      location: ""
+```
+
+
+Example playbook to make sure server "server.example.com" is present with service weight 1:
+
+```yaml
+---
+- name: Playbook to manage IPA server.
+  hosts: ipaserver
+  become: yes
+
+  tasks:
+  - ipaserver:
+      ipaadmin_password: SomeADMINpassword
+      name: server.example.com
+      service_weight: 1
+```
+
+
+Example playbook to make sure server "server.example.com" is present without service weight:
+
+```yaml
+---
+- name: Playbook to manage IPA server.
+  hosts: ipaserver
+  become: yes
+
+  tasks:
+  - ipaserver:
+      ipaadmin_password: SomeADMINpassword
+      name: server.example.com
+      service_weight: -1
+```
+
+
+Example playbook to make sure server "server.example.com" is present and hidden:
+
+```yaml
+---
+- name: Playbook to manage IPA server.
+  hosts: ipaserver
+  become: yes
+
+  tasks:
+  - ipaserver:
+      ipaadmin_password: SomeADMINpassword
+      name: server.example.com
+      hidden: yes
+```
+
+
+Example playbook to make sure server "server.example.com" is present and not hidden:
+
+```yaml
+---
+- name: Playbook to manage IPA server.
+  hosts: ipaserver
+  become: yes
+
+  tasks:
+  - ipaserver:
+      ipaadmin_password: SomeADMINpassword
+      name: server.example.com
+      hidden: no
+```
+
+
+Example playbook to make sure server "server.example.com" is absent:
+
+```yaml
+---
+- name: Playbook to manage IPA server.
+  hosts: ipaserver
+  become: yes
+
+  tasks:
+  - ipaserver:
+      ipaadmin_password: SomeADMINpassword
+      name: server.example.com
+      state: absent
+```
+
+
+Example playbook to make sure server "server.example.com" is absent in continuous mode in error case:
+
+```yaml
+---
+- name: Playbook to manage IPA server.
+  hosts: ipaserver
+  become: yes
+
+  tasks:
+  - ipaserver:
+      ipaadmin_password: SomeADMINpassword
+      name: server.example.com
+      continue: yes
+      state: absent
+```
+
+
+Example playbook to make sure server "server.example.com" is absent with last of role check skip:
+
+```yaml
+---
+- name: Playbook to manage IPA server.
+  hosts: ipaserver
+  become: yes
+
+  tasks:
+  - ipaserver:
+      ipaadmin_password: SomeADMINpassword
+      name: server.example.com
+      ignore_last_of_role: yes
+      state: absent
+```
+
+
+Example playbook to make sure server "server.example.com" is absent iwith topology disconnect check skip:
+
+```yaml
+---
+- name: Playbook to manage IPA server.
+  hosts: ipaserver
+  become: yes
+
+  tasks:
+  - ipaserver:
+      ipaadmin_password: SomeADMINpassword
+      name: server.example.com
+      ignore_topology_disconnect: yes
+      state: absent
+```
+
+
+MORE EXAMPLE PLAYBOOKS HERE
+
+
+Variables
+---------
+
+ipaserver
+-------
+
+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 list of server name strings. | yes
+`location` \| `ipalocation_location` | The server location string. Only in state: present. "" for location reset. | no
+`service_weight` \| `ipaserviceweight` | Weight for server services. Type Values 0 to 65535, -1 for weight reset. Only in state: present. (int) | no
+`hidden` | Set hidden state of a server. Only in state: present. (bool) | no
+`no_members` | Suppress processing of membership attributes. Only in state: present. (bool) | no
+`delete_continue` \| `continue` | Continuous mode: Don't stop on errors. Only in state: absent. (bool) | no
+`ignore_last_of_role` | Skip a check whether the last CA master or DNS server is removed. Only in state: absent. (bool) | no
+`ignore_topology_disconnect` | Ignore topology connectivity problems after removal. Only in state: absent. (bool) | no
+`force` | Force server removal even if it does not exist. Will always result in changed. Only in state: absent. (bool) | no
+`state` | The state to ensure. It can be one of `present`, `absent`, default: `present`. `present` is only working with existing servers. | no
+
+
+Authors
+=======
+
+Thomas Woerner

README-service.md

@@ -311,6 +311,8 @@ Variable | Description | Required
 `allow_retrieve_keytab_host` \| `ipaallowedtoperform_read_keys_host` | Hosts allowed to retrieve a keytab from of host. | no
 `allow_retrieve_keytab_hostgroup` \| `ipaallowedtoperform_read_keys_hostgroup` | Host groups allowed to retrieve a keytab of this host. | no
 `continue` | Continuous mode: don't stop on errors. Valid only if `state` is `absent`. Default: `no` (bool) | no
+`smb` | Service is an SMB service. If set, `cifs/` will be prefixed to the service name if needed. | no
+`netbiosname` | NETBIOS name for the SMB service. Only with `smb: yes`. | no
 `action` | Work on service or member level. It can be on of `member` or `service` and defaults to `service`. | no
 `state` | The state to ensure. It can be one of `present`, `absent`, or `disabled`, default: `present`. | no
 

README-sudocmd.md

@@ -67,7 +67,7 @@ Example playbook to make sure sudocmd is absent:
 
   tasks:
   # Ensure sudocmd are absent
-  - ipahostgroup:
+  - ipasudocmd:
       ipaadmin_password: SomeADMINpassword
       name: /usr/bin/su
       state: absent

README-sudorule.md

@@ -125,7 +125,7 @@ Variable | Description | Required
 `usercategory` \| `usercat` | User category the rule applies to. Choices: ["all", ""] | no
 `hostcategory` \| `hostcat` | Host category the rule applies to. Choices: ["all", ""] | no
 `cmdcategory` \| `cmdcat` | Command category the rule applies to. Choices: ["all", ""] | no
-`runasusercategory` \| `rusasusercat` | RunAs User category the rule applies to. Choices: ["all", ""] | no
+`runasusercategory` \| `runasusercat` | RunAs User category the rule applies to. Choices: ["all", ""] | no
 `runasgroupcategory` \| `runasgroupcat` | RunAs Group category the rule applies to. Choices: ["all", ""] | no
 `nomembers` | Suppress processing of membership attributes. (bool) | no
 `host` | List of host name strings assigned to this sudorule. | no
@@ -136,8 +136,8 @@ Variable | Description | Required
 `deny_sudocmd` | List of sudocmd name strings assigned to the deny group of this sudorule. | no
 `allow_sudocmdgroup` | List of sudocmd groups name strings assigned to the allow group of this sudorule. | no
 `deny_sudocmdgroup` | List of sudocmd groups name strings assigned to the deny group of this sudorule. | no
-`sudooption` \| `option` | List of options to the sudorule | no
-`order` | Integer to order the sudorule | no
+`sudooption` \| `options` | List of options to the sudorule | no
+`order` \| `sudoorder` | Integer to order the sudorule | no
 `runasuser` | List of users for Sudo to execute as. | no
 `runasgroup` | List of groups for Sudo to execute as. | no
 `action` | Work on sudorule or member level. It can be on of `member` or `sudorule` and defaults to `sudorule`. | no

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:
@@ -212,23 +219,25 @@ Variable | Description | Required
 `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 list of vault name strings. | yes
 `description` | The vault description string. | no
-`nomembers` | Suppress processing of membership attributes. (bool) | no
 `password` \| `vault_password` \| `ipavaultpassword` \| `old_password`| Vault password. | no
 `password_file` \| `vault_password_file` \| `old_password_file`| File containing Base64 encoded Vault password. | no
 `new_password` | Vault new password. | no
 `new_password_file` | File containing Base64 encoded new Vault password. | no
-`public_key ` \| `vault_public_key` \| `old_password_file` | Base64 encoded vault public key. | no
+`public_key ` \| `vault_public_key` \| `ipavaultpublickey` | Base64 encoded vault public key. | no
 `public_key_file` \| `vault_public_key_file` | Path to file with public key. | no
-`private_key `\| `vault_private_key` | Base64 encoded vault private key. Used only to retrieve data. | no
+`private_key `\| `vault_private_key` \| `ipavaultprivatekey` | Base64 encoded vault private key. Used only to retrieve data. | no
 `private_key_file` \| `vault_private_key_file` | Path to file with private key. Used only to retrieve data. | no
 `salt` \| `vault_salt` \| `ipavaultsalt` | Vault salt. | no
 `vault_type` \| `ipavaulttype` | Vault types are based on security level. It can be one of `standard`, `symmetric` or `asymmetric`, default: `symmetric` | no
-`user` \| `username` | Any user can own one or more user vaults. | no
+`username` \| `user` | Any user can own one or more user vaults. | no
 `service` | Any service can own one or more service vaults. | no
 `shared` | Vault is shared. Default to false. (bool) | no
-`users` | Users that are members of the vault. | no
-`groups` | Groups that are member of the vault. | no
-`services` | Services that are member of the vault. | no
+`users` | List of users that are members of the vault. | no
+`groups` | List of groups that are member of the vault. | no
+`services` | List of services that are member of the vault. | no
+`owners` \| `ownerusers` | List of users that are owners of the vault. | no
+`ownergroups` | List of groups that are owners of the vault. | no
+`ownerservices` | List of services that are owners of the vault. | no
 `data` \|`vault_data` \| `ipavaultdata` | Data to be stored in the vault. | no
 `in` \| `datafile_in` | Path to file with data to be stored in the vault. | no
 `out` \| `datafile_out` | Path to file to store data retrieved from the vault. | no

README.md

@@ -3,7 +3,7 @@ FreeIPA Ansible collection
 
 This repository contains [Ansible](https://www.ansible.com/) roles and playbooks to install and uninstall [FreeIPA](https://www.freeipa.org/) `servers`, `replicas` and `clients`. Also modules for group, host, topology and user management.
 
-**Note**: The ansible playbooks and roles 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.
+**Note**: The Ansible playbooks and roles 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
 --------
@@ -11,6 +11,11 @@ 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 automembership rule management
+* 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,14 +25,19 @@ 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 server management
 * Modules for service management
 * Modules for sudocmd management
 * Modules for sudocmdgroup management
 * Modules for sudorule management
 * Modules for topology management
-* Modules fot trust management
+* Modules for trust management
 * Modules for user management
 * Modules for vault management
 
@@ -104,7 +114,7 @@ ansible-freeipa/plugins/module_utils to ~/.ansible/plugins/
 
 There are RPM packages available for Fedora 29+. These are installing the roles and modules into the global Ansible directories for `roles`, `plugins/modules` and `plugins/module_utils` in the `/usr/share/ansible` directory. Therefore is it possible to use the roles and modules without adapting the names like it is done in the example playbooks.
 
-**Ansible galaxy**
+**Ansible Galaxy**
 
 This command will get the whole collection from galaxy:
 
@@ -128,7 +138,7 @@ The needed adaptions of collection prefixes for `modules` and `module_utils` wil
 Ansible inventory file
 ----------------------
 
-The most important parts of the inventory file is the definition of the nodes, settings and the management modules. Please remember to use [Ansible vault](https://docs.ansible.com/ansible/latest/user_guide/vault.html) for passwords. The examples here are not using vault for better readability.
+The most important parts of the inventory file is the definition of the nodes, settings and the management modules. Please remember to use [Ansible Vault](https://docs.ansible.com/ansible/latest/user_guide/vault.html) for passwords. The examples here are not using vault for better readability.
 
 **Master server**
 
@@ -146,7 +156,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
@@ -272,7 +282,7 @@ ipaserver_domain=test.local
 ipaserver_realm=TEST.LOCAL
 ```
 
-For enhanced security it is possible to use a auto-generated one-time-password (OTP). This will be generated on the controller using the (first) server. It is needed to have the Python gssapi bindings installed on the controller for this.
+For enhanced security it is possible to use a auto-generated one-time-password (OTP). This will be generated on the controller using the (first) server. It is needed to have the python-gssapi bindings installed on the controller for this.
 To enable the generation of the one-time-password:
 ```yaml
 [ipaclients:vars]
@@ -337,7 +347,7 @@ With this playbook it is possible to add a list of topology segments using the `
 Playbooks
 =========
 
-The playbooks needed to deploy or undeploy server, replicas and clients are part of the repository and placed in the playbooks folder. There are also playbooks to deploy and undeploy clusters. With them it is only needed to add an inventory file:
+The playbooks needed to deploy or undeploy servers, replicas and clients are part of the repository and placed in the playbooks folder. There are also playbooks to deploy and undeploy clusters. With them it is only needed to add an inventory file:
 ```
 playbooks\
         install-client.yml
@@ -358,7 +368,7 @@ ansible-playbook -v -i inventory/hosts install-server.yml
 ```
 This will deploy the master server defined in the inventory file.
 
-If Ansible vault is used for passwords, then it is needed to adapt the playbooks in this way:
+If Ansible Vault is used for passwords, then it is needed to adapt the playbooks in this way:
 ```yaml
 ---
 - name: Playbook to configure IPA servers
@@ -408,10 +418,14 @@ 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
 =========================
 
+* [ipaautomember](README-automember.md)
+* [ipaconfig](README-config.md)
+* [ipadelegation](README-delegation.md)
 * [ipadnsconfig](README-dnsconfig.md)
 * [ipadnsforwardzone](README-dnsforwardzone.md)
 * [ipadnsrecord](README-dnsrecord.md)
@@ -422,8 +436,13 @@ 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)
+* [ipaserver](README-server.md)
 * [ipaservice](README-service.md)
 * [ipasudocmd](README-sudocmd.md)
 * [ipasudocmdgroup](README-sudocmdgroup.md)

galaxy.yml

@@ -14,8 +14,6 @@ issues: "https://github.com/freeipa/ansible-freeipa/issues"
 readme: "README.md"
 license: "GPL-3.0-or-later"
 
-dependencies:
-
 tags:
   - "system"
   - "identity"

meta/runtime.yml

@@ -0,0 +1 @@
+requires_ansible: ">=2.9"

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:

molecule/resources/playbooks/prepare-build.yml

@@ -25,3 +25,4 @@
       ipadm_password: SomeDMpassword
       ipaserver_domain: test.local
       ipaserver_realm: TEST.LOCAL
+      ipaclient_no_ntp: yes

playbooks/automember/automember-group-absent.yml

@@ -0,0 +1,11 @@
+---
+- name: Automember group absent example
+  hosts: ipaserver
+  become: true
+  tasks:
+  - name: Ensure group automember rule admins is absent
+    ipaautomember:
+      ipaadmin_password: SomeADMINpassword
+      name: admins
+      automember_type: group
+      state: absent

playbooks/automember/automember-group-present.yml

@@ -0,0 +1,11 @@
+---
+- name: Automember group present example
+  hosts: ipaserver
+  become: true
+  tasks:
+  - name: Ensure group automember rule admins is present
+    ipaautomember:
+      ipaadmin_password: SomeADMINpassword
+      name: admins
+      automember_type: group
+      state: present

playbooks/automember/automember-hostgroup-absent.yml

@@ -0,0 +1,11 @@
+---
+- name: Automember hostgroup absent example
+  hosts: ipaserver
+  become: true
+  tasks:
+  - name: Ensure hostgroup automember rule ipaservers is absent
+    ipaautomember:
+      ipaadmin_password: SomeADMINpassword
+      name: ipaservers
+      automember_type: hostgroup
+      state: absent

playbooks/automember/automember-hostgroup-present.yml

@@ -0,0 +1,11 @@
+---
+- name: Automember hostgroup present example
+  hosts: ipaserver
+  become: true
+  tasks:
+  - name: Ensure hostgroup automember rule ipaservers is absent
+    ipaautomember:
+      ipaadmin_password: SomeADMINpassword
+      name: ipaservers
+      automember_type: hostgroup
+      state: present

playbooks/automember/automember-hostgroup-rule-absent.yml

@@ -0,0 +1,15 @@
+---
+- name: Automember hostgroup rule member absent example
+  hosts: ipaserver
+  become: true
+  tasks:
+  - name: Ensure hostgroup automember condition is absent
+    ipaautomember:
+      ipaadmin_password: SomeADMINpassword
+      name: "My domain hosts"
+      automember_type: hostgroup
+      state: absent
+      action: member
+      inclusive:
+        - key: fqdn
+          expression: ".*.mydomain.com"

playbooks/automember/automember-hostgroup-rule-present.yml

@@ -0,0 +1,15 @@
+---
+- name: Automember hostgroup rule member present example
+  hosts: ipaserver
+  become: true
+  tasks:
+  - name: Ensure hostgroup automember condition is present
+    ipaautomember:
+      ipaadmin_password: SomeADMINpassword
+      name: "My domain hosts"
+      automember_type: hostgroup
+      state: present
+      action: member
+      inclusive:
+        - key: fqdn
+          expression: ".*.mydomain.com"

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/server/server-absent-continue.yml

@@ -0,0 +1,12 @@
+---
+- name: Server absent continuous mode example
+  hosts: ipaserver
+  become: true
+
+  tasks:
+  - name: Ensure server "absent.example.com" is absent continuous mode
+    ipaserver:
+      ipaadmin_password: SomeADMINpassword
+      name: absent.example.com
+      continue: yes
+      state: absent

playbooks/server/server-absent-force.yml

@@ -0,0 +1,12 @@
+---
+- name: Server absent with force example
+  hosts: ipaserver
+  become: true
+
+  tasks:
+  - name: Ensure server "absent.example.com" is absent with force
+    ipaserver:
+      ipaadmin_password: SomeADMINpassword
+      name: absent.example.com
+      force: yes
+      state: absent

playbooks/server/server-absent-ignore_last_of_role.yml

@@ -0,0 +1,12 @@
+---
+- name: Server absent with last of role skip example
+  hosts: ipaserver
+  become: true
+
+  tasks:
+  - name: Ensure server "absent.example.com" is absent with last of role skip
+    ipaserver:
+      ipaadmin_password: SomeADMINpassword
+      name: absent.example.com
+      ignore_last_of_role: yes
+      state: absent

playbooks/server/server-absent-ignore_topology_disconnect.yml

@@ -0,0 +1,12 @@
+---
+- name: Server absent with ignoring topology disconnects example
+  hosts: ipaserver
+  become: true
+
+  tasks:
+  - name: Ensure server "absent.example.com" is absent with ignoring topology disconnects
+    ipaserver:
+      ipaadmin_password: SomeADMINpassword
+      name: absent.example.com
+      ignore_topology_disconnect: yes
+      state: absent

playbooks/server/server-absent.yml

@@ -0,0 +1,11 @@
+---
+- name: Server absent example
+  hosts: ipaserver
+  become: true
+
+  tasks:
+  - name: Ensure server "absent.example.com" is absent
+    ipaserver:
+      ipaadmin_password: SomeADMINpassword
+      name: absent.example.com
+      state: absent

playbooks/server/server-hidden.yml

@@ -0,0 +1,11 @@
+---
+- name: Server hidden example
+  hosts: ipaserver
+  become: true
+
+  tasks:
+  - name: Ensure server "ipareplica1.example.com" is hidden
+    ipaserver:
+      ipaadmin_password: SomeADMINpassword
+      name: ipareplica1.example.com
+      hidden: True

playbooks/server/server-location.yml

@@ -0,0 +1,11 @@
+---
+- name: Server enabled example
+  hosts: ipaserver
+  become: true
+
+  tasks:
+  - name: Ensure server "{{ 'ipareplica1.' + ipaserver_domain }}" with location "mylocation"
+    ipaserver:
+      ipaadmin_password: SomeADMINpassword
+      name: "{{ 'ipareplica1.' + ipaserver_domain }}"
+      location: "mylocation"

playbooks/server/server-no-location.yml

@@ -0,0 +1,11 @@
+---
+- name: Server no location example
+  hosts: ipaserver
+  become: true
+
+  tasks:
+  - name: Ensure server "ipareplica1.example.com" with no location
+    ipaserver:
+      ipaadmin_password: SomeADMINpassword
+      name: ipareplica1.example.com
+      location: ""

playbooks/server/server-no-service-weight.yml

@@ -0,0 +1,11 @@
+---
+- name: Server service weight example
+  hosts: ipaserver
+  become: true
+
+  tasks:
+  - name: Ensure server "ipareplica1.example.com" with no service weight
+    ipaserver:
+      ipaadmin_password: SomeADMINpassword
+      name: ipareplica1.example.com
+      service_weight: -1

playbooks/server/server-not-hidden.yml

@@ -0,0 +1,11 @@
+---
+- name: Server not hidden example
+  hosts: ipaserver
+  become: true
+
+  tasks:
+  - name: Ensure server "ipareplica1.example.com" is not hidden
+    ipaserver:
+      ipaadmin_password: SomeADMINpassword
+      name: ipareplica1.example.com
+      hidden: no

playbooks/server/server-present.yml

@@ -0,0 +1,10 @@
+---
+- name: Server present example
+  hosts: ipaserver
+  become: true
+
+  tasks:
+  - name: Ensure server "ipareplica1.exmple.com" is present
+    ipaserver:
+      ipaadmin_password: SomeADMINpassword
+      name: ipareplica1.example.com

playbooks/server/server-service-weight.yml

@@ -0,0 +1,11 @@
+---
+- name: Server service weight example
+  hosts: ipaserver
+  become: true
+
+  tasks:
+  - name: Ensure server "ipareplica1.example.com" with service weight 1
+    ipaserver:
+      ipaadmin_password: SomeADMINpassword
+      name: ipareplica1.example.com
+      service_weight: 1

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 }}"

playbooks/vault/vault-is-present-with-password-file.yml

@@ -7,7 +7,7 @@
   tasks:
   - copy:
       src: "{{ playbook_dir }}/password.txt"
-      dest: "{{ ansible_env.HOME }}/password.txt"
+      dest: "{{ ansible_facts['env'].HOME }}/password.txt"
       owner: "{{ ansible_user }}"
       group: "{{ ansible_user }}"
       mode: 0600
@@ -16,7 +16,7 @@
       name: symvault
       username: admin
       vault_type: symmetric
-      vault_password_file: "{{ ansible_env.HOME }}/password.txt"
+      vault_password_file: "{{ ansible_facts['env'].HOME }}/password.txt"
   - file:
-      path: "{{ ansible_env.HOME }}/password.txt"
+      path: "{{ ansible_facts['env'].HOME }}/password.txt"
       state: absent

playbooks/vault/vault-is-present-with-public-key-file.yml

@@ -12,7 +12,7 @@
   tasks:
   - copy:
       src: "{{ playbook_dir }}/public.pem"
-      dest: "{{ ansible_env.HOME }}/public.pem"
+      dest: "{{ ansible_facts['env'].HOME }}/public.pem"
       owner: "{{ ansible_user }}"
       group: "{{ ansible_user }}"
       mode: 0600
@@ -21,7 +21,7 @@
       name: asymvault
       username: admin
       vault_type: asymmetric
-      vault_public_key_file: "{{ ansible_env.HOME }}/public.pem"
+      vault_public_key_file: "{{ ansible_facts['env'].HOME }}/public.pem"
   - file:
-      path: "{{ ansible_env.HOME }}/public.pem"
+      path: "{{ ansible_facts['env'].HOME }}/public.pem"
       state: absent

plugins/modules/ipaautomember.py

@@ -0,0 +1,454 @@
+#!/usr/bin/python
+# -*- coding: utf-8 -*-
+
+# Authors:
+#   Mark Hahl <mhahl@redhat.com>
+#   Jake Reynolds <jakealexis@gmail.com>
+#
+# Copyright (C) 2021 Red Hat
+# see file 'COPYING' for use and warranty information
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+
+from ansible.module_utils._text import to_text
+from ansible.module_utils.ansible_freeipa_module import (
+    api_command, api_command_no_name, api_connect, compare_args_ipa,
+    gen_add_del_lists, temp_kdestroy, temp_kinit, valid_creds,
+    ipalib_errors
+)
+from ansible.module_utils.basic import AnsibleModule
+
+ANSIBLE_METADATA = {
+    "metadata_version": "1.0",
+    "supported_by": "community",
+    "status": ["preview"],
+}
+
+
+DOCUMENTATION = """
+---
+module: ipaautomember
+short description: Add and delete FreeIPA Auto Membership Rules.
+description: Add, modify and delete an IPA Auto Membership Rules.
+options:
+  ipaadmin_principal:
+    description: The admin principal
+    default: admin
+  ipaadmin_password:
+    description: The admin password
+    required: false
+  name:
+    description: The automember rule
+    required: true
+    aliases: ["cn"]
+  description:
+    description: A description of this auto member rule
+    required: false
+  automember_type:
+    description: Grouping to which the rule applies
+    required: true
+    type: str
+    choices: ["group", "hostgroup"]
+  exclusive:
+    description: List of dictionaries containing the attribute and expression.
+    type: list
+    elements: dict
+    aliases: ["automemberexclusiveregex"]
+    options:
+      key:
+        description: The attribute of the regex
+        type: str
+        required: true
+      expression:
+        description: The expression of the regex
+        type: str
+        required: true
+  inclusive:
+    description: List of dictionaries containing the attribute and expression.
+    type: list
+    elements: dict
+    aliases: ["automemberinclusiveregex"]
+    options:
+      key:
+        description: The attribute of the regex
+        type: str
+        required: true
+      expression:
+        description: The expression of the regex
+        type: str
+        required: true
+  action:
+    description: Work on automember or member level
+    default: automember
+    choices: ["member", "automember"]
+  state:
+    description: State to ensure
+    default: present
+    choices: ["present", "absent"]
+author:
+    - Mark Hahl
+    - Jake Reynolds
+"""
+
+EXAMPLES = """
+# Ensure an automember rule exists
+- ipaautomember:
+    ipaadmin_password: SomeADMINpassword
+    name: admins
+    description: "example description"
+    automember_type: group
+    state: present
+    inclusive:
+    - key: "mail"
+      expression: "example.com$
+
+# Delete an automember rule
+- ipaautomember:
+    ipaadmin_password: SomeADMINpassword
+    name: admins
+    description: "my automember rule"
+    automember_type: group
+    state: absent
+
+# Add an inclusive condition to an existing rule
+- ipaautomember:
+    ipaadmin_password: SomeADMINpassword
+    name: "My domain hosts"
+    automember_tye: hostgroup
+    action: member
+    inclusive:
+      - key: fqdn
+        expression: ".*.mydomain.com"
+
+"""
+
+RETURN = """
+"""
+
+
+def find_automember(module, name, grouping):
+    _args = {
+        "all": True,
+        "type": to_text(grouping)
+    }
+
+    try:
+        _result = api_command(module, "automember_show", to_text(name), _args)
+    except ipalib_errors.NotFound:
+        return None
+    return _result["result"]
+
+
+def gen_condition_args(grouping,
+                       key,
+                       inclusiveregex=None,
+                       exclusiveregex=None):
+    _args = {}
+    if grouping is not None:
+        _args['type'] = to_text(grouping)
+    if key is not None:
+        _args['key'] = to_text(key)
+    if inclusiveregex is not None:
+        _args['automemberinclusiveregex'] = to_text(inclusiveregex)
+    if exclusiveregex is not None:
+        _args['automemberexclusiveregex'] = to_text(exclusiveregex)
+
+    return _args
+
+
+def gen_args(description, grouping):
+    _args = {}
+    if description is not None:
+        _args["description"] = to_text(description)
+    if grouping is not None:
+        _args['type'] = to_text(grouping)
+
+    return _args
+
+
+def transform_conditions(conditions):
+    """Transform a list of dicts into a list with the format of key=value."""
+    transformed = ['%s=%s' % (condition['key'], condition['expression'])
+                   for condition in conditions]
+    return transformed
+
+
+def check_condition_keys(ansible_module, conditions, aciattrs):
+    if conditions is None:
+        return
+    for condition in conditions:
+        if condition["key"] not in aciattrs:
+            ansible_module.fail_json(
+                msg="Invalid automember condition key '%s'" % condition["key"])
+
+
+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),
+
+            inclusive=dict(type="list",
+                           aliases=["automemberinclusiveregex"], default=None,
+                           options=dict(
+                               key=dict(type="str", required=True),
+                               expression=dict(type="str", required=True)
+                           ),
+                           elements="dict", required=False),
+            exclusive=dict(type="list", aliases=[
+                           "automemberexclusiveregex"], default=None,
+                           options=dict(
+                               key=dict(type="str", required=True),
+                               expression=dict(type="str", required=True)
+                           ),
+                           elements="dict", required=False),
+            name=dict(type="list", aliases=["cn"],
+                      default=None, required=True),
+            description=dict(type="str", default=None),
+            automember_type=dict(type='str', required=False,
+                                 choices=['group', 'hostgroup']),
+            action=dict(type="str", default="automember",
+                        choices=["member", "automember"]),
+            state=dict(type="str", default="present",
+                       choices=["present", "absent", "rebuild"]),
+            users=dict(type="list", default=None),
+            hosts=dict(type="list", default=None),
+        ),
+        supports_check_mode=True,
+    )
+
+    ansible_module._ansible_debug = True
+
+    # Get parameters
+
+    # general
+    ipaadmin_principal = ansible_module.params.get("ipaadmin_principal")
+    ipaadmin_password = ansible_module.params.get("ipaadmin_password")
+    names = ansible_module.params.get("name")
+
+    # present
+    description = ansible_module.params.get("description")
+
+    # conditions
+    inclusive = ansible_module.params.get("inclusive")
+    exclusive = ansible_module.params.get("exclusive")
+
+    # action
+    action = ansible_module.params.get("action")
+    # state
+    state = ansible_module.params.get("state")
+
+    # grouping/type
+    automember_type = ansible_module.params.get("automember_type")
+
+    rebuild_users = ansible_module.params.get("users")
+    rebuild_hosts = ansible_module.params.get("hosts")
+
+    if (rebuild_hosts or rebuild_users) and state != "rebuild":
+        ansible_module.fail_json(
+            msg="'hosts' and 'users' are only valid with state: rebuild")
+    if not automember_type and state != "rebuild":
+        ansible_module.fail_json(
+            msg="'automember_type' is required unless state: rebuild")
+
+    # Init
+    changed = False
+    exit_args = {}
+    ccache_dir = None
+    ccache_name = None
+    res_find = 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 automember rule exists
+            res_find = find_automember(ansible_module, name, automember_type)
+
+            # Check inclusive and exclusive conditions
+            if inclusive is not None or exclusive is not None:
+                # automember_type is either "group" or "hostgorup"
+                if automember_type == "group":
+                    _type = "user"
+                elif automember_type == "hostgroup":
+                    _type = "host"
+                else:
+                    ansible_module.fail_json(
+                        msg="Bad automember type '%s'" % automember_type)
+
+                try:
+                    aciattrs = api_command(
+                        ansible_module, "json_metadata", to_text(_type), {}
+                    )['objects'][_type]['aciattrs']
+                except Exception as ex:
+                    ansible_module.fail_json(
+                        msg="%s: %s: %s" % ("json_metadata", _type, str(ex)))
+
+                check_condition_keys(ansible_module, inclusive, aciattrs)
+                check_condition_keys(ansible_module, exclusive, aciattrs)
+
+            # Create command
+            if state == 'present':
+                args = gen_args(description, automember_type)
+
+                if action == "automember":
+                    if res_find is not None:
+                        if not compare_args_ipa(ansible_module,
+                                                args,
+                                                res_find,
+                                                ignore=['type']):
+                            commands.append([name, 'automember_mod', args])
+                    else:
+                        commands.append([name, 'automember_add', args])
+                        res_find = {}
+
+                    inclusive_add, inclusive_del = gen_add_del_lists(
+                        transform_conditions(inclusive or []),
+                        res_find.get("automemberinclusiveregex", [])
+                    )
+
+                    exclusive_add, exclusive_del = gen_add_del_lists(
+                        transform_conditions(exclusive or []),
+                        res_find.get("automemberexclusiveregex", [])
+                    )
+
+                elif action == "member":
+                    if res_find is None:
+                        ansible_module.fail_json(
+                            msg="No automember '%s'" % name)
+
+                    inclusive_add = transform_conditions(inclusive or [])
+                    inclusive_del = []
+                    exclusive_add = transform_conditions(exclusive or [])
+                    exclusive_del = []
+
+                for _inclusive in inclusive_add:
+                    key, regex = _inclusive.split("=", 1)
+                    condition_args = gen_condition_args(
+                        automember_type, key, inclusiveregex=regex)
+                    commands.append([name, 'automember_add_condition',
+                                     condition_args])
+
+                for _inclusive in inclusive_del:
+                    key, regex = _inclusive.split("=", 1)
+                    condition_args = gen_condition_args(
+                        automember_type, key, inclusiveregex=regex)
+                    commands.append([name, 'automember_remove_condition',
+                                     condition_args])
+
+                for _exclusive in exclusive_add:
+                    key, regex = _exclusive.split("=", 1)
+                    condition_args = gen_condition_args(
+                        automember_type, key, exclusiveregex=regex)
+                    commands.append([name, 'automember_add_condition',
+                                     condition_args])
+
+                for _exclusive in exclusive_del:
+                    key, regex = _exclusive.split("=", 1)
+                    condition_args = gen_condition_args(
+                        automember_type, key, exclusiveregex=regex)
+                    commands.append([name, 'automember_remove_condition',
+                                     condition_args])
+
+            elif state == 'absent':
+                if action == "automember":
+                    if res_find is not None:
+                        commands.append([name, 'automember_del',
+                                         {'type': to_text(automember_type)}])
+
+                elif action == "member":
+                    if res_find is None:
+                        ansible_module.fail_json(
+                            msg="No automember '%s'" % name)
+
+                    if inclusive is not None:
+                        for _inclusive in transform_conditions(inclusive):
+                            key, regex = _inclusive.split("=", 1)
+                            condition_args = gen_condition_args(
+                                automember_type, key, inclusiveregex=regex)
+                            commands.append(
+                                [name, 'automember_remove_condition',
+                                 condition_args])
+
+                    if exclusive is not None:
+                        for _exclusive in transform_conditions(exclusive):
+                            key, regex = _exclusive.split("=", 1)
+                            condition_args = gen_condition_args(
+                                automember_type, key, exclusiveregex=regex)
+                            commands.append([name,
+                                             'automember_remove_condition',
+                                            condition_args])
+
+            elif state == "rebuild":
+                if automember_type:
+                    commands.append([None, 'automember_rebuild',
+                                     {"type": to_text(automember_type)}])
+                if rebuild_users:
+                    commands.append([None, 'automember_rebuild',
+                                    {"users": [
+                                        to_text(_u)
+                                        for _u in rebuild_users]}])
+                if rebuild_hosts:
+                    commands.append([None, 'automember_rebuild',
+                                    {"hosts": [
+                                        to_text(_h)
+                                        for _h in rebuild_hosts]}])
+
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
+        for name, command, args in commands:
+            try:
+                if name is None:
+                    result = api_command_no_name(ansible_module, command, args)
+                else:
+                    result = api_command(ansible_module, command,
+                                         to_text(name), args)
+
+                if "completed" in result:
+                    if result["completed"] > 0:
+                        changed = True
+                else:
+                    changed = True
+            except Exception as ex:
+                ansible_module.fail_json(msg="%s: %s: %s" % (command, name,
+                                                             str(ex)))
+
+            # result["failed"] is used only for INCLUDE_RE, EXCLUDE_RE
+            # if entries could not be added that are already there and
+            # it entries could not be removed that are not there.
+            # All other issues like invalid attributes etc. are handled
+            # as exceptions. Therefore the error section is not here as
+            # in other modules.
+
+    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/ipaconfig.py

@@ -254,8 +254,7 @@ config:
 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_no_name, \
-    compare_args_ipa, module_params_get
-import ipalib.errors
+    compare_args_ipa, module_params_get, ipalib_errors
 
 
 def config_show(module):
@@ -265,10 +264,7 @@ def config_show(module):
 
 
 def gen_args(params):
-    _args = {}
-    for k, v in params.items():
-        if v is not None:
-            _args[k] = v
+    _args = {k: v for k, v in params.items() if v is not None}
 
     return _args
 
@@ -369,7 +365,7 @@ def main():
     reverse_field_map = {v: k for k, v in field_map.items()}
 
     params = {}
-    for x in field_map.keys():
+    for x in field_map:
         val = module_params_get(ansible_module, x)
 
         if val is not None:
@@ -403,11 +399,11 @@ def main():
         ("ipasearchrecordslimit", -1, 2147483647),
         ("ipapwdexpadvnotify", 0, 2147483647),
     ]
-    for arg, min, max in args_with_limits:
-        if arg in params and (params[arg] > max or params[arg] < min):
+    for arg, minimum, maximum in args_with_limits:
+        if arg in params and (params[arg] > maximum or params[arg] < minimum):
             ansible_module.fail_json(
                 msg="Argument '%s' must be between %d and %d."
-                    % (arg, min, max))
+                    % (arg, minimum, maximum))
 
     changed = False
     exit_args = {}
@@ -428,13 +424,14 @@ 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", {})
             result = rawresult['result']
             del result['dn']
-            for key, v in result.items():
+            for key, value in result.items():
                 k = reverse_field_map.get(key, key)
                 if ansible_module.argument_spec.get(k):
                     if k == 'ipaselinuxusermaporder':
@@ -449,21 +446,21 @@ def main():
                     elif k == 'groupsearch':
                         exit_args['groupsearch'] = \
                             result.get(key)[0].split(',')
-                    elif isinstance(v, str) and \
+                    elif isinstance(value, str) and \
                             ansible_module.argument_spec[k]['type'] == "list":
-                        exit_args[k] = [v]
-                    elif isinstance(v, list) and \
+                        exit_args[k] = [value]
+                    elif isinstance(value, list) and \
                             ansible_module.argument_spec[k]['type'] == "str":
-                        exit_args[k] = ",".join(v)
-                    elif isinstance(v, list) and \
+                        exit_args[k] = ",".join(value)
+                    elif isinstance(value, list) and \
                             ansible_module.argument_spec[k]['type'] == "int":
-                        exit_args[k] = ",".join(v)
-                    elif isinstance(v, list) and \
+                        exit_args[k] = ",".join(value)
+                    elif isinstance(value, list) and \
                             ansible_module.argument_spec[k]['type'] == "bool":
-                        exit_args[k] = (v[0] == "TRUE")
+                        exit_args[k] = (value[0] == "TRUE")
                     else:
-                        exit_args[k] = v
-    except ipalib.errors.EmptyModlist:
+                        exit_args[k] = value
+    except ipalib_errors.EmptyModlist:
         changed = False
     except Exception as e:
         ansible_module.fail_json(msg="%s %s" % (params, str(e)))

plugins/modules/ipadelegation.py

@@ -50,7 +50,7 @@ options:
     description: Attribute list to which the delegation applies
     required: false
     aliases: ["attrs"]
-  membergroup
+  membergroup:
     description: User group to apply delegation to
     required: false
     aliases: ["memberof"]
@@ -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

@@ -114,8 +114,8 @@ def find_dnsconfig(module):
         if _result["result"].get('idnsforwarders', None) is None:
             _result["result"]['idnsforwarders'] = ['']
         return _result["result"]
-    else:
-        module.fail_json(msg="Could not retrieve current DNS configuration.")
+
+    module.fail_json(msg="Could not retrieve current DNS configuration.")
     return None
 
 
@@ -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
 
@@ -124,8 +135,8 @@ def find_dnsforwardzone(module, name):
             msg="There is more than one dnsforwardzone '%s'" % (name))
     elif len(_result["result"]) == 1:
         return _result["result"][0]
-    else:
-        return None
+
+    return None
 
 
 def gen_args(forwarders, forwardpolicy, skip_overlap_check):
@@ -369,8 +380,14 @@ def main():
                         [name, 'dnsforwardzone_remove_permission', {}]
                     )
 
-            for name, command, args in commands:
-                api_command(ansible_module, command, name, args)
+            # 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
 
     except Exception as e:

plugins/modules/ipadnsrecord.py

@@ -49,9 +49,11 @@ options:
         aliases: ["record_name"]
         required: true
       zone_name:
-        description: The DNS zone name to which DNS record needs to be managed.
+        description: |
+          The DNS zone name to which DNS record needs to be managed.
+          Required if not provided globally.
         aliases: ["dnszone"]
-        required: true (if not provided globally)
+        required: false
       record_type:
         description: The type of DNS record.
         choices: ["A", "AAAA", "A6", "AFSDB", "CERT", "CNAME", "DLV", "DNAME",
@@ -159,7 +161,7 @@ options:
         required: false
         type: string
       a_create_reverse:
-        description:
+        description: |
           Create reverse record for A records.
           There is no equivalent to remove reverse records.
         type: bool
@@ -169,13 +171,13 @@ options:
         required: false
         type: string
       aaaa_create_reverse:
-        description:
+        description: |
           Create reverse record for AAAA records.
           There is no equivalent to remove reverse records.
         type: bool
         required: false
       create_reverse:
-        description:
+        description: |
           Create reverse record for A or AAAA record types.
           There is no equivalent to remove reverse records.
         type: bool
@@ -189,11 +191,11 @@ options:
         required: false
         type: int
       afsdb_hostname:
-        discription: AFSDB Hostname
+        description: AFSDB Hostname
         required: false
         type: string
       cert_type:
-        descriptioon: CERT Certificate Type
+        description: CERT Certificate Type
         required: false
         type: int
       cert_key_tag:
@@ -225,7 +227,7 @@ options:
         required: false
         type: int
       dlv_digest:
-        descriptinion: DLV Digest
+        description: DLV Digest
         required: false
         type: string
       dname_target:
@@ -245,11 +247,11 @@ options:
         required: false
         type: int
       ds_digest:
-        descriptinion: DS Digest
+        description: DS Digest
         required: false
         type: string
       kx_preference:
-        description:
+        description: |
           Preference given to this exchanger. Lower values are more preferred.
         required: false
         type: int
@@ -306,7 +308,7 @@ options:
         required: false
         type: float
       mx_preference:
-        description:
+        description: |
           Preference given to this exchanger. Lower values are more preferred.
         required: false
         type: int
@@ -347,7 +349,7 @@ options:
         required: false
         type: string
       srv_priority:
-        description:
+        description: |
           Lower number means higher priority. Clients will attempt to contact
           the server with the lowest-numbered priority they can reach.
         required: false
@@ -361,13 +363,15 @@ options:
         required: false
         type: int
       srv_target:
-        description:
+        description: |
           The domain name of the target host or '.' if the service is decidedly
           not available at this domain.
         required: false
         type: string
       sshfp_algorithm:
         description: SSHFP Algorithm
+        required: False
+        type: int
       sshfp_fp_type:
         description: SSHFP Fingerprint Type
         required: False
@@ -385,15 +389,15 @@ options:
         required: false
         type: int
       tlsa_selector:
-        descrpition: TLSA Selector
+        description: TLSA Selector
         required: false
         type: int
       tlsa_matching_type:
-        descrpition: TLSA Matching Type
+        description: TLSA Matching Type
         required: false
         type: int
       tlsa_cert_association_data:
-        descrpition: TLSA Certificate Association Data
+        description: TLSA Certificate Association Data
         required: false
         type: string
       uri_target:
@@ -401,7 +405,7 @@ options:
         required: false
         type: string
       uri_priority:
-        description:
+        description: |
           Lower number means higher priority. Clients will attempt to contact
           the URI with the lowest-numbered priority they can reach.
         required: false
@@ -411,9 +415,11 @@ options:
         required: false
         type: int
   zone_name:
-    description: The DNS zone name to which DNS record needs to be managed.
+    description: |
+      The DNS zone name to which DNS record needs to be managed.
+      Required if not provided globally.
     aliases: ["dnszone"]
-    required: true (if not provided on each record)
+    required: false
   name:
     description: The DNS record name to manage.
     aliases: ["record_name"]
@@ -519,11 +525,10 @@ 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:
-    description:
+    description: |
       Create reverse record for A or AAAA record types.
       There is no equivalent to remove reverse records.
     type: bool
@@ -534,7 +539,7 @@ options:
     required: false
     type: string
   a_create_reverse:
-    description:
+    description: |
       Create reverse record for A records.
       There is no equivalent to remove reverse records.
     type: bool
@@ -544,7 +549,7 @@ options:
     required: false
     type: string
   aaaa_create_reverse:
-    description:
+    description: |
       Create reverse record for AAAA records.
       There is no equivalent to remove reverse records.
     type: bool
@@ -554,11 +559,11 @@ options:
     required: false
     type: int
   afsdb_hostname:
-    discription: AFSDB Hostname
+    description: AFSDB Hostname
     required: false
     type: string
   cert_type:
-    descriptioon: CERT Certificate Type
+    description: CERT Certificate Type
     required: false
     type: int
   cert_key_tag:
@@ -590,7 +595,7 @@ options:
     required: false
     type: int
   dlv_digest:
-    descriptinion: DLV Digest
+    description: DLV Digest
     required: false
     type: string
   dname_target:
@@ -610,11 +615,11 @@ options:
     required: false
     type: int
   ds_digest:
-    descriptinion: DS Digest
+    description: DS Digest
     required: false
     type: string
   kx_preference:
-    description:
+    description: |
       Preference given to this exchanger. Lower values are more preferred.
     required: false
     type: int
@@ -671,7 +676,7 @@ options:
     required: false
     type: float
   mx_preference:
-    description:
+    description: |
       Preference given to this exchanger. Lower values are more preferred.
     required: false
     type: int
@@ -712,7 +717,7 @@ options:
     required: false
     type: string
   srv_priority:
-    description:
+    description: |
       Lower number means higher priority. Clients will attempt to contact the
       server with the lowest-numbered priority they can reach.
     required: false
@@ -726,20 +731,22 @@ options:
     required: false
     type: int
   srv_target:
-    description:
+    description: |
       The domain name of the target host or '.' if the service is decidedly not
       available at this domain.
     required: false
     type: string
   sshfp_algorithm:
     description: SSHFP Algorithm
+    required: false
+    type: int
   sshfp_fp_type:
     description: SSHFP Fingerprint Type
-    required: False
+    required: false
     type: int
   sshfp_fingerprint:
     description: SSHFP Fingerprint
-    required: False
+    required: false
     type: string
   txt_data:
     description: TXT Text Data
@@ -750,15 +757,15 @@ options:
     required: false
     type: int
   tlsa_selector:
-    descrpition: TLSA Selector
+    description: TLSA Selector
     required: false
     type: int
   tlsa_matching_type:
-    descrpition: TLSA Matching Type
+    description: TLSA Matching Type
     required: false
     type: int
   tlsa_cert_association_data:
-    descrpition: TLSA Certificate Association Data
+    description: TLSA Certificate Association Data
     required: false
     type: string
   uri_target:
@@ -766,7 +773,7 @@ options:
     required: false
     type: string
   uri_priority:
-    description:
+    description: |
       Lower number means higher priority. Clients will attempt to contact the
       URI with the lowest-numbered priority they can reach.
     required: false
@@ -861,10 +868,10 @@ 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, module_params_get, \
-    is_ipv4_addr, is_ipv6_addr
+    is_ipv4_addr, is_ipv6_addr, ipalib_errors
 import dns.reversename
 import dns.resolver
-import ipalib.errors
+
 import six
 
 
@@ -882,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',
@@ -945,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": [
@@ -952,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'
     ],
@@ -1125,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)
-    except ipalib.errors.NotFound:
+            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):
@@ -1166,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)
 
@@ -1278,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())
@@ -1316,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
@@ -1327,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:
@@ -1338,36 +1359,36 @@ 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:
+                for f in part_fields:   # pylint: disable=invalid-name
                     if f in args:
                         del args[f]
             else:
                 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
@@ -1382,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)
@@ -1396,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
@@ -1474,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:
@@ -1485,9 +1509,9 @@ def main():
                 else:
                     changed = True
 
-            except ipalib.errors.EmptyModlist:
+            except ipalib_errors.EmptyModlist:
                 continue
-            except ipalib.errors.DuplicateEntry:
+            except ipalib_errors.DuplicateEntry:
                 continue
             except Exception as e:
                 error_message = str(e)

plugins/modules/ipadnszone.py

@@ -27,6 +27,7 @@ ANSIBLE_METADATA = {
 }
 
 DOCUMENTATION = """
+---
 module: ipadnszone
 short description: Manage FreeIPA dnszone
 description: Manage FreeIPA dnszone
@@ -37,7 +38,6 @@ options:
   ipaadmin_password:
     description: The admin password
     required: false
-
   name:
     description: The zone name string.
     required: true
@@ -132,11 +132,14 @@ options:
     required: false
     type: int
   nsec3param_rec:
-    description: NSEC3PARAM record for zone in format: hash_algorithm flags iterations salt.
+    description: |
+      NSEC3PARAM record for zone in format: hash_algorithm flags iterations
+       salt.
     required: false
     type: str
   skip_overlap_check:
-    description: Force DNS zone creation even if it will overlap with an existing zone
+    description: |
+      Force DNS zone creation even if it will overlap with an existing zone
     required: false
     type: bool
   skip_nameserver_check:
@@ -207,9 +210,10 @@ dnszone:
 from ipapython.dnsutil import DNSName  # noqa: E402
 from ansible.module_utils.ansible_freeipa_module import (
     FreeIPABaseModule,
-    is_ipv4_addr,
-    is_ipv6_addr,
+    is_ip_address,
+    is_ip_network_address,
     is_valid_port,
+    ipalib_errors
 )  # noqa: E402
 import netaddr
 import six
@@ -248,27 +252,29 @@ 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)
 
-    def is_valid_nsec3param_rec(self, nsec3param_rec):
+    def is_valid_nsec3param_rec(self, nsec3param_rec):  # pylint: disable=R0201
         try:
             part1, part2, part3, part4 = nsec3param_rec.split(" ")
         except ValueError:
             return False
 
-        if not all([part1.isdigit(), part2.isdigit(), part3.isdigit()]):
-            return False
-
-        if not 0 <= int(part1) <= 255:
-            return False
-
-        if not 0 <= int(part2) <= 255:
-            return False
-
-        if not 0 <= int(part3) <= 65535:
+        if (
+            not all([part1.isdigit(), part2.isdigit(), part3.isdigit()])
+            or not 0 <= int(part1) <= 255
+            or not 0 <= int(part2) <= 255
+            or not 0 <= int(part3) <= 65535
+        ):
             return False
 
         try:
@@ -288,7 +294,7 @@ class DNSZoneModule(FreeIPABaseModule):
 
         return True
 
-    def get_ipa_nsec3paramrecord(self, **kwargs):
+    def get_ipa_nsec3paramrecord(self, **_kwargs):  # pylint: disable=R1710
         nsec3param_rec = self.ipa_params.nsec3param_rec
         if nsec3param_rec is not None:
             error_msg = (
@@ -300,12 +306,12 @@ class DNSZoneModule(FreeIPABaseModule):
                 self.fail_json(msg=error_msg)
             return nsec3param_rec
 
-    def get_ipa_idnsforwarders(self, **kwargs):
+    def get_ipa_idnsforwarders(self, **_kwargs):  # pylint: disable=R1710
         if self.ipa_params.forwarders is not None:
             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
                     )
@@ -324,14 +330,14 @@ class DNSZoneModule(FreeIPABaseModule):
 
             return forwarders
 
-    def get_ipa_idnsallowtransfer(self, **kwargs):
+    def get_ipa_idnsallowtransfer(self, **_kwargs):  # pylint: disable=R1710
         if self.ipa_params.allow_transfer is not None:
             error_msg = "Invalid ip_address for DNS allow_transfer: %s"
             self.validate_ips(self.ipa_params.allow_transfer, error_msg)
 
             return (";".join(self.ipa_params.allow_transfer) or "none") + ";"
 
-    def get_ipa_idnsallowquery(self, **kwargs):
+    def get_ipa_idnsallowquery(self, **_kwargs):  # pylint: disable=R1710
         if self.ipa_params.allow_query is not None:
             error_msg = "Invalid ip_address for DNS allow_query: %s"
             self.validate_ips(self.ipa_params.allow_query, error_msg)
@@ -354,27 +360,27 @@ class DNSZoneModule(FreeIPABaseModule):
 
         return ".".join((name, domain))
 
-    def get_ipa_idnssoarname(self, **kwargs):
+    def get_ipa_idnssoarname(self, **_kwargs):  # pylint: disable=R1710
         if self.ipa_params.admin_email is not None:
             return DNSName(
                 self._replace_at_symbol_in_rname(self.ipa_params.admin_email)
             )
 
-    def get_ipa_idnssoamname(self, **kwargs):
+    def get_ipa_idnssoamname(self, **_kwargs):  # pylint: disable=R1710
         if self.ipa_params.name_server is not None:
             return DNSName(self.ipa_params.name_server)
 
-    def get_ipa_skip_overlap_check(self, **kwargs):
+    def get_ipa_skip_overlap_check(self, **kwargs):  # pylint: disable=R1710
         zone = kwargs.get('zone')
         if not zone and self.ipa_params.skip_overlap_check is not None:
             return self.ipa_params.skip_overlap_check
 
-    def get_ipa_skip_nameserver_check(self, **kwargs):
+    def get_ipa_skip_nameserver_check(self, **kwargs):  # pylint: disable=R1710
         zone = kwargs.get('zone')
         if not zone and self.ipa_params.skip_nameserver_check is not None:
             return self.ipa_params.skip_nameserver_check
 
-    def __reverse_zone_name(self, ipaddress):
+    def __reverse_zone_name(self, ipaddress):  # pylint: disable=R1710
         """
         Infer reverse zone name from an ip address.
 
@@ -394,20 +400,20 @@ class DNSZoneModule(FreeIPABaseModule):
             ip_version = ip.version
         if ip_version == 4:
             return u'.'.join(items[4 - prefixlen // 8:])
-        elif ip_version == 6:
+        if ip_version == 6:
             return u'.'.join(items[32 - prefixlen // 4:])
-        else:
-            self.fail_json(msg="Invalid IP version for reverse zone.")
+        self.fail_json(msg="Invalid IP version for reverse zone.")
 
     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
@@ -445,7 +451,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:
@@ -453,7 +462,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
@@ -461,28 +470,37 @@ 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
+            # Only set SOA Serial if it is not set already.
+            if (set_serial and
+                (zone is None
+                 or "idnssoaserial" not in zone
+                 or zone["idnssoaserial"] is None
+                 or zone["idnssoaserial"][0] != str(self.ipa_params.serial)
+                 )):
                 args = {
                     "idnssoaserial": self.ipa_params.serial,
                 }
                 self.add_ipa_command("dnszone_mod", zone_name, args)
 
     def process_command_result(self, name, command, args, result):
+        # pylint: disable=super-with-arguments
         super(DNSZoneModule, self).process_command_result(
             name, command, args, result
         )

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
@@ -171,7 +185,8 @@ 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, \
-    api_check_param, module_params_get, gen_add_del_lists, api_check_command
+    api_check_param, module_params_get, gen_add_del_lists, api_check_command, \
+    gen_add_list, gen_intersection_list
 
 
 def find_group(module, name):
@@ -187,8 +202,8 @@ def find_group(module, name):
             msg="There is more than one group '%s'" % (name))
     elif len(_result["result"]) == 1:
         return _result["result"][0]
-    else:
-        return None
+
+    return None
 
 
 def gen_args(description, gid, nomembers):
@@ -203,7 +218,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 +226,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 +253,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 +269,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 +299,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 +340,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 +367,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 +437,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 +462,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,21 +542,58 @@ 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."
+                        )
+
+                    # Reduce add lists for member_user, member_group,
+                    # member_service and member_external to new entries
+                    # only that are not in res_find.
+                    if user is not None and "member_user" in res_find:
+                        user = gen_add_list(
+                            user, res_find["member_user"])
+                    if group is not None and "member_group" in res_find:
+                        group = gen_add_list(
+                            group, res_find["member_group"])
+                    if service is not None and "member_service" in res_find:
+                        service = gen_add_list(
+                            service, res_find["member_service"])
+                    if externalmember is not None \
+                       and "member_external" in res_find:
+                        externalmember = gen_add_list(
+                            externalmember, res_find["member_external"])
+
+                    if any([user, group, service, externalmember]):
+                        commands.append(
+                            [name, "group_add_member", add_member_args]
+                        )
 
                     if has_add_membermanager:
+                        # Reduce add list for membermanager_user and
+                        # membermanager_group to new entries only that are
+                        # not in res_find.
+                        if membermanager_user is not None \
+                           and "membermanager_user" in res_find:
+                            membermanager_user = gen_add_list(
+                                membermanager_user,
+                                res_find["membermanager_user"])
+                        if membermanager_group is not None \
+                           and "membermanager_group" in res_find:
+                            membermanager_group = gen_add_list(
+                                membermanager_group,
+                                res_find["membermanager_group"])
+
                         # Add membermanager users and groups
                         if membermanager_user is not None or \
                            membermanager_group is not None:
@@ -527,21 +614,54 @@ 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."
+                        )
+
+                    # Reduce del lists of member_user, member_group,
+                    # member_service and member_external to the entries only
+                    # that are in res_find.
+                    if user is not None:
+                        user = gen_intersection_list(
+                            user, res_find.get("member_user"))
+                    if group is not None:
+                        group = gen_intersection_list(
+                            group, res_find.get("member_group"))
+                    if service is not None:
+                        service = gen_intersection_list(
+                            service, res_find.get("member_service"))
+                    if externalmember is not None:
+                        externalmember = gen_intersection_list(
+                            externalmember, res_find.get("member_external"))
+
+                    if any([user, group, service, externalmember]):
+                        commands.append(
+                            [name, "group_remove_member", del_member_args]
+                        )
 
                     if has_add_membermanager:
+                        # Reduce del lists of membermanager_user and
+                        # membermanager_group to the entries only that are
+                        # in res_find.
+                        if membermanager_user is not None:
+                            membermanager_user = gen_intersection_list(
+                                membermanager_user,
+                                res_find.get("membermanager_user"))
+                        if membermanager_group is not None:
+                            membermanager_group = gen_intersection_list(
+                                membermanager_group,
+                                res_find.get("membermanager_group"))
+
                         # Remove membermanager users and groups
                         if membermanager_user is not None or \
                            membermanager_group is not None:
@@ -556,6 +676,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:
@@ -571,16 +695,12 @@ def main():
                 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:

plugins/modules/ipahbacrule.py

@@ -159,7 +159,7 @@ 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, gen_add_del_lists
+    module_params_get, gen_add_del_lists, gen_add_list, gen_intersection_list
 
 
 def find_hbacrule(module, name):
@@ -175,8 +175,8 @@ def find_hbacrule(module, name):
             msg="There is more than one hbacrule '%s'" % (name))
     elif len(_result["result"]) == 1:
         return _result["result"][0]
-    else:
-        return None
+
+    return None
 
 
 def gen_args(description, usercategory, hostcategory, servicecategory,
@@ -340,6 +340,22 @@ def main():
                 if action == "hbacrule":
                     # Found the hbacrule
                     if res_find is not None:
+                        # Remove usercategory, hostcategory and
+                        # servicecategory from args if "" and category
+                        # not in res_find (needed for idempotency)
+                        if "usercategory" in args and \
+                           args["usercategory"] == "" and \
+                           "usercategory" not in res_find:
+                            del args["usercategory"]
+                        if "hostcategory" in args and \
+                           args["hostcategory"] == "" and \
+                           "hostcategory" not in res_find:
+                            del args["hostcategory"]
+                        if "servicecategory" in args and \
+                           args["servicecategory"] == "" and \
+                           "servicecategory" not in res_find:
+                            del args["servicecategory"]
+
                         # For all settings is args, check if there are
                         # different settings in the find result.
                         # If yes: modify
@@ -420,6 +436,18 @@ def main():
                     if res_find is None:
                         ansible_module.fail_json(msg="No hbacrule '%s'" % name)
 
+                    # Generate add lists for host, hostgroup and
+                    # res_find to only try to add hosts and hostgroups
+                    # that not in hbacrule already
+                    if host is not None and \
+                       "memberhost_host" in res_find:
+                        host = gen_add_list(
+                            host, res_find["memberhost_host"])
+                    if hostgroup is not None and \
+                       "memberhost_hostgroup" in res_find:
+                        hostgroup = gen_add_list(
+                            hostgroup, res_find["memberhost_hostgroup"])
+
                     # Add hosts and hostgroups
                     if host is not None or hostgroup is not None:
                         commands.append([name, "hbacrule_add_host",
@@ -428,6 +456,19 @@ def main():
                                              "hostgroup": hostgroup,
                                          }])
 
+                    # Generate add lists for hbacsvc, hbacsvcgroup and
+                    # res_find to only try to add hbacsvcs and hbacsvcgroups
+                    # that not in hbacrule already
+                    if hbacsvc is not None and \
+                       "memberservice_hbacsvc" in res_find:
+                        hbacsvc = gen_add_list(
+                            hbacsvc, res_find["memberservice_hbacsvc"])
+                    if hbacsvcgroup is not None and \
+                       "memberservice_hbacsvcgroup" in res_find:
+                        hbacsvcgroup = gen_add_list(
+                            hbacsvcgroup,
+                            res_find["memberservice_hbacsvcgroup"])
+
                     # Add hbacsvcs and hbacsvcgroups
                     if hbacsvc is not None or hbacsvcgroup is not None:
                         commands.append([name, "hbacrule_add_service",
@@ -436,6 +477,18 @@ def main():
                                              "hbacsvcgroup": hbacsvcgroup,
                                          }])
 
+                    # Generate add lists for user, group and
+                    # res_find to only try to add users and groups
+                    # that not in hbacrule already
+                    if user is not None and \
+                       "memberuser_user" in res_find:
+                        user = gen_add_list(
+                            user, res_find["memberuser_user"])
+                    if group is not None and \
+                       "memberuser_group" in res_find:
+                        group = gen_add_list(
+                            group, res_find["memberuser_group"])
+
                     # Add users and groups
                     if user is not None or group is not None:
                         commands.append([name, "hbacrule_add_user",
@@ -453,6 +506,22 @@ def main():
                     if res_find is None:
                         ansible_module.fail_json(msg="No hbacrule '%s'" % name)
 
+                    # Generate intersection lists for host, hostgroup and
+                    # res_find to only try to remove hosts and hostgroups
+                    # that are in hbacrule
+                    if host is not None:
+                        if "memberhost_host" in res_find:
+                            host = gen_intersection_list(
+                                host, res_find["memberhost_host"])
+                        else:
+                            host = None
+                    if hostgroup is not None:
+                        if "memberhost_hostgroup" in res_find:
+                            hostgroup = gen_intersection_list(
+                                hostgroup, res_find["memberhost_hostgroup"])
+                        else:
+                            hostgroup = None
+
                     # Remove hosts and hostgroups
                     if host is not None or hostgroup is not None:
                         commands.append([name, "hbacrule_remove_host",
@@ -461,6 +530,23 @@ def main():
                                              "hostgroup": hostgroup,
                                          }])
 
+                    # Generate intersection lists for hbacsvc, hbacsvcgroup
+                    # and res_find to only try to remove hbacsvcs and
+                    # hbacsvcgroups that are in hbacrule
+                    if hbacsvc is not None:
+                        if "memberservice_hbacsvc" in res_find:
+                            hbacsvc = gen_intersection_list(
+                                hbacsvc, res_find["memberservice_hbacsvc"])
+                        else:
+                            hbacsvc = None
+                    if hbacsvcgroup is not None:
+                        if "memberservice_hbacsvcgroup" in res_find:
+                            hbacsvcgroup = gen_intersection_list(
+                                hbacsvcgroup,
+                                res_find["memberservice_hbacsvcgroup"])
+                        else:
+                            hbacsvcgroup = None
+
                     # Remove hbacsvcs and hbacsvcgroups
                     if hbacsvc is not None or hbacsvcgroup is not None:
                         commands.append([name, "hbacrule_remove_service",
@@ -469,6 +555,22 @@ def main():
                                              "hbacsvcgroup": hbacsvcgroup,
                                          }])
 
+                    # Generate intersection lists for user, group and
+                    # res_find to only try to remove users and groups
+                    # that are in hbacrule
+                    if user is not None:
+                        if "memberuser_user" in res_find:
+                            user = gen_intersection_list(
+                                user, res_find["memberuser_user"])
+                        else:
+                            user = None
+                    if group is not None:
+                        if "memberuser_group" in res_find:
+                            group = gen_intersection_list(
+                                group, res_find["memberuser_group"])
+                        else:
+                            group = None
+
                     # Remove users and groups
                     if user is not None or group is not None:
                         commands.append([name, "hbacrule_remove_user",
@@ -500,6 +602,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 = []
@@ -516,16 +622,12 @@ def main():
                 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.
             if "failed" in result and len(result["failed"]) > 0:
                 for item in result["failed"]:
                     failed_item = result["failed"][item]
                     for member_type in failed_item:
                         for member, failure in failed_item[member_type]:
-                            if "already a member" in failure \
-                               or "not a member" in failure:
-                                continue
                             errors.append("%s: %s %s: %s" % (
                                 command, member_type, member, failure))
         if len(errors) > 0:

plugins/modules/ipahbacsvc.py

@@ -89,8 +89,8 @@ def find_hbacsvc(module, name):
             msg="There is more than one hbacsvc '%s'" % (name))
     elif len(_result["result"]) == 1:
         return _result["result"][0]
-    else:
-        return None
+
+    return None
 
 
 def gen_args(description):
@@ -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

@@ -121,8 +121,8 @@ def find_hbacsvcgroup(module, name):
             msg="There is more than one hbacsvcgroup '%s'" % (name))
     elif len(_result["result"]) == 1:
         return _result["result"][0]
-    else:
-        return None
+
+    return None
 
 
 def gen_args(description, nomembers):
@@ -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"]
 
@@ -466,7 +466,7 @@ def show_host(module, name):
 
 def gen_args(description, locality, location, platform, os, password, random,
              mac_address, sshpubkey, userclass, auth_ind, requires_pre_auth,
-             ok_as_delegate, ok_to_auth_as_delegate, force, reverse,
+             ok_as_delegate, ok_to_auth_as_delegate, force, _reverse,
              ip_address, update_dns):
     # certificate, managedby_host, principal, create_keytab_* and
     # allow_retrieve_keytab_* are not handled here
@@ -529,7 +529,7 @@ def gen_dnsrecord_args(module, ip_address, reverse):
     return _args
 
 
-def check_parameters(
+def check_parameters(   # pylint: disable=unused-argument
         module, state, action,
         description, locality, location, platform, os, password, random,
         certificate, managedby_host, principal, allow_create_keytab_user,
@@ -862,7 +862,7 @@ def main():
                     ok_to_auth_as_delegate, force, reverse, ip_address,
                     update_dns, update_password)
 
-            elif isinstance(host, str) or isinstance(host, unicode):
+            elif isinstance(host, (str, unicode)):
                 name = host
             else:
                 ansible_module.fail_json(msg="Host '%s' is not valid" %
@@ -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
@@ -1324,6 +1327,23 @@ def main():
 
                     dnsrecord_args = gen_dnsrecord_args(ansible_module,
                                                         ip_address, reverse)
+
+                    # Remove arecord and aaaarecord from dnsrecord_args
+                    # if the record does not exits in res_find_dnsrecord
+                    # to prevent "DNS resource record not found" error
+                    if "arecord" in dnsrecord_args \
+                       and dnsrecord_args["arecord"] is not None \
+                       and len(dnsrecord_args["arecord"]) > 0 \
+                       and (res_find_dnsrecord is None
+                            or "arecord" not in res_find_dnsrecord):
+                        del dnsrecord_args["arecord"]
+                    if "aaaarecord" in dnsrecord_args \
+                       and dnsrecord_args["aaaarecord"] is not None \
+                       and len(dnsrecord_args["aaaarecord"]) > 0 \
+                       and (res_find_dnsrecord is None
+                            or "aaaarecord" not in res_find_dnsrecord):
+                        del dnsrecord_args["aaaarecord"]
+
                     if "arecord" in dnsrecord_args or \
                        "aaaarecord" in dnsrecord_args:
                         domain_name = name[name.find(".")+1:]
@@ -1344,6 +1364,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

@@ -141,7 +141,8 @@ 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, gen_add_del_lists, api_check_command, api_check_param
+    module_params_get, gen_add_del_lists, api_check_command, api_check_param, \
+    gen_add_list, gen_intersection_list
 
 
 def find_hostgroup(module, name):
@@ -157,8 +158,8 @@ def find_hostgroup(module, name):
             msg="There is more than one hostgroup '%s'" % (name))
     elif len(_result["result"]) == 1:
         return _result["result"][0]
-    else:
-        return None
+
+    return None
 
 
 def gen_args(description, nomembers, rename):
@@ -396,6 +397,15 @@ def main():
                         ansible_module.fail_json(
                             msg="No hostgroup '%s'" % name)
 
+                    # Reduce add lists for member_host and member_hostgroup,
+                    # to new entries only that are not in res_find.
+                    if host is not None and "member_host" in res_find:
+                        host = gen_add_list(host, res_find["member_host"])
+                    if hostgroup is not None \
+                       and "member_hostgroup" in res_find:
+                        hostgroup = gen_add_list(
+                            hostgroup, res_find["member_hostgroup"])
+
                     # Ensure members are present
                     commands.append([name, "hostgroup_add_member",
                                      {
@@ -404,6 +414,20 @@ def main():
                                      }])
 
                     if has_add_membermanager:
+                        # Reduce add list for membermanager_user and
+                        # membermanager_group to new entries only that are
+                        # not in res_find.
+                        if membermanager_user is not None \
+                           and "membermanager_user" in res_find:
+                            membermanager_user = gen_add_list(
+                                membermanager_user,
+                                res_find["membermanager_user"])
+                        if membermanager_group is not None \
+                           and "membermanager_group" in res_find:
+                            membermanager_group = gen_add_list(
+                                membermanager_group,
+                                res_find["membermanager_group"])
+
                         # Add membermanager users and groups
                         if membermanager_user is not None or \
                            membermanager_group is not None:
@@ -441,6 +465,15 @@ def main():
                         ansible_module.fail_json(
                             msg="No hostgroup '%s'" % name)
 
+                    # Reduce del lists of member_host and member_hostgroup,
+                    # to the entries only that are in res_find.
+                    if host is not None:
+                        host = gen_intersection_list(
+                            host, res_find.get("member_host"))
+                    if hostgroup is not None:
+                        hostgroup = gen_intersection_list(
+                            hostgroup, res_find.get("member_hostgroup"))
+
                     # Ensure members are absent
                     commands.append([name, "hostgroup_remove_member",
                                      {
@@ -449,6 +482,18 @@ def main():
                                      }])
 
                     if has_add_membermanager:
+                        # Reduce del lists of membermanager_user and
+                        # membermanager_group to the entries only that are
+                        # in res_find.
+                        if membermanager_user is not None:
+                            membermanager_user = gen_intersection_list(
+                                membermanager_user,
+                                res_find.get("membermanager_user"))
+                        if membermanager_group is not None:
+                            membermanager_group = gen_intersection_list(
+                                membermanager_group,
+                                res_find.get("membermanager_group"))
+
                         # Remove membermanager users and groups
                         if membermanager_user is not None or \
                            membermanager_group is not None:
@@ -463,6 +508,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:
@@ -483,9 +532,6 @@ def main():
                 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:

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

@@ -234,14 +234,22 @@ def main():
                 if action == "privilege":
                     # Found the privilege
                     if res_find is not None:
+                        res_cmp = {
+                            k: v for k, v in res_find.items()
+                            if k not in [
+                                "objectclass", "cn", "dn",
+                                "memberof_permisssion"
+                            ]
+                        }
                         # 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):
+                        if args and not compare_args_ipa(ansible_module, args,
+                                                         res_cmp):
                             commands.append([name, "privilege_mod", args])
                     else:
                         commands.append([name, "privilege_add", args])
+                        res_find = {}
 
                     member_args = {}
                     if permission:
@@ -312,6 +320,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

@@ -130,8 +130,8 @@ def find_pwpolicy(module, name):
             msg="There is more than one pwpolicy '%s'" % (name))
     elif len(_result["result"]) == 1:
         return _result["result"][0]
-    else:
-        return None
+
+    return None
 
 
 def gen_args(maxlife, minlife, history, minclasses, minlength, priority,
@@ -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

@@ -45,10 +45,10 @@ options:
     required: true
     aliases: ["cn"]
   description:
-    descrpition: A description for the role.
+    description: A description for the role.
     required: false
   rename:
-    descrpition: Rename the role object.
+    description: Rename the role object.
     required: false
   user:
     description: List of users.
@@ -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/ipaserver.py

@@ -0,0 +1,440 @@
+#!/usr/bin/python
+# -*- coding: utf-8 -*-
+
+# Authors:
+#   Thomas Woerner <twoerner@redhat.com>
+#
+# Copyright (C) 2021 Red Hat
+# see file 'COPYING' for use and warranty information
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+ANSIBLE_METADATA = {
+    "metadata_version": "1.0",
+    "supported_by": "community",
+    "status": ["preview"],
+}
+
+DOCUMENTATION = """
+---
+module: ipaserver
+short description: Manage FreeIPA server
+description: Manage FreeIPA server
+options:
+  ipaadmin_principal:
+    description: The admin principal.
+    default: admin
+  ipaadmin_password:
+    description: The admin password.
+    required: false
+  name:
+    description: The list of server name strings.
+    required: true
+    aliases: ["cn"]
+  location:
+    description: |
+      The server location string.
+      "" for location reset.
+      Only in state: present.
+    required: false
+    aliases: ["ipalocation_location"]
+  service_weight:
+    description: |
+      Weight for server services
+      Values 0 to 65535, -1 for weight reset.
+      Only in state: present.
+    required: false
+    type: int
+    aliases: ["ipaserviceweight"]
+  hidden:
+    description: |
+      Set hidden state of a server.
+      Only in state: present.
+    required: false
+    type: bool
+  no_members:
+    description: |
+      Suppress processing of membership attributes
+      Only in state: present.
+    required: false
+    type: bool
+  delete_continue:
+    description: |
+      Continuous mode: Don't stop on errors.
+      Only in state: absent.
+    required: false
+    type: bool
+    aliases: ["continue"]
+  ignore_last_of_role:
+    description: |
+      Skip a check whether the last CA master or DNS server is removed.
+      Only in state: absent.
+    required: false
+    type: bool
+  ignore_topology_disconnect:
+    description: |
+      Ignore topology connectivity problems after removal.
+      Only in state: absent.
+    required: false
+    type: bool
+  force:
+    description: |
+      Force server removal even if it does not exist.
+      Will always result in changed.
+      Only in state: absent.
+    required: false
+    type: bool
+  state:
+    description: The state to ensure.
+    choices: ["present", "absent"]
+    default: present
+    required: true
+"""
+
+EXAMPLES = """
+# Ensure server server.example.com is present
+- ipaserver:
+    ipaadmin_password: SomeADMINpassword
+    name: server.example.com
+
+# Ensure server server.example.com is absent
+- ipaserver:
+    ipaadmin_password: SomeADMINpassword
+    name: server.example.com
+    state: absent
+
+# Ensure server server.example.com is present with location mylocation
+- ipaserver:
+    ipaadmin_password: SomeADMINpassword
+    name: server.example.com
+    location: mylocation
+
+# Ensure server server.example.com is present without a location
+- ipaserver:
+    ipaadmin_password: SomeADMINpassword
+    name: server.example.com
+    location: ""
+
+# Ensure server server.example.com is present with service weight 1
+- ipaserver:
+    ipaadmin_password: SomeADMINpassword
+    name: server.example.com
+    service_weight: 1
+
+# Ensure server server.example.com is present without service weight
+- ipaserver:
+    ipaadmin_password: SomeADMINpassword
+    name: server.example.com
+    service_weight: -1
+
+# Ensure server server.example.com is present and hidden
+- ipaserver:
+    ipaadmin_password: SomeADMINpassword
+    name: server.example.com
+    hidden: yes
+
+# Ensure server server.example.com is present and not hidden
+- ipaserver:
+    ipaadmin_password: SomeADMINpassword
+    name: server.example.com
+    hidden: no
+
+# Ensure server server.example.com is absent in continuous mode in error case
+- ipaserver:
+    ipaadmin_password: SomeADMINpassword
+    name: server.example.com
+    continue: yes
+    state: absent
+
+# Ensure server server.example.com is absent with last of role check skip
+- ipaserver:
+    ipaadmin_password: SomeADMINpassword
+    name: server.example.com
+    ignore_last_of_role: yes
+    state: absent
+
+# Ensure server server.example.com is absent with topology disconnect check
+# skip
+- ipaserver:
+    ipaadmin_password: SomeADMINpassword
+    name: server.example.com
+    ignore_topology_disconnect: yes
+    state: absent
+
+# Ensure server server.example.com is absent in force mode
+- ipaserver:
+    ipaadmin_password: SomeADMINpassword
+    name: server.example.com
+    force: yes
+    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, \
+    api_command_no_name, compare_args_ipa, module_params_get, DNSName
+import six
+
+if six.PY3:
+    unicode = str
+
+
+def find_server(module, name):
+    """Find if a server with the given name already exist."""
+    try:
+        _result = api_command(module, "server_show", name, {"all": True})
+    except Exception:  # pylint: disable=broad-except
+        # An exception is raised if server name is not found.
+        return None
+    else:
+        return _result["result"]
+
+
+def server_role_status(module, name):
+    """Get server role of a hidden server with the given name."""
+    try:
+        _result = api_command_no_name(module, "server_role_find",
+                                      {"server_server": name,
+                                       "role_servrole": 'IPA master',
+                                       "include_master": True,
+                                       "raw": True,
+                                       "all": True})
+    except Exception:  # pylint: disable=broad-except
+        # An exception is raised if server name is not found.
+        return None
+    else:
+        return _result["result"][0]
+
+
+def gen_args(location, service_weight, no_members, delete_continue,
+             ignore_topology_disconnect, ignore_last_of_role, force):
+    _args = {}
+    if location is not None:
+        if location != "":
+            _args["ipalocation_location"] = DNSName(location)
+        else:
+            _args["ipalocation_location"] = None
+    if service_weight is not None:
+        _args["ipaserviceweight"] = service_weight
+    if no_members is not None:
+        _args["no_members"] = no_members
+    if delete_continue is not None:
+        _args["continue"] = delete_continue
+    if ignore_topology_disconnect is not None:
+        _args["ignore_topology_disconnect"] = ignore_topology_disconnect
+    if ignore_last_of_role is not None:
+        _args["ignore_last_of_role"] = ignore_last_of_role
+    if force is not None:
+        _args["force"] = force
+
+    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
+            location=dict(required=False, type='str',
+                          aliases=["ipalocation_location"], default=None),
+            service_weight=dict(required=False, type='int',
+                                aliases=["ipaserviceweight"], default=None),
+            hidden=dict(required=False, type='bool', default=None),
+            no_members=dict(required=False, type='bool', default=None),
+            # absent
+            delete_continue=dict(required=False, type='bool',
+                                 aliases=["continue"], default=None),
+            ignore_topology_disconnect=dict(required=False, type='bool',
+                                            default=None),
+            ignore_last_of_role=dict(required=False, type='bool',
+                                     default=None),
+            force=dict(required=False, type='bool',
+                       default=None),
+            # state
+            state=dict(type="str", default="present",
+                       choices=["present", "absent"]),
+        ),
+        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
+    location = module_params_get(ansible_module, "location")
+    service_weight = module_params_get(ansible_module, "service_weight")
+    # Service weight smaller than 0 leads to resetting service weight
+    if service_weight is not None and \
+       (service_weight < -1 or service_weight > 65535):
+        ansible_module.fail_json(
+            msg="service_weight %d is out of range [-1 .. 65535]" %
+            service_weight)
+    if service_weight == -1:
+        service_weight = ""
+    hidden = module_params_get(ansible_module, "hidden")
+    no_members = module_params_get(ansible_module, "no_members")
+
+    # absent
+    delete_continue = module_params_get(ansible_module, "delete_continue")
+    ignore_topology_disconnect = module_params_get(
+        ansible_module, "ignore_topology_disconnect")
+    ignore_last_of_role = module_params_get(ansible_module,
+                                            "ignore_last_of_role")
+    force = module_params_get(ansible_module, "force")
+
+    # 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 server can be ensured at a time.")
+        invalid = ["delete_continue", "ignore_topology_disconnect",
+                   "ignore_last_of_role", "force"]
+
+    if state == "absent":
+        if len(names) < 1:
+            ansible_module.fail_json(msg="No name given.")
+        invalid = ["location", "service_weight", "hidden", "no_members"]
+
+    for x in invalid:
+        if vars()[x] is not None:
+            ansible_module.fail_json(
+                msg="Argument '%s' can not be used with state '%s'" %
+                (x, state))
+
+    # 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 server exists
+            res_find = find_server(ansible_module, name)
+
+            # Generate args
+            args = gen_args(location, service_weight, no_members,
+                            delete_continue, ignore_topology_disconnect,
+                            ignore_last_of_role, force)
+
+            # Create command
+            if state == "present":
+                # Server not found
+                if res_find is None:
+                    ansible_module.fail_json(
+                        msg="Server '%s' not found" % name)
+
+                # Remove location from args if "" (transformed to None)
+                # and "ipalocation_location" not in res_find for idempotency
+                if "ipalocation_location" in args and \
+                   args["ipalocation_location"] is None and \
+                   "ipalocation_location" not in res_find:
+                    del args["ipalocation_location"]
+
+                # Remove service weight from args if ""
+                # and "ipaserviceweight" not in res_find for idempotency
+                if "ipaserviceweight" in args and \
+                   args["ipaserviceweight"] == "" and \
+                   "ipaserviceweight" not in res_find:
+                    del args["ipaserviceweight"]
+
+                # 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, "server_mod", args])
+
+                # hidden handling
+                if hidden is not None:
+                    res_role_status = server_role_status(ansible_module,
+                                                         name)
+
+                    if "status" in res_role_status:
+                        # Fail if status is configured, it should be done
+                        # only in the installer
+                        if res_role_status["status"] == "configured":
+                            ansible_module.fail_json(
+                                msg="'%s' in configured state, "
+                                "unable to change state" % state)
+
+                        if hidden and res_role_status["status"] == "enabled":
+                            commands.append([name, "server_state",
+                                             {"state": "hidden"}])
+                        if not hidden and \
+                           res_role_status["status"] == "hidden":
+                            commands.append([name, "server_state",
+                                             {"state": "enabled"}])
+
+            elif state == "absent":
+                if res_find is not None or force:
+                    commands.append([name, "server_del", args])
+            else:
+                ansible_module.fail_json(msg="Unkown state '%s'" % state)
+
+        # 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)))
+
+    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/ipaservice.py

@@ -47,7 +47,7 @@ options:
     description: Base-64 encoded service certificate.
     required: false
     type: list
-    aliases=['usercertificate']
+    aliases: ["usercertificate"]
   pac_type:
     description: Supported PAC type.
     required: false
@@ -79,19 +79,19 @@ options:
     type: bool
     default: False
     aliases: ["ipakrbokasdelegate"]
-  ok_to_auth_as_delegate: Allow service to authenticate on behalf of a client.
-    description: .
+  ok_to_auth_as_delegate:
+    description: Allow service to authenticate on behalf of a client.
     required: false
     type: bool
     default: False
-    aliases:["ipakrboktoauthasdelegate"]
+    aliases: ["ipakrboktoauthasdelegate"]
   principal:
     description: List of principal aliases for the service.
     required: false
     type: list
     aliases: ["krbprincipalname"]
   smb:
-    description: Add a SMB service. Can only be used with new services.
+    description: Add a SMB service.
     required: false
     type: bool
   netbiosname:
@@ -104,42 +104,42 @@ options:
     type: list
     aliases: ["managedby_host"]
   allow_create_keytab_user:
-    descrption: Users allowed to create a keytab of this host.
+    description: Users allowed to create a keytab of this host.
     required: false
     type: list
     aliases: ["ipaallowedtoperform_write_keys_user"]
   allow_create_keytab_group:
-    descrption: Groups allowed to create a keytab of this host.
+    description: Groups allowed to create a keytab of this host.
     required: false
     type: list
     aliases: ["ipaallowedtoperform_write_keys_group"]
   allow_create_keytab_host:
-    descrption: Hosts allowed to create a keytab of this host.
+    description: Hosts allowed to create a keytab of this host.
     required: false
     type: list
     aliases: ["ipaallowedtoperform_write_keys_host"]
   allow_create_keytab_hostgroup:
-    descrption: Host group allowed to create a keytab of this host.
+    description: Host group allowed to create a keytab of this host.
     required: false
     type: list
     aliases: ["ipaallowedtoperform_write_keys_hostgroup"]
   allow_retrieve_keytab_user:
-    descrption: User allowed to retrieve a keytab of this host.
+    description: User allowed to retrieve a keytab of this host.
     required: false
     type: list
     aliases: ["ipaallowedtoperform_read_keys_user"]
   allow_retrieve_keytab_group:
-    descrption: Groups allowed to retrieve a keytab of this host.
+    description: Groups allowed to retrieve a keytab of this host.
     required: false
     type: list
     aliases: ["ipaallowedtoperform_read_keys_group"]
   allow_retrieve_keytab_host:
-    descrption: Hosts allowed to retrieve a keytab of this host.
+    description: Hosts allowed to retrieve a keytab of this host.
     required: false
     type: list
     aliases: ["ipaallowedtoperform_read_keys_host"]
   allow_retrieve_keytab_hostgroup:
-    descrption: Host groups allowed to retrieve a keytab of this host.
+    description: Host groups allowed to retrieve a keytab of this host.
     required: false
     type: list
     aliases: ["ipaallowedtoperform_read_keys_hostgroup"]
@@ -230,28 +230,17 @@ 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, \
     encode_certificate, gen_add_del_lists, module_params_get, to_text, \
-    api_check_param
-import ipalib.errors
+    api_check_param, ipalib_errors
 
 
-def find_service(module, name, netbiosname):
+def find_service(module, name):
     _args = {
         "all": True,
     }
 
-    # Search for a SMB/cifs service.
-    if netbiosname is not None:
-        _result = api_command(
-            module, "service_find", to_text(netbiosname), _args)
-
-        for _res_find in _result.get('result', []):
-            for uid in _res_find.get('uid', []):
-                if uid.startswith("%s$@" % netbiosname):
-                    return _res_find
-
     try:
         _result = api_command(module, "service_show", to_text(name), _args)
-    except ipalib.errors.NotFound:
+    except ipalib_errors.NotFound:
         return None
 
     if "result" in _result:
@@ -261,8 +250,8 @@ def find_service(module, name, netbiosname):
             _res["usercertificate"] = [encode_certificate(cert) for
                                        cert in certs]
         return _res
-    else:
-        return None
+
+    return None
 
 
 def gen_args(pac_type, auth_ind, skip_host_check, force, requires_pre_auth,
@@ -287,6 +276,19 @@ def gen_args(pac_type, auth_ind, skip_host_check, force, requires_pre_auth,
     return _args
 
 
+def gen_args_smb(netbiosname, ok_as_delegate, ok_to_auth_as_delegate):
+    _args = {}
+
+    if netbiosname is not None:
+        _args['ipantflatname'] = netbiosname
+    if ok_as_delegate is not None:
+        _args['ipakrbokasdelegate'] = (ok_as_delegate)
+    if ok_to_auth_as_delegate is not None:
+        _args['ipakrboktoauthasdelegate'] = (ok_to_auth_as_delegate)
+
+    return _args
+
+
 def check_parameters(module, state, action, names, parameters):
     assert isinstance(parameters, dict)
 
@@ -310,15 +312,13 @@ def check_parameters(module, state, action, names, parameters):
         if action == 'service':
             invalid = ['delete_continue']
 
-            if parameters.get('smb', False):
-                invalid.extend(['force', 'auth_ind', 'skip_host_check',
-                                'requires_pre_auth', 'auth_ind', 'pac_type'])
-
-                for _invalid in invalid:
-                    if parameters.get(_invalid, False):
-                        module.fail_json(
-                            msg="Argument '%s' can not be used with SMB "
-                                "service." % _invalid)
+            if (
+                not parameters.get('smb', False)
+                and parameters.get('netbiosname')
+            ):
+                module.fail_json(
+                    msg="Argument 'netbiosname' can not be used without "
+                        "SMB service.")
         else:
             invalid.append('delete_continue')
 
@@ -494,11 +494,9 @@ def main():
         commands = []
 
         for name in names:
-            res_find = find_service(ansible_module, name, netbiosname)
+            res_find = find_service(ansible_module, name)
 
             if state == "present":
-                # if service exists, 'smb' cannot be used.
-
                 if action == "service":
                     args = gen_args(
                         pac_type, auth_ind, skip_host_check, force,
@@ -507,13 +505,24 @@ def main():
                     if not has_skip_host_check and 'skip_host_check' in args:
                         del args['skip_host_check']
 
+                    if smb:
+                        if res_find is None:
+                            _name = "cifs/" + name
+                            res_find = find_service(ansible_module, _name)
+                            if res_find is None:
+                                _args = gen_args_smb(
+                                    netbiosname, ok_as_delegate,
+                                    ok_to_auth_as_delegate)
+                                commands.append(
+                                    [name, 'service_add_smb', _args])
+                                res_find = {}
+                            # service_add_smb will prefix 'name' with
+                            # "cifs/", so we will need to change it here,
+                            # so that service_mod, if called later, works.
+                            name = _name
+
                     if res_find is None:
-                        if smb:
-                            if netbiosname is not None:
-                                args['ipantflatname'] = netbiosname
-                            commands.append([name, 'service_add_smb', args])
-                        else:
-                            commands.append([name, 'service_add', args])
+                        commands.append([name, 'service_add', args])
 
                         certificate_add = certificate or []
                         certificate_del = []
@@ -551,6 +560,15 @@ def main():
                             if remove in args:
                                 del args[remove]
 
+                        if (
+                            "krbprincipalauthind" in args
+                            and (
+                                args.get("krbprincipalauthind", [""]) ==
+                                res_find.get("krbprincipalauthind", [""])
+                            )
+                          ):
+                            del args["krbprincipalauthind"]
+
                         if not compare_args_ipa(ansible_module, args,
                                                 res_find):
                             commands.append([name, "service_mod", args])
@@ -753,7 +771,7 @@ def main():
             elif state == "absent":
                 if action == "service":
                     if res_find is not None:
-                        args = {'continue': True if delete_continue else False}
+                        args = {'continue': delete_continue}
                         commands.append([name, 'service_del', args])
 
                 elif action == "member":
@@ -824,6 +842,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

@@ -56,15 +56,15 @@ author:
 
 EXAMPLES = """
 # Ensure sudocmd is present
-- ipacommand:
+- ipasudocmd:
     ipaadmin_password: SomeADMINpassword
-    name: su
+    name: /usr/bin/su
     state: present
 
 # Ensure sudocmd is absent
-- ipacommand:
+- ipasudocmd:
     ipaadmin_password: SomeADMINpassword
-    name: su
+    name: /usr/bin/su
     state: absent
 """
 
@@ -90,8 +90,8 @@ def find_sudocmd(module, name):
             msg="There is more than one sudocmd '%s'" % (name))
     elif len(_result["result"]) == 1:
         return _result["result"][0]
-    else:
-        return None
+
+    return None
 
 
 def gen_args(description):
@@ -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
@@ -111,24 +107,18 @@ 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, compare_args_ipa, \
-    gen_add_del_lists
+    gen_add_del_lists, ipalib_errors
 
 
 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 +131,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 +151,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 +173,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 +246,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 +296,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

@@ -53,7 +53,7 @@ options:
     required: false
     choices: ["all", ""]
     aliases: ["usercat"]
-  usergroup:
+  group:
     description: List of user groups assigned to the sudo rule.
     required: false
   runasgroupcategory:
@@ -109,7 +109,7 @@ options:
     required: false
     type: int
   sudooption:
-    description:
+    description: List of sudo options.
     required: false
     type: list
     aliases: ["options"]
@@ -206,8 +206,8 @@ def find_sudorule(module, name):
             msg="There is more than one sudorule '%s'" % (name))
     elif len(_result["result"]) == 1:
         return _result["result"][0]
-    else:
-        return None
+
+    return None
 
 
 def gen_args(description, usercat, hostcat, cmdcat, runasusercat,
@@ -416,6 +416,32 @@ def main():
                 if action == "sudorule":
                     # Found the sudorule
                     if res_find is not None:
+                        # Remove empty usercategory, hostcategory,
+                        # cmdcaterory, runasusercategory and hostcategory
+                        # from args if "" and if the category is not in the
+                        # sudorule. The empty string is used to reset the
+                        # category.
+                        if "usercategory" in args \
+                           and args["usercategory"] == "" \
+                           and "usercategory" not in res_find:
+                            del args["usercategory"]
+                        if "hostcategory" in args \
+                           and args["hostcategory"] == "" \
+                           and "hostcategory" not in res_find:
+                            del args["hostcategory"]
+                        if "cmdcategory" in args \
+                           and args["cmdcategory"] == "" \
+                           and "cmdcategory" not in res_find:
+                            del args["cmdcategory"]
+                        if "ipasudorunasusercategory" in args \
+                           and args["ipasudorunasusercategory"] == "" \
+                           and "ipasudorunasusercategory" not in res_find:
+                            del args["ipasudorunasusercategory"]
+                        if "ipasudorunasgroupcategory" in args \
+                           and args["ipasudorunasgroupcategory"] == "" \
+                           and "ipasudorunasgroupcategory" not in res_find:
+                            del args["ipasudorunasgroupcategory"]
+
                         # For all settings is args, check if there are
                         # different settings in the find result.
                         # If yes: modify
@@ -429,16 +455,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 +712,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

@@ -59,7 +59,7 @@ options:
   state:
     description: State to ensure
     default: present
-    choices: ["present", "absent", "enabled", "disabled", "reinitialized"
+    choices: ["present", "absent", "enabled", "disabled", "reinitialized",
               "checked" ]
 author:
     - Thomas Woerner
@@ -132,8 +132,8 @@ def find_left_right(module, suffix, left, right):
             "not unique for suffix '%s'" % (left, right, suffix))
     elif len(_result["result"]) == 1:
         return _result["result"][0]
-    else:
-        return None
+
+    return None
 
 
 def find_cn(module, suffix, name):
@@ -147,8 +147,8 @@ def find_cn(module, suffix, name):
             msg="CN '%s' is not unique for suffix '%s'" % (name, suffix))
     elif len(_result["result"]) == 1:
         return _result["result"][0]
-    else:
-        return None
+
+    return None
 
 
 def find_left_right_cn(module, suffix, left, right, name):
@@ -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

@@ -125,8 +125,8 @@ def find_trust(module, realm):
         module.fail_json(msg="There is more than one realm '%s'" % (realm))
     elif len(_result["result"]) == 1:
         return _result["result"][0]
-    else:
-        return None
+
+    return None
 
 
 def del_trust(module, realm):
@@ -136,8 +136,6 @@ def del_trust(module, realm):
     if len(_result["result"]["failed"]) > 0:
         module.fail_json(
             msg="Trust deletion has failed for '%s'" % (realm))
-    else:
-        return None
 
 
 def add_trust(module, realm, args):
@@ -148,12 +146,10 @@ def add_trust(module, realm, args):
     if "cn" not in _result["result"]:
         module.fail_json(
             msg="Trust add has failed for '%s'" % (realm))
-    else:
-        return None
 
 
 def gen_args(trust_type, admin, password, server, trust_secret, base_id,
-             range_size, range_type, two_way, external):
+             range_size, _range_type, two_way, external):
     _args = {}
     if trust_type is not None:
         _args["trust_type"] = trust_type
@@ -244,7 +240,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 +253,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

@@ -80,20 +80,20 @@ options:
         required: false
         aliases: ["principalname", "krbprincipalname"]
       principalexpiration:
-        description:
-        - The kerberos principal expiration date
-        - (possible formats: YYYYMMddHHmmssZ, YYYY-MM-ddTHH:mm:ssZ,
-        - YYYY-MM-ddTHH:mmZ, YYYY-MM-ddZ, YYYY-MM-dd HH:mm:ssZ,
-        - YYYY-MM-dd HH:mmZ) The trailing 'Z' can be skipped.
+        description: |
+          The kerberos principal expiration date
+          (possible formats: YYYYMMddHHmmssZ, YYYY-MM-ddTHH:mm:ssZ,
+          YYYY-MM-ddTHH:mmZ, YYYY-MM-ddZ, YYYY-MM-dd HH:mm:ssZ,
+          YYYY-MM-dd HH:mmZ) The trailing 'Z' can be skipped.
         required: false
         aliases: ["krbprincipalexpiration"]
       passwordexpiration:
-        description:
-        - The kerberos password expiration date (FreeIPA-4.7+)
-        - (possible formats: YYYYMMddHHmmssZ, YYYY-MM-ddTHH:mm:ssZ,
-        - YYYY-MM-ddTHH:mmZ, YYYY-MM-ddZ, YYYY-MM-dd HH:mm:ssZ,
-        - YYYY-MM-dd HH:mmZ) The trailing 'Z' can be skipped.
-        - Only usable with IPA versions 4.7 and up.
+        description: |
+          The kerberos password expiration date (FreeIPA-4.7+)
+          (possible formats: YYYYMMddHHmmssZ, YYYY-MM-ddTHH:mm:ssZ,
+          YYYY-MM-ddTHH:mmZ, YYYY-MM-ddZ, YYYY-MM-dd HH:mm:ssZ,
+          YYYY-MM-dd HH:mmZ) The trailing 'Z' can be skipped.
+          Only usable with IPA versions 4.7 and up.
         required: false
         aliases: ["krbpasswordexpiration"]
       password:
@@ -156,7 +156,7 @@ options:
         description:
           List of supported user authentication types
           Use empty string to reset userauthtype to the initial value.
-        choices=['password', 'radius', 'otp', '']
+        choices: ['password', 'radius', 'otp', '']
         required: false
         aliases: ["ipauserauthtype"]
       userclass:
@@ -245,20 +245,20 @@ options:
     required: false
     aliases: ["principalname", "krbprincipalname"]
   principalexpiration:
-    description:
-    - The kerberos principal expiration date
-    - (possible formats: YYYYMMddHHmmssZ, YYYY-MM-ddTHH:mm:ssZ,
-    - YYYY-MM-ddTHH:mmZ, YYYY-MM-ddZ, YYYY-MM-dd HH:mm:ssZ,
-    - YYYY-MM-dd HH:mmZ) The trailing 'Z' can be skipped.
+    description: |
+      The kerberos principal expiration date
+      (possible formats: YYYYMMddHHmmssZ, YYYY-MM-ddTHH:mm:ssZ,
+      YYYY-MM-ddTHH:mmZ, YYYY-MM-ddZ, YYYY-MM-dd HH:mm:ssZ,
+      YYYY-MM-dd HH:mmZ) The trailing 'Z' can be skipped.
     required: false
     aliases: ["krbprincipalexpiration"]
   passwordexpiration:
-    description:
-    - The kerberos password expiration date (FreeIPA-4.7+)
-    - (possible formats: YYYYMMddHHmmssZ, YYYY-MM-ddTHH:mm:ssZ,
-    - YYYY-MM-ddTHH:mmZ, YYYY-MM-ddZ, YYYY-MM-dd HH:mm:ssZ,
-    - YYYY-MM-dd HH:mmZ) The trailing 'Z' can be skipped.
-    - Only usable with IPA versions 4.7 and up.
+    description: |
+      The kerberos password expiration date (FreeIPA-4.7+)
+      (possible formats: YYYYMMddHHmmssZ, YYYY-MM-ddTHH:mm:ssZ,
+      YYYY-MM-ddTHH:mmZ, YYYY-MM-ddZ, YYYY-MM-dd HH:mm:ssZ,
+      YYYY-MM-dd HH:mmZ) The trailing 'Z' can be skipped.
+      Only usable with IPA versions 4.7 and up.
     required: false
     aliases: ["krbpasswordexpiration"]
   password:
@@ -321,7 +321,7 @@ options:
     description:
       List of supported user authentication types
       Use empty string to reset userauthtype to the initial value.
-    choices=['password', 'radius', 'otp', '']
+    choices: ['password', 'radius', 'otp', '']
     required: false
     aliases: ["ipauserauthtype"]
   userclass:
@@ -512,10 +512,9 @@ def find_user(module, name, preserved=False):
         if certs is not None:
             _result["usercertificate"] = [encode_certificate(x)
                                           for x in certs]
-
         return _result
-    else:
-        return None
+
+    return None
 
 
 def gen_args(first, last, fullname, displayname, initials, homedir, shell,
@@ -599,17 +598,14 @@ def gen_args(first, last, fullname, displayname, initials, homedir, shell,
     return _args
 
 
-def check_parameters(module, state, action,
-                     first, last, fullname, displayname, initials, homedir,
-                     shell, email, principal, principalexpiration,
-                     passwordexpiration, password, random, uid, gid, city,
-                     phone, mobile, pager, fax, orgunit, title, manager,
-                     carlicense, sshpubkey, userauthtype, userclass, radius,
-                     radiususer, departmentnumber, employeenumber,
-                     employeetype, preferredlanguage, certificate,
-                     certmapdata, noprivate, nomembers, preserve,
-                     update_password):
-
+def check_parameters(  # pylint: disable=unused-argument
+        module, state, action, first, last, fullname, displayname, initials,
+        homedir, shell, email, principal, principalexpiration,
+        passwordexpiration, password, random, uid, gid, city, phone, mobile,
+        pager, fax, orgunit, title, manager, carlicense, sshpubkey,
+        userauthtype, userclass, radius, radiususer, departmentnumber,
+        employeenumber, employeetype, preferredlanguage, certificate,
+        certmapdata, noprivate, nomembers, preserve, update_password):
     if state == "present":
         if action == "member":
             invalid = ["first", "last", "fullname", "displayname", "initials",
@@ -715,7 +711,7 @@ def check_certmapdata(data):
         return False
 
     i = data.find("<I>", 4)
-    s = data.find("<S>", i)
+    s = data.find("<S>", i)   # pylint: disable=invalid-name
     issuer = data[i+3:s]
     subject = data[s+3:]
 
@@ -1033,7 +1029,7 @@ def main():
 
                 email = extend_emails(email, default_email_domain)
 
-            elif isinstance(user, str) or isinstance(user, unicode):
+            elif isinstance(user, (str, unicode)):
                 name = user
             else:
                 ansible_module.fail_json(msg="User '%s' is not valid" %
@@ -1115,8 +1111,13 @@ def main():
                         # 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):
+                        # The nomembers parameter is added to args for the
+                        # api command. But no_members is never part of
+                        # res_find from user-show, therefore this parameter
+                        # needs to be ignored in compare_args_ipa.
+                        if not compare_args_ipa(
+                                ansible_module, args, res_find,
+                                ignore=["no_members"]):
                             commands.append([name, "user_mod", args])
 
                     else:
@@ -1377,6 +1378,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

@@ -119,6 +119,7 @@ options:
     description: Users that are owners of the vault.
     required: false
     type: list
+    aliases: ["ownerusers"]
   ownergroups:
     description: Groups that are owners of the vault.
     required: false
@@ -317,10 +318,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, \
+    ipalib_errors
 
 
 def find_vault(module, name, username, service, shared):
@@ -347,11 +349,13 @@ def find_vault(module, name, username, service, shared):
     return None
 
 
-def gen_args(description, username, service, shared, vault_type, salt,
-             password, password_file, public_key, public_key_file, vault_data,
-             datafile_in, datafile_out):
+def gen_args(
+        description, username, service, shared, vault_type, salt,
+        public_key, public_key_file):
     _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 +364,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 +404,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 +418,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,21 +441,18 @@ 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
 
 
-def check_parameters(module, state, action, description, username, service,
-                     shared, users, groups, services, owners, ownergroups,
-                     ownerservices, vault_type, salt, password, password_file,
-                     public_key, public_key_file, private_key,
-                     private_key_file, vault_data, datafile_in, datafile_out,
-                     new_password, new_password_file):
+def check_parameters(  # pylint: disable=unused-argument
+        module, state, action, description, username, service, shared, users,
+        groups, services, owners, ownergroups, ownerservices, vault_type, salt,
+        password, password_file, public_key, public_key_file, private_key,
+        private_key_file, vault_data, datafile_in, datafile_out, 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 +465,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,20 +491,18 @@ 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,
-                            public_key_file, private_key, private_key_file,
-                            vault_data, datafile_in, datafile_out,
-                            new_password, new_password_file, res_find):
+def check_encryption_params(  # pylint: disable=unused-argument
+        module, state, action, vault_type, salt, password, password_file,
+        public_key, public_key_file, private_key, private_key_file, vault_data,
+        datafile_in, datafile_out, new_password, new_password_file, res_find):
+    """Check parameters used for (de)vault data encryption."""
     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 +545,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 ipalib_errors.NotFound:
+        return None
+
+    return result['result'].get('data')
 
 
 def main():
@@ -595,10 +601,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,21 +751,16 @@ 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,
-                            public_key_file, vault_data, datafile_in,
-                            datafile_out)
+                            salt, public_key, public_key_file)
             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 +770,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 +891,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 +921,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 +935,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 +961,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 = []
@@ -936,7 +990,7 @@ def main():
                             changed = True
                     else:
                         changed = True
-            except EmptyModlist:
+            except ipalib_errors.EmptyModlist:
                 result = {}
             except Exception as exception:
                 ansible_module.fail_json(
@@ -965,7 +1019,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

requirements.txt

@@ -1 +0,0 @@
-ansible>=2.8.0

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/library/ipabackup_get_backup_dir.py

@@ -0,0 +1,69 @@
+#!/usr/bin/python
+# -*- coding: utf-8 -*-
+
+# Authors:
+#   Thomas Woerner <twoerner@redhat.com>
+#
+# Copyright (C) 2021  Red Hat
+# see file 'COPYING' for use and warranty information
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+ANSIBLE_METADATA = {
+    'metadata_version': '1.0',
+    'supported_by': 'community',
+    'status': ['preview'],
+}
+
+DOCUMENTATION = '''
+---
+module: ipabackup_get_backup_dir
+short description:
+  Get IPA_BACKUP_DIR from ipaplatform
+description:
+  Get IPA_BACKUP_DIR from ipaplatform
+options:
+author:
+    - Thomas Woerner
+'''
+
+EXAMPLES = '''
+# Get IPA_BACKUP_DIR from ipaplatform
+- name: ipabackup_get_backup_dir:
+  register result
+'''
+
+RETURN = '''
+backup_dir:
+  description: IPA_BACKUP_DIR from ipaplatform
+  returned: always
+  type: str
+'''
+
+from ansible.module_utils.basic import AnsibleModule
+from ipaplatform.paths import paths
+
+
+def main():
+    module = AnsibleModule(
+        argument_spec=dict(),
+        supports_check_mode=True,
+    )
+
+    module.exit_json(changed=False,
+                     backup_dir=paths.IPA_BACKUP_DIR)
+
+
+if __name__ == '__main__':
+    main()

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_facts['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=