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@v2
+  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

@@ -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-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**
 
@@ -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,7 +22,20 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 
+__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"]
+
 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
@@ -33,6 +46,9 @@ 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:
@@ -41,7 +57,7 @@ except ImportError:
         # FreeIPA releases.
         import re
 
-    class version:
+        class version:  # pylint: disable=invalid-name, too-few-public-methods
             @staticmethod
             def parse(version_str):
                 """
@@ -49,7 +65,7 @@ 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
@@ -65,6 +81,7 @@ 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
@@ -89,11 +106,9 @@ try:
     except ImportError:
         from collections import Mapping  # noqa
 
-
     if six.PY3:
         unicode = str
 
-
     def valid_creds(module, principal):  # noqa
         """Get valid credentials matching the princial, try GSSAPI first."""
         if "KRB5CCNAME" in os.environ:
@@ -131,7 +146,6 @@ def valid_creds(module, principal):  # noqa
             return True
         return False
 
-
     def temp_kinit(principal, password):
         """Kinit with password using a temporary ccache."""
         if not password:
@@ -150,7 +164,6 @@ def temp_kinit(principal, password):
         os.environ["KRB5CCNAME"] = ccache_name
         return ccache_dir, ccache_name
 
-
     def temp_kdestroy(ccache_dir, ccache_name):
         """Destroy temporary ticket and remove temporary ccache."""
         if ccache_name is not None:
@@ -159,7 +172,6 @@ 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):
         """
         Initialize IPA API with the provided context.
@@ -173,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'
 
@@ -188,27 +202,22 @@ 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):
         """Return if command exists in command list."""
         return command in api.Command
 
-
     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):
         """
         Compare the installed IPA version against a requested version.
@@ -224,10 +233,10 @@ 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):
         """
@@ -249,7 +258,8 @@ 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):
         accepted_date_formats = [
@@ -261,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
                 )
@@ -308,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)
@@ -330,32 +355,36 @@ def compare_args_ipa(module, args, ipa):  # noqa
                         return False
         return True
 
-
     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
 
+        return value
 
     def module_params_get(module, name):
         return _afm_convert(module.params.get(name))
 
-
     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."""
-    # The user list is None, therefore the parameter should not be touched
+        """
+        Generate the lists for the addition and removal of members.
+
+        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 [], []
 
@@ -364,6 +393,37 @@ 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.
+
+        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):
         """
@@ -379,7 +439,6 @@ def encode_certificate(cert):
             encoded = encoded.decode('ascii')
         return encoded
 
-
     def load_cert_from_str(cert):
         cert = cert.strip()
         if not cert.startswith("-----BEGIN CERTIFICATE-----"):
@@ -393,17 +452,14 @@ 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):
         if not isinstance(port, int):
             return False
@@ -413,7 +469,6 @@ 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:
@@ -422,7 +477,6 @@ def is_ip_address(ipaddr):
             return False
         return True
 
-
     def is_ip_network_address(ipaddr):
         """Test if given IP address is a valid IPv4 or IPv6 address."""
         try:
@@ -431,7 +485,6 @@ def is_ip_network_address(ipaddr):
             return False
         return True
 
-
     def is_ipv4_addr(ipaddr):
         """Test if given IP address is a valid IPv4 address."""
         try:
@@ -440,7 +493,6 @@ def is_ipv4_addr(ipaddr):
             return False
         return True
 
-
     def is_ipv6_addr(ipaddr):
         """Test if given IP address is a valid IPv6 address."""
         try:
@@ -449,7 +501,6 @@ def is_ipv6_addr(ipaddr):
             return False
         return True
 
-
     def exit_raw_json(module, **kwargs):
         """
         Print the raw parameters in JSON format, without masking.
@@ -469,7 +520,6 @@ def exit_raw_json(module, **kwargs):
         print(jsonify(kwargs))
         sys.exit(0)
 
-
     class AnsibleFreeIPAParams(Mapping):
         def __init__(self, ansible_module):
             self.mapping = ansible_module.params
@@ -477,7 +527,8 @@ class AnsibleFreeIPAParams(Mapping):
 
         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):
@@ -493,7 +544,6 @@ class AnsibleFreeIPAParams(Mapping):
         def __getattr__(self, name):
             return self.get(name)
 
