From 50abd7f46cca9bcae720a28a159f012824818f68 Mon Sep 17 00:00:00 2001 From: Moritz Bunkus Date: Tue, 5 Mar 2019 22:19:03 +0100 Subject: [PATCH 1/3] file state: fix comment function if both commented & uncommented exist When you had a file containing the pattern to comment in both commented and uncommented form, the old logic was wrong. In such a case it was not commenting the lines that could be commented and instead returned that the pattern was already commented. The new logic gives commenting precedence: if at least one line exists that can be commented, they all will be, no matter if other lines that have already been commented exist. Only if no lines to comment exist will a check be made to see if there's an already commented pattern or if the pattern doesn't exist at all. The new behavior matches the file.comment module function's behavior. This fixes #41818. --- salt/states/file.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/salt/states/file.py b/salt/states/file.py index 92d21c9d4b4c..f5c444707226 100644 --- a/salt/states/file.py +++ b/salt/states/file.py @@ -5392,15 +5392,9 @@ def comment(name, regex, char='#', backup='.bak'): comment_regex = char + unanchor_regex - # Check if the line is already commented - if __salt__['file.search'](name, comment_regex, multiline=True): - commented = True - else: - commented = False - # Make sure the pattern appears in the file before continuing - if commented or not __salt__['file.search'](name, regex, multiline=True): - if __salt__['file.search'](name, unanchor_regex, multiline=True): + if not __salt__['file.search'](name, regex, multiline=True): + if __salt__['file.search'](name, comment_regex, multiline=True): ret['comment'] = 'Pattern already commented' ret['result'] = True return ret From 04e324e4f42b8d67dc7c2370e1dde1c12fdfae4f Mon Sep 17 00:00:00 2001 From: Moritz Bunkus Date: Tue, 5 Mar 2019 22:26:34 +0100 Subject: [PATCH 2/3] file state: fix uncomment function if both commented & uncommented exist When you had a file containing the pattern to comment in both commented and uncommented form, the old logic was wrong. In such a case it was not uncommenting the lines that could be uncommented and instead returned that the pattern was already uncommented. The new logic gives uncommenting precedence: if at least one line exists that can be uncommented, they all will be, no matter if other lines that have already been commented exist. Only if no lines to uncomment exist will a check be made to see if there's an already uncommented pattern or if the pattern doesn't exist at all. The new behavior matches the file.uncomment module function's behavior. See #41818 for an analog problem in the file.comment state function. --- salt/states/file.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/salt/states/file.py b/salt/states/file.py index f5c444707226..bbe6cdf8ff7b 100644 --- a/salt/states/file.py +++ b/salt/states/file.py @@ -5492,18 +5492,18 @@ def uncomment(name, regex, char='#', backup='.bak'): # Make sure the pattern appears in the file if __salt__['file.search']( + name, + '{0}[ \t]*{1}'.format(char, regex.lstrip('^')), + multiline=True): + # Line exists and is commented + pass + elif __salt__['file.search']( name, '^[ \t]*{0}'.format(regex.lstrip('^')), multiline=True): ret['comment'] = 'Pattern already uncommented' ret['result'] = True return ret - elif __salt__['file.search']( - name, - '{0}[ \t]*{1}'.format(char, regex.lstrip('^')), - multiline=True): - # Line exists and is commented - pass else: return _error(ret, '{0}: Pattern not found'.format(regex)) From 9b7ce299f3365e3c0b7e25df958fbbaac6abd6ea Mon Sep 17 00:00:00 2001 From: Moritz Bunkus Date: Wed, 6 Mar 2019 13:52:54 +0100 Subject: [PATCH 3/3] tests: update for changes to file.(un)comment state functions --- tests/unit/states/test_file.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/unit/states/test_file.py b/tests/unit/states/test_file.py index 1d1312234f09..a9f93b5dfb60 100644 --- a/tests/unit/states/test_file.py +++ b/tests/unit/states/test_file.py @@ -1167,7 +1167,7 @@ def test_comment(self): with patch.object(os.path, 'isabs', mock_t): with patch.dict(filestate.__salt__, - {'file.search': MagicMock(side_effect=[True, True, True, False, False])}): + {'file.search': MagicMock(side_effect=[False, True, False, False])}): comt = ('Pattern already commented') ret.update({'comment': comt, 'result': True}) self.assertDictEqual(filestate.comment(name, regex), ret) @@ -1177,7 +1177,7 @@ def test_comment(self): self.assertDictEqual(filestate.comment(name, regex), ret) with patch.dict(filestate.__salt__, - {'file.search': MagicMock(side_effect=[False, True, False, True, True]), + {'file.search': MagicMock(side_effect=[True, True, True]), 'file.comment': mock_t, 'file.comment_line': mock_t}): with patch.dict(filestate.__opts__, {'test': True}): @@ -1215,7 +1215,9 @@ def test_uncomment(self): mock_t = MagicMock(return_value=True) mock_f = MagicMock(return_value=False) - mock = MagicMock(side_effect=[True, False, False, False, True, False, + mock = MagicMock(side_effect=[False, True, + False, False, + True, True, True]) with patch.object(os.path, 'isabs', mock_f): comt = ('Specified file {0} is not an absolute path'.format(name))