Merge pull request #44794 from terminalmage/issue44365

Fix regression in file.managed when source_hash used with local file
This commit is contained in:
Nicole Thomas 2017-12-04 09:23:28 -05:00 committed by GitHub
commit 88c0d66b4e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 114 additions and 24 deletions

View file

@ -3764,8 +3764,15 @@ def get_managed(
parsed_scheme = urlparsed_source.scheme
parsed_path = os.path.join(
urlparsed_source.netloc, urlparsed_source.path).rstrip(os.sep)
unix_local_source = parsed_scheme in ('file', '')
if parsed_scheme and parsed_scheme.lower() in 'abcdefghijklmnopqrstuvwxyz':
if unix_local_source:
sfn = parsed_path
if not os.path.exists(sfn):
msg = 'Local file source {0} does not exist'.format(sfn)
return '', {}, msg
if parsed_scheme and parsed_scheme.lower() in string.ascii_lowercase:
parsed_path = ':'.join([parsed_scheme, parsed_path])
parsed_scheme = 'file'
@ -3773,9 +3780,10 @@ def get_managed(
source_sum = __salt__['cp.hash_file'](source, saltenv)
if not source_sum:
return '', {}, 'Source file {0} not found'.format(source)
elif not source_hash and parsed_scheme == 'file':
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):
# This should happen on Windows
source_sum = _get_local_file_source_sum(source)
else:
if not skip_verify:
@ -4650,21 +4658,22 @@ def manage_file(name,
if source_sum and ('hsum' in source_sum):
source_sum['hsum'] = source_sum['hsum'].lower()
if source and not sfn:
# File is not present, cache it
sfn = __salt__['cp.cache_file'](source, saltenv)
if source:
if not sfn:
return _error(
ret, 'Source file \'{0}\' not found'.format(source))
htype = source_sum.get('hash_type', __opts__['hash_type'])
# Recalculate source sum now that file has been cached
source_sum = {
'hash_type': htype,
'hsum': get_hash(sfn, form=htype)
}
# File is not present, cache it
sfn = __salt__['cp.cache_file'](source, saltenv)
if not sfn:
return _error(
ret, 'Source file \'{0}\' not found'.format(source))
htype = source_sum.get('hash_type', __opts__['hash_type'])
# Recalculate source sum now that file has been cached
source_sum = {
'hash_type': htype,
'hsum': get_hash(sfn, form=htype)
}
if keep_mode:
if _urlparse(source).scheme in ('salt', 'file') \
or source.startswith('/'):
if _urlparse(source).scheme in ('salt', 'file', ''):
try:
mode = __salt__['cp.stat_file'](source, saltenv=saltenv, octal=True)
except Exception as exc:
@ -4694,7 +4703,7 @@ def manage_file(name,
# source, and we are not skipping checksum verification, then
# verify that it matches the specified checksum.
if not skip_verify \
and _urlparse(source).scheme not in ('salt', ''):
and _urlparse(source).scheme != 'salt':
dl_sum = get_hash(sfn, source_sum['hash_type'])
if dl_sum != source_sum['hsum']:
ret['comment'] = (
@ -4859,8 +4868,6 @@ def manage_file(name,
group=group, mode=dir_mode)
if source:
# It is a new file, set the diff accordingly
ret['changes']['diff'] = 'New file'
# Apply the new file
if not sfn:
sfn = __salt__['cp.cache_file'](source, saltenv)
@ -4884,6 +4891,8 @@ def manage_file(name,
)
ret['result'] = False
return ret
# It is a new file, set the diff accordingly
ret['changes']['diff'] = 'New file'
if not os.path.isdir(contain_dir):
if makedirs:
_set_mode_and_make_dirs(name, dir_mode, mode, user, group)

View file

@ -1605,6 +1605,9 @@ def managed(name,
'name': name,
'result': True}
if not name:
return _error(ret, 'Destination file name is required')
if mode is not None and salt.utils.is_windows():
return _error(ret, 'The \'mode\' option is not supported on Windows')
@ -1755,8 +1758,6 @@ def managed(name,
ret['comment'] = 'Error while applying template on contents'
return ret
if not name:
return _error(ret, 'Must provide name to file.exists')
user = _test_owner(kwargs, user=user)
if salt.utils.is_windows():
if group is not None:

View file

@ -104,9 +104,9 @@ def _test_managed_file_mode_keep_helper(testcase, local=False):
# Get the current mode so that we can put the file back the way we
# found it when we're done.
grail_fs_mode = os.stat(grail_fs_path).st_mode
initial_mode = 504 # 0770 octal
new_mode_1 = 384 # 0600 octal
new_mode_2 = 420 # 0644 octal
initial_mode = 0o770
new_mode_1 = 0o600
new_mode_2 = 0o644
# Set the initial mode, so we can be assured that when we set the mode
# to "keep", we're actually changing the permissions of the file to the
@ -587,6 +587,86 @@ class FileTest(integration.ModuleCase, integration.SaltReturnAssertsMixIn):
for typ in managed_files:
os.remove(managed_files[typ])
def test_managed_local_source_with_source_hash(self):
'''
Make sure that we enforce the source_hash even with local files
'''
name = os.path.join(integration.TMP, 'local_source_with_source_hash')
local_path = os.path.join(
integration.FILES, 'file', 'base', 'grail', 'scene33')
actual_hash = '567fd840bf1548edc35c48eb66cdd78bfdfcccff'
# Reverse the actual hash
bad_hash = actual_hash[::-1]
def remove_file():
try:
os.remove(name)
except OSError as exc:
if exc.errno != errno.ENOENT:
raise
def do_test(clean=False):
for proto in ('file://', ''):
source = proto + local_path
log.debug('Trying source %s', source)
try:
ret = self.run_state(
'file.managed',
name=name,
source=source,
source_hash='sha1={0}'.format(bad_hash))
self.assertSaltFalseReturn(ret)
ret = ret[next(iter(ret))]
# Shouldn't be any changes
self.assertFalse(ret['changes'])
# Check that we identified a hash mismatch
self.assertIn(
'does not match actual checksum', ret['comment'])
ret = self.run_state(
'file.managed',
name=name,
source=source,
source_hash='sha1={0}'.format(actual_hash))
self.assertSaltTrueReturn(ret)
finally:
if clean:
remove_file()
remove_file()
log.debug('Trying with nonexistant destination file')
do_test()
log.debug('Trying with destination file already present')
with salt.utils.fopen(name, 'w'):
pass
try:
do_test(clean=False)
finally:
remove_file()
def test_managed_local_source_does_not_exist(self):
'''
Make sure that we exit gracefully when a local source doesn't exist
'''
name = os.path.join(integration.TMP, 'local_source_does_not_exist')
local_path = os.path.join(
integration.FILES, 'file', 'base', 'grail', 'scene99')
for proto in ('file://', ''):
source = proto + local_path
log.debug('Trying source %s', source)
ret = self.run_state(
'file.managed',
name=name,
source=source)
self.assertSaltFalseReturn(ret)
ret = ret[next(iter(ret))]
# Shouldn't be any changes
self.assertFalse(ret['changes'])
# Check that we identified a hash mismatch
self.assertIn(
'does not exist', ret['comment'])
def test_directory(self):
'''
file.directory

View file

@ -581,7 +581,7 @@ class FileTestCase(TestCase):
'file.copy': mock_cp,
'file.manage_file': mock_ex,
'cmd.run_all': mock_cmd_fail}):
comt = ('Must provide name to file.exists')
comt = ('Destination file name is required')
ret.update({'comment': comt, 'name': '', 'pchanges': {}})
self.assertDictEqual(filestate.managed(''), ret)