Fix typo in README-permission.md

There is a typo "Eure" instead of "Ensure" in the rename task.

.github/workflows/lint.yml

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

.pre-commit-config.yaml

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

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

@@ -154,7 +154,7 @@ ipaserver_domain=test.local
 ipaserver_realm=TEST.LOCAL
 ```
 
-The admin principle is ```admin``` by default. Please set ```ipaadmin_principal``` if you need to change it.
+The admin principal is ```admin``` by default. Please set ```ipaadmin_principal``` if you need to change it.
 
 You can also add more setting here, like for example to enable the DNS server or to set auto-forwarders:
 ```yaml

plugins/module_utils/ansible_freeipa_module.py

@@ -28,6 +28,7 @@ import os
 import uuid
 import tempfile
 import shutil
+import netaddr
 import gssapi
 from datetime import datetime
 from pprint import pformat
@@ -413,6 +414,24 @@ def is_valid_port(port):
     return False
 
 
+def is_ip_address(ipaddr):
+    """Test if given IP address is a valid IPv4 or IPv6 address."""
+    try:
+        netaddr.IPAddress(str(ipaddr))
+    except (netaddr.AddrFormatError, ValueError):
+        return False
+    return True
+
+
+def is_ip_network_address(ipaddr):
+    """Test if given IP address is a valid IPv4 or IPv6 address."""
+    try:
+        netaddr.IPNetwork(str(ipaddr))
+    except (netaddr.AddrFormatError, ValueError):
+        return False
+    return True
+
+
 def is_ipv4_addr(ipaddr):
     """Test if given IP address is a valid IPv4 address."""
     try:

plugins/modules/ipaconfig.py

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

plugins/modules/ipadelegation.py

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

plugins/modules/ipadnsconfig.py

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

plugins/modules/ipadnsforwardzone.py

@@ -380,6 +380,12 @@ def main():
                         [name, 'dnsforwardzone_remove_permission', {}]
                     )
 
+            # Check mode exit
+            if ansible_module.check_mode:
+                ansible_module.exit_json(changed=len(commands) > 0,
+                                         **exit_args)
+
+            # Execute commands
             for name, command, args in commands:
                 api_command(ansible_module, command, name, args)
                 changed = True

plugins/modules/ipadnsrecord.py

@@ -1375,10 +1375,9 @@ 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 if k in args}
-                        _args = {k: args[k] for k in part_fields}
+                    _args['idnsname'] = name
-                        _args['idnsname'] = name
+                    _commands.append([zone_name, 'dnsrecord_add', _args])
-                        _commands.append([zone_name, 'dnsrecord_add', _args])
                 # clean used fields from args
                 for f in part_fields:
                     if f in args:
@@ -1497,6 +1496,10 @@ def main():
             if cmds:
                 commands.extend(cmds)
 
+        # Check mode exit
+        if ansible_module.check_mode:
+            ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
+
         # Execute commands
         for name, command, args in commands:
             try:

plugins/modules/ipadnszone.py

@@ -210,9 +210,9 @@ dnszone:
 from ipapython.dnsutil import DNSName  # noqa: E402
 from ansible.module_utils.ansible_freeipa_module import (
     FreeIPABaseModule,
-    is_ipv4_addr,
+    is_ip_address,
-    is_ipv6_addr,
+    is_ip_network_address,
-    is_valid_port,
+    is_valid_port
 )  # noqa: E402
 import ipalib.errors
 import netaddr
@@ -252,7 +252,13 @@ class DNSZoneModule(FreeIPABaseModule):
 
     def validate_ips(self, ips, error_msg):
         invalid_ips = [
-            ip for ip in ips if not is_ipv4_addr(ip) or is_ipv6_addr(ip)
+            ip for ip in ips
+            if not any([
+                is_ip_address(ip),
+                is_ip_network_address(ip),
+                ip == "any",
+                ip == "none"
+            ])
         ]
         if any(invalid_ips):
             self.fail_json(msg=error_msg % invalid_ips)
@@ -309,7 +315,7 @@ class DNSZoneModule(FreeIPABaseModule):
             forwarders = []
             for forwarder in self.ipa_params.forwarders:
                 ip_address = forwarder.get("ip_address")
-                if not (is_ipv4_addr(ip_address) or is_ipv6_addr(ip_address)):
+                if not (is_ip_address(ip_address)):
                     self.fail_json(
                         msg="Invalid IP for DNS forwarder: %s" % ip_address
                     )

plugins/modules/ipagroup.py

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

plugins/modules/ipahbacrule.py

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

plugins/modules/ipahbacsvc.py

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

plugins/modules/ipahbacsvcgroup.py

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

plugins/modules/ipahost.py

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

plugins/modules/ipahostgroup.py

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

plugins/modules/ipalocation.py

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

plugins/modules/ipapermission.py

@@ -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, \
+    compare_args_ipa, module_params_get, api_check_ipa_version
-    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,12 @@ def main():
             ansible_module.fail_json(
                 msg="Only one permission can be added at a time.")
         if action == "member":
