From 6e0c62d3e281a8893072c28d6f9a6aeceb64a833 Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Mon, 28 Jul 2025 19:21:55 +0200 Subject: [PATCH] [PR #10417/44ca3661 backport][stable-11] sysrc: refactor (#10504) sysrc: refactor (#10417) * sysrc: refactor * sysrc: refactor changelog fragment * sysrc: forgot the os import * sysrc: update test to edit the correct file * sysrc: Added copyright info to the test conf file * sysrc: Added full copyright info to the test conf file * sysrc: Detect permission denied when using sysrc * sysrc: Fixed the permission check and 2.7 compatibility * sysrc: Fix typo of import * sysrc: Fix err.find check * sysrc: Add bugfixes changelog fragment * sysrc: Use `StateModuleHelper` * sysrc: updated imports * sysrc: remove re import and set errno.EACCES on the OSError * sysrc: format code properly * sysrc: fix Python 2.7 compatibility and set changed manually * sysrc: add missing name format check Also use `self.module.fail_json` through out * sysrc: Removed os import by accident * sysrc: updated per review, and the way the existing value is retrieved (cherry picked from commit 44ca36617306b228384d42f96d1498603b92bc81) Co-authored-by: David Lundgren --- changelogs/fragments/10417-sysrc-refactor.yml | 4 + plugins/modules/sysrc.py | 233 ++++++++---------- .../targets/sysrc/files/10394.conf | 7 + .../integration/targets/sysrc/tasks/main.yml | 69 +++++- 4 files changed, 177 insertions(+), 136 deletions(-) create mode 100644 changelogs/fragments/10417-sysrc-refactor.yml create mode 100644 tests/integration/targets/sysrc/files/10394.conf diff --git a/changelogs/fragments/10417-sysrc-refactor.yml b/changelogs/fragments/10417-sysrc-refactor.yml new file mode 100644 index 0000000000..b1b5db632b --- /dev/null +++ b/changelogs/fragments/10417-sysrc-refactor.yml @@ -0,0 +1,4 @@ +minor_changes: + - sysrc - adjustments to the code (https://github.com/ansible-collections/community.general/pull/10417). +bugfixes: + - sysrc - fixes parsing with multi-line variables (https://github.com/ansible-collections/community.general/issues/10394, https://github.com/ansible-collections/community.general/pull/10417). \ No newline at end of file diff --git a/plugins/modules/sysrc.py b/plugins/modules/sysrc.py index dad379d1c0..937e41c1d8 100644 --- a/plugins/modules/sysrc.py +++ b/plugins/modules/sysrc.py @@ -7,6 +7,7 @@ # SPDX-License-Identifier: GPL-3.0-or-later from __future__ import absolute_import, division, print_function + __metaclass__ = type DOCUMENTATION = r""" @@ -102,157 +103,121 @@ changed: sample: true """ -from ansible.module_utils.basic import AnsibleModule +from ansible_collections.community.general.plugins.module_utils.module_helper import StateModuleHelper + +import os import re -class Sysrc(object): - def __init__(self, module, name, value, path, delim, jail): - self.module = module - self.name = name - self.changed = False - self.value = value - self.path = path - self.delim = delim - self.jail = jail - self.sysrc = module.get_bin_path('sysrc', True) - - def has_unknown_variable(self, out, err): - # newer versions of sysrc use stderr instead of stdout - return err.find("unknown variable") > 0 or out.find("unknown variable") > 0 - - def exists(self): - """ - Tests whether the name is in the file. If parameter value is defined, - then tests whether name=value is in the file. These tests are necessary - because sysrc doesn't use exit codes. Instead, let sysrc read the - file's content and create a dictionary comprising the configuration. - Use this dictionary to preform the tests. - """ - (rc, out, err) = self.run_sysrc('-e', '-a') - conf = dict([i.split('=', 1) for i in out.splitlines()]) - if self.value is None: - return self.name in conf - else: - return self.name in conf and conf[self.name] == '"%s"' % self.value - - def contains(self): - (rc, out, err) = self.run_sysrc('-n', self.name) - if self.has_unknown_variable(out, err): - return False - - return self.value in out.strip().split(self.delim) - - def present(self): - if self.exists(): - return - - if not self.module.check_mode: - (rc, out, err) = self.run_sysrc("%s=%s" % (self.name, self.value)) - - self.changed = True - - def absent(self): - if not self.exists(): - return - - # inversed since we still need to mark as changed - if not self.module.check_mode: - (rc, out, err) = self.run_sysrc('-x', self.name) - if self.has_unknown_variable(out, err): - return - - self.changed = True - - def value_present(self): - if self.contains(): - return - - if self.module.check_mode: - self.changed = True - return - - setstring = '%s+=%s%s' % (self.name, self.delim, self.value) - (rc, out, err) = self.run_sysrc(setstring) - if out.find("%s:" % self.name) == 0: - values = out.split(' -> ')[1].strip().split(self.delim) - if self.value in values: - self.changed = True - - def value_absent(self): - if not self.contains(): - return - - if self.module.check_mode: - self.changed = True - return - - setstring = '%s-=%s%s' % (self.name, self.delim, self.value) - (rc, out, err) = self.run_sysrc(setstring) - if out.find("%s:" % self.name) == 0: - values = out.split(' -> ')[1].strip().split(self.delim) - if self.value not in values: - self.changed = True - - def run_sysrc(self, *args): - cmd = [self.sysrc, '-f', self.path] - if self.jail: - cmd += ['-j', self.jail] - cmd.extend(args) - - (rc, out, err) = self.module.run_command(cmd) - - return (rc, out, err) - - -def main(): - module = AnsibleModule( +class Sysrc(StateModuleHelper): + module = dict( argument_spec=dict( name=dict(type='str', required=True), value=dict(type='str', default=None), state=dict(type='str', default='present', choices=['absent', 'present', 'value_present', 'value_absent']), path=dict(type='str', default='/etc/rc.conf'), delim=dict(type='str', default=' '), - jail=dict(type='str', default=None), + jail=dict(type='str', default=None) ), - supports_check_mode=True, + supports_check_mode=True ) + output_params = ('value',) + use_old_vardict = False - name = module.params.pop('name') - # OID style names are not supported - if not re.match('^[a-zA-Z0-9_]+$', name): - module.fail_json( - msg="Name may only contain alphanumeric and underscore characters" - ) + def __init_module__(self): + # OID style names are not supported + if not re.match(r'^\w+$', self.vars.name, re.ASCII): + self.module.fail_json(msg="Name may only contain alpha-numeric and underscore characters") - value = module.params.pop('value') - state = module.params.pop('state') - path = module.params.pop('path') - delim = module.params.pop('delim') - jail = module.params.pop('jail') - result = dict( - name=name, - state=state, - value=value, - path=path, - delim=delim, - jail=jail - ) + self.sysrc = self.module.get_bin_path('sysrc', True) - rc_value = Sysrc(module, name, value, path, delim, jail) + def _contains(self): + value = self._get() + if value is None: + return False, None - if state == 'present': - rc_value.present() - elif state == 'absent': - rc_value.absent() - elif state == 'value_present': - rc_value.value_present() - elif state == 'value_absent': - rc_value.value_absent() + value = value.split(self.vars.delim) - result['changed'] = rc_value.changed + return self.vars.value in value, value - module.exit_json(**result) + def _get(self): + if not os.path.exists(self.vars.path): + return None + + (rc, out, err) = self._sysrc('-v', '-n', self.vars.name) + if "unknown variable" in err or "unknown variable" in out: + # Prior to FreeBSD 11.1 sysrc would write "unknown variable" to stdout and not stderr + # https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=229806 + return None + + if out.startswith(self.vars.path): + return out.split(':', 1)[1].strip() + + return None + + def _modify(self, op, changed): + (rc, out, err) = self._sysrc("%s%s=%s%s" % (self.vars.name, op, self.vars.delim, self.vars.value)) + if out.startswith("%s:" % self.vars.name): + return changed(out.split(' -> ')[1].strip().split(self.vars.delim)) + + return False + + def _sysrc(self, *args): + cmd = [self.sysrc, '-f', self.vars.path] + if self.vars.jail: + cmd += ['-j', self.vars.jail] + cmd.extend(args) + + (rc, out, err) = self.module.run_command(cmd) + if "Permission denied" in err: + self.module.fail_json(msg="Permission denied for %s" % self.vars.path) + + return rc, out, err + + def state_absent(self): + if self._get() is None: + return + + if not self.check_mode: + self._sysrc('-x', self.vars.name) + + self.changed = True + + def state_present(self): + value = self._get() + if value == self.vars.value: + return + + if self.vars.value is None: + self.vars.set('value', value) + return + + if not self.check_mode: + self._sysrc("%s=%s" % (self.vars.name, self.vars.value)) + + self.changed = True + + def state_value_absent(self): + (contains, _unused) = self._contains() + if not contains: + return + + self.changed = self.check_mode or self._modify('-', lambda values: self.vars.value not in values) + + def state_value_present(self): + (contains, value) = self._contains() + if contains: + return + + if self.vars.value is None: + self.vars.set('value', value) + return + + self.changed = self.check_mode or self._modify('+', lambda values: self.vars.value in values) + + +def main(): + Sysrc.execute() if __name__ == '__main__': diff --git a/tests/integration/targets/sysrc/files/10394.conf b/tests/integration/targets/sysrc/files/10394.conf new file mode 100644 index 0000000000..fe0bc5b145 --- /dev/null +++ b/tests/integration/targets/sysrc/files/10394.conf @@ -0,0 +1,7 @@ +# Copyright (c) Ansible Project +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later +k1="v1" +jail_list=" + foo + bar" \ No newline at end of file diff --git a/tests/integration/targets/sysrc/tasks/main.yml b/tests/integration/targets/sysrc/tasks/main.yml index 549a1b8387..f1135d488f 100644 --- a/tests/integration/targets/sysrc/tasks/main.yml +++ b/tests/integration/targets/sysrc/tasks/main.yml @@ -369,14 +369,79 @@ - "value_2 == sysrc_equals_sign_2.value" - "value_2 == conf.spamd_flags" + ## + ## sysrc - #10004 state=absent when using default settings will report `changed=true` + ## + - name: Test that a key from /etc/defaults/rc.conf is not used to mark changed + sysrc: + name: dumpdev + state: absent + path: /tmp/10004.conf + register: sysrc_10004_absent + failed_when: sysrc_10004_absent.changed + + - name: Test that a delimited key from /etc/defaults/rc.conf is not used to mark changed + sysrc: + name: rc_conf_files + state: value_absent + path: /tmp/10004.conf + register: sysrc_10004_value_absent + failed_when: sysrc_10004_value_absent.changed + + - name: Test that a key from /etc/defaults/rc.conf is not used to mark changed without a path + sysrc: + name: static_routes + state: absent + register: sysrc_absent_default + failed_when: sysrc_absent_default.changed + + ## + ## sysrc - #10394 Ensure that files with multi-line values work + ## + - name: Copy 10394.conf + copy: + src: 10394.conf + dest: /tmp/10394.conf + + - name: Change value for k1 + sysrc: + name: k1 + value: v2 + path: /tmp/10394.conf + register: sysrc_10394_changed + + - name: Get file content + shell: "cat /tmp/10394.conf" + register: sysrc_10394_content + + - name: Ensure sysrc changed k1 from v1 to v2 + assert: + that: + - sysrc_10394_changed.changed + - > + 'k1="v2"' in sysrc_10394_content.stdout_lines + + ## + ## sysrc - additional tests + ## + - name: Ensure failure on OID style name since sysrc does not support them + sysrc: + name: not.valid.var + value: test + register: sysrc_name_check + failed_when: + - sysrc_name_check is not failed + - > + 'Name may only contain alpha-numeric and underscore characters' != sysrc_name_check.msg + always: - name: Restore /etc/rc.conf copy: - content: "{{ cached_etc_rcconf_content }}" + content: "{{ cached_etc_rcconf_content.stdout }}" dest: /etc/rc.conf - name: Restore /boot/loader.conf copy: - content: "{{ cached_boot_loaderconf_content }}" + content: "{{ cached_boot_loaderconf_content.stdout }}" dest: /boot/loader.conf