Update README-automember.md

Missing variable "action" added in the automember module documentation.

.github/workflows/docs.yml

@@ -4,8 +4,8 @@ on:
   - push
   - pull_request
 jobs:
-  check_docs:
-    name: Check Ansible Documentation.
+  check_docs_29:
+    name: Check Ansible Documentation with Ansible 2.9.
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v2
@@ -13,4 +13,20 @@ jobs:
         with:
           python-version: '3.x'
       - name: Run ansible-doc-test
-        run: ANSIBLE_LIBRARY="." python utils/ansible-doc-test roles plugins
+        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

@@ -4,7 +4,7 @@ repos:
   rev: v4.3.5
   hooks:
   - id: ansible-lint
-    always_run: true
+    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
@@ -12,7 +12,7 @@ repos:
   rev: v1.25.0
   hooks:
   - id: yamllint
-    args: ['.']
+    files: \.(yaml|yml)$
 - repo: https://gitlab.com/pycqa/flake8
   rev: 3.8.4
   hooks:
@@ -21,11 +21,18 @@ repos:
   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: script
+    language: python
     entry: utils/ansible-doc-test
     # args: ['-v', 'roles', 'plugins']
     files: ^.*.py$

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

@@ -43,7 +43,7 @@ Example playbook to make sure permission "MyPermission" is present:
 
 ```yaml
 ---
-- name: Playbook to create an IPA permission.
+- name: Playbook to handle IPA permissions
   hosts: ipaserver
   become: yes
 
@@ -56,39 +56,61 @@ Example playbook to make sure permission "MyPermission" is present:
       right: all
 ```
 
-Example playbook to make sure permission "MyPermission" member "privilege" with value "User Administrators" is present:
+
+Example playbook to ensure permission "MyPermission" is present with attr carlicense:
 
 ```yaml
 ---
-- name: Permission add privilege to a permission
+- name: Playbook to handle IPA permissions
   hosts: ipaserver
-  become: true
+  become: yes
 
   tasks:
-  - name: Ensure permission MyPermission is present with the User Administrators privilege present
+  - name: Ensure permission "MyPermission" is present with attr carlicense
     ipapermission:
       ipaadmin_password: SomeADMINpassword
       name: MyPermission
-      privilege: "User Administrators"
+      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 make sure permission "MyPermission" member "privilege" with value "User Administrators" is absent:
-
+Example playbook to ensure attr gecos is absent in permission "MyPermission":
 
 ```yaml
 ---
-- name: Permission remove privilege from a permission
+- name: Playbook to handle IPA permissions
   hosts: ipaserver
-  become: true
+  become: yes
 
   tasks:
-  - name: Ensure permission MyPermission is present without the User Administrators privilege
+  - name: Ensure attr gecos is present in permission "MyPermission"
     ipapermission:
       ipaadmin_password: SomeADMINpassword
       name: MyPermission
-      privilege: "User Administrators"
+      attrs:
+      - gecos
       action: member
       state: absent
 ```
@@ -98,27 +120,30 @@ Example playbook to make sure permission "MyPermission" is absent:
 
 ```yaml
 ---
-- name: Playbook to manage IPA permission.
+- name: Playbook to handle IPA permissions
   hosts: ipaserver
   become: yes
 
   tasks:
-  - ipapermission:
+  - 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 manage IPA permission.
+- name: Playbook to handle IPA permissions
   hosts: ipaserver
   become: yes
 
   tasks:
-  - ipapermission:
+  - name: Ensure permission "MyPermission" is renamed to "MyNewPermission
+    ipapermission:
       ipaadmin_password: SomeADMINpassword
       name: MyPermission
       rename: MyNewPermission