-
     class FreeIPABaseModule(AnsibleModule):
         """
         Base class for FreeIPA Ansible modules.
@@ -508,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):
 
@@ -528,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()
@@ -562,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)
@@ -592,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
@@ -626,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)
                     )
@@ -639,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."""
@@ -724,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."""
@@ -738,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 = {}
@@ -435,7 +431,7 @@ def main():
             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':
@@ -450,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/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
 

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
 
 
@@ -386,8 +386,8 @@ def main():
                                          **exit_args)
 
             # Execute commands
-            for name, command, args in commands:
-                api_command(ansible_module, command, name, args)
+            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:
@@ -1379,7 +1377,7 @@ def define_commands_for_present_state(module, zone_name, entry, res_find):
                     _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:
@@ -1511,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

@@ -212,9 +212,9 @@ from ansible.module_utils.ansible_freeipa_module import (
     FreeIPABaseModule,
     is_ip_address,
     is_ip_network_address,
-    is_valid_port
+    is_valid_port,
+    ipalib_errors
 )  # noqa: E402
-import ipalib.errors
 import netaddr
 import six
 
@@ -263,22 +263,18 @@ class DNSZoneModule(FreeIPABaseModule):
         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:
@@ -298,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 = (
@@ -310,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_ip_address(ip_address)):
+                if not is_ip_address(ip_address):
                     self.fail_json(
                         msg="Invalid IP for DNS forwarder: %s" % ip_address
                     )
@@ -334,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)
@@ -364,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.
 
@@ -404,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):
@@ -414,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:
@@ -492,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:
@@ -635,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",
@@ -520,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
 
 

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
 
 

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:]

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:
@@ -487,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/ipapermission.py

@@ -277,10 +277,8 @@ def main():
             ansible_module.fail_json(
                 msg="Only one permission can be added at a time.")
         if action == "member":
-            invalid = ["right", "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"]
 
@@ -299,13 +297,12 @@ def main():
     if state == "absent":
         if len(names) < 1:
             ansible_module.fail_json(msg="No name given.")
-        invalid = ["right",
-                   "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 != "member":
-            invalid += ["attrs"]
+            invalid += ["right", "attrs", "memberof",
+                        "extra_target_filter", "rawfilter"]
 
     for x in invalid:
         if vars()[x] is not None:
@@ -317,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
@@ -359,16 +361,31 @@ def main():
                         ansible_module.fail_json(
                             msg="No permission '%s'" % name)
 
-                    # attrs
-                    if attrs is not None:
-                        _attrs = list(set(list(res_find["attrs"]) + attrs))
-                        if len(_attrs) > len(res_find["attrs"]):
-                            commands.append([name, "permission_mod",
-                                             {"attrs": _attrs}])
+                    member_attrs = {}
+                    check_members = {
+                        "attrs": attrs,
+                        "memberof": memberof,
+                        "ipapermright": right,
+                        "ipapermtargetfilter": rawfilter,
+                        "extratargetfilter": extra_target_filter,
+                        # subtree member management is currently disabled.
+                        # "ipapermlocation": subtree,
+                    }
+
+                    for _member, _member_change in check_members.items():
+                        if _member_change is not None:
+                            _res_list = res_find[_member]
+                            _new_set = set(_res_list + _member_change)
+                            if _new_set != set(_res_list):
+                                member_attrs[_member] = list(_new_set)
+
+                    if member_attrs:
+                        commands.append([name, "permission_mod", member_attrs])
 
                 else:
                     ansible_module.fail_json(
                         msg="Unknown action '%s'" % action)
+
             elif state == "renamed":
                 if action == "permission":
                     # Generate args
@@ -393,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:
@@ -403,20 +421,26 @@ def main():
                         ansible_module.fail_json(
                             msg="No permission '%s'" % name)
 
-                    # attrs
-                    if attrs is not None:
-                        # New attribute list (remove given ones from find
-                        # result)
-                        # Make list with unique entries
-                        _attrs = list(set(res_find["attrs"]) - set(attrs))
-                        if len(_attrs) < 1:
-                            ansible_module.fail_json(
-                                msg="At minimum one attribute is needed.")
+                    member_attrs = {}
+                    check_members = {
+                        "attrs": attrs,
+                        "memberof": memberof,
+                        "ipapermright": right,
+                        "ipapermtargetfilter": rawfilter,
+                        "extratargetfilter": extra_target_filter,
+                        # subtree member management is currently disabled.
+                        # "ipapermlocation": subtree,
+                    }
 
-                        # Entries New number of attributes is smaller
-                        if len(_attrs) < len(res_find["attrs"]):
-                            commands.append([name, "permission_mod",
-                                             {"attrs": _attrs}])
+                    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)

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:

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
 
 

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

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
 
 

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

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,

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
 
 

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

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:

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
@@ -320,8 +321,8 @@ 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, NotFound
+    gen_add_del_lists, compare_args_ipa, module_params_get, exit_raw_json, \
+    ipalib_errors
 
 
 def find_vault(module, name, username, service, shared):
@@ -348,9 +349,9 @@ 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")
 
@@ -443,12 +444,12 @@ def data_storage_args(vault_type, args, data, password, password_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 = ['datafile_out']
@@ -491,11 +492,11 @@ def check_parameters(module, state, action, description, username, service,
                     "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
@@ -579,7 +580,7 @@ def get_stored_data(module, res_find, args):
     # retrieve vault stored data
     try:
         result = api_command(module, 'vault_retrieve', name, pwdargs)
-    except NotFound:
+    except ipalib_errors.NotFound:
         return None
 
     return result['result'].get('data')
@@ -757,9 +758,7 @@ def main():
 
             # 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
 
             # Create command
@@ -991,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.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/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

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,16 +45,25 @@ __all__ = ["gssapi", "version", "ipadiscovery", "api", "errors", "x509",
            "configure_firefox", "sync_time", "check_ldap_conf",
            "sssd_enable_ifp"]
 
+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:
     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_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):
         def __init__(self):
             pass
@@ -76,14 +85,14 @@ 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
@@ -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

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

@@ -46,20 +46,26 @@ __all__ = ["contextlib", "dnsexception", "dnsresolver", "dnsreversename",
            "dnsname", "kernel_keyring", "krbinstance"]
 
 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 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_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:
         # IPA version >= 4.6
 
@@ -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,23 +145,19 @@ if NUM_VERSION >= 40600:
             from ipaserver.install import ntpinstance
             time_service = "ntpd"
 
-
     else:
         # IPA version < 4.6
 
         raise Exception("freeipa version '%s' is too old" % VERSION)
 
-
     logger = logging.getLogger("ipa-server-install")
 
-
     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):
         sys.stdout = f
@@ -161,7 +166,6 @@ def redirect_stdout(f):
         finally:
             sys.stdout = sys.__stdout__
 
-
     class AnsibleModuleLog():
         def __init__(self, module):
             self.module = module
@@ -195,7 +199,6 @@ class AnsibleModuleLog():
             self.module.debug(msg)
             # self.module.warn(msg)
 
-
     class installer_obj(object):
         def __init__(self):
             # CompatServerReplicaInstall
@@ -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,14 +247,14 @@ 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
 
@@ -268,7 +272,6 @@ options.add_agents = False
     options.subject_base = None
     options.ca_subject = None
 
-
     def gen_env_boostrap_finalize_core(etc_ipa, default_config):
         env = Env()
         # env._bootstrap(context='installer', confdir=paths.ETC_IPA, log=None)
@@ -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):
         # 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():
         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,7 +337,6 @@ def gen_ReplicaConfig():
 
         return config
 
-
     def replica_ds_init_info(ansible_log,
                              config, options, ca_is_configured, remote_api,
                              ds_ca_subject, ca_file,
@@ -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,7 +402,6 @@ def replica_ds_init_info(ansible_log,
 
         return ds
 
-
     def ansible_module_get_parsed_ip_addresses(ansible_module,
                                                param='ip_addresses'):
         ip_addrs = []
@@ -405,11 +409,11 @@ def ansible_module_get_parsed_ip_addresses(ansible_module,
             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):
         ldapuri = 'ldaps://%s' % ipautil.format_netloc(master_host_name)
         xmlrpc_uri = 'https://{}/ipa/xml'.format(

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

@@ -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,22 +41,30 @@ __all__ = ["IPAChangeConf", "certmonger", "sysrestore", "root_logger",
            "check_available_memory"]
 
 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 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_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:
         # IPA version >= 4.5
 
@@ -172,17 +180,14 @@ else:
 
         raise Exception("freeipa version '%s' is too old" % VERSION)
 
-
     logger = logging.getLogger("ipa-server-install")
 
-
     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):
         sys.stdout = f
@@ -191,7 +196,6 @@ def redirect_stdout(f):
         finally:
             sys.stdout = sys.__stdout__
 
-
     class AnsibleModuleLog():
         def __init__(self, module):
             self.module = module
@@ -225,7 +229,6 @@ class AnsibleModuleLog():
             self.module.debug(msg)
             # self.module.warn(msg)
 
-
     class options_obj(object):
         def __init__(self):
             self._replica_install = False
@@ -249,7 +252,6 @@ class options_obj(object):
             for name in self.__dict__:
                 yield self, name
 
-
     options = options_obj()
     installer = options
 
@@ -257,7 +259,6 @@ installer = options
     options.add_sids = True
     options.add_agents = False
 
-
     # Installable
     options.uninstalling = False
 
@@ -295,7 +296,6 @@ options.dirsrv_cert_files = None
     options.ignore_topology_disconnect = False
     options.ignore_last_of_role = 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,
@@ -309,7 +309,6 @@ 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,
                      realm_name, host_name, domain_name, dm_password,
                      idstart, idmax, subject_base, ca_subject,
@@ -341,7 +340,6 @@ def ds_init_info(ansible_log, fstore, domainlevel, dirsrv_config_file,
 
         return ds
 
-
     def ansible_module_get_parsed_ip_addresses(ansible_module,
                                                param='ip_addresses'):
         ip_addrs = []
@@ -349,11 +347,11 @@ def ansible_module_get_parsed_ip_addresses(ansible_module,
             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):
         """
         Encode a certificate using base64.
