Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added clean_parent argument for the archive state #55418

Merged
merged 4 commits into from
Dec 3, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ Versions are `MAJOR.PATCH`.
### Added

- [#54917](https://github.com/saltstack/salt/pull/54917) - Added get_settings, put_settings and flush_synced methods for Elasticsearch module. - [@Oloremo](https://github.com/Oloremo)
- [#55418](https://github.com/saltstack/salt/pull/55418) - Added clean_parent argument for the archive state. - [@Oloremo](https://github.com/Oloremo)

---

Expand Down
43 changes: 43 additions & 0 deletions salt/states/archive.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ def extracted(name,
force=False,
overwrite=False,
clean=False,
clean_parent=False,
user=None,
group=None,
if_missing=None,
Expand Down Expand Up @@ -522,6 +523,14 @@ def extracted(name,

.. versionadded:: 2016.11.1

clean_parent : False
If ``True``, and the archive is extracted, delete the parent
directory (i.e. the directory into which the archive is extracted), and
then re-create that directory before extracting. Note that ``clean``
and ``clean_parent`` are mutually exclusive.

.. versionadded:: Sodium

user
The user to own each extracted file. Not available on Windows.

Expand Down Expand Up @@ -1076,6 +1085,11 @@ def extracted(name,
))
return ret

if clean and clean_parent:
ret['comment'] = "Only one of 'clean' and 'clean_parent' can be set to True"
ret['result'] = False
return ret

extraction_needed = overwrite
contents_missing = False

Expand Down Expand Up @@ -1148,6 +1162,15 @@ def extracted(name,
)
)
return ret
if __opts__['test'] and clean_parent and contents is not None:
ret['result'] = None
ret['comment'] += (
' Since the \'clean_parent\' option is enabled, the '
'destination parent directory would be removed first '
'and then re-created and the archive would be '
'extracted'
)
return ret

# Skip notices of incorrect types if we're cleaning
if not (clean and contents is not None):
Expand Down Expand Up @@ -1216,6 +1239,26 @@ def extracted(name,
_add_explanation(ret, source_hash_trigger, contents_missing)
return ret

if clean_parent and contents is not None:
errors = []
log.debug('Removing directory %s due to clean_parent set to True', name)
try:
salt.utils.files.rm_rf(name.rstrip(os.sep))
ret['changes'].setdefault(
'removed', "Directory {} was removed prior to the extraction".format(name))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you've removed the directory, you need to recreate it now. And you also need to take into account the value of the makedirs argument when you do. Otherwise, the destination directory won't exist when it comes time to extract, and the extraction will fail.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct me if I'm wrong but parent directory going to be created no matter what:

1286         if not os.path.isdir(name):
1287             __states__['file.directory'](name, user=user, makedirs=True)
1288             created_destdir = True

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh yes, you are correct. I mistakenly thought that this state used a makedirs param, I must have been confusing it with something else.

except OSError as exc:
if exc.errno != errno.ENOENT:
errors.append(six.text_type(exc))
if errors:
msg = (
'Unable to remove the directory {}. The following '
'errors were observed:\n'.format(name)
)
for error in errors:
msg += '\n- {0}'.format(error)
ret['comment'] = msg
return ret

if clean and contents is not None:
errors = []
log.debug('Cleaning archive paths from within %s', name)
Expand Down
44 changes: 44 additions & 0 deletions tests/unit/states/test_archive.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,3 +225,47 @@ def test_extracted_when_if_missing_path_exists(self):
ret['comment'],
'Path {0} exists'.format(if_missing)
)

def test_clean_parent_conflict(self):
'''
Tests the call of extraction with gnutar with both clean_parent plus clean set to True
'''
gnutar = MagicMock(return_value='tar (GNU tar)')
source = '/tmp/foo.tar.gz'
ret_comment = "Only one of 'clean' and 'clean_parent' can be set to True"
mock_false = MagicMock(return_value=False)
mock_true = MagicMock(return_value=True)
state_single_mock = MagicMock(return_value={'local': {'result': True}})
run_all = MagicMock(return_value={'retcode': 0, 'stdout': 'stdout', 'stderr': 'stderr'})
mock_source_list = MagicMock(return_value=(source, None))
list_mock = MagicMock(return_value={
'dirs': [],
'files': ['stdout'],
'links': [],
'top_level_dirs': [],
'top_level_files': ['stdout'],
'top_level_links': [],
})
isfile_mock = MagicMock(side_effect=_isfile_side_effect)

with patch.dict(archive.__salt__, {'cmd.run': gnutar,
'file.directory_exists': mock_false,
'file.file_exists': mock_false,
'state.single': state_single_mock,
'file.makedirs': mock_true,
'cmd.run_all': run_all,
'archive.list': list_mock,
'file.source_list': mock_source_list}),\
patch.dict(archive.__states__, {'file.directory': mock_true}),\
patch.object(os.path, 'isfile', isfile_mock),\
patch('salt.utils.path.which', MagicMock(return_value=True)):
ret = archive.extracted(os.path.join(os.sep + 'tmp', 'out'),
source,
options='xvzf',
enforce_toplevel=False,
clean=True,
clean_parent=True,
keep=True)
self.assertEqual(ret['result'], False)
self.assertEqual(ret['changes'], {})
self.assertEqual(ret['comment'], ret_comment)