Merge pull request #569 from rjeffman/docs_ipaserver_role

Fix display of variables in ipaserver role README.

.github/workflows/docs.yml

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

.github/workflows/lint.yml

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

.pre-commit-config.yaml

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

README-automember.md

@@ -0,0 +1,136 @@
+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
+`state` | The state to ensure. It can be one of `present`, `absent`, default: `present`. | no
+
+
+Authors
+=======
+
+Mark Hahl

README-permission.md

@@ -43,7 +43,7 @@ Example playbook to make sure permission "MyPermission" is present:
 
 ```yaml
 ---
-- name: Playbook to create an IPA permission.
+- name: Playbook to handle IPA permissions
   hosts: ipaserver
   become: yes
 
@@ -56,39 +56,61 @@ Example playbook to make sure permission "MyPermission" is present:
       right: all
 ```
 
-Example playbook to make sure permission "MyPermission" member "privilege" with value "User Administrators" is present:
+
+Example playbook to ensure permission "MyPermission" is present with attr carlicense:
 
 ```yaml
 ---
-- name: Permission add privilege to a permission
+- name: Playbook to handle IPA permissions
   hosts: ipaserver
-  become: true
+  become: yes
 
   tasks:
-  - name: Ensure permission MyPermission is present with the User Administrators privilege present
+  - name: Ensure permission "MyPermission" is present with attr carlicense
     ipapermission:
       ipaadmin_password: SomeADMINpassword
       name: MyPermission
-      privilege: "User Administrators"
+      object_type: host
+      right: all
+      attrs:
+      - carlicense
+```
+
+
+Example playbook to ensure attr gecos is present in permission "MyPermission":
+
+```yaml
+---
+- name: Playbook to handle IPA permissions
+  hosts: ipaserver
+  become: yes
+
+  tasks:
+  - name: Ensure attr gecos is present in permission "MyPermission"
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      name: MyPermission
+      attrs:
+      - gecos
       action: member
 ```
 
 
-Example playbook to make sure permission "MyPermission" member "privilege" with value "User Administrators" is absent:
-
+Example playbook to ensure attr gecos is absent in permission "MyPermission":
 
 ```yaml
 ---
-- name: Permission remove privilege from a permission
+- name: Playbook to handle IPA permissions
   hosts: ipaserver
-  become: true
+  become: yes
 
   tasks:
-  - name: Ensure permission MyPermission is present without the User Administrators privilege
+  - name: Ensure attr gecos is present in permission "MyPermission"
     ipapermission:
       ipaadmin_password: SomeADMINpassword
       name: MyPermission
-      privilege: "User Administrators"
+      attrs:
+      - gecos
       action: member
       state: absent
 ```
@@ -98,27 +120,30 @@ Example playbook to make sure permission "MyPermission" is absent:
 
 ```yaml
 ---
-- name: Playbook to manage IPA permission.
+- name: Playbook to handle IPA permissions
   hosts: ipaserver
   become: yes
 
   tasks:
-  - ipapermission:
+  - name: Ensure permission "MyPermission" is absent
+    ipapermission:
       ipaadmin_password: SomeADMINpassword
       name: MyPermission
       state: absent
 ```
 
+
 Example playbook to make sure permission "MyPermission" is renamed to "MyNewPermission":
 
 ```yaml
 ---
-- name: Playbook to manage IPA permission.
+- name: Playbook to handle IPA permissions
   hosts: ipaserver
   become: yes
 
   tasks:
-  - ipapermission:
+  - name: Ensure permission "MyPermission" is renamed to "MyNewPermission
+    ipapermission:
       ipaadmin_password: SomeADMINpassword
       name: MyPermission
       rename: MyNewPermission
@@ -126,8 +151,6 @@ Example playbook to make sure permission "MyPermission" is renamed to "MyNewPerm
 ```
 
 
-
-
 Variables
 ---------
 
@@ -140,7 +163,7 @@ Variable | Description | Required
 `ipaadmin_password` | The admin password is a string and is required if there is no admin ticket available on the node | no
 `name` \| `cn` | The permission name string. | yes
 `right` \| `ipapermright` | Rights to grant. It can be a list of one or more of `read`, `search`, `compare`, `write`, `add`, `delete`, and `all` default: `all` | no
-`attrs` | All attributes to which the permission applies | no
+`attrs` | All attributes to which the permission applies. | no
 `bindtype` \| `ipapermbindruletype` | Bind rule type. It can be one of `permission`, `all`, `self`, or `anonymous` defaults to `permission` for new permissions. Bind rule type `self` can only be used on IPA versions 4.8.7 or up.| no
 `subtree` \| `ipapermlocation` | Subtree to apply permissions to | no
 `filter` \| `extratargetfilter` | Extra target filter | no
@@ -153,10 +176,12 @@ Variable | Description | Required
 `object_type` | Type of IPA object (sets subtree and objectClass targetfilter) | no
 `no_members` | Suppress processing of membership | no
 `rename` | Rename the permission object | no
-`privilege` | Member Privilege of Permission | no
 `action` | Work on permission or member level. It can be on of `member` or `permission` and defaults to `permission`. | no
 `state` | The state to ensure. It can be one of `present`, `absent`, or `renamed` default: `present`. | no
 
+The `includedattrs` and `excludedattrs` variables are only usable for managed permisions and are not exposed by the module. Using `attrs` for managed permissions will result in the automatic generation of `includedattrs` and `excludedattrs` in the IPA server.
+
+
 Authors
 =======
 

README-server.md

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

README-service.md

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

README-sudocmd.md

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

README-sudorule.md

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

README-vault.md

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

README.md

@@ -3,7 +3,7 @@ FreeIPA Ansible collection
 
 This repository contains [Ansible](https://www.ansible.com/) roles and playbooks to install and uninstall [FreeIPA](https://www.freeipa.org/) `servers`, `replicas` and `clients`. Also modules for group, host, topology and user management.
 
-**Note**: The ansible playbooks and roles require a configured ansible environment where the ansible nodes are reachable and are properly set up to have an IP address and a working package manager.
+**Note**: The Ansible playbooks and roles require a configured Ansible environment where the Ansible nodes are reachable and are properly set up to have an IP address and a working package manager.
 
 Features
 --------
@@ -12,6 +12,7 @@ Features
 * One-time-password (OTP) support for client installation
 * Repair mode for clients
 * Backup and restore, also to and from controller
+* Modules for automembership rule management
 * Modules for config management
 * Modules for delegation management
 * Modules for dns config management
@@ -30,12 +31,13 @@ Features
 * Modules for pwpolicy management
 * Modules for role management
 * Modules for self service management
+* Modules for server management
 * Modules for service management
 * Modules for sudocmd management
 * Modules for sudocmdgroup management
 * Modules for sudorule management
 * Modules for topology management
-* Modules fot trust management
+* Modules for trust management
 * Modules for user management
 * Modules for vault management
 
@@ -112,7 +114,7 @@ ansible-freeipa/plugins/module_utils to ~/.ansible/plugins/
 
 There are RPM packages available for Fedora 29+. These are installing the roles and modules into the global Ansible directories for `roles`, `plugins/modules` and `plugins/module_utils` in the `/usr/share/ansible` directory. Therefore is it possible to use the roles and modules without adapting the names like it is done in the example playbooks.
 
-**Ansible galaxy**
+**Ansible Galaxy**
 
 This command will get the whole collection from galaxy:
 
@@ -136,7 +138,7 @@ The needed adaptions of collection prefixes for `modules` and `module_utils` wil
 Ansible inventory file
 ----------------------
 
-The most important parts of the inventory file is the definition of the nodes, settings and the management modules. Please remember to use [Ansible vault](https://docs.ansible.com/ansible/latest/user_guide/vault.html) for passwords. The examples here are not using vault for better readability.
+The most important parts of the inventory file is the definition of the nodes, settings and the management modules. Please remember to use [Ansible Vault](https://docs.ansible.com/ansible/latest/user_guide/vault.html) for passwords. The examples here are not using vault for better readability.
 
 **Master server**
 
@@ -154,7 +156,7 @@ ipaserver_domain=test.local
 ipaserver_realm=TEST.LOCAL
 ```
 
-The admin principle is ```admin``` by default. Please set ```ipaadmin_principal``` if you need to change it.
+The admin principal is ```admin``` by default. Please set ```ipaadmin_principal``` if you need to change it.
 
 You can also add more setting here, like for example to enable the DNS server or to set auto-forwarders:
 ```yaml
@@ -280,7 +282,7 @@ ipaserver_domain=test.local
 ipaserver_realm=TEST.LOCAL
 ```
 
-For enhanced security it is possible to use a auto-generated one-time-password (OTP). This will be generated on the controller using the (first) server. It is needed to have the Python gssapi bindings installed on the controller for this.
+For enhanced security it is possible to use a auto-generated one-time-password (OTP). This will be generated on the controller using the (first) server. It is needed to have the python-gssapi bindings installed on the controller for this.
 To enable the generation of the one-time-password:
 ```yaml
 [ipaclients:vars]
@@ -345,7 +347,7 @@ With this playbook it is possible to add a list of topology segments using the `
 Playbooks
 =========
 
-The playbooks needed to deploy or undeploy server, replicas and clients are part of the repository and placed in the playbooks folder. There are also playbooks to deploy and undeploy clusters. With them it is only needed to add an inventory file:
+The playbooks needed to deploy or undeploy servers, replicas and clients are part of the repository and placed in the playbooks folder. There are also playbooks to deploy and undeploy clusters. With them it is only needed to add an inventory file:
 ```
 playbooks\
         install-client.yml
@@ -366,7 +368,7 @@ ansible-playbook -v -i inventory/hosts install-server.yml
 ```
 This will deploy the master server defined in the inventory file.
 
-If Ansible vault is used for passwords, then it is needed to adapt the playbooks in this way:
+If Ansible Vault is used for passwords, then it is needed to adapt the playbooks in this way:
 ```yaml
 ---
 - name: Playbook to configure IPA servers
@@ -421,6 +423,7 @@ Roles
 Modules in plugin/modules
 =========================
 
+* [ipaautomember](README-automember.md)
 * [ipaconfig](README-config.md)
 * [ipadelegation](README-delegation.md)
 * [ipadnsconfig](README-dnsconfig.md)
@@ -439,6 +442,7 @@ Modules in plugin/modules
 * [ipapwpolicy](README-pwpolicy.md)
 * [iparole](README-role.md)
 * [ipaselfservice](README-ipaselfservice.md)
+* [ipaserver](README-server.md)
 * [ipaservice](README-service.md)
 * [ipasudocmd](README-sudocmd.md)
 * [ipasudocmdgroup](README-sudocmdgroup.md)

galaxy.yml

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

molecule/centos-8-build/molecule.yml

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

molecule/fedora-latest-build/molecule.yml

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

molecule/resources/playbooks/prepare-build.yml

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

playbooks/automember/automember-group-absent.yml

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

playbooks/automember/automember-group-present.yml

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

playbooks/automember/automember-hostgroup-absent.yml

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

playbooks/automember/automember-hostgroup-present.yml

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

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

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

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

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

playbooks/permission/permission-absent.yml

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

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

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

playbooks/permission/permission-member-absent.yml

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

playbooks/permission/permission-member-present.yml

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

playbooks/permission/permission-present.yml

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

playbooks/permission/permission-renamed.yml

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

playbooks/selfservice/selfservice-absent.yml

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

playbooks/selfservice/selfservice-member-absent.yml

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

playbooks/selfservice/selfservice-member-present.yml

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

playbooks/selfservice/selfservice-present.yml

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

playbooks/server/server-absent-continue.yml

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

playbooks/server/server-absent-force.yml

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

playbooks/server/server-absent-ignore_last_of_role.yml

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

playbooks/server/server-absent-ignore_topology_disconnect.yml

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

playbooks/server/server-absent.yml

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

playbooks/server/server-hidden.yml

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

playbooks/server/server-location.yml

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

playbooks/server/server-no-location.yml

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

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

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

playbooks/server/server-not-hidden.yml

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

playbooks/server/server-present.yml

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

playbooks/server/server-service-weight.yml

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

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

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

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

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

plugins/modules/ipaautomember.py

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

plugins/modules/ipaconfig.py

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

plugins/modules/ipadelegation.py

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

plugins/modules/ipadnsconfig.py

@@ -114,8 +114,8 @@ def find_dnsconfig(module):
         if _result["result"].get('idnsforwarders', None) is None:
             _result["result"]['idnsforwarders'] = ['']
         return _result["result"]
-    else:
-        module.fail_json(msg="Could not retrieve current DNS configuration.")
+
+    module.fail_json(msg="Could not retrieve current DNS configuration.")
     return None
 
 
@@ -233,7 +233,8 @@ def main():
         # Execute command only if configuration changes.
         if not compare_args_ipa(ansible_module, args, res_find):
             try:
-                api_command_no_name(ansible_module, 'dnsconfig_mod', args)
+                if not ansible_module.check_mode:
+                    api_command_no_name(ansible_module, 'dnsconfig_mod', args)
                 # If command did not fail, something changed.
                 changed = True
 

plugins/modules/ipadnsforwardzone.py

@@ -135,8 +135,8 @@ def find_dnsforwardzone(module, name):
             msg="There is more than one dnsforwardzone '%s'" % (name))
     elif len(_result["result"]) == 1:
         return _result["result"][0]
-    else:
-        return None
+
+    return None
 
 
 def gen_args(forwarders, forwardpolicy, skip_overlap_check):
@@ -380,8 +380,14 @@ def main():
                         [name, 'dnsforwardzone_remove_permission', {}]
                     )
 
-            for name, command, args in commands:
-                api_command(ansible_module, command, name, args)
+            # Check mode exit
+            if ansible_module.check_mode:
+                ansible_module.exit_json(changed=len(commands) > 0,
+                                         **exit_args)
+
+            # Execute commands
+            for _name, command, args in commands:
+                api_command(ansible_module, command, _name, args)
                 changed = True
 
     except Exception as e:

plugins/modules/ipadnsrecord.py

@@ -868,10 +868,10 @@ from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils._text import to_text
 from ansible.module_utils.ansible_freeipa_module import temp_kinit, \
     temp_kdestroy, valid_creds, api_connect, api_command, module_params_get, \
-    is_ipv4_addr, is_ipv6_addr
+    is_ipv4_addr, is_ipv6_addr, ipalib_errors
 import dns.reversename
 import dns.resolver
-import ipalib.errors
+
 import six
 
 
@@ -1150,7 +1150,7 @@ def find_dnsrecord(module, dnszone, name):
     try:
         _result = api_command(
             module, "dnsrecord_show", to_text(dnszone), _args)
-    except ipalib.errors.NotFound:
+    except ipalib_errors.NotFound:
         return None
 
     return _result["result"]
@@ -1350,8 +1350,6 @@ def define_commands_for_present_state(module, zone_name, entry, res_find):
                     module, zone_name, name, args[record])
                 _commands.extend(cmds)
                 del args['%s_extra_create_reverse' % ipv]
-                if '%s_ip_address' not in args:
-                    del args[record]
         for record, fields in _RECORD_PARTS.items():
             part_fields = [f for f in fields if f in args]
             if part_fields:
@@ -1375,12 +1373,11 @@ def define_commands_for_present_state(module, zone_name, entry, res_find):
                     # remove record from args, as it will not be used again.
                     del args[record]
                 else:
-                    for f in part_fields:
-                        _args = {k: args[k] for k in part_fields}
-                        _args['idnsname'] = name
-                        _commands.append([zone_name, 'dnsrecord_add', _args])
+                    _args = {k: args[k] for k in part_fields if k in args}
+                    _args['idnsname'] = name
+                    _commands.append([zone_name, 'dnsrecord_add', _args])
                 # clean used fields from args