@@ -368,13 +366,12 @@ def encode_certificate(cert):
             encoded = encoded.decode('ascii')
         return encoded
 
-
     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

tests/azure/azure-pipelines.yml

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

tests/azure/build-containers.yml

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

tests/azure/templates/build_container.yml

@@ -15,7 +15,7 @@ jobs:
     inputs:
       versionSpec: '3.6'
 
-  - script: python -m pip install --upgrade pip setuptools wheel
+  - script: python -m pip install --upgrade pip setuptools wheel ansible
     displayName: Install tools
 
   - script: pip install molecule[docker]
@@ -23,6 +23,8 @@ jobs:
 
   - script: molecule create -s ${{ parameters.build_scenario_name }}
     displayName: Create test container
+    env:
+      ANSIBLE_LIBRARY: ./molecule
 
   - script: |
       docker stop ${{ parameters.build_scenario_name }}

tests/azure/templates/playbook_tests.yml

@@ -34,6 +34,9 @@ jobs:
         "ansible${{ parameters.ansible_version }}"
     displayName: Install molecule and Ansible
 
+  - script: |
+      ansible-galaxy collection install community.docker
+
   - script: pip install -r requirements-tests.txt
     displayName: Install dependencies
 
@@ -44,6 +47,8 @@ jobs:
       cp -a plugins/module_utils/* ~/.ansible/module_utils
       molecule create -s ${{ parameters.scenario }}
     displayName: Setup test container
+    env:
+      ANSIBLE_LIBRARY: ./molecule
 
   - script: |
       pytest \

tests/azure/templates/pytest_tests.yml

@@ -26,6 +26,9 @@ jobs:
         "ansible${{ parameters.ansible_version }}"
     displayName: Install molecule and Ansible
 
+  - script: |
+      ansible-galaxy collection install community.docker
+
   - script: pip install -r requirements-tests.txt
     displayName: Install dependencies
 
@@ -36,6 +39,8 @@ jobs:
       cp -a plugins/module_utils/* ~/.ansible/module_utils
       molecule create -s ${{ parameters.scenario }}
     displayName: Setup test container
+    env:
+      ANSIBLE_LIBRARY: ./molecule
 
   - script: |
       pytest \

tests/config/test_config.yml

@@ -110,84 +110,84 @@
       ipaadmin_password: SomeADMINpassword
       emaildomain: somedomain.test
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: Ensure the default e-mail domain is somedomain.test, again.
     ipaconfig:
       ipaadmin_password: SomeADMINpassword
       emaildomain: somedomain.test
     register: result
-    failed_when: result.changed
+    failed_when: result.changed or result.failed
 
   - name: set default shell to '/bin/someshell'
     ipaconfig:
       ipaadmin_password: SomeADMINpassword
       defaultshell: /bin/someshell
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: set default shell to '/bin/someshell', again.
     ipaconfig:
       ipaadmin_password: SomeADMINpassword
       defaultshell: /bin/someshell
     register: result
-    failed_when: result.changed
+    failed_when: result.changed or result.failed
 
   - name: set default group
     ipaconfig:
       ipaadmin_password: SomeADMINpassword
       defaultgroup: somedefaultgroup
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
-  - name: set default group
+  - name: set default group, again
     ipaconfig:
       ipaadmin_password: SomeADMINpassword
       defaultgroup: somedefaultgroup
     register: result
-    failed_when: result.changed
+    failed_when: result.changed or result.failed
 
   - name: set default home directory
     ipaconfig:
       ipaadmin_password: SomeADMINpassword
       homedirectory: /Users
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
-  - name: set default home directory
+  - name: set default home directory, again
     ipaconfig:
       ipaadmin_password: SomeADMINpassword
       homedirectory: /Users
     register: result
-    failed_when: result.changed
+    failed_when: result.changed or result.failed
 
   - name: set pac-type
     ipaconfig:
       ipaadmin_password: SomeADMINpassword
       pac_type: "nfs:NONE"
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: set pac-type, again.
     ipaconfig:
       ipaadmin_password: SomeADMINpassword
       pac_type: "nfs:NONE"
     register: result
-    failed_when: result.changed
+    failed_when: result.changed or result.failed
 
   - name: set maxusername to 33
     ipaconfig:
       ipaadmin_password: SomeADMINpassword
       maxusername: 33
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: set maxusername to 33, again.
     ipaconfig:
       ipaadmin_password: SomeADMINpassword
       maxusername: 33
     register: result
-    failed_when: result.changed
+    failed_when: result.changed or result.failed
 
   - name: set maxhostname to 77
     block:
@@ -195,13 +195,13 @@
           ipaadmin_password: SomeADMINpassword
           maxhostname: 77
         register: result
-        failed_when: not result.changed
+        failed_when: not result.changed or result.failed
 
       - ipaconfig:
           ipaadmin_password: SomeADMINpassword
           maxhostname: 77
         register: result
-        failed_when: result.changed
+        failed_when: result.changed or result.failed
     when: ipa_version is version('4.8.0', '>=')
 
   - name: set pwdexpnotify to 17
@@ -209,126 +209,126 @@
       ipaadmin_password: SomeADMINpassword
       pwdexpnotify: 17
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: set pwdexpnotify to 17, again
     ipaconfig:
       ipaadmin_password: SomeADMINpassword
       pwdexpnotify: 17
     register: result
-    failed_when: result.changed
+    failed_when: result.changed or result.failed
 
   - name: set searchrecordslimit to -1
     ipaconfig:
       ipaadmin_password: SomeADMINpassword
       searchrecordslimit: -1
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: set searchrecordslimit to -1, again.
     ipaconfig:
       ipaadmin_password: SomeADMINpassword
       searchrecordslimit: -1
     register: result
-    failed_when: result.changed
+    failed_when: result.changed or result.failed
 
   - name: set searchtimelimit to 12345
     ipaconfig:
       ipaadmin_password: SomeADMINpassword
       searchtimelimit: 12345
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: set searchtimelimit to 12345, again.
     ipaconfig:
       ipaadmin_password: SomeADMINpassword
       searchtimelimit: 12345
     register: result
-    failed_when: result.changed
+    failed_when: result.changed or result.failed
 
   - name: change enable_migration
     ipaconfig:
       ipaadmin_password: SomeADMINpassword
       enable_migration: '{{ not previousconfig.config.enable_migration }}'
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: change enable_migration, again
     ipaconfig:
       ipaadmin_password: SomeADMINpassword
       enable_migration: '{{ not previousconfig.config.enable_migration }}'
     register: result
-    failed_when: result.changed
+    failed_when: result.changed or result.failed
 
   - name: set configstring to AllowNThash
     ipaconfig:
       ipaadmin_password: SomeADMINpassword
       configstring: AllowNThash
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: set configstring to AllowNThash, again.
     ipaconfig:
       ipaadmin_password: SomeADMINpassword
       configstring: AllowNThash
     register: result
-    failed_when: result.changed
+    failed_when: result.changed or result.failed
 
   - name: set selinuxusermaporder
     ipaconfig:
       ipaadmin_password: SomeADMINpassword
       selinuxusermaporder: 'user_u:s0$staff_u:s0-s0:c0.c1023$sysadm_u:s0-s0:c0.c1023$unconfined_u:s0-s0:c0.c1023'
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: set selinuxusermaporder, again
     ipaconfig:
       ipaadmin_password: SomeADMINpassword
       selinuxusermaporder: 'user_u:s0$staff_u:s0-s0:c0.c1023$sysadm_u:s0-s0:c0.c1023$unconfined_u:s0-s0:c0.c1023'
     register: result
-    failed_when: result.changed
+    failed_when: result.changed or result.failed
 
   - name: set selinuxusermapdefault
     ipaconfig:
       ipaadmin_password: SomeADMINpassword
       selinuxusermapdefault: 'user_u:s0'
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: set selinuxusermapdefault, again
     ipaconfig:
       ipaadmin_password: SomeADMINpassword
       selinuxusermapdefault: 'user_u:s0'
     register: result
-    failed_when: result.changed
+    failed_when: result.changed or result.failed
 
   - name: set groupsearch to `description`
     ipaconfig:
       ipaadmin_password: SomeADMINpassword
       groupsearch: description
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: set groupsearch to `gidNumber`, again
     ipaconfig:
       ipaadmin_password: SomeADMINpassword
       groupsearch: description
     register: result
-    failed_when: result.changed
+    failed_when: result.changed or result.failed
 
   - name: set usersearch to `uidNumber`
     ipaconfig:
       ipaadmin_password: SomeADMINpassword
       usersearch: uidNumber
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: set usersearch to `uidNumber`, again
     ipaconfig:
       ipaadmin_password: SomeADMINpassword
       usersearch: uidNumber
     register: result
-    failed_when: result.changed
+    failed_when: result.changed or result.failed
 
   - name: reset changed fields
     ipaconfig:
@@ -354,7 +354,7 @@
       domain_resolution_order: '{{previousconfig.config.domain_resolution_order | default(omit)}}'
       ca_renewal_master_server: '{{previousconfig.config.ca_renewal_master_server | default(omit)}}'
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: reset maxhostname
     block:
@@ -387,7 +387,7 @@
       domain_resolution_order: '{{previousconfig.config.domain_resolution_order | default(omit)}}'
       ca_renewal_master_server: '{{previousconfig.config.ca_renewal_master_server | default(omit)}}'
     register: result
-    failed_when: result.changed
+    failed_when: result.changed or result.failed
 
   - name: reset maxhostname
     block:

tests/dnsconfig/test_dnsconfig.yml

@@ -53,7 +53,7 @@
       forward_policy: only
       allow_sync_ptr: yes
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: Set dnsconfig, with the same values.
     ipadnsconfig:
@@ -66,7 +66,7 @@
       forward_policy: only
       allow_sync_ptr: yes
     register: result
-    failed_when: result.changed
+    failed_when: result.changed or result.failed
 
   - name: Ensure forwarder is absent.
     ipadnsconfig:
@@ -75,7 +75,7 @@
         - ip_address: 8.8.8.8
       state: absent
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: Ensure forwarder is absent, again.
     ipadnsconfig:
@@ -84,63 +84,63 @@
         - ip_address: 8.8.8.8
       state: absent
     register: result
-    failed_when: result.changed
+    failed_when: result.changed or result.failed
 
   - name: Disable global forwarders.
     ipadnsconfig:
       ipaadmin_password: SomeADMINpassword
       forward_policy: none
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: Disable global forwarders, again.
     ipadnsconfig:
       ipaadmin_password: SomeADMINpassword
       forward_policy: none
     register: result
-    failed_when: result.changed
+    failed_when: result.changed or result.failed
 
   - name: Re-enable global forwarders.
     ipadnsconfig:
       ipaadmin_password: SomeADMINpassword
       forward_policy: first
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: Re-enable global forwarders, again.
     ipadnsconfig:
       ipaadmin_password: SomeADMINpassword
       forward_policy: first
     register: result
-    failed_when: result.changed
+    failed_when: result.changed or result.failed
 
   - name: Disable PTR record synchronization.
     ipadnsconfig:
       ipaadmin_password: SomeADMINpassword
       allow_sync_ptr: no
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: Disable PTR record synchronization, again.
     ipadnsconfig:
       ipaadmin_password: SomeADMINpassword
       allow_sync_ptr: no
     register: result
-    failed_when: result.changed
+    failed_when: result.changed or result.failed
 
   - name: Re-enable PTR record synchronization.
     ipadnsconfig:
       ipaadmin_password: SomeADMINpassword
       allow_sync_ptr: yes
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: Re-enable PTR record synchronization, again.
     ipadnsconfig:
       ipaadmin_password: SomeADMINpassword
       allow_sync_ptr: yes
     register: result
-    failed_when: result.changed
+    failed_when: result.changed or result.failed
 
   - name: Ensure all forwarders are absent.
     ipadnsconfig:
@@ -152,7 +152,7 @@
           port: 53
       state: absent
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
 
   - name: Ensure all forwarders are absent, again.
@@ -165,7 +165,7 @@
           port: 53
       state: absent
     register: result
-    failed_when: result.changed
+    failed_when: result.changed or result.failed
 
   # Cleanup.
   - name: Ensure forwarders are absent.

tests/dnsforwardzone/test_dnsforwardzone.yml

@@ -23,7 +23,7 @@
       forwardpolicy: first
       skip_overlap_check: true
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: ensure forwardzone example.com is present again
     ipadnsforwardzone:
@@ -35,7 +35,7 @@
       forwardpolicy: first
       skip_overlap_check: true
     register: result
-    failed_when: result.changed
+    failed_when: result.changed or result.failed
 
   - name: ensure forwardzone example.com has two forwarders
     ipadnsforwardzone:
@@ -49,7 +49,7 @@
       forwardpolicy: first
       skip_overlap_check: true
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: ensure forwardzone example.com has one forwarder again
     ipadnsforwardzone:
@@ -61,7 +61,7 @@
       skip_overlap_check: true
       state: present
     register: result
-    failed_when: result.changed
+    failed_when: result.changed or result.failed
 
   - name: skip_overlap_check can only be set on creation so change nothing
     ipadnsforwardzone:
@@ -73,7 +73,7 @@
       skip_overlap_check: false
       state: present
     register: result
-    failed_when: result.changed
+    failed_when: result.changed or result.failed
 
   - name: ensure forwardzone example.com is absent.
     ipadnsforwardzone:
@@ -81,7 +81,7 @@
       name: example.com
       state: absent
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: ensure forwardzone example.com is absent, again.
     ipadnsforwardzone:
@@ -89,7 +89,7 @@
       name: example.com
       state: absent
     register: result
-    failed_when: result.changed
+    failed_when: result.changed or result.failed
 
   - name: change all the things at once
     ipadnsforwardzone:
@@ -104,7 +104,7 @@
       skip_overlap_check: true
       permission: yes
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: change zone forward policy
     ipadnsforwardzone:
@@ -112,7 +112,7 @@
       name: example.com
       forwardpolicy: first
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: change zone forward policy, again
     ipadnsforwardzone:
@@ -120,13 +120,23 @@
       name: example.com
       forwardpolicy: first
     register: result
-    failed_when: result.changed
+    failed_when: result.changed or result.failed
 
   - name: ensure forwardzone example.com is absent.
     ipadnsforwardzone:
       ipaadmin_password: SomeADMINpassword
       name: example.com
       state: absent
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: ensure forwardzone example.com is absent, again.
+    ipadnsforwardzone:
+      ipaadmin_password: SomeADMINpassword
+      name: example.com
+      state: absent
+    register: result
+    failed_when: result.changed or result.failed
 
   - name: ensure forwardzone example.com is created with minimal args
     ipadnsforwardzone:
@@ -137,7 +147,18 @@
       forwarders:
         - ip_address: 8.8.8.8
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
+
+  - name: ensure forwardzone example.com is created with minimal args, again
+    ipadnsforwardzone:
+      ipaadmin_password: SomeADMINpassword
+      state: present
+      name: example.com
+      skip_overlap_check: true
+      forwarders:
+        - ip_address: 8.8.8.8
+    register: result
+    failed_when: result.changed or result.failed
 
   - name: add a forwarder to any existing ones
     ipadnsforwardzone:
@@ -149,7 +170,19 @@
           port: 8053
       action: member
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
+
+  - name: add a forwarder to any existing ones, again
+    ipadnsforwardzone:
+      ipaadmin_password: SomeADMINpassword
+      state: present
+      name: example.com
+      forwarders:
+        - ip_address: 4.4.4.4
+          port: 8053
+      action: member
+    register: result
+    failed_when: result.changed or result.failed
 
   - name: check the list of forwarders is what we expect
     ipadnsforwardzone:
@@ -162,7 +195,7 @@
         - ip_address: 8.8.8.8
       action: member
     register: result
-    failed_when: result.changed
+    failed_when: result.changed or result.failed
 
   - name: remove a single forwarder
     ipadnsforwardzone:
@@ -173,7 +206,18 @@
         - ip_address: 8.8.8.8
       action: member
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
+
+  - name: remove a single forwarder, again
+    ipadnsforwardzone:
+      ipaadmin_password: SomeADMINpassword
+      state: absent
+      name: example.com
+      forwarders:
+        - ip_address: 8.8.8.8
+      action: member
+    register: result
+    failed_when: result.changed or result.failed
 
   - name: check the list of forwarders is what we expect now
     ipadnsforwardzone:
@@ -185,7 +229,7 @@
           port: 8053
       action: member
     register: result
-    failed_when: result.changed
+    failed_when: result.changed or result.failed
 
   - name: Add a permission for per-forward zone access delegation.
     ipadnsforwardzone:
@@ -194,7 +238,7 @@
       permission: yes
       action: member
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: Add a permission for per-forward zone access delegation, again.
     ipadnsforwardzone:
@@ -203,7 +247,7 @@
       permission: yes
       action: member
     register: result
-    failed_when: result.changed
+    failed_when: result.changed or result.failed
 
   - name: Remove a permission for per-forward zone access delegation.
     ipadnsforwardzone:
@@ -212,7 +256,7 @@
       permission: no
       action: member
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: Remove a permission for per-forward zone access delegation, again.
     ipadnsforwardzone:
@@ -221,7 +265,7 @@
       permission: no
       action: member
     register: result
-    failed_when: result.changed
+    failed_when: result.changed or result.failed
 
   - name: disable the forwarder
     ipadnsforwardzone:
@@ -229,7 +273,7 @@
       name: example.com
       state: disabled
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: disable the forwarder again
     ipadnsforwardzone:
@@ -237,7 +281,7 @@
       name: example.com
       state: disabled
     register: result
-    failed_when: result.changed
+    failed_when: result.changed or result.failed
 
   - name: enable the forwarder
     ipadnsforwardzone:
@@ -245,7 +289,7 @@
       name: example.com
       state: enabled
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: enable the forwarder, again
     ipadnsforwardzone:
@@ -253,7 +297,7 @@
       name: example.com
       state: enabled
     register: result
-    failed_when: result.changed
+    failed_when: result.changed or result.failed
 
   - name: ensure forwardzone example.com is absent again
     ipadnsforwardzone:

tests/dnsrecord/env_vars.yml

@@ -2,9 +2,9 @@
 # Set common vars and facts for test.
 - name: Set IPv4 address prefix.
   set_fact:
-    ipv4_prefix: "{{ ansible_default_ipv4.address.split('.')[:-1] |
+    ipv4_prefix: "{{ ansible_facts['default_ipv4'].address.split('.')[:-1] |
                      join('.') }}"
-    ipv4_reverse_sufix: "{{ ansible_default_ipv4.address.split('.')[:-1] |
+    ipv4_reverse: "{{ ansible_facts['default_ipv4'].address.split('.')[:-1] |
                       reverse |
                       join('.') }}"
 
@@ -14,7 +14,7 @@
     safezone: 'safezone.test'
     zone_ipv6_reverse: "ip6.arpa."
     zone_ipv6_reverse_workaround: "d.f.ip6.arpa."
-    zone_prefix_reverse: "in-addr.arpa"
-    zone_prefix_reverse_24: "{{ ipv4_prefix.split('.')[::-1] | join ('.') }}.in-addr.arpa"
-    zone_prefix_reverse_16: "{{ ipv4_prefix.split('.')[1::-1] | join ('.') }}.in-addr.arpa"
-    zone_prefix_reverse_8: "{{ ipv4_prefix.split('.')[2::-1] | join ('.') }}.in-addr.arpa"
+    zone_prefix_reverse: "in-addr.arpa."
+    zone_prefix_reverse_24: "{{ ipv4_reverse.split('.')[:] | join ('.') }}.in-addr.arpa."
+    zone_prefix_reverse_16: "{{ ipv4_reverse.split('.')[1:] | join ('.') }}.in-addr.arpa."
+    zone_prefix_reverse_8: "{{ ipv4_reverse.split('.')[2:] | join ('.') }}.in-addr.arpa."

tests/dnsrecord/test_compatibility_with_ansible_module.yml

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