Charm Helpers

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

  1. lp1712203
  2. Merge into devel
Proposed by Shane Peters
Status: Merged
Approved by: Alex Kavanagh
Approved revision: 786
Merged at revision: 788
Proposed branch: lp:~shaner/charm-helpers/lp1712203
Merge into: lp:charm-helpers
Diff against target: 35 lines (+11/-2)
2 files modified
charmhelpers/contrib/hardening/audits/apache.py (+2/-2)
tests/contrib/hardening/audits/test_apache_audits.py (+9/-0)
To merge this branch: bzr merge lp:~shaner/charm-helpers/lp1712203
Reviewer Review Type Date Requested Status
Alex Kavanagh Approve
Review via email: mp+329376@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Alex Kavanagh (ajkavanagh) wrote :

Added improvement comment to the code. Please see inline.

lp:~shaner/charm-helpers/lp1712203 updated
786. By Shane Peters

[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

Revision history for this message
Shane Peters (shaner) wrote :

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

Revision history for this message
Alex Kavanagh (ajkavanagh) wrote :

LGTM

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'charmhelpers/contrib/hardening/audits/apache.py'
2--- charmhelpers/contrib/hardening/audits/apache.py 2016-07-06 14:41:05 +0000
3+++ charmhelpers/contrib/hardening/audits/apache.py 2017-08-23 15:36:26 +0000
4@@ -70,12 +70,12 @@
5 """Returns the modules which are enabled in Apache."""
6 output = subprocess.check_output(['apache2ctl', '-M'])
7 modules = []
8- for line in output.strip().split():
9+ for line in output.splitlines():
10 # Each line of the enabled module output looks like:
11 # module_name (static|shared)
12 # Plus a header line at the top of the output which is stripped
13 # out by the regex.
14- matcher = re.search(r'^ (\S*)', line)
15+ matcher = re.search(r'^ (\S*)_module (\S*)', line)
16 if matcher:
17 modules.append(matcher.group(1))
18 return modules
19
20=== modified file 'tests/contrib/hardening/audits/test_apache_audits.py'
21--- tests/contrib/hardening/audits/test_apache_audits.py 2016-07-06 14:41:05 +0000
22+++ tests/contrib/hardening/audits/test_apache_audits.py 2017-08-23 15:36:26 +0000
23@@ -75,3 +75,12 @@
24 audit.ensure_compliance()
25 mock_disable_module.assert_has_calls([call('bar')])
26 mock_restart_apache.assert_has_calls([call()])
27+
28+ @patch('subprocess.check_output')
29+ def test_get_loaded_modules(self, mock_check_output):
30+ mock_check_output.return_value = ('Loaded Modules:\n'
31+ ' foo_module (static)\n'
32+ ' bar_module (shared)\n')
33+ audit = apache.DisabledModuleAudit('bar')
34+ result = audit._get_loaded_modules()
35+ self.assertEqual(['foo', 'bar'], result)

Subscribers

People subscribed via source and target branches