From d4637e9b1c3c5a1bfcf5e6be970b2b5c0f5db4fc Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Mon, 20 Sep 2021 19:58:05 +0200 Subject: [PATCH] kernel_blacklist - revamped the module (#3329) (#3409) * kernel_blacklist - revamped the module * file default and setting "lines" when file does not exist * added changelog fragment * added change in arg_spec to the documentation block * Update plugins/modules/system/kernel_blacklist.py Co-authored-by: Felix Fontein * Update plugins/modules/system/kernel_blacklist.py Co-authored-by: Felix Fontein * Update plugins/modules/system/kernel_blacklist.py Co-authored-by: Felix Fontein * fixed: initialize self.pattern before self.var.is_blacklisted * File writing recoded * added try/finally for the file * multiple changes: - fixed case when last line of the existing file has no newline char - added integration tests * PR: integration test now using remote_tmp_dir Co-authored-by: Felix Fontein (cherry picked from commit 2ad7ed4f83701b2919e287de0cd50b17884371de) Co-authored-by: Alexei Znamensky <103110+russoz@users.noreply.github.com> --- .../3329-kernel_blacklist-improvements.yaml | 2 + plugins/modules/system/kernel_blacklist.py | 141 +++++++----------- .../targets/kernel_blacklist/aliases | 1 + .../targets/kernel_blacklist/files/blacklist | 3 + .../targets/kernel_blacklist/meta/main.yml | 2 + .../targets/kernel_blacklist/tasks/main.yml | 90 +++++++++++ 6 files changed, 151 insertions(+), 88 deletions(-) create mode 100644 changelogs/fragments/3329-kernel_blacklist-improvements.yaml create mode 100644 tests/integration/targets/kernel_blacklist/aliases create mode 100644 tests/integration/targets/kernel_blacklist/files/blacklist create mode 100644 tests/integration/targets/kernel_blacklist/meta/main.yml create mode 100644 tests/integration/targets/kernel_blacklist/tasks/main.yml diff --git a/changelogs/fragments/3329-kernel_blacklist-improvements.yaml b/changelogs/fragments/3329-kernel_blacklist-improvements.yaml new file mode 100644 index 0000000000..2c1dd31da5 --- /dev/null +++ b/changelogs/fragments/3329-kernel_blacklist-improvements.yaml @@ -0,0 +1,2 @@ +minor_changes: + - kernel_blacklist - revamped the module using ``ModuleHelper`` (https://github.com/ansible-collections/community.general/pull/3329). diff --git a/plugins/modules/system/kernel_blacklist.py b/plugins/modules/system/kernel_blacklist.py index d8cb4a9e9d..ad0241b31a 100644 --- a/plugins/modules/system/kernel_blacklist.py +++ b/plugins/modules/system/kernel_blacklist.py @@ -1,6 +1,7 @@ #!/usr/bin/python # -*- coding: utf-8 -*- +# Copyright: (c) 2021, Alexei Znamensky (@russoz) # Copyright: (c) 2013, Matthias Vogelgesang # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) @@ -32,6 +33,7 @@ options: description: - If specified, use this blacklist file instead of C(/etc/modprobe.d/blacklist-ansible.conf). + default: /etc/modprobe.d/blacklist-ansible.conf ''' EXAMPLES = ''' @@ -43,110 +45,73 @@ EXAMPLES = ''' import os import re +import tempfile -from ansible.module_utils.basic import AnsibleModule +from ansible_collections.community.general.plugins.module_utils.module_helper import StateModuleHelper -class Blacklist(object): - def __init__(self, module, filename, checkmode): - self.filename = filename - self.module = module - self.checkmode = checkmode - - def create_file(self): - if not self.checkmode and not os.path.exists(self.filename): - open(self.filename, 'a').close() - return True - elif self.checkmode and not os.path.exists(self.filename): - self.filename = os.devnull - return True - else: - return False - - def get_pattern(self): - return r'^blacklist\s*' + self.module + '$' - - def readlines(self): - f = open(self.filename, 'r') - lines = f.readlines() - f.close() - return lines - - def module_listed(self): - lines = self.readlines() - pattern = self.get_pattern() - - for line in lines: - stripped = line.strip() - if stripped.startswith('#'): - continue - - if re.match(pattern, stripped): - return True - - return False - - def remove_module(self): - lines = self.readlines() - pattern = self.get_pattern() - - if self.checkmode: - f = open(os.devnull, 'w') - else: - f = open(self.filename, 'w') - - for line in lines: - if not re.match(pattern, line.strip()): - f.write(line) - - f.close() - - def add_module(self): - if self.checkmode: - f = open(os.devnull, 'a') - else: - f = open(self.filename, 'a') - - f.write('blacklist %s\n' % self.module) - - f.close() - - -def main(): - module = AnsibleModule( +class Blacklist(StateModuleHelper): + output_params = ('name', 'state') + module = dict( argument_spec=dict( name=dict(type='str', required=True), state=dict(type='str', default='present', choices=['absent', 'present']), - blacklist_file=dict(type='str') + blacklist_file=dict(type='str', default='/etc/modprobe.d/blacklist-ansible.conf'), ), supports_check_mode=True, ) - args = dict(changed=False, failed=False, - name=module.params['name'], state=module.params['state']) + def __init_module__(self): + self.pattern = re.compile(r'^blacklist\s+{0}$'.format(re.escape(self.vars.name))) + self.vars.filename = self.vars.blacklist_file + self.vars.set('file_exists', os.path.exists(self.vars.filename), output=False, change=True) + if not self.vars.file_exists: + with open(self.vars.filename, 'a'): + pass + self.vars.file_exists = True + self.vars.set('lines', [], change=True, diff=True) + else: + with open(self.vars.filename) as fd: + self.vars.set('lines', [x.rstrip() for x in fd.readlines()], change=True, diff=True) + self.vars.set('is_blacklisted', self._is_module_blocked(), change=True) - filename = '/etc/modprobe.d/blacklist-ansible.conf' + def _is_module_blocked(self): + for line in self.vars.lines: + stripped = line.strip() + if stripped.startswith('#'): + continue + if self.pattern.match(stripped): + return True + return False - if module.params['blacklist_file']: - filename = module.params['blacklist_file'] + def state_absent(self): + if not self.vars.is_blacklisted: + return + self.vars.is_blacklisted = False + self.vars.lines = [line for line in self.vars.lines if not self.pattern.match(line.strip())] - blacklist = Blacklist(args['name'], filename, module.check_mode) + def state_present(self): + if self.vars.is_blacklisted: + return + self.vars.is_blacklisted = True + self.vars.lines = self.vars.lines + ['blacklist %s' % self.vars.name] - if blacklist.create_file(): - args['changed'] = True - else: - args['changed'] = False + def __quit_module__(self): + if self.has_changed() and not self.module.check_mode: + dummy, tmpfile = tempfile.mkstemp() + try: + os.remove(tmpfile) + self.module.preserved_copy(self.vars.filename, tmpfile) # ensure right perms/ownership + with open(tmpfile, 'w') as fd: + fd.writelines(["{0}\n".format(x) for x in self.vars.lines]) + self.module.atomic_move(tmpfile, self.vars.filename) + finally: + if os.path.exists(tmpfile): + os.remove(tmpfile) - if blacklist.module_listed(): - if args['state'] == 'absent': - blacklist.remove_module() - args['changed'] = True - else: - if args['state'] == 'present': - blacklist.add_module() - args['changed'] = True - module.exit_json(**args) +def main(): + Blacklist.execute() if __name__ == '__main__': diff --git a/tests/integration/targets/kernel_blacklist/aliases b/tests/integration/targets/kernel_blacklist/aliases new file mode 100644 index 0000000000..a6dafcf8cd --- /dev/null +++ b/tests/integration/targets/kernel_blacklist/aliases @@ -0,0 +1 @@ +shippable/posix/group1 diff --git a/tests/integration/targets/kernel_blacklist/files/blacklist b/tests/integration/targets/kernel_blacklist/files/blacklist new file mode 100644 index 0000000000..4e815584eb --- /dev/null +++ b/tests/integration/targets/kernel_blacklist/files/blacklist @@ -0,0 +1,3 @@ +blacklist aaaa +blacklist bbbb +blacklist cccc \ No newline at end of file diff --git a/tests/integration/targets/kernel_blacklist/meta/main.yml b/tests/integration/targets/kernel_blacklist/meta/main.yml new file mode 100644 index 0000000000..1810d4bec9 --- /dev/null +++ b/tests/integration/targets/kernel_blacklist/meta/main.yml @@ -0,0 +1,2 @@ +dependencies: + - setup_remote_tmp_dir diff --git a/tests/integration/targets/kernel_blacklist/tasks/main.yml b/tests/integration/targets/kernel_blacklist/tasks/main.yml new file mode 100644 index 0000000000..de3d5d8edd --- /dev/null +++ b/tests/integration/targets/kernel_blacklist/tasks/main.yml @@ -0,0 +1,90 @@ +--- +- name: set destination filename + set_fact: + bl_file: '{{ remote_tmp_dir }}/blacklist-ansible.conf' + +- name: copy blacklist file + copy: + src: 'files/blacklist' + dest: '{{ bl_file }}' + +- name: Original stat + stat: + path: '{{ bl_file }}' + register: orig_stat + +- name: remove non-existing item from list + community.general.kernel_blacklist: + blacklist_file: '{{ bl_file }}' + state: absent + name: zzzz + register: bl_test_1 + +- name: add existing item from list + community.general.kernel_blacklist: + blacklist_file: '{{ bl_file }}' + state: present + name: bbbb + register: bl_test_1a + +- name: stat_test_1 + stat: + path: '{{ bl_file }}' + register: stat_test_1 + +- name: assert file is unchanged + assert: + that: + - bl_test_1 is not changed + - bl_test_1a is not changed + - orig_stat.stat.size == stat_test_1.stat.size + - orig_stat.stat.checksum == stat_test_1.stat.checksum + - orig_stat.stat.mtime == stat_test_1.stat.mtime + - stat_test_1.stat.checksum == 'blacklist aaaa\nblacklist bbbb\nblacklist cccc'|checksum + +- name: add new item to list + community.general.kernel_blacklist: + blacklist_file: '{{ bl_file }}' + state: present + name: dddd + register: bl_test_2 + +- name: slurp_test_2 + slurp: + src: '{{ bl_file }}' + register: slurp_test_2 + +- name: assert element is added + assert: + that: + - bl_test_2 is changed + - slurp_test_2.content|b64decode == content + vars: + content: | + blacklist aaaa + blacklist bbbb + blacklist cccc + blacklist dddd + +- name: remove item from list + community.general.kernel_blacklist: + blacklist_file: '{{ bl_file }}' + state: absent + name: bbbb + register: bl_test_3 + +- name: slurp_test_3 + slurp: + src: '{{ bl_file }}' + register: slurp_test_3 + +- name: assert element is added + assert: + that: + - bl_test_3 is changed + - slurp_test_3.content|b64decode == content + vars: + content: | + blacklist aaaa + blacklist cccc + blacklist dddd