Merge pull request #596 from freeipa/automember_verify_condition_keys

automember: Verify condition keys

README-automember.md

@@ -127,6 +127,7 @@ Variable | Description | Required
 `automember_type` | Grouping to which the rule applies. It can be one of `group`, `hostgroup`. | yes
 `inclusive` | List of dictionaries in the format of `{'key': attribute, 'expression': inclusive_regex}` | no
 `exclusive` | List of dictionaries in the format of `{'key': attribute, 'expression': exclusive_regex}` | no
+`action` | Work on automember or member level. It can be one of `member` or `automember` and defaults to `automember`. | no
 `state` | The state to ensure. It can be one of `present`, `absent`, default: `present`. | no
 
 

galaxy.yml

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

meta/runtime.yml

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

plugins/modules/ipaautomember.py

@@ -66,15 +66,33 @@ options:
     type: list
     elements: dict
     aliases: ["automemberexclusiveregex"]
+    options:
+      key:
+        description: The attribute of the regex
+        type: str
+        required: true
+      expression:
+        description: The expression of the regex
+        type: str
+        required: true
   inclusive:
     description: List of dictionaries containing the attribute and expression.
     type: list
     elements: dict
     aliases: ["automemberinclusiveregex"]
+    options:
+      key:
+        description: The attribute of the regex
+        type: str
+        required: true
+      expression:
+        description: The expression of the regex
+        type: str
+        required: true
   action:
-    description: Work on service or member level
-    default: service
-    choices: ["member", "service"]
+    description: Work on automember or member level
+    default: automember
+    choices: ["member", "automember"]
   state:
     description: State to ensure
     default: present
@@ -167,6 +185,15 @@ def transform_conditions(conditions):
     return transformed
 
 
+def check_condition_keys(ansible_module, conditions, aciattrs):
+    if conditions is None:
+        return
+    for condition in conditions:
+        if condition["key"] not in aciattrs:
+            ansible_module.fail_json(
+                msg="Invalid automember condition key '%s'" % condition["key"])
+
+
 def main():
     ansible_module = AnsibleModule(
         argument_spec=dict(
@@ -174,17 +201,27 @@ def main():
             ipaadmin_principal=dict(type="str", default="admin"),
             ipaadmin_password=dict(type="str", required=False, no_log=True),
 
-            inclusive=dict(type="list", aliases=[
-                           "automemberinclusiveregex"], default=None),
+            inclusive=dict(type="list",
+                           aliases=["automemberinclusiveregex"], default=None,
+                           options=dict(
+                               key=dict(type="str", required=True),
+                               expression=dict(type="str", required=True)
+                           ),
+                           elements="dict", required=False),
             exclusive=dict(type="list", aliases=[
-                           "automemberexclusiveregex"], default=None),
+                           "automemberexclusiveregex"], default=None,
+                           options=dict(
+                               key=dict(type="str", required=True),
+                               expression=dict(type="str", required=True)
+                           ),
+                           elements="dict", required=False),
             name=dict(type="list", aliases=["cn"],
                       default=None, required=True),
             description=dict(type="str", default=None),
             automember_type=dict(type='str', required=False,
                                  choices=['group', 'hostgroup']),
-            action=dict(type="str", default="service",
-                        choices=["member", "service"]),
+            action=dict(type="str", default="automember",
+                        choices=["member", "automember"]),
             state=dict(type="str", default="present",
                        choices=["present", "absent", "rebuild"]),
             users=dict(type="list", default=None),
@@ -246,11 +283,33 @@ def main():
             # Make sure automember rule exists
             res_find = find_automember(ansible_module, name, automember_type)
 
+            # Check inclusive and exclusive conditions
+            if inclusive is not None or exclusive is not None:
+                # automember_type is either "group" or "hostgorup"
+                if automember_type == "group":
+                    _type = "user"
+                elif automember_type == "hostgroup":
+                    _type = "host"
+                else:
+                    ansible_module.fail_json(
+                        msg="Bad automember type '%s'" % automember_type)
+
+                try:
+                    aciattrs = api_command(
+                        ansible_module, "json_metadata", to_text(_type), {}
+                    )['objects'][_type]['aciattrs']
+                except Exception as ex:
+                    ansible_module.fail_json(
+                        msg="%s: %s: %s" % ("json_metadata", _type, str(ex)))
+
+                check_condition_keys(ansible_module, inclusive, aciattrs)
+                check_condition_keys(ansible_module, exclusive, aciattrs)
+
             # Create command
             if state == 'present':
                 args = gen_args(description, automember_type)
 
-                if action == "service":
+                if action == "automember":
                     if res_find is not None:
                         if not compare_args_ipa(ansible_module,
                                                 args,
@@ -273,7 +332,8 @@ def main():
 
                 elif action == "member":
                     if res_find is None:
-                        ansible_module.fail_json(msg="No service '%s'" % name)
+                        ansible_module.fail_json(
+                            msg="No automember '%s'" % name)
 
                     inclusive_add = transform_conditions(inclusive or [])
                     inclusive_del = []
@@ -309,14 +369,15 @@ def main():
                                      condition_args])
 
             elif state == 'absent':
-                if action == "service":
+                if action == "automember":
                     if res_find is not None:
                         commands.append([name, 'automember_del',
                                          {'type': to_text(automember_type)}])
 
                 elif action == "member":
                     if res_find is None:
-                        ansible_module.fail_json(msg="No service '%s'" % name)
+                        ansible_module.fail_json(
+                            msg="No automember '%s'" % name)
 
                     if inclusive is not None:
                         for _inclusive in transform_conditions(inclusive):
@@ -355,7 +416,6 @@ def main():
         if ansible_module.check_mode:
             ansible_module.exit_json(changed=len(commands) > 0, **exit_args)
 
-        errors = []
         for name, command, args in commands:
             try:
                 if name is None:
@@ -372,16 +432,13 @@ def main():
             except Exception as ex:
                 ansible_module.fail_json(msg="%s: %s: %s" % (command, name,
                                                              str(ex)))
-            # Get all errors
-            if "failed" in result and len(result["failed"]) > 0:
-                for item in result["failed"]:
-                    failed_item = result["failed"][item]
-                    for member_type in failed_item:
-                        for member, failure in failed_item[member_type]:
-                            errors.append("%s: %s %s: %s" % (
-                                command, member_type, member, failure))
-        if len(errors) > 0:
-            ansible_module.fail_json(msg=", ".join(errors))
+
+            # result["failed"] is used only for INCLUDE_RE, EXCLUDE_RE
+            # if entries could not be added that are already there and
+            # it entries could not be removed that are not there.
+            # All other issues like invalid attributes etc. are handled
+            # as exceptions. Therefore the error section is not here as
+            # in other modules.
 
     except Exception as e:
         ansible_module.fail_json(msg=str(e))

plugins/modules/ipasudorule.py

@@ -53,7 +53,7 @@ options:
     required: false
     choices: ["all", ""]
     aliases: ["usercat"]
-  usergroup:
+  group:
     description: List of user groups assigned to the sudo rule.
     required: false
   runasgroupcategory:

requirements.txt

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

roles/ipabackup/library/ipabackup_get_backup_dir.py

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

roles/ipabackup/tasks/get_ipabackup_dir.yml

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

roles/ipaserver/README.md

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

tests/automember/test_automember.yml

@@ -164,6 +164,34 @@
     register: result
     failed_when: result.changed or result.failed
 
+  - name: Ensure testgroup group automember conditions fails on invalid inclusive key
+    ipaautomember:
+      ipaadmin_principal: admin
+      ipaadmin_password: SomeADMINpassword
+      name: testgroup
+      automember_type: group
+      inclusive:
+        - key: cns
+          expression: 'foo'
+      action: member
+    register: result
+    failed_when: result.changed or not result.failed or
+                 "Invalid automember condition key 'cns'" not in result.msg
+
+  - name: Ensure testgroup group automember conditions fails on invalid exlusive key
+    ipaautomember:
+      ipaadmin_principal: admin
+      ipaadmin_password: SomeADMINpassword
+      name: testgroup
+      automember_type: group
+      exclusive:
+        - key: cns
+          expression: 'foo'
+      action: member
+    register: result
+    failed_when: result.changed or not result.failed or
+                 "Invalid automember condition key 'cns'" not in result.msg
+
   - name: Ensure testhostgroup hostgroup automember rule is present
     ipaautomember:
       ipaadmin_password: SomeADMINpassword
@@ -282,6 +310,35 @@
     register: result
     failed_when: result.changed or result.failed
 
+
+  - name: Ensure testhostgroup hostgroup automember conditions fails on invalid inclusive key
+    ipaautomember:
+      ipaadmin_principal: admin
+      ipaadmin_password: SomeADMINpassword
+      name: testhostgroup
+      automember_type: hostgroup
+      inclusive:
+        - key: cns
+          expression: 'foo'
+      action: member
+    register: result
+    failed_when: result.changed or not result.failed or
+                 "Invalid automember condition key 'cns'" not in result.msg
+
+  - name: Ensure testhostgroup hostgroup automember conditions fails on invalid exlusive key
+    ipaautomember:
+      ipaadmin_principal: admin
+      ipaadmin_password: SomeADMINpassword
+      name: testhostgroup
+      automember_type: hostgroup
+      exclusive:
+        - key: cns
+          expression: 'foo'
+      action: member
+    register: result
+    failed_when: result.changed or not result.failed or
+                 "Invalid automember condition key 'cns'" not in result.msg
+
   # CLEANUP TEST ITEMS
 
   - name: Ensure group testgroup is absent

tests/pwpolicy/test_pwpolicy.yml

@@ -81,8 +81,7 @@
       ipaadmin_password: SomeADMINpassword
       state: absent
     register: result
-    ignore_errors: True
-    failed_when: (result is defined and result) or result.failed
+    failed_when: not result.failed or "'global_policy' can not be made absent." not in result.msg
 
   - name: Ensure absence of pwpolicies for group ops
     ipapwpolicy: