saltstack/salt
1
0
Fork 0
mirror of https://github.com/saltstack/salt.git synced 2025-04-17 10:10:20 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

Improve reliability/idempotence of file.blockreplace state (2018.3 branch)

Browse source 
This makes the following changes:

- The `append_newline` argument to the `file.blockreplace`
  remote-execution function has been modified so that if its value is
  `None`, it only appends a newline when the content block does not end
  in one.
- A couple of fixes were made to newline handling. The existing code
  normalized the newlines in the content block, replacing them with
  os.linesep. However, when the file contains newlines that don't match
  the OS (i.e. POSIX newlines in a file on a Windows box, or Windows
  newlines on a Linux/Mac/BSD/etc. box), then we would still end up with
  mixed newlines. The line separator is now detected when we read in the
  original file, and the detected line separator is used when writing
  the content block. Additionally, the same newline mismatch was
  possible when appending/prepending the content block. This has been
  fixed by using a common function for appending, prepending, and
  replacing the content block.
- Support for the `append_newline` argument has been added to the
  `file.blockreplace` state. The default value for the state is `None`.
  A `versionchanged` has been added to the remote execution function to
  let users know that the Fluorine release will change the default value
  of that variable.
- 20 new integration tests have been written to test the
  `file.blockreplace` state.
This commit is contained in:
Erik Johnson 2018-03-07 15:45:38 -06:00
parent 0c66507aff
commit 4798187035
No known key found for this signature in database
GPG key ID: 5E5583C437808F3F
5 changed files with 1473 additions and 160 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

209
salt/modules/file.py
View file

@ -2433,8 +2433,7 @@ def blockreplace(path,
backup='.bak',
dry_run=False,
show_changes=True,
append_newline=False,
):
append_newline=False):
'''
.. versionadded:: 2014.1.0
@ -2481,18 +2480,30 @@ def blockreplace(path,
The file extension to use for a backup of the file if any edit is made.
Set to ``False`` to skip making a backup.
dry_run
Don't make any edits to the file.
dry_run : False
If ``True``, do not make any edits to the file and simply return the
changes that *would* be made.
show_changes
Output a unified diff of the old file and the new file. If ``False``,
return a boolean if any changes were made.
show_changes : True
Controls how changes are presented. If ``True``, this function will
return a unified diff of the changes made. If False, then it will
return a boolean (``True`` if any changes were made, otherwise
``False``).
append_newline:
Append a newline to the content block. For more information see:
https://github.com/saltstack/salt/issues/33686
append_newline : False
Controls whether or not a newline is appended to the content block. If
the value of this argument is ``True`` then a newline will be added to
the content block. If it is ``False``, then a newline will *not* be
added to the content block. If it is ``None`` then a newline will only
be added to the content block if it does not already end in a newline.
.. versionadded:: 2016.3.4
.. versionchanged:: 2017.7.5,2018.3.1
New behavior added when value is ``None``.
.. versionchanged:: Fluorine
The default value of this argument will change to ``None`` to match
the behavior of the :py:func:`file.blockreplace state
<salt.states.file.blockreplace>`
CLI Example:
@ -2502,87 +2513,137 @@ def blockreplace(path,
'#-- end managed zone foobar --' $'10.0.1.1 foo.foobar\\n10.0.1.2 bar.foobar' True
'''
path = os.path.expanduser(path)
if not os.path.exists(path):
raise SaltInvocationError('File not found: {0}'.format(path))
if append_if_not_found and prepend_if_not_found:
raise SaltInvocationError(
'Only one of append and prepend_if_not_found is permitted'
)
path = os.path.expanduser(path)
if not os.path.exists(path):
raise SaltInvocationError('File not found: {0}'.format(path))
if not __utils__['files.is_text'](path):
raise SaltInvocationError(
'Cannot perform string replacements on a binary file: {0}'
.format(path)
)
# Search the file; track if any changes have been made for the return val
if append_newline is None and not content.endswith((os.linesep, '\n')):
append_newline = True
# Split the content into a list of lines, removing newline characters. To
# ensure that we handle both Windows and POSIX newlines, first split on
# Windows newlines, and then split on POSIX newlines.
split_content = []
for win_line in content.split('\r\n'):
for content_line in win_line.split('\n'):
split_content.append(content_line)
line_count = len(split_content)
has_changes = False
orig_file = []
new_file = []
in_block = False
old_content = ''
done = False
# we do not use in_place editing to avoid file attrs modifications when
block_found = False
linesep = None
def _add_content(linesep, lines=None, include_marker_start=True,
end_line=None):
if lines is None:
lines = []
include_marker_start = True
if end_line is None:
end_line = marker_end
end_line = end_line.rstrip('\r\n') + linesep
if include_marker_start:
lines.append(marker_start + linesep)
if split_content:
for index, content_line in enumerate(split_content, 1):
if index != line_count:
lines.append(content_line + linesep)
else:
# We're on the last line of the content block
if append_newline:
lines.append(content_line + linesep)
lines.append(end_line)
else:
lines.append(content_line + end_line)
else:
lines.append(end_line)
return lines
# We do not use in-place editing to avoid file attrs modifications when
# no changes are required and to avoid any file access on a partially
# written file.
# we could also use salt.utils.filebuffer.BufferedReader
#
# We could also use salt.utils.filebuffer.BufferedReader
try:
fi_file = fileinput.input(path,
inplace=False, backup=False,
bufsize=1, mode='rb')
for line in fi_file:
fi_file = fileinput.input(
path,
inplace=False,
backup=False,
bufsize=1,
mode='rb')
for line in fi_file:
line = salt.utils.stringutils.to_unicode(line)
result = line
write_line_to_new_file = True
if linesep is None:
# Auto-detect line separator
if line.endswith('\r\n'):
linesep = '\r\n'
elif line.endswith('\n'):
linesep = '\n'
else:
# No newline(s) in file, fall back to system's linesep
linesep = os.linesep
if marker_start in line:
# managed block start found, start recording
# We've entered the content block
in_block = True
else:
if in_block:
if marker_end in line:
# end of block detected
# We're not going to write the lines from the old file to
# the new file until we have exited the block.
write_line_to_new_file = False
marker_end_pos = line.find(marker_end)
if marker_end_pos != -1:
# End of block detected
in_block = False
# We've found and exited the block
block_found = True
# Handle situations where there may be multiple types
# of line endings in the same file. Separate the content
# into lines. Account for Windows-style line endings
# using os.linesep, then by linux-style line endings
# using '\n'
split_content = []
for linesep_line in content.split(os.linesep):
for content_line in linesep_line.split('\n'):
split_content.append(content_line)
# Trim any trailing new lines to avoid unwanted
# additional new lines
while not split_content[-1]:
split_content.pop()
# push new block content in file
for content_line in split_content:
new_file.append(content_line + os.linesep)
done = True
else:
# remove old content, but keep a trace
old_content += line
result = None
# else: we are not in the marked block, keep saving things
_add_content(linesep, lines=new_file,
include_marker_start=False,
end_line=line[marker_end_pos:])
# Save the line from the original file
orig_file.append(line)
if result is not None:
new_file.append(result)
# end for. If we are here without block management we maybe have some problems,
# or we need to initialise the marked block
if write_line_to_new_file:
new_file.append(line)
except (IOError, OSError) as exc:
raise CommandExecutionError(
'Failed to read from {0}: {1}'.format(path, exc)
)
finally:
fi_file.close()
if linesep is None:
# If the file was empty, we will not have set linesep yet. Assume
# the system's line separator. This is needed for when we
# prepend/append later on.
linesep = os.linesep
try:
fi_file.close()
except Exception:
pass
if in_block:
# unterminated block => bad, always fail
@ -2590,35 +2651,27 @@ def blockreplace(path,
'Unterminated marked block. End of file reached before marker_end.'
)
if not done:
if not block_found:
if prepend_if_not_found:
# add the markers and content at the beginning of file
new_file.insert(0, marker_end + os.linesep)
if append_newline is True:
new_file.insert(0, content + os.linesep)
else:
new_file.insert(0, content)
new_file.insert(0, marker_start + os.linesep)
done = True
prepended_content = _add_content(linesep)
prepended_content.extend(new_file)
new_file = prepended_content
block_found = True
elif append_if_not_found:
# Make sure we have a newline at the end of the file
if 0 != len(new_file):
if not new_file[-1].endswith(os.linesep):
new_file[-1] += os.linesep
if not new_file[-1].endswith(linesep):
new_file[-1] += linesep
# add the markers and content at the end of file
new_file.append(marker_start + os.linesep)
if append_newline is True:
new_file.append(content + os.linesep)
else:
new_file.append(content)
new_file.append(marker_end + os.linesep)
done = True
_add_content(linesep, lines=new_file)
block_found = True
else:
raise CommandExecutionError(
'Cannot edit marked block. Markers were not found in file.'
)
if done:
if block_found:
diff = ''.join(difflib.unified_diff(orig_file, new_file))
has_changes = diff is not ''
if has_changes and not dry_run:

98
salt/states/file.py
View file

@ -4143,11 +4143,20 @@ def blockreplace(
append_if_not_found=False,
prepend_if_not_found=False,
backup='.bak',
show_changes=True):
show_changes=True,
append_newline=None):
'''
Maintain an edit in a file in a zone delimited by two line markers
.. versionadded:: 2014.1.0
.. versionchanged:: 2017.7.5,2018.3.1
``append_newline`` argument added. Additionally, to improve
idempotence, if the string represented by ``marker_end`` is found in
the middle of the line, the content preceding the marker will be
removed when the block is replaced. This allows one to remove
``append_newline: False`` from the SLS and have the block properly
replaced if the end of the content block is immediately followed by the
``marker_end`` (i.e. no newline before the marker).
A block of content delimited by comments can help you manage several lines
entries without worrying about old entries removal. This can help you
@ -4232,41 +4241,54 @@ def blockreplace(
See the ``source_hash`` parameter description for :mod:`file.managed
<salt.states.file.managed>` function for more details and examples.
template
The named templating engine will be used to render the downloaded file.
Defaults to ``jinja``. The following templates are supported:
template : jinja
Templating engine to be used to render the downloaded file. The
following engines are supported:
- :mod:`cheetah<salt.renderers.cheetah>`
- :mod:`genshi<salt.renderers.genshi>`
- :mod:`jinja<salt.renderers.jinja>`
- :mod:`mako<salt.renderers.mako>`
- :mod:`py<salt.renderers.py>`
- :mod:`wempy<salt.renderers.wempy>`
- :mod:`cheetah <salt.renderers.cheetah>`
- :mod:`genshi <salt.renderers.genshi>`
- :mod:`jinja <salt.renderers.jinja>`
- :mod:`mako <salt.renderers.mako>`
- :mod:`py <salt.renderers.py>`
- :mod:`wempy <salt.renderers.wempy>`
context
Overrides default context variables passed to the template.
Overrides default context variables passed to the template
defaults
Default context passed to the template.
Default context passed to the template
append_if_not_found
If markers are not found and set to True then the markers and content
will be appended to the file. Default is ``False``
append_if_not_found : False
If markers are not found and this option is set to ``True``, the
content block will be appended to the file.
prepend_if_not_found
If markers are not found and set to True then the markers and content
will be prepended to the file. Default is ``False``
prepend_if_not_found : False
If markers are not found and this option is set to ``True``, the
content block will be prepended to the file.
backup
The file extension to use for a backup of the file if any edit is made.
Set this to ``False`` to skip making a backup.
dry_run
Don't make any edits to the file
dry_run : False
If ``True``, do not make any edits to the file and simply return the
changes that *would* be made.
show_changes
Output a unified diff of the old file and the new file. If ``False``
return a boolean if any changes were made
show_changes : True
Controls how changes are presented. If ``True``, the ``Changes``
section of the state return will contain a unified diff of the changes
made. If False, then it will contain a boolean (``True`` if any changes
were made, otherwise ``False``).
append_newline
Controls whether or not a newline is appended to the content block. If
the value of this argument is ``True`` then a newline will be added to
the content block. If it is ``False``, then a newline will *not* be
added to the content block. If it is unspecified, then a newline will
only be added to the content block if it does not already end in a
newline.
.. versionadded:: 2017.7.5,2018.3.1
Example of usage with an accumulator and with a variable:
@ -4368,17 +4390,25 @@ def blockreplace(
for index, item in enumerate(text):
content += six.text_type(item)
changes = __salt__['file.blockreplace'](
name,
marker_start,
marker_end,
content=content,
append_if_not_found=append_if_not_found,
prepend_if_not_found=prepend_if_not_found,
backup=backup,
dry_run=__opts__['test'],
show_changes=show_changes
)
try:
changes = __salt__['file.blockreplace'](
name,
marker_start,
marker_end,
content=content,
append_if_not_found=append_if_not_found,
prepend_if_not_found=prepend_if_not_found,
backup=backup,
dry_run=__opts__['test'],
show_changes=show_changes,
append_newline=append_newline)
except Exception as exc:
log.exception('Encountered error managing block')
ret['comment'] = (
'Encountered error managing block: {0}. '
'See the log for details.'.format(exc)
)
return ret
if changes:
ret['pchanges'] = {'diff': changes}

1216
tests/integration/states/test_file.py
View file

File diff suppressed because it is too large Load diff

21
tests/support/helpers.py
View file

@ -24,6 +24,7 @@ import signal
import socket
import string
import sys
import tempfile
import threading
import time
import tornado.ioloop
@ -50,7 +51,7 @@ except ImportError:
# Import Salt Tests Support libs
from tests.support.unit import skip, _id
from tests.support.mock import patch
from tests.support.paths import FILES
from tests.support.paths import FILES, TMP
log = logging.getLogger(__name__)
@ -954,6 +955,24 @@ def with_system_user_and_group(username, group,
return decorator
def with_tempfile(func):
'''
Generates a tempfile and cleans it up when test completes.
'''
@functools.wraps(func)
def wrapper(self, *args, **kwargs):
fd_, name = tempfile.mkstemp(prefix='__salt.test.', dir=TMP)
os.close(fd_)
del fd_
ret = func(self, name, *args, **kwargs)
try:
os.remove(name)
except Exception:
pass
return ret
return wrapper
def requires_system_grains(func):
'''
Function decorator which loads and passes the system's grains to the test

89
tests/unit/modules/test_file.py
View file

@ -285,10 +285,11 @@ class FileBlockReplaceTestCase(TestCase, LoaderModuleMockMixin):
"We shall say 'Ni' again to you, if you do not appease us."
])
filemod.blockreplace(self.tfile.name,
'#-- START BLOCK 1',
'#-- END BLOCK 1',
new_multiline_content,
backup=False)
marker_start='#-- START BLOCK 1',
marker_end='#-- END BLOCK 1',
content=new_multiline_content,
backup=False,
append_newline=None)
with salt.utils.files.fopen(self.tfile.name, 'rb') as fp:
filecontent = fp.read()
@ -306,9 +307,9 @@ class FileBlockReplaceTestCase(TestCase, LoaderModuleMockMixin):
CommandExecutionError,
filemod.blockreplace,
self.tfile.name,
'#-- START BLOCK 2',
'#-- END BLOCK 2',
new_content,
marker_start='#-- START BLOCK 2',
marker_end='#-- END BLOCK 2',
content=new_content,
append_if_not_found=False,
backup=False
)
@ -319,9 +320,9 @@ class FileBlockReplaceTestCase(TestCase, LoaderModuleMockMixin):
)
filemod.blockreplace(self.tfile.name,
'#-- START BLOCK 2',
'#-- END BLOCK 2',
new_content,
marker_start='#-- START BLOCK 2',
marker_end='#-- END BLOCK 2',
content=new_content,
backup=False,
append_if_not_found=True)
@ -382,9 +383,9 @@ class FileBlockReplaceTestCase(TestCase, LoaderModuleMockMixin):
CommandExecutionError,
filemod.blockreplace,
self.tfile.name,
'#-- START BLOCK 2',
'#-- END BLOCK 2',
new_content,
marker_start='#-- START BLOCK 2',
marker_end='#-- END BLOCK 2',
content=new_content,
prepend_if_not_found=False,
backup=False
)
@ -396,8 +397,9 @@ class FileBlockReplaceTestCase(TestCase, LoaderModuleMockMixin):
fp.read())
filemod.blockreplace(self.tfile.name,
'#-- START BLOCK 2', '#-- END BLOCK 2',
new_content,
marker_start='#-- START BLOCK 2',
marker_end='#-- END BLOCK 2',
content=new_content,
backup=False,
prepend_if_not_found=True)
@ -410,9 +412,9 @@ class FileBlockReplaceTestCase(TestCase, LoaderModuleMockMixin):
def test_replace_partial_marked_lines(self):
filemod.blockreplace(self.tfile.name,
'// START BLOCK',
'// END BLOCK',
'new content 1',
marker_start='// START BLOCK',
marker_end='// END BLOCK',
content='new content 1',
backup=False)
with salt.utils.files.fopen(self.tfile.name, 'r') as fp:
@ -420,7 +422,7 @@ class FileBlockReplaceTestCase(TestCase, LoaderModuleMockMixin):
self.assertIn('new content 1', filecontent)
self.assertNotIn('to be removed', filecontent)
self.assertIn('first part of start line', filecontent)
self.assertIn('first part of end line', filecontent)
self.assertNotIn('first part of end line', filecontent)
self.assertIn('part of start line not removed', filecontent)
self.assertIn('part of end line not removed', filecontent)
@ -430,7 +432,9 @@ class FileBlockReplaceTestCase(TestCase, LoaderModuleMockMixin):
filemod.blockreplace(
self.tfile.name,
'// START BLOCK', '// END BLOCK', 'new content 2',
marker_start='// START BLOCK',
marker_end='// END BLOCK',
content='new content 2',
backup=fext)
self.assertTrue(os.path.exists(bak_file))
@ -441,22 +445,27 @@ class FileBlockReplaceTestCase(TestCase, LoaderModuleMockMixin):
bak_file = '{0}{1}'.format(self.tfile.name, fext)
filemod.blockreplace(self.tfile.name,
'// START BLOCK', '// END BLOCK', 'new content 3',
marker_start='// START BLOCK',
marker_end='// END BLOCK',
content='new content 3',
backup=False)
self.assertFalse(os.path.exists(bak_file))
def test_no_modifications(self):
filemod.blockreplace(self.tfile.name,
'// START BLOCK', '// END BLOCK',
'new content 4',
backup=False)
marker_start='#-- START BLOCK 1',
marker_end='#-- END BLOCK 1',
content='new content 4',
backup=False,
append_newline=None)
before_ctime = os.stat(self.tfile.name).st_mtime
filemod.blockreplace(self.tfile.name,
'// START BLOCK',
'// END BLOCK',
'new content 4',
backup=False)
marker_start='#-- START BLOCK 1',
marker_end='#-- END BLOCK 1',
content='new content 4',
backup=False,
append_newline=None)
after_ctime = os.stat(self.tfile.name).st_mtime
self.assertEqual(before_ctime, after_ctime)
@ -464,9 +473,9 @@ class FileBlockReplaceTestCase(TestCase, LoaderModuleMockMixin):
def test_dry_run(self):
before_ctime = os.stat(self.tfile.name).st_mtime
filemod.blockreplace(self.tfile.name,
'// START BLOCK',
'// END BLOCK',
'new content 5',
marker_start='// START BLOCK',
marker_end='// END BLOCK',
content='new content 5',
dry_run=True)
after_ctime = os.stat(self.tfile.name).st_mtime
@ -474,18 +483,18 @@ class FileBlockReplaceTestCase(TestCase, LoaderModuleMockMixin):
def test_show_changes(self):
ret = filemod.blockreplace(self.tfile.name,
'// START BLOCK',
'// END BLOCK',
'new content 6',
marker_start='// START BLOCK',
marker_end='// END BLOCK',
content='new content 6',
backup=False,
show_changes=True)
self.assertTrue(ret.startswith('---')) # looks like a diff
ret = filemod.blockreplace(self.tfile.name,
'// START BLOCK',
'// END BLOCK',
'new content 7',
marker_start='// START BLOCK',
marker_end='// END BLOCK',
content='new content 7',
backup=False,
show_changes=False)
@ -496,9 +505,9 @@ class FileBlockReplaceTestCase(TestCase, LoaderModuleMockMixin):
CommandExecutionError,
filemod.blockreplace,
self.tfile.name,
'#-- START BLOCK UNFINISHED',
'#-- END BLOCK UNFINISHED',
'foobar',
marker_start='#-- START BLOCK UNFINISHED',
marker_end='#-- END BLOCK UNFINISHED',
content='foobar',
backup=False
)