-                for f in part_fields:
+                for f in part_fields:   # pylint: disable=invalid-name
                     if f in args:
                         del args[f]
             else:
@@ -1497,6 +1494,10 @@ def main():
             if cmds:
                 commands.extend(cmds)
 
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
         # Execute commands
         for name, command, args in commands:
             try:
@@ -1508,9 +1509,9 @@ def main():
                 else:
                     changed = True
 
-            except ipalib.errors.EmptyModlist:
+            except ipalib_errors.EmptyModlist:
                 continue
-            except ipalib.errors.DuplicateEntry:
+            except ipalib_errors.DuplicateEntry:
                 continue
             except Exception as e:
                 error_message = str(e)

plugins/modules/ipadnszone.py

@@ -210,11 +210,11 @@ dnszone:
 from ipapython.dnsutil import DNSName  # noqa: E402
 from ansible.module_utils.ansible_freeipa_module import (
     FreeIPABaseModule,
-    is_ipv4_addr,
-    is_ipv6_addr,
+    is_ip_address,
+    is_ip_network_address,
     is_valid_port,
+    ipalib_errors
 )  # noqa: E402
-import ipalib.errors
 import netaddr
 import six
 
@@ -252,27 +252,29 @@ class DNSZoneModule(FreeIPABaseModule):
 
     def validate_ips(self, ips, error_msg):
         invalid_ips = [
-            ip for ip in ips if not is_ipv4_addr(ip) or is_ipv6_addr(ip)
+            ip for ip in ips
+            if not any([
+                is_ip_address(ip),
+                is_ip_network_address(ip),
+                ip == "any",
+                ip == "none"
+            ])
         ]
         if any(invalid_ips):
             self.fail_json(msg=error_msg % invalid_ips)
 
-    def is_valid_nsec3param_rec(self, nsec3param_rec):
+    def is_valid_nsec3param_rec(self, nsec3param_rec):  # pylint: disable=R0201
         try:
             part1, part2, part3, part4 = nsec3param_rec.split(" ")
         except ValueError:
             return False
 
-        if not all([part1.isdigit(), part2.isdigit(), part3.isdigit()]):
-            return False
-
-        if not 0 <= int(part1) <= 255:
-            return False
-
-        if not 0 <= int(part2) <= 255:
-            return False
-
-        if not 0 <= int(part3) <= 65535:
+        if (
+            not all([part1.isdigit(), part2.isdigit(), part3.isdigit()])
+            or not 0 <= int(part1) <= 255
+            or not 0 <= int(part2) <= 255
+            or not 0 <= int(part3) <= 65535
+        ):
             return False
 
         try:
@@ -292,7 +294,7 @@ class DNSZoneModule(FreeIPABaseModule):
 
         return True
 
-    def get_ipa_nsec3paramrecord(self, **kwargs):
+    def get_ipa_nsec3paramrecord(self, **_kwargs):  # pylint: disable=R1710
         nsec3param_rec = self.ipa_params.nsec3param_rec
         if nsec3param_rec is not None:
             error_msg = (
@@ -304,12 +306,12 @@ class DNSZoneModule(FreeIPABaseModule):
                 self.fail_json(msg=error_msg)
             return nsec3param_rec
 
-    def get_ipa_idnsforwarders(self, **kwargs):
+    def get_ipa_idnsforwarders(self, **_kwargs):  # pylint: disable=R1710
         if self.ipa_params.forwarders is not None:
             forwarders = []
             for forwarder in self.ipa_params.forwarders:
                 ip_address = forwarder.get("ip_address")
-                if not (is_ipv4_addr(ip_address) or is_ipv6_addr(ip_address)):
+                if not is_ip_address(ip_address):
                     self.fail_json(
                         msg="Invalid IP for DNS forwarder: %s" % ip_address
                     )
@@ -328,14 +330,14 @@ class DNSZoneModule(FreeIPABaseModule):
 
             return forwarders
 
-    def get_ipa_idnsallowtransfer(self, **kwargs):
+    def get_ipa_idnsallowtransfer(self, **_kwargs):  # pylint: disable=R1710
         if self.ipa_params.allow_transfer is not None:
             error_msg = "Invalid ip_address for DNS allow_transfer: %s"
             self.validate_ips(self.ipa_params.allow_transfer, error_msg)
 
             return (";".join(self.ipa_params.allow_transfer) or "none") + ";"
 
-    def get_ipa_idnsallowquery(self, **kwargs):
+    def get_ipa_idnsallowquery(self, **_kwargs):  # pylint: disable=R1710
         if self.ipa_params.allow_query is not None:
             error_msg = "Invalid ip_address for DNS allow_query: %s"
             self.validate_ips(self.ipa_params.allow_query, error_msg)
@@ -358,27 +360,27 @@ class DNSZoneModule(FreeIPABaseModule):
 
         return ".".join((name, domain))
 
-    def get_ipa_idnssoarname(self, **kwargs):
+    def get_ipa_idnssoarname(self, **_kwargs):  # pylint: disable=R1710
         if self.ipa_params.admin_email is not None:
             return DNSName(
                 self._replace_at_symbol_in_rname(self.ipa_params.admin_email)
             )
 
-    def get_ipa_idnssoamname(self, **kwargs):
+    def get_ipa_idnssoamname(self, **_kwargs):  # pylint: disable=R1710
         if self.ipa_params.name_server is not None:
             return DNSName(self.ipa_params.name_server)
 
-    def get_ipa_skip_overlap_check(self, **kwargs):
+    def get_ipa_skip_overlap_check(self, **kwargs):  # pylint: disable=R1710
         zone = kwargs.get('zone')
         if not zone and self.ipa_params.skip_overlap_check is not None:
             return self.ipa_params.skip_overlap_check
 
-    def get_ipa_skip_nameserver_check(self, **kwargs):
+    def get_ipa_skip_nameserver_check(self, **kwargs):  # pylint: disable=R1710
         zone = kwargs.get('zone')
         if not zone and self.ipa_params.skip_nameserver_check is not None:
             return self.ipa_params.skip_nameserver_check
 
-    def __reverse_zone_name(self, ipaddress):
+    def __reverse_zone_name(self, ipaddress):  # pylint: disable=R1710
         """
         Infer reverse zone name from an ip address.
 
@@ -398,17 +400,16 @@ class DNSZoneModule(FreeIPABaseModule):
             ip_version = ip.version
         if ip_version == 4:
             return u'.'.join(items[4 - prefixlen // 8:])
-        elif ip_version == 6:
+        if ip_version == 6:
             return u'.'.join(items[32 - prefixlen // 4:])
-        else:
-            self.fail_json(msg="Invalid IP version for reverse zone.")
+        self.fail_json(msg="Invalid IP version for reverse zone.")
 
     def get_zone(self, zone_name):
         get_zone_args = {"idnsname": zone_name, "all": True}
 
         try:
             response = self.api_command("dnszone_show", args=get_zone_args)
-        except ipalib.errors.NotFound:
+        except ipalib_errors.NotFound:
             zone = None
             is_zone_active = False
         else:
@@ -486,13 +487,20 @@ class DNSZoneModule(FreeIPABaseModule):
             # See:
             #   - https://pagure.io/freeipa/issue/8227
             #   - https://pagure.io/freeipa/issue/8489
-            if set_serial:
+            # Only set SOA Serial if it is not set already.
+            if (set_serial and
+                (zone is None
+                 or "idnssoaserial" not in zone
+                 or zone["idnssoaserial"] is None
+                 or zone["idnssoaserial"][0] != str(self.ipa_params.serial)
+                 )):
                 args = {
                     "idnssoaserial": self.ipa_params.serial,
                 }
                 self.add_ipa_command("dnszone_mod", zone_name, args)
 
     def process_command_result(self, name, command, args, result):
+        # pylint: disable=super-with-arguments
         super(DNSZoneModule, self).process_command_result(
             name, command, args, result
         )

plugins/modules/ipagroup.py

@@ -185,7 +185,8 @@ RETURN = """
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.ansible_freeipa_module import temp_kinit, \
     temp_kdestroy, valid_creds, api_connect, api_command, compare_args_ipa, \
-    api_check_param, module_params_get, gen_add_del_lists, api_check_command
+    api_check_param, module_params_get, gen_add_del_lists, api_check_command, \
+    gen_add_list, gen_intersection_list
 
 
 def find_group(module, name):
@@ -201,8 +202,8 @@ def find_group(module, name):
             msg="There is more than one group '%s'" % (name))
     elif len(_result["result"]) == 1:
         return _result["result"][0]
-    else:
-        return None
+
+    return None
 
 
 def gen_args(description, gid, nomembers):
@@ -556,12 +557,43 @@ def main():
                                 "non-external group."
                         )
 
+                    # Reduce add lists for member_user, member_group,
+                    # member_service and member_external to new entries
+                    # only that are not in res_find.
+                    if user is not None and "member_user" in res_find:
+                        user = gen_add_list(
+                            user, res_find["member_user"])
+                    if group is not None and "member_group" in res_find:
+                        group = gen_add_list(
+                            group, res_find["member_group"])
+                    if service is not None and "member_service" in res_find:
+                        service = gen_add_list(
+                            service, res_find["member_service"])
+                    if externalmember is not None \
+                       and "member_external" in res_find:
+                        externalmember = gen_add_list(
+                            externalmember, res_find["member_external"])
+
                     if any([user, group, service, externalmember]):
                         commands.append(
                             [name, "group_add_member", add_member_args]
                         )
 
                     if has_add_membermanager:
+                        # Reduce add list for membermanager_user and
+                        # membermanager_group to new entries only that are
+                        # not in res_find.
+                        if membermanager_user is not None \
+                           and "membermanager_user" in res_find:
+                            membermanager_user = gen_add_list(
+                                membermanager_user,
+                                res_find["membermanager_user"])
+                        if membermanager_group is not None \
+                           and "membermanager_group" in res_find:
+                            membermanager_group = gen_add_list(
+                                membermanager_group,
+                                res_find["membermanager_group"])
+
                         # Add membermanager users and groups
                         if membermanager_user is not None or \
                            membermanager_group is not None:
@@ -596,12 +628,40 @@ def main():
                                 "non-external group."
                         )
 
+                    # Reduce del lists of member_user, member_group,
+                    # member_service and member_external to the entries only
+                    # that are in res_find.
+                    if user is not None:
+                        user = gen_intersection_list(
+                            user, res_find.get("member_user"))
+                    if group is not None:
+                        group = gen_intersection_list(
+                            group, res_find.get("member_group"))
+                    if service is not None:
+                        service = gen_intersection_list(
+                            service, res_find.get("member_service"))
+                    if externalmember is not None:
+                        externalmember = gen_intersection_list(
+                            externalmember, res_find.get("member_external"))
+
                     if any([user, group, service, externalmember]):
                         commands.append(
                             [name, "group_remove_member", del_member_args]
                         )
 
                     if has_add_membermanager:
+                        # Reduce del lists of membermanager_user and
+                        # membermanager_group to the entries only that are
+                        # in res_find.
+                        if membermanager_user is not None:
+                            membermanager_user = gen_intersection_list(
+                                membermanager_user,
+                                res_find.get("membermanager_user"))
+                        if membermanager_group is not None:
+                            membermanager_group = gen_intersection_list(
+                                membermanager_group,
+                                res_find.get("membermanager_group"))
+
                         # Remove membermanager users and groups
                         if membermanager_user is not None or \
                            membermanager_group is not None:
@@ -616,6 +676,10 @@ def main():
             else:
                 ansible_module.fail_json(msg="Unkown state '%s'" % state)
 
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
         # Execute commands
 
         for name, command, args in commands:
@@ -631,16 +695,12 @@ def main():
                 ansible_module.fail_json(msg="%s: %s: %s" % (command, name,
                                                              str(e)))
             # Get all errors
-            # All "already a member" and "not a member" failures in the
             # result are ignored. All others are reported.
             errors = []
             for failed_item in result.get("failed", []):
                 failed = result["failed"][failed_item]
                 for member_type in failed:
                     for member, failure in failed[member_type]:
-                        if "already a member" in failure \
-                           or "not a member" in failure:
-                            continue
                         errors.append("%s: %s %s: %s" % (
                             command, member_type, member, failure))
             if len(errors) > 0:

plugins/modules/ipahbacrule.py

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

plugins/modules/ipahbacsvc.py

@@ -89,8 +89,8 @@ def find_hbacsvc(module, name):
             msg="There is more than one hbacsvc '%s'" % (name))
     elif len(_result["result"]) == 1:
         return _result["result"][0]
-    else:
-        return None
+
+    return None
 
 
 def gen_args(description):
@@ -195,6 +195,10 @@ def main():
             else:
                 ansible_module.fail_json(msg="Unkown state '%s'" % state)
 
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
         # Execute commands
 
         for name, command, args in commands:

plugins/modules/ipahbacsvcgroup.py

@@ -121,8 +121,8 @@ def find_hbacsvcgroup(module, name):
             msg="There is more than one hbacsvcgroup '%s'" % (name))
     elif len(_result["result"]) == 1:
         return _result["result"][0]
-    else:
-        return None
+
+    return None
 
 
 def gen_args(description, nomembers):
@@ -300,6 +300,10 @@ def main():
             else:
                 ansible_module.fail_json(msg="Unkown state '%s'" % state)
 
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
         # Execute commands
         errors = []
         for name, command, args in commands:

plugins/modules/ipahost.py

@@ -466,7 +466,7 @@ def show_host(module, name):
 
 def gen_args(description, locality, location, platform, os, password, random,
              mac_address, sshpubkey, userclass, auth_ind, requires_pre_auth,
-             ok_as_delegate, ok_to_auth_as_delegate, force, reverse,
+             ok_as_delegate, ok_to_auth_as_delegate, force, _reverse,
              ip_address, update_dns):
     # certificate, managedby_host, principal, create_keytab_* and
     # allow_retrieve_keytab_* are not handled here
@@ -529,7 +529,7 @@ def gen_dnsrecord_args(module, ip_address, reverse):
     return _args
 
 
