kernel_blacklist - revamped the module (#3329)

* 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 <felix@fontein.de>

* Update plugins/modules/system/kernel_blacklist.py

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update plugins/modules/system/kernel_blacklist.py

Co-authored-by: Felix Fontein <felix@fontein.de>

* 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 <felix@fontein.de>

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

plugins/modules/system/kernel_blacklist.py

@@ -1,6 +1,7 @@
 #!/usr/bin/python
 # -*- coding: utf-8 -*-
 
+# Copyright: (c) 2021, Alexei Znamensky (@russoz) <russoz@gmail.com>
 # Copyright: (c) 2013, Matthias Vogelgesang <matthias.vogelgesang@gmail.com>
 # 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):
+class Blacklist(StateModuleHelper):
-    def __init__(self, module, filename, checkmode):
+    output_params = ('name', 'state')
-        self.filename = filename
+    module = dict(
-        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(
         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,
+    def __init_module__(self):
-                name=module.params['name'], state=module.params['state'])
+        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']:
+    def state_absent(self):
-        filename = module.params['blacklist_file']
+        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():
+    def __quit_module__(self):
-        args['changed'] = True
+        if self.has_changed() and not self.module.check_mode:
-    else:
+            dummy, tmpfile = tempfile.mkstemp()
-        args['changed'] = False
+            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__':

tests/integration/targets/kernel_blacklist/aliases

@@ -0,0 +1 @@
+shippable/posix/group1

tests/integration/targets/kernel_blacklist/files/blacklist

@@ -0,0 +1,3 @@
+blacklist aaaa
+blacklist bbbb
+blacklist cccc

tests/integration/targets/kernel_blacklist/meta/main.yml

@@ -0,0 +1,2 @@
+dependencies:
+  - setup_remote_tmp_dir

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