From 91e2a8fba166e45b798042f93a52679025164c20 Mon Sep 17 00:00:00 2001 From: Proskurin Kirill Date: Thu, 25 Jul 2019 19:04:49 +0100 Subject: [PATCH 1/4] Added clean_parent argument for the archive state --- salt/states/archive.py | 41 ++++++++++++++++++++++++++++ tests/unit/states/test_archive.py | 44 +++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+) diff --git a/salt/states/archive.py b/salt/states/archive.py index 61ec3d79227f..f70eafe34b6d 100644 --- a/salt/states/archive.py +++ b/salt/states/archive.py @@ -183,6 +183,7 @@ def extracted(name, force=False, overwrite=False, clean=False, + clean_parent=False, user=None, group=None, if_missing=None, @@ -522,6 +523,12 @@ def extracted(name, .. versionadded:: 2016.11.1 + clean_parent : False + Set this to ``True`` to remove a parent archive directory before the extration. + ``clean`` and ``clean_parent`` args are mutually exclusive. + + .. versionadded:: Sodium + user The user to own each extracted file. Not available on Windows. @@ -1076,6 +1083,11 @@ def extracted(name, )) return ret + if clean and clean_parent: + ret['comment'] = "You can't set both 'clean' and 'clean_parent' to True." + ret['result'] = False + return ret + extraction_needed = overwrite contents_missing = False @@ -1148,6 +1160,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 than 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): @@ -1216,6 +1237,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)) + 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) diff --git a/tests/unit/states/test_archive.py b/tests/unit/states/test_archive.py index 03d3c7d18573..5242d9649126 100644 --- a/tests/unit/states/test_archive.py +++ b/tests/unit/states/test_archive.py @@ -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 = "You can't set both 'clean' and 'clean_parent' 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) From 0b61c5723f40c79ca8303db590709ff7df77f4b6 Mon Sep 17 00:00:00 2001 From: Proskurin Kirill Date: Mon, 25 Nov 2019 18:52:00 +0000 Subject: [PATCH 2/4] Changed description and conflic error message --- salt/states/archive.py | 8 +++++--- tests/unit/states/test_archive.py | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/salt/states/archive.py b/salt/states/archive.py index f70eafe34b6d..630da0c9792c 100644 --- a/salt/states/archive.py +++ b/salt/states/archive.py @@ -524,8 +524,10 @@ def extracted(name, .. versionadded:: 2016.11.1 clean_parent : False - Set this to ``True`` to remove a parent archive directory before the extration. - ``clean`` and ``clean_parent`` args are mutually exclusive. + 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 @@ -1084,7 +1086,7 @@ def extracted(name, return ret if clean and clean_parent: - ret['comment'] = "You can't set both 'clean' and 'clean_parent' to True." + ret['comment'] = "Only one of 'clean' and 'clean_parent' can be set to True" ret['result'] = False return ret diff --git a/tests/unit/states/test_archive.py b/tests/unit/states/test_archive.py index 5242d9649126..25d908b0d7d2 100644 --- a/tests/unit/states/test_archive.py +++ b/tests/unit/states/test_archive.py @@ -232,7 +232,7 @@ def test_clean_parent_conflict(self): ''' gnutar = MagicMock(return_value='tar (GNU tar)') source = '/tmp/foo.tar.gz' - ret_comment = "You can't set both 'clean' and 'clean_parent' to True." + 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}}) From 54d3c3f4ed4470eef35cf85ad3e7215aeba60bc1 Mon Sep 17 00:00:00 2001 From: Proskurin Kirill Date: Mon, 25 Nov 2019 23:12:02 +0000 Subject: [PATCH 3/4] Updated changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 07199208d0e7..1fd3386dab38 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) --- From 9047b01f511f9a02a54b2dd18d56ce79ad68c97b Mon Sep 17 00:00:00 2001 From: Proskurin Kirill Date: Mon, 2 Dec 2019 17:38:46 -0500 Subject: [PATCH 4/4] Fixed typo --- salt/states/archive.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/states/archive.py b/salt/states/archive.py index 630da0c9792c..5cd333336576 100644 --- a/salt/states/archive.py +++ b/salt/states/archive.py @@ -1167,7 +1167,7 @@ def extracted(name, ret['comment'] += ( ' Since the \'clean_parent\' option is enabled, the ' 'destination parent directory would be removed first ' - 'and than re-created and the archive would be ' + 'and then re-created and the archive would be ' 'extracted' ) return ret