@@ -126,8 +151,6 @@ Example playbook to make sure permission "MyPermission" is renamed to "MyNewPerm
 ```
 
 
-
-
 Variables
 ---------
 
@@ -140,7 +163,7 @@ 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 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
+`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
@@ -153,10 +176,12 @@ Variable | Description | Required
 `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
-`privilege` | Member Privilege of Permission | 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
 =======
 

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

@@ -219,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
 --------
@@ -12,6 +12,7 @@ Features
 * 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
@@ -30,12 +31,13 @@ Features
 * 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
 
@@ -112,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:
 
@@ -136,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**
 
@@ -154,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
@@ -280,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]
@@ -345,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
@@ -366,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
@@ -421,6 +423,7 @@ Roles
 Modules in plugin/modules
 =========================
 
+* [ipaautomember](README-automember.md)
 * [ipaconfig](README-config.md)
 * [ipadelegation](README-delegation.md)
 * [ipadnsconfig](README-dnsconfig.md)
@@ -439,6 +442,7 @@ Modules in plugin/modules
 * [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"

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

@@ -4,8 +4,8 @@
   become: true
 
   tasks:
-  - name: Ensure permission TestPerm1 is absent
+  - name: Ensure permission is absent
     ipapermission:
+      ipaadmin_password: SomeADMINpassword
       name: TestPerm1
       state: absent
-

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

@@ -4,11 +4,12 @@
   become: true
 
   tasks:
-  - name: Ensure permission TestPerm2 is present with Read rights to employeenumber
+  - name: Ensure permission is present with set of rights to attribute employeenumber
     ipapermission:
-      name: TestPerm2
+      ipaadmin_password: SomeADMINpassword
+      name: TestPerm1
       object_type: user
-      perm_rights: 
+      right:
         - read
         - search
         - compare

playbooks/permission/permission-member-absent.yml

@@ -4,8 +4,9 @@
   become: true
 
   tasks:
-  - name: Ensure privilege User Administrators privilege is absent on Permission TestPerm1
+  - name: Ensure permission privilege, "User Administrators", is absent
     ipapermission:
+      ipaadmin_password: SomeADMINpassword
       name: TestPerm1
       privilege: "User Administrators"
       action: member

playbooks/permission/permission-member-present.yml

@@ -4,8 +4,9 @@
   become: true
 
   tasks:
-  - name: Ensure permission TestPerm1 is present with the User Administrators privilege present
+  - 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

@@ -4,8 +4,9 @@
   become: true
 
   tasks:
-  - name: Ensure permission TestPerm1 is present
+  - name: Ensure permission is present
     ipapermission:
+      ipaadmin_password: SomeADMINpassword
       name: TestPerm1
       object_type: host
-      perm_rights: all
+      right: all

playbooks/permission/permission-renamed.yml

@@ -4,8 +4,9 @@
   become: true
 
   tasks:
-  - name: Ensure permission TestPerm1 is present
+  - name: Ensure permission TestPerm1 is renamed to TestPermRenamed
     ipapermission:
+      ipaadmin_password: SomeADMINpassword
       name: TestPerm1
       rename: TestPermRenamed
       state: renamed

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/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/module_utils/ansible_freeipa_module.py

@@ -22,25 +22,42 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 
-import sys
-import operator
-import os
-import uuid
-import tempfile
-import shutil
-import gssapi
-from datetime import datetime
-from pprint import pformat
+__all__ = ["gssapi", "netaddr", "api", "ipalib_errors", "Env",
+           "DEFAULT_CONFIG", "LDAP_GENERALIZED_TIME_FORMAT",
+           "kinit_password", "kinit_keytab", "run", "DN", "VERSION",
+           "paths", "get_credentials_if_valid", "Encoding",
+           "load_pem_x509_certificate", "DNSName"]
 
-try:
+import sys
+
+# HACK: workaround for Ansible 2.9
+# https://github.com/ansible/ansible/issues/68361
+if 'ansible.executor' in sys.modules:
+    for attr in __all__:
+        setattr(sys.modules[__name__], attr, None)
+else:
+    import operator
+    import os
+    import uuid
+    import tempfile
+    import shutil
+    import netaddr
+    import gssapi
+    from datetime import datetime
+    from pprint import pformat
+
+    # ansible-freeipa requires locale to be C, IPA requires utf-8.
+    os.environ["LANGUAGE"] = "C"
+
+    try:
         from packaging import version
-except ImportError:
+    except ImportError:
         # If `packaging` not found, split version string for creating version
         # object. Although it is not PEP 440 compliant, it will work for stable
         # FreeIPA releases.
         import re
 
-    class version:
+        class version:  # pylint: disable=invalid-name, too-few-public-methods
             @staticmethod
             def parse(version_str):
                 """
@@ -48,52 +65,51 @@ except ImportError:
 
                 This will not work for `rc`, `dev` or similar version string.
                 """
-            return tuple(re.split("[-_\.]", version_str))  # noqa: W605
+                return tuple(re.split("[-_.]", version_str))  # noqa: W605
 
-from ipalib import api
-from ipalib import errors as ipalib_errors  # noqa
-from ipalib.config import Env
-from ipalib.constants import DEFAULT_CONFIG, LDAP_GENERALIZED_TIME_FORMAT
+    from ipalib import api
+    from ipalib import errors as ipalib_errors  # noqa
+    from ipalib.config import Env
+    from ipalib.constants import DEFAULT_CONFIG, LDAP_GENERALIZED_TIME_FORMAT
 
-try:
+    try:
         from ipalib.install.kinit import kinit_password, kinit_keytab
-except ImportError:
+    except ImportError:
         from ipapython.ipautil import kinit_password, kinit_keytab
-from ipapython.ipautil import run
-from ipapython.dn import DN
-from ipapython.version import VERSION
-from ipaplatform.paths import paths
-from ipalib.krb_utils import get_credentials_if_valid
-from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils._text import to_text
-from ansible.module_utils.common.text.converters import jsonify
+    from ipapython.ipautil import run
+    from ipapython.dn import DN
+    from ipapython.version import VERSION
+    from ipaplatform.paths import paths
+    from ipalib.krb_utils import get_credentials_if_valid
+    from ipapython.dnsutil import DNSName
+    from ansible.module_utils.basic import AnsibleModule
+    from ansible.module_utils._text import to_text
+    from ansible.module_utils.common.text.converters import jsonify
 
-try:
+    try:
         from ipalib.x509 import Encoding
-except ImportError:
+    except ImportError:
         from cryptography.hazmat.primitives.serialization import Encoding
 
-try:
+    try:
         from ipalib.x509 import load_pem_x509_certificate
-except ImportError:
+    except ImportError:
         from ipalib.x509 import load_certificate
         load_pem_x509_certificate = None
 
-import socket
-import base64
-import six
+    import socket
+    import base64
+    import six
 
-try:
+    try:
         from collections.abc import Mapping  # noqa
-except ImportError:
+    except ImportError:
         from collections import Mapping  # noqa
 
-
-if six.PY3:
+    if six.PY3:
         unicode = str
 
-
-def valid_creds(module, principal):  # noqa
+    def valid_creds(module, principal):  # noqa
         """Get valid credentials matching the princial, try GSSAPI first."""
         if "KRB5CCNAME" in os.environ:
             ccache = os.environ["KRB5CCNAME"]
@@ -130,8 +146,7 @@ def valid_creds(module, principal):  # noqa
             return True
         return False
 
-
-def temp_kinit(principal, password):
+    def temp_kinit(principal, password):
         """Kinit with password using a temporary ccache."""
         if not password:
             raise RuntimeError("The password is not set")
@@ -149,8 +164,7 @@ def temp_kinit(principal, password):
         os.environ["KRB5CCNAME"] = ccache_name
         return ccache_dir, ccache_name
 
-
-def temp_kdestroy(ccache_dir, ccache_name):
+    def temp_kdestroy(ccache_dir, ccache_name):
         """Destroy temporary ticket and remove temporary ccache."""
         if ccache_name is not None:
             run([paths.KDESTROY, '-c', ccache_name], raiseonerr=False)
@@ -158,8 +172,7 @@ def temp_kdestroy(ccache_dir, ccache_name):
         if ccache_dir is not None:
             shutil.rmtree(ccache_dir, ignore_errors=True)
 
-
-def api_connect(context=None):
+    def api_connect(context=None):
         """
         Initialize IPA API with the provided context.
 
@@ -172,7 +185,9 @@ def api_connect(context=None):
         env._bootstrap()
         env._finalize_core(**dict(DEFAULT_CONFIG))
 
-    # available contexts are 'server', 'ansible-freeipa' and 'cli_installer'
+        # available contexts are 'server', 'ansible-freeipa' and
+        # 'cli_installer'
+
         if context is None:
             context = 'server'
 
@@ -187,28 +202,23 @@ def api_connect(context=None):
         if not backend.isconnected():
             backend.connect(ccache=os.environ.get('KRB5CCNAME', None))
 
-
-def api_command(module, command, name, args):
+    def api_command(_module, command, name, args):
         """Call ipa.Command."""
         return api.Command[command](name, **args)
 
-
-def api_command_no_name(module, command, args):
+    def api_command_no_name(_module, command, args):
         """Call ipa.Command without a name."""
         return api.Command[command](**args)
 
-
-def api_check_command(command):
+    def api_check_command(command):
         """Return if command exists in command list."""
         return command in api.Command
 
-
-def api_check_param(command, name):
+    def api_check_param(command, name):
         """Check if param exists in command param list."""
         return name in api.Command[command].params
 
-
-def api_check_ipa_version(oper, requested_version):
+    def api_check_ipa_version(oper, requested_version):
         """
         Compare the installed IPA version against a requested version.
 
@@ -223,12 +233,12 @@ def api_check_ipa_version(oper, requested_version):
             "!=": operator.ne,
         }
         operation = oper_map.get(oper)
-    if not(operation):
+        if not operation:
             raise NotImplementedError("Invalid operator: %s" % oper)
-    return operation(version.parse(VERSION), version.parse(requested_version))
+        return operation(version.parse(VERSION),
+                         version.parse(requested_version))
 
-
-def execute_api_command(module, principal, password, command, name, args):
+    def execute_api_command(module, principal, password, command, name, args):
         """
         Execute an API command.
 
@@ -248,9 +258,10 @@ def execute_api_command(module, principal, password, command, name, args):
 
         finally:
             temp_kdestroy(ccache_dir, ccache_name)
+        # fix pylint inconsistent return
+        return None
 
-
-def date_format(value):
+    def date_format(value):
         accepted_date_formats = [
             LDAP_GENERALIZED_TIME_FORMAT,  # generalized time
             '%Y-%m-%dT%H:%M:%SZ',  # ISO 8601, second precision
@@ -260,29 +271,43 @@ def date_format(value):
             '%Y-%m-%d %H:%MZ',     # non-ISO 8601, minute precision
         ]
 
-    for date_format in accepted_date_formats:
+        for _date_format in accepted_date_formats:
             try:
-            return datetime.strptime(value, date_format)
+                return datetime.strptime(value, _date_format)
             except ValueError:
                 pass
         raise ValueError("Invalid date '%s'" % value)
 
-
-def compare_args_ipa(module, args, ipa):  # noqa
+    def compare_args_ipa(module, args, ipa, ignore=None):  # noqa
         """Compare IPA obj attrs with the command args.
 
         This function compares IPA objects attributes with the args the
-    module is intending to use to call a command. This is useful to know
-    if call to IPA server will be needed or not.
-    In other to compare we have to prepare the perform slight changes in
-    data formats.
+        module is intending to use to call a command. ignore can be a list
+        of attributes, that should be ignored in the comparison.
+        This is useful to know if a call to IPA server will be needed or not.
+        In order to compare we have to perform slight changes in data formats.
 
         Returns True if they are the same and False otherwise.
         """
         base_debug_msg = "Ansible arguments and IPA commands differed. "
 
-    for key in args.keys():
-        if key not in ipa:
+        # If both args and ipa are None, return there's no difference.
+        # If only one is None, return there is a difference.
+        # This tests avoid unecessary invalid access to attributes.
+        if args is None or ipa is None:
+            return args is None and ipa is None
+
+        # Fail if args or ipa are not dicts.
+        if not (isinstance(args, dict) and isinstance(ipa, dict)):
+            raise TypeError("Expected 'dicts' to compare.")
+
+        # Create filtered_args using ignore
+        if ignore is None:
+            ignore = []
+        filtered_args = [key for key in args if key not in ignore]
+
+        for key in filtered_args:
+            if key not in ipa:  # pylint: disable=no-else-return
                 module.debug(
                     base_debug_msg + "Command key not present in IPA: %s" % key
                 )
@@ -307,7 +332,8 @@ def compare_args_ipa(module, args, ipa):  # noqa
                         return False
                     if isinstance(ipa_arg[0], str) and isinstance(arg[0], int):
                         arg = [to_text(_arg) for _arg in arg]
-                if isinstance(ipa_arg[0], unicode) and isinstance(arg[0], int):
+                    if isinstance(ipa_arg[0], unicode) \
+                       and isinstance(arg[0], int):
                         arg = [to_text(_arg) for _arg in arg]
                 try:
                     arg_set = set(arg)
@@ -329,32 +355,36 @@ def compare_args_ipa(module, args, ipa):  # noqa
                         return False
         return True
 
-
-def _afm_convert(value):
+    def _afm_convert(value):
         if value is not None:
             if isinstance(value, list):
                 return [_afm_convert(x) for x in value]
-        elif isinstance(value, dict):
-            return {_afm_convert(k): _afm_convert(v) for k, v in value.items()}
-        elif isinstance(value, str):
+            if isinstance(value, dict):
+                return {_afm_convert(k): _afm_convert(v)
+                        for k, v in value.items()}
+            if isinstance(value, str):
                 return to_text(value)
-        else:
-            return value
-    else:
+
         return value
 
-
-def module_params_get(module, name):
+    def module_params_get(module, name):
         return _afm_convert(module.params.get(name))
 
-
-def api_get_realm():
+    def api_get_realm():
         return api.env.realm
 
+    def gen_add_del_lists(user_list, res_list):
+        """
+        Generate the lists for the addition and removal of members.
 
-def gen_add_del_lists(user_list, res_list):
-    """Generate the lists for the addition and removal of members."""
-    # The user list is None, therefore the parameter should not be touched
+        This function should be used to apply a new user list as a set
+        operation without action: members.
+
+        For the addition of new and the removal of existing members with
+        action: members gen_add_list and gen_intersection_list should
+        be used.
+        """
+        # The user list is None, no need to do anything, return empty lists
         if user_list is None:
             return [], []
 
@@ -363,8 +393,39 @@ def gen_add_del_lists(user_list, res_list):
 
         return add_list, del_list
 
+    def gen_add_list(user_list, res_list):
+        """
+        Generate add list for addition of new members.
 
-def encode_certificate(cert):
+        This function should be used to add new members with action: members
+        and state: present.
+
+        It is returning the difference of the user and res list if the user
+        list is not None.
+        """
+        # The user list is None, no need to do anything, return empty list
+        if user_list is None:
+            return []
+
+        return list(set(user_list or []) - set(res_list or []))
+
+    def gen_intersection_list(user_list, res_list):
+        """
+        Generate the intersection list for removal of existing members.
+
+        This function should be used to remove existing members with
+        action: members and state: absent.
+
+        It is returning the intersection of the user and res list if the
+        user list is not None.
+        """
+        # The user list is None, no need to do anything, return empty list
+        if user_list is None:
+            return []
+
+        return list(set(res_list or []).intersection(set(user_list or [])))
+
+    def encode_certificate(cert):
         """
         Encode a certificate using base64.
 
@@ -378,8 +439,7 @@ def encode_certificate(cert):
             encoded = encoded.decode('ascii')
         return encoded
 
-
-def load_cert_from_str(cert):
+    def load_cert_from_str(cert):
         cert = cert.strip()
         if not cert.startswith("-----BEGIN CERTIFICATE-----"):
             cert = "-----BEGIN CERTIFICATE-----\n" + cert
@@ -392,18 +452,15 @@ def load_cert_from_str(cert):
             cert = load_certificate(cert.encode('utf-8'))
         return cert
 
-
-def DN_x500_text(text):
+    def DN_x500_text(text):  # pylint: disable=invalid-name
         if hasattr(DN, "x500_text"):
             return DN(text).x500_text()
-    else:
         # Emulate x500_text
         dn = DN(text)
         dn.rdns = reversed(dn.rdns)
         return str(dn)
 
-
-def is_valid_port(port):
+    def is_valid_port(port):
         if not isinstance(port, int):
             return False
 
@@ -412,8 +469,23 @@ def is_valid_port(port):
 
         return False
 
+    def is_ip_address(ipaddr):
+        """Test if given IP address is a valid IPv4 or IPv6 address."""
+        try:
+            netaddr.IPAddress(str(ipaddr))
+        except (netaddr.AddrFormatError, ValueError):
+            return False
+        return True
 
-def is_ipv4_addr(ipaddr):
+    def is_ip_network_address(ipaddr):
+        """Test if given IP address is a valid IPv4 or IPv6 address."""
+        try:
+            netaddr.IPNetwork(str(ipaddr))
+        except (netaddr.AddrFormatError, ValueError):
+            return False
+        return True
+
+    def is_ipv4_addr(ipaddr):
         """Test if given IP address is a valid IPv4 address."""
         try:
             socket.inet_pton(socket.AF_INET, ipaddr)
@@ -421,8 +493,7 @@ def is_ipv4_addr(ipaddr):
             return False
         return True
 
-
-def is_ipv6_addr(ipaddr):
+    def is_ipv6_addr(ipaddr):
         """Test if given IP address is a valid IPv6 address."""
         try:
             socket.inet_pton(socket.AF_INET6, ipaddr)
@@ -430,8 +501,7 @@ def is_ipv6_addr(ipaddr):
             return False
         return True
 
-
-def exit_raw_json(module, **kwargs):
+    def exit_raw_json(module, **kwargs):
         """
         Print the raw parameters in JSON format, without masking.
 
@@ -450,15 +520,15 @@ def exit_raw_json(module, **kwargs):
         print(jsonify(kwargs))
         sys.exit(0)
 
-
-class AnsibleFreeIPAParams(Mapping):
+    class AnsibleFreeIPAParams(Mapping):
         def __init__(self, ansible_module):
             self.mapping = ansible_module.params
             self.ansible_module = ansible_module
 
         def __getitem__(self, key):
             param = self.mapping[key]
-        if param is not None:
+            if param is None:
+                return None
             return _afm_convert(param)
 
         def __iter__(self):
@@ -474,8 +544,7 @@ class AnsibleFreeIPAParams(Mapping):
         def __getattr__(self, name):
             return self.get(name)
 
-
-class FreeIPABaseModule(AnsibleModule):
+    class FreeIPABaseModule(AnsibleModule):
         """
         Base class for FreeIPA Ansible modules.
 
@@ -489,7 +558,8 @@ class FreeIPABaseModule(AnsibleModule):
         2. Implement the method ``define_ipa_commands()``
         3. Implement the method ``check_ipa_params()`` (optional)
 
-    After instantiating the class the method ``ipa_run()`` should be called.
+        After instantiating the class the method ``ipa_run()`` should be
+        called.
 
         Example (ansible-freeipa/plugins/modules/ipasomemodule.py):
 
@@ -509,7 +579,8 @@ class FreeIPABaseModule(AnsibleModule):
                 # Validate your params here
                 # Example:
                 if not self.ipa_params.module_param in VALID_OPTIONS:
-                self.fail_json(msg="Invalid value for argument module_param")
+                    self.fail_json(
+                        msg="Invalid value for argument module_param")
 
             def define_ipa_commands(self):
                 args = self.get_ipa_command_args()
@@ -543,6 +614,7 @@ class FreeIPABaseModule(AnsibleModule):
         ipa_param_mapping = None
 
         def __init__(self, *args, **kwargs):
+            # pylint: disable=super-with-arguments
             super(FreeIPABaseModule, self).__init__(*args, **kwargs)
 
             # Attributes to store kerberos credentials (if needed)
@@ -573,7 +645,8 @@ class FreeIPABaseModule(AnsibleModule):
             """
             Return a dict to be passed to an IPA command.
 
-        The keys of ``ipa_param_mapping`` are also the keys of the return dict.
+            The keys of ``ipa_param_mapping`` are also the keys of the return
+            dict.
 
             The values of ``ipa_param_mapping`` needs to be either:
                 * A str with the name of a defined method; or
@@ -607,8 +680,8 @@ class FreeIPABaseModule(AnsibleModule):
                 else:
                     self.fail_json(
                         msg=(
-                        "Couldn't get a value for '%s'. Option '%s' is not "
-                        "a module argument neither a defined method."
+                            "Couldn't get a value for '%s'. Option '%s' is "
+                            "not a module argument neither a defined method."
                         )
                         % (ipa_param_name, param_name)
                     )
@@ -620,7 +693,7 @@ class FreeIPABaseModule(AnsibleModule):
 
         def check_ipa_params(self):
             """Validate ipa_params before command is called."""
-        pass
+            pass  # pylint: disable=unnecessary-pass
 
         def define_ipa_commands(self):
             """Define commands that will be run in IPA server."""
@@ -705,7 +778,7 @@ class FreeIPABaseModule(AnsibleModule):
                         )
 
             if len(errors) > 0:
-            self.fail_json(", ".join("errors"))
+                self.fail_json(", ".join("errors"))  # pylint: disable=E1121
 
         def add_ipa_command(self, command, name=None, args=None):
             """Add a command to the list of commands to be executed."""
@@ -719,16 +792,18 @@ class FreeIPABaseModule(AnsibleModule):
                 try:
                     result = self.api_command(command, name, args)
                 except Exception as excpt:
-                self.fail_json(msg="%s: %s: %s" % (command, name, str(excpt)))
+                    self.fail_json(msg="%s: %s: %s" % (command, name,
+                                                       str(excpt)))
                 else:
                     self.process_command_result(name, command, args, result)
                 self.get_command_errors(command, result)
 
-    def process_command_result(self, name, command, args, result):
+        def process_command_result(self, _name, _command, _args, result):
             """
             Process an API command result.
 
-        This method can be overriden in subclasses, and change self.exit_values
+            This method can be overriden in subclasses, and
+            change self.exit_values
             to return data in the result for the controller.
             """
             if "completed" in result:

plugins/modules/ipaautomember.py

@@ -0,0 +1,423 @@
+#!/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 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)
+
+            # 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
+                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

@@ -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,7 +114,7 @@ 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.")
     return None
 
@@ -233,6 +233,7 @@ def main():
         # Execute command only if configuration changes.
         if not compare_args_ipa(ansible_module, args, res_find):
             try:
+                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

@@ -135,7 +135,7 @@ 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
 
 
@@ -380,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

@@ -868,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
 
 
@@ -1150,7 +1150,7 @@ def find_dnsrecord(module, dnszone, name):
     try:
         _result = api_command(
             module, "dnsrecord_show", to_text(dnszone), _args)
-    except ipalib.errors.NotFound:
+    except ipalib_errors.NotFound:
         return None
 
     return _result["result"]
@@ -1350,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:
@@ -1375,12 +1373,11 @@ def define_commands_for_present_state(module, zone_name, entry, res_find):
                     # 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 = {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:
@@ -1497,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:
@@ -1508,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

@@ -210,11 +210,11 @@ dnszone:
 from ipapython.dnsutil import DNSName  # noqa: E402
 from ansible.module_utils.ansible_freeipa_module import (
     FreeIPABaseModule,
-    is_ipv4_addr,
-    is_ipv6_addr,
+    is_ip_address,
+    is_ip_network_address,
     is_valid_port,
+    ipalib_errors
 )  # noqa: E402
-import ipalib.errors
 import netaddr
 import six
 
@@ -252,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:
@@ -292,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 = (
@@ -304,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
                     )
@@ -328,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)
@@ -358,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.
 
@@ -398,9 +400,8 @@ 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.")
 
     def get_zone(self, zone_name):
@@ -408,7 +409,7 @@ class DNSZoneModule(FreeIPABaseModule):
 
         try:
             response = self.api_command("dnszone_show", args=get_zone_args)
-        except ipalib.errors.NotFound:
+        except ipalib_errors.NotFound:
             zone = None
             is_zone_active = False
         else:
@@ -486,13 +487,20 @@ class DNSZoneModule(FreeIPABaseModule):
             # See:
             #   - https://pagure.io/freeipa/issue/8227
             #   - https://pagure.io/freeipa/issue/8489
-            if set_serial:
+            # 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

@@ -185,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):
@@ -201,7 +202,7 @@ 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
 
 
@@ -556,12 +557,43 @@ def main():
                                 "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:
@@ -596,12 +628,40 @@ def main():
                                 "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:
@@ -616,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:
@@ -631,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,7 +175,7 @@ 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
 
 
@@ -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,7 +89,7 @@ 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
 
 
@@ -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,7 +121,7 @@ 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
 
 
@@ -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

@@ -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" %
@@ -1327,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:]
@@ -1347,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,7 +158,7 @@ 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
 
 
@@ -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

@@ -102,10 +102,6 @@ options:
   rename:
     description: Rename the permission object
     required: false
-  privilege:
-    description: Member Privilege of Permission
-    required: false
-    type: list
   action:
     description: Work on permission or member privilege level.
     choices: ["permission", "member"]
@@ -126,19 +122,6 @@ EXAMPLES = """
     bindtype: permission
     object_type: host
 
-# Ensure permission "NAME" member privilege VALUE is present
-- ipapermission:
-    name: "Add Automember Rebuild Membership Task"
-    privilege: "Automember Task Administrator"
-    action: member
-
-# Ensure permission "NAME" member privilege VALUE is absent
-- ipapermission:
-    name: "Add Automember Rebuild Membership Task"
-    privilege: "IPA Masters Readers"
-    action: member
-    state: absent
-
 # Ensure permission NAME is absent
 - ipapermission:
     name: "Removed Permission Name"
@@ -152,8 +135,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, \
-    api_check_ipa_version
+    compare_args_ipa, module_params_get, api_check_ipa_version
 import six
 
 if six.PY3:
@@ -207,13 +189,6 @@ def gen_args(right, attrs, bindtype, subtree,
     return _args
 
 
-def gen_member_args(privilege):
-    _args = {}
-    if privilege is not None:
-        _args["privilege"] = privilege
-    return _args
-
-
 def main():
     ansible_module = AnsibleModule(
         argument_spec=dict(
@@ -252,7 +227,6 @@ def main():
                              required=False),
             no_members=dict(type=bool, default=None, require=False),
             rename=dict(type="str", default=None, required=False),
-            privilege=dict(type="list", default=None, required=False),
 
             action=dict(type="str", default="permission",
                         choices=["member", "permission"]),
@@ -289,7 +263,6 @@ def main():
     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")
-    privilege = module_params_get(ansible_module, "privilege")
     action = module_params_get(ansible_module, "action")
 
     # state
@@ -304,10 +277,10 @@ def main():
             ansible_module.fail_json(
                 msg="Only one permission can be added at a time.")
         if action == "member":
-            invalid = ["right", "attrs", "bindtype", "subtree",
-                       "extra_target_filter", "rawfilter", "target",
-                       "targetto", "targetfrom", "memberof", "targetgroup",
-                       "object_type", "rename"]
+            invalid = ["bindtype", "target", "targetto", "targetfrom",
+                       "subtree", "targetgroup", "object_type", "rename"]
+        else:
+            invalid = ["rename"]
 
     if state == "renamed":
         if len(names) != 1:
@@ -315,7 +288,7 @@ def main():
                 msg="Only one permission can be renamed at a time.")
         if action == "member":
             ansible_module.fail_json(
-                msg="Member Privileges cannot be renamed")
+                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",
@@ -324,12 +297,12 @@ def main():
     if state == "absent":
         if len(names) < 1:
             ansible_module.fail_json(msg="No name given.")
-        invalid = ["right", "attrs", "bindtype", "subtree",
-                   "extra_target_filter", "rawfilter", "target", "targetto",
-                   "targetfrom", "memberof", "targetgroup", "object_type",
+        invalid = ["bindtype", "subtree", "target", "targetto",
+                   "targetfrom", "targetgroup", "object_type",
                    "no_members", "rename"]
-        if action == "permission":
-            invalid.append("privilege")
+        if action != "member":
+            invalid += ["right", "attrs", "memberof",
+                        "extra_target_filter", "rawfilter"]
 
     for x in invalid:
         if vars()[x] is not None:
@@ -341,6 +314,11 @@ def main():
         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
@@ -366,11 +344,6 @@ def main():
                                 targetto, targetfrom, memberof, targetgroup,
                                 object_type, no_members, rename)
 
-                no_members_value = False
-
-                if no_members is not None:
-                    no_members_value = no_members
-
                 if action == "permission":
                     # Found the permission
                     if res_find is not None:
@@ -383,44 +356,36 @@ def main():
                     else:
                         commands.append([name, "permission_add", args])
 
-                    member_args = gen_member_args(privilege)
-                    if not compare_args_ipa(ansible_module, member_args,
-                                            res_find):
-
-                        # Generate addition and removal lists
-                        privilege_add, privilege_del = gen_add_del_lists(
-                                privilege, res_find.get("member_privilege"))
-
-                        # Add members
-                        if len(privilege_add) > 0:
-                            commands.append([name, "permission_add_member",
-                                             {
-                                                 "privilege": privilege_add,
-                                                 "no_members": no_members_value
-                                             }])
-                        # Remove members
-                        if len(privilege_del) > 0:
-                            commands.append([name, "permission_remove_member",
-                                             {
-                                                 "privilege": privilege_del,
-                                                 "no_members": no_members_value
-                                             }])
                 elif action == "member":
                     if res_find is None:
                         ansible_module.fail_json(
                             msg="No permission '%s'" % name)
 
-                    if privilege is None:
-                        ansible_module.fail_json(msg="No privilege given")
+                    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])
 
-                    commands.append([name, "permission_add_member",
-                                     {
-                                         "privilege": privilege,
-                                         "no_members": no_members_value
-                                     }])
                 else:
                     ansible_module.fail_json(
                         msg="Unknown action '%s'" % action)
+
             elif state == "renamed":
                 if action == "permission":
                     # Generate args
@@ -445,6 +410,7 @@ def main():
                 else:
                     ansible_module.fail_json(
                         msg="Unknown action '%s'" % action)
+
             elif state == "absent":
                 if action == "permission":
                     if res_find is not None:
@@ -455,17 +421,34 @@ def main():
                         ansible_module.fail_json(
                             msg="No permission '%s'" % name)
 
-                    if privilege is None:
-                        ansible_module.fail_json(msg="No privilege given")
+                    member_attrs = {}
+                    check_members = {
+                        "attrs": attrs,
+                        "memberof": memberof,
+                        "ipapermright": right,
+                        "ipapermtargetfilter": rawfilter,
+                        "extratargetfilter": extra_target_filter,
+                        # subtree member management is currently disabled.
+                        # "ipapermlocation": subtree,
+                    }
 
-                    commands.append([name, "permission_remove_member",
-                                     {
-                                         "privilege": privilege,
-                                     }])
+                    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:

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

plugins/modules/iparole.py

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

plugins/modules/ipaselfservice.py

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

plugins/modules/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

@@ -91,7 +91,7 @@ options:
     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:
@@ -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,7 +250,7 @@ def find_service(module, name, netbiosname):
             _res["usercertificate"] = [encode_certificate(cert) for
                                        cert in certs]
         return _res
-    else:
+
     return None
 
 
@@ -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):
+            if (
+                not parameters.get('smb', False)
+                and parameters.get('netbiosname')
+            ):
                 module.fail_json(
-                            msg="Argument '%s' can not be used with SMB "
-                                "service." % _invalid)
+                    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,12 +505,23 @@ def main():
                     if not has_skip_host_check and 'skip_host_check' in args:
                         del args['skip_host_check']
 
-                    if res_find is None:
                     if smb:
-                            if netbiosname is not None:
-                                args['ipantflatname'] = netbiosname
-                            commands.append([name, 'service_add_smb', args])
-                        else:
+                        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:
                         commands.append([name, 'service_add', args])
 
                         certificate_add = certificate or []
@@ -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,7 +90,7 @@ 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
 
 
@@ -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

@@ -107,9 +107,7 @@ 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
-
-import ipalib
+    gen_add_del_lists, ipalib_errors
 
 
 def find_sudocmdgroup(module, name):
@@ -117,7 +115,7 @@ def find_sudocmdgroup(module, name):
 
     try:
         _result = api_command(module, "sudocmdgroup_show", to_text(name), args)
-    except ipalib.errors.NotFound:
+    except ipalib_errors.NotFound:
         return None
     else:
         return _result["result"]
@@ -298,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:
@@ -206,7 +206,7 @@ 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
 
 
@@ -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

@@ -132,7 +132,7 @@ 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
 
 
@@ -147,7 +147,7 @@ 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
 
 
@@ -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,7 +125,7 @@ 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
 
 
@@ -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,6 +240,7 @@ def main():
 
         if state == "absent":
             if res_find is not None:
+                if not ansible_module.check_mode:
                     del_trust(ansible_module, realm)
                 changed = True
         elif res_find is None:
@@ -256,6 +253,7 @@ def main():
                                 trust_secret, base_id, range_size, range_type,
                                 two_way, external)
 
+                if not ansible_module.check_mode:
                     add_trust(ansible_module, realm, args)
                 changed = True
 

plugins/modules/ipauser.py

@@ -512,9 +512,8 @@ 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
 
 
@@ -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, exit_raw_json
-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 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,11 +418,13 @@ def data_storage_args(args, data, password, password_file, private_key,
     if 'shared' in args:
         _args['shared'] = args['shared']
 
+    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 vault_type == "asymmetric":
         if private_key is not None:
             _args['private_key'] = private_key
         if private_key_file is not None:
@@ -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,47 +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_file"] = password_file
-    # retrieve current stored data
-    result = api_command(module, 'vault_retrieve', name, args)
+    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.
-    args = {"override_password": True, "data": result['result']['data']}
-    if new_password:
-        args["password"] = new_password
-    if new_password_file:
-        args["password_file"] = new_password_file
-    # 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():
@@ -594,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']),
@@ -742,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
@@ -766,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:
@@ -851,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:
@@ -875,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']
 
@@ -888,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":
@@ -910,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 = []
@@ -935,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(

requirements-dev.txt

@@ -1,3 +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/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/tasks/backup.yml

@@ -4,13 +4,13 @@
 - name: Create backup
   shell: >
     ipa-backup
-    {{ "--gpg" if ipabackup_gpg | bool }}
-    {{ "--gpg-keyring="+ipabackup_gpg_keyring if ipabackup_gpg_keyring is defined }}
-    {{ "--data" if ipabackup_data | bool }}
-    {{ "--logs" if ipabackup_logs | bool }}
-    {{ "--online" if ipabackup_online | bool }}
-    {{ "--disable-role-check" if ipabackup_disable_role_check | bool }}
-    {{ "--log-file="+ipabackup_log_file if ipabackup_log_file is defined }}
+    {{ "--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:

roles/ipabackup/tasks/copy_backup_from_server.yml

@@ -10,7 +10,7 @@
   set_fact:
     ipabackup_controller_dir:
         "{{ ipabackup_controller_path | default(lookup('env','PWD')) }}/{{
-         ipabackup_name_prefix | default(ansible_fqdn) }}_{{
+         ipabackup_name_prefix | default(ansible_facts['fqdn']) }}_{{
          ipabackup_item }}/"
 
 - name: Stat backup on server

roles/ipabackup/tasks/get_ipabackup_dir.yml

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

roles/ipabackup/tasks/restore.yml

@@ -6,9 +6,9 @@
 - name: Import variables specific to distribution
   include_vars: "{{ item }}"
   with_first_found:
-    - "{{ role_path }}/vars/{{ ansible_distribution }}-{{ ansible_distribution_version }}.yml"
-    - "{{ role_path }}/vars/{{ ansible_distribution }}-{{ ansible_distribution_major_version }}.yml"
-    - "{{ role_path }}/vars/{{ ansible_distribution }}.yml"
+    - "{{ role_path }}/vars/{{ ansible_facts['distribution'] }}-{{ ansible_facts['distribution_version'] }}.yml"
+    - "{{ role_path }}/vars/{{ ansible_facts['distribution'] }}-{{ ansible_facts['distribution_major_version'] }}.yml"
+    - "{{ role_path }}/vars/{{ ansible_facts['distribution'] }}.yml"
     - "{{ role_path }}/vars/default.yml"
 
 ### GET SERVICES FROM BACKUP
@@ -105,13 +105,13 @@
     ipa-restore
     {{ ipabackup_item }}
     --unattended
-    {{ "--password="+ipabackup_password if ipabackup_password is defined }}
-    {{ "--data" if ipabackup_data | bool }}
-    {{ "--online" if ipabackup_online | bool }}
-    {{ "--instance="+ipabackup_instance if ipabackup_instance is defined }}
-    {{ "--backend="+ipabackup_backend if ipabackup_backend is defined }}
-    {{ "--no-logs" if ipabackup_no_logs | bool }}
-    {{ "--log-file="+ipabackup_log_file if ipabackup_log_file is defined }}
+    {{ "--password="+ipabackup_password if ipabackup_password is defined else "" }}
+    {{ "--data" if ipabackup_data | bool else "" }}
+    {{ "--online" if ipabackup_online | bool else "" }}
+    {{ "--instance="+ipabackup_instance if ipabackup_instance is defined else "" }}
+    {{ "--backend="+ipabackup_backend if ipabackup_backend is defined else "" }}
+    {{ "--no-logs" if ipabackup_no_logs | bool else "" }}
+    {{ "--log-file="+ipabackup_log_file if ipabackup_log_file is defined else "" }}
   register: result_iparestore
   ignore_errors: yes
 
@@ -127,21 +127,21 @@
   command: >
     firewall-cmd
     --permanent
-    --zone="{{ ipabackup_firewalld_zone if ipabackup_firewalld_zone is defined }}"
+    {{ "--zone="+ipabackup_firewalld_zone if ipabackup_firewalld_zone is defined else "" }}
     --add-service=freeipa-ldap
     --add-service=freeipa-ldaps
-    {{ "--add-service=freeipa-trust" if ipabackup_service_adtrust in ipabackup_services }}
-    {{ "--add-service=dns" if ipabackup_service_dns in ipabackup_services }}
-    {{ "--add-service=ntp" if ipabackup_service_ntp in ipabackup_services }}
+    {{ "--add-service=freeipa-trust" if ipabackup_service_adtrust in ipabackup_services else "" }}
+    {{ "--add-service=dns" if ipabackup_service_dns in ipabackup_services else "" }}
+    {{ "--add-service=ntp" if ipabackup_service_ntp in ipabackup_services else "" }}
   when: ipabackup_setup_firewalld | bool
 
 - name: Configure firewalld runtime
   command: >
     firewall-cmd
-    --zone="{{ ipabackup_firewalld_zone if ipabackup_firewalld_zone is defined }}"
+    {{ "--zone="+ipabackup_firewalld_zone if ipabackup_firewalld_zone is defined else "" }}
     --add-service=freeipa-ldap
     --add-service=freeipa-ldaps
-    {{ "--add-service=freeipa-trust" if ipabackup_service_adtrust in ipabackup_services }}
-    {{ "--add-service=dns" if ipabackup_service_dns in ipabackup_services }}
-    {{ "--add-service=ntp" if ipabackup_service_ntp in ipabackup_services }}
+    {{ "--add-service=freeipa-trust" if ipabackup_service_adtrust in ipabackup_services else "" }}
+    {{ "--add-service=dns" if ipabackup_service_dns in ipabackup_services else "" }}
+    {{ "--add-service=ntp" if ipabackup_service_ntp in ipabackup_services else "" }}
   when: ipabackup_setup_firewalld | bool

roles/ipaclient/library/ipaclient_get_facts.py

@@ -1,6 +1,15 @@
 #!/usr/bin/python
 # -*- coding: utf-8 -*-
 
+DOCUMENTATION = """
+---
+module: ipaclient_get_facts
+short description: Get facts about IPA client and server configuration.
+description: Get facts about IPA client and server configuration.
+author:
+    - Thomas Woerner
+"""
+
 import os
 import re
 import six

roles/ipaclient/library/ipaclient_test.py

@@ -180,9 +180,9 @@ ntp_servers:
   type: list
   sample: ["ntp.example.com"]
 ipa_python_version:
-  description:
-  - The IPA python version as a number:
-  - <major version>*10000+<minor version>*100+<release>
+  description: >
+    The IPA python version as a number:
+    <major version>*10000+<minor version>*100+<release>
   returned: always
   type: int
   sample: 040400

roles/ipaclient/module_utils/ansible_ipa_client.py

@@ -45,17 +45,26 @@ __all__ = ["gssapi", "version", "ipadiscovery", "api", "errors", "x509",
            "configure_firefox", "sync_time", "check_ldap_conf",
            "sssd_enable_ifp"]
 
-from ipapython.version import NUM_VERSION, VERSION
+import sys
+
+# HACK: workaround for Ansible 2.9
+# https://github.com/ansible/ansible/issues/68361
+if 'ansible.executor' in sys.modules:
+    for attr in __all__:
+        setattr(sys.modules[__name__], attr, None)
 
-if NUM_VERSION < 30201:
-    # See ipapython/version.py
-    IPA_MAJOR, IPA_MINOR, IPA_RELEASE = [int(x) for x in VERSION.split(".", 2)]
-    IPA_PYTHON_VERSION = IPA_MAJOR*10000 + IPA_MINOR*100 + IPA_RELEASE
 else:
+    from ipapython.version import NUM_VERSION, VERSION
+
+    if NUM_VERSION < 30201:
+        # See ipapython/version.py
+        IPA_MAJOR, IPA_MINOR, IPA_RELEASE = [int(x) for x in
+                                             VERSION.split(".", 2)]
+        IPA_PYTHON_VERSION = IPA_MAJOR*10000 + IPA_MINOR*100 + IPA_RELEASE
+    else:
         IPA_PYTHON_VERSION = NUM_VERSION
 
-
-class installer_obj(object):
+    class installer_obj(object):
         def __init__(self):
             pass
 
@@ -76,22 +85,22 @@ class installer_obj(object):
         #    return getattr(self, attr)
 
         # def __setattr__(self, attr, value):
-    #    logger.debug("  --> Setting installer.%s to %s" % (attr, repr(value)))
+        #    logger.debug("  --> Setting installer.%s to %s" %
+        #                 (attr, repr(value)))
         #    return super(installer_obj, self).__setattr__(attr, value)
 
         def knobs(self):
             for name in self.__dict__:
                 yield self, name
 
+    # Initialize installer settings
+    installer = installer_obj()
+    # Create options
+    options = installer
+    options.interactive = False
+    options.unattended = not options.interactive
 
-# Initialize installer settings
-installer = installer_obj()
-# Create options
-options = installer
-options.interactive = False
-options.unattended = not options.interactive
-
-if NUM_VERSION >= 40400:
+    if NUM_VERSION >= 40400:
         # IPA version >= 4.4
 
         import sys
@@ -147,9 +156,11 @@ if NUM_VERSION >= 40400:
             from ipaclient.install.client import configure_krb5_conf, \
                 get_ca_certs, SECURE_PATH, get_server_connection_interface, \
                 disable_ra, client_dns, \
-            configure_certmonger, update_ssh_keys, configure_openldap_conf, \
+                configure_certmonger, update_ssh_keys, \
+                configure_openldap_conf, \
                 hardcode_ldap_server, get_certs_from_ldap, save_state, \
-            create_ipa_nssdb, configure_ssh_config, configure_sshd_config, \
+                create_ipa_nssdb, configure_ssh_config, \
+                configure_sshd_config, \
                 configure_automount, configure_firefox, configure_nisdomain, \
                 CLIENT_INSTALL_ERROR, is_ipa_client_installed, \
                 CLIENT_ALREADY_CONFIGURED, nssldap_exists, remove_file, \
@@ -182,7 +193,8 @@ if NUM_VERSION >= 40400:
             shutil.rmtree(temp_dir, ignore_errors=True)
             sys.path.remove(temp_dir)
 
-        argspec = inspect.getargspec(ipa_client_install.configure_krb5_conf)
+            argspec = inspect.getargspec(
+                ipa_client_install.configure_krb5_conf)
             if argspec.keywords is None:
                 def configure_krb5_conf(
                         cli_realm, cli_domain, cli_server, cli_kdc, dnsok,
@@ -192,8 +204,8 @@ if NUM_VERSION >= 40400:
                     options.force = force
                     options.sssd = configure_sssd
                     return ipa_client_install.configure_krb5_conf(
-                    cli_realm, cli_domain, cli_server, cli_kdc, dnsok, options,
-                    filename, client_domain, client_hostname)
+                        cli_realm, cli_domain, cli_server, cli_kdc, dnsok,
+                        options, filename, client_domain, client_hostname)
             else:
                 configure_krb5_conf = ipa_client_install.configure_krb5_conf
             if NUM_VERSION < 40100:
@@ -211,19 +223,22 @@ if NUM_VERSION >= 40400:
             client_dns = ipa_client_install.client_dns
             configure_certmonger = ipa_client_install.configure_certmonger
             update_ssh_keys = ipa_client_install.update_ssh_keys
-        configure_openldap_conf = ipa_client_install.configure_openldap_conf
+            configure_openldap_conf = \
+                ipa_client_install.configure_openldap_conf
             hardcode_ldap_server = ipa_client_install.hardcode_ldap_server
             get_certs_from_ldap = ipa_client_install.get_certs_from_ldap
             save_state = ipa_client_install.save_state
 
             create_ipa_nssdb = certdb.create_ipa_nssdb
 
-        argspec = inspect.getargspec(ipa_client_install.configure_nisdomain)
+            argspec = \
+                inspect.getargspec(ipa_client_install.configure_nisdomain)
             if len(argspec.args) == 3:
                 configure_nisdomain = ipa_client_install.configure_nisdomain
             else:
                 def configure_nisdomain(options, domain, statestore=None):
-                return ipa_client_install.configure_nisdomain(options, domain)
+                    return ipa_client_install.configure_nisdomain(options,
+                                                                  domain)
 
             configure_ldap_conf = ipa_client_install.configure_ldap_conf
             configure_nslcd_conf = ipa_client_install.configure_nslcd_conf
@@ -264,7 +279,7 @@ if NUM_VERSION >= 40400:
         logger = logging.getLogger("ipa-client-install")
         root_logger = logger
 
-else:
+    else:
         # IPA version < 4.4
 
         raise Exception("freeipa version '%s' is too old" % VERSION)

roles/ipaclient/tasks/install.yml

@@ -33,7 +33,7 @@
     domain: "{{ ipaserver_domain | default(ipaclient_domain) | default(omit) }}"
     servers: "{{ ipaclient_servers | default(omit) }}"
     realm: "{{ ipaserver_realm | default(ipaclient_realm) | default(omit) }}"
-    hostname: "{{ ipaclient_hostname | default(ansible_fqdn) }}"
+    hostname: "{{ ipaclient_hostname | default(ansible_facts['fqdn']) }}"
     ntp_servers: "{{ ipaclient_ntp_servers | default(omit) }}"
     ntp_pool: "{{ ipaclient_ntp_pool | default(omit) }}"
     no_ntp: "{{ ipaclient_no_ntp }}"
@@ -181,8 +181,12 @@
     # Do not fail on error codes 3 and 5:
     #   3 - Unable to open keytab
     #   5 - Principal name or realm not found in keytab
+    #   7 - Failed to set cursor, typically when errcode
+    #       would be issued in past
     failed_when: result_ipa_rmkeytab.rc != 0 and
-                 result_ipa_rmkeytab.rc != 3 and result_ipa_rmkeytab.rc != 5
+                 result_ipa_rmkeytab.rc != 3 and
+                 result_ipa_rmkeytab.rc != 5 and
+                 result_ipa_rmkeytab.rc != 7
     when: (ipaclient_use_otp | bool or ipaclient_force_join | bool) and not ipaclient_on_master | bool
 
   - name: Install - Backup and set hostname

roles/ipaclient/tasks/main.yml

@@ -4,9 +4,9 @@
 - name: Import variables specific to distribution
   include_vars: "{{ item }}"
   with_first_found:
-    - "{{ role_path }}/vars/{{ ansible_distribution }}-{{ ansible_distribution_version }}.yml"
-    - "{{ role_path }}/vars/{{ ansible_distribution }}-{{ ansible_distribution_major_version }}.yml"
-    - "{{ role_path }}/vars/{{ ansible_distribution }}.yml"
+    - "{{ role_path }}/vars/{{ ansible_facts['distribution'] }}-{{ ansible_facts['distribution_version'] }}.yml"
+    - "{{ role_path }}/vars/{{ ansible_facts['distribution'] }}-{{ ansible_facts['distribution_major_version'] }}.yml"
+    - "{{ role_path }}/vars/{{ ansible_facts['distribution'] }}.yml"
     - "{{ role_path }}/vars/default.yml"
 
 - name: Install IPA client

roles/ipareplica/library/ipareplica_prepare.py

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

roles/ipareplica/library/ipareplica_setup_dns.py

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

roles/ipareplica/module_utils/ansible_ipa_replica.py

@@ -46,21 +46,27 @@ __all__ = ["contextlib", "dnsexception", "dnsresolver", "dnsreversename",
            "dnsname", "kernel_keyring", "krbinstance"]
 
 import sys
-import logging
-from contextlib import contextmanager as contextlib_contextmanager
 
-
-from ipapython.version import NUM_VERSION, VERSION
-
-if NUM_VERSION < 30201:
-    # See ipapython/version.py
-    IPA_MAJOR, IPA_MINOR, IPA_RELEASE = [int(x) for x in VERSION.split(".", 2)]
-    IPA_PYTHON_VERSION = IPA_MAJOR*10000 + IPA_MINOR*100 + IPA_RELEASE
+# HACK: workaround for Ansible 2.9
+# https://github.com/ansible/ansible/issues/68361
+if 'ansible.executor' in sys.modules:
+    for attr in __all__:
+        setattr(sys.modules[__name__], attr, None)
 else:
+    import logging
+    from contextlib import contextmanager as contextlib_contextmanager
+
+    from ipapython.version import NUM_VERSION, VERSION
+
+    if NUM_VERSION < 30201:
+        # See ipapython/version.py
+        IPA_MAJOR, IPA_MINOR, IPA_RELEASE = [int(x) for x in
+                                             VERSION.split(".", 2)]
+        IPA_PYTHON_VERSION = IPA_MAJOR*10000 + IPA_MINOR*100 + IPA_RELEASE
+    else:
         IPA_PYTHON_VERSION = NUM_VERSION
 
-
-if NUM_VERSION >= 40600:
+    if NUM_VERSION >= 40600:
         # IPA version >= 4.6
 
         import contextlib
@@ -77,7 +83,8 @@ if NUM_VERSION >= 40600:
         from ipapython.ipautil import ipa_generate_password
         from ipalib.install.kinit import kinit_keytab
         from ipapython import ipaldap, ipautil, kernel_keyring
-    from ipapython.certdb import IPA_CA_TRUST_FLAGS, EXTERNAL_CA_TRUST_FLAGS
+        from ipapython.certdb import IPA_CA_TRUST_FLAGS, \
+            EXTERNAL_CA_TRUST_FLAGS
         from ipapython.dn import DN
         from ipapython.admintool import ScriptError
         from ipapython.ipa_log_manager import standard_logging_setup
@@ -89,7 +96,8 @@ if NUM_VERSION >= 40600:
         from ipalib.util import (
             validate_domain_name,
             no_matching_interface_for_ip_address_warning)
-    from ipaclient.install.client import configure_krb5_conf, purge_host_keytab
+        from ipaclient.install.client import configure_krb5_conf, \
+            purge_host_keytab
         from ipaserver.install import (
             adtrust, bindinstance, ca, certs, dns, dsinstance, httpinstance,
             installutils, kra, krbinstance,
@@ -111,7 +119,8 @@ if NUM_VERSION >= 40600:
         from ipaserver.install.server.replicainstall import (
             make_pkcs12_info, install_replica_ds, install_krb, install_ca_cert,
             install_http, install_dns_records, create_ipa_conf, check_dirsrv,
-        check_dns_resolution, configure_certmonger, remove_replica_info_dir,
+            check_dns_resolution, configure_certmonger,
+            remove_replica_info_dir,
             # common_cleanup,
             preserve_enrollment_state, uninstall_client,
             promote_sssd, promote_openldap_conf, rpc_client,
@@ -136,33 +145,28 @@ if NUM_VERSION >= 40600:
             from ipaserver.install import ntpinstance
             time_service = "ntpd"
 
-
-else:
+    else:
         # IPA version < 4.6
 
         raise Exception("freeipa version '%s' is too old" % VERSION)
 
+    logger = logging.getLogger("ipa-server-install")
 
-logger = logging.getLogger("ipa-server-install")
-
-
-def setup_logging():
+    def setup_logging():
         # logger.setLevel(logging.DEBUG)
         standard_logging_setup(
             paths.IPAREPLICA_INSTALL_LOG, verbose=False, debug=False,
             filemode='a', console_format='%(message)s')
 
-
-@contextlib_contextmanager
-def redirect_stdout(f):
+    @contextlib_contextmanager
+    def redirect_stdout(f):
         sys.stdout = f
         try:
             yield f
         finally:
             sys.stdout = sys.__stdout__
 
-
-class AnsibleModuleLog():
+    class AnsibleModuleLog():
         def __init__(self, module):
             self.module = module
             _ansible_module_log = self
@@ -195,8 +199,7 @@ class AnsibleModuleLog():
             self.module.debug(msg)
             # self.module.warn(msg)
 
-
-class installer_obj(object):
+    class installer_obj(object):
         def __init__(self):
             # CompatServerReplicaInstall
             self.ca_cert_files = None
@@ -234,7 +237,8 @@ class installer_obj(object):
         #     value = super(installer_obj, self).__getattribute__(attr)
         #     if not attr.startswith("--") and not attr.endswith("--"):
         #         logger.debug(
-    #             "  <-- Accessing installer.%s (%s)" % (attr, repr(value)))
+        #             "  <-- Accessing installer.%s (%s)" %
+        #             (attr, repr(value)))
         #     return value
 
         def __getattr__(self, attr):
@@ -243,33 +247,32 @@ class installer_obj(object):
             return getattr(self, attr)
 
         # def __setattr__(self, attr, value):
-    #    logger.debug("  --> Setting installer.%s to %s" % (attr, repr(value)))
+        #    logger.debug("  --> Setting installer.%s to %s" %
+        #                 (attr, repr(value)))
         #    return super(installer_obj, self).__setattr__(attr, value)
 
         def knobs(self):
             for name in self.__dict__:
                 yield self, name
 
+    installer = installer_obj()
+    options = installer
 
-installer = installer_obj()
-options = installer
+    # DNSInstallInterface
+    options.dnssec_master = False
+    options.disable_dnssec_master = False
+    options.kasp_db_file = None
+    options.force = False
 
-# DNSInstallInterface
-options.dnssec_master = False
-options.disable_dnssec_master = False
-options.kasp_db_file = None
-options.force = False
+    # ServerMasterInstall
+    options.add_sids = False
+    options.add_agents = False
 
-# ServerMasterInstall
-options.add_sids = False
-options.add_agents = False
+    # ServerReplicaInstall
+    options.subject_base = None
+    options.ca_subject = None
 
-# ServerReplicaInstall
-options.subject_base = None
-options.ca_subject = None
-
-
-def gen_env_boostrap_finalize_core(etc_ipa, default_config):
+    def gen_env_boostrap_finalize_core(etc_ipa, default_config):
         env = Env()
         # env._bootstrap(context='installer', confdir=paths.ETC_IPA, log=None)
         # env._finalize_core(**dict(constants.DEFAULT_CONFIG))
@@ -277,10 +280,10 @@ def gen_env_boostrap_finalize_core(etc_ipa, default_config):
         env._finalize_core(**dict(default_config))
         return env
 
-
-def api_bootstrap_finalize(env):
+    def api_bootstrap_finalize(env):
         # pylint: disable=no-member
-    xmlrpc_uri = 'https://{}/ipa/xml'.format(ipautil.format_netloc(env.host))
+        xmlrpc_uri = \
+            'https://{}/ipa/xml'.format(ipautil.format_netloc(env.host))
         api.bootstrap(in_server=True,
                       context='installer',
                       confdir=paths.ETC_IPA,
@@ -289,14 +292,14 @@ def api_bootstrap_finalize(env):
         # pylint: enable=no-member
         api.finalize()
 
-
-def gen_ReplicaConfig():
+    def gen_ReplicaConfig():
         class ExtendedReplicaConfig(ReplicaConfig):
             def __init__(self, top_dir=None):
                 super(ExtendedReplicaConfig, self).__init__(top_dir)
 
             # def __getattribute__(self, attr):
-        #    value = super(ExtendedReplicaConfig, self).__getattribute__(attr)
+            #     value = super(ExtendedReplicaConfig, self).__getattribute__(
+            #         attr)
             #    if attr not in ["__dict__", "knobs"]:
             #        logger.debug("  <== Accessing config.%s (%s)" %
             #                     (attr, repr(value)))
@@ -308,8 +311,10 @@ def gen_ReplicaConfig():
                 return getattr(self, attr)
 
             # def __setattr__(self, attr, value):
-        #   logger.debug("  ==> Setting config.%s to %s" % (attr, repr(value)))
-        #   return super(ExtendedReplicaConfig, self).__setattr__(attr, value)
+            #   logger.debug("  ==> Setting config.%s to %s" %
+            #                (attr, repr(value)))
+            #   return super(ExtendedReplicaConfig, self).__setattr__(attr,
+            #                                                         value)
 
             def knobs(self):
                 for name in self.__dict__:
@@ -332,8 +337,7 @@ def gen_ReplicaConfig():
 
         return config
 
-
-def replica_ds_init_info(ansible_log,
+    def replica_ds_init_info(ansible_log,
                              config, options, ca_is_configured, remote_api,
                              ds_ca_subject, ca_file,
                              promote=False, pkcs12_info=None):
@@ -352,7 +356,8 @@ def replica_ds_init_info(ansible_log,
         # if ca_is_configured:
         #     ca_subject = ca.lookup_ca_subject(_api, config.subject_base)
         # else:
-    #     ca_subject = installutils.default_ca_subject_dn(config.subject_base)
+        #     ca_subject = installutils.default_ca_subject_dn(
+        #         config.subject_base)
         ca_subject = ds_ca_subject
 
         ds = dsinstance.DsInstance(
@@ -397,20 +402,19 @@ def replica_ds_init_info(ansible_log,
 
         return ds
 
-
-def ansible_module_get_parsed_ip_addresses(ansible_module,
+    def ansible_module_get_parsed_ip_addresses(ansible_module,
                                                param='ip_addresses'):
         ip_addrs = []
         for ip in ansible_module.params.get(param):
             try:
                 ip_parsed = ipautil.CheckedIPAddress(ip)
             except Exception as e:
-            ansible_module.fail_json(msg="Invalid IP Address %s: %s" % (ip, e))
+                ansible_module.fail_json(
+                    msg="Invalid IP Address %s: %s" % (ip, e))
             ip_addrs.append(ip_parsed)
         return ip_addrs
 
-
-def gen_remote_api(master_host_name, etc_ipa):
+    def gen_remote_api(master_host_name, etc_ipa):
         ldapuri = 'ldaps://%s' % ipautil.format_netloc(master_host_name)
         xmlrpc_uri = 'https://{}/ipa/xml'.format(
             ipautil.format_netloc(master_host_name))

roles/ipareplica/tasks/install.yml

@@ -72,7 +72,7 @@
             default(omit) }}"
     servers: "{{ ipareplica_servers | default(omit) }}"
     realm: "{{ ipareplica_realm | default(ipaserver_realm) |default(omit) }}"
-    hostname: "{{ ipareplica_hostname | default(ansible_fqdn) }}"
+    hostname: "{{ ipareplica_hostname | default(ansible_facts['fqdn']) }}"
     ca_cert_files: "{{ ipareplica_ca_cert_files | default([]) }}"
     hidden_replica: "{{ ipareplica_hidden_replica }}"
     skip_mem_check: "{{ not ipareplica_mem_check }}"

roles/ipareplica/tasks/main.yml

@@ -4,9 +4,9 @@
 - name: Import variables specific to distribution
   include_vars: "{{ item }}"
   with_first_found:
-    - "vars/{{ ansible_distribution }}-{{ ansible_distribution_version }}.yml"
-    - "vars/{{ ansible_distribution }}-{{ ansible_distribution_major_version }}.yml"
-    - "vars/{{ ansible_distribution }}.yml"
+    - "vars/{{ ansible_facts['distribution'] }}-{{ ansible_facts['distribution_version'] }}.yml"
+    - "vars/{{ ansible_facts['distribution'] }}-{{ ansible_facts['distribution_major_version'] }}.yml"
+    - "vars/{{ ansible_facts['distribution'] }}.yml"
     - "vars/default.yml"
 
 - name: Install IPA replica

roles/ipareplica/tasks/uninstall.yml

@@ -25,7 +25,7 @@
 #  command: >
 #    /usr/sbin/ipa-replica-manage
 #    del
-#    {{ ipareplica_hostname | default(ansible_fqdn) }}
+#    {{ ipareplica_hostname | default(ansible_facts['fqdn']) }}
 #    --force
 #    --password={{ ipadm_password }}
 #  failed_when: False

roles/ipaserver/README.md

@@ -260,12 +260,12 @@ Certificate system Variables
 Variable | Description | Required
 -------- | ----------- | --------
 `ipaserver_external_ca` | Generate a CSR for the IPA CA certificate to be signed by an external CA. (bool, default: false) | no
-`ipaserver_external_ca_type` | Type of the external CA. (choice: generic,ms-cs) | no
+`ipaserver_external_ca_type` | Type of the external CA. (choice: generic, ms-cs) | no
 `ipaserver_external_ca_profile` | Specify the certificate profile/template to use at the external CA. (string) | no
 `ipaserver_external_cert_files` | Files containing the IPA CA certificates and the external CA certificate chains (list of string) | no
 `ipaserver_subject_base` | The certificate subject base (default O=<realm-name>). RDNs are in LDAP order (most specific RDN first). (string) | no
 `ipaserver_ca_subject` | The CA certificate subject DN (default CN=Certificate Authority,O=<realm-name>). RDNs are in LDAP order (most specific RDN first). (string) | no
-`ipaserver_ca_signing_algorithm` | Signing algorithm of the IPA CA certificate. (choice: SHA1withRSA,SHA256withRSA,SHA512withRSA) | no
+`ipaserver_ca_signing_algorithm` | Signing algorithm of the IPA CA certificate. (choice: SHA1withRSA, SHA256withRSA, SHA512withRSA) | no
 
 DNS Variables
 -------------
@@ -280,7 +280,7 @@ Variable | Description | Required
 `ipaserver_forwarders` | Add DNS forwarders to the DNS configuration. (list of strings) | no
 `ipaserver_no_forwarders` | Do not add any DNS forwarders. Root DNS servers will be used instead. (bool, default: false) | no
 `ipaserver_auto_forwarders` | Add DNS forwarders configured in /etc/resolv.conf to the list of forwarders used by IPA DNS. (bool, default: false) | no
-`ipaserver_forward_policy` | DNS forwarding policy for global forwarders specified using other options. (choice: first|only) | no
+`ipaserver_forward_policy` | DNS forwarding policy for global forwarders specified using other options. (choice: first, only) | no
 `ipaserver_no_dnssec_validation` | Disable DNSSEC validation on this server. (bool, default: false) | no
 
 AD trust Variables

roles/ipaserver/module_utils/ansible_ipa_server.py

@@ -41,23 +41,31 @@ __all__ = ["IPAChangeConf", "certmonger", "sysrestore", "root_logger",
            "check_available_memory"]
 
 import sys
-import logging
-from contextlib import contextmanager as contextlib_contextmanager
-import six
-import base64
 
+# HACK: workaround for Ansible 2.9
+# https://github.com/ansible/ansible/issues/68361
+if 'ansible.executor' in sys.modules:
+    for attr in __all__:
+        setattr(sys.modules[__name__], attr, None)
 
-from ipapython.version import NUM_VERSION, VERSION
-
-if NUM_VERSION < 30201:
-    # See ipapython/version.py
-    IPA_MAJOR, IPA_MINOR, IPA_RELEASE = [int(x) for x in VERSION.split(".", 2)]
-    IPA_PYTHON_VERSION = IPA_MAJOR*10000 + IPA_MINOR*100 + IPA_RELEASE
 else:
+
+    import logging
+    from contextlib import contextmanager as contextlib_contextmanager
+    import six
+    import base64
+
+    from ipapython.version import NUM_VERSION, VERSION
+
+    if NUM_VERSION < 30201:
+        # See ipapython/version.py
+        IPA_MAJOR, IPA_MINOR, IPA_RELEASE = [int(x) for x in
+                                             VERSION.split(".", 2)]
+        IPA_PYTHON_VERSION = IPA_MAJOR*10000 + IPA_MINOR*100 + IPA_RELEASE
+    else:
         IPA_PYTHON_VERSION = NUM_VERSION
 
-
-if NUM_VERSION >= 40500:
+    if NUM_VERSION >= 40500:
         # IPA version >= 4.5
 
         from ipaclient.install.ipachangeconf import IPAChangeConf
@@ -167,32 +175,28 @@ if NUM_VERSION >= 40500:
             from ipalib.x509 import load_certificate
             load_pem_x509_certificate = None
 
-else:
+    else:
         # IPA version < 4.5
 
         raise Exception("freeipa version '%s' is too old" % VERSION)
 
+    logger = logging.getLogger("ipa-server-install")
 
-logger = logging.getLogger("ipa-server-install")
-
-
-def setup_logging():
+    def setup_logging():
         # logger.setLevel(logging.DEBUG)
         standard_logging_setup(
             paths.IPASERVER_INSTALL_LOG, verbose=False, debug=False,
             filemode='a', console_format='%(message)s')
 
-
-@contextlib_contextmanager
-def redirect_stdout(f):
+    @contextlib_contextmanager
+    def redirect_stdout(f):
         sys.stdout = f
         try:
             yield f
         finally:
             sys.stdout = sys.__stdout__
 
-
-class AnsibleModuleLog():
+    class AnsibleModuleLog():
         def __init__(self, module):
             self.module = module
             _ansible_module_log = self
@@ -225,8 +229,7 @@ class AnsibleModuleLog():
             self.module.debug(msg)
             # self.module.warn(msg)
 
-
-class options_obj(object):
+    class options_obj(object):
         def __init__(self):
             self._replica_install = False
             self.dnssec_master = False  # future unknown
@@ -249,54 +252,51 @@ class options_obj(object):
             for name in self.__dict__:
                 yield self, name
 
+    options = options_obj()
+    installer = options
 
-options = options_obj()
-installer = options
+    # ServerMasterInstall
+    options.add_sids = True
+    options.add_agents = False
 
-# ServerMasterInstall
-options.add_sids = True
-options.add_agents = False
+    # Installable
+    options.uninstalling = False
 
+    # ServerInstallInterface
+    options.description = "Server"
 
-# Installable
-options.uninstalling = False
+    options.kinit_attempts = 1
+    options.fixed_primary = True
+    options.permit = False
+    options.enable_dns_updates = False
+    options.no_krb5_offline_passwords = False
+    options.preserve_sssd = False
+    options.no_sssd = False
 
-# ServerInstallInterface
-options.description = "Server"
+    # ServerMasterInstall
+    options.force_join = False
+    options.servers = None
+    options.no_wait_for_dns = True
+    options.host_password = None
+    options.keytab = None
+    options.setup_ca = True
+    # always run sidgen task and do not allow adding agents on first master
+    options.add_sids = True
+    options.add_agents = False
 
-options.kinit_attempts = 1
-options.fixed_primary = True
-options.permit = False
-options.enable_dns_updates = False
-options.no_krb5_offline_passwords = False
-options.preserve_sssd = False
-options.no_sssd = False
+    # ADTrustInstallInterface
+    # no_msdcs is deprecated
+    options.no_msdcs = False
 
-# ServerMasterInstall
-options.force_join = False
-options.servers = None
-options.no_wait_for_dns = True
-options.host_password = None
-options.keytab = None
-options.setup_ca = True
-# always run sidgen task and do not allow adding agents on first master
-options.add_sids = True
-options.add_agents = False
+    # For pylint
+    options.external_cert_files = None
+    options.dirsrv_cert_files = None
 
-# ADTrustInstallInterface
-# no_msdcs is deprecated
-options.no_msdcs = False
+    # Uninstall
+    options.ignore_topology_disconnect = False
+    options.ignore_last_of_role = False
 
-# For pylint
-options.external_cert_files = None
-options.dirsrv_cert_files = None
-
-# Uninstall
-options.ignore_topology_disconnect = False
-options.ignore_last_of_role = False
-
-
-def api_Backend_ldap2(host_name, setup_ca, connect=False):
+    def api_Backend_ldap2(host_name, setup_ca, connect=False):
         # we are sure we have the configuration file ready.
         cfg = dict(context='installer', confdir=paths.ETC_IPA, in_server=True,
                    host=host_name)
@@ -309,8 +309,7 @@ def api_Backend_ldap2(host_name, setup_ca, connect=False):
         if connect:
             api.Backend.ldap2.connect()
 
-
-def ds_init_info(ansible_log, fstore, domainlevel, dirsrv_config_file,
+    def ds_init_info(ansible_log, fstore, domainlevel, dirsrv_config_file,
                      realm_name, host_name, domain_name, dm_password,
                      idstart, idmax, subject_base, ca_subject,
                      no_hbac_allow, dirsrv_pkcs12_info, no_pkinit):
@@ -341,20 +340,19 @@ def ds_init_info(ansible_log, fstore, domainlevel, dirsrv_config_file,
 
         return ds
 
-
-def ansible_module_get_parsed_ip_addresses(ansible_module,
+    def ansible_module_get_parsed_ip_addresses(ansible_module,
                                                param='ip_addresses'):
         ip_addrs = []
         for ip in ansible_module.params.get(param):
             try:
                 ip_parsed = ipautil.CheckedIPAddress(ip)
             except Exception as e:
-            ansible_module.fail_json(msg="Invalid IP Address %s: %s" % (ip, e))
+                ansible_module.fail_json(
+                    msg="Invalid IP Address %s: %s" % (ip, e))
             ip_addrs.append(ip_parsed)
         return ip_addrs
 
-
-def encode_certificate(cert):
+    def encode_certificate(cert):
         """
         Encode a certificate using base64.
 
@@ -368,13 +366,12 @@ def encode_certificate(cert):
             encoded = encoded.decode('ascii')
         return encoded
 
-
-def decode_certificate(cert):
+    def decode_certificate(cert):
         """
         Decode a certificate using base64.
 
-    It also takes FreeIPA versions into account and returns a IPACertificate
-    for newer IPA versions.
+        It also takes FreeIPA versions into account and returns a
+        IPACertificate for newer IPA versions.
         """
         if hasattr(x509, "IPACertificate"):
             cert = cert.strip()

roles/ipaserver/tasks/install.yml

@@ -65,7 +65,7 @@
     master_password: "{{ ipaserver_master_password | default(omit) }}"
     domain: "{{ ipaserver_domain | default(omit) }}"
     realm: "{{ ipaserver_realm | default(omit) }}"
-    hostname: "{{ ipaserver_hostname | default(ansible_fqdn) }}"
+    hostname: "{{ ipaserver_hostname | default(ansible_facts['fqdn']) }}"
     ca_cert_files: "{{ ipaserver_ca_cert_files | default(omit) }}"
     no_host_dns: "{{ ipaserver_no_host_dns }}"
     pki_config_override: "{{ ipaserver_pki_config_override | default(omit) }}"

roles/ipaserver/tasks/main.yml

@@ -4,9 +4,9 @@
 - name: Import variables specific to distribution
   include_vars: "{{ item }}"
   with_first_found:
-    - "vars/{{ ansible_distribution }}-{{ ansible_distribution_version }}.yml"
-    - "vars/{{ ansible_distribution }}-{{ ansible_distribution_major_version }}.yml"
-    - "vars/{{ ansible_distribution }}.yml"
+    - "vars/{{ ansible_facts['distribution'] }}-{{ ansible_facts['distribution_version'] }}.yml"
+    - "vars/{{ ansible_facts['distribution'] }}-{{ ansible_facts['distribution_major_version'] }}.yml"
+    - "vars/{{ ansible_facts['distribution'] }}.yml"
     - "vars/default.yml"
 
 - name: Install IPA server

setup.cfg

@@ -31,3 +31,40 @@ per-file-ignores =
 [pydocstyle]
 inherit = false
 ignore = D1,D212,D203
+
+[pylint.MASTER]
+disable =
+    c-extension-no-member,
+    missing-module-docstring,
+    missing-class-docstring,
+    missing-function-docstring,
+    wrong-import-order,
+    ungrouped-imports,
+    wrong-import-position,
+    protected-access,
+    no-name-in-module,
+    too-many-arguments,
+    too-many-statements,
+    too-many-lines,
+    raise-missing-from,
+    duplicate-code,
+    broad-except,
+    too-many-branches,
+    too-many-locals,
+    fixme
+
+[pylint.BASIC]
+good-names = ex, i, j, k, Run, _, e, x, dn, cn, ip, os, unicode
+
+[pylint.IMPORTS]
+ignored-modules =
+    ansible.module_utils.ansible_freeipa_module,
+    ipalib, ipalib.config, ipalib.constants, ipalib.krb_utils, ipalib.errors,
+    ipapython.ipautil, ipapython.dn, ipapython.version, ipapython.dnsutil,
+    ipaplatform.paths
+
+[pylint.REFACTORING]
+max-nested-blocks = 9
+
+[pylint.FORMAT]
+max-line-length = 80

tests/ansible.cfg

@@ -3,3 +3,4 @@ roles_path = ../roles:~/.ansible/roles:/usr/share/ansible/roles:/etc/ansible/rol
 library = ../plugins/modules:~/.ansible/plugins/modules:/usr/share/ansible/plugins/modules
 module_utils = ../plugins/module_utils:~/.ansible/plugins/module_utils:/usr/share/ansible/plugins/module_utils
 host_key_checking = false
+inject_facts_as_vars = false

tests/automember/test_automember.yml

@@ -0,0 +1,311 @@
+---
+- name: Test automember
+  hosts: ipaserver
+  become: true
+
+  tasks:
+
+  # CLEANUP TEST ITEMS
+
+  - name: Ensure group testgroup is absent
+    ipagroup:
+      ipaadmin_password: SomeADMINpassword
+      name: testgroup
+      state: absent
+
+  - name: Ensure hostgroup testhostgroup is absent
+    ipahostgroup:
+      ipaadmin_password: SomeADMINpassword
+      name: testhostgroup
+      state: absent
+
+  - name: Ensure group automember rule testgroup is absent
+    ipaautomember:
+      ipaadmin_password: SomeADMINpassword
+      name: testgroup
+      state: absent
+      automember_type: group
+
+  - name: Ensure hostgroup automember rule testhostgroup is absent
+    ipaautomember:
+      ipaadmin_password: SomeADMINpassword
+      name: testhostgroup
+      state: absent
+      automember_type: hostgroup
+
+  # CREATE TEST ITEMS
+
+  # TESTS
+  - name: Ensure testgroup group is present
+    ipagroup:
+      ipaadmin_password: SomeADMINpassword
+      name: testgroup
+
+  - name: Ensure testhostgroup hostgroup is present
+    ipahostgroup:
+      ipaadmin_password: SomeADMINpassword
+      name: testhostgroup
+
+  - name: Ensure testgroup group automember rule is present
+    ipaautomember:
+      ipaadmin_password: SomeADMINpassword
+      name: testgroup
+      description: testgroup automember rule.
+      automember_type: group
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure testgroup group automember rule is present again
+    ipaautomember:
+      ipaadmin_password: SomeADMINpassword
+      name: testgroup
+      description: testgroup automember rule.
+      automember_type: group
+    register: result
+    failed_when: result.changed or result.failed
+
+  - name: Change testgroup group automember rule description
+    ipaautomember:
+      ipaadmin_password: SomeADMINpassword
+      name: testgroup
+      description: testgroup automember rule description.
+      automember_type: group
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure testgroup group automember rule has conditions
+    ipaautomember:
+      ipaadmin_password: SomeADMINpassword
+      name: testgroup
+      automember_type: group
+      inclusive:
+        - key: 'uid'
+          expression: 'uid'
+        - key: 'uidnumber'
+          expression: 'uidnumber'
+      exclusive:
+        - key: 'uid'
+          expression: 'uid'
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure testgroup group automember rule has conditions again
+    ipaautomember:
+      ipaadmin_password: SomeADMINpassword
+      name: testgroup
+      automember_type: group
+      inclusive:
+        - key: 'uid'
+          expression: 'uid'
+        - key: 'uidnumber'
+          expression: 'uidnumber'
+      exclusive:
+        - key: 'uid'
+          expression: 'uid'
+    register: result
+    failed_when: result.changed or result.failed
+
+  - name: Add testgroup group automember rule member condition
+    ipaautomember:
+      ipaadmin_password: SomeADMINpassword
+      name: testgroup
+      automember_type: group
+      action: member
+      inclusive:
+        - key: 'manager'
+          expression: 'uid=mscott'
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure testgroup group automember rule has conditions
+    ipaautomember:
+      ipaadmin_password: SomeADMINpassword
+      name: testgroup
+      automember_type: group
+      inclusive:
+        - key: 'uid'
+          expression: 'uid'
+        - key: 'uidnumber'
+          expression: 'uidnumber'
+        - key: 'manager'
+          expression: 'uid=mscott'
+      exclusive:
+        - key: 'uid'
+          expression: 'uid'
+    register: result
+    failed_when: result.changed or result.failed
+
+  - name: Remove testgroup group automember rule member condition
+    ipaautomember:
+      ipaadmin_password: SomeADMINpassword
+      name: testgroup
+      automember_type: group
+      action: member
+      state: absent
+      inclusive:
+        - key: 'manager'
+          expression: 'uid=mscott'
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure testgroup group automember rule has conditions again
+    ipaautomember:
+      ipaadmin_password: SomeADMINpassword
+      name: testgroup
+      automember_type: group
+      inclusive:
+        - key: 'uid'
+          expression: 'uid'
+        - key: 'uidnumber'
+          expression: 'uidnumber'
+      exclusive:
+        - key: 'uid'
+          expression: 'uid'
+    register: result
+    failed_when: result.changed or result.failed
+
+  - name: Ensure testhostgroup hostgroup automember rule is present
+    ipaautomember:
+      ipaadmin_password: SomeADMINpassword
+      name: testhostgroup
+      description: testhostgroup automember rule
+      automember_type: hostgroup
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure testhostgroup hostgroup automember rule is present again
+    ipaautomember:
+      ipaadmin_password: SomeADMINpassword
+      name: testhostgroup
+      description: testhostgroup automember rule
+      automember_type: hostgroup
+    register: result
+    failed_when: result.changed or result.failed
+
+  - name: Change testhostgroup hostgroup automember rule description
+    ipaautomember:
+      ipaadmin_password: SomeADMINpassword
+      name: testhostgroup
+      description: testhostgroup test automember rule
+      automember_type: hostgroup
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure testhostgroup hostgroup automember rule has conditions
+    ipaautomember:
+      ipaadmin_password: SomeADMINpassword
+      name: testhostgroup
+      automember_type: hostgroup
+      inclusive:
+        - key: 'description'
+          expression: 'description'
+        - key: 'description'
+          expression: 'description'
+      exclusive:
+        - key: 'cn'
+          expression: 'cn'
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure testhostgroup hostgroup automember rule has conditions again
+    ipaautomember:
+      ipaadmin_password: SomeADMINpassword
+      name: testhostgroup
+      automember_type: hostgroup
+      inclusive:
+        - key: 'description'
+          expression: 'description'
+        - key: 'description'
+          expression: 'description'
+      exclusive:
+        - key: 'cn'
+          expression: 'cn'
+    register: result
+    failed_when: result.changed or result.failed
+
+  - name: Add testhostgroup hostgroup automember rule member condition
+    ipaautomember:
+      ipaadmin_password: SomeADMINpassword
+      name: testhostgroup
+      automember_type: hostgroup
+      action: member
+      inclusive:
+        - key: 'fqdn'
+          expression: '.*.domain.com'
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure testhostgroup hostgroup automember rule has conditions
+    ipaautomember:
+      ipaadmin_password: SomeADMINpassword
+      name: testhostgroup
+      automember_type: hostgroup
+      inclusive:
+        - key: 'description'
+          expression: 'description'
+        - key: 'description'
+          expression: 'description'
+        - key: 'fqdn'
+          expression: '.*.domain.com'
+      exclusive:
+        - key: 'cn'
+          expression: 'cn'
+    register: result
+    failed_when: result.changed or result.failed
+
+  - name: Remove testhostgroup hostgroup automember rule member condition
+    ipaautomember:
+      ipaadmin_password: SomeADMINpassword
+      name: testhostgroup
+      automember_type: hostgroup
+      action: member
+      state: absent
+      inclusive:
+        - key: 'fqdn'
+          expression: '.*.domain.com'
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure testhostgroup hostgroup automember rule has conditions
+    ipaautomember:
+      ipaadmin_password: SomeADMINpassword
+      name: testhostgroup
+      automember_type: hostgroup
+      inclusive:
+        - key: 'description'
+          expression: 'description'
+        - key: 'description'
+          expression: 'description'
+      exclusive:
+        - key: 'cn'
+          expression: 'cn'
+    register: result
+    failed_when: result.changed or result.failed
+
+  # CLEANUP TEST ITEMS
+
+  - name: Ensure group testgroup is absent
+    ipagroup:
+      ipaadmin_password: SomeADMINpassword
+      name: testgroup
+      state: absent
+
+  - name: Ensure hostgroup testhostgroup is absent
+    ipahostgroup:
+      ipaadmin_password: SomeADMINpassword
+      name: testhostgroup
+      state: absent
+
+  - name: Ensure group automember rule testgroup is absent
+    ipaautomember:
+      ipaadmin_password: SomeADMINpassword
+      automember_type: group
+      name: testgroup
+      state: absent
+
+  - name: Ensure hostgroup automember rule testhostgroup is absent
+    ipaautomember:
+      ipaadmin_password: SomeADMINpassword
+      automember_type: hostgroup
+      name: testhostgroup
+      state: absent