Merge pull request #51844 from mbunkus/fix-file-removal

Fix file removal

doc/topics/releases/neon.rst

@@ -369,12 +369,102 @@ Also, slot parsing is now supported inside of nested state data structures (dict
             - "DO NOT OVERRIDE"
           ignore_if_missing: True
 
+- The :py:func:`file.symlink <salt.states.file.symlink>` state was
+  fixed to remove existing file system entries other than files,
+  directories and symbolic links properly.
+
+- The ``onchanges`` and ``prereq`` :ref:`requisites <requisites>` now behave
+  properly in test mode.
+
 State Changes
 =============
 
 - Added new :py:func:`ssh_auth.manage <salt.states.ssh_auth.manage>` state to
   ensure only the specified ssh keys are present for the specified user.
 
+- Added new :py:func:`saltutil <salt.states.saltutil>` state to use instead of
+  ``module.run`` to more easily handle change.
+
+- Added new `onfail_all` requisite form to allow for AND logic when adding
+  onfail states.
+
+Module Changes
+==============
+
+- The :py:func:`debian_ip <salt.modules.debian_ip>` module used by the
+  :py:func:`network.managed <salt.states.network.managed>` state has been
+  heavily refactored. The order that options appear in inet/inet6 blocks may
+  produce cosmetic changes. Many options without an 'ipvX' prefix will now be
+  shared between inet and inet6 blocks. The options ``enable_ipv4`` and
+  ``enabled_ipv6`` will now fully remove relevant inet/inet6 blocks. Overriding
+  options by prefixing them with 'ipvX' will now work with most options (i.e.
+  ``dns`` can be overriden by ``ipv4dns`` or ``ipv6dns``). The ``proto`` option
+  is now required.
+
+- Added new :py:func:`boto_ssm <salt.modules.boto_ssm>` module to set and query
+  secrets in AWS SSM parameters.
+
+- Added new :py:func:`flatpak <salt.modules.flatpak>` module to work with flatpak packages.
+  
+- The :py:func:`file.set_selinux_context <salt.modules.file.set_selinux_context>`
+  module now supports perstant changes with ``persist=True`` by calling the
+  :py:func:`selinux.fcontext_add_policy <salt.modules.selinux.fcontext_add_policy>` module.
+
+- The :py:func:`file.remove <salt.modules.file.remove>` module was
+  fixed to remove file system entries other than files, directories
+  and symbolic links properly.
+
+- The :py:func:`yumpkg <salt.modules.yumpkg>` module has been updated to support
+  VMWare's Photon OS, which uses tdnf (a C implementation of dnf).
+
+- The :py:func:`chocolatey.bootstrap <salt.modules.chocolatey.bootstrap>` function
+  has been updated to support offline installation.
+
+- The :py:func:`chocolatey.unbootstrap <salt.modules.chocolatey.unbootstrap>` function
+  has been added to uninstall Chocolatey.
+
+Runner Changes
+==============
+
+- The :py:func:`saltutil.sync_auth <salt.runners.saltutil.sync_auth>` function
+  has been added to sync loadable auth modules. :py:func:`saltutil.sync_all <salt.runners.saltutil.sync_all>`
+  will also include these modules.
+
+Util Changes
+============
+
+- The :py:func:`win_dotnet <salt.utils.win_dotnet>` Salt util has been added to
+  make it easier to detect the versions of .NET installed on the system. It includes
+  the following functions:
+
+    - :py:func:`versions <salt.utils.win_dotnet.versions>`
+    - :py:func:`versions_list <salt.utils.win_dotnet.versions_list>`
+    - :py:func:`versions_details <salt.utils.win_dotnet.versions_details>`
+    - :py:func:`version_at_least <salt.utils.win_dotnet.version_at_least>`
+
+Serializer Changes
+==================
+
+- The configparser serializer and deserializer functions can now be made to preserve
+  case of item names by passing 'preserve_case=True' in the options parameter of the function.
+
+  .. note::
+      This is a parameter consumed only by the salt.serializer.configparser serialize and
+      deserialize functions and not the low-level configparser python object.
+
+  For example, in a file.serialze state:
+
+  .. code-block:: yaml
+
+    some.ini:
+      - file.serialize:
+         - formatter: configparser
+         - merge_if_exists: True
+         - deserializer_opts:
+           - preserve_case: True
+         - serializer_opts:
+           - preserve_case: True
+
 Enhancements to Engines
 =======================
 

salt/modules/file.py

@@ -3775,6 +3775,10 @@ def remove(path):
     .. code-block:: bash
 
         salt '*' file.remove /tmp/foo
+
+    .. versionchanged:: Neon
+        The method now works on all types of file system entries, not just
+        files, directories and symlinks.
     '''
     path = os.path.expanduser(path)
 
@@ -3782,7 +3786,7 @@ def remove(path):
         raise SaltInvocationError('File path must be absolute: {0}'.format(path))
 
     try:
-        if os.path.isfile(path) or os.path.islink(path):
+        if os.path.islink(path) or (os.path.exists(path) and not os.path.isdir(path)):
             os.remove(path)
             return True
         elif os.path.isdir(path):

salt/states/file.py

@@ -1609,10 +1609,10 @@ def symlink(
     Create a symbolic link (symlink, soft link)
 
     If the file already exists and is a symlink pointing to any location other
-    than the specified target, the symlink will be replaced. If the symlink is
-    a regular file or directory then the state will return False. If the
-    regular file or directory is desired to be replaced with a symlink pass
-    force: True, if it is to be renamed, pass a backupname.
+    than the specified target, the symlink will be replaced. If an entry with
+    the same name exists then the state will return False. If the existing
+    entry is desired to be replaced with a symlink pass force: True, if it is
+    to be renamed, pass a backupname.
 
     name
         The location of the symlink to create
@@ -1623,10 +1623,14 @@ def symlink(
     force
         If the name of the symlink exists and is not a symlink and
         force is set to False, the state will fail. If force is set to
-        True, the file or directory in the way of the symlink file
+        True, the existing entry in the way of the symlink file
         will be deleted to make room for the symlink, unless
         backupname is set, when it will be renamed
 
+        .. versionchanged:: Neon
+            Force will now remove all types of existing file system entries,
+            not just files, directories and symlinks.
+
     backupname
         If the name of the symlink exists and is not a symlink, it will be
         renamed to the backupname. If the backupname already
@@ -1845,8 +1849,8 @@ def symlink(
                             '{1}:{2}'.format(name, user, group))
             return ret
 
-    elif os.path.isfile(name) or os.path.isdir(name):
-        # It is not a link, but a file or dir
+    elif os.path.exists(name):
+        # It is not a link, but a file, dir, socket, FIFO etc.
         if backupname is not None:
             if not os.path.isabs(backupname):
                 if backupname == os.path.basename(backupname):
@@ -1883,14 +1887,12 @@ def symlink(
                 __salt__['file.remove'](name)
         else:
             # Otherwise throw an error
-            if os.path.isfile(name):
-                return _error(ret,
-                              ('File exists where the symlink {0} should be'
-                               .format(name)))
-            else:
-                return _error(ret, ((
-                                        'Directory exists where the symlink {0} should be'
-                                    ).format(name)))
+            fs_entry_type = 'File' if os.path.isfile(name) else \
+                'Directory' if os.path.isdir(name) else \
+                'File system entry'
+            return _error(ret,
+                          ('{0} exists where the symlink {1} should be'
+                           .format(fs_entry_type, name)))
 
     if not os.path.exists(name):
         # The link is not present, make it

tests/unit/states/test_file.py

@@ -286,7 +286,7 @@ class TestFileState(TestCase, LoaderModuleMockMixin):
                                              'user.current': mock_user}),\
                 patch.dict(filestate.__opts__, {'test': False}),\
                 patch.object(os.path, 'isdir', mock_t),\
-                patch.object(os.path, 'exists', mock_f),\
+                patch.object(os.path, 'exists', mock_t),\
                 patch.object(os.path, 'lexists', mock_t),\
                 patch('salt.utils.win_functions.get_sid_from_name', return_value='test-sid'):
             comt = 'Symlink & backup dest exists and Force not set. {0} -> ' \
@@ -307,6 +307,7 @@ class TestFileState(TestCase, LoaderModuleMockMixin):
                                              'user.info': mock_empty,
                                              'user.current': mock_user}),\
                 patch.dict(filestate.__opts__, {'test': False}),\
+                patch.object(os.path, 'exists', mock_t),\
                 patch.object(os.path, 'isfile', mock_t), \
                 patch.object(os.path, 'isdir', mock_t),\
                 patch('salt.utils.win_functions.get_sid_from_name', return_value='test-sid'):
@@ -327,7 +328,7 @@ class TestFileState(TestCase, LoaderModuleMockMixin):
                                              'user.current': mock_user}),\
                 patch.dict(filestate.__opts__, {'test': False}),\
                 patch.object(os.path, 'isdir', mock_t),\
-                patch.object(os.path, 'exists', mock_f),\
+                patch.object(os.path, 'exists', mock_t),\
                 patch.object(os.path, 'isfile', mock_t),\
                 patch('salt.utils.win_functions.get_sid_from_name', return_value='test-sid'):
             comt = 'File exists where the symlink {0} should be'.format(name)
@@ -349,7 +350,7 @@ class TestFileState(TestCase, LoaderModuleMockMixin):
                 patch.dict(filestate.__opts__, {'test': False}),\
                 patch.object(os.path, 'isdir', MagicMock(side_effect=[True, False])),\
                 patch.object(os.path, 'isdir', mock_t),\
-                patch.object(os.path, 'exists', mock_f),\
+                patch.object(os.path, 'exists', mock_t),\
                 patch('salt.utils.win_functions.get_sid_from_name', return_value='test-sid'):
             comt = 'Directory exists where the symlink {0} should be'.format(name)
             ret = return_val({'comment': comt,