Merge lp:~shaner/charm-helpers/lp1712203 into lp:charm-helpers

[Shane Peters]

[Alex Kavanagh (ajkavanagh)]

Added improvement comment to the code. Please see inline.

[lp:~shaner/charm-helpers/lp1712203]

786. By Shane Peters on 2017-08-23

[shaner,r=] Fixes parsing of currently loaded apache modules

When hardening apache, the returned list of currently
enabled modules is always empty.

This change splits on linefeeds from the output of
'apachectl -M' instead of spaces which allows the
regex to match.

Since the returned list of modules could potentitialy
be used as input to '_disable_module' the regex was tweaked
to truncate the module name, removing the '_module' since
this is what a2dismod expects.

Closes-Bug: #1712203

[Shane Peters (shaner)]

Thanks Alex, testing this, it seems using splitlines() removes the need for 'strip()' as well.

[Alex Kavanagh (ajkavanagh)]

LGTM

review: Approve

=== modified file 'charmhelpers/contrib/hardening/audits/apache.py'
--- charmhelpers/contrib/hardening/audits/apache.py	2016-07-06 14:41:05 +0000
+++ charmhelpers/contrib/hardening/audits/apache.py	2017-08-23 15:36:26 +0000
@@ -70,12 +70,12 @@
         """Returns the modules which are enabled in Apache."""
         output = subprocess.check_output(['apache2ctl', '-M'])
         modules = []
-        for line in output.strip().split():
+        for line in output.splitlines():
             # Each line of the enabled module output looks like:
             #  module_name (static|shared)
             # Plus a header line at the top of the output which is stripped
             # out by the regex.
-            matcher = re.search(r'^ (\S*)', line)
+            matcher = re.search(r'^ (\S*)_module (\S*)', line)
             if matcher:
                 modules.append(matcher.group(1))
         return modules
 
=== modified file 'tests/contrib/hardening/audits/test_apache_audits.py'
--- tests/contrib/hardening/audits/test_apache_audits.py	2016-07-06 14:41:05 +0000
+++ tests/contrib/hardening/audits/test_apache_audits.py	2017-08-23 15:36:26 +0000
@@ -75,3 +75,12 @@
         audit.ensure_compliance()
         mock_disable_module.assert_has_calls([call('bar')])
         mock_restart_apache.assert_has_calls([call()])
+
+    @patch('subprocess.check_output')
+    def test_get_loaded_modules(self, mock_check_output):
+        mock_check_output.return_value = ('Loaded Modules:\n'
+                                          ' foo_module (static)\n'
+                                          ' bar_module (shared)\n')
+        audit = apache.DisabledModuleAudit('bar')
+        result = audit._get_loaded_modules()
+        self.assertEqual(['foo', 'bar'], result)