-def check_parameters(
+def check_parameters(   # pylint: disable=unused-argument
         module, state, action,
         description, locality, location, platform, os, password, random,
         certificate, managedby_host, principal, allow_create_keytab_user,
@@ -862,7 +862,7 @@ def main():
                     ok_to_auth_as_delegate, force, reverse, ip_address,
                     update_dns, update_password)
 
-            elif isinstance(host, str) or isinstance(host, unicode):
+            elif isinstance(host, (str, unicode)):
                 name = host
             else:
                 ansible_module.fail_json(msg="Host '%s' is not valid" %
@@ -1327,6 +1327,23 @@ def main():
 
                     dnsrecord_args = gen_dnsrecord_args(ansible_module,
                                                         ip_address, reverse)
+
+                    # Remove arecord and aaaarecord from dnsrecord_args
+                    # if the record does not exits in res_find_dnsrecord
+                    # to prevent "DNS resource record not found" error
+                    if "arecord" in dnsrecord_args \
+                       and dnsrecord_args["arecord"] is not None \
+                       and len(dnsrecord_args["arecord"]) > 0 \
+                       and (res_find_dnsrecord is None
+                            or "arecord" not in res_find_dnsrecord):
+                        del dnsrecord_args["arecord"]
+                    if "aaaarecord" in dnsrecord_args \
+                       and dnsrecord_args["aaaarecord"] is not None \
+                       and len(dnsrecord_args["aaaarecord"]) > 0 \
+                       and (res_find_dnsrecord is None
+                            or "aaaarecord" not in res_find_dnsrecord):
+                        del dnsrecord_args["aaaarecord"]
+
                     if "arecord" in dnsrecord_args or \
                        "aaaarecord" in dnsrecord_args:
                         domain_name = name[name.find(".")+1:]
@@ -1347,6 +1364,10 @@ def main():
 
         del host_set
 
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
         # Execute commands
 
         errors = []

plugins/modules/ipahostgroup.py

@@ -141,7 +141,8 @@ RETURN = """
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.ansible_freeipa_module import temp_kinit, \
     temp_kdestroy, valid_creds, api_connect, api_command, compare_args_ipa, \
-    module_params_get, gen_add_del_lists, api_check_command, api_check_param
+    module_params_get, gen_add_del_lists, api_check_command, api_check_param, \
+    gen_add_list, gen_intersection_list
 
 
 def find_hostgroup(module, name):
@@ -157,8 +158,8 @@ def find_hostgroup(module, name):
             msg="There is more than one hostgroup '%s'" % (name))
     elif len(_result["result"]) == 1:
         return _result["result"][0]
-    else:
-        return None
+
+    return None
 
 
 def gen_args(description, nomembers, rename):
@@ -396,6 +397,15 @@ def main():
                         ansible_module.fail_json(
                             msg="No hostgroup '%s'" % name)
 
+                    # Reduce add lists for member_host and member_hostgroup,
+                    # to new entries only that are not in res_find.
+                    if host is not None and "member_host" in res_find:
+                        host = gen_add_list(host, res_find["member_host"])
+                    if hostgroup is not None \
+                       and "member_hostgroup" in res_find:
+                        hostgroup = gen_add_list(
+                            hostgroup, res_find["member_hostgroup"])
+
                     # Ensure members are present
                     commands.append([name, "hostgroup_add_member",
                                      {
@@ -404,6 +414,20 @@ def main():
                                      }])
 
                     if has_add_membermanager:
+                        # Reduce add list for membermanager_user and
+                        # membermanager_group to new entries only that are
+                        # not in res_find.
+                        if membermanager_user is not None \
+                           and "membermanager_user" in res_find:
+                            membermanager_user = gen_add_list(
+                                membermanager_user,
+                                res_find["membermanager_user"])
+                        if membermanager_group is not None \
+                           and "membermanager_group" in res_find:
+                            membermanager_group = gen_add_list(
+                                membermanager_group,
+                                res_find["membermanager_group"])
+
                         # Add membermanager users and groups
                         if membermanager_user is not None or \
                            membermanager_group is not None:
@@ -441,6 +465,15 @@ def main():
                         ansible_module.fail_json(
                             msg="No hostgroup '%s'" % name)
 
+                    # Reduce del lists of member_host and member_hostgroup,
+                    # to the entries only that are in res_find.
+                    if host is not None:
+                        host = gen_intersection_list(
+                            host, res_find.get("member_host"))
+                    if hostgroup is not None:
+                        hostgroup = gen_intersection_list(
+                            hostgroup, res_find.get("member_hostgroup"))
+
                     # Ensure members are absent
                     commands.append([name, "hostgroup_remove_member",
                                      {
@@ -449,6 +482,18 @@ def main():
                                      }])
 
                     if has_add_membermanager:
+                        # Reduce del lists of membermanager_user and
+                        # membermanager_group to the entries only that are
+                        # in res_find.
+                        if membermanager_user is not None:
+                            membermanager_user = gen_intersection_list(
+                                membermanager_user,
+                                res_find.get("membermanager_user"))
+                        if membermanager_group is not None:
+                            membermanager_group = gen_intersection_list(
+                                membermanager_group,
+                                res_find.get("membermanager_group"))
+
                         # Remove membermanager users and groups
                         if membermanager_user is not None or \
                            membermanager_group is not None:
@@ -463,6 +508,10 @@ def main():
             else:
                 ansible_module.fail_json(msg="Unkown state '%s'" % state)
 
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
         # Execute commands
         for name, command, args in commands:
             try:
@@ -483,9 +532,6 @@ def main():
                 failed = result["failed"][failed_item]
                 for member_type in failed:
                     for member, failure in failed[member_type]:
-                        if "already a member" in failure \
-                           or "not a member" in failure:
-                            continue
                         errors.append("%s: %s %s: %s" % (
                             command, member_type, member, failure))
             if len(errors) > 0:

plugins/modules/ipalocation.py

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

plugins/modules/ipapermission.py

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

plugins/modules/ipaprivilege.py

@@ -234,14 +234,22 @@ def main():
                 if action == "privilege":
                     # Found the privilege
                     if res_find is not None:
+                        res_cmp = {
+                            k: v for k, v in res_find.items()
+                            if k not in [
+                                "objectclass", "cn", "dn",
+                                "memberof_permisssion"
+                            ]
+                        }
                         # For all settings is args, check if there are
                         # different settings in the find result.
                         # If yes: modify
-                        if not compare_args_ipa(ansible_module, args,
-                                                res_find):
+                        if args and not compare_args_ipa(ansible_module, args,
+                                                         res_cmp):
                             commands.append([name, "privilege_mod", args])
                     else:
                         commands.append([name, "privilege_add", args])
+                        res_find = {}
 
                     member_args = {}
                     if permission:
@@ -312,6 +320,10 @@ def main():
             else:
                 ansible_module.fail_json(msg="Unkown state '%s'" % state)
 
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
         # Execute commands
 
         for name, command, args in commands:

plugins/modules/ipapwpolicy.py

@@ -130,8 +130,8 @@ def find_pwpolicy(module, name):
             msg="There is more than one pwpolicy '%s'" % (name))
     elif len(_result["result"]) == 1:
         return _result["result"][0]
-    else:
-        return None
+
+    return None
 
 
 def gen_args(maxlife, minlife, history, minclasses, minlength, priority,
@@ -284,6 +284,10 @@ def main():
             else:
                 ansible_module.fail_json(msg="Unkown state '%s'" % state)
 
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
         # Execute commands
 
         for name, command, args in commands:

plugins/modules/iparole.py

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

plugins/modules/ipaselfservice.py

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

plugins/modules/ipaserver.py

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

plugins/modules/ipaservice.py

@@ -91,7 +91,7 @@ options:
     type: list
     aliases: ["krbprincipalname"]
   smb:
-    description: Add a SMB service. Can only be used with new services.
+    description: Add a SMB service.
     required: false
     type: bool
   netbiosname:
@@ -230,28 +230,17 @@ from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.ansible_freeipa_module import temp_kinit, \
     temp_kdestroy, valid_creds, api_connect, api_command, compare_args_ipa, \
     encode_certificate, gen_add_del_lists, module_params_get, to_text, \
-    api_check_param
-import ipalib.errors
+    api_check_param, ipalib_errors
 
 
-def find_service(module, name, netbiosname):
+def find_service(module, name):
     _args = {
         "all": True,
     }
 
-    # Search for a SMB/cifs service.
-    if netbiosname is not None:
-        _result = api_command(
-            module, "service_find", to_text(netbiosname), _args)
-
-        for _res_find in _result.get('result', []):
-            for uid in _res_find.get('uid', []):
-                if uid.startswith("%s$@" % netbiosname):
-                    return _res_find
-
     try:
         _result = api_command(module, "service_show", to_text(name), _args)
-    except ipalib.errors.NotFound:
+    except ipalib_errors.NotFound:
         return None
 
     if "result" in _result:
@@ -261,8 +250,8 @@ def find_service(module, name, netbiosname):
             _res["usercertificate"] = [encode_certificate(cert) for
                                        cert in certs]
         return _res
-    else:
-        return None
+
+    return None
 
 
 def gen_args(pac_type, auth_ind, skip_host_check, force, requires_pre_auth,
@@ -287,6 +276,19 @@ def gen_args(pac_type, auth_ind, skip_host_check, force, requires_pre_auth,
     return _args
 
 
+def gen_args_smb(netbiosname, ok_as_delegate, ok_to_auth_as_delegate):
+    _args = {}
+
+    if netbiosname is not None:
+        _args['ipantflatname'] = netbiosname
+    if ok_as_delegate is not None:
+        _args['ipakrbokasdelegate'] = (ok_as_delegate)
+    if ok_to_auth_as_delegate is not None:
+        _args['ipakrboktoauthasdelegate'] = (ok_to_auth_as_delegate)
+
+    return _args
+
+
 def check_parameters(module, state, action, names, parameters):
     assert isinstance(parameters, dict)
 
@@ -310,15 +312,13 @@ def check_parameters(module, state, action, names, parameters):
         if action == 'service':
             invalid = ['delete_continue']
 
-            if parameters.get('smb', False):
-                invalid.extend(['force', 'auth_ind', 'skip_host_check',
-                                'requires_pre_auth', 'auth_ind', 'pac_type'])
-
-                for _invalid in invalid:
-                    if parameters.get(_invalid, False):
-                        module.fail_json(
-                            msg="Argument '%s' can not be used with SMB "
-                                "service." % _invalid)
+            if (
+                not parameters.get('smb', False)
+                and parameters.get('netbiosname')
+            ):
+                module.fail_json(
+                    msg="Argument 'netbiosname' can not be used without "
+                        "SMB service.")
         else:
             invalid.append('delete_continue')
 
@@ -494,11 +494,9 @@ def main():
         commands = []
 
         for name in names:
-            res_find = find_service(ansible_module, name, netbiosname)
+            res_find = find_service(ansible_module, name)
 
             if state == "present":
-                # if service exists, 'smb' cannot be used.
-
                 if action == "service":
                     args = gen_args(
                         pac_type, auth_ind, skip_host_check, force,
@@ -507,13 +505,24 @@ def main():
                     if not has_skip_host_check and 'skip_host_check' in args:
                         del args['skip_host_check']
 
+                    if smb:
+                        if res_find is None:
+                            _name = "cifs/" + name
+                            res_find = find_service(ansible_module, _name)
+                            if res_find is None:
+                                _args = gen_args_smb(
+                                    netbiosname, ok_as_delegate,
+                                    ok_to_auth_as_delegate)
+                                commands.append(
+                                    [name, 'service_add_smb', _args])
+                                res_find = {}
+                            # service_add_smb will prefix 'name' with
+                            # "cifs/", so we will need to change it here,
+                            # so that service_mod, if called later, works.
+                            name = _name
+
                     if res_find is None:
-                        if smb:
-                            if netbiosname is not None:
-                                args['ipantflatname'] = netbiosname
-                            commands.append([name, 'service_add_smb', args])
-                        else:
-                            commands.append([name, 'service_add', args])
+                        commands.append([name, 'service_add', args])
 
                         certificate_add = certificate or []
                         certificate_del = []
@@ -551,6 +560,15 @@ def main():
                             if remove in args:
                                 del args[remove]
 
+                        if (
+                            "krbprincipalauthind" in args
+                            and (
+                                args.get("krbprincipalauthind", [""]) ==
+                                res_find.get("krbprincipalauthind", [""])
+                            )
+                          ):
+                            del args["krbprincipalauthind"]
+
                         if not compare_args_ipa(ansible_module, args,
                                                 res_find):
                             commands.append([name, "service_mod", args])
@@ -753,7 +771,7 @@ def main():
             elif state == "absent":
                 if action == "service":
                     if res_find is not None:
-                        args = {'continue': True if delete_continue else False}
+                        args = {'continue': delete_continue}
                         commands.append([name, 'service_del', args])
 
                 elif action == "member":
@@ -824,6 +842,10 @@ def main():
             else:
                 ansible_module.fail_json(msg="Unkown state '%s'" % state)
 
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
         # Execute commands
         errors = []
         for name, command, args in commands:

plugins/modules/ipasudocmd.py

@@ -56,15 +56,15 @@ author:
 
 EXAMPLES = """
 # Ensure sudocmd is present
-- ipacommand:
+- ipasudocmd:
     ipaadmin_password: SomeADMINpassword
-    name: su
+    name: /usr/bin/su
     state: present
 
 # Ensure sudocmd is absent
-- ipacommand:
+- ipasudocmd:
     ipaadmin_password: SomeADMINpassword
-    name: su
+    name: /usr/bin/su
     state: absent
 """
 
@@ -90,8 +90,8 @@ def find_sudocmd(module, name):
             msg="There is more than one sudocmd '%s'" % (name))
     elif len(_result["result"]) == 1:
         return _result["result"][0]
-    else:
-        return None
+
+    return None
 
 
 def gen_args(description):
@@ -182,6 +182,10 @@ def main():
             else:
                 ansible_module.fail_json(msg="Unkown state '%s'" % state)
 
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
         # Execute commands
         for name, command, args in commands:
             try:

plugins/modules/ipasudocmdgroup.py

@@ -107,9 +107,7 @@ from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils._text import to_text
 from ansible.module_utils.ansible_freeipa_module import temp_kinit, \
     temp_kdestroy, valid_creds, api_connect, api_command, compare_args_ipa, \
-    gen_add_del_lists
-
-import ipalib
+    gen_add_del_lists, ipalib_errors
 
 
 def find_sudocmdgroup(module, name):
@@ -117,7 +115,7 @@ def find_sudocmdgroup(module, name):
 
     try:
         _result = api_command(module, "sudocmdgroup_show", to_text(name), args)
-    except ipalib.errors.NotFound:
+    except ipalib_errors.NotFound:
         return None
     else:
         return _result["result"]
@@ -298,6 +296,10 @@ def main():
             else:
                 ansible_module.fail_json(msg="Unkown state '%s'" % state)
 
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
         # Execute commands
         for name, command, args in commands:
             try:

plugins/modules/ipasudorule.py

@@ -53,7 +53,7 @@ options:
     required: false
     choices: ["all", ""]
     aliases: ["usercat"]
-  usergroup:
+  group:
     description: List of user groups assigned to the sudo rule.
     required: false
   runasgroupcategory:
@@ -206,8 +206,8 @@ def find_sudorule(module, name):
             msg="There is more than one sudorule '%s'" % (name))
     elif len(_result["result"]) == 1:
         return _result["result"][0]
-    else:
-        return None
+
+    return None
 
 
 def gen_args(description, usercat, hostcat, cmdcat, runasusercat,
@@ -416,6 +416,32 @@ def main():
                 if action == "sudorule":
                     # Found the sudorule
                     if res_find is not None:
+                        # Remove empty usercategory, hostcategory,
+                        # cmdcaterory, runasusercategory and hostcategory
+                        # from args if "" and if the category is not in the
+                        # sudorule. The empty string is used to reset the
+                        # category.
+                        if "usercategory" in args \
+                           and args["usercategory"] == "" \
+                           and "usercategory" not in res_find:
+                            del args["usercategory"]
+                        if "hostcategory" in args \
+                           and args["hostcategory"] == "" \
+                           and "hostcategory" not in res_find:
+                            del args["hostcategory"]
+                        if "cmdcategory" in args \
+                           and args["cmdcategory"] == "" \
+                           and "cmdcategory" not in res_find:
+                            del args["cmdcategory"]
+                        if "ipasudorunasusercategory" in args \
+                           and args["ipasudorunasusercategory"] == "" \
+                           and "ipasudorunasusercategory" not in res_find:
+                            del args["ipasudorunasusercategory"]
+                        if "ipasudorunasgroupcategory" in args \
+                           and args["ipasudorunasgroupcategory"] == "" \
+                           and "ipasudorunasgroupcategory" not in res_find:
+                            del args["ipasudorunasgroupcategory"]
+
                         # For all settings is args, check if there are
                         # different settings in the find result.
                         # If yes: modify
@@ -429,16 +455,16 @@ def main():
 
                     # Generate addition and removal lists
                     host_add, host_del = gen_add_del_lists(
-                        host, res_find.get('member_host', []))
+                        host, res_find.get('memberhost_host', []))
 
                     hostgroup_add, hostgroup_del = gen_add_del_lists(
-                        hostgroup, res_find.get('member_hostgroup', []))
+                        hostgroup, res_find.get('memberhost_hostgroup', []))
 
                     user_add, user_del = gen_add_del_lists(
-                        user, res_find.get('member_user', []))
+                        user, res_find.get('memberuser_user', []))
 
                     group_add, group_del = gen_add_del_lists(
-                        group, res_find.get('member_group', []))
+                        group, res_find.get('memberuser_group', []))
 
                     allow_cmd_add, allow_cmd_del = gen_add_del_lists(
                         allow_sudocmd,
@@ -686,6 +712,10 @@ def main():
             else:
                 ansible_module.fail_json(msg="Unkown state '%s'" % state)
 
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
         # Execute commands
 
         errors = []

plugins/modules/ipatopologysegment.py

@@ -132,8 +132,8 @@ def find_left_right(module, suffix, left, right):
             "not unique for suffix '%s'" % (left, right, suffix))
     elif len(_result["result"]) == 1:
         return _result["result"][0]
-    else:
-        return None
+
+    return None
 
 
 def find_cn(module, suffix, name):
@@ -147,8 +147,8 @@ def find_cn(module, suffix, name):
             msg="CN '%s' is not unique for suffix '%s'" % (name, suffix))
     elif len(_result["result"]) == 1:
         return _result["result"][0]
-    else:
-        return None
+
+    return None
 
 
 def find_left_right_cn(module, suffix, left, right, name):
@@ -326,6 +326,10 @@ def main():
             else:
                 ansible_module.fail_json(msg="Unkown state '%s'" % state)
 
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
         # Execute command
 
         for command, args, _suffix in commands:

plugins/modules/ipatrust.py

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

plugins/modules/ipauser.py

@@ -512,10 +512,9 @@ def find_user(module, name, preserved=False):
         if certs is not None:
             _result["usercertificate"] = [encode_certificate(x)
                                           for x in certs]
-
         return _result
-    else:
-        return None
+
+    return None
 
 
 def gen_args(first, last, fullname, displayname, initials, homedir, shell,
@@ -599,17 +598,14 @@ def gen_args(first, last, fullname, displayname, initials, homedir, shell,
     return _args
 
 
-def check_parameters(module, state, action,
-                     first, last, fullname, displayname, initials, homedir,
-                     shell, email, principal, principalexpiration,
-                     passwordexpiration, password, random, uid, gid, city,
-                     phone, mobile, pager, fax, orgunit, title, manager,
-                     carlicense, sshpubkey, userauthtype, userclass, radius,
-                     radiususer, departmentnumber, employeenumber,
-                     employeetype, preferredlanguage, certificate,
-                     certmapdata, noprivate, nomembers, preserve,
-                     update_password):
-
+def check_parameters(  # pylint: disable=unused-argument
+        module, state, action, first, last, fullname, displayname, initials,
+        homedir, shell, email, principal, principalexpiration,
+        passwordexpiration, password, random, uid, gid, city, phone, mobile,
+        pager, fax, orgunit, title, manager, carlicense, sshpubkey,
+        userauthtype, userclass, radius, radiususer, departmentnumber,
+        employeenumber, employeetype, preferredlanguage, certificate,
+        certmapdata, noprivate, nomembers, preserve, update_password):
     if state == "present":
         if action == "member":
             invalid = ["first", "last", "fullname", "displayname", "initials",
@@ -715,7 +711,7 @@ def check_certmapdata(data):
         return False
 
     i = data.find("<I>", 4)
-    s = data.find("<S>", i)
+    s = data.find("<S>", i)   # pylint: disable=invalid-name
     issuer = data[i+3:s]
     subject = data[s+3:]
 
@@ -1033,7 +1029,7 @@ def main():
 
                 email = extend_emails(email, default_email_domain)
 
-            elif isinstance(user, str) or isinstance(user, unicode):
+            elif isinstance(user, (str, unicode)):
                 name = user
             else:
                 ansible_module.fail_json(msg="User '%s' is not valid" %
@@ -1115,8 +1111,13 @@ def main():
                         # For all settings is args, check if there are
                         # different settings in the find result.
                         # If yes: modify
-                        if not compare_args_ipa(ansible_module, args,
-                                                res_find):
+                        # The nomembers parameter is added to args for the
+                        # api command. But no_members is never part of
+                        # res_find from user-show, therefore this parameter
+                        # needs to be ignored in compare_args_ipa.
+                        if not compare_args_ipa(
+                                ansible_module, args, res_find,
+                                ignore=["no_members"]):
                             commands.append([name, "user_mod", args])
 
                     else:
@@ -1377,6 +1378,10 @@ def main():
 
         del user_set
 
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
         # Execute commands
 
         errors = []

plugins/modules/ipavault.py

@@ -119,6 +119,7 @@ options:
     description: Users that are owners of the vault.
     required: false
     type: list
+    aliases: ["ownerusers"]
   ownergroups:
     description: Groups that are owners of the vault.
     required: false
@@ -317,10 +318,11 @@ vault:
 import os
 from base64 import b64decode
 from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils._text import to_text
 from ansible.module_utils.ansible_freeipa_module import temp_kinit, \
     temp_kdestroy, valid_creds, api_connect, api_command, \
-    gen_add_del_lists, compare_args_ipa, module_params_get, exit_raw_json
-from ipalib.errors import EmptyModlist
+    gen_add_del_lists, compare_args_ipa, module_params_get, exit_raw_json, \
+    ipalib_errors
 
 
 def find_vault(module, name, username, service, shared):
@@ -347,11 +349,13 @@ def find_vault(module, name, username, service, shared):
     return None
 
 
-def gen_args(description, username, service, shared, vault_type, salt,
-             password, password_file, public_key, public_key_file, vault_data,
-             datafile_in, datafile_out):
+def gen_args(
+        description, username, service, shared, vault_type, salt,
+        public_key, public_key_file):
     _args = {}
+    vault_type = vault_type or to_text("symmetric")
 
+    _args['ipavaulttype'] = vault_type
     if description is not None:
         _args['description'] = description
     if username is not None:
@@ -360,27 +364,32 @@ def gen_args(description, username, service, shared, vault_type, salt,
         _args['service'] = service
     if shared is not None:
         _args['shared'] = shared
-    if vault_type is not None:
-        _args['ipavaulttype'] = vault_type
-    if salt is not None:
-        _args['ipavaultsalt'] = salt
-    if public_key is not None:
-        _args['ipavaultpublickey'] = b64decode(public_key.encode('utf-8'))
-    if public_key_file is not None:
-        with open(public_key_file, 'r') as keyfile:
-            keydata = keyfile.read()
-            _args['ipavaultpublickey'] = keydata.strip().encode('utf-8')
+
+    if vault_type == "symmetric":
+        if salt is not None:
+            _args['ipavaultsalt'] = salt
+        _args['ipavaultpublickey'] = None
+
+    elif vault_type == "asymmetric":
+        if public_key is not None:
+            _args['ipavaultpublickey'] = b64decode(public_key.encode('utf-8'))
+        if public_key_file is not None:
+            with open(public_key_file, 'r') as keyfile:
+                keydata = keyfile.read()
+                _args['ipavaultpublickey'] = keydata.strip().encode('utf-8')
+        _args['ipavaultsalt'] = None
+
+    elif vault_type == "standard":
+        _args['ipavaultsalt'] = None
+        _args['ipavaultpublickey'] = None
 
     return _args
 
 
 def gen_member_args(args, users, groups, services):
-    _args = args.copy()
-
-    for arg in ['ipavaulttype', 'description', 'ipavaultpublickey',
-                'ipavaultsalt']:
-        if arg in _args:
-            del _args[arg]
+    remove = ['ipavaulttype', 'description', 'ipavaultpublickey',
+              'ipavaultsalt']
+    _args = {k: v for k, v in args.items() if k not in remove}
 
     if any([users, groups, services]):
         if users is not None:
@@ -395,9 +404,12 @@ def gen_member_args(args, users, groups, services):
     return None
 
 
-def data_storage_args(args, data, password, password_file, private_key,
-                      private_key_file, datafile_in, datafile_out):
-    _args = {}
+def data_storage_args(vault_type, args, data, password, password_file,
+                      private_key, private_key_file, datafile_in,
+                      datafile_out):
+    remove = ['ipavaulttype', 'description', 'ipavaultpublickey',
+              'ipavaultsalt']
+    _args = {k: v for k, v in args.items() if k not in remove}
 
     if 'username' in args:
         _args['username'] = args['username']
@@ -406,15 +418,17 @@ def data_storage_args(args, data, password, password_file, private_key,
     if 'shared' in args:
         _args['shared'] = args['shared']
 
-    if password is not None:
-        _args['password'] = password
-    if password_file is not None:
-        _args['password_file'] = password_file
+    if vault_type is None or vault_type == "symmetric":
+        if password is not None:
+            _args['password'] = password
+        if password_file is not None:
+            _args['password_file'] = password_file
 
-    if private_key is not None:
-        _args['private_key'] = private_key
-    if private_key_file is not None:
-        _args['private_key_file'] = private_key_file
+    if vault_type == "asymmetric":
+        if private_key is not None:
+            _args['private_key'] = private_key
+        if private_key_file is not None:
+            _args['private_key_file'] = private_key_file
 
     if datafile_in is not None:
         _args['in'] = datafile_in
@@ -427,21 +441,18 @@ def data_storage_args(args, data, password, password_file, private_key,
     if datafile_out is not None:
         _args['out'] = datafile_out
 
-    if private_key_file is not None:
-        _args['private_key_file'] = private_key_file
-
     return _args
 
 
-def check_parameters(module, state, action, description, username, service,
-                     shared, users, groups, services, owners, ownergroups,
-                     ownerservices, vault_type, salt, password, password_file,
-                     public_key, public_key_file, private_key,
-                     private_key_file, vault_data, datafile_in, datafile_out,
-                     new_password, new_password_file):
+def check_parameters(  # pylint: disable=unused-argument
+        module, state, action, description, username, service, shared, users,
+        groups, services, owners, ownergroups, ownerservices, vault_type, salt,
+        password, password_file, public_key, public_key_file, private_key,
+        private_key_file, vault_data, datafile_in, datafile_out, new_password,
+        new_password_file):
     invalid = []
     if state == "present":
-        invalid = ['private_key', 'private_key_file', 'datafile_out']
+        invalid = ['datafile_out']
 
         if all([password, password_file]) \
            or all([new_password, new_password_file]):
@@ -454,7 +465,7 @@ def check_parameters(module, state, action, description, username, service,
                     "change symmetric vault password.")
 
         if action == "member":
-            invalid.extend(['description'])
+            invalid.extend(['description', 'vault_type'])
 
     elif state == "absent":
         invalid = ['description', 'salt', 'vault_type', 'private_key',
@@ -480,20 +491,18 @@ def check_parameters(module, state, action, description, username, service,
                 msg="Argument '%s' can not be used with state '%s', "
                     "action '%s'" % (arg, state, action))
 
-    for arg in invalid:
-        if vars()[arg] is not None:
-            module.fail_json(
-                msg="Argument '%s' can not be used with state '%s', "
-                    "action '%s'" % (arg, state, action))
 
-
-def check_encryption_params(module, state, action, vault_type, salt,
-                            password, password_file, public_key,
-                            public_key_file, private_key, private_key_file,
-                            vault_data, datafile_in, datafile_out,
-                            new_password, new_password_file, res_find):
+def check_encryption_params(  # pylint: disable=unused-argument
+        module, state, action, vault_type, salt, password, password_file,
+        public_key, public_key_file, private_key, private_key_file, vault_data,
+        datafile_in, datafile_out, new_password, new_password_file, res_find):
+    """Check parameters used for (de)vault data encryption."""
     vault_type_invalid = []
 
+    existing_type = None
+    if res_find:
+        existing_type = res_find["ipavaulttype"][0]
+
     if vault_type is None and res_find is not None:
         vault_type = res_find['ipavaulttype']
         if isinstance(vault_type, (tuple, list)):
@@ -536,47 +545,45 @@ def check_encryption_params(module, state, action, vault_type, salt,
                 msg="Assymmetric vault requires public_key "
                     "or public_key_file to store data.")
 
-    for param in vault_type_invalid:
+    valid_fields = []
+    if existing_type == "symmetric":
+        valid_fields = [
+            'password', 'password_file', 'new_password', 'new_password_file',
+            'salt'
+        ]
+    if existing_type == "asymmetric":
+        valid_fields = [
+            'public_key', 'public_key_file', 'private_key', 'private_key_file'
+        ]
+
+    check_fields = [f for f in vault_type_invalid if f not in valid_fields]
+
+    for param in check_fields:
         if vars()[param] is not None:
             module.fail_json(
                 msg="Argument '%s' cannot be used with vault type '%s'" %
                 (param, vault_type or 'symmetric'))
 
 
-def change_password(module, res_find, password, password_file, new_password,
-                    new_password_file):
-    """
-    Change the password of a symmetric vault.
-
-    To change the password of a vault, it is needed to retrieve the stored
-    data with the current password, and store the data again, with the new
-    password, forcing it to override the old one.
-    """
-    # verify parameters.
-    if not any([new_password, new_password_file]):
-        return []
-    if res_find["ipavaulttype"][0] != "symmetric":
-        module.fail_json(msg="Cannot change password of `%s` vault."
-                             % res_find["ipavaulttype"])
-
+def get_stored_data(module, res_find, args):
+    """Retrieve data stored in the vault."""
     # prepare arguments to retrieve data.
     name = res_find["cn"][0]
-    args = {}
-    if password:
-        args["password"] = password
-    if password_file:
-        args["password_file"] = password_file
-    # retrieve current stored data
-    result = api_command(module, 'vault_retrieve', name, args)
+    copy_args = []
+    if res_find['ipavaulttype'][0] == "symmetric":
+        copy_args = ["password", "password_file"]
+    if res_find['ipavaulttype'][0] == "asymmetric":
+        copy_args = ["private_key", "private_key_file"]
 
-    # modify arguments to store data with new password.
-    args = {"override_password": True, "data": result['result']['data']}
-    if new_password:
-        args["password"] = new_password
-    if new_password_file:
-        args["password_file"] = new_password_file
-    # return the command to store data with the new password.
-    return [(name, "vault_archive", args)]
+    pwdargs = {arg: args[arg] for arg in copy_args if arg in args}
+
+    # retrieve vault stored data
+    try:
+        result = api_command(module, 'vault_retrieve', name, pwdargs)
+    except ipalib_errors.NotFound:
+        return None
+
+    return result['result'].get('data')
 
 
 def main():
@@ -594,10 +601,12 @@ def main():
                             default=None, required=False,
                             choices=["standard", "symmetric", "asymmetric"]),
             vault_public_key=dict(type="str", required=False, default=None,
-                                  aliases=['ipavaultpublickey', 'public_key']),
+                                  aliases=['ipavaultpublickey', 'public_key',
+                                           'new_public_key']),
             vault_public_key_file=dict(type="str", required=False,
                                        default=None,
-                                       aliases=['public_key_file']),
+                                       aliases=['public_key_file',
+                                                'new_public_key_file']),
             vault_private_key=dict(
                 type="str", required=False, default=None, no_log=True,
                 aliases=['ipavaultprivatekey', 'private_key']),
@@ -742,21 +751,16 @@ def main():
             res_find = find_vault(
                 ansible_module, name, username, service, shared)
 
+            # Set default vault_type if needed.
+            res_type = res_find.get('ipavaulttype')[0] if res_find else None
+            if vault_type is None:
+                vault_type = res_type if res_find is not None else u"symmetric"
+
             # Generate args
             args = gen_args(description, username, service, shared, vault_type,
-                            salt, password, password_file, public_key,
-                            public_key_file, vault_data, datafile_in,
-                            datafile_out)
+                            salt, public_key, public_key_file)
             pwdargs = None
 
-            # Set default vault_type if needed.
-            if vault_type is None and vault_data is not None:
-                if res_find is not None:
-                    res_vault_type = res_find.get('ipavaulttype')[0]
-                    args['ipavaulttype'] = vault_type = res_vault_type
-                else:
-                    args['ipavaulttype'] = vault_type = u"symmetric"
-
             # Create command
             if state == "present":
                 # verify data encription args
@@ -766,16 +770,52 @@ def main():
                     private_key_file, vault_data, datafile_in, datafile_out,
                     new_password, new_password_file, res_find)
 
-                # Found the vault
+                change_passwd = any([
+                    new_password, new_password_file,
+                    (private_key or private_key_file) and
+                    (public_key or public_key_file)
+                ])
                 if action == "vault":
+                    # Found the vault
                     if res_find is not None:
-                        # For all settings is args, check if there are
-                        # different settings in the find result.
-                        # If yes: modify
-                        if not compare_args_ipa(ansible_module, args,
-                                                res_find):
-                            commands.append([name, "vault_mod_internal", args])
+                        arg_type = args.get("ipavaulttype")
 
+                        modified = not compare_args_ipa(ansible_module,
+                                                        args, res_find)
+
+                        if arg_type != res_type or change_passwd:
+                            stargs = data_storage_args(
+                                res_type, args, vault_data, password,
+                                password_file, private_key,
+                                private_key_file, datafile_in,
+                                datafile_out)
+                            stored = get_stored_data(
+                                ansible_module, res_find, stargs
+                            )
+                            if stored:
+                                vault_data = \
+                                    (stored or b"").decode("utf-8")
+
+                            remove_attrs = {
+                                "symmetric": ["private_key", "public_key"],
+                                "asymmetric": ["password", "ipavaultsalt"],
+                                "standard": [
+                                    "private_key", "public_key",
+                                    "password", "ipavaultsalt"
+                                ],
+                            }
+                            for attr in remove_attrs.get(arg_type, []):
+                                if attr in args:
+                                    del args[attr]
+
+                            if vault_type == 'symmetric':
+                                if 'ipavaultsalt' not in args:
+                                    args['ipavaultsalt'] = os.urandom(32)
+                            else:
+                                args['ipavaultsalt'] = b''
+
+                        if modified:
+                            commands.append([name, "vault_mod_internal", args])
                     else:
                         if vault_type == 'symmetric' \
                            and 'ipavaultsalt' not in args:
@@ -851,16 +891,22 @@ def main():
                                                      ownerservices)
                         commands.append([name, 'vault_add_owner', owner_args])
 
-                pwdargs = data_storage_args(
-                    args, vault_data, password, password_file, private_key,
-                    private_key_file, datafile_in, datafile_out)
                 if any([vault_data, datafile_in]):
-                    commands.append([name, "vault_archive", pwdargs])
+                    if change_passwd:
+                        pwdargs = data_storage_args(
+                            vault_type, args, vault_data, new_password,
+                            new_password_file, private_key, private_key_file,
+                            datafile_in, datafile_out)
+                    else:
+                        pwdargs = data_storage_args(
+                            vault_type, args, vault_data, password,
+                            password_file, private_key, private_key_file,
+                            datafile_in, datafile_out)
 
-                cmds = change_password(
-                    ansible_module, res_find, password, password_file,
-                    new_password, new_password_file)
-                commands.extend(cmds)
+                    pwdargs['override_password'] = True
+                    pwdargs.pop("private_key", None)
+                    pwdargs.pop("private_key_file", None)
+                    commands.append([name, "vault_archive", pwdargs])
 
             elif state == "retrieved":
                 if res_find is None:
@@ -875,8 +921,9 @@ def main():
                     new_password, new_password_file, res_find)
 
                 pwdargs = data_storage_args(
-                    args, vault_data, password, password_file, private_key,
-                    private_key_file, datafile_in, datafile_out)
+                    res_find["ipavaulttype"][0], args, vault_data, password,
+                    password_file, private_key, private_key_file, datafile_in,
+                    datafile_out)
                 if 'data' in pwdargs:
                     del pwdargs['data']
 
@@ -888,6 +935,10 @@ def main():
 
                 if action == "vault":
                     if res_find is not None:
+                        remove = ['ipavaultsalt', 'ipavaultpublickey']
+                        args = {
+                            k: v for k, v in args.items() if k not in remove
+                        }
                         commands.append([name, "vault_del", args])
 
                 elif action == "member":
@@ -910,6 +961,10 @@ def main():
             else:
                 ansible_module.fail_json(msg="Unknown state '%s'" % state)
 
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
         # Execute commands
 
         errors = []
@@ -935,7 +990,7 @@ def main():
                             changed = True
                     else:
                         changed = True
-            except EmptyModlist:
+            except ipalib_errors.EmptyModlist:
                 result = {}
             except Exception as exception:
                 ansible_module.fail_json(

requirements-dev.txt

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

requirements-tests.txt

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

requirements.txt

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

roles/ipabackup/library/ipabackup_get_backup_dir.py

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

roles/ipabackup/tasks/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,229 +45,244 @@ __all__ = ["gssapi", "version", "ipadiscovery", "api", "errors", "x509",
            "configure_firefox", "sync_time", "check_ldap_conf",
            "sssd_enable_ifp"]
 
-from ipapython.version import NUM_VERSION, VERSION
+import sys
+
+# HACK: workaround for Ansible 2.9
+# https://github.com/ansible/ansible/issues/68361
+if 'ansible.executor' in sys.modules:
+    for attr in __all__:
+        setattr(sys.modules[__name__], attr, None)
 
-if NUM_VERSION < 30201:
-    # See ipapython/version.py
-    IPA_MAJOR, IPA_MINOR, IPA_RELEASE = [int(x) for x in VERSION.split(".", 2)]
-    IPA_PYTHON_VERSION = IPA_MAJOR*10000 + IPA_MINOR*100 + IPA_RELEASE
 else:
-    IPA_PYTHON_VERSION = NUM_VERSION
+    from ipapython.version import NUM_VERSION, VERSION
 
+    if NUM_VERSION < 30201:
+        # See ipapython/version.py
+        IPA_MAJOR, IPA_MINOR, IPA_RELEASE = [int(x) for x in
+                                             VERSION.split(".", 2)]
+        IPA_PYTHON_VERSION = IPA_MAJOR*10000 + IPA_MINOR*100 + IPA_RELEASE
+    else:
+        IPA_PYTHON_VERSION = NUM_VERSION
 
-class installer_obj(object):
-    def __init__(self):
-        pass
+    class installer_obj(object):
+        def __init__(self):
+            pass
 
-    def set_logger(self, logger):
-        self.logger = logger
+        def set_logger(self, logger):
+            self.logger = logger
 
-    # def __getattribute__(self, attr):
-    #    value = super(installer_obj, self).__getattribute__(attr)
-    #    if not attr.startswith("--") and not attr.endswith("--"):
-    #        logger.debug(
-    #            "  <-- Accessing installer.%s (%s)" % (attr, repr(value)))
-    #    return value
+        # def __getattribute__(self, attr):
+        #    value = super(installer_obj, self).__getattribute__(attr)
+        #    if not attr.startswith("--") and not attr.endswith("--"):
+        #        logger.debug(
+        #            "  <-- Accessing installer.%s (%s)" % (attr, repr(value)))
+        #    return value
 
-    # def __getattr__(self, attr):
-    #    # logger.info("  --> ADDING missing installer.%s" % attr)
-    #    self.logger.warn("  --> ADDING missing installer.%s" % attr)
-    #    setattr(self, attr, None)
-    #    return getattr(self, attr)
+        # def __getattr__(self, attr):
+        #    # logger.info("  --> ADDING missing installer.%s" % attr)
+        #    self.logger.warn("  --> ADDING missing installer.%s" % attr)
+        #    setattr(self, attr, None)
+        #    return getattr(self, attr)
 
-    # def __setattr__(self, attr, value):
-    #    logger.debug("  --> Setting installer.%s to %s" % (attr, repr(value)))
-    #    return super(installer_obj, self).__setattr__(attr, value)
+        # def __setattr__(self, attr, 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
+        def knobs(self):
+            for name in self.__dict__:
+                yield self, name
 
+    # Initialize installer settings
+    installer = installer_obj()
+    # Create options
+    options = installer
+    options.interactive = False
+    options.unattended = not options.interactive
 
-# Initialize installer settings
-installer = installer_obj()
-# Create options
-options = installer
-options.interactive = False
-options.unattended = not options.interactive
+    if NUM_VERSION >= 40400:
+        # IPA version >= 4.4
 
-if NUM_VERSION >= 40400:
-    # IPA version >= 4.4
+        import sys
+        import inspect
+        import gssapi
+        import logging
 
-    import sys
-    import inspect
-    import gssapi
-    import logging
-
-    from ipapython import version
-    try:
-        from ipaclient.install import ipadiscovery
-    except ImportError:
-        from ipaclient import ipadiscovery
-    from ipalib import api, errors, x509
-    from ipalib import constants
-    try:
-        from ipalib import sysrestore
-    except ImportError:
+        from ipapython import version
         try:
-            from ipalib.install import sysrestore
+            from ipaclient.install import ipadiscovery
         except ImportError:
-            from ipapython import sysrestore
-    try:
-        from ipalib.install import certmonger
-    except ImportError:
-        from ipapython import certmonger
-    try:
-        from ipalib.install import certstore
-    except ImportError:
-        from ipalib import certstore
-    from ipalib.rpc import delete_persistent_client_session_data
-    from ipapython import certdb, ipautil
-    from ipapython.admintool import ScriptError
-    from ipapython.ipautil import CheckedIPAddress
-    from ipalib.util import validate_domain_name, normalize_hostname, \
-        validate_hostname
-    from ipaplatform import services
-    from ipaplatform.paths import paths
-    from ipaplatform.tasks import tasks
-    try:
-        from cryptography.hazmat.primitives import serialization
-    except ImportError:
-        serialization = None
-    from ipapython.ipautil import CalledProcessError, write_tmp_file, \
-        ipa_generate_password
-    from ipapython.dn import DN
-    try:
-        from ipalib.install.kinit import kinit_keytab, kinit_password
-    except ImportError:
-        from ipapython.ipautil import kinit_keytab, kinit_password
-    from ipapython.ipa_log_manager import standard_logging_setup
-    from gssapi.exceptions import GSSError
-    try:
-        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, \
-            hardcode_ldap_server, get_certs_from_ldap, save_state, \
-            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, \
-            check_ip_addresses, print_port_conf_info, configure_ipa_conf, \
-            purge_host_keytab, configure_sssd_conf, configure_ldap_conf, \
-            configure_nslcd_conf, nosssd_files
-        get_ca_cert = None
-    except ImportError:
-        # Create temporary copy of ipa-client-install script (as
-        # ipa_client_install.py) to be able to import the script easily
-        # and also to remove the global finally clause in which the
-        # generated ccache file gets removed. The ccache file will be
-        # needed in the next step.
-        # This is done in a temporary directory that gets removed right
-        # after ipa_client_install has been imported.
-        import shutil
-        import tempfile
-        temp_dir = tempfile.mkdtemp(dir="/tmp")
-        sys.path.append(temp_dir)
-        temp_file = "%s/ipa_client_install.py" % temp_dir
-
-        with open("/usr/sbin/ipa-client-install", "r") as f_in:
-            with open(temp_file, "w") as f_out:
-                for line in f_in:
-                    if line.startswith("finally:"):
-                        break
-                    f_out.write(line)
-        import ipa_client_install
-
-        shutil.rmtree(temp_dir, ignore_errors=True)
-        sys.path.remove(temp_dir)
-
-        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,
-                    filename, client_domain, client_hostname, force=False,
-                    configure_sssd=True):
-                global options
-                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)
-        else:
-            configure_krb5_conf = ipa_client_install.configure_krb5_conf
-        if NUM_VERSION < 40100:
-            get_ca_cert = ipa_client_install.get_ca_cert
-            get_ca_certs = None
-        else:
+            from ipaclient import ipadiscovery
+        from ipalib import api, errors, x509
+        from ipalib import constants
+        try:
+            from ipalib import sysrestore
+        except ImportError:
+            try:
+                from ipalib.install import sysrestore
+            except ImportError:
+                from ipapython import sysrestore
+        try:
+            from ipalib.install import certmonger
+        except ImportError:
+            from ipapython import certmonger
+        try:
+            from ipalib.install import certstore
+        except ImportError:
+            from ipalib import certstore
+        from ipalib.rpc import delete_persistent_client_session_data
+        from ipapython import certdb, ipautil
+        from ipapython.admintool import ScriptError
+        from ipapython.ipautil import CheckedIPAddress
+        from ipalib.util import validate_domain_name, normalize_hostname, \
+            validate_hostname
+        from ipaplatform import services
+        from ipaplatform.paths import paths
+        from ipaplatform.tasks import tasks
+        try:
+            from cryptography.hazmat.primitives import serialization
+        except ImportError:
+            serialization = None
+        from ipapython.ipautil import CalledProcessError, write_tmp_file, \
+            ipa_generate_password
+        from ipapython.dn import DN
+        try:
+            from ipalib.install.kinit import kinit_keytab, kinit_password
+        except ImportError:
+            from ipapython.ipautil import kinit_keytab, kinit_password
+        from ipapython.ipa_log_manager import standard_logging_setup
+        from gssapi.exceptions import GSSError
+        try:
+            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, \
+                hardcode_ldap_server, get_certs_from_ldap, save_state, \
+                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, \
+                check_ip_addresses, print_port_conf_info, configure_ipa_conf, \
+                purge_host_keytab, configure_sssd_conf, configure_ldap_conf, \
+                configure_nslcd_conf, nosssd_files
             get_ca_cert = None
-            get_ca_certs = ipa_client_install.get_ca_certs
-        SECURE_PATH = ("/bin:/sbin:/usr/kerberos/bin:/usr/kerberos/sbin:"
-                       "/usr/bin:/usr/sbin")
-
-        get_server_connection_interface = \
-            ipa_client_install.get_server_connection_interface
-        disable_ra = ipa_client_install.disable_ra
-        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
-        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)
-        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)
-
-        configure_ldap_conf = ipa_client_install.configure_ldap_conf
-        configure_nslcd_conf = ipa_client_install.configure_nslcd_conf
-        nosssd_files = ipa_client_install.nosssd_files
-
-        configure_ssh_config = ipa_client_install.configure_ssh_config
-        configure_sshd_config = ipa_client_install.configure_sshd_config
-        configure_automount = ipa_client_install.configure_automount
-        configure_firefox = ipa_client_install.configure_firefox
-
-    from ipapython.ipautil import realm_to_suffix, run
-
-    try:
-        from ipaclient.install import timeconf
-        time_service = "chronyd"
-    except ImportError:
-        try:
-            from ipaclient.install import ntpconf as timeconf
         except ImportError:
-            from ipaclient import ntpconf as timeconf
-        time_service = "ntpd"
+            # Create temporary copy of ipa-client-install script (as
+            # ipa_client_install.py) to be able to import the script easily
+            # and also to remove the global finally clause in which the
+            # generated ccache file gets removed. The ccache file will be
+            # needed in the next step.
+            # This is done in a temporary directory that gets removed right
+            # after ipa_client_install has been imported.
+            import shutil
+            import tempfile
+            temp_dir = tempfile.mkdtemp(dir="/tmp")
+            sys.path.append(temp_dir)
+            temp_file = "%s/ipa_client_install.py" % temp_dir
 
-    try:
-        from ipaclient.install.client import sync_time
-    except ImportError:
-        sync_time = None
+            with open("/usr/sbin/ipa-client-install", "r") as f_in:
+                with open(temp_file, "w") as f_out:
+                    for line in f_in:
+                        if line.startswith("finally:"):
+                            break
+                        f_out.write(line)
+            import ipa_client_install
 
-    try:
-        from ipaclient.install.client import check_ldap_conf
-    except ImportError:
-        check_ldap_conf = None
+            shutil.rmtree(temp_dir, ignore_errors=True)
+            sys.path.remove(temp_dir)
 
-    try:
-        from ipaclient.install.client import sssd_enable_ifp
-    except ImportError:
-        sssd_enable_ifp = None
+            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,
+                        filename, client_domain, client_hostname, force=False,
+                        configure_sssd=True):
+                    global options
+                    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)
+            else:
+                configure_krb5_conf = ipa_client_install.configure_krb5_conf
+            if NUM_VERSION < 40100:
+                get_ca_cert = ipa_client_install.get_ca_cert
+                get_ca_certs = None
+            else:
+                get_ca_cert = None
+                get_ca_certs = ipa_client_install.get_ca_certs
+            SECURE_PATH = ("/bin:/sbin:/usr/kerberos/bin:/usr/kerberos/sbin:"
+                           "/usr/bin:/usr/sbin")
 
-    logger = logging.getLogger("ipa-client-install")
-    root_logger = logger
+            get_server_connection_interface = \
+                ipa_client_install.get_server_connection_interface
+            disable_ra = ipa_client_install.disable_ra
+            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
+            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
 
-else:
-    # IPA version < 4.4
+            create_ipa_nssdb = certdb.create_ipa_nssdb
 
-    raise Exception("freeipa version '%s' is too old" % VERSION)
+            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)
+
+            configure_ldap_conf = ipa_client_install.configure_ldap_conf
+            configure_nslcd_conf = ipa_client_install.configure_nslcd_conf
+            nosssd_files = ipa_client_install.nosssd_files
+
+            configure_ssh_config = ipa_client_install.configure_ssh_config
+            configure_sshd_config = ipa_client_install.configure_sshd_config
+            configure_automount = ipa_client_install.configure_automount
+            configure_firefox = ipa_client_install.configure_firefox
+
+        from ipapython.ipautil import realm_to_suffix, run
+
+        try:
+            from ipaclient.install import timeconf
+            time_service = "chronyd"
+        except ImportError:
+            try:
+                from ipaclient.install import ntpconf as timeconf
+            except ImportError:
+                from ipaclient import ntpconf as timeconf
+            time_service = "ntpd"
+
+        try:
+            from ipaclient.install.client import sync_time
+        except ImportError:
+            sync_time = None
+
+        try:
+            from ipaclient.install.client import check_ldap_conf
+        except ImportError:
+            check_ldap_conf = None
+
+        try:
+            from ipaclient.install.client import sssd_enable_ifp
+        except ImportError:
+            sssd_enable_ifp = None
+
+        logger = logging.getLogger("ipa-client-install")
+        root_logger = logger
+
+    else:
+        # IPA version < 4.4
+
+        raise Exception("freeipa version '%s' is too old" % VERSION)
 
 
 def setup_logging():

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,379 +46,383 @@ __all__ = ["contextlib", "dnsexception", "dnsresolver", "dnsreversename",
            "dnsname", "kernel_keyring", "krbinstance"]
 
 import sys
-import logging
-from contextlib import contextmanager as contextlib_contextmanager
 
-
-from ipapython.version import NUM_VERSION, VERSION
-
-if NUM_VERSION < 30201:
-    # See ipapython/version.py
-    IPA_MAJOR, IPA_MINOR, IPA_RELEASE = [int(x) for x in VERSION.split(".", 2)]
-    IPA_PYTHON_VERSION = IPA_MAJOR*10000 + IPA_MINOR*100 + IPA_RELEASE
+# HACK: workaround for Ansible 2.9
+# https://github.com/ansible/ansible/issues/68361
+if 'ansible.executor' in sys.modules:
+    for attr in __all__:
+        setattr(sys.modules[__name__], attr, None)
 else:
-    IPA_PYTHON_VERSION = NUM_VERSION
+    import logging
+    from contextlib import contextmanager as contextlib_contextmanager
 
+    from ipapython.version import NUM_VERSION, VERSION
 
-if NUM_VERSION >= 40600:
-    # IPA version >= 4.6
+    if NUM_VERSION < 30201:
+        # See ipapython/version.py
+        IPA_MAJOR, IPA_MINOR, IPA_RELEASE = [int(x) for x in
+                                             VERSION.split(".", 2)]
+        IPA_PYTHON_VERSION = IPA_MAJOR*10000 + IPA_MINOR*100 + IPA_RELEASE
+    else:
+        IPA_PYTHON_VERSION = NUM_VERSION
 
-    import contextlib
+    if NUM_VERSION >= 40600:
+        # IPA version >= 4.6
 
-    import dns.exception as dnsexception
-    import dns.name as dnsname
-    import dns.resolver as dnsresolver
-    import dns.reversename as dnsreversename
+        import contextlib
 
-    from pkg_resources import parse_version
+        import dns.exception as dnsexception
+        import dns.name as dnsname
+        import dns.resolver as dnsresolver
+        import dns.reversename as dnsreversename
 
-    from ipaclient.install.ipachangeconf import IPAChangeConf
-    from ipalib.install import certstore, sysrestore
-    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.dn import DN
-    from ipapython.admintool import ScriptError
-    from ipapython.ipa_log_manager import standard_logging_setup
-    from ipaplatform import services
-    from ipaplatform.tasks import tasks
-    from ipaplatform.paths import paths
-    from ipalib import api, constants, create_api, errors, rpc, x509
-    from ipalib.config import Env
-    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 ipaserver.install import (
-        adtrust, bindinstance, ca, certs, dns, dsinstance, httpinstance,
-        installutils, kra, krbinstance,
-        otpdinstance, custodiainstance, service, upgradeinstance)
-    try:
-        from ipaserver.masters import (
-            find_providing_servers, find_providing_server)
-    except ImportError:
-        from ipaserver.install.service import (
-            find_providing_servers, find_providing_server)
-    from ipaserver.install.installutils import (
-        ReplicaConfig, load_pkcs12)
-    try:
-        from ipalib.facts import is_ipa_configured
-    except ImportError:
-        from ipaserver.install.installutils import is_ipa_configured
-    from ipaserver.install.replication import (
-        ReplicationManager, replica_conn_check)
-    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,
-        # common_cleanup,
-        preserve_enrollment_state, uninstall_client,
-        promote_sssd, promote_openldap_conf, rpc_client,
-        check_remote_fips_mode, check_remote_version, common_check,
-        current_domain_level, check_domain_level_is_supported,
-        # enroll_dl0_replica,
-        # ensure_enrolled,
-        promotion_check_ipa_domain
-    )
-    import SSSDConfig
-    from subprocess import CalledProcessError
+        from pkg_resources import parse_version
 
-    try:
-        from ipaclient.install import timeconf
-        time_service = "chronyd"
-        ntpinstance = None
-    except ImportError:
+        from ipaclient.install.ipachangeconf import IPAChangeConf
+        from ipalib.install import certstore, sysrestore
+        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.dn import DN
+        from ipapython.admintool import ScriptError
+        from ipapython.ipa_log_manager import standard_logging_setup
+        from ipaplatform import services
+        from ipaplatform.tasks import tasks
+        from ipaplatform.paths import paths
+        from ipalib import api, constants, create_api, errors, rpc, x509
+        from ipalib.config import Env
+        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 ipaserver.install import (
+            adtrust, bindinstance, ca, certs, dns, dsinstance, httpinstance,
+            installutils, kra, krbinstance,
+            otpdinstance, custodiainstance, service, upgradeinstance)
         try:
-            from ipaclient.install import ntpconf as timeconf
+            from ipaserver.masters import (
+                find_providing_servers, find_providing_server)
         except ImportError:
-            from ipaclient import ntpconf as timeconf
-        from ipaserver.install import ntpinstance
-        time_service = "ntpd"
+            from ipaserver.install.service import (
+                find_providing_servers, find_providing_server)
+        from ipaserver.install.installutils import (
+            ReplicaConfig, load_pkcs12)
+        try:
+            from ipalib.facts import is_ipa_configured
+        except ImportError:
+            from ipaserver.install.installutils import is_ipa_configured
+        from ipaserver.install.replication import (
+            ReplicationManager, replica_conn_check)
+        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,
+            # common_cleanup,
+            preserve_enrollment_state, uninstall_client,
+            promote_sssd, promote_openldap_conf, rpc_client,
+            check_remote_fips_mode, check_remote_version, common_check,
+            current_domain_level, check_domain_level_is_supported,
+            # enroll_dl0_replica,
+            # ensure_enrolled,
+            promotion_check_ipa_domain
+        )
+        import SSSDConfig
+        from subprocess import CalledProcessError
 
+        try:
+            from ipaclient.install import timeconf
+            time_service = "chronyd"
+            ntpinstance = None
+        except ImportError:
+            try:
+                from ipaclient.install import ntpconf as timeconf
+            except ImportError:
+                from ipaclient import ntpconf as timeconf
+            from ipaserver.install import ntpinstance
+            time_service = "ntpd"
 
-else:
-    # IPA version < 4.6
+    else:
+        # IPA version < 4.6
 
-    raise Exception("freeipa version '%s' is too old" % VERSION)
+        raise Exception("freeipa version '%s' is too old" % VERSION)
 
+    logger = logging.getLogger("ipa-server-install")
 
-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
+        try:
+            yield f
+        finally:
+            sys.stdout = sys.__stdout__
 
-def setup_logging():
-    # logger.setLevel(logging.DEBUG)
-    standard_logging_setup(
-        paths.IPAREPLICA_INSTALL_LOG, verbose=False, debug=False,
-        filemode='a', console_format='%(message)s')
+    class AnsibleModuleLog():
+        def __init__(self, module):
+            self.module = module
+            _ansible_module_log = self
 
+            class AnsibleLoggingHandler(logging.Handler):
+                def emit(self, record):
+                    _ansible_module_log.write(self.format(record))
 
-@contextlib_contextmanager
-def redirect_stdout(f):
-    sys.stdout = f
-    try:
-        yield f
-    finally:
-        sys.stdout = sys.__stdout__
+            self.logging_handler = AnsibleLoggingHandler()
+            logger.setLevel(logging.DEBUG)
+            logger.root.addHandler(self.logging_handler)
 
+        def close(self):
+            self.flush()
 
-class AnsibleModuleLog():
-    def __init__(self, module):
-        self.module = module
-        _ansible_module_log = self
+        def flush(self):
+            pass
 
-        class AnsibleLoggingHandler(logging.Handler):
-            def emit(self, record):
-                _ansible_module_log.write(self.format(record))
+        def log(self, msg):
+            # self.write(msg+"\n")
+            self.write(msg)
 
-        self.logging_handler = AnsibleLoggingHandler()
-        logger.setLevel(logging.DEBUG)
-        logger.root.addHandler(self.logging_handler)
+        def debug(self, msg):
+            self.module.debug(msg)
 
-    def close(self):
-        self.flush()
+        def info(self, msg):
+            self.module.debug(msg)
 
-    def flush(self):
-        pass
+        def write(self, msg):
+            self.module.debug(msg)
+            # self.module.warn(msg)
 
-    def log(self, msg):
-        # self.write(msg+"\n")
-        self.write(msg)
-
-    def debug(self, msg):
-        self.module.debug(msg)
-
-    def info(self, msg):
-        self.module.debug(msg)
-
-    def write(self, msg):
-        self.module.debug(msg)
-        # self.module.warn(msg)
-
-
-class installer_obj(object):
-    def __init__(self):
-        # CompatServerReplicaInstall
-        self.ca_cert_files = None
-        self.all_ip_addresses = False
-        self.no_wait_for_dns = True
-        self.nisdomain = None
-        self.no_nisdomain = False
-        self.no_sudo = False
-        self.request_cert = False
-        self.ca_file = None
-        self.zonemgr = None
-        self.replica_file = None
-        # ServerReplicaInstall
-        self.subject_base = None
-        self.ca_subject = None
-        # others
-        self._ccache = None
-        self.password = None
-        self.reverse_zones = []
-        # def _is_promote(self):
-        #     return self.replica_file is None
-        # self.skip_conncheck = False
-        self._replica_install = False
-        # self.dnssec_master = False # future unknown
-        # self.disable_dnssec_master = False # future unknown
-        # self.domainlevel = MAX_DOMAIN_LEVEL # deprecated
-        # self.domain_level = self.domainlevel # deprecated
-        self.interactive = False
-        self.unattended = not self.interactive
-        # self.promote = self.replica_file is None
-        self.promote = True
-        self.skip_schema_check = None
-
-    # def __getattribute__(self, attr):
-    #     value = super(installer_obj, self).__getattribute__(attr)
-    #     if not attr.startswith("--") and not attr.endswith("--"):
-    #         logger.debug(
-    #             "  <-- Accessing installer.%s (%s)" % (attr, repr(value)))
-    #     return value
-
-    def __getattr__(self, attr):
-        logger.info("  --> ADDING missing installer.%s", attr)
-        setattr(self, attr, None)
-        return getattr(self, attr)
-
-    # def __setattr__(self, attr, 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
-
-# DNSInstallInterface
-options.dnssec_master = False
-options.disable_dnssec_master = False
-options.kasp_db_file = None
-options.force = False
-
-# ServerMasterInstall
-options.add_sids = False
-options.add_agents = False
-
-# ServerReplicaInstall
-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)
-    # env._finalize_core(**dict(constants.DEFAULT_CONFIG))
-    env._bootstrap(context='installer', confdir=etc_ipa, log=None)
-    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))
-    api.bootstrap(in_server=True,
-                  context='installer',
-                  confdir=paths.ETC_IPA,
-                  ldap_uri=installutils.realm_to_ldapi_uri(env.realm),
-                  xmlrpc_uri=xmlrpc_uri)
-    # pylint: enable=no-member
-    api.finalize()
-
-
-def gen_ReplicaConfig():
-    class ExtendedReplicaConfig(ReplicaConfig):
-        def __init__(self, top_dir=None):
-            super(ExtendedReplicaConfig, self).__init__(top_dir)
+    class installer_obj(object):
+        def __init__(self):
+            # CompatServerReplicaInstall
+            self.ca_cert_files = None
+            self.all_ip_addresses = False
+            self.no_wait_for_dns = True
+            self.nisdomain = None
+            self.no_nisdomain = False
+            self.no_sudo = False
+            self.request_cert = False
+            self.ca_file = None
+            self.zonemgr = None
+            self.replica_file = None
+            # ServerReplicaInstall
+            self.subject_base = None
+            self.ca_subject = None
+            # others
+            self._ccache = None
+            self.password = None
+            self.reverse_zones = []
+            # def _is_promote(self):
+            #     return self.replica_file is None
+            # self.skip_conncheck = False
+            self._replica_install = False
+            # self.dnssec_master = False # future unknown
+            # self.disable_dnssec_master = False # future unknown
+            # self.domainlevel = MAX_DOMAIN_LEVEL # deprecated
+            # self.domain_level = self.domainlevel # deprecated
+            self.interactive = False
+            self.unattended = not self.interactive
+            # self.promote = self.replica_file is None
+            self.promote = True
+            self.skip_schema_check = None
 
         # def __getattribute__(self, attr):
-        #    value = super(ExtendedReplicaConfig, self).__getattribute__(attr)
-        #    if attr not in ["__dict__", "knobs"]:
-        #        logger.debug("  <== Accessing config.%s (%s)" %
-        #                     (attr, repr(value)))
-        #    return value
+        #     value = super(installer_obj, self).__getattribute__(attr)
+        #     if not attr.startswith("--") and not attr.endswith("--"):
+        #         logger.debug(
+        #             "  <-- Accessing installer.%s (%s)" %
+        #             (attr, repr(value)))
+        #     return value
 
         def __getattr__(self, attr):
-            logger.info("  ==> ADDING missing config.%s", attr)
+            logger.info("  --> ADDING missing installer.%s", attr)
             setattr(self, attr, None)
             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 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
 
-    # config = ReplicaConfig()
-    config = ExtendedReplicaConfig()
-    config.realm_name = api.env.realm
-    config.host_name = api.env.host
-    config.domain_name = api.env.domain
-    config.master_host_name = api.env.server
-    config.ca_host_name = api.env.ca_host
-    config.kra_host_name = config.ca_host_name
-    config.ca_ds_port = 389
-    config.setup_ca = options.setup_ca
-    config.setup_kra = options.setup_kra
-    config.dir = options._top_dir
-    config.basedn = api.env.basedn
-    # config.subject_base = options.subject_base
+    installer = installer_obj()
+    options = installer
 
-    return config
+    # DNSInstallInterface
+    options.dnssec_master = False
+    options.disable_dnssec_master = False
+    options.kasp_db_file = None
+    options.force = False
 
+    # ServerMasterInstall
+    options.add_sids = False
+    options.add_agents = False
 
-def replica_ds_init_info(ansible_log,
-                         config, options, ca_is_configured, remote_api,
-                         ds_ca_subject, ca_file,
-                         promote=False, pkcs12_info=None):
+    # ServerReplicaInstall
+    options.subject_base = None
+    options.ca_subject = None
 
-    dsinstance.check_ports()
+    def gen_env_boostrap_finalize_core(etc_ipa, default_config):
+        env = Env()
+        # env._bootstrap(context='installer', confdir=paths.ETC_IPA, log=None)
+        # env._finalize_core(**dict(constants.DEFAULT_CONFIG))
+        env._bootstrap(context='installer', confdir=etc_ipa, log=None)
+        env._finalize_core(**dict(default_config))
+        return env
 
-    # if we have a pkcs12 file, create the cert db from
-    # that. Otherwise the ds setup will create the CA
-    # cert
-    if pkcs12_info is None:
-        pkcs12_info = make_pkcs12_info(config.dir, "dscert.p12",
-                                       "dirsrv_pin.txt")
+    def api_bootstrap_finalize(env):
+        # pylint: disable=no-member
+        xmlrpc_uri = \
+            'https://{}/ipa/xml'.format(ipautil.format_netloc(env.host))
+        api.bootstrap(in_server=True,
+                      context='installer',
+                      confdir=paths.ETC_IPA,
+                      ldap_uri=installutils.realm_to_ldapi_uri(env.realm),
+                      xmlrpc_uri=xmlrpc_uri)
+        # pylint: enable=no-member
+        api.finalize()
 
-    # during replica install, this gets invoked before local DS is
-    # available, so use the remote api.
-    # 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 = ds_ca_subject
+    def gen_ReplicaConfig():
+        class ExtendedReplicaConfig(ReplicaConfig):
+            def __init__(self, top_dir=None):
+                super(ExtendedReplicaConfig, self).__init__(top_dir)
 
-    ds = dsinstance.DsInstance(
-        config_ldif=options.dirsrv_config_file)
-    ds.set_output(ansible_log)
+            # def __getattribute__(self, attr):
+            #     value = super(ExtendedReplicaConfig, self).__getattribute__(
+            #         attr)
+            #    if attr not in ["__dict__", "knobs"]:
+            #        logger.debug("  <== Accessing config.%s (%s)" %
+            #                     (attr, repr(value)))
+            #    return value
 
-    # Source: ipaserver/install/dsinstance.py
+            def __getattr__(self, attr):
+                logger.info("  ==> ADDING missing config.%s", attr)
+                setattr(self, attr, None)
+                return getattr(self, attr)
 
-    # idstart and idmax are configured so that the range is seen as
-    # depleted by the DNA plugin and the replica will go and get a
-    # new range from the master.
-    # This way all servers use the initially defined range by default.
-    idstart = 1101
-    idmax = 1100
+            # def __setattr__(self, attr, value):
+            #   logger.debug("  ==> Setting config.%s to %s" %
+            #                (attr, repr(value)))
+            #   return super(ExtendedReplicaConfig, self).__setattr__(attr,
+            #                                                         value)
 
-    with redirect_stdout(ansible_log):
-        ds.init_info(
-            realm_name=config.realm_name,
-            fqdn=config.host_name,
-            domain_name=config.domain_name,
-            dm_password=config.dirman_password,
-            subject_base=config.subject_base,
-            ca_subject=ca_subject,
-            idstart=idstart,
-            idmax=idmax,
-            pkcs12_info=pkcs12_info,
-            ca_file=ca_file,
-            setup_pkinit=not options.no_pkinit,
-        )
-    ds.master_fqdn = config.master_host_name
-    if ca_is_configured is not None:
-        ds.ca_is_configured = ca_is_configured
-    ds.promote = promote
-    ds.api = remote_api
+            def knobs(self):
+                for name in self.__dict__:
+                    yield self, name
 
-    # from __setup_replica
+        # config = ReplicaConfig()
+        config = ExtendedReplicaConfig()
+        config.realm_name = api.env.realm
+        config.host_name = api.env.host
+        config.domain_name = api.env.domain
+        config.master_host_name = api.env.server
+        config.ca_host_name = api.env.ca_host
+        config.kra_host_name = config.ca_host_name
+        config.ca_ds_port = 389
+        config.setup_ca = options.setup_ca
+        config.setup_kra = options.setup_kra
+        config.dir = options._top_dir
+        config.basedn = api.env.basedn
+        # config.subject_base = options.subject_base
 
-    # Always connect to ds over ldapi
-    ldap_uri = ipaldap.get_ldap_uri(protocol='ldapi', realm=ds.realm)
-    conn = ipaldap.LDAPClient(ldap_uri)
-    conn.external_bind()
+        return config
 
-    return ds
+    def replica_ds_init_info(ansible_log,
+                             config, options, ca_is_configured, remote_api,
+                             ds_ca_subject, ca_file,
+                             promote=False, pkcs12_info=None):
 
+        dsinstance.check_ports()
 
-def ansible_module_get_parsed_ip_addresses(ansible_module,
-                                           param='ip_addresses'):
-    ip_addrs = []
-    for ip in ansible_module.params.get(param):
-        try:
-            ip_parsed = ipautil.CheckedIPAddress(ip)
-        except Exception as e:
-            ansible_module.fail_json(msg="Invalid IP Address %s: %s" % (ip, e))
-        ip_addrs.append(ip_parsed)
-    return ip_addrs
+        # if we have a pkcs12 file, create the cert db from
+        # that. Otherwise the ds setup will create the CA
+        # cert
+        if pkcs12_info is None:
+            pkcs12_info = make_pkcs12_info(config.dir, "dscert.p12",
+                                           "dirsrv_pin.txt")
 
+        # during replica install, this gets invoked before local DS is
+        # available, so use the remote api.
+        # 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 = ds_ca_subject
 
-def gen_remote_api(master_host_name, etc_ipa):
-    ldapuri = 'ldaps://%s' % ipautil.format_netloc(master_host_name)
-    xmlrpc_uri = 'https://{}/ipa/xml'.format(
-        ipautil.format_netloc(master_host_name))
-    remote_api = create_api(mode=None)
-    remote_api.bootstrap(in_server=True,
-                         context='installer',
-                         confdir=etc_ipa,
-                         ldap_uri=ldapuri,
-                         xmlrpc_uri=xmlrpc_uri)
-    remote_api.finalize()
-    return remote_api
+        ds = dsinstance.DsInstance(
+            config_ldif=options.dirsrv_config_file)
+        ds.set_output(ansible_log)
+
+        # Source: ipaserver/install/dsinstance.py
+
+        # idstart and idmax are configured so that the range is seen as
+        # depleted by the DNA plugin and the replica will go and get a
+        # new range from the master.
+        # This way all servers use the initially defined range by default.
+        idstart = 1101
+        idmax = 1100
+
+        with redirect_stdout(ansible_log):
+            ds.init_info(
+                realm_name=config.realm_name,
+                fqdn=config.host_name,
+                domain_name=config.domain_name,
+                dm_password=config.dirman_password,
+                subject_base=config.subject_base,
+                ca_subject=ca_subject,
+                idstart=idstart,
+                idmax=idmax,
+                pkcs12_info=pkcs12_info,
+                ca_file=ca_file,
+                setup_pkinit=not options.no_pkinit,
+            )
+        ds.master_fqdn = config.master_host_name
+        if ca_is_configured is not None:
+            ds.ca_is_configured = ca_is_configured
+        ds.promote = promote
+        ds.api = remote_api
+
+        # from __setup_replica
+
+        # Always connect to ds over ldapi
+        ldap_uri = ipaldap.get_ldap_uri(protocol='ldapi', realm=ds.realm)
+        conn = ipaldap.LDAPClient(ldap_uri)
+        conn.external_bind()
+
+        return ds
+
+    def ansible_module_get_parsed_ip_addresses(ansible_module,
+                                               param='ip_addresses'):
+        ip_addrs = []
+        for ip in ansible_module.params.get(param):
+            try:
+                ip_parsed = ipautil.CheckedIPAddress(ip)
+            except Exception as e:
+                ansible_module.fail_json(
+                    msg="Invalid IP Address %s: %s" % (ip, e))
+            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(
+            ipautil.format_netloc(master_host_name))
+        remote_api = create_api(mode=None)
+        remote_api.bootstrap(in_server=True,
+                             context='installer',
+                             confdir=etc_ipa,
+                             ldap_uri=ldapuri,
+                             xmlrpc_uri=xmlrpc_uri)
+        remote_api.finalize()
+        return remote_api

roles/ipareplica/tasks/install.yml

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

roles/ipareplica/tasks/main.yml

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

roles/ipareplica/tasks/uninstall.yml

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

roles/ipaserver/README.md

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

roles/ipaserver/module_utils/ansible_ipa_server.py

@@ -41,352 +41,349 @@ __all__ = ["IPAChangeConf", "certmonger", "sysrestore", "root_logger",
            "check_available_memory"]
 
 import sys
-import logging
-from contextlib import contextmanager as contextlib_contextmanager
-import six
-import base64
 
+# HACK: workaround for Ansible 2.9
+# https://github.com/ansible/ansible/issues/68361
+if 'ansible.executor' in sys.modules:
+    for attr in __all__:
+        setattr(sys.modules[__name__], attr, None)
 
-from ipapython.version import NUM_VERSION, VERSION
-
-if NUM_VERSION < 30201:
-    # See ipapython/version.py
-    IPA_MAJOR, IPA_MINOR, IPA_RELEASE = [int(x) for x in VERSION.split(".", 2)]
-    IPA_PYTHON_VERSION = IPA_MAJOR*10000 + IPA_MINOR*100 + IPA_RELEASE
 else:
-    IPA_PYTHON_VERSION = NUM_VERSION
 
+    import logging
+    from contextlib import contextmanager as contextlib_contextmanager
+    import six
+    import base64
 
-if NUM_VERSION >= 40500:
-    # IPA version >= 4.5
+    from ipapython.version import NUM_VERSION, VERSION
 
-    from ipaclient.install.ipachangeconf import IPAChangeConf
-    from ipalib.install import certmonger
-    try:
-        from ipalib import sysrestore
-    except ImportError:
-        from ipalib.install import sysrestore
-    from ipapython import ipautil
-    from ipapython.ipa_log_manager import standard_logging_setup
-    try:
-        from ipapython.ipa_log_manager import root_logger
-    except ImportError:
-        root_logger = None
-    from ipapython.ipautil import (
-        ipa_generate_password, run)
-    from ipapython.admintool import ScriptError
-    from ipaplatform import services
-    from ipaplatform.paths import paths
-    from ipaplatform.tasks import tasks
-    from ipalib import api, errors, x509
-    from ipalib.constants import DOMAIN_LEVEL_0, MIN_DOMAIN_LEVEL, \
-        MAX_DOMAIN_LEVEL
-    try:
-        from ipalib.constants import IPAAPI_USER
-    except ImportError:
-        IPAAPI_USER = None
-    from ipalib.util import (
-        validate_domain_name,
-        no_matching_interface_for_ip_address_warning,
-    )
-    from ipapython.dnsutil import check_zone_overlap
-    from ipapython.dn import DN
-    try:
-        from ipaclient.install import timeconf
-        from ipaclient.install.client import sync_time
-        time_service = "chronyd"
-        ntpinstance = None
-    except ImportError:
+    if NUM_VERSION < 30201:
+        # See ipapython/version.py
+        IPA_MAJOR, IPA_MINOR, IPA_RELEASE = [int(x) for x in
+                                             VERSION.split(".", 2)]
+        IPA_PYTHON_VERSION = IPA_MAJOR*10000 + IPA_MINOR*100 + IPA_RELEASE
+    else:
+        IPA_PYTHON_VERSION = NUM_VERSION
+
+    if NUM_VERSION >= 40500:
+        # IPA version >= 4.5
+
+        from ipaclient.install.ipachangeconf import IPAChangeConf
+        from ipalib.install import certmonger
         try:
-            from ipaclient.install import ntpconf as timeconf
+            from ipalib import sysrestore
         except ImportError:
-            from ipaclient import ntpconf as timeconf
-        from ipaserver.install import ntpinstance
-        time_service = "ntpd"
-        sync_time = None
-    from ipaserver.install import (
-        adtrust, bindinstance, ca, dns, dsinstance,
-        httpinstance, installutils, kra, krbinstance,
-        otpdinstance, custodiainstance, replication, service,
-        sysupgrade)
-    adtrust_imported = True
-    kra_imported = True
-    from ipaserver.install.installutils import (
-        BadHostError, get_fqdn, get_server_ip_address,
-        load_pkcs12, read_password, verify_fqdn,
-        update_hosts_file)
-    try:
-        from ipalib.facts import is_ipa_configured
-    except ImportError:
-        from ipaserver.install.installutils import is_ipa_configured
-    from ipaserver.install.server.install import (
-        check_dirsrv, validate_admin_password, validate_dm_password,
-        read_cache, write_cache)
-    try:
-        from ipaserver.install.dogtaginstance import PKIIniLoader
-    except ImportError:
-        PKIIniLoader = None
-    try:
-        from ipaserver.install.installutils import default_subject_base
-    except ImportError:
-        def default_subject_base(realm_name):
-            return DN(('O', realm_name))
-    try:
-        from ipalib.facts import IPA_MODULES
-    except ImportError:
-        from ipaserver.install.installutils import IPA_MODULES
-    try:
-        from ipaserver.install.installutils import default_ca_subject_dn
-    except ImportError:
-        def default_ca_subject_dn(subject_base):
-            return DN(('CN', 'Certificate Authority'), subject_base)
-    try:
-        from ipaserver.install.installutils import check_available_memory
-    except ImportError:
-        check_available_memory = None
-
-    try:
-        from ipaserver.install import adtrustinstance
-        _server_trust_ad_installed = True
-    except ImportError:
-        _server_trust_ad_installed = False
-
-    try:
-        from ipaclient.install.client import check_ldap_conf
-    except ImportError:
-        check_ldap_conf = None
-
-    try:
-        from ipalib.x509 import Encoding
-    except ImportError:
-        from cryptography.hazmat.primitives.serialization import Encoding
-
-    try:
-        from ipalib.x509 import load_pem_x509_certificate
-    except ImportError:
-        from ipalib.x509 import load_certificate
-        load_pem_x509_certificate = None
-
-else:
-    # IPA version < 4.5
-
-    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
-    try:
-        yield f
-    finally:
-        sys.stdout = sys.__stdout__
-
-
-class AnsibleModuleLog():
-    def __init__(self, module):
-        self.module = module
-        _ansible_module_log = self
-
-        class AnsibleLoggingHandler(logging.Handler):
-            def emit(self, record):
-                _ansible_module_log.write(self.format(record))
-
-        self.logging_handler = AnsibleLoggingHandler()
-        logger.setLevel(logging.DEBUG)
-        logger.root.addHandler(self.logging_handler)
-
-    def close(self):
-        self.flush()
-
-    def flush(self):
-        pass
-
-    def log(self, msg):
-        # self.write(msg+"\n")
-        self.write(msg)
-
-    def debug(self, msg):
-        self.module.debug(msg)
-
-    def info(self, msg):
-        self.module.debug(msg)
-
-    def write(self, msg):
-        self.module.debug(msg)
-        # self.module.warn(msg)
-
-
-class options_obj(object):
-    def __init__(self):
-        self._replica_install = False
-        self.dnssec_master = False  # future unknown
-        self.disable_dnssec_master = False  # future unknown
-        self.domainlevel = MAX_DOMAIN_LEVEL  # deprecated
-        self.domain_level = self.domainlevel  # deprecated
-        self.interactive = False
-        self.unattended = not self.interactive
-
-    # def __getattribute__(self, attr):
-    #    logger.info(" <-- Accessing options.%s" % attr)
-    #    return super(options_obj, self).__getattribute__(attr)
-
-    # def __getattr__(self, attr):
-    #    logger.info(" --> Adding missing options.%s" % attr)
-    #    setattr(self, attr, None)
-    #    return getattr(self, attr)
-
-    def knobs(self):
-        for name in self.__dict__:
-            yield self, name
-
-
-options = options_obj()
-installer = options
-
-# ServerMasterInstall
-options.add_sids = True
-options.add_agents = False
-
-
-# Installable
-options.uninstalling = False
-
-# ServerInstallInterface
-options.description = "Server"
-
-options.kinit_attempts = 1
-options.fixed_primary = True
-options.permit = False
-options.enable_dns_updates = False
-options.no_krb5_offline_passwords = False
-options.preserve_sssd = False
-options.no_sssd = False
-
-# ServerMasterInstall
-options.force_join = False
-options.servers = None
-options.no_wait_for_dns = True
-options.host_password = None
-options.keytab = None
-options.setup_ca = True
-# always run sidgen task and do not allow adding agents on first master
-options.add_sids = True
-options.add_agents = False
-
-# ADTrustInstallInterface
-# no_msdcs is deprecated
-options.no_msdcs = False
-
-# For pylint
-options.external_cert_files = None
-options.dirsrv_cert_files = None
-
-# Uninstall
-options.ignore_topology_disconnect = False
-options.ignore_last_of_role = False
-
-
-def api_Backend_ldap2(host_name, setup_ca, connect=False):
-    # we are sure we have the configuration file ready.
-    cfg = dict(context='installer', confdir=paths.ETC_IPA, in_server=True,
-               host=host_name)
-    if setup_ca:
-        # we have an IPA-integrated CA
-        cfg['ca_host'] = host_name
-
-    api.bootstrap(**cfg)
-    api.finalize()
-    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,
-                 no_hbac_allow, dirsrv_pkcs12_info, no_pkinit):
-
-    if not options.external_cert_files:
-        ds = dsinstance.DsInstance(fstore=fstore, domainlevel=domainlevel,
-                                   config_ldif=dirsrv_config_file)
-        ds.set_output(ansible_log)
-
-        if options.dirsrv_cert_files:
-            _dirsrv_pkcs12_info = dirsrv_pkcs12_info
-        else:
-            _dirsrv_pkcs12_info = None
-
-        with redirect_stdout(ansible_log):
-            ds.init_info(realm_name, host_name, domain_name, dm_password,
-                         subject_base, ca_subject, idstart, idmax,
-                         # hbac_allow=not no_hbac_allow,
-                         _dirsrv_pkcs12_info, setup_pkinit=not no_pkinit)
-    else:
-        ds = dsinstance.DsInstance(fstore=fstore, domainlevel=domainlevel)
-        ds.set_output(ansible_log)
-
-        with redirect_stdout(ansible_log):
-            ds.init_info(realm_name, host_name, domain_name, dm_password,
-                         subject_base, ca_subject, 1101, 1100, None,
-                         setup_pkinit=not no_pkinit)
-
-    return ds
-
-
-def ansible_module_get_parsed_ip_addresses(ansible_module,
-                                           param='ip_addresses'):
-    ip_addrs = []
-    for ip in ansible_module.params.get(param):
+            from ipalib.install import sysrestore
+        from ipapython import ipautil
+        from ipapython.ipa_log_manager import standard_logging_setup
         try:
-            ip_parsed = ipautil.CheckedIPAddress(ip)
-        except Exception as e:
-            ansible_module.fail_json(msg="Invalid IP Address %s: %s" % (ip, e))
-        ip_addrs.append(ip_parsed)
-    return ip_addrs
+            from ipapython.ipa_log_manager import root_logger
+        except ImportError:
+            root_logger = None
+        from ipapython.ipautil import (
+            ipa_generate_password, run)
+        from ipapython.admintool import ScriptError
+        from ipaplatform import services
+        from ipaplatform.paths import paths
+        from ipaplatform.tasks import tasks
+        from ipalib import api, errors, x509
+        from ipalib.constants import DOMAIN_LEVEL_0, MIN_DOMAIN_LEVEL, \
+            MAX_DOMAIN_LEVEL
+        try:
+            from ipalib.constants import IPAAPI_USER
+        except ImportError:
+            IPAAPI_USER = None
+        from ipalib.util import (
+            validate_domain_name,
+            no_matching_interface_for_ip_address_warning,
+        )
+        from ipapython.dnsutil import check_zone_overlap
+        from ipapython.dn import DN
+        try:
+            from ipaclient.install import timeconf
+            from ipaclient.install.client import sync_time
+            time_service = "chronyd"
+            ntpinstance = None
+        except ImportError:
+            try:
+                from ipaclient.install import ntpconf as timeconf
+            except ImportError:
+                from ipaclient import ntpconf as timeconf
+            from ipaserver.install import ntpinstance
+            time_service = "ntpd"
+            sync_time = None
+        from ipaserver.install import (
+            adtrust, bindinstance, ca, dns, dsinstance,
+            httpinstance, installutils, kra, krbinstance,
+            otpdinstance, custodiainstance, replication, service,
+            sysupgrade)
+        adtrust_imported = True
+        kra_imported = True
+        from ipaserver.install.installutils import (
+            BadHostError, get_fqdn, get_server_ip_address,
+            load_pkcs12, read_password, verify_fqdn,
+            update_hosts_file)
+        try:
+            from ipalib.facts import is_ipa_configured
+        except ImportError:
+            from ipaserver.install.installutils import is_ipa_configured
+        from ipaserver.install.server.install import (
+            check_dirsrv, validate_admin_password, validate_dm_password,
+            read_cache, write_cache)
+        try:
+            from ipaserver.install.dogtaginstance import PKIIniLoader
+        except ImportError:
+            PKIIniLoader = None
+        try:
+            from ipaserver.install.installutils import default_subject_base
+        except ImportError:
+            def default_subject_base(realm_name):
+                return DN(('O', realm_name))
+        try:
+            from ipalib.facts import IPA_MODULES
+        except ImportError:
+            from ipaserver.install.installutils import IPA_MODULES
+        try:
+            from ipaserver.install.installutils import default_ca_subject_dn
+        except ImportError:
+            def default_ca_subject_dn(subject_base):
+                return DN(('CN', 'Certificate Authority'), subject_base)
+        try:
+            from ipaserver.install.installutils import check_available_memory
+        except ImportError:
+            check_available_memory = None
 
+        try:
+            from ipaserver.install import adtrustinstance
+            _server_trust_ad_installed = True
+        except ImportError:
+            _server_trust_ad_installed = False
 
-def encode_certificate(cert):
-    """
-    Encode a certificate using base64.
+        try:
+            from ipaclient.install.client import check_ldap_conf
+        except ImportError:
+            check_ldap_conf = None
+
+        try:
+            from ipalib.x509 import Encoding
+        except ImportError:
+            from cryptography.hazmat.primitives.serialization import Encoding
+
+        try:
+            from ipalib.x509 import load_pem_x509_certificate
+        except ImportError:
+            from ipalib.x509 import load_certificate
+            load_pem_x509_certificate = None
 
-    It also takes FreeIPA and Python versions into account.
-    """
-    if isinstance(cert, (str, bytes)):
-        encoded = base64.b64encode(cert)
     else:
-        encoded = base64.b64encode(cert.public_bytes(Encoding.DER))
-    if not six.PY2:
-        encoded = encoded.decode('ascii')
-    return encoded
+        # IPA version < 4.5
 
+        raise Exception("freeipa version '%s' is too old" % VERSION)
 
-def decode_certificate(cert):
-    """
-    Decode a certificate using base64.
+    logger = logging.getLogger("ipa-server-install")
 
-    It also takes FreeIPA versions into account and returns a IPACertificate
-    for newer IPA versions.
-    """
-    if hasattr(x509, "IPACertificate"):
-        cert = cert.strip()
-        if not cert.startswith("-----BEGIN CERTIFICATE-----"):
-            cert = "-----BEGIN CERTIFICATE-----\n" + cert
-        if not cert.endswith("-----END CERTIFICATE-----"):
-            cert += "\n-----END CERTIFICATE-----"
+    def setup_logging():
+        # logger.setLevel(logging.DEBUG)
+        standard_logging_setup(
+            paths.IPASERVER_INSTALL_LOG, verbose=False, debug=False,
+            filemode='a', console_format='%(message)s')
 
-        if load_pem_x509_certificate is not None:
-            cert = load_pem_x509_certificate(cert.encode('utf-8'))
+    @contextlib_contextmanager
+    def redirect_stdout(f):
+        sys.stdout = f
+        try:
+            yield f
+        finally:
+            sys.stdout = sys.__stdout__
+
+    class AnsibleModuleLog():
+        def __init__(self, module):
+            self.module = module
+            _ansible_module_log = self
+
+            class AnsibleLoggingHandler(logging.Handler):
+                def emit(self, record):
+                    _ansible_module_log.write(self.format(record))
+
+            self.logging_handler = AnsibleLoggingHandler()
+            logger.setLevel(logging.DEBUG)
+            logger.root.addHandler(self.logging_handler)
+
+        def close(self):
+            self.flush()
+
+        def flush(self):
+            pass
+
+        def log(self, msg):
+            # self.write(msg+"\n")
+            self.write(msg)
+
+        def debug(self, msg):
+            self.module.debug(msg)
+
+        def info(self, msg):
+            self.module.debug(msg)
+
+        def write(self, msg):
+            self.module.debug(msg)
+            # self.module.warn(msg)
+
+    class options_obj(object):
+        def __init__(self):
+            self._replica_install = False
+            self.dnssec_master = False  # future unknown
+            self.disable_dnssec_master = False  # future unknown
+            self.domainlevel = MAX_DOMAIN_LEVEL  # deprecated
+            self.domain_level = self.domainlevel  # deprecated
+            self.interactive = False
+            self.unattended = not self.interactive
+
+        # def __getattribute__(self, attr):
+        #    logger.info(" <-- Accessing options.%s" % attr)
+        #    return super(options_obj, self).__getattribute__(attr)
+
+        # def __getattr__(self, attr):
+        #    logger.info(" --> Adding missing options.%s" % attr)
+        #    setattr(self, attr, None)
+        #    return getattr(self, attr)
+
+        def knobs(self):
+            for name in self.__dict__:
+                yield self, name
+
+    options = options_obj()
+    installer = options
+
+    # ServerMasterInstall
+    options.add_sids = True
+    options.add_agents = False
+
+    # Installable
+    options.uninstalling = False
+
+    # ServerInstallInterface
+    options.description = "Server"
+
+    options.kinit_attempts = 1
+    options.fixed_primary = True
+    options.permit = False
+    options.enable_dns_updates = False
+    options.no_krb5_offline_passwords = False
+    options.preserve_sssd = False
+    options.no_sssd = False
+
+    # ServerMasterInstall
+    options.force_join = False
+    options.servers = None
+    options.no_wait_for_dns = True
+    options.host_password = None
+    options.keytab = None
+    options.setup_ca = True
+    # always run sidgen task and do not allow adding agents on first master
+    options.add_sids = True
+    options.add_agents = False
+
+    # ADTrustInstallInterface
+    # no_msdcs is deprecated
+    options.no_msdcs = False
+
+    # For pylint
+    options.external_cert_files = None
+    options.dirsrv_cert_files = None
+
+    # Uninstall
+    options.ignore_topology_disconnect = False
+    options.ignore_last_of_role = False
+
+    def api_Backend_ldap2(host_name, setup_ca, connect=False):
+        # we are sure we have the configuration file ready.
+        cfg = dict(context='installer', confdir=paths.ETC_IPA, in_server=True,
+                   host=host_name)
+        if setup_ca:
+            # we have an IPA-integrated CA
+            cfg['ca_host'] = host_name
+
+        api.bootstrap(**cfg)
+        api.finalize()
+        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,
+                     no_hbac_allow, dirsrv_pkcs12_info, no_pkinit):
+
+        if not options.external_cert_files:
+            ds = dsinstance.DsInstance(fstore=fstore, domainlevel=domainlevel,
+                                       config_ldif=dirsrv_config_file)
+            ds.set_output(ansible_log)
+
+            if options.dirsrv_cert_files:
+                _dirsrv_pkcs12_info = dirsrv_pkcs12_info
+            else:
+                _dirsrv_pkcs12_info = None
+
+            with redirect_stdout(ansible_log):
+                ds.init_info(realm_name, host_name, domain_name, dm_password,
+                             subject_base, ca_subject, idstart, idmax,
+                             # hbac_allow=not no_hbac_allow,
+                             _dirsrv_pkcs12_info, setup_pkinit=not no_pkinit)
         else:
-            cert = load_certificate(cert.encode('utf-8'))
-    else:
-        cert = base64.b64decode(cert)
-    return cert
+            ds = dsinstance.DsInstance(fstore=fstore, domainlevel=domainlevel)
+            ds.set_output(ansible_log)
+
+            with redirect_stdout(ansible_log):
+                ds.init_info(realm_name, host_name, domain_name, dm_password,
+                             subject_base, ca_subject, 1101, 1100, None,
+                             setup_pkinit=not no_pkinit)
+
+        return ds
+
+    def ansible_module_get_parsed_ip_addresses(ansible_module,
+                                               param='ip_addresses'):
+        ip_addrs = []
+        for ip in ansible_module.params.get(param):
+            try:
+                ip_parsed = ipautil.CheckedIPAddress(ip)
+            except Exception as e:
+                ansible_module.fail_json(
+                    msg="Invalid IP Address %s: %s" % (ip, e))
+            ip_addrs.append(ip_parsed)
+        return ip_addrs
+
+    def encode_certificate(cert):
+        """
+        Encode a certificate using base64.
+
+        It also takes FreeIPA and Python versions into account.
+        """
+        if isinstance(cert, (str, bytes)):
+            encoded = base64.b64encode(cert)
+        else:
+            encoded = base64.b64encode(cert.public_bytes(Encoding.DER))
+        if not six.PY2:
+            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.
+        """
+        if hasattr(x509, "IPACertificate"):
+            cert = cert.strip()
+            if not cert.startswith("-----BEGIN CERTIFICATE-----"):
+                cert = "-----BEGIN CERTIFICATE-----\n" + cert
+            if not cert.endswith("-----END CERTIFICATE-----"):
+                cert += "\n-----END CERTIFICATE-----"
+
+            if load_pem_x509_certificate is not None:
+                cert = load_pem_x509_certificate(cert.encode('utf-8'))
+            else:
+                cert = load_certificate(cert.encode('utf-8'))
+        else:
+            cert = base64.b64decode(cert)
+        return cert

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