-            invalid = ["right", "attrs", "bindtype", "subtree",
+            invalid = ["right", "bindtype", "subtree",
                        "extra_target_filter", "rawfilter", "target",
                        "targetto", "targetfrom", "memberof", "targetgroup",
                        "object_type", "rename"]
+        else:
+            invalid = ["rename"]
 
     if state == "renamed":
         if len(names) != 1:
@@ -315,7 +290,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 +299,13 @@ def main():
     if state == "absent":
         if len(names) < 1:
             ansible_module.fail_json(msg="No name given.")
-        invalid = ["right", "attrs", "bindtype", "subtree",
+        invalid = ["right",
+                   "bindtype", "subtree",
                    "extra_target_filter", "rawfilter", "target", "targetto",
                    "targetfrom", "memberof", "targetgroup", "object_type",
                    "no_members", "rename"]
-        if action == "permission":
+        if action != "member":
-            invalid.append("privilege")
+            invalid += ["attrs"]
 
     for x in invalid:
         if vars()[x] is not None:
@@ -366,11 +342,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,41 +354,18 @@ 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:
+                    # attrs
-                        ansible_module.fail_json(msg="No privilege given")
+                    if attrs is not None:
+                        _attrs = list(set(list(res_find["attrs"]) + attrs))
+                        if len(_attrs) > len(res_find["attrs"]):
+                            commands.append([name, "permission_mod",
+                                             {"attrs": _attrs}])
 
-                    commands.append([name, "permission_add_member",
-                                     {
-                                         "privilege": privilege,
-                                         "no_members": no_members_value
-                                     }])
                 else:
                     ansible_module.fail_json(
                         msg="Unknown action '%s'" % action)
@@ -455,17 +403,28 @@ def main():
                         ansible_module.fail_json(
                             msg="No permission '%s'" % name)
 
-                    if privilege is None:
+                    # attrs
-                        ansible_module.fail_json(msg="No privilege given")
+                    if attrs is not None:
+                        # New attribute list (remove given ones from find
+                        # result)
+                        # Make list with unique entries
+                        _attrs = list(set(res_find["attrs"]) - set(attrs))
+                        if len(_attrs) < 1:
+                            ansible_module.fail_json(
+                                msg="At minimum one attribute is needed.")
 
-                    commands.append([name, "permission_remove_member",
+                        # Entries New number of attributes is smaller
-                                     {
+                        if len(_attrs) < len(res_find["attrs"]):
-                                         "privilege": privilege,
+                            commands.append([name, "permission_mod",
-                                     }])
+                                             {"attrs": _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

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

plugins/modules/ipapwpolicy.py

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

plugins/modules/iparole.py

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

plugins/modules/ipaselfservice.py

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

plugins/modules/ipaservice.py

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

plugins/modules/ipasudocmd.py

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

plugins/modules/ipasudocmdgroup.py

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

plugins/modules/ipasudorule.py

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

plugins/modules/ipatopologysegment.py

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

plugins/modules/ipatrust.py

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

plugins/modules/ipauser.py

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

plugins/modules/ipavault.py

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

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

roles/ipabackup/tasks/backup.yml

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

roles/ipabackup/tasks/restore.yml

@@ -105,13 +105,13 @@
     ipa-restore
     {{ ipabackup_item }}
     --unattended
-    {{ "--password="+ipabackup_password if ipabackup_password is defined }}
+    {{ "--password="+ipabackup_password if ipabackup_password is defined else "" }}
-    {{ "--data" if ipabackup_data | bool }}
+    {{ "--data" if ipabackup_data | bool else "" }}
-    {{ "--online" if ipabackup_online | bool }}
+    {{ "--online" if ipabackup_online | bool else "" }}
-    {{ "--instance="+ipabackup_instance if ipabackup_instance is defined }}
+    {{ "--instance="+ipabackup_instance if ipabackup_instance is defined else "" }}
-    {{ "--backend="+ipabackup_backend if ipabackup_backend is defined }}
+    {{ "--backend="+ipabackup_backend if ipabackup_backend is defined else "" }}
-    {{ "--no-logs" if ipabackup_no_logs | bool }}
+    {{ "--no-logs" if ipabackup_no_logs | bool else "" }}
-    {{ "--log-file="+ipabackup_log_file if ipabackup_log_file is defined }}
+    {{ "--log-file="+ipabackup_log_file if ipabackup_log_file is defined else "" }}
   register: result_iparestore
   ignore_errors: yes
 
@@ -127,21 +127,21 @@
   command: >
     firewall-cmd
     --permanent
-    --zone="{{ ipabackup_firewalld_zone if ipabackup_firewalld_zone is defined }}"
+    {{ "--zone="+ipabackup_firewalld_zone if ipabackup_firewalld_zone is defined else "" }}
     --add-service=freeipa-ldap
     --add-service=freeipa-ldaps
-    {{ "--add-service=freeipa-trust" if ipabackup_service_adtrust in ipabackup_services }}
+    {{ "--add-service=freeipa-trust" if ipabackup_service_adtrust in ipabackup_services else "" }}
-    {{ "--add-service=dns" if ipabackup_service_dns in ipabackup_services }}
+    {{ "--add-service=dns" if ipabackup_service_dns in ipabackup_services else "" }}
-    {{ "--add-service=ntp" if ipabackup_service_ntp in ipabackup_services }}
+    {{ "--add-service=ntp" if ipabackup_service_ntp in ipabackup_services else "" }}
   when: ipabackup_setup_firewalld | bool
 
 - name: Configure firewalld runtime
   command: >
     firewall-cmd
-    --zone="{{ ipabackup_firewalld_zone if ipabackup_firewalld_zone is defined }}"
+    {{ "--zone="+ipabackup_firewalld_zone if ipabackup_firewalld_zone is defined else "" }}
     --add-service=freeipa-ldap
     --add-service=freeipa-ldaps
-    {{ "--add-service=freeipa-trust" if ipabackup_service_adtrust in ipabackup_services }}
+    {{ "--add-service=freeipa-trust" if ipabackup_service_adtrust in ipabackup_services else "" }}
-    {{ "--add-service=dns" if ipabackup_service_dns in ipabackup_services }}
+    {{ "--add-service=dns" if ipabackup_service_dns in ipabackup_services else "" }}
-    {{ "--add-service=ntp" if ipabackup_service_ntp in ipabackup_services }}
+    {{ "--add-service=ntp" if ipabackup_service_ntp in ipabackup_services else "" }}
   when: ipabackup_setup_firewalld | bool

roles/ipareplica/library/ipareplica_prepare.py

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

roles/ipareplica/library/ipareplica_setup_dns.py

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

tests/permission/test_permission.yml

@@ -37,41 +37,127 @@
     register: result
     failed_when: result.changed or result.failed
 
-  - name: Ensure permission perm-test-1 member User Administrators privilege is present
+  - name: Ensure permission perm-test-1 is present with attr carlicense
     ipapermission:
       ipaadmin_password: SomeADMINpassword
       name: perm-test-1
-      privilege: "User Administrators"
+      attrs:
+      - carlicense
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure permission perm-test-1 is present with attr carlicense again
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      name: perm-test-1
+      attrs:
+      - carlicense
+    register: result
+    failed_when: result.changed or result.failed
+
+  - name: Ensure permission perm-test-1 is present with attr carlicense and displayname
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      name: perm-test-1
+      attrs:
+      - carlicense
+      - displayname
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure permission perm-test-1 is present with attr carlicense and displayname again
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      name: perm-test-1
+      attrs:
+      - carlicense
+      - displayname
+    register: result
+    failed_when: result.changed or result.failed
+
+  - name: Ensure attr gecos is present in permission perm-test-1
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      name: perm-test-1
+      attrs:
+      - gecos
       action: member
     register: result
     failed_when: not result.changed or result.failed
 
-  - name: Ensure permission perm-test-1 member User Administrators privilege is present again
+  - name: Ensure attr gecos is present in permission perm-test-1 again
     ipapermission:
       ipaadmin_password: SomeADMINpassword
       name: perm-test-1
-      privilege: "User Administrators"
+      attrs:
+      - gecos
       action: member
     register: result
     failed_when: result.changed or result.failed
 
-  - name: Ensure permission perm-test-1 member User Administrators privilege is absent
+  - name: Ensure attr gecos is absent in permission perm-test-1
     ipapermission:
       ipaadmin_password: SomeADMINpassword
       name: perm-test-1
-      privilege: "User Administrators"
+      attrs:
+      - gecos
       action: member
       state: absent
     register: result
     failed_when: not result.changed or result.failed
 
-  # NOTE: We use the "User Administrators" Privilege here since we don't have a module
+  - name: Ensure attr gecos is absent in permission perm-test-1 again
-  # to make one. A test privilege should be used in the future.
-  - name: Ensure permission perm-test-1 member User Administrators privilege is absent again
     ipapermission:
       ipaadmin_password: SomeADMINpassword
       name: perm-test-1
-      privilege: "User Administrators"
+      attrs:
+      - gecos
+      action: member
+      state: absent
+    register: result
+    failed_when: result.changed or result.failed
+
+  - name: Ensure attributes carlicense and displayname are present in permission "System{{':'}} Update DNS Entries"
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      name: "System: Update DNS Entries"
+      attrs:
+      - carlicense
+      - displayname
+      action: member
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure attributes carlicense and displayname are present in permission "System{{':'}} Update DNS Entries" again
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      name: "System: Update DNS Entries"
+      attrs:
+      - carlicense
+      - displayname
+      action: member
+    register: result
+    failed_when: result.changed or result.failed
+
+  - name: Ensure attributes carlicense and displayname are present in permission "System{{':'}} Update DNS Entries"
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      name: "System: Update DNS Entries"
+      attrs:
+      - carlicense
+      - displayname
+      action: member
+      state: absent
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure attributes carlicense and displayname are present in permission "System{{':'}} Update DNS Entries" again
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      name: "System: Update DNS Entries"
+      attrs:
+      - carlicense
+      - displayname
       action: member
       state: absent
     register: result

tests/role/env_cleanup.yml

@@ -2,31 +2,42 @@
 - name: Ensure test user is absent.
   ipauser:
     ipaadmin_password: SomeADMINpassword
-    name: user01
+    name:
+    - user01
+    - user02
+    - user03
     state: absent
 
 - name: Ensure test group is absent.
   ipagroup:
     ipaadmin_password: SomeADMINpassword
-    name: group01
+    name:
+    - group01
+    - group02
     state: absent
 
 - name: Ensure test hostgroup is absent.
   ipahostgroup:
     ipaadmin_password: SomeADMINpassword
-    name: hostgroup01
+    name:
+    - hostgroup01
+    - hostgroup02
     state: absent
 
 - name: Ensure test host is absent.
   ipahost:
     ipaadmin_password: SomeADMINpassword
-    name: "{{ host1_fqdn }}"
+    name:
+    - "{{ host1_fqdn }}"
+    - "{{ host2_fqdn }}"
     state: absent
 
 - name: Ensure test service is absent.
   ipaservice:
     ipaadmin_password: SomeADMINpassword
-    name: "service01/{{ host1_fqdn }}"
+    name:
+    - "service01/{{ host1_fqdn }}"
+    - "service02/{{ host2_fqdn }}"
     state: absent
 
 - name: Ensure test roles are absent.

tests/role/env_facts.yml

@@ -1,7 +1,7 @@
 ---
 - name: Get Domain from server name
   set_fact:
-    ipaserver_domain: "{{ ansible_fqdn | join ('.') }}"
+    ipaserver_domain: "{{ ansible_fqdn.split('.')[1:] | join ('.') }}"
   when: ipaserver_domain is not defined
 
 - name: Set fact for realm name
@@ -12,3 +12,4 @@
 - name: Create FQDN for host01
   set_fact:
     host1_fqdn: "host01.{{ ipaserver_domain }}"
+    host2_fqdn: "host02.{{ ipaserver_domain }}"

tests/role/env_setup.yml

@@ -5,30 +5,49 @@
 - name: Ensure test user is present.
   ipauser:
     ipaadmin_password: SomeADMINpassword
-    name: user01
+    users:
-    first: First
+    - name: user01
-    last: Last
+      first: First
+      last: Last
+    - name: user02
+      first: First
+      last: Last
+    - name: user03
+      first: First
+      last: Last
 
 - name: Ensure test group is present.
   ipagroup:
     ipaadmin_password: SomeADMINpassword
-    name: group01
+    name: "{{ item }}"
+  with_items:
+  - group01
+  - group02
 
 - name: Ensure test host is present.
   ipahost:
     ipaadmin_password: SomeADMINpassword
-    name: "{{ host1_fqdn }}"
+    name: "{{ item }}"
     force: yes
+  with_items:
+  - "{{ host1_fqdn }}"
+  - "{{ host2_fqdn }}"
 
 - name: Ensure test hostgroup is present.
   ipahostgroup:
     ipaadmin_password: SomeADMINpassword
-    name: hostgroup01
+    name: "{{ item[0] }}"
     host:
-    - "{{ host1_fqdn }}"
+      - "{{ item[1] }}"
+  with_nested:
+  - [hostgroup01, hostgroup02]
+  - ["{{ host1_fqdn }}", "{{ host2_fqdn }}"]
 
 - name: Ensure test service is present.
   ipaservice:
     ipaadmin_password: SomeADMINpassword
-    name: "service01/{{ host1_fqdn }}"
+    name: "{{ item }}"
     force: yes
+  with_items:
+  - "service01/{{ host1_fqdn }}"
+  - "service02/{{ host2_fqdn }}"

tests/role/test_role_lists_handling.yml

@@ -0,0 +1,259 @@
+---
+- name: Test service member in role module.
+  hosts: ipaserver
+  become: yes
+  gather_facts: yes
+
+  tasks:
+  - name: Set environment facts.
+    import_tasks: env_facts.yml
+
+  - name: Setup environment.
+    import_tasks: env_setup.yml
+
+  - name: Add role.
+    iparole:
+      ipaadmin_password: SomeADMINpassword
+      name: testrole
+      user: user01
+      group: group01
+      hostgroup: hostgroup01
+      host: "{{ host1_fqdn }}"
+      service: "service01/{{ host1_fqdn }}"
+      privilege:
+        - Automember Readers
+        - ADTrust Agents
+    register: result
+    failed_when: result.failed or not result.changed
+
+  # Test fix for https://github.com/freeipa/ansible-freeipa/issues/409
+  - name: Add new privileges to role.
+    iparole:
+      ipaadmin_password: SomeADMINpassword
+      name: testrole
+      privilege:
+        - DNS Servers
+        - Host Administrators
+        - DNS Administrators
+        - Group Administrators
+      action: member
+    register: result
+    failed_when: result.failed or not result.changed
+
+  - name: Verify role privileges.
+    shell:
+      cmd: |
+        echo SomeADMINpassword | kinit -c {{ KRB5CCNAME }} admin
+        KRB5CCNAME={{ KRB5CCNAME }} ipa role-show testrole
+        kdestroy -A -q -c {{ KRB5CCNAME }}
+    register: result
+    failed_when: |
+      result.failed or not (
+        "Automember Readers" in result.stdout
+        and "ADTrust Agents" in result.stdout
+        and "DNS Servers" in result.stdout
+        and "Host Administrators" in result.stdout
+        and "DNS Administrators" in result.stdout
+        and "Group Administrators" in result.stdout
+      )
+    vars:
+      KRB5CCNAME: verify_issue_409
+  # End of test fix for https://github.com/freeipa/ansible-freeipa/issues/409
+
+  # Test fix for https://github.com/freeipa/ansible-freeipa/issues/412
+  - name: Add new user to role.
+    iparole:
+      ipaadmin_password: SomeADMINpassword
+      name: testrole
+      user: user02
+      action: member
+    register: result
+    failed_when: result.failed or not result.changed
+
+  - name: Verify role users.
+    shell:
+      cmd: |
+        echo SomeADMINpassword | kinit -c {{ KRB5CCNAME }} admin
+        KRB5CCNAME={{ KRB5CCNAME }} ipa role-show testrole
+        kdestroy -A -q -c {{ KRB5CCNAME }}
+    register: result
+    failed_when: |
+      result.failed or not (
+        "user01" in result.stdout
+        and "user02" in result.stdout
+      )
+    vars:
+      KRB5CCNAME: verify_issue_412
+
+  - name: Add new group to role.
+    iparole:
+      ipaadmin_password: SomeADMINpassword
+      name: testrole
+      group: group02
+      action: member
+    register: result
+    failed_when: result.failed or not result.changed
+
+  - name: Verify role group.
+    shell:
+      cmd: |
+        echo SomeADMINpassword | kinit -c {{ KRB5CCNAME }} admin
+        KRB5CCNAME={{ KRB5CCNAME }} ipa role-show testrole
+        kdestroy -A -q -c {{ KRB5CCNAME }}
+    register: result
+    failed_when: |
+      result.failed or not (
+        "group01" in result.stdout
+        and "group02" in result.stdout
+      )
+    vars:
+      KRB5CCNAME: verify_issue_412
+
+  - name: Add new host to role.
+    iparole:
+      ipaadmin_password: SomeADMINpassword
+      name: testrole
+      host: "{{ host2_fqdn }}"
+      action: member
+    register: result
+    failed_when: result.failed or not result.changed
+
+  - name: Verify role hosts.
+    shell:
+      cmd: |
+        echo SomeADMINpassword | kinit -c {{ KRB5CCNAME }} admin
+        KRB5CCNAME={{ KRB5CCNAME }} ipa role-show testrole
+        kdestroy -A -q -c {{ KRB5CCNAME }}
+    register: result
+    failed_when: |
+      result.failed or not (
+        host1 in result.stdout
+        and host2 in result.stdout
+      )
+    vars:
+      KRB5CCNAME: verify_issue_412
+      host1: " {{ host1_fqdn }}"
+      host2: " {{ host2_fqdn }}"
+
+  - name: Add new hostgroup to role.
+    iparole:
+      ipaadmin_password: SomeADMINpassword
+      name: testrole
+      hostgroup: hostgroup02
+      action: member
+    register: result
+    failed_when: result.failed or not result.changed
+
+  - name: Verify role hostgroups.
+    shell:
+      cmd: |
+        echo SomeADMINpassword | kinit -c {{ KRB5CCNAME }} admin
+        KRB5CCNAME={{ KRB5CCNAME }} ipa role-show testrole
+        kdestroy -A -q -c {{ KRB5CCNAME }}
+    register: result
+    failed_when: |
+      result.failed or not (
+        " hostgroup01" in result.stdout
+        and " hostgroup02" in result.stdout
+      )
+    vars:
+      KRB5CCNAME: verify_issue_412
+
+  - name: Add new service to role.
+    iparole:
+      ipaadmin_password: SomeADMINpassword
+      name: testrole
+      service: "service02/{{ host2_fqdn }}"
+      action: member
+    register: result
+    failed_when: result.failed or not result.changed
+
+  - name: Verify role services.
+    shell:
+      cmd: |
+        echo SomeADMINpassword | kinit -c {{ KRB5CCNAME }} admin
+        KRB5CCNAME={{ KRB5CCNAME }} ipa role-show testrole
+        kdestroy -A -q -c {{ KRB5CCNAME }}
+    register: result
+    failed_when: |
+      result.failed or not (
+        service1 in result.stdout
+        and service1 in result.stdout
+      )
+    vars:
+      KRB5CCNAME: verify_issue_412
+      service1: "service01/{{ host1_fqdn }}"
+      service2: "service02/{{ host2_fqdn }}"
+  # End of test fix for https://github.com/freeipa/ansible-freeipa/issues/412
+
+  # Test fix for https://github.com/freeipa/ansible-freeipa/issues/413
+  - name: Add new user to role.
+    iparole:
+      ipaadmin_password: SomeADMINpassword
+      name: testrole
+      user: user03
+      action: member
+    register: result
+    failed_when: result.failed or not result.changed
+
+  - name: Verify role services.
+    shell:
+      cmd: |
+        echo SomeADMINpassword | kinit -c {{ KRB5CCNAME }} admin
+        KRB5CCNAME={{ KRB5CCNAME }} ipa role-show testrole
+        kdestroy -A -q -c {{ KRB5CCNAME }}
+    register: result
+    failed_when: |
+      result.failed or not (
+        service1 in result.stdout
+        and service1 in result.stdout
+        and "user03" in result.stdout
+      )
+    vars:
+      KRB5CCNAME: verify_issue_413
+      service1: "service01/{{ host1_fqdn }}"
+      service2: "service02/{{ host2_fqdn }}"
+
+  - name: Remove user from role.
+    iparole:
+      ipaadmin_password: SomeADMINpassword
+      name: testrole
+      user: user03
+      action: member
+      state: absent
+    register: result
+    failed_when: result.failed or not result.changed
+
+  - name: Verify role services.
+    shell:
+      cmd: |
+        echo SomeADMINpassword | kinit -c {{ KRB5CCNAME }} admin
+        KRB5CCNAME={{ KRB5CCNAME }} ipa role-show testrole
+        kdestroy -A -q -c {{ KRB5CCNAME }}
+    register: result
+    failed_when: |
+      result.failed or not (
+        service1 in result.stdout
+        and service1 in result.stdout
+        and "user03" not in result.stdout
+      )
+    vars:
+      KRB5CCNAME: verify_issue_413
+      service1: "service01/{{ host1_fqdn }}"
+      service2: "service02/{{ host2_fqdn }}"
+  # End of test fix for https://github.com/freeipa/ansible-freeipa/issues/413
+
+  # Test fix for https://github.com/freeipa/ansible-freeipa/issues/411
+  - name: Add non-existing user to role.
+    iparole:
+      ipaadmin_password: SomeADMINpassword
+      name: testrole
+      user: nonexisiting_user
+      action: member
+    register: result
+    failed_when: not result.failed
+  # End of test fix for https://github.com/freeipa/ansible-freeipa/issues/411
+
+  # cleanup
+  - name: Cleanup environment.
+    include_tasks: env_cleanup.yml

tests/utils.py

@@ -169,7 +169,7 @@ def list_test_yaml(dir_path):
     `test_` and the extension is `.yml`.
     """
     yamls = []
-    for root, dirs, files in os.walk(dir_path):
+    for root, _dirs, files in os.walk(dir_path):
         for yaml_name in files:
             if yaml_name.startswith("test_") and yaml_name.endswith(".yml"):
                 test_yaml_path = os.path.join(root, yaml_name)

tests/vault/env_cleanup.yml

@@ -38,35 +38,35 @@
       name: vaultgroup
       state: absent
 
-  - name: Remove password file from target host.
+  - name: Remove files from target host.
     file:
-      path: "{{ ansible_env.HOME }}/password.txt"
+      path: "{{ ansible_env.HOME }}/{{ item }}"
       state: absent
+    with_items:
+    - A_private.pem
+    - A_public.pem
+    - B_private.pem
+    - B_public.pem
+    - A_private.b64
+    - A_public.b64
+    - B_private.b64
+    - B_public.b64
+    - password.txt
+    - in.txt
+    - out.txt
 
-  - name: Remove public key file from target host.
+  - name: Remove files from controller.
     file:
-      path: "{{ ansible_env.HOME }}/public.pem"
+      path: "{{ playbook_dir }}/{{ item }}"
       state: absent
-
-  - name: Remove private key file from target host.
-    file:
-      path: "{{ ansible_env.HOME }}/private.pem"
-      state: absent
-
-  - name: Remove output data file from target host.
-    file:
-      path: "{{ ansible_env.HOME }}/data.txt"
-      state: absent
-
-  - name: Remove input data file from target host.
-    file:
-      path: "{{ ansible_env.HOME }}/in.txt"
-      state: absent
-
-  - name: Remove private/public key files.
-    shell:
-      cmd: rm -f private.pem public.pem
     delegate_to: localhost
     become: no
-    args:
+    with_items:
-      warn: no  # suppres warning for not using the `file` module.
+    - A_private.pem
+    - A_public.pem
+    - B_private.pem
+    - B_public.pem
+    - A_private.b64
+    - A_public.b64
+    - B_private.b64
+    - B_public.b64

tests/vault/env_setup.yml

@@ -3,37 +3,34 @@
   - name: Ensure environment is clean.
     import_tasks: env_cleanup.yml
 
-  - name: Create private key file.
+  - name: Create private/public key pair.
     shell:
-      cmd: openssl genrsa -out private.pem 2048
+      cmd: |
+        openssl genrsa -out "{{ item }}private.pem" 2048
+        openssl rsa -in "{{ item }}private.pem" -outform PEM -pubout -out "{{ item }}public.pem"
+        base64 "{{ item }}public.pem" -w5000 > "{{ item }}public.b64"
+        base64 "{{ item }}private.pem" -w5000 > "{{ item }}private.b64"
     delegate_to: localhost
     become: no
+    with_items:
+    - A_
+    - B_
 
-  - name: Create public key file.
+  - name: Copy files to target host.
-    shell:
-      cmd: openssl rsa -in private.pem -outform PEM -pubout -out public.pem
-    delegate_to: localhost
-    become: no
-
-  - name: Copy password file to target host.
     copy:
-      src: "{{ playbook_dir }}/password.txt"
+      src: "{{ playbook_dir }}/{{ item }}"
-      dest: "{{ ansible_env.HOME }}/password.txt"
+      dest: "{{ ansible_env.HOME }}/{{ item }}"
-
+    with_items:
-  - name: Copy public key file to target host.
+    - A_private.pem
-    copy:
+    - A_public.pem
-      src: "{{ playbook_dir }}/public.pem"
+    - B_private.pem
-      dest: "{{ ansible_env.HOME }}/public.pem"
+    - B_public.pem
-
+    - A_private.b64
-  - name: Copy private key file to target host.
+    - A_public.b64
-    copy:
+    - B_private.b64
-      src: "{{ playbook_dir }}/private.pem"
+    - B_public.b64
-      dest: "{{ ansible_env.HOME }}/private.pem"
+    - password.txt
-
+    - in.txt
-  - name: Copy input data file to target host.
-    copy:
-      src: "{{ playbook_dir }}/in.txt"
-      dest: "{{ ansible_env.HOME }}/in.txt"
 
   - name: Ensure vaultgroup exists.
     ipagroup:

tests/vault/tasks_vault_members.yml

@@ -25,9 +25,9 @@
   - name: Ensure vault is present
     ipavault:
       ipaadmin_password: SomeADMINpassword
-      name: "{{vault.name}}"
+      name: "{{ vault.name }}"
-      vault_type: "{{vault.vault_type}}"
+      vault_type: "{{ vault.vault_type }}"
-      public_key: "{{lookup('file', 'private.pem', rstrip=False) | b64encode}}"
+      public_key: "{{lookup('file', 'A_private.b64')}}"
     register: result
     failed_when: not result.changed
     when: vault.vault_type == 'asymmetric'

tests/vault/test_vault_asymmetric.yml

@@ -14,18 +14,111 @@
       ipaadmin_password: SomeADMINpassword
       name: asymvault
       vault_type: asymmetric
-      public_key: "{{ lookup('file', 'public.pem', rstrip=False) | b64encode }}"
+      public_key: "{{ lookup('file', 'A_public.b64') }}"
     register: result
-    failed_when: not result.changed
+    failed_when: result.failed or not result.changed
 
   - name: Ensure asymmetric vault is present, again
     ipavault:
       ipaadmin_password: SomeADMINpassword
       name: asymvault
       vault_type: asymmetric
-      public_key: "{{ lookup('file', 'public.pem', rstrip=False) | b64encode }}"
+      public_key: "{{ lookup('file', 'A_public.b64') }}"
     register: result
-    failed_when: result.changed
+    failed_when: result.failed or result.changed
+
+  - name: Archive data to asymmetric vault.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: asymvault
+      vault_data: SomeValue
+    register: result
+    failed_when: result.failed or not result.changed
+
+  - name: Retrieve data from asymmetric vault using key A.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: asymvault
+      private_key: "{{ lookup('file', 'A_private.b64') }}"
+      state: retrieved
+    register: result
+    failed_when: result.failed or result.changed or result.vault.data != 'SomeValue'
+
+  - name: Change asymmetric vault key to B.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: asymvault
+      vault_type: asymmetric
+      public_key: "{{ lookup('file', 'B_public.b64') }}"
+      private_key: "{{ lookup('file', 'A_private.b64') }}"
+    register: result
+    failed_when: result.failed or not result.changed
+
+  - name: Retrieve data from asymmetric vault using key B.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: asymvault
+      private_key: "{{ lookup('file', 'B_private.b64') }}"
+      state: retrieved
+    register: result
+    failed_when: result.failed or result.changed or result.vault.data != 'SomeValue'
+
+  - name: Change asymmetric vault key to A, using key_file
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: asymvault
+      vault_type: asymmetric
+      public_key_file: "{{ ansible_env.HOME }}/A_public.pem"
+      private_key: "{{ lookup('file', 'B_private.b64') }}"
+    register: result
+    failed_when: result.failed or not result.changed
+
+  - name: Retrieve data from asymmetric vault using key A, with key_file.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: asymvault
+      private_key_file: "{{ ansible_env.HOME }}/A_private.pem"
+      state: retrieved
+    register: result
+    failed_when: result.failed or result.changed or result.vault.data != 'SomeValue'
+
+  - name: Change asymmetric vault key to B key, using key_files
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: asymvault
+      vault_type: asymmetric
+      public_key_file: "{{ ansible_env.HOME }}/B_public.pem"
+      private_key_file: "{{ ansible_env.HOME }}/A_private.pem"
+    register: result
+    failed_when: result.failed or not result.changed
+
+  - name: Retrieve data from asymmetric vault, using key B.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: asymvault
+      private_key: "{{ lookup('file', 'B_private.b64') }}"
+      state: retrieved
+    register: result
+    failed_when: result.failed or result.changed or result.vault.data != 'SomeValue'
+
+  - name: Change asymmetric vault key to A, without specifying vault_type.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: asymvault
+      vault_type: asymmetric
+      public_key: "{{ lookup('file', 'A_public.b64') }}"
+      private_key: "{{ lookup('file', 'B_private.b64') }}"
+    register: result
+    failed_when: result.failed or not result.changed
+
+  - name: Change asymmetric vault key to B, with key files, without specifying vault_type.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: asymvault
+      public_key_file: "{{ ansible_env.HOME }}/B_public.pem"
+      private_key_file: "{{ ansible_env.HOME }}/A_private.pem"
+    register: result
+    failed_when: result.failed or not result.changed
 
   - name: Archive data to asymmetric vault, matching `no_log` field.
     ipavault:
@@ -39,12 +132,12 @@
     ipavault:
       ipaadmin_password: SomeADMINpassword
       name: asymvault
-      private_key: "{{ lookup('file', 'private.pem', rstrip=False) | b64encode }}"
+      private_key: "{{ lookup('file', 'B_private.b64') }}"
       state: retrieved
     register: result
     failed_when: result.vault.data != 'SomeADMINpassword' or result.changed
 
-  - name: Archive data to asymmetric vault
+  - name: Change data in asymmetric vault
     ipavault:
       ipaadmin_password: SomeADMINpassword
       name: asymvault
@@ -52,11 +145,11 @@
     register: result
     failed_when: not result.changed
 
-  - name: Retrieve data from asymmetric vault.
+  - name: Retrieve changed data from asymmetric vault.
     ipavault:
       ipaadmin_password: SomeADMINpassword
       name: asymvault
-      private_key: "{{ lookup('file', 'private.pem', rstrip=False) | b64encode }}"
+      private_key: "{{ lookup('file', 'B_private.b64') }}"
       state: retrieved
     register: result
     failed_when: result.vault.data != 'Hello World.' or result.changed
@@ -66,7 +159,7 @@
       ipaadmin_password: SomeADMINpassword
       name: asymvault
       out: "{{ ansible_env.HOME }}/data.txt"
-      private_key: "{{ lookup('file', 'private.pem', rstrip=False) | b64encode }}"
+      private_key: "{{ lookup('file', 'B_private.b64') }}"
       state: retrieved
     register: result
     failed_when: result.changed or result.failed or (result.vault.data | default(false))
@@ -89,7 +182,7 @@
     ipavault:
       ipaadmin_password: SomeADMINpassword
       name: asymvault
-      private_key: "{{ lookup('file', 'private.pem', rstrip=False) | b64encode }}"
+      private_key: "{{ lookup('file', 'B_private.b64') }}"
       state: retrieved
     register: result
     failed_when: result.vault.data != 'The world of π is half rounded.' or result.changed
@@ -107,7 +200,7 @@
     ipavault:
       ipaadmin_password: SomeADMINpassword
       name: asymvault
-      private_key: "{{ lookup('file', 'private.pem', rstrip=False) | b64encode }}"
+      private_key: "{{ lookup('file', 'B_private.b64') }}"
       state: retrieved
     register: result
     failed_when: result.vault.data != 'Another World.' or result.changed
@@ -124,7 +217,7 @@
     ipavault:
       ipaadmin_password: SomeADMINpassword
       name: asymvault
-      private_key: "{{ lookup('file', 'private.pem', rstrip=False) | b64encode }}"
+      private_key: "{{ lookup('file', 'B_private.b64') }}"
       state: retrieved
     register: result
     failed_when: result.vault.data != 'c' or result.changed
@@ -149,7 +242,7 @@
     ipavault:
       ipaadmin_password: SomeADMINpassword
       name: asymvault
-      public_key_file: "{{ ansible_env.HOME }}/public.pem"
+      public_key_file: "{{ ansible_env.HOME }}/B_public.pem"
       vault_type: asymmetric
     register: result
     failed_when: not result.changed
@@ -158,7 +251,7 @@
     ipavault:
       ipaadmin_password: SomeADMINpassword
       name: asymvault
-      public_key_file: "{{ ansible_env.HOME }}/public.pem"
+      public_key_file: "{{ ansible_env.HOME }}/B_public.pem"
       vault_type: asymmetric
     register: result
     failed_when: result.changed
@@ -175,7 +268,7 @@
     ipavault:
       ipaadmin_password: SomeADMINpassword
       name: asymvault
-      private_key: "{{ lookup('file', 'private.pem', rstrip=False) | b64encode }}"
+      private_key: "{{ lookup('file', 'B_private.b64') }}"
       state: retrieved
     register: result
     failed_when: result.vault.data != 'Hello World.' or result.changed
@@ -184,7 +277,7 @@
     ipavault:
       ipaadmin_password: SomeADMINpassword
       name: asymvault
-      private_key_file: "{{ ansible_env.HOME }}/private.pem"
+      private_key_file: "{{ ansible_env.HOME }}/B_private.pem"
       state: retrieved
     register: result
     failed_when: result.vault.data != 'Hello World.' or result.changed
@@ -206,4 +299,4 @@
     failed_when: result.changed
 
   - name: Cleanup testing environment.
-    import_tasks: env_setup.yml
+    import_tasks: env_cleanup.yml

tests/vault/test_vault_change_type.yml

@@ -0,0 +1,304 @@
+---
+- name: Test vault
+  hosts: ipaserver
+  become: true
+  # Need to gather facts for ansible_env.
+  gather_facts: true
+
+  tasks:
+  - name: Setup testing environment.
+    import_tasks: env_setup.yml
+
+  - name: Ensure test_vault is absent.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: test_vault
+      state: absent
+
+  - name: Create standard vault with no data archived.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: test_vault
+      vault_type: standard
+
+  - name: Change from standard to asymmetric
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: test_vault
+      vault_type: asymmetric
+      public_key: "{{ lookup('file', 'A_public.b64') }}"
+    register: result
+    failed_when: result.failed or not result.changed
+
+  - block:
+    - name: Change from asymmetric to symmetric
+      ipavault:
+        ipaadmin_password: SomeADMINpassword
+        name: test_vault
+        vault_type: symmetric
+        private_key: "{{ lookup('file', 'A_private.b64') }}"
+        password: SomeVAULTpassword
+      register: result
+      failed_when: result.failed or not result.changed
+
+    - name: Verify assymetric-only fields are not present.
+      shell: |
+         echo SomeADMINpassword | kinit -c {{ KRB5CCNAME }} admin
+         KRB5CCNAME={{ KRB5CCNAME }} ipa vault-show test_vault
+         kdestroy -A -q -c {{ KRB5CCNAME }}
+      register: result
+      failed_when: result.failed or "Public Key:" in result.stdout
+
+    vars:
+      KRB5CCNAME: verify_change_from_asymmetric
+
+  - block:
+    - name: Change from symmetric to standard
+      ipavault:
+        ipaadmin_password: SomeADMINpassword
+        name: test_vault
+        vault_type: standard
+        password: SomeVAULTpassword
+      register: result
+      failed_when: result.failed or not result.changed
+
+    - name: Verify salt is not present.
+      shell: |
+         echo SomeADMINpassword | kinit -c {{ KRB5CCNAME }} admin
+         KRB5CCNAME={{ KRB5CCNAME }} ipa vault-show test_vault
+         kdestroy -A -q -c {{ KRB5CCNAME }}
+      register: result
+      failed_when: result.failed or "Salt:" in result.stdout
+
+    vars:
+      KRB5CCNAME: verify_change_from_symmetric
+
+  - name: Change from standard to symmetric
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: test_vault
+      vault_type: symmetric
+      password: SomeVAULTpassword
+    register: result
+    failed_when: result.failed or not result.changed
+
+  - block:
+    - name: Change from symmetric to asymmetric
+      ipavault:
+        ipaadmin_password: SomeADMINpassword
+        name: test_vault
+        vault_type: asymmetric
+        password: SomeVAULTpassword
+        public_key: "{{ lookup('file', 'A_public.b64') }}"
+      register: result
+      failed_when: result.failed or not result.changed
+
+    - name: Verify salt is not present.
+      shell: |
+         echo SomeADMINpassword | kinit -c {{ KRB5CCNAME }} admin
+         KRB5CCNAME={{ KRB5CCNAME }} ipa vault-show test_vault
+         kdestroy -A -q -c {{ KRB5CCNAME }}
+      register: result
+      failed_when: result.failed or "Salt:" in result.stdout
+
+    vars:
+      KRB5CCNAME: verify_change_from_symmetric
+
+  - block:
+    - name: Change from asymmetric to standard
+      ipavault:
+        ipaadmin_password: SomeADMINpassword
+        name: test_vault
+        vault_type: standard
+        private_key: "{{ lookup('file', 'A_private.b64') }}"
+      register: result
+      failed_when: result.failed or not result.changed
+
+    - name: Verify assymetric-only fields are not present.
+      shell: |
+         echo SomeADMINpassword | kinit -c {{ KRB5CCNAME }} admin
+         KRB5CCNAME={{ KRB5CCNAME }} ipa vault-show test_vault
+         kdestroy -A -q -c {{ KRB5CCNAME }}
+      register: result
+      failed_when: result.failed or "Public Key:" in result.stdout
+
+    vars:
+      KRB5CCNAME: verify_change_from_asymmetric
+
+  - name: Ensure test_vault is absent.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: test_vault
+      state: absent
+
+  - name: Create standard vault with data archived.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: test_vault
+      vault_type: standard
+      data: hello
+
+  - name: Change from standard to asymmetric, with data
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: test_vault
+      vault_type: asymmetric
+      public_key: "{{ lookup('file', 'A_public.b64') }}"
+    register: result
+    failed_when: result.failed or not result.changed
+
+  - name: Retrieve data from asymmetric vault.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: test_vault
+      private_key: "{{ lookup('file', 'A_private.b64') }}"
+      state: retrieved
+    register: result
+    failed_when: result.failed or result.changed or result.vault.data != 'hello'
+
+  - block:
+    - name: Change from asymmetric to symmetric, with data
+      ipavault:
+        ipaadmin_password: SomeADMINpassword
+        name: test_vault
+        vault_type: symmetric
+        private_key: "{{ lookup('file', 'A_private.b64') }}"
+        password: SomeVAULTpassword
+      register: result
+      failed_when: result.failed or not result.changed
+
+    - name: Verify assymetric-only fields are not present.
+      shell: |
+         echo SomeADMINpassword | kinit -c {{ KRB5CCNAME }} admin
+         KRB5CCNAME={{ KRB5CCNAME }} ipa vault-show test_vault
+         kdestroy -A -q -c {{ KRB5CCNAME }}
+      register: result
+      failed_when: result.failed or "Public Key:" in result.stdout
+
+    vars:
+      KRB5CCNAME: verify_change_from_asymmetric
+
+  - name: Retrieve data from symmetric vault.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: test_vault
+      password: SomeVAULTpassword
+      state: retrieved
+    register: result
+    failed_when: result.failed or result.changed or result.vault.data != 'hello'
+
+  - block:
+    - name: Change from symmetric to standard, with data
+      ipavault:
+        ipaadmin_password: SomeADMINpassword
+        name: test_vault
+        vault_type: standard
+        password: SomeVAULTpassword
+      register: result
+      failed_when: result.failed or not result.changed
+
+    - name: Verify salt is not present.
+      shell: |
+         echo SomeADMINpassword | kinit -c {{ KRB5CCNAME }} admin
+         KRB5CCNAME={{ KRB5CCNAME }} ipa vault-show test_vault
+         kdestroy -A -q -c {{ KRB5CCNAME }}
+      register: result
+      failed_when: result.failed or "Salt:" in result.stdout
+
+    vars:
+      KRB5CCNAME: verify_change_from_symmetric
+
+  - name: Retrieve data from standard vault.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: test_vault
+      state: retrieved
+    register: result
+    failed_when: result.failed or result.changed or result.vault.data != 'hello'
+
+  - name: Change from standard to symmetric, with data
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: test_vault
+      vault_type: symmetric
+      password: SomeVAULTpassword
+    register: result
+    failed_when: result.failed or not result.changed
+
+  - name: Retrieve data from symmetric vault.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: test_vault
+      state: retrieved
+      password: SomeVAULTpassword
+    register: result
+    failed_when: result.failed or result.changed or result.vault.data != 'hello'
+
+  - block:
+    - name: Change from symmetric to asymmetric, with data
+      ipavault:
+        ipaadmin_password: SomeADMINpassword
+        name: test_vault
+        vault_type: asymmetric
+        password: SomeVAULTpassword
+        public_key: "{{ lookup('file', 'A_public.b64') }}"
+      register: result
+      failed_when: result.failed or not result.changed
+
+    - name: Verify salt is not present.
+      shell: |
+         echo SomeADMINpassword | kinit -c {{ KRB5CCNAME }} admin
+         KRB5CCNAME={{ KRB5CCNAME }} ipa vault-show test_vault
+         kdestroy -A -q -c {{ KRB5CCNAME }}
+      register: result
+      failed_when: result.failed or "Salt:" in result.stdout
+
+    vars:
+      KRB5CCNAME: verify_change_from_symmetric
+
+  - name: Retrieve data from asymmetric vault.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: test_vault
+      state: retrieved
+      private_key: "{{ lookup('file', 'A_private.b64') }}"
+    register: result
+    failed_when: result.failed or result.changed or result.vault.data != 'hello'
+
+  - block:
+    - name: Change from asymmetric to standard, with data
+      ipavault:
+        ipaadmin_password: SomeADMINpassword
+        name: test_vault
+        vault_type: standard
+        private_key: "{{ lookup('file', 'A_private.b64') }}"
+      register: result
+      failed_when: result.failed or not result.changed or result.failed
+
+    - name: Verify assymetric-only fields are not present.
+      shell: |
+         echo SomeADMINpassword | kinit -c {{ KRB5CCNAME }} admin
+         KRB5CCNAME={{ KRB5CCNAME }} ipa vault-show test_vault
+         kdestroy -A -q -c {{ KRB5CCNAME }}
+      register: result
+      failed_when: result.failed or "Public Key:" in result.stdout
+
+    vars:
+      KRB5CCNAME: verify_change_from_asymmetric
+
+  - name: Retrieve data from standard vault.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: test_vault
+      state: retrieved
+    register: result
+    failed_when: result.failed or result.changed or result.vault.data != 'hello'
+
+  - name: Remove test_vault.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: test_vault
+      state: absent
+
+  - name: Cleanup testing environment.
+    import_tasks: env_cleanup.yml

tests/vault/test_vault_standard.yml

@@ -138,4 +138,4 @@
     failed_when: result.changed
 
   - name: Cleanup testing environment.
-    import_tasks: env_setup.yml
+    import_tasks: env_cleanup.yml

tests/vault/test_vault_symmetric.yml

@@ -43,7 +43,7 @@
       password: SomeVAULTpassword
       state: retrieved
     register: result
-    failed_when: result.vault.data != 'SomeADMINpassword' or result.changed
+    failed_when: result.changed or result.failed or result.vault.data != 'SomeADMINpassword'
 
   - name: Archive data to symmetric vault
     ipavault:
@@ -61,7 +61,7 @@
       password: SomeVAULTpassword
       state: retrieved
     register: result
-    failed_when: result.vault.data != 'Hello World.' or result.changed
+    failed_when: result.changed or result.failed or result.vault.data != 'Hello World.'
 
   - name: Retrieve data from symmetric vault into file {{ ansible_env.HOME }}/data.txt.
     ipavault:
@@ -86,7 +86,7 @@
       password: SomeVAULTpassword
       vault_data: The world of π is half rounded.
     register: result
-    failed_when: not result.changed
+    failed_when: result.failed or not result.changed
 
   - name: Retrieve data from symmetric vault.
     ipavault:
@@ -95,7 +95,7 @@
       password: SomeVAULTpassword
       state: retrieved
     register: result
-    failed_when: result.vault.data != 'The world of π is half rounded.' or result.changed
+    failed_when: result.failed or result.changed or result.vault.data != 'The world of π is half rounded.'
 
   - name: Archive data in symmetric vault, from file.
     ipavault:
@@ -104,7 +104,7 @@
       in: "{{ ansible_env.HOME }}/in.txt"
       password: SomeVAULTpassword
     register: result
-    failed_when: not result.changed
+    failed_when: result.failed or not result.changed
 
   - name: Retrieve data from symmetric vault.
     ipavault:
@@ -113,7 +113,7 @@
       password: SomeVAULTpassword
       state: retrieved
     register: result
-    failed_when: result.vault.data != 'Another World.' or result.changed
+    failed_when: result.failed or result.changed or result.vault.data != 'Another World.'
 
   - name: Archive data with single character to symmetric vault
     ipavault:
@@ -122,7 +122,7 @@
       password: SomeVAULTpassword
       vault_data: c
     register: result
-    failed_when: not result.changed
+    failed_when: result.failed or not result.changed
 
   - name: Retrieve data from symmetric vault.
     ipavault:
@@ -131,7 +131,7 @@
       password: SomeVAULTpassword
       state: retrieved
     register: result
-    failed_when: result.vault.data != 'c' or result.changed
+    failed_when: result.failed or result.changed or result.vault.data != 'c'
 
   - name: Ensure symmetric vault is absent
     ipavault:
@@ -139,7 +139,7 @@
       name: symvault
       state: absent
     register: result
-    failed_when: not result.changed
+    failed_when: result.failed or not result.changed
 
   - name: Ensure symmetric vault is absent, again
     ipavault:
@@ -147,7 +147,7 @@
       name: symvault
       state: absent
     register: result
-    failed_when: result.changed
+    failed_when: result.failed or result.changed
 
   - name: Ensure symmetric vault is present, with password from file.
     ipavault:
@@ -157,7 +157,7 @@
       password_file: "{{ ansible_env.HOME }}/password.txt"
       vault_type: symmetric
     register: result
-    failed_when: not result.changed
+    failed_when: result.failed or not result.changed
 
   - name: Ensure symmetric vault is present, with password from file, again.
     ipavault:
@@ -167,7 +167,7 @@
       password_file: "{{ ansible_env.HOME }}/password.txt"
       vault_type: symmetric
     register: result
-    failed_when: result.changed
+    failed_when: result.failed or result.changed
 
   - name: Archive data to symmetric vault
     ipavault:
@@ -176,7 +176,7 @@
       vault_data: Hello World.
       password: SomeVAULTpassword
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: Retrieve data from symmetric vault.
     ipavault:
@@ -185,7 +185,7 @@
       password: SomeVAULTpassword
       state: retrieved
     register: result
-    failed_when: result.vault.data != 'Hello World.' or result.changed
+    failed_when: result.failed or result.changed or result.vault.data != 'Hello World.'
 
   - name: Retrieve data from symmetric vault, with password file.
     ipavault:
@@ -194,7 +194,7 @@
       password_file: "{{ ansible_env.HOME }}/password.txt"
       state: retrieved
     register: result
-    failed_when: result.vault.data != 'Hello World.' or result.changed
+    failed_when: result.failed or result.changed or result.vault.data != 'Hello World.'
 
   - name: Retrieve data from symmetric vault, with wrong password.
     ipavault:
@@ -203,7 +203,7 @@
       password: SomeWRONGpassword
       state: retrieved
     register: result
-    failed_when: not result.failed or "Invalid credentials" not in result.msg
+    failed_when: result.changed or not result.failed or "Invalid credentials" not in result.msg
 
   - name: Change vault password.
     ipavault:
@@ -212,7 +212,7 @@
       password: SomeVAULTpassword
       new_password: SomeNEWpassword
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: Retrieve data from symmetric vault, with new password.
     ipavault:
@@ -221,7 +221,7 @@
       password: SomeNEWpassword
       state: retrieved
     register: result
-    failed_when: result.vault.data != 'Hello World.' or result.changed
+    failed_when: result.failed or result.changed or result.vault.data != 'Hello World.'
 
   - name: Retrieve data from symmetric vault, with old password.
     ipavault:
@@ -240,7 +240,7 @@
       new_password: SomeVAULTpassword
       salt: AAAAAAAAAAAAAAAAAAAAAAA=
     register: result
-    failed_when: not result.changed
+    failed_when: result.failed or not result.changed
 
   - name: Change symmetric vault salt, without changing password
     ipavault:
@@ -250,7 +250,7 @@
       new_password: SomeVAULTpassword
       salt: MTIzNDU2Nzg5MDEyMzQ1Ngo=
     register: result
-    failed_when: not result.changed
+    failed_when: result.failed or not result.changed
 
   - name: Try to change symmetric vault salt, without providing any password
     ipavault:
@@ -258,7 +258,7 @@
       name: symvault
       salt: MTIzNDU2Nzg5MDEyMzQ1Ngo=
     register: result
-    failed_when: not result.failed and  "Vault `salt` can only change when changing the password." not in result.msg
+    failed_when: not result.failed and "Vault `salt` can only change when changing the password." not in result.msg
 
   - name: Try to change symmetric vault salt, without providing `password`
     ipavault:
@@ -294,7 +294,7 @@
       name: symvault
       state: absent
     register: result
-    failed_when: not result.changed
+    failed_when: result.failed or not result.changed
 
   - name: Ensure symmetric vault is absent, again
     ipavault:
@@ -302,7 +302,7 @@
       name: symvault
       state: absent
     register: result
-    failed_when: result.changed
+    failed_when: result.failed or result.changed
 
   - name: Try to change password of inexistent vault.
     ipavault:
@@ -340,7 +340,7 @@
       password: SomeVAULTpassword
       state: retrieved
     register: result
-    failed_when: result.vault.data != 'Hello World.' or result.changed
+    failed_when: result.failed or result.changed or result.vault.data != 'Hello World.'
 
   - name: Ensure symmetric vault is absent
     ipavault:
@@ -348,7 +348,7 @@
       name: symvault
       state: absent
     register: result
-    failed_when: not result.changed
+    failed_when: result.failed or not result.changed
 
   - name: Cleanup testing environment.
     import_tasks: env_cleanup.yml

utils/build-galaxy-release.sh

@@ -1,7 +1,7 @@
 #!/bin/bash
 
-namespace="${1:freeipa}"
+namespace="${1-freeipa}"
-collection="${2:ansible_freeipa}"
+collection="${2-ansible_freeipa}"
 collection_prefix="${namespace}.${collection}"
 
 galaxy_version=$(git describe --tags | sed -e "s/^v//")
@@ -14,46 +14,50 @@ find . -name "*~" -exec rm {} \;
 
 sed -i -e "s/ansible.module_utils.ansible_freeipa_module/ansible_collections.${collection_prefix}.plugins.module_utils.ansible_freeipa_module/" plugins/modules/*.py
 
-cd plugins/module_utils && {
+(cd plugins/module_utils && {
     ln -s ../../roles/*/module_utils/*.py .
-    cd ../..
+})
-}
 
-cd plugins/modules && {
+(cd plugins/modules && {
     sed -i -e "s/ansible.module_utils.ansible_ipa_/ansible_collections.${collection_prefix}.plugins.module_utils.ansible_ipa_/" ../../roles/*/library/*.py
     ln -s ../../roles/*/library/*.py .
-    cd ../..
+})
-}
 
 [ ! -x plugins/action_plugins ] && mkdir plugins/action_plugins
-cd plugins/action_plugins && {
+(cd plugins/action_plugins && {
     ln -s ../../roles/*/action_plugins/*.py .
-    cd ../..
+})
-}
 
-for x in $(find plugins/modules -name "*.py" -print); do
+find plugins/modules -name "*.py" -print0 |
-    python utils/galaxyfy-module-EXAMPLES.py "$x" "ipa" "$collection_prefix"
+    while IFS= read -d -r '' line; do
-done
+        python utils/galaxyfy-module-EXAMPLES.py "$x" \
+               "ipa" "$collection_prefix"
+    done
 
-for x in $(find roles/*/library -name "*.py" -print); do
+find roles/*/library -name "*.py" -print0 |
-    python utils/galaxyfy-module-EXAMPLES.py "$x" "ipa" "$collection_prefix"
+    while IFS= read -d -r '' line; do
-done
+        python utils/galaxyfy-module-EXAMPLES.py "$x" \
+               "ipa" "$collection_prefix"
+    done
 
 for x in roles/*/tasks/*.yml; do
     python utils/galaxyfy-playbook.py "$x" "ipa" "$collection_prefix"
 done
 
-for x in $(find playbooks -name "*.yml" -print); do
+find playbooks -name "*.yml" -print0 |
-    python utils/galaxyfy-playbook.py "$x" "ipa" "$collection_prefix"
+    while IFS= read -d -r '' line; do
-done
+        python utils/galaxyfy-playbook.py "$x" "ipa" "$collection_prefix"
+    done
 
-for x in $(find . -name "README*.md" -print); do
+find . -name "README*.md" -print0 |
-    python utils/galaxyfy-README.py "$x" "ipa" "$collection_prefix"
+    while IFS= read -d -r '' line; do
-done
+        python utils/galaxyfy-README.py "$x" "ipa" "$collection_prefix"
+    done
 
-for x in $(find tests -name "*.yml" -print); do
+find tests -name "*.yml" -print0 |
-    python utils/galaxyfy-playbook.py "$x" "ipa" "$collection_prefix"
+    while IFS= read -d -r '' line; do
-done
+        python utils/galaxyfy-playbook.py "$x" "ipa" "$collection_prefix"
+    done
 
 #git diff
 

utils/changelog

@@ -87,7 +87,7 @@ def store(commits, prs, authors, commit, author, merge, msg):
 
 def get_commit(commits, commit):
     _commits = [value for key, value in commits.items()
-                if key.startswith(merge)]
+                if key.startswith(commit)]
     if len(_commits) == 1:
         return _commits[0]
     return commit

utils/gen_modules_docs.sh

@@ -1,3 +1,5 @@
+#!/bin/bash
+
 for i in roles/ipa*/*/*.py; do
     python utils/gen_module_docs.py $i
 done

utils/lint_check.sh

@@ -1,6 +1,6 @@
 #!/bin/bash
 
-topdir=`dirname $(dirname $0)`
+topdir="`dirname $(dirname $0)`"
 
 flake8 .
 pydocstyle .
@@ -11,15 +11,18 @@ ANSIBLE_MODULE_UTILS=${ANSIBLE_MODULE_UTILS:-"${topdir}/plugins/module_utils"}
 export ANSIBLE_LIBRARY ANSIBLE_MODULE_UTILS
 
 yaml_dirs=(
-    "${topdir}/tests/*.yml"
+    "${topdir}/tests"
-    "${topdir}/tests/*/*.yml"
+    "${topdir}/playbooks"
-    "${topdir}/tests/*/*/*.yml"
+    "${topdir}/molecule"
-    "${topdir}/playbooks/*.yml"
-    "${topdir}/playbooks/*/*.yml"
-    "${topdir}/molecule/*/*.yml"
-    "${topdir}/molecule/*/*/*.yml"
 )
 
-ansible-lint --force-color ${yaml_dirs[@]}
+for dir in "${yaml_dirs[@]}"
+do
+    find "${dir}" -type f -name "*.yml" | xargs ansible-lint --force-color
+done
 
-yamllint -f colored ${yaml_dirs[@]}
+
+for dir in "${yaml_dirs[@]}"
+do
+    find "${dir}" -type f -name "*.yml" | xargs yamllint 
+done

utils/new_module

@@ -73,7 +73,7 @@ author=$2
 email=$3
 year=$(date +"%Y")
 
-if [ -z "$name" -o -z "$author" -o -z "$email" ]; then
+if [ -z "$name" ] || [ -z "$author" ] || [ -z "$email" ]; then
     [ -z "$name" ] && echo "ERROR: name is not valid"
     [ -z "$author" ] && echo "ERROR: author is not valid"
     [ -z "$email" ] && echo "ERROR: email is not valid"

utils/templates/ipamodule+member.py.in

@@ -286,6 +286,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:

utils/templates/ipamodule.py.in

@@ -207,6 +207,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: