From da775a21b2d1d080bea7da746b101ac14d642779 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Mon, 4 Nov 2024 19:55:13 -0300 Subject: [PATCH 1/3] ansible_freeipa_module_utils: Add EntryFactory class This patch adds the class EntryFactory to the ansible-freeipa module utils. This class allows the handling of modules with multiple object entries as list of objects. When the multi-object parameter is not used, it creates a list of a single object, allowing for the same code idiom to be used. The entries created can be used both as objects, by acessing the values as properties, or as dictionaires, by accessing the elements as key-value pairs. --- .../module_utils/ansible_freeipa_module.py | 233 ++++++++++++++++++ setup.cfg | 3 + 2 files changed, 236 insertions(+) diff --git a/plugins/module_utils/ansible_freeipa_module.py b/plugins/module_utils/ansible_freeipa_module.py index 2f90b3e10f..3386cb8c10 100644 --- a/plugins/module_utils/ansible_freeipa_module.py +++ b/plugins/module_utils/ansible_freeipa_module.py @@ -526,6 +526,10 @@ def module_params_get(module, name, allow_empty_list_item=False): def module_params_get_lowercase(module, name, allow_empty_list_item=False): value = module_params_get(module, name, allow_empty_list_item) + return convert_param_value_to_lowercase(value) + + +def convert_param_value_to_lowercase(value): if isinstance(value, list): value = [v.lower() for v in value] if isinstance(value, (str, unicode)): @@ -1584,3 +1588,232 @@ def tm_warn(self, warning): ts = time.time() # pylint: disable=super-with-arguments super(IPAAnsibleModule, self).warn("%f %s" % (ts, warning)) + + +class EntryFactory: + """ + Implement an Entry Factory to extract objects from modules. + + When defining an ansible-freeipa module which allows the setting of + multiple objects in a single task, the object parameters can be set + as a set of parameters, or as a list of dictionaries with multiple + objects. + + The EntryFactory abstracts the extraction of the entry values so + that the entries set in a module can be treated as a list of objects + independent of the way the objects have been defined (as single object + defined by its parameters or as a list). + + Parameters + ---------- + ansible_module: The ansible module to be processed. + invalid_params: The list of invalid parameters for the current + state/action combination. + multiname: The name of the list of objects parameters. + params: a dict of the entry parameters with its configuration as a + dict. The 'convert' configuration is a list of functions to be + applied, in order, to the provided value for the paarameter. Any + other configuration field is ignored in the current implementation. + validate_entry: an optional function to validate the entry values. + This function is called after the parameters for the current + state/action are checked, and can be used to perform further + validation or modification to the entry values. If the entry is + not valid, 'fail_json' should be called. The function must return + the entry, modified or not. The funcion signature is + 'def fn(module:IPAAnsibleModule, entry: Entry) -> Entry:' + **user_vars: any other keyword argument is passed to the + validate_entry callback as user data. + + Example + ------- + def validate_entry(module, entry, mydata): + if (something_is_wrong(entry)): + module.fail_json(msg=f"Something wrong with {entry.name}") + entry.some_field = mydata + return entry + + def main(): + # ... + # Create param mapping, all moudle parameters must be + # present as keys of this dictionary + params = { + "name": {}, + "description": {} + "user": { + "convert": [convert_param_value_to_lowercase] + }, + "group": {"convert": [convert_param_value_to_lowercase]} + } + entries = EntryFactory( + module, invalid_params, "entries", params, + validate_entry=validate_entry, + mydata=1234 + ) + #... + with module.ipa_connect(context=context): + # ... + for entry in entries: + # process entry and create commands + # ... + + """ + + def __init__( + self, + ansible_module, + invalid_params, + multiname, + params, + validate_entry=None, + **user_vars + ): + """Initialize the Entry Factory.""" + self.ansible_module = ansible_module + self.invalid_params = set(invalid_params) + self.multiname = multiname + self.params = params + self.convert = { + param: (config or {}).get("convert", []) + for param, config in params.items() + } + self.validate_entry = validate_entry + self.user_vars = user_vars + self.__entries = self._get_entries() + + def __iter__(self): + """Initialize factory iterator.""" + return iter(self.__entries) + + def __next__(self): + """Retrieve next entry.""" + return next(self.__entries) + + def check_invalid_parameter_usage(self, entry_dict, fail_on_check=True): + """ + Check if entry_dict parameters are valid for the current state/action. + + Parameters + ---------- + entry_dict: A dictionary representing the module parameters. + fail_on_check: If set to True wil make the module execution fail + if invalid parameters are used. + + Return + ------ + If fail_on_check is not True, returns True if the entry parameters + are valid and execution should proceed, False otherwise. + + """ + state = self.ansible_module.params_get("state") + action = self.ansible_module.params_get("action") + + if action is None: + msg = "Arguments '{0}' can not be used with state '{1}'" + else: + msg = "Arguments '{0}' can not be used with action " \ + "'{2}' and state '{1}'" + + entry_params = set(k for k, v in entry_dict.items() if v is not None) + match_invalid = self.invalid_params & entry_params + if match_invalid: + if fail_on_check: + self.ansible_module.fail_json( + msg=msg.format(", ".join(match_invalid), state, action)) + return False + + if not entry_dict.get("name"): + if fail_on_check: + self.ansible_module.fail_json(msg="Entry 'name' is not set.") + return False + + return True + + class Entry: + """Provide an abstraction to handle module entries.""" + + def __init__(self, values): + """Initialize entry to be used as dict or object.""" + self.values = values + for key, value in values.items(): + setattr(self, key, value) + + def copy(self): + """Make a copy of the entry.""" + return EntryFactory.Entry(self.values.copy()) + + def __setitem__(self, item, value): + """Allow entries to be treated as dictionaries.""" + self.values[item] = value + setattr(self, item, value) + + def __getitem__(self, item): + """Allow entries to be treated as dictionaries.""" + return self.values[item] + + def __setattr__(self, attrname, value): + if attrname != "values" and attrname in self.values: + self.values[attrname] = value + super().__setattr__(attrname, value) + + def __repr__(self): + """Provide a string representation of the stored values.""" + return repr(self.values) + + def _get_entries(self): + """Retrieve all entries from the module.""" + def copy_entry_and_set_name(entry, name): + _result = entry.copy() + _result.name = name + return _result + + names = self.ansible_module.params_get("name") + if names is not None: + if not isinstance(names, list): + names = [names] + # Get entrie(s) defined by the 'name' parameter. + # For most states and modules, 'name' will represent a single + # entry, but for some states, like 'absent', it could be a + # list of names. + _entry = self._extract_entry( + self.ansible_module, + IPAAnsibleModule.params_get + ) + # copy attribute values if 'name' returns a list + _entries = [ + copy_entry_and_set_name(_entry, _name) + for _name in names + ] + else: + _entries = [ + self._extract_entry(data, dict.get) + for data in self.ansible_module.params_get(self.multiname) + ] + + return _entries + + def _extract_entry(self, data, param_get): + """Extract an entry from the given data, using the given method.""" + def get_entry_param_value(parameter, conversions): + _value = param_get(data, parameter) + if _value and conversions: + for fn in conversions: + _value = fn(_value) + return _value + + # Build 'parameter: value' mapping for all module parameters + _entry = { + parameter: get_entry_param_value(parameter, conversions) + for parameter, conversions in self.convert.items() + } + + # Check if any invalid parameter is used. + self.check_invalid_parameter_usage(_entry) + + # Create Entry object + _result = EntryFactory.Entry(_entry) + # Call entry validation callback, if provided. + if self.validate_entry: + _result = self.validate_entry( + self.ansible_module, _result, **self.user_vars) + + return _result diff --git a/setup.cfg b/setup.cfg index c44ac599c7..c1cd4c64d1 100644 --- a/setup.cfg +++ b/setup.cfg @@ -82,6 +82,9 @@ ignored-modules = os, SSSDConfig +[pylint.DESIGN] +max-attributes=12 + [pylint.REFACTORING] max-nested-blocks = 9 From 4fa0621156ff4a3ed4b51063af6892f1fcbfc9a9 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Wed, 4 Sep 2024 21:43:35 -0300 Subject: [PATCH 2/3] ipasudorule: Add support for batch mode and multiple sudorules Currently, ipasudorule must add or modify a single sudorule at a time, incurring in more load in the server if there are many rules to be processed. This patch adds suport for adding multiple sudorules in one playbook task by using the parameter 'sudorules' and defining a list of sudorules configurations to be ensured. As multiple sudorules will be processed, the patch also enables batch mode processing of sudorules, trying to reduce the load on the server. Test 'tests/sudorule/test_sudorule_client_context.yml' was modified to include tasks with 'sudorules' to be executed both on the server or on the client context. New tests were added to the sudorule test suite: tests/sudorule/test_sudorules.yml tests/sudorule/test_sudorules_member_case_insensitive.yml --- README-sudorule.md | 47 +- plugins/modules/ipasudorule.py | 732 +++++++++++------- .../sudorule/test_sudorule_client_context.yml | 12 + tests/sudorule/test_sudorules.yml | 382 +++++++++ ...test_sudorules_member_case_insensitive.yml | 311 ++++++++ 5 files changed, 1210 insertions(+), 274 deletions(-) create mode 100644 tests/sudorule/test_sudorules.yml create mode 100644 tests/sudorule/test_sudorules_member_case_insensitive.yml diff --git a/README-sudorule.md b/README-sudorule.md index 6945138cb3..a30e11e8b0 100644 --- a/README-sudorule.md +++ b/README-sudorule.md @@ -129,6 +129,49 @@ Example playbook to make sure Sudo Rule is absent: state: absent ``` +Example playbook to ensure multiple Sudo Rule are present using batch mode: + +```yaml +--- +- name: Playbook to handle sudorules + hosts: ipaserver + become: true + +- name: Ensure multiple Sudo Rules are present using batch mode. + ipasudorule: + ipaadmin_password: SomeADMINpassword + sudorules: + - name: testrule1 + hostmask: + - 192.168.122.1/24 + - name: testrule2 + hostcategory: all +``` + +Example playbook to ensure multiple Sudo Rule members are present using batch mode: + +```yaml +--- +- name: Playbook to handle sudorules + hosts: ipaserver + become: true + +- name: Ensure multiple Sudo Rules are present using batch mode. + ipasudorule: + ipaadmin_password: SomeADMINpassword + action: member + sudorules: + - name: testrule1 + user: + - user01 + - user02 + group: + - group01 + - name: testrule2 + hostgroup: + - hostgroup01 + - hostgroup02 +``` Variables ========= @@ -139,7 +182,9 @@ 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 `ipaapi_context` | The context in which the module will execute. Executing in a server context is preferred. If not provided context will be determined by the execution environment. Valid values are `server` and `client`. | no `ipaapi_ldap_cache` | Use LDAP cache for IPA connection. The bool setting defaults to yes. (bool) | no -`name` \| `cn` | The list of sudorule name strings. | yes +`name` \| `cn` | The list of sudorule name strings. | no +`sudorules` | The list of sudorule dicts. Each `sudorule` dict entry can contain sudorule variables.
There is one required option in the `sudorule` dict:| no +  | `name` - The sudorule name string of the entry. | yes `description` | The sudorule description string. | no `usercategory` \| `usercat` | User category the rule applies to. Choices: ["all", ""] | no `hostcategory` \| `hostcat` | Host category the rule applies to. Choices: ["all", ""] | no diff --git a/plugins/modules/ipasudorule.py b/plugins/modules/ipasudorule.py index a4d5571fe8..d41bebd25d 100644 --- a/plugins/modules/ipasudorule.py +++ b/plugins/modules/ipasudorule.py @@ -42,8 +42,128 @@ description: The sudorule name type: list elements: str - required: true + required: false aliases: ["cn"] + sudorules: + description: The list of sudorule dicts. + type: list + elements: dict + suboptions: + name: + description: The sudorule name + type: list + elements: str + required: true + aliases: ["cn"] + description: + description: The sudorule description + type: str + required: false + user: + description: List of users assigned to the sudo rule. + type: list + elements: str + required: false + usercategory: + description: User category the sudo rule applies to + type: str + required: false + choices: ["all", ""] + aliases: ["usercat"] + group: + description: List of user groups assigned to the sudo rule. + type: list + elements: str + required: false + runasgroupcategory: + description: RunAs Group category applied to the sudo rule. + type: str + required: false + choices: ["all", ""] + aliases: ["runasgroupcat"] + runasusercategory: + description: RunAs User category applied to the sudorule. + type: str + required: false + choices: ["all", ""] + aliases: ["runasusercat"] + nomembers: + description: Suppress processing of membership attributes + required: false + type: bool + host: + description: List of host names assigned to this sudorule. + required: false + type: list + elements: str + hostgroup: + description: List of host groups assigned to this sudorule. + required: false + type: list + elements: str + hostcategory: + description: Host category the sudo rule applies to. + type: str + required: false + choices: ["all", ""] + aliases: ["hostcat"] + allow_sudocmd: + description: List of allowed sudocmds assigned to this sudorule. + required: false + type: list + elements: str + allow_sudocmdgroup: + description: List of allowed sudocmd groups assigned to this sudorule. + required: false + type: list + elements: str + deny_sudocmd: + description: List of denied sudocmds assigned to this sudorule. + required: false + type: list + elements: str + deny_sudocmdgroup: + description: List of denied sudocmd groups assigned to this sudorule. + required: false + type: list + elements: str + cmdcategory: + description: Command category the sudo rule applies to + type: str + required: false + choices: ["all", ""] + aliases: ["cmdcat"] + order: + description: Order to apply this rule. + required: false + type: int + aliases: ["sudoorder"] + sudooption: + description: List of sudo options. + required: false + type: list + elements: str + aliases: ["options"] + runasuser: + description: List of users for Sudo to execute as. + required: false + type: list + elements: str + runasuser_group: + description: List of groups for Sudo to execute as. + required: false + type: list + elements: str + runasgroup: + description: List of groups for Sudo to execute as. + required: false + type: list + elements: str + hostmask: + description: Host masks of allowed hosts. + required: false + type: list + elements: str description: description: The sudorule description type: str @@ -232,6 +352,16 @@ ipaadmin_password: SomeADMINpassword name: testrule1 state: absent + +# Ensure multiple Sudo Rules are present using batch mode. +- ipasudorule: + ipaadmin_password: SomeADMINpassword + sudorules: + - name: testrule1 + hostmask: + - 192.168.122.1/24 + - name: testrule2 + hostcategory: all """ RETURN = """ @@ -239,180 +369,196 @@ from ansible.module_utils.ansible_freeipa_module import \ IPAAnsibleModule, compare_args_ipa, gen_add_del_lists, gen_add_list, \ - gen_intersection_list, api_get_domain, ensure_fqdn, netaddr, to_text + gen_intersection_list, api_get_domain, ensure_fqdn, netaddr, to_text, \ + ipalib_errors, convert_param_value_to_lowercase, EntryFactory def find_sudorule(module, name): _args = { "all": True, - "cn": name, } - _result = module.ipa_command("sudorule_find", name, _args) - - if len(_result["result"]) > 1: - module.fail_json( - msg="There is more than one sudorule '%s'" % (name)) - elif len(_result["result"]) == 1: - return _result["result"][0] - - return None + try: + _result = module.ipa_command("sudorule_show", name, _args) + except ipalib_errors.NotFound: + return None + return _result["result"] -def gen_args(description, usercat, hostcat, cmdcat, runasusercat, - runasgroupcat, order, nomembers): +def gen_args(entry): + """Generate args for sudorule.""" _args = {} - if description is not None: - _args['description'] = description - if usercat is not None: - _args['usercategory'] = usercat - if hostcat is not None: - _args['hostcategory'] = hostcat - if cmdcat is not None: - _args['cmdcategory'] = cmdcat - if runasusercat is not None: - _args['ipasudorunasusercategory'] = runasusercat - if runasgroupcat is not None: - _args['ipasudorunasgroupcategory'] = runasgroupcat - if order is not None: - _args['sudoorder'] = order - if nomembers is not None: - _args['nomembers'] = nomembers + if entry.description is not None: + _args['description'] = entry.description + if entry.usercategory is not None: + _args['usercategory'] = entry.usercategory + if entry.hostcategory is not None: + _args['hostcategory'] = entry.hostcategory + if entry.cmdcategory is not None: + _args['cmdcategory'] = entry.cmdcategory + if entry.runasusercategory is not None: + _args['ipasudorunasusercategory'] = entry.runasusercategory + if entry.runasgroupcategory is not None: + _args['ipasudorunasgroupcategory'] = entry.runasgroupcategory + if entry.order is not None: + _args['sudoorder'] = entry.order + if entry.nomembers is not None: + _args['nomembers'] = entry.nomembers return _args -def main(): - ansible_module = IPAAnsibleModule( - argument_spec=dict( - # general - name=dict(type="list", elements="str", aliases=["cn"], - required=True), - # present - description=dict(required=False, type="str", default=None), - usercategory=dict(required=False, type="str", default=None, - choices=["all", ""], aliases=['usercat']), - hostcategory=dict(required=False, type="str", default=None, - choices=["all", ""], aliases=['hostcat']), - nomembers=dict(required=False, type='bool', default=None), - host=dict(required=False, type='list', elements="str", +def init_ansible_module(): + """Initialize IPAAnsibleModule object for sudorule.""" + sudorule_spec = dict( + description=dict(required=False, type="str", default=None), + usercategory=dict(required=False, type="str", default=None, + choices=["all", ""], aliases=['usercat']), + hostcategory=dict(required=False, type="str", default=None, + choices=["all", ""], aliases=['hostcat']), + nomembers=dict(required=False, type='bool', default=None), + host=dict(required=False, type='list', elements="str", + default=None), + hostgroup=dict(required=False, type='list', elements="str", + default=None), + hostmask=dict(required=False, type='list', elements="str", default=None), - hostgroup=dict(required=False, type='list', elements="str", + user=dict(required=False, type='list', elements="str", + default=None), + group=dict(required=False, type='list', elements="str", + default=None), + allow_sudocmd=dict(required=False, type="list", elements="str", default=None), - hostmask=dict(required=False, type='list', elements="str", + deny_sudocmd=dict(required=False, type="list", elements="str", default=None), - user=dict(required=False, type='list', elements="str", - default=None), - group=dict(required=False, type='list', elements="str", - default=None), - allow_sudocmd=dict(required=False, type="list", elements="str", + allow_sudocmdgroup=dict(required=False, type="list", + elements="str", default=None), + deny_sudocmdgroup=dict(required=False, type="list", elements="str", default=None), - deny_sudocmd=dict(required=False, type="list", elements="str", - default=None), - allow_sudocmdgroup=dict(required=False, type="list", - elements="str", default=None), - deny_sudocmdgroup=dict(required=False, type="list", elements="str", - default=None), - cmdcategory=dict(required=False, type="str", default=None, - choices=["all", ""], aliases=['cmdcat']), - runasusercategory=dict(required=False, type="str", default=None, - choices=["all", ""], - aliases=['runasusercat']), - runasgroupcategory=dict(required=False, type="str", default=None, - choices=["all", ""], - aliases=['runasgroupcat']), - runasuser=dict(required=False, type="list", elements="str", - default=None), - runasgroup=dict(required=False, type="list", elements="str", - default=None), - runasuser_group=dict(required=False, type="list", elements="str", - default=None), - order=dict(type="int", required=False, aliases=['sudoorder']), - sudooption=dict(required=False, type='list', elements="str", - default=None, aliases=["options"]), + cmdcategory=dict(required=False, type="str", default=None, + choices=["all", ""], aliases=['cmdcat']), + runasusercategory=dict(required=False, type="str", default=None, + choices=["all", ""], + aliases=['runasusercat']), + runasgroupcategory=dict(required=False, type="str", default=None, + choices=["all", ""], + aliases=['runasgroupcat']), + runasuser=dict(required=False, type="list", elements="str", + default=None), + runasgroup=dict(required=False, type="list", elements="str", + default=None), + runasuser_group=dict(required=False, type="list", elements="str", + default=None), + order=dict(type="int", required=False, aliases=['sudoorder']), + sudooption=dict(required=False, type='list', elements="str", + default=None, aliases=["options"]), + ) + + ansible_module = IPAAnsibleModule( + argument_spec=dict( + # general + name=dict(type="list", elements="str", aliases=["cn"], + required=False), + sudorules=dict( + type="list", + defalut=None, + options=dict( + # name of the sudorule + name=dict(type="str", required=True, aliases=["cn"]), + # sudorule specific parameters + **sudorule_spec + ), + elements='dict', + required=False, + ), + # action action=dict(type="str", default="sudorule", choices=["member", "sudorule"]), # state state=dict(type="str", default="present", choices=["present", "absent", "enabled", "disabled"]), + # Specific parameters for simple use case + **sudorule_spec ), + mutually_exclusive=[["name", "sudorules"]], + required_one_of=[["name", "sudorules"]], supports_check_mode=True, ) ansible_module._ansible_debug = True + return ansible_module + + +def convert_list_of_hostmask(hostmasks): + """Ensure all hostmasks is hostmask_list is a CIDR value.""" + return [ + to_text(netaddr.IPNetwork(mask).cidr) + for mask in ( + hostmasks if isinstance(hostmasks, (list, tuple)) + else [hostmasks] + ) + ] + + +def convert_list_of_hostnames(hostnames): + """Ensure all hostnames in hostnames are lowercase FQDN.""" + return list( + set( + ensure_fqdn(value.lower(), api_get_domain()) + for value in ( + hostnames if isinstance(hostnames, (list, tuple)) + else [hostnames] + ) + ) + ) - # Get parameters +def validate_entry(module, entry, state, action): + """Ensure entry object is valid.""" + if state == "present" and action == "sudorule": + # Ensure the entry is valid for state:present, action:sudorule. + if entry.hostcategory == 'all' and any([entry.host, entry.hostgroup]): + module.fail_json( + msg="Hosts cannot be added when host category='all'" + ) + if entry.usercategory == 'all' and any([entry.user, entry.group]): + module.fail_json( + msg="Users cannot be added when user category='all'" + ) + if entry.cmdcategory == 'all' \ + and any([entry.allow_sudocmd, entry.allow_sudocmdgroup]): + module.fail_json( + msg="Commands cannot be added when command category='all'" + ) + return entry + + +def main(): + ansible_module = init_ansible_module() + # Get parameters # general names = ansible_module.params_get("name") - - # present - # The 'noqa' variables are not used here, but required for vars(). - # The use of 'noqa' ensures flake8 does not complain about them. - description = ansible_module.params_get("description") # noqa - cmdcategory = ansible_module.params_get('cmdcategory') # noqa - usercategory = ansible_module.params_get("usercategory") # noqa - hostcategory = ansible_module.params_get("hostcategory") # noqa - runasusercategory = ansible_module.params_get( # noqa - "runasusercategory") - runasgroupcategory = ansible_module.params_get( # noqa - "runasgroupcategory") - hostcategory = ansible_module.params_get("hostcategory") # noqa - nomembers = ansible_module.params_get("nomembers") # noqa - host = ansible_module.params_get("host") - hostgroup = ansible_module.params_get_lowercase("hostgroup") - hostmask = ansible_module.params_get("hostmask") - user = ansible_module.params_get_lowercase("user") - group = ansible_module.params_get_lowercase("group") - allow_sudocmd = ansible_module.params_get('allow_sudocmd') - allow_sudocmdgroup = \ - ansible_module.params_get_lowercase('allow_sudocmdgroup') - deny_sudocmd = ansible_module.params_get('deny_sudocmd') - deny_sudocmdgroup = \ - ansible_module.params_get_lowercase('deny_sudocmdgroup') - sudooption = ansible_module.params_get("sudooption") - order = ansible_module.params_get("order") - runasuser = ansible_module.params_get_lowercase("runasuser") - runasuser_group = ansible_module.params_get_lowercase("runasuser_group") - runasgroup = ansible_module.params_get_lowercase("runasgroup") + # sudorules = ansible_module.params_get("sudorules") + # action action = ansible_module.params_get("action") - # state state = ansible_module.params_get("state") - # ensure hostmasks are network cidr - if hostmask is not None: - hostmask = [to_text(netaddr.IPNetwork(x).cidr) for x in hostmask] - # Check parameters invalid = [] if state == "present": - if len(names) != 1: + if names is not None and len(names) != 1: ansible_module.fail_json( - msg="Only one sudorule can be added at a time.") + msg="Only one sudorule can be added at a time using 'name'.") if action == "member": invalid = ["description", "usercategory", "hostcategory", "cmdcategory", "runasusercategory", "runasgroupcategory", "order", "nomembers"] - else: - if hostcategory == 'all' and any([host, hostgroup]): - ansible_module.fail_json( - msg="Hosts cannot be added when host category='all'") - if usercategory == 'all' and any([user, group]): - ansible_module.fail_json( - msg="Users cannot be added when user category='all'") - if cmdcategory == 'all' \ - and any([allow_sudocmd, allow_sudocmdgroup]): - ansible_module.fail_json( - msg="Commands cannot be added when command category='all'") - elif state == "absent": - if len(names) < 1: - ansible_module.fail_json(msg="No name given.") invalid = ["description", "usercategory", "hostcategory", "cmdcategory", "runasusercategory", "runasgroupcategory", "nomembers", "order"] @@ -424,8 +570,6 @@ def main(): "runasuser_group"]) elif state in ["enabled", "disabled"]: - if len(names) < 1: - ansible_module.fail_json(msg="No name given.") if action == "member": ansible_module.fail_json( msg="Action member can not be used with states enabled and " @@ -439,48 +583,81 @@ def main(): else: ansible_module.fail_json(msg="Invalid state '%s'" % state) - ansible_module.params_fail_used_invalid(invalid, state, action) - # Init - changed = False exit_args = {} + # Factory parameters + params = { + "name": {}, + "description": {}, + "cmdcategory": {}, + "usercategory": {}, + "hostcategory": {}, + "runasusercategory": {}, + "runasgroupcategory": {}, + "host": {"convert": [convert_list_of_hostnames]}, + "hostgroup": {"convert": [convert_param_value_to_lowercase]}, + "hostmask": {"convert": [convert_list_of_hostmask]}, + "user": {"convert": [convert_param_value_to_lowercase]}, + "group": {"convert": [convert_param_value_to_lowercase]}, + "allow_sudocmd": {}, + "allow_sudocmdgroup": {"convert": [convert_param_value_to_lowercase]}, + "deny_sudocmd": {}, + "deny_sudocmdgroup": {"convert": [convert_param_value_to_lowercase]}, + "sudooption": {}, + "order": {}, + "runasuser": {"convert": [convert_param_value_to_lowercase]}, + "runasuser_group": {"convert": [convert_param_value_to_lowercase]}, + "runasgroup": {"convert": [convert_param_value_to_lowercase]}, + "nomembers": {}, + } + # Connect to IPA API with ansible_module.ipa_connect(): - default_domain = api_get_domain() - - # Ensure host is not short hostname. - if host: - host = list( - {ensure_fqdn(value.lower(), default_domain) for value in host} - ) - commands = [] - host_add, host_del = [], [] - user_add, user_del = [], [] - group_add, group_del = [], [] - hostgroup_add, hostgroup_del = [], [] - hostmask_add, hostmask_del = [], [] - allow_cmd_add, allow_cmd_del = [], [] - allow_cmdgroup_add, allow_cmdgroup_del = [], [] - deny_cmd_add, deny_cmd_del = [], [] - deny_cmdgroup_add, deny_cmdgroup_del = [], [] - sudooption_add, sudooption_del = [], [] - runasuser_add, runasuser_del = [], [] - runasuser_group_add, runasuser_group_del = [], [] - runasgroup_add, runasgroup_del = [], [] - - for name in names: - # Make sure sudorule exists - res_find = find_sudorule(ansible_module, name) + + # Creating factory after connect as host conversion + # requires 'api_get_domain()' to be available + entry_factory = EntryFactory( + ansible_module, + invalid, + "sudorules", + params, + validate_entry=validate_entry, + state=state, + action=action, + ) + + for entry in entry_factory: + host_add, host_del = [], [] + user_add, user_del = [], [] + group_add, group_del = [], [] + hostgroup_add, hostgroup_del = [], [] + hostmask_add, hostmask_del = [], [] + allow_cmd_add, allow_cmd_del = [], [] + allow_cmdgroup_add, allow_cmdgroup_del = [], [] + deny_cmd_add, deny_cmd_del = [], [] + deny_cmdgroup_add, deny_cmdgroup_del = [], [] + sudooption_add, sudooption_del = [], [] + runasuser_add, runasuser_del = [], [] + runasuser_group_add, runasuser_group_del = [], [] + runasgroup_add, runasgroup_del = [], [] + + # Try to retrieve sudorule + res_find = find_sudorule(ansible_module, entry.name) + + # Fail if sudorule must exist but is not found + if ( + (state in ["enabled", "disabled"] or action == "member") + and res_find is None + ): + ansible_module.fail_json(msg="No sudorule '%s'" % entry.name) # Create command if state == "present": # Generate args - args = gen_args(description, usercategory, hostcategory, - cmdcategory, runasusercategory, - runasgroupcategory, order, nomembers) + args = gen_args(entry) if action == "sudorule": # Found the sudorule if res_find is not None: @@ -489,25 +666,35 @@ def main(): # from args if "" and if the category is not in the # sudorule. The empty string is used to reset the # category. - if "usercategory" in args \ - and args["usercategory"] == "" \ - and "usercategory" not in res_find: + if ( + "usercategory" in args + and args["usercategory"] == "" + and "usercategory" not in res_find + ): del args["usercategory"] - if "hostcategory" in args \ - and args["hostcategory"] == "" \ - and "hostcategory" not in res_find: + if ( + "hostcategory" in args + and args["hostcategory"] == "" + and "hostcategory" not in res_find + ): del args["hostcategory"] - if "cmdcategory" in args \ - and args["cmdcategory"] == "" \ - and "cmdcategory" not in res_find: + if ( + "cmdcategory" in args + and args["cmdcategory"] == "" + and "cmdcategory" not in res_find + ): del args["cmdcategory"] - if "ipasudorunasusercategory" in args \ - and args["ipasudorunasusercategory"] == "" \ - and "ipasudorunasusercategory" not in res_find: + if ( + "ipasudorunasusercategory" in args + and args["ipasudorunasusercategory"] == "" + and "ipasudorunasusercategory" not in res_find + ): del args["ipasudorunasusercategory"] - if "ipasudorunasgroupcategory" in args \ - and args["ipasudorunasgroupcategory"] == "" \ - and "ipasudorunasgroupcategory" not in res_find: + if ( + "ipasudorunasgroupcategory" in args + and args["ipasudorunasgroupcategory"] == "" + and "ipasudorunasgroupcategory" not in res_find + ): del args["ipasudorunasgroupcategory"] # For all settings is args, check if there are @@ -515,46 +702,48 @@ def main(): # If yes: modify if not compare_args_ipa(ansible_module, args, res_find): - commands.append([name, "sudorule_mod", args]) + commands.append([entry.name, "sudorule_mod", args]) else: - commands.append([name, "sudorule_add", args]) + commands.append([entry.name, "sudorule_add", args]) # Set res_find to empty dict for next step res_find = {} # Generate addition and removal lists host_add, host_del = gen_add_del_lists( - host, res_find.get('memberhost_host', [])) + entry.host, res_find.get('memberhost_host', [])) hostgroup_add, hostgroup_del = gen_add_del_lists( - hostgroup, res_find.get('memberhost_hostgroup', [])) + entry.hostgroup, + res_find.get('memberhost_hostgroup', []) + ) hostmask_add, hostmask_del = gen_add_del_lists( - hostmask, res_find.get('hostmask', [])) + entry.hostmask, res_find.get('hostmask', [])) user_add, user_del = gen_add_del_lists( - user, res_find.get('memberuser_user', [])) + entry.user, res_find.get('memberuser_user', [])) group_add, group_del = gen_add_del_lists( - group, res_find.get('memberuser_group', [])) + entry.group, res_find.get('memberuser_group', [])) allow_cmd_add, allow_cmd_del = gen_add_del_lists( - allow_sudocmd, + entry.allow_sudocmd, res_find.get('memberallowcmd_sudocmd', [])) allow_cmdgroup_add, allow_cmdgroup_del = gen_add_del_lists( - allow_sudocmdgroup, + entry.allow_sudocmdgroup, res_find.get('memberallowcmd_sudocmdgroup', [])) deny_cmd_add, deny_cmd_del = gen_add_del_lists( - deny_sudocmd, + entry.deny_sudocmd, res_find.get('memberdenycmd_sudocmd', [])) deny_cmdgroup_add, deny_cmdgroup_del = gen_add_del_lists( - deny_sudocmdgroup, + entry.deny_sudocmdgroup, res_find.get('memberdenycmd_sudocmdgroup', [])) sudooption_add, sudooption_del = gen_add_del_lists( - sudooption, res_find.get('ipasudoopt', [])) + entry.sudooption, res_find.get('ipasudoopt', [])) # runasuser attribute can be used with both IPA and # non-IPA (external) users. IPA will handle the correct @@ -562,15 +751,15 @@ def main(): # the provided list against both users and external # users list. runasuser_add, runasuser_del = gen_add_del_lists( - runasuser, + entry.runasuser, ( - res_find.get('ipasudorunas_user', []) - + res_find.get('ipasudorunasextuser', []) + list(res_find.get('ipasudorunas_user', [])) + + list(res_find.get('ipasudorunasextuser', [])) ) ) runasuser_group_add, runasuser_group_del = ( gen_add_del_lists( - runasuser_group, + entry.runasuser_group, res_find.get('ipasudorunas_group', []) ) ) @@ -581,82 +770,81 @@ def main(): # the provided list against both groups and external # groups list. runasgroup_add, runasgroup_del = gen_add_del_lists( - runasgroup, + entry.runasgroup, ( - res_find.get('ipasudorunasgroup_group', []) - + res_find.get('ipasudorunasextgroup', []) + list(res_find.get('ipasudorunasgroup_group', [])) + + list(res_find.get('ipasudorunasextgroup', [])) ) ) elif action == "member": - if res_find is None: - ansible_module.fail_json(msg="No sudorule '%s'" % name) - # Generate add lists for host, hostgroup, user, group, # allow_sudocmd, allow_sudocmdgroup, deny_sudocmd, # deny_sudocmdgroup, sudooption, runasuser, runasgroup # and res_find to only try to add the items that not in # the sudorule already - if host is not None: + if entry.host is not None: host_add = gen_add_list( - host, res_find.get("memberhost_host")) - if hostgroup is not None: + entry.host, res_find.get("memberhost_host")) + if entry.hostgroup is not None: hostgroup_add = gen_add_list( - hostgroup, res_find.get("memberhost_hostgroup")) - if hostmask is not None: + entry.hostgroup, + res_find.get("memberhost_hostgroup") + ) + if entry.hostmask is not None: hostmask_add = gen_add_list( - hostmask, res_find.get("hostmask")) - if user is not None: + entry.hostmask, res_find.get("hostmask")) + if entry.user is not None: user_add = gen_add_list( - user, res_find.get("memberuser_user")) - if group is not None: + entry.user, res_find.get("memberuser_user")) + if entry.group is not None: group_add = gen_add_list( - group, res_find.get("memberuser_group")) - if allow_sudocmd is not None: + entry.group, res_find.get("memberuser_group")) + if entry.allow_sudocmd is not None: allow_cmd_add = gen_add_list( - allow_sudocmd, + entry.allow_sudocmd, res_find.get("memberallowcmd_sudocmd") ) - if allow_sudocmdgroup is not None: + if entry.allow_sudocmdgroup is not None: allow_cmdgroup_add = gen_add_list( - allow_sudocmdgroup, + entry.allow_sudocmdgroup, res_find.get("memberallowcmd_sudocmdgroup") ) - if deny_sudocmd is not None: + if entry.deny_sudocmd is not None: deny_cmd_add = gen_add_list( - deny_sudocmd, + entry.deny_sudocmd, res_find.get("memberdenycmd_sudocmd") ) - if deny_sudocmdgroup is not None: + if entry.deny_sudocmdgroup is not None: deny_cmdgroup_add = gen_add_list( - deny_sudocmdgroup, + entry.deny_sudocmdgroup, res_find.get("memberdenycmd_sudocmdgroup") ) - if sudooption is not None: + if entry.sudooption is not None: sudooption_add = gen_add_list( - sudooption, res_find.get("ipasudoopt")) + entry.sudooption, res_find.get("ipasudoopt")) # runasuser attribute can be used with both IPA and # non-IPA (external) users, so we need to compare # the provided list against both users and external # users list. - if runasuser is not None: + if entry.runasuser is not None: runasuser_add = gen_add_list( - runasuser, + entry.runasuser, (list(res_find.get('ipasudorunas_user', [])) + list(res_find.get('ipasudorunasextuser', []))) ) - if runasuser_group is not None: + if entry.runasuser_group is not None: runasuser_group_add = gen_add_list( - runasuser_group, + entry.runasuser_group, res_find.get('ipasudorunas_group', []) ) # runasgroup attribute can be used with both IPA and # non-IPA (external) groups, so we need to compare # the provided list against both users and external # groups list. - if runasgroup is not None: + if entry.runasgroup is not None: runasgroup_add = gen_add_list( - runasgroup, + entry.runasgroup, (list(res_find.get("ipasudorunasgroup_group", [])) + list(res_find.get("ipasudorunasextgroup", []))) ) @@ -664,84 +852,83 @@ def main(): elif state == "absent": if action == "sudorule": if res_find is not None: - commands.append([name, "sudorule_del", {}]) + commands.append([entry.name, "sudorule_del", {}]) elif action == "member": - if res_find is None: - ansible_module.fail_json(msg="No sudorule '%s'" % name) - # Generate intersection lists for host, hostgroup, user, # group, allow_sudocmd, allow_sudocmdgroup, deny_sudocmd # deny_sudocmdgroup, sudooption, runasuser, runasgroup # and res_find to only try to remove the items that are # in sudorule - if host is not None: + if entry.host is not None: host_del = gen_intersection_list( - host, res_find.get("memberhost_host")) + entry.host, res_find.get("memberhost_host")) - if hostgroup is not None: + if entry.hostgroup is not None: hostgroup_del = gen_intersection_list( - hostgroup, res_find.get("memberhost_hostgroup")) + entry.hostgroup, + res_find.get("memberhost_hostgroup") + ) - if hostmask is not None: + if entry.hostmask is not None: hostmask_del = gen_intersection_list( - hostmask, res_find.get("hostmask")) + entry.hostmask, res_find.get("hostmask")) - if user is not None: + if entry.user is not None: user_del = gen_intersection_list( - user, res_find.get("memberuser_user")) + entry.user, res_find.get("memberuser_user")) - if group is not None: + if entry.group is not None: group_del = gen_intersection_list( - group, res_find.get("memberuser_group")) + entry.group, res_find.get("memberuser_group")) - if allow_sudocmd is not None: + if entry.allow_sudocmd is not None: allow_cmd_del = gen_intersection_list( - allow_sudocmd, + entry.allow_sudocmd, res_find.get("memberallowcmd_sudocmd") ) - if allow_sudocmdgroup is not None: + if entry.allow_sudocmdgroup is not None: allow_cmdgroup_del = gen_intersection_list( - allow_sudocmdgroup, + entry.allow_sudocmdgroup, res_find.get("memberallowcmd_sudocmdgroup") ) - if deny_sudocmd is not None: + if entry.deny_sudocmd is not None: deny_cmd_del = gen_intersection_list( - deny_sudocmd, + entry.deny_sudocmd, res_find.get("memberdenycmd_sudocmd") ) - if deny_sudocmdgroup is not None: + if entry.deny_sudocmdgroup is not None: deny_cmdgroup_del = gen_intersection_list( - deny_sudocmdgroup, + entry.deny_sudocmdgroup, res_find.get("memberdenycmd_sudocmdgroup") ) - if sudooption is not None: + if entry.sudooption is not None: sudooption_del = gen_intersection_list( - sudooption, res_find.get("ipasudoopt")) + entry.sudooption, res_find.get("ipasudoopt")) # runasuser attribute can be used with both IPA and # non-IPA (external) users, so we need to compare # the provided list against both users and external # users list. - if runasuser is not None: + if entry.runasuser is not None: runasuser_del = gen_intersection_list( - runasuser, + entry.runasuser, ( list(res_find.get('ipasudorunas_user', [])) + list(res_find.get('ipasudorunasextuser', [])) ) ) - if runasuser_group is not None: + if entry.runasuser_group is not None: runasuser_group_del = gen_intersection_list( - runasuser_group, + entry.runasuser_group, res_find.get('ipasudorunas_group', []) ) # runasgroup attribute can be used with both IPA and # non-IPA (external) groups, so we need to compare # the provided list against both groups and external # groups list. - if runasgroup is not None: + if entry.runasgroup is not None: runasgroup_del = gen_intersection_list( - runasgroup, + entry.runasgroup, ( list(res_find.get( "ipasudorunasgroup_group", [])) @@ -751,8 +938,6 @@ def main(): ) elif state == "enabled": - if res_find is None: - ansible_module.fail_json(msg="No sudorule '%s'" % name) # sudorule_enable is not failing on an enabled sudorule # Therefore it is needed to have a look at the ipaenabledflag # in res_find. @@ -762,11 +947,9 @@ def main(): # See: https://github.com/freeipa/freeipa/pull/6294 enabled_flag = str(res_find.get("ipaenabledflag", [False])[0]) if enabled_flag.upper() != "TRUE": - commands.append([name, "sudorule_enable", {}]) + commands.append([entry.name, "sudorule_enable", {}]) elif state == "disabled": - if res_find is None: - ansible_module.fail_json(msg="No sudorule '%s'" % name) # sudorule_disable is not failing on an disabled sudorule # Therefore it is needed to have a look at the ipaenabledflag # in res_find. @@ -776,7 +959,7 @@ def main(): # See: https://github.com/freeipa/freeipa/pull/6294 enabled_flag = str(res_find.get("ipaenabledflag", [False])[0]) if enabled_flag.upper() != "FALSE": - commands.append([name, "sudorule_disable", {}]) + commands.append([entry.name, "sudorule_disable", {}]) else: ansible_module.fail_json(msg="Unkown state '%s'" % state) @@ -788,31 +971,31 @@ def main(): # An empty Hostmask cannot be used, or IPA API will fail. if hostmask_add: params["hostmask"] = hostmask_add - commands.append([name, "sudorule_add_host", params]) + commands.append([entry.name, "sudorule_add_host", params]) if any([host_del, hostgroup_del, hostmask_del]): params = {"host": host_del, "hostgroup": hostgroup_del} # An empty Hostmask cannot be used, or IPA API will fail. if hostmask_del: params["hostmask"] = hostmask_del - commands.append([name, "sudorule_remove_host", params]) + commands.append([entry.name, "sudorule_remove_host", params]) # Manage users and groups if user_add or group_add: commands.append([ - name, "sudorule_add_user", + entry.name, "sudorule_add_user", {"user": user_add, "group": group_add} ]) if user_del or group_del: commands.append([ - name, "sudorule_remove_user", + entry.name, "sudorule_remove_user", {"user": user_del, "group": group_del} ]) # Manage commands allowed if allow_cmd_add or allow_cmdgroup_add: commands.append([ - name, "sudorule_add_allow_command", + entry.name, "sudorule_add_allow_command", { "sudocmd": allow_cmd_add, "sudocmdgroup": allow_cmdgroup_add, @@ -820,7 +1003,7 @@ def main(): ]) if allow_cmd_del or allow_cmdgroup_del: commands.append([ - name, "sudorule_remove_allow_command", + entry.name, "sudorule_remove_allow_command", { "sudocmd": allow_cmd_del, "sudocmdgroup": allow_cmdgroup_del @@ -829,7 +1012,7 @@ def main(): # Manage commands denied if deny_cmd_add or deny_cmdgroup_add: commands.append([ - name, "sudorule_add_deny_command", + entry.name, "sudorule_add_deny_command", { "sudocmd": deny_cmd_add, "sudocmdgroup": deny_cmdgroup_add, @@ -837,7 +1020,7 @@ def main(): ]) if deny_cmd_del or deny_cmdgroup_del: commands.append([ - name, "sudorule_remove_deny_command", + entry.name, "sudorule_remove_deny_command", { "sudocmd": deny_cmd_del, "sudocmdgroup": deny_cmdgroup_del @@ -851,10 +1034,10 @@ def main(): _args["user"] = runasuser_add if runasuser_group_add: _args["group"] = runasuser_group_add - commands.append([name, "sudorule_add_runasuser", _args]) + commands.append([entry.name, "sudorule_add_runasuser", _args]) if runasuser_del or runasuser_group_del: commands.append([ - name, + entry.name, "sudorule_remove_runasuser", {"user": runasuser_del, "group": runasuser_group_del} ]) @@ -862,29 +1045,32 @@ def main(): # Manage RunAS Groups if runasgroup_add: commands.append([ - name, "sudorule_add_runasgroup", {"group": runasgroup_add} + entry.name, "sudorule_add_runasgroup", + {"group": runasgroup_add} ]) if runasgroup_del: commands.append([ - name, "sudorule_remove_runasgroup", + entry.name, "sudorule_remove_runasgroup", {"group": runasgroup_del} ]) # Manage sudo options if sudooption_add: for option in sudooption_add: commands.append([ - name, "sudorule_add_option", {"ipasudoopt": option} + entry.name, "sudorule_add_option", + {"ipasudoopt": option} ]) if sudooption_del: for option in sudooption_del: commands.append([ - name, "sudorule_remove_option", {"ipasudoopt": option} + entry.name, "sudorule_remove_option", + {"ipasudoopt": option} ]) # Execute commands changed = ansible_module.execute_ipa_commands( - commands, fail_on_member_errors=True) + commands, batch=True, fail_on_member_errors=True) # Done diff --git a/tests/sudorule/test_sudorule_client_context.yml b/tests/sudorule/test_sudorule_client_context.yml index 9df585cb37..331f138be4 100644 --- a/tests/sudorule/test_sudorule_client_context.yml +++ b/tests/sudorule/test_sudorule_client_context.yml @@ -37,3 +37,15 @@ when: groups['ipaclients'] is not defined or not groups['ipaclients'] vars: ipa_context: client + +- name: Test sudorule using client context, in client host. + ansible.builtin.import_playbook: test_sudorules.yml + when: groups['ipaclients'] + vars: + ipa_test_host: ipaclients + +- name: Test sudorule using client context, in server host. + ansible.builtin.import_playbook: test_sudorules.yml + when: groups['ipaclients'] is not defined or not groups['ipaclients'] + vars: + ipa_context: client diff --git a/tests/sudorule/test_sudorules.yml b/tests/sudorule/test_sudorules.yml new file mode 100644 index 0000000000..745926897b --- /dev/null +++ b/tests/sudorule/test_sudorules.yml @@ -0,0 +1,382 @@ +--- +- name: Test sudorule + hosts: "{{ ipa_test_host | default('ipaserver') }}" + become: false + gather_facts: true # required for ansible_facts['fqdn'] + + module_defaults: + ipauser: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + ipagroup: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + ipahostgroup: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + ipasudocmdgroup: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + ipasudocmd: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + ipasudorule: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + + tasks: + + # setup + - name: Ensure ansible facts for DNS are available + ansible.builtin.setup: + gather_subset: dns + + - name: Ensure test users are absent + ipauser: + name: + - user01 + - user02 + state: absent + + - name: Ensure test groups are absent + ipagroup: + name: + - group01 + - group02 + state: absent + + - name: Ensure test hostgroup is absent + ipahostgroup: + name: cluster + state: absent + + - name: Ensure test users are present + ipauser: + users: + - name: user01 + first: user + last: zeroone + - name: user02 + first: user + last: zerotwo + + - name: Ensure groups are present + ipagroup: + groups: + - name: group01 + user: user01 + - name: group02 + + - name: Ensure sudocmdgroup is absent + ipasudocmdgroup: + name: test_sudorule_cmdgroup + state: absent + + - name: Ensure hostgroup is present, with a host. + ipahostgroup: + name: cluster + host: "{{ ansible_facts['fqdn'] }}" + + - name: Ensure some sudocmds are available + ipasudocmd: + name: + - /sbin/ifconfig + - /usr/bin/vim + - /usr/bin/emacs + state: present + + - name: Ensure sudocmdgroup is available + ipasudocmdgroup: + name: test_sudorule_cmdgroup + sudocmd: /usr/bin/vim + state: present + + - name: Ensure another sudocmdgroup is available + ipasudocmdgroup: + name: test_sudorule_cmdgroup_2 + sudocmd: /usr/bin/emacs + state: present + + - name: Ensure sudorules are absent + ipasudorule: + name: + - testrule1 + - testrule2 + - allusers + - allhosts + - allcommands + state: absent + + # tests + - name: Run sudorules tests. + block: + - name: Ensure sudorules are present + ipasudorule: + sudorules: + - name: testrule1 + - name: testrule2 + - name: allhosts + - name: allcommands + register: result + failed_when: not result.changed or result.failed + + - name: Ensure sudorules are present, again + ipasudorule: + sudorules: + - name: testrule1 + - name: testrule2 + - name: allhosts + - name: allcommands + register: result + failed_when: result.changed or result.failed + + - name: Ensure testrule1 and testrule2 are absent + ipasudorule: + sudorules: + - name: testrule1 + - name: testrule2 + state: absent + register: result + failed_when: not result.changed or result.failed + + - name: Ensure testrule1 and testrule2 are absent, again + ipasudorule: + sudorules: + - name: testrule1 + - name: testrule2 + state: absent + register: result + failed_when: result.changed or result.failed + + - name: Ensure allhosts and allcommands sudorules are still present + ipasudorule: + sudorules: + - name: allhosts + - name: allcomands + state: absent + check_mode: true + register: result + failed_when: not result.changed or result.failed + + - name: Ensure sudorules with parameters are present + ipasudorule: + sudorules: + - name: testrule1 + runasuser: + - user01 + - name: testrule2 + runasuser_group: + - group01 + state: present + register: result + failed_when: not result.changed or result.failed + + - name: Ensure sudorules with parameters are present, again + ipasudorule: + sudorules: + - name: testrule1 + runasuser: + - user01 + - name: testrule2 + runasuser_group: + - group01 + state: present + register: result + failed_when: result.changed or result.failed + + - name: Ensure sudorules with parameters are modified + ipasudorule: + sudorules: + - name: testrule1 + runasuser: + - user02 + - name: testrule2 + runasuser_group: + - group02 + state: present + register: result + failed_when: not result.changed or result.failed + + - name: Ensure sudorules with parameters are modified again + ipasudorule: + sudorules: + - name: testrule1 + runasuser: + - user02 + - name: testrule2 + runasuser_group: + - group02 + state: present + register: result + failed_when: result.changed or result.failed + + - name: Ensure sudorules members can be modified + ipasudorule: + sudorules: + - name: testrule1 + runasuser: + - user01 + - name: testrule2 + runasuser_group: + - group01 + action: member + state: present + register: result + failed_when: not result.changed or result.failed + + - name: Ensure sudorules members can modified, again + ipasudorule: + sudorules: + - name: testrule1 + runasuser: + - user01 + - user02 + - name: testrule2 + runasuser_group: + - group01 + - group02 + action: member + state: present + register: result + failed_when: result.changed or result.failed + + - name: Ensure sudorules members are absent + ipasudorule: + sudorules: + - name: testrule1 + runasuser: + - user01 + - name: testrule2 + runasuser_group: + - group02 + action: member + state: absent + register: result + failed_when: not result.changed or result.failed + + - name: Ensure sudorules members are absent, again + ipasudorule: + sudorules: + - name: testrule1 + runasuser: + - user01 + - name: testrule2 + runasuser_group: + - group02 + action: member + state: absent + register: result + failed_when: result.changed or result.failed + + - name: Ensure testrule1 and testrule2 are present, with proper attributes + ipasudorule: + sudorules: + - name: testrule1 + runasuser: + - user02 + - name: testrule2 + runasuser_group: + - group01 + state: present + register: result + failed_when: result.changed or result.failed + + - name: Ensure testrule1 and testrule2 are disabled + ipasudorule: + sudorules: + - name: testrule1 + - name: testrule2 + state: disabled + register: result + failed_when: not result.changed or result.failed + + - name: Ensure testrule1 and testrule2 are disabled, again + ipasudorule: + sudorules: + - name: testrule1 + - name: testrule2 + state: disabled + register: result + failed_when: result.changed or result.failed + + - name: Ensure testrule1 and testrule2 are enabled + ipasudorule: + sudorules: + - name: testrule1 + - name: testrule2 + state: enabled + register: result + failed_when: not result.changed or result.failed + + - name: Ensure testrule1 and testrule2 are enabled, again + ipasudorule: + sudorules: + - name: testrule1 + - name: testrule2 + state: enabled + register: result + failed_when: result.changed or result.failed + + - name: Ensure multiple sudorules cannot be enabled with invalid parameters + ipasudorule: + sudorules: + - name: testrule1 + runasuser: user01 + - name: testrule2 + runasuser: user01 + state: enabled + register: result + failed_when: not result.failed and "Argument 'runasuser' can not be used with action 'sudorule' and state 'enabled'" not in result.msg + + - name: Ensure multiple sudorules cannot be disabled with invalid parameters + ipasudorule: + sudorules: + - name: testrule1 + runasuser: user01 + - name: testrule2 + runasuser: user01 + state: disabled + register: result + failed_when: not result.failed and "Argument 'runasuser' can not be used with action 'sudorule' and state 'disabled'" not in result.msg + + # cleanup + always: + - name: Cleanup sudorules + ipasudorule: + name: + - testrule1 + - testrule2 + - allusers + - allhosts + - allcommands + state: absent + + - name: Ensure sudocmdgroup is absent + ipasudocmdgroup: + name: + - test_sudorule_cmdgroup + - test_sudorule_cmdgroup_2 + state: absent + + - name: Ensure sudocmds are absent + ipasudocmd: + name: + - /sbin/ifconfig + - /usr/bin/vim + - /usr/bin/emacs + state: absent + + - name: Ensure hostgroup is absent. + ipahostgroup: + name: cluster + state: absent + + - name: Ensure groups are absent + ipagroup: + name: group01,group02 + state: absent + + - name: Ensure user is absent + ipauser: + name: user01,user02 + state: absent diff --git a/tests/sudorule/test_sudorules_member_case_insensitive.yml b/tests/sudorule/test_sudorules_member_case_insensitive.yml new file mode 100644 index 0000000000..d926c718ea --- /dev/null +++ b/tests/sudorule/test_sudorules_member_case_insensitive.yml @@ -0,0 +1,311 @@ +--- +- name: Test sudorules members should be case insensitive. + hosts: "{{ ipa_test_host | default('ipaserver') }}" + become: false + gather_facts: false + + module_defaults: + ipauser: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + ipagroup: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + ipahost: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + ipahostgroup: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + ipasudocmdgroup: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + ipasudocmd: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + ipasudorule: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + + vars: + groups_present: + - eleMENT1 + - Element2 + - eLeMenT3 + - ElemENT4 + + tasks: + - name: Test sudorule member case insensitive + block: + # SETUP + - name: Ensure domain name + ansible.builtin.set_fact: + ipa_domain: ipa.test + when: ipa_domain is not defined + + - name: Ensure test groups are absent. + ipagroup: + name: "{{ groups_present }}" + state: absent + + - name: Ensure test hostgroups are absent. + ipahostgroup: + name: "{{ groups_present }}" + state: absent + + - name: Ensure test users are absent. + ipauser: + name: "{{ groups_present }}" + state: absent + + - name: Ensure test groups exist. + ipagroup: + name: "{{ item }}" + loop: "{{ groups_present }}" + + - name: Ensure test hostgroups exist. + ipahostgroup: + name: "{{ item }}" + loop: "{{ groups_present }}" + + - name: Ensure test hosts exist. + ipahost: + name: "{{ item }}.{{ ipa_domain }}" + force: yes + loop: "{{ groups_present }}" + + - name: Ensure test users exist. + ipauser: + name: "user{{ item }}" + first: "{{ item }}" + last: "{{ item }}" + loop: "{{ groups_present }}" + + - name: Ensure sudorule do not exist + ipasudorule: + sudorules: + - name: "{{ item }}" + state: absent + loop: "{{ groups_present }}" + + # TESTS + - name: Ensure sudorule exist with runasusers members + ipasudorule: + sudorules: + - name: "{{ item }}" + cmdcategory: all + runasuser: "user{{ item }}" + loop: "{{ groups_present }}" + register: result + failed_when: result.failed or not result.changed + + - name: Ensure sudorule exist with lowercase runasusers members + ipasudorule: + sudorules: + - name: "{{ item }}" + cmdcategory: all + runasuser: "user{{ item | lower }}" + loop: "{{ groups_present }}" + register: result + failed_when: result.failed or result.changed + + - name: Ensure sudorule exist with uppercase runasusers members + ipasudorule: + sudorules: + - name: "{{ item }}" + cmdcategory: all + runasuser: "user{{ item | upper }}" + loop: "{{ groups_present }}" + register: result + failed_when: result.failed or result.changed + + - name: Ensure sudorule exist with runasgroup members + ipasudorule: + sudorules: + - name: "{{ item }}" + cmdcategory: all + runasgroup: "{{ item }}" + loop: "{{ groups_present }}" + register: result + failed_when: result.failed or not result.changed + + - name: Ensure sudorule exist with lowercase runasgroup members + ipasudorule: + sudorules: + - name: "{{ item }}" + cmdcategory: all + runasgroup: "{{ item | lower }}" + loop: "{{ groups_present }}" + register: result + failed_when: result.failed or result.changed + + - name: Ensure sudorule exist with uppercase runasgroup members + ipasudorule: + sudorules: + - name: "{{ item }}" + cmdcategory: all + runasgroup: "{{ item | upper }}" + loop: "{{ groups_present }}" + register: result + failed_when: result.failed or result.changed + + - name: Ensure sudorule do not exist + ipasudorule: + sudorules: + - name: "{{ item }}" + state: absent + loop: "{{ groups_present }}" + + ##### + + - name: Ensure sudorule exist with members + ipasudorule: + sudorules: + - name: "{{ item }}" + cmdcategory: all + hostgroup: "{{ item }}" + host: "{{ item }}.{{ ipa_domain }}" + group: "{{ item }}" + user: "user{{ item }}" + loop: "{{ groups_present }}" + register: result + failed_when: result.failed or not result.changed + + - name: Ensure sudorule exist with members, lowercase + ipasudorule: + sudorules: + - name: "{{ item }}" + cmdcategory: all + hostgroup: "{{ item | lower }}" + host: "{{ item | lower }}.{{ ipa_domain }}" + group: "{{ item | lower }}" + user: "user{{ item | lower }}" + loop: "{{ groups_present }}" + register: result + failed_when: result.failed or result.changed + + - name: Ensure sudorule exist with members, uppercase + ipasudorule: + sudorules: + - name: "{{ item }}" + cmdcategory: all + hostgroup: "{{ item | upper }}" + host: "{{ item | upper }}.{{ ipa_domain }}" + group: "{{ item | upper }}" + user: "user{{ item | upper }}" + loop: "{{ groups_present }}" + register: result + failed_when: result.failed or result.changed + + - name: Ensure sudorule member is absent + ipasudorule: + sudorules: + - name: "{{ item }}" + hostgroup: "{{ item }}" + host: "{{ item }}.{{ ipa_domain }}" + group: "{{ item }}" + user: "user{{ item }}" + action: member + state: absent + loop: "{{ groups_present }}" + register: result + failed_when: result.failed or not result.changed + + - name: Ensure sudorule member is absent, lowercase + ipasudorule: + sudorules: + - name: "{{ item }}" + hostgroup: "{{ item | lower }}" + host: "{{ item | lower }}.{{ ipa_domain }}" + group: "{{ item | lower }}" + user: "user{{ item | lower }}" + action: member + state: absent + loop: "{{ groups_present }}" + register: result + failed_when: result.failed or result.changed + + - name: Ensure sudorule member is absent, upercase + ipasudorule: + sudorules: + - name: "{{ item }}" + hostgroup: "{{ item | upper }}" + host: "{{ item | upper }}.{{ ipa_domain }}" + group: "{{ item | upper }}" + user: "user{{ item | upper }}" + action: member + state: absent + loop: "{{ groups_present }}" + register: result + failed_when: result.failed or result.changed + + - name: Ensure sudorule member is present, upercase + ipasudorule: + sudorules: + - name: "{{ item }}" + hostgroup: "{{ item | upper }}" + host: "{{ item | upper }}.{{ ipa_domain }}" + group: "{{ item | upper }}" + user: "user{{ item | upper }}" + action: member + loop: "{{ groups_present }}" + register: result + failed_when: result.failed or not result.changed + + - name: Ensure sudorule member is present, lowercase + ipasudorule: + sudorules: + - name: "{{ item }}" + hostgroup: "{{ item | lower }}" + host: "{{ item | lower }}.{{ ipa_domain }}" + group: "{{ item | lower }}" + user: "user{{ item | lower }}" + action: member + loop: "{{ groups_present }}" + register: result + failed_when: result.failed or result.changed + + - name: Ensure sudorule member is present, mixed case + ipasudorule: + sudorules: + - name: "{{ item }}" + hostgroup: "{{ item }}" + host: "{{ item }}.{{ ipa_domain }}" + group: "{{ item }}" + user: "user{{ item }}" + action: member + loop: "{{ groups_present }}" + register: result + failed_when: result.failed or result.changed + + # cleanup + always: + - name: Ensure sudorule do not exist + ipasudorule: + name: "{{ item }}" + state: absent + loop: "{{ groups_present }}" + + - name: Ensure test groups do not exist. + ipagroup: + name: "{{ item }}" + state: absent + loop: "{{ groups_present }}" + + - name: Ensure test hostgroups do not exist. + ipahostgroup: + name: "{{ item }}" + state: absent + loop: "{{ groups_present }}" + + - name: Ensure test hosts do not exist. + ipahost: + name: "{{ item }}.{{ ipa_domain }}" + state: absent + loop: "{{ groups_present }}" + + - name: Ensure test users do not exist. + ipauser: + name: "user{{ item }}" + state: absent + loop: "{{ groups_present }}" From 6c94fe9bd59cc31f684b5b2744027e1071518e1a Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Tue, 5 Nov 2024 11:08:13 -0300 Subject: [PATCH 3/3] tests/sudorule: Don't become or gather_facts and use only true/false Unless there's a real need to use privileged access or to gather Ansible facts upfront, we should always set "become: false" and "gather_facts: false". In the case that only a few Ansible facts are required, 'ansible.builtin.setup' with 'gather_subset' should be used. As the YAML 1.2 standard dictates, boolean values should only use 'true' or 'false' values. This patch fixes these issues in the 'sudorule' test suite. --- tests/sudorule/test_sudorule.yml | 8 ++++++-- tests/sudorule/test_sudorule_categories.yml | 19 ++++++++++++++----- .../sudorule/test_sudorule_client_context.yml | 4 ++-- .../test_sudorule_member_case_insensitive.yml | 6 +++--- .../test_sudorule_single_hostnames.yml | 19 ++++++++++++------- tests/sudorule/test_sudorules.yml | 2 +- 6 files changed, 38 insertions(+), 20 deletions(-) diff --git a/tests/sudorule/test_sudorule.yml b/tests/sudorule/test_sudorule.yml index 476fb1d89b..25dd84cdd1 100644 --- a/tests/sudorule/test_sudorule.yml +++ b/tests/sudorule/test_sudorule.yml @@ -3,11 +3,15 @@ - name: Test sudorule hosts: "{{ ipa_test_host | default('ipaserver') }}" become: true - gather_facts: true + gather_facts: false tasks: # setup + - name: Ensure DNS Ansible facts are available + ansible.builtin.setup: + gather_subset: dns + - name: Ensure test user is present ipauser: ipaadmin_password: SomeADMINpassword @@ -1157,7 +1161,7 @@ hostmask: 192.168.120.0/24 action: member register: result - check_mode: yes + check_mode: true failed_when: not result.changed or result.failed - name: Ensure sudorule hostmask member is present diff --git a/tests/sudorule/test_sudorule_categories.yml b/tests/sudorule/test_sudorule_categories.yml index 95b94f1282..91b9dca0d9 100644 --- a/tests/sudorule/test_sudorule_categories.yml +++ b/tests/sudorule/test_sudorule_categories.yml @@ -1,13 +1,22 @@ --- - name: Test sudorule user category hosts: ipaserver - become: yes - gather_facts: yes + become: false + gather_facts: false tasks: - - name: Get Domain from the server name - ansible.builtin.set_fact: - ipaserver_domain: "{{ ansible_facts['fqdn'].split('.')[1:] | join('.') }}" + - name: Test sudorule single hostnames + block: + # setup test environment + - name: Ensure ipaserver_domain is set + when: ipaserver_domain is not defined + block: + - name: Retrieve host information + ansible.builtin.setup: + gather_subset: dns + - name: Get Domain from the server name + ansible.builtin.set_fact: + ipaserver_domain: "{{ ansible_facts['fqdn'].split('.')[1:] | join('.') }}" - name: Ensure sudorules are absent ipasudorule: diff --git a/tests/sudorule/test_sudorule_client_context.yml b/tests/sudorule/test_sudorule_client_context.yml index 331f138be4..faa111b931 100644 --- a/tests/sudorule/test_sudorule_client_context.yml +++ b/tests/sudorule/test_sudorule_client_context.yml @@ -1,8 +1,8 @@ --- - name: Test sudorule hosts: ipaclients, ipaserver - become: no - gather_facts: no + become: false + gather_facts: false tasks: - name: Include FreeIPA facts. diff --git a/tests/sudorule/test_sudorule_member_case_insensitive.yml b/tests/sudorule/test_sudorule_member_case_insensitive.yml index cc212406a4..21ffe3028e 100644 --- a/tests/sudorule/test_sudorule_member_case_insensitive.yml +++ b/tests/sudorule/test_sudorule_member_case_insensitive.yml @@ -1,8 +1,8 @@ --- - name: Test sudorule members should be case insensitive. hosts: "{{ ipa_test_host | default('ipaserver') }}" - become: no - gather_facts: no + become: false + gather_facts: false vars: groups_present: @@ -37,7 +37,7 @@ ipahost: ipaadmin_password: SomeADMINpassword name: "{{ item }}.{{ ipa_domain }}" - force: yes + force: true loop: "{{ groups_present }}" - name: Ensure test users exist. diff --git a/tests/sudorule/test_sudorule_single_hostnames.yml b/tests/sudorule/test_sudorule_single_hostnames.yml index cc6a781928..728aab1cc7 100644 --- a/tests/sudorule/test_sudorule_single_hostnames.yml +++ b/tests/sudorule/test_sudorule_single_hostnames.yml @@ -1,17 +1,22 @@ --- - name: Test sudorule with single hostnames. hosts: "{{ ipa_test_host | default('ipaserver') }}" - become: no - gather_facts: no + become: false + gather_facts: false tasks: - name: Test sudorule single hostnames block: # setup test environment - - name: Get Domain from the server name - ansible.builtin.set_fact: - ipaserver_domain: "{{ ansible_facts['fqdn'].split('.')[1:] | join('.') }}" + - name: Ensure ipaserver_domain is set when: ipaserver_domain is not defined + block: + - name: Retrieve host information + ansible.builtin.setup: + gather_subset: dns + - name: Get Domain from the server name + ansible.builtin.set_fact: + ipaserver_domain: "{{ ansible_facts['fqdn'].split('.')[1:] | join('.') }}" - name: Ensure test sudo rule is absent ipasudorule: @@ -24,9 +29,9 @@ ipaadmin_password: SomeADMINpassword hosts: - name: "host01.{{ ipaserver_domain }}" - force: yes + force: true - name: "host02.{{ ipaserver_domain }}" - force: yes + force: true # start tests - name: Ensure sudorule exist with host member using FQDN. diff --git a/tests/sudorule/test_sudorules.yml b/tests/sudorule/test_sudorules.yml index 745926897b..0df3066c89 100644 --- a/tests/sudorule/test_sudorules.yml +++ b/tests/sudorule/test_sudorules.yml @@ -2,7 +2,7 @@ - name: Test sudorule hosts: "{{ ipa_test_host | default('ipaserver') }}" become: false - gather_facts: true # required for ansible_facts['fqdn'] + gather_facts: false module_defaults: ipauser: