diff --git a/doc/topics/releases/fluorine.rst b/doc/topics/releases/fluorine.rst index 485c42f131..f990bb0cd7 100644 --- a/doc/topics/releases/fluorine.rst +++ b/doc/topics/releases/fluorine.rst @@ -48,6 +48,20 @@ serialized. See the documentation for the new ``serializer_opts`` option in the information. +:py:func:`file.patch ` State Rewritten +------------------------------------------------------------- + +The :py:func:`file.patch ` state has been rewritten with +several new features: + +- Patch sources can now be remote files instead of only ``salt://`` URLs +- Multi-file patches are now supported +- Patch files can be templated + +In addition, it is no longer necessary to specify what the hash of the patched +file should be. + + Deprecations ------------ diff --git a/salt/modules/file.py b/salt/modules/file.py index 98a0fc1501..b011388992 100644 --- a/salt/modules/file.py +++ b/salt/modules/file.py @@ -4080,7 +4080,7 @@ def get_managed( if parsed_scheme == 'salt': source_sum = __salt__['cp.hash_file'](source, saltenv) if not source_sum: - return '', {}, 'Source file {0} not found'.format(source) + return '', {}, 'Source file {0} not found in saltenv \'{1}\''.format(source, saltenv) elif not source_hash and unix_local_source: source_sum = _get_local_file_source_sum(parsed_path) elif not source_hash and source.startswith(os.sep): diff --git a/salt/states/file.py b/salt/states/file.py index 6cd1a230ff..752b8be685 100644 --- a/salt/states/file.py +++ b/salt/states/file.py @@ -266,6 +266,7 @@ For example: # Import python libs from __future__ import absolute_import, print_function, unicode_literals +import copy import difflib import itertools import logging @@ -294,7 +295,7 @@ import salt.utils.templates import salt.utils.url import salt.utils.versions from salt.utils.locales import sdecode -from salt.exceptions import CommandExecutionError, SaltInvocationError +from salt.exceptions import CommandExecutionError from salt.state import get_accumulator_dir as _get_accumulator_dir if salt.utils.platform.is_windows(): @@ -314,6 +315,10 @@ log = logging.getLogger(__name__) COMMENT_REGEX = r'^([[:space:]]*){0}[[:space:]]?' __NOT_FOUND = object() +__func_alias__ = { + 'copy_': 'copy', +} + def _get_accumulator_filepath(): ''' @@ -5155,120 +5160,426 @@ def prepend(name, def patch(name, source=None, + source_hash=None, + source_hash_name=None, + skip_verify=False, + template=None, + context=None, + defaults=None, options='', - dry_run_first=True, + reject_file=None, + strip=None, + saltenv=None, **kwargs): ''' - Ensure that a patch has been applied to the specified file + Ensure that a patch has been applied to the specified file or directory + + .. versionchanged:: Fluorine + The ``hash`` and ``dry_run_first`` options are now ignored, as the + logic which determines whether or not the patch has already been + applied no longer requires them. Additionally, this state now supports + patch files that modify more than one file. To use these sort of + patches, specify a directory (and, if necessary, the ``strip`` option) + instead of a file. .. note:: - A suitable ``patch`` executable must be available on the minion + A suitable ``patch`` executable must be available on the minion. Also, + keep in mind that the pre-check this state does to determine whether or + not changes need to be made will create a temp file and send all patch + output to that file. This means that, in the event that the patch would + not have applied cleanly, the comment included in the state results will + reference a temp file that will no longer exist once the state finishes + running. name - The file to which the patch should be applied + The file or directory to which the patch should be applied source - The source patch to download to the minion, this source file must be - hosted on the salt master server. If the file is located in the - directory named spam, and is called eggs, the source string is - salt://spam/eggs. A source is required. + The patch file to apply - hash - The hash of the patched file. If the hash of the target file matches - this value then the patch is assumed to have been applied. For versions - 2016.11.4 and newer, the hash can be specified without an accompanying - hash type (e.g. ``e138491e9d5b97023cea823fe17bac22``), but for earlier - releases it is necessary to also specify the hash type in the format - ``:`` (e.g. - ``md5:e138491e9d5b97023cea823fe17bac22``). + .. versionchanged:: Fluorine + The source can now be from any file source supported by Salt + (``salt://``, ``http://``, ``https://``, ``ftp://``, etc.). + Templating is also now supported. + + source_hash + Works the same way as in :py:func:`file.managed + `. + + .. versionadded:: Fluorine + + source_hash_name + Works the same way as in :py:func:`file.managed + ` + + .. versionadded:: Fluorine + + skip_verify + Works the same way as in :py:func:`file.managed + ` + + .. versionadded:: Fluorine + + template + Works the same way as in :py:func:`file.managed + ` + + .. versionadded:: Fluorine + + context + Works the same way as in :py:func:`file.managed + ` + + .. versionadded:: Fluorine + + defaults + Works the same way as in :py:func:`file.managed + ` + + .. versionadded:: Fluorine options - Extra options to pass to patch. + Extra options to pass to patch. This should not be necessary in most + cases. - dry_run_first : ``True`` - Run patch with ``--dry-run`` first to check if it will apply cleanly. + .. note:: + For best results, short opts should be separate from one another. + The ``-N`` and ``-r``, and ``-o`` options are used internally by + this state and cannot be used here. Additionally, instead of using + ``-pN`` or ``--strip=N``, use the ``strip`` option documented + below. + + reject_file + If specified, any rejected hunks will be written to this file. If not + specified, then they will be written to a temp file which will be + deleted when the state finishes running. + + .. important:: + The parent directory must exist. Also, this will overwrite the file + if it is already present. + + .. versionadded:: Fluorine + + strip + Number of directories to strip from paths in the patch file. For + example, using the below SLS would instruct Salt to use ``-p1`` when + applying the patch: + + .. code-block:: yaml + + /etc/myfile.conf: + file.patch: + - source: salt://myfile.patch + - strip: 1 + + .. versionadded:: Fluorine + In previous versions, ``-p1`` would need to be passed as part of + the ``options`` value. saltenv Specify the environment from which to retrieve the patch file indicated by the ``source`` parameter. If not provided, this defaults to the environment from which the state is being executed. + .. note:: + Ignored when the patch file is from a non-``salt://`` source. + **Usage:** .. code-block:: yaml - # Equivalent to ``patch --forward /opt/file.txt file.patch`` - /opt/file.txt: + # Equivalent to ``patch --forward /opt/myfile.txt myfile.patch`` + /opt/myfile.txt: file.patch: - - source: salt://file.patch - - hash: e138491e9d5b97023cea823fe17bac22 - - .. note:: - For minions running version 2016.11.3 or older, the hash in the example - above would need to be specified with the hash type (i.e. - ``md5:e138491e9d5b97023cea823fe17bac22``). + - source: salt://myfile.patch ''' - hash_ = kwargs.pop('hash', None) - - if 'env' in kwargs: - # "env" is not supported; Use "saltenv". - kwargs.pop('env') - - name = os.path.expanduser(name) - ret = {'name': name, 'changes': {}, 'result': False, 'comment': ''} + + if not salt.utils.path.which('patch'): + ret['comment'] = 'patch executable not found on minion' + return ret + + # is_dir should be defined if we proceed past the if/else block below, but + # just in case, avoid a NameError. + is_dir = False + if not name: - return _error(ret, 'Must provide name to file.patch') - check_res, check_msg = _check_file(name) - if not check_res: - return _error(ret, check_msg) - if not source: - return _error(ret, 'Source is required') - if hash_ is None: - return _error(ret, 'Hash is required') + ret['comment'] = 'A file/directory to be patched is required' + return ret + else: + try: + name = os.path.expanduser(name) + except Exception: + ret['comment'] = 'Invalid path \'{0}\''.format(name) + return ret + else: + if not os.path.isabs(name): + ret['comment'] = '{0} is not an absolute path'.format(name) + return ret + elif not os.path.exists(name): + ret['comment'] = '{0} does not exist'.format(name) + return ret + else: + is_dir = os.path.isdir(name) + + for deprecated_arg in ('hash', 'dry_run_first'): + if deprecated_arg in kwargs: + ret.setdefault('warnings', []).append( + 'The \'{0}\' argument is no longer used and has been ' + 'ignored.'.format(deprecated_arg) + ) + + if reject_file is not None: + try: + reject_file_parent = os.path.dirname(reject_file) + except Exception: + ret['comment'] = 'Invalid path \'{0}\' for reject_file'.format( + reject_file + ) + return ret + else: + if not os.path.isabs(reject_file_parent): + ret['comment'] = '\'{0}\' is not an absolute path'.format( + reject_file + ) + return ret + elif not os.path.isdir(reject_file_parent): + ret['comment'] = ( + 'Parent directory for reject_file \'{0}\' either does ' + 'not exist, or is not a directory'.format(reject_file) + ) + return ret + + sanitized_options = [] + options = salt.utils.args.shlex_split(options) + index = 0 + max_index = len(options) - 1 + # Not using enumerate here because we may need to consume more than one + # option if --strip is used. + blacklisted_options = [] + while index <= max_index: + option = options[index] + if not isinstance(option, six.string_types): + option = six.text_type(option) + + for item in ('-N', '--forward', '-r', '--reject-file', '-o', '--output'): + if option.startswith(item): + blacklisted = option + break + else: + blacklisted = None + + if blacklisted is not None: + blacklisted_options.append(blacklisted) + + if option.startswith('-p'): + try: + strip = int(option[2:]) + except Exception: + ret['comment'] = ( + 'Invalid format for \'-p\' CLI option. Consider using ' + 'the \'strip\' option for this state.' + ) + return ret + elif option.startswith('--strip'): + if '=' in option: + # Assume --strip=N + try: + strip = int(option.rsplit('=', 1)[-1]) + except Exception: + ret['comment'] = ( + 'Invalid format for \'-strip\' CLI option. Consider ' + 'using the \'strip\' option for this state.' + ) + return ret + else: + # Assume --strip N and grab the next option in the list + try: + strip = int(options[index + 1]) + except Exception: + ret['comment'] = ( + 'Invalid format for \'-strip\' CLI option. Consider ' + 'using the \'strip\' option for this state.' + ) + return ret + else: + # We need to increment again because we grabbed the next + # option in the list. + index += 1 + else: + sanitized_options.append(option) + + # Increment the index + index += 1 + + if blacklisted_options: + ret['comment'] = ( + 'The following CLI options are not allowed: {0}'.format( + ', '.join(blacklisted_options) + ) + ) + return ret + + options = sanitized_options try: - if hash_ and __salt__['file.check_hash'](name, hash_): - ret['result'] = True - ret['comment'] = 'Patch is already applied' - return ret - except (SaltInvocationError, ValueError) as exc: - ret['comment'] = exc.__str__() - return ret - - # get cached file or copy it to cache - cached_source_path = __salt__['cp.cache_file'](source, __env__) - if not cached_source_path: - ret['comment'] = ('Unable to cache {0} from saltenv \'{1}\'' - .format(source, __env__)) - return ret - - log.debug( - 'State patch.applied cached source %s -> %s', - source, cached_source_path - ) - - if dry_run_first or __opts__['test']: - ret['changes'] = __salt__['file.patch']( - name, cached_source_path, options=options, dry_run=True - ) - if __opts__['test']: - ret['comment'] = 'File {0} will be patched'.format(name) - ret['result'] = None - return ret - if ret['changes']['retcode'] != 0: - return ret - - ret['changes'] = __salt__['file.patch']( - name, cached_source_path, options=options - ) - ret['result'] = ret['changes']['retcode'] == 0 - # No need to check for SaltInvocationError or ValueError this time, since - # these exceptions would have been caught above. - if ret['result'] and hash_ and not __salt__['file.check_hash'](name, hash_): + source_match = __salt__['file.source_list'](source, + source_hash, + __env__)[0] + except CommandExecutionError as exc: ret['result'] = False - ret['comment'] = 'Hash mismatch after patch was applied' - return ret + ret['comment'] = exc.strerror + return ret + else: + # Passing the saltenv to file.managed to pull down the patch file is + # not supported, because the saltenv is already being passed via the + # state compiler and this would result in two values for that argument + # (and a traceback). Therefore, we will add the saltenv to the source + # URL to ensure we pull the file from the correct environment. + if saltenv is not None: + source_match_url, source_match_saltenv = \ + salt.utils.url.parse(source_match) + if source_match_url.startswith('salt://'): + if source_match_saltenv is not None \ + and source_match_saltenv != saltenv: + ret.setdefault('warnings', []).append( + 'Ignoring \'saltenv\' option in favor of saltenv ' + 'included in the source URL.' + ) + else: + source_match += '?saltenv={0}'.format(saltenv) + + cleanup = [] + + try: + patch_file = salt.utils.files.mkstemp() + cleanup.append(patch_file) + + try: + orig_test = __opts__['test'] + __opts__['test'] = False + sys.modules[__salt__['test.ping'].__module__].__opts__['test'] = False + result = managed(patch_file, + source=source_match, + source_hash=source_hash, + source_hash_name=source_hash_name, + skip_verify=skip_verify, + template=template, + context=context, + defaults=defaults) + except Exception as exc: + msg = 'Failed to cache patch file {0}: {1}'.format( + salt.utils.url.redact_http_basic_auth(source_match), + exc + ) + log.exception(msg) + ret['comment'] = msg + return ret + else: + log.debug('file.managed: %s', result) + finally: + __opts__['test'] = orig_test + sys.modules[__salt__['test.ping'].__module__].__opts__['test'] = orig_test + + if not result['result']: + log.debug( + 'failed to download %s', + salt.utils.url.redact_http_basic_auth(source_match) + ) + return result + + def _patch(patch_file, options=None, dry_run=False): + patch_opts = copy.copy(sanitized_options) + if options is not None: + patch_opts.extend(options) + return __salt__['file.patch']( + name, + patch_file, + options=patch_opts, + dry_run=dry_run) + + if reject_file is not None: + patch_rejects = reject_file + else: + # No rejects file specified, create a temp file + patch_rejects = salt.utils.files.mkstemp() + cleanup.append(patch_rejects) + + patch_output = salt.utils.files.mkstemp() + cleanup.append(patch_output) + + # Older patch releases can only write patch output to regular files, + # meaning that /dev/null can't be relied on. Also, if we ever want this + # to work on Windows with patch.exe, /dev/null is a non-starter. + # Therefore, redirect all patch output to a temp file, which we will + # then remove. + patch_opts = ['-N', '-r', patch_rejects, '-o', patch_output] + if is_dir and strip is not None: + patch_opts.append('-p{0}'.format(strip)) + + pre_check = _patch(patch_file, patch_opts) + if pre_check['retcode'] != 0: + # Try to reverse-apply hunks from rejects file using a dry-run. + # If this returns a retcode of 0, we know that the patch was + # already applied. Rejects are written from the base of the + # directory, so the strip option doesn't apply here. + reverse_pass = _patch(patch_rejects, ['-R', '-f'], dry_run=True) + already_applied = reverse_pass['retcode'] == 0 + + if already_applied: + ret['comment'] = 'Patch was already applied' + ret['result'] = True + return ret + else: + ret['comment'] = ( + 'Patch would not apply cleanly, no changes made. Results ' + 'of dry-run are below.' + ) + if reject_file is None: + ret['comment'] += ( + ' Run state again using the reject_file option to ' + 'save rejects to a persistent file.' + ) + opts = copy.copy(__opts__) + opts['color'] = False + ret['comment'] += '\n\n' + salt.output.out_format( + pre_check, + 'nested', + opts, + nested_indent=14) + return ret + + if __opts__['test']: + ret['result'] = None + ret['comment'] = 'The patch would be applied' + ret['changes'] = pre_check + return ret + + # If we've made it here, the patch should apply cleanly + patch_opts = [] + if is_dir and strip is not None: + patch_opts.append('-p{0}'.format(strip)) + ret['changes'] = _patch(patch_file, patch_opts) + + if ret['changes']['retcode'] == 0: + ret['comment'] = 'Patch successfully applied' + ret['result'] = True + else: + ret['comment'] = 'Failed to apply patch' + + return ret + + finally: + # Clean up any temp files + for path in cleanup: + try: + os.remove(path) + except OSError as exc: + if exc.errno != os.errno.ENOENT: + log.error( + 'file.patch: Failed to remove temp file %s: %s', + path, exc + ) def touch(name, atime=None, mtime=None, makedirs=False): @@ -5343,17 +5654,16 @@ def touch(name, atime=None, mtime=None, makedirs=False): return ret -def copy( - name, - source, - force=False, - makedirs=False, - preserve=False, - user=None, - group=None, - mode=None, - subdir=False, - **kwargs): +def copy_(name, + source, + force=False, + makedirs=False, + preserve=False, + user=None, + group=None, + mode=None, + subdir=False, + **kwargs): ''' If the file defined by the ``source`` option exists on the minion, copy it to the named path. The file will not be overwritten if it already exists, diff --git a/tests/integration/files/file/base/patches/all.patch b/tests/integration/files/file/base/patches/all.patch new file mode 100644 index 0000000000..3c98518419 --- /dev/null +++ b/tests/integration/files/file/base/patches/all.patch @@ -0,0 +1,24 @@ +diff -ur a/foo/bar/math.txt b/foo/bar/math.txt +--- a/foo/bar/math.txt 2018-04-09 18:43:52.883205365 -0500 ++++ b/foo/bar/math.txt 2018-04-09 18:44:58.525061654 -0500 +@@ -1,3 +1,3 @@ +-Five plus five is ten ++5 + 5 = 10 + +-Four squared is sixteen ++4² = 16 +diff -ur a/foo/numbers.txt b/foo/numbers.txt +--- a/foo/numbers.txt 2018-04-09 18:43:58.014272504 -0500 ++++ b/foo/numbers.txt 2018-04-09 18:44:46.487905044 -0500 +@@ -1,7 +1,7 @@ +-one +-two + three ++two ++one + +-1 +-2 + 3 ++2 ++1 diff --git a/tests/integration/files/file/base/patches/all.patch.jinja b/tests/integration/files/file/base/patches/all.patch.jinja new file mode 100644 index 0000000000..312f3cea95 --- /dev/null +++ b/tests/integration/files/file/base/patches/all.patch.jinja @@ -0,0 +1,24 @@ +diff -ur a/foo/bar/math.txt b/foo/bar/math.txt +--- a/foo/bar/math.txt 2018-04-09 18:43:52.883205365 -0500 ++++ b/foo/bar/math.txt 2018-04-09 18:44:58.525061654 -0500 +@@ -1,3 +1,3 @@ +-Five plus five is ten ++5 + 5 = {{ ten }} + +-Four squared is sixteen ++4² = 16 +diff -ur a/foo/numbers.txt b/foo/numbers.txt +--- a/foo/numbers.txt 2018-04-09 18:43:58.014272504 -0500 ++++ b/foo/numbers.txt 2018-04-09 18:44:46.487905044 -0500 +@@ -1,7 +1,7 @@ +-one +-two + three ++{{ two }} ++one + +-1 +-2 + 3 ++2 ++1 diff --git a/tests/integration/files/file/base/patches/math.patch b/tests/integration/files/file/base/patches/math.patch new file mode 100644 index 0000000000..6628f58e56 --- /dev/null +++ b/tests/integration/files/file/base/patches/math.patch @@ -0,0 +1,8 @@ +--- a/foo/bar/math.txt 2018-04-09 18:43:52.883205365 -0500 ++++ b/foo/bar/math.txt 2018-04-09 18:44:58.525061654 -0500 +@@ -1,3 +1,3 @@ +-Five plus five is ten ++5 + 5 = 10 + +-Four squared is sixteen ++4² = 16 diff --git a/tests/integration/files/file/base/patches/math.patch.jinja b/tests/integration/files/file/base/patches/math.patch.jinja new file mode 100644 index 0000000000..2f70e9184e --- /dev/null +++ b/tests/integration/files/file/base/patches/math.patch.jinja @@ -0,0 +1,8 @@ +--- a/foo/bar/math.txt 2018-04-09 18:43:52.883205365 -0500 ++++ b/foo/bar/math.txt 2018-04-09 18:44:58.525061654 -0500 +@@ -1,3 +1,3 @@ +-Five plus five is ten ++5 + 5 = {{ ten }} + +-Four squared is sixteen ++4² = 16 diff --git a/tests/integration/files/file/base/patches/numbers.patch b/tests/integration/files/file/base/patches/numbers.patch new file mode 100644 index 0000000000..982a778926 --- /dev/null +++ b/tests/integration/files/file/base/patches/numbers.patch @@ -0,0 +1,14 @@ +--- a/foo/numbers.txt 2018-04-09 18:43:58.014272504 -0500 ++++ b/foo/numbers.txt 2018-04-09 18:44:46.487905044 -0500 +@@ -1,7 +1,7 @@ +-one +-two + three ++two ++one + +-1 +-2 + 3 ++2 ++1 diff --git a/tests/integration/files/file/base/patches/numbers.patch.jinja b/tests/integration/files/file/base/patches/numbers.patch.jinja new file mode 100644 index 0000000000..2ac5b9d8a6 --- /dev/null +++ b/tests/integration/files/file/base/patches/numbers.patch.jinja @@ -0,0 +1,14 @@ +--- a/foo/numbers.txt 2018-04-09 18:43:58.014272504 -0500 ++++ b/foo/numbers.txt 2018-04-09 18:44:46.487905044 -0500 +@@ -1,7 +1,7 @@ +-one +-two + three ++{{ two }} ++one + +-1 +-2 + 3 ++2 ++1 diff --git a/tests/integration/states/test_file.py b/tests/integration/states/test_file.py index fb785ab545..f6bfbeba23 100644 --- a/tests/integration/states/test_file.py +++ b/tests/integration/states/test_file.py @@ -1883,39 +1883,6 @@ class FileTest(ModuleCase, SaltReturnAssertsMixin): if os.path.isfile(tmp_file_append): os.remove(tmp_file_append) - def do_patch(self, patch_name='hello', src='Hello\n'): - if not self.run_function('cmd.has_exec', ['patch']): - self.skipTest('patch is not installed') - src_file = os.path.join(TMP, 'src.txt') - with salt.utils.files.fopen(src_file, 'w+') as fp: - fp.write(src) - ret = self.run_state( - 'file.patch', - name=src_file, - source='salt://{0}.patch'.format(patch_name), - hash='md5=f0ef7081e1539ac00ef5b761b4fb01b3', - ) - return src_file, ret - - def test_patch(self): - src_file, ret = self.do_patch() - self.assertSaltTrueReturn(ret) - with salt.utils.files.fopen(src_file) as fp: - self.assertEqual(fp.read(), 'Hello world\n') - - def test_patch_hash_mismatch(self): - src_file, ret = self.do_patch('hello_dolly') - self.assertSaltFalseReturn(ret) - self.assertInSaltComment( - 'Hash mismatch after patch was applied', - ret - ) - - def test_patch_already_applied(self): - src_file, ret = self.do_patch(src='Hello world\n') - self.assertSaltTrueReturn(ret) - self.assertInSaltComment('Patch is already applied', ret) - def test_issue_2401_file_comment(self): # Get a path to the temporary file tmp_file = os.path.join(TMP, 'issue-2041-comment.txt') @@ -3815,3 +3782,591 @@ class RemoteFileTest(ModuleCase, SaltReturnAssertsMixin): skip_verify=True) log.debug('ret = %s', ret) self.assertSaltTrueReturn(ret) + + +@skipIf(not salt.utils.path.which('patch'), 'patch is not installed') +class PatchTest(ModuleCase, SaltReturnAssertsMixin): + + @classmethod + def setUpClass(cls): + cls.webserver = Webserver() + cls.webserver.start() + + cls.numbers_patch_name = 'numbers.patch' + cls.math_patch_name = 'math.patch' + cls.all_patch_name = 'all.patch' + cls.numbers_patch_template_name = cls.numbers_patch_name + '.jinja' + cls.math_patch_template_name = cls.math_patch_name + '.jinja' + cls.all_patch_template_name = cls.all_patch_name + '.jinja' + + cls.numbers_patch_path = 'patches/' + cls.numbers_patch_name + cls.math_patch_path = 'patches/' + cls.math_patch_name + cls.all_patch_path = 'patches/' + cls.all_patch_name + cls.numbers_patch_template_path = \ + 'patches/' + cls.numbers_patch_template_name + cls.math_patch_template_path = \ + 'patches/' + cls.math_patch_template_name + cls.all_patch_template_path = \ + 'patches/' + cls.all_patch_template_name + + cls.numbers_patch = 'salt://' + cls.numbers_patch_path + cls.math_patch = 'salt://' + cls.math_patch_path + cls.all_patch = 'salt://' + cls.all_patch_path + cls.numbers_patch_template = 'salt://' + cls.numbers_patch_template_path + cls.math_patch_template = 'salt://' + cls.math_patch_template_path + cls.all_patch_template = 'salt://' + cls.all_patch_template_path + + cls.numbers_patch_http = cls.webserver.url(cls.numbers_patch_path) + cls.math_patch_http = cls.webserver.url(cls.math_patch_path) + cls.all_patch_http = cls.webserver.url(cls.all_patch_path) + cls.numbers_patch_template_http = \ + cls.webserver.url(cls.numbers_patch_template_path) + cls.math_patch_template_http = \ + cls.webserver.url(cls.math_patch_template_path) + cls.all_patch_template_http = \ + cls.webserver.url(cls.all_patch_template_path) + + patches_dir = os.path.join(FILES, 'file', 'base', 'patches') + cls.numbers_patch_hash = salt.utils.hashutils.get_hash( + os.path.join(patches_dir, cls.numbers_patch_name) + ) + cls.math_patch_hash = salt.utils.hashutils.get_hash( + os.path.join(patches_dir, cls.math_patch_name) + ) + cls.all_patch_hash = salt.utils.hashutils.get_hash( + os.path.join(patches_dir, cls.all_patch_name) + ) + cls.numbers_patch_template_hash = salt.utils.hashutils.get_hash( + os.path.join(patches_dir, cls.numbers_patch_template_name) + ) + cls.math_patch_template_hash = salt.utils.hashutils.get_hash( + os.path.join(patches_dir, cls.math_patch_template_name) + ) + cls.all_patch_template_hash = salt.utils.hashutils.get_hash( + os.path.join(patches_dir, cls.all_patch_template_name) + ) + + cls.context = {'two': 'two', 'ten': 10} + + @classmethod + def tearDownClass(cls): + cls.webserver.stop() + + def setUp(self): + ''' + Create a new unpatched set of files + ''' + self.base_dir = tempfile.mkdtemp(dir=TMP) + os.makedirs(os.path.join(self.base_dir, 'foo', 'bar')) + self.numbers_file = os.path.join(self.base_dir, 'foo', 'numbers.txt') + self.math_file = os.path.join(self.base_dir, 'foo', 'bar', 'math.txt') + with salt.utils.files.fopen(self.numbers_file, 'w') as fp_: + fp_.write(textwrap.dedent('''\ + one + two + three + + 1 + 2 + 3 + ''')) + with salt.utils.files.fopen(self.math_file, 'w') as fp_: + fp_.write(textwrap.dedent('''\ + Five plus five is ten + + Four squared is sixteen + ''')) + + self.addCleanup(shutil.rmtree, self.base_dir, ignore_errors=True) + + def test_patch_single_file(self): + ''' + Test file.patch using a patch applied to a single file + ''' + ret = self.run_state( + 'file.patch', + name=self.numbers_file, + source=self.numbers_patch, + ) + self.assertSaltTrueReturn(ret) + ret = ret[next(iter(ret))] + self.assertEqual(ret['comment'], 'Patch successfully applied') + + # Re-run the state, should succeed and there should be a message about + # a partially-applied hunk. + ret = self.run_state( + 'file.patch', + name=self.numbers_file, + source=self.numbers_patch, + ) + self.assertSaltTrueReturn(ret) + ret = ret[next(iter(ret))] + self.assertEqual(ret['comment'], 'Patch was already applied') + self.assertEqual(ret['changes'], {}) + + def test_patch_directory(self): + ''' + Test file.patch using a patch applied to a directory, with changes + spanning multiple files. + ''' + ret = self.run_state( + 'file.patch', + name=self.base_dir, + source=self.all_patch, + strip=1, + ) + self.assertSaltTrueReturn(ret) + ret = ret[next(iter(ret))] + self.assertEqual(ret['comment'], 'Patch successfully applied') + + # Re-run the state, should succeed and there should be a message about + # a partially-applied hunk. + ret = self.run_state( + 'file.patch', + name=self.base_dir, + source=self.all_patch, + strip=1, + ) + self.assertSaltTrueReturn(ret) + ret = ret[next(iter(ret))] + self.assertEqual(ret['comment'], 'Patch was already applied') + self.assertEqual(ret['changes'], {}) + + def test_patch_strip_parsing(self): + ''' + Test that we successfuly parse -p/--strip when included in the options + ''' + # Run the state using -p1 + ret = self.run_state( + 'file.patch', + name=self.base_dir, + source=self.all_patch, + options='-p1', + ) + self.assertSaltTrueReturn(ret) + ret = ret[next(iter(ret))] + self.assertEqual(ret['comment'], 'Patch successfully applied') + + # Re-run the state using --strip=1 + ret = self.run_state( + 'file.patch', + name=self.base_dir, + source=self.all_patch, + options='--strip=1', + ) + self.assertSaltTrueReturn(ret) + ret = ret[next(iter(ret))] + self.assertEqual(ret['comment'], 'Patch was already applied') + self.assertEqual(ret['changes'], {}) + + # Re-run the state using --strip 1 + ret = self.run_state( + 'file.patch', + name=self.base_dir, + source=self.all_patch, + options='--strip 1', + ) + self.assertSaltTrueReturn(ret) + ret = ret[next(iter(ret))] + self.assertEqual(ret['comment'], 'Patch was already applied') + self.assertEqual(ret['changes'], {}) + + def test_patch_saltenv(self): + ''' + Test that we attempt to download the patch from a non-base saltenv + ''' + # This state will fail because we don't have a patch file in that + # environment, but that is OK, we just want to test that we're looking + # in an environment other than base. + ret = self.run_state( + 'file.patch', + name=self.math_file, + source=self.math_patch, + saltenv='prod', + ) + self.assertSaltFalseReturn(ret) + ret = ret[next(iter(ret))] + self.assertEqual( + ret['comment'], + "Source file {0} not found in saltenv 'prod'".format(self.math_patch) + ) + + def test_patch_single_file_failure(self): + ''' + Test file.patch using a patch applied to a single file. This tests a + failed patch. + ''' + # Empty the file to ensure that the patch doesn't apply cleanly + with salt.utils.files.fopen(self.numbers_file, 'w'): + pass + + ret = self.run_state( + 'file.patch', + name=self.numbers_file, + source=self.numbers_patch, + ) + self.assertSaltFalseReturn(ret) + ret = ret[next(iter(ret))] + self.assertIn('Patch would not apply cleanly', ret['comment']) + + # Test the reject_file option and ensure that the rejects are written + # to the path specified. + reject_file = salt.utils.files.mkstemp() + ret = self.run_state( + 'file.patch', + name=self.numbers_file, + source=self.numbers_patch, + reject_file=reject_file, + strip=1, + ) + self.assertSaltFalseReturn(ret) + ret = ret[next(iter(ret))] + self.assertIn('Patch would not apply cleanly', ret['comment']) + self.assertIn( + 'saving rejects to file {0}'.format(reject_file), + ret['comment'] + ) + + def test_patch_directory_failure(self): + ''' + Test file.patch using a patch applied to a directory, with changes + spanning multiple files. + ''' + # Empty the file to ensure that the patch doesn't apply + with salt.utils.files.fopen(self.math_file, 'w'): + pass + + ret = self.run_state( + 'file.patch', + name=self.base_dir, + source=self.all_patch, + strip=1, + ) + self.assertSaltFalseReturn(ret) + ret = ret[next(iter(ret))] + self.assertIn('Patch would not apply cleanly', ret['comment']) + + # Test the reject_file option and ensure that the rejects are written + # to the path specified. + reject_file = salt.utils.files.mkstemp() + ret = self.run_state( + 'file.patch', + name=self.base_dir, + source=self.all_patch, + reject_file=reject_file, + strip=1, + ) + self.assertSaltFalseReturn(ret) + ret = ret[next(iter(ret))] + self.assertIn('Patch would not apply cleanly', ret['comment']) + self.assertIn( + 'saving rejects to file {0}'.format(reject_file), + ret['comment'] + ) + + def test_patch_single_file_remote_source(self): + ''' + Test file.patch using a patch applied to a single file, with the patch + coming from a remote source. + ''' + # Try without a source_hash and without skip_verify=True, this should + # fail with a message about the source_hash + ret = self.run_state( + 'file.patch', + name=self.math_file, + source=self.math_patch_http, + ) + self.assertSaltFalseReturn(ret) + ret = ret[next(iter(ret))] + self.assertIn('Unable to verify upstream hash', ret['comment']) + + # Re-run the state with a source hash, it should now succeed + ret = self.run_state( + 'file.patch', + name=self.math_file, + source=self.math_patch_http, + source_hash=self.math_patch_hash, + ) + self.assertSaltTrueReturn(ret) + ret = ret[next(iter(ret))] + self.assertEqual(ret['comment'], 'Patch successfully applied') + + # Re-run again, this time with no hash and skip_verify=True to test + # skipping hash verification + ret = self.run_state( + 'file.patch', + name=self.math_file, + source=self.math_patch_http, + skip_verify=True, + ) + self.assertSaltTrueReturn(ret) + ret = ret[next(iter(ret))] + self.assertEqual(ret['comment'], 'Patch was already applied') + self.assertEqual(ret['changes'], {}) + + def test_patch_directory_remote_source(self): + ''' + Test file.patch using a patch applied to a directory, with changes + spanning multiple files, and the patch file coming from a remote + source. + ''' + # Try without a source_hash and without skip_verify=True, this should + # fail with a message about the source_hash + ret = self.run_state( + 'file.patch', + name=self.base_dir, + source=self.all_patch_http, + strip=1, + ) + self.assertSaltFalseReturn(ret) + ret = ret[next(iter(ret))] + self.assertIn('Unable to verify upstream hash', ret['comment']) + + # Re-run the state with a source hash, it should now succeed + ret = self.run_state( + 'file.patch', + name=self.base_dir, + source=self.all_patch_http, + source_hash=self.all_patch_hash, + strip=1, + ) + self.assertSaltTrueReturn(ret) + ret = ret[next(iter(ret))] + self.assertEqual(ret['comment'], 'Patch successfully applied') + + # Re-run again, this time with no hash and skip_verify=True to test + # skipping hash verification + ret = self.run_state( + 'file.patch', + name=self.base_dir, + source=self.all_patch_http, + strip=1, + skip_verify=True, + ) + self.assertSaltTrueReturn(ret) + ret = ret[next(iter(ret))] + self.assertEqual(ret['comment'], 'Patch was already applied') + self.assertEqual(ret['changes'], {}) + + def test_patch_single_file_template(self): + ''' + Test file.patch using a patch applied to a single file, with jinja + templating applied to the patch file. + ''' + ret = self.run_state( + 'file.patch', + name=self.numbers_file, + source=self.numbers_patch_template, + template='jinja', + context=self.context, + ) + self.assertSaltTrueReturn(ret) + ret = ret[next(iter(ret))] + self.assertEqual(ret['comment'], 'Patch successfully applied') + + # Re-run the state, should succeed and there should be a message about + # a partially-applied hunk. + ret = self.run_state( + 'file.patch', + name=self.numbers_file, + source=self.numbers_patch_template, + template='jinja', + context=self.context, + ) + self.assertSaltTrueReturn(ret) + ret = ret[next(iter(ret))] + self.assertEqual(ret['comment'], 'Patch was already applied') + self.assertEqual(ret['changes'], {}) + + def test_patch_directory_template(self): + ''' + Test file.patch using a patch applied to a directory, with changes + spanning multiple files, and with jinja templating applied to the patch + file. + ''' + ret = self.run_state( + 'file.patch', + name=self.base_dir, + source=self.all_patch_template, + template='jinja', + context=self.context, + strip=1, + ) + self.assertSaltTrueReturn(ret) + ret = ret[next(iter(ret))] + self.assertEqual(ret['comment'], 'Patch successfully applied') + + # Re-run the state, should succeed and there should be a message about + # a partially-applied hunk. + ret = self.run_state( + 'file.patch', + name=self.base_dir, + source=self.all_patch_template, + template='jinja', + context=self.context, + strip=1, + ) + self.assertSaltTrueReturn(ret) + ret = ret[next(iter(ret))] + self.assertEqual(ret['comment'], 'Patch was already applied') + self.assertEqual(ret['changes'], {}) + + def test_patch_single_file_remote_source_template(self): + ''' + Test file.patch using a patch applied to a single file, with the patch + coming from a remote source. + ''' + # Try without a source_hash and without skip_verify=True, this should + # fail with a message about the source_hash + ret = self.run_state( + 'file.patch', + name=self.math_file, + source=self.math_patch_template_http, + template='jinja', + context=self.context, + ) + self.assertSaltFalseReturn(ret) + ret = ret[next(iter(ret))] + self.assertIn('Unable to verify upstream hash', ret['comment']) + + # Re-run the state with a source hash, it should now succeed + ret = self.run_state( + 'file.patch', + name=self.math_file, + source=self.math_patch_template_http, + source_hash=self.math_patch_template_hash, + template='jinja', + context=self.context, + ) + self.assertSaltTrueReturn(ret) + ret = ret[next(iter(ret))] + self.assertEqual(ret['comment'], 'Patch successfully applied') + + # Re-run again, this time with no hash and skip_verify=True to test + # skipping hash verification + ret = self.run_state( + 'file.patch', + name=self.math_file, + source=self.math_patch_template_http, + template='jinja', + context=self.context, + skip_verify=True, + ) + self.assertSaltTrueReturn(ret) + ret = ret[next(iter(ret))] + self.assertEqual(ret['comment'], 'Patch was already applied') + self.assertEqual(ret['changes'], {}) + + def test_patch_directory_remote_source_template(self): + ''' + Test file.patch using a patch applied to a directory, with changes + spanning multiple files, and the patch file coming from a remote + source. + ''' + # Try without a source_hash and without skip_verify=True, this should + # fail with a message about the source_hash + ret = self.run_state( + 'file.patch', + name=self.base_dir, + source=self.all_patch_template_http, + template='jinja', + context=self.context, + strip=1, + ) + self.assertSaltFalseReturn(ret) + ret = ret[next(iter(ret))] + self.assertIn('Unable to verify upstream hash', ret['comment']) + + # Re-run the state with a source hash, it should now succeed + ret = self.run_state( + 'file.patch', + name=self.base_dir, + source=self.all_patch_template_http, + source_hash=self.all_patch_template_hash, + template='jinja', + context=self.context, + strip=1, + ) + self.assertSaltTrueReturn(ret) + ret = ret[next(iter(ret))] + self.assertEqual(ret['comment'], 'Patch successfully applied') + + # Re-run again, this time with no hash and skip_verify=True to test + # skipping hash verification + ret = self.run_state( + 'file.patch', + name=self.base_dir, + source=self.all_patch_template_http, + template='jinja', + context=self.context, + strip=1, + skip_verify=True, + ) + self.assertSaltTrueReturn(ret) + ret = ret[next(iter(ret))] + self.assertEqual(ret['comment'], 'Patch was already applied') + self.assertEqual(ret['changes'], {}) + + def test_patch_test_mode(self): + ''' + Test file.patch using test=True + ''' + # Try without a source_hash and without skip_verify=True, this should + # fail with a message about the source_hash + ret = self.run_state( + 'file.patch', + name=self.numbers_file, + source=self.numbers_patch, + test=True, + ) + self.assertSaltNoneReturn(ret) + ret = ret[next(iter(ret))] + self.assertEqual(ret['comment'], 'The patch would be applied') + self.assertTrue(ret['changes']) + + # Apply the patch for real. We'll then be able to test below that we + # exit with a True rather than a None result if test=True is used on an + # already-applied patch. + ret = self.run_state( + 'file.patch', + name=self.numbers_file, + source=self.numbers_patch, + ) + self.assertSaltTrueReturn(ret) + ret = ret[next(iter(ret))] + self.assertEqual(ret['comment'], 'Patch successfully applied') + self.assertTrue(ret['changes']) + + # Run again with test=True. Since the pre-check happens before we do + # the __opts__['test'] check, we should exit with a True result just + # the same as if we try to run this state on an already-patched file + # *without* test=True. + ret = self.run_state( + 'file.patch', + name=self.numbers_file, + source=self.numbers_patch, + test=True, + ) + self.assertSaltTrueReturn(ret) + ret = ret[next(iter(ret))] + self.assertEqual(ret['comment'], 'Patch was already applied') + self.assertEqual(ret['changes'], {}) + + # Empty the file to ensure that the patch doesn't apply cleanly + with salt.utils.files.fopen(self.numbers_file, 'w'): + pass + + # Run again with test=True. Similar to the above run, we are testing + # that we return before we reach the __opts__['test'] check. In this + # case we should return a False result because we should already know + # by this point that the patch will not apply cleanly. + ret = self.run_state( + 'file.patch', + name=self.numbers_file, + source=self.numbers_patch, + test=True, + ) + self.assertSaltFalseReturn(ret) + ret = ret[next(iter(ret))] + self.assertIn('Patch would not apply cleanly', ret['comment']) + self.assertEqual(ret['changes'], {}) diff --git a/tests/unit/states/test_file.py b/tests/unit/states/test_file.py index 61be8418d1..503e9ac82a 100644 --- a/tests/unit/states/test_file.py +++ b/tests/unit/states/test_file.py @@ -1217,76 +1217,6 @@ class TestFileState(TestCase, LoaderModuleMockMixin): self.assertDictEqual(filestate.prepend (name, text=text), ret) - # 'patch' function tests: 1 - - def test_patch(self): - ''' - Test to apply a patch to a file. - ''' - name = '/opt/file.txt' - source = 'salt://file.patch' - ha_sh = 'md5=e138491e9d5b97023cea823fe17bac22' - - ret = {'name': name, - 'result': False, - 'comment': '', - 'changes': {}} - - comt = ('Must provide name to file.patch') - ret.update({'comment': comt, 'name': ''}) - self.assertDictEqual(filestate.patch(''), ret) - - comt = ('{0}: file not found'.format(name)) - ret.update({'comment': comt, 'name': name}) - self.assertDictEqual(filestate.patch(name), ret) - - mock_t = MagicMock(return_value=True) - mock_true = MagicMock(side_effect=[True, False, False, False, False]) - mock_false = MagicMock(side_effect=[False, True, True, True]) - mock_ret = MagicMock(return_value={'retcode': True}) - with patch.object(os.path, 'isabs', mock_t): - with patch.object(os.path, 'exists', mock_t): - comt = ('Source is required') - ret.update({'comment': comt}) - self.assertDictEqual(filestate.patch(name), ret) - - comt = ('Hash is required') - ret.update({'comment': comt}) - self.assertDictEqual(filestate.patch(name, source=source), ret) - - with patch.dict(filestate.__salt__, - {'file.check_hash': mock_true, - 'cp.cache_file': mock_false, - 'file.patch': mock_ret}): - comt = ('Patch is already applied') - ret.update({'comment': comt, 'result': True}) - self.assertDictEqual(filestate.patch(name, source=source, - hash=ha_sh), ret) - - comt = ("Unable to cache salt://file.patch" - " from saltenv 'base'") - ret.update({'comment': comt, 'result': False}) - self.assertDictEqual(filestate.patch(name, source=source, - hash=ha_sh), ret) - - with patch.dict(filestate.__opts__, {'test': True}): - comt = ('File /opt/file.txt will be patched') - ret.update({'comment': comt, 'result': None, - 'changes': {'retcode': True}}) - self.assertDictEqual(filestate.patch(name, - source=source, - hash=ha_sh), ret) - - with patch.dict(filestate.__opts__, {'test': False}): - ret.update({'comment': '', 'result': False}) - self.assertDictEqual(filestate.patch(name, - source=source, - hash=ha_sh), ret) - - self.assertDictEqual(filestate.patch - (name, source=source, hash=ha_sh, - dry_run_first=False), ret) - # 'touch' function tests: 1 def test_touch(self): @@ -1351,7 +1281,7 @@ class TestFileState(TestCase, LoaderModuleMockMixin): comt = ('Must provide name to file.copy') ret.update({'comment': comt, 'name': ''}) - self.assertDictEqual(filestate.copy('', source), ret) + self.assertDictEqual(filestate.copy_('', source), ret) mock_t = MagicMock(return_value=True) mock_f = MagicMock(return_value=False) @@ -1363,13 +1293,13 @@ class TestFileState(TestCase, LoaderModuleMockMixin): with patch.object(os.path, 'isabs', mock_f): comt = ('Specified file {0} is not an absolute path'.format(name)) ret.update({'comment': comt, 'name': name}) - self.assertDictEqual(filestate.copy(name, source), ret) + self.assertDictEqual(filestate.copy_(name, source), ret) with patch.object(os.path, 'isabs', mock_t): with patch.object(os.path, 'exists', mock_f): comt = ('Source file "{0}" is not present'.format(source)) ret.update({'comment': comt, 'result': False}) - self.assertDictEqual(filestate.copy(name, source), ret) + self.assertDictEqual(filestate.copy_(name, source), ret) with patch.object(os.path, 'exists', mock_t): with patch.dict(filestate.__salt__, @@ -1389,8 +1319,8 @@ class TestFileState(TestCase, LoaderModuleMockMixin): comt = ('User salt is not available Group saltstack' ' is not available') ret.update({'comment': comt, 'result': False}) - self.assertDictEqual(filestate.copy(name, source, user=user, - group=group), ret) + self.assertDictEqual(filestate.copy_(name, source, user=user, + group=group), ret) comt1 = ('Failed to delete "{0}" in preparation for' ' forced move'.format(name)) @@ -1406,7 +1336,7 @@ class TestFileState(TestCase, LoaderModuleMockMixin): {'file.remove': mock_io}): ret.update({'comment': comt1, 'result': False}) - self.assertDictEqual(filestate.copy + self.assertDictEqual(filestate.copy_ (name, source, preserve=True, force=True), ret) @@ -1414,14 +1344,14 @@ class TestFileState(TestCase, LoaderModuleMockMixin): with patch.object(os.path, 'isfile', mock_t): ret.update({'comment': comt2, 'result': True}) - self.assertDictEqual(filestate.copy + self.assertDictEqual(filestate.copy_ (name, source, preserve=True), ret) with patch.object(os.path, 'lexists', mock_f): with patch.dict(filestate.__opts__, {'test': True}): ret.update({'comment': comt3, 'result': None}) - self.assertDictEqual(filestate.copy + self.assertDictEqual(filestate.copy_ (name, source, preserve=True), ret) @@ -1429,7 +1359,7 @@ class TestFileState(TestCase, LoaderModuleMockMixin): comt = ('The target directory /tmp is' ' not present') ret.update({'comment': comt, 'result': False}) - self.assertDictEqual(filestate.copy + self.assertDictEqual(filestate.copy_ (name, source, preserve=True), ret)