fixes saltstack/salt#54508 fix mount.mounted does not handle blanks properly

changelog/54508.fixed

@@ -0,0 +1 @@
+Fix mount.mounted does not handle blanks properly

salt/modules/mount.py

@@ -1312,9 +1312,9 @@ def mount(
 
     cmd = "mount "
     if device:
-        cmd += "{} {} {} ".format(args, device, name)
+        cmd += "{} '{}' '{}' ".format(args, device, name)
     else:
-        cmd += "{} ".format(name)
+        cmd += "'{}' ".format(name)
     out = __salt__["cmd.run_all"](cmd, runas=user, python_shell=False)
     if out["retcode"]:
         return out["stderr"]
@@ -1374,9 +1374,9 @@ def remount(name, device, mkmnt=False, fstype="", opts="defaults", user=None):
                 args += " -t {}".format(fstype)
 
         if __grains__["os"] not in ["OpenBSD", "MacOS", "Darwin"] or force_mount:
-            cmd = "mount {} {} {} ".format(args, device, name)
+            cmd = "mount {} '{}' '{}' ".format(args, device, name)
         else:
-            cmd = "mount -u {} {} {} ".format(args, device, name)
+            cmd = "mount -u {} '{}' '{}' ".format(args, device, name)
         out = __salt__["cmd.run_all"](cmd, runas=user, python_shell=False)
         if out["retcode"]:
             return out["stderr"]
@@ -1416,9 +1416,9 @@ def umount(name, device=None, user=None, util="mount"):
         return "{} does not have anything mounted".format(name)
 
     if not device:
-        cmd = "umount {}".format(name)
+        cmd = "umount '{}'".format(name)
     else:
-        cmd = "umount {}".format(device)
+        cmd = "umount '{}'".format(device)
     out = __salt__["cmd.run_all"](cmd, runas=user, python_shell=False)
     if out["retcode"]:
         return out["stderr"]
@@ -1535,11 +1535,11 @@ def swapon(name, priority=None):
 
     if __grains__["kernel"] == "SunOS":
         if __grains__["virtual"] != "zone":
-            __salt__["cmd.run"]("swap -a {}".format(name), python_shell=False)
+            __salt__["cmd.run"]("swap -a '{}'".format(name), python_shell=False)
         else:
             return False
     else:
-        cmd = "swapon {}".format(name)
+        cmd = "swapon '{}'".format(name)
         if priority and "AIX" not in __grains__["kernel"]:
             cmd += " -p {}".format(priority)
         __salt__["cmd.run"](cmd, python_shell=False)
@@ -1569,13 +1569,13 @@ def swapoff(name):
     if name in on_:
         if __grains__["kernel"] == "SunOS":
             if __grains__["virtual"] != "zone":
-                __salt__["cmd.run"]("swap -a {}".format(name), python_shell=False)
+                __salt__["cmd.run"]("swap -a '{}'".format(name), python_shell=False)
             else:
                 return False
         elif __grains__["os"] != "OpenBSD":
-            __salt__["cmd.run"]("swapoff {}".format(name), python_shell=False)
+            __salt__["cmd.run"]("swapoff '{}'".format(name), python_shell=False)
         else:
-            __salt__["cmd.run"]("swapctl -d {}".format(name), python_shell=False)
+            __salt__["cmd.run"]("swapctl -d '{}'".format(name), python_shell=False)
         on_ = swaps()
         if name in on_:
             return False

salt/states/mount.py

@@ -241,10 +241,12 @@ def mounted(
     # Get the active data
     active = __salt__["mount.active"](extended=True)
     real_name = os.path.realpath(name)
+    # real_name for comparisons to the active mount list
+    comp_real_name = real_name.replace(" ", "\\040")
     if device.startswith("/"):
-        if "bind" in opts and real_name in active:
-            _device = device
-            if active[real_name]["device"].startswith("/"):
+        if "bind" in opts and comp_real_name in active:
+            _device = device.replace(" ", "\\040")
+            if active[comp_real_name]["device"].startswith("/"):
                 # Find the device that the bind really points at.
                 while True:
                     if _device in active:
@@ -257,9 +259,9 @@ def mounted(
                                     + active[_device]["superopts"]
                                 )
                             )
-                        active[real_name]["opts"].append("bind")
+                        active[comp_real_name]["opts"].append("bind")
                         break
-                    _device = os.path.dirname(_device)
+                    _device = os.path.dirname(_device.replace("\\040", " "))
                 real_device = _real_device
             else:
                 # Remote file systems act differently.
@@ -272,8 +274,8 @@ def mounted(
                                 + active[_device]["superopts"]
                             )
                         )
-                    active[real_name]["opts"].append("bind")
-                real_device = active[real_name]["device"]
+                    active[comp_real_name]["opts"].append("bind")
+                real_device = active[comp_real_name]["device"]
         else:
             real_device = os.path.realpath(device)
     elif device.upper().startswith("UUID="):
@@ -324,25 +326,25 @@ def mounted(
         if "device_name" in fuse_match.groupdict():
             real_device = fuse_match.group("device_name")
 
-    if real_name in active:
-        if "superopts" not in active[real_name]:
-            active[real_name]["superopts"] = []
+    if comp_real_name in active:
+        if "superopts" not in active[comp_real_name]:
+            active[comp_real_name]["superopts"] = []
         if mount:
-            device_list.append(active[real_name]["device"])
+            device_list.append(active[comp_real_name]["device"])
             device_list.append(os.path.realpath(device_list[0]))
             alt_device = (
-                active[real_name]["alt_device"]
-                if "alt_device" in active[real_name]
+                active[comp_real_name]["alt_device"]
+                if "alt_device" in active[comp_real_name]
                 else None
             )
             uuid_device = (
-                active[real_name]["device_uuid"]
-                if "device_uuid" in active[real_name]
+                active[comp_real_name]["device_uuid"]
+                if "device_uuid" in active[comp_real_name]
                 else None
             )
             label_device = (
-                active[real_name]["device_label"]
-                if "device_label" in active[real_name]
+                active[comp_real_name]["device_label"]
+                if "device_label" in active[comp_real_name]
                 else None
             )
             if alt_device and alt_device not in device_list:
@@ -450,7 +452,7 @@ def mounted(
                                 _id = _info[_param]
                         opt = _param + "=" + str(_id)
 
-                    _active_superopts = active[real_name].get("superopts", [])
+                    _active_superopts = active[comp_real_name].get("superopts", [])
                     for _active_opt in _active_superopts:
                         size_match = re.match(
                             r"size=(?P<size_value>[0-9]+)(?P<size_unit>k|m|g)",
@@ -464,7 +466,7 @@ def mounted(
                             _active_superopts.append(_active_opt)
 
                     if (
-                        opt not in active[real_name]["opts"]
+                        opt not in active[comp_real_name]["opts"]
                         and opt not in _active_superopts
                         and opt not in mount_invisible_options
                         and opt not in mount_ignore_fs_keys.get(fstype, [])
@@ -622,7 +624,7 @@ def mounted(
                     ret["changes"]["umount"] += ", current: " + ", ".join(device_list)
                     out = __salt__["mount.umount"](real_name, user=user)
                     active = __salt__["mount.active"](extended=True)
-                    if real_name in active:
+                    if comp_real_name in active:
                         ret["comment"] = "Unable to unmount"
                         ret["result"] = None
                         return ret
@@ -630,7 +632,7 @@ def mounted(
             else:
                 ret["comment"] = "Target was already mounted"
     # using a duplicate check so I can catch the results of a umount
-    if real_name not in active:
+    if comp_real_name not in active:
         if mount:
             # The mount is not present! Mount it
             if __opts__["test"]:
@@ -658,7 +660,7 @@ def mounted(
                 ret["comment"] = out
                 ret["result"] = False
                 return ret
-            elif real_name in active:
+            elif comp_real_name in active:
                 # (Re)mount worked!
                 ret["comment"] = "Target was successfully mounted"
                 ret["changes"]["mount"] = True
@@ -939,10 +941,11 @@ def unmounted(
 
     # Get the active data
     active = __salt__["mount.active"](extended=True)
-    if name not in active:
+    comp_name = name.replace(" ", "\\040")
+    if comp_name not in active:
         # Nothing to unmount
         ret["comment"] = "Target was already unmounted"
-    if name in active:
+    if comp_name in active:
         # The mount is present! Unmount it
         if __opts__["test"]:
             ret["result"] = None

tests/pytests/unit/modules/test_mount.py

@@ -473,13 +473,13 @@ def test_mount():
                 with patch.dict(mount.__salt__, {"cmd.run_all": mock}):
                     assert mount.mount("name", "device")
                     mock.assert_called_with(
-                        "mount  device name ", python_shell=False, runas=None
+                        "mount  'device' 'name' ", python_shell=False, runas=None
                     )
 
                 with patch.dict(mount.__salt__, {"cmd.run_all": mock}):
                     assert mount.mount("name", "device", fstype="fstype")
                     mock.assert_called_with(
-                        "mount  -t fstype device name ",
+                        "mount  -t fstype 'device' 'name' ",
                         python_shell=False,
                         runas=None,
                     )
@@ -497,13 +497,13 @@ def test_mount():
                 with patch.dict(mount.__salt__, {"cmd.run_all": mock}):
                     assert mount.mount("name", "device")
                     mock.assert_called_with(
-                        "mount  device name ", python_shell=False, runas=None
+                        "mount  'device' 'name' ", python_shell=False, runas=None
                     )
 
                 with patch.dict(mount.__salt__, {"cmd.run_all": mock}):
                     assert mount.mount("name", "device", fstype="fstype")
                     mock.assert_called_with(
-                        "mount  -v fstype device name ",
+                        "mount  -v fstype 'device' 'name' ",
                         python_shell=False,
                         runas=None,
                     )
@@ -521,7 +521,7 @@ def test_mount():
                 with patch.dict(mount.__salt__, {"cmd.run_all": mock}):
                     assert mount.mount("name", "device")
                     mock.assert_called_with(
-                        "mount -o defaults device name ",
+                        "mount -o defaults 'device' 'name' ",
                         python_shell=False,
                         runas=None,
                     )
@@ -529,7 +529,7 @@ def test_mount():
                 with patch.dict(mount.__salt__, {"cmd.run_all": mock}):
                     assert mount.mount("name", "device", fstype="fstype")
                     mock.assert_called_with(
-                        "mount -o defaults -t fstype device name ",
+                        "mount -o defaults -t fstype 'device' 'name' ",
                         python_shell=False,
                         runas=None,
                     )
@@ -578,7 +578,7 @@ def test_remount_already_mounted_no_fstype():
             with patch.dict(mount.__salt__, {"cmd.run_all": mock}):
                 assert mount.remount("name", "device")
                 mock.assert_called_with(
-                    "mount -u -o noowners device name ",
+                    "mount -u -o noowners 'device' 'name' ",
                     python_shell=False,
                     runas=None,
                 )
@@ -590,7 +590,7 @@ def test_remount_already_mounted_no_fstype():
             with patch.dict(mount.__salt__, {"cmd.run_all": mock}):
                 assert mount.remount("name", "device")
                 mock.assert_called_with(
-                    "mount -o remount device name ", python_shell=False, runas=None
+                    "mount -o remount 'device' 'name' ", python_shell=False, runas=None
                 )
 
     with patch.dict(mount.__grains__, {"os": "Linux"}):
@@ -600,7 +600,7 @@ def test_remount_already_mounted_no_fstype():
             with patch.dict(mount.__salt__, {"cmd.run_all": mock}):
                 assert mount.remount("name", "device")
                 mock.assert_called_with(
-                    "mount -o defaults,remount device name ",
+                    "mount -o defaults,remount 'device' 'name' ",
                     python_shell=False,
                     runas=None,
                 )
@@ -618,7 +618,7 @@ def test_remount_already_mounted_with_fstype():
             with patch.dict(mount.__salt__, {"cmd.run_all": mock}):
                 assert mount.remount("name", "device", fstype="type")
                 mock.assert_called_with(
-                    "mount -u -o noowners -t type device name ",
+                    "mount -u -o noowners -t type 'device' 'name' ",
                     python_shell=False,
                     runas=None,
                 )
@@ -630,7 +630,7 @@ def test_remount_already_mounted_with_fstype():
             with patch.dict(mount.__salt__, {"cmd.run_all": mock}):
                 assert mount.remount("name", "device", fstype="type")
                 mock.assert_called_with(
-                    "mount -o remount -v type device name ",
+                    "mount -o remount -v type 'device' 'name' ",
                     python_shell=False,
                     runas=None,
                 )
@@ -642,7 +642,7 @@ def test_remount_already_mounted_with_fstype():
             with patch.dict(mount.__salt__, {"cmd.run_all": mock}):
                 assert mount.remount("name", "device", fstype="type")
                 mock.assert_called_with(
-                    "mount -o defaults,remount -t type device name ",
+                    "mount -o defaults,remount -t type 'device' 'name' ",
                     python_shell=False,
                     runas=None,
                 )
@@ -681,30 +681,24 @@ def test_is_fuse_exec():
     with patch.object(salt.utils.path, "which", return_value=None):
         assert not mount.is_fuse_exec("cmd")
 
-    def _ldd_side_effect(cmd, *args, **kwargs):
-        """
-        Neither of these are full ldd output, but what is_fuse_exec is
-        looking for is 'libfuse' in the ldd output, so these examples
-        should be sufficient enough to test both the True and False cases.
-        """
-        return {
-            "ldd cmd1": textwrap.dedent(
+    which_mock = MagicMock(side_effect=lambda x: x)
+    ldd_mock = MagicMock(
+        side_effect=[
+            textwrap.dedent(
                 """\
                 linux-vdso.so.1 (0x00007ffeaf5fb000)
                 libfuse3.so.3 => /usr/lib/libfuse3.so.3 (0x00007f91e66ac000)
                 """
             ),
-            "ldd cmd2": textwrap.dedent(
+            textwrap.dedent(
                 """\
                 linux-vdso.so.1 (0x00007ffeaf5fb000)
                 """
             ),
-        }[cmd]
-
-    which_mock = MagicMock(side_effect=lambda x: x)
-    ldd_mock = MagicMock(side_effect=_ldd_side_effect)
+        ]
+    )
     with patch.object(salt.utils.path, "which", which_mock):
-        with patch.dict(mount.__salt__, {"cmd.run": _ldd_side_effect}):
+        with patch.dict(mount.__salt__, {"cmd.run": ldd_mock}):
             assert mount.is_fuse_exec("cmd1")
             assert not mount.is_fuse_exec("cmd2")
 

tests/pytests/unit/states/test_mount.py

@@ -1291,9 +1291,11 @@ def test_fstab_absent_absent():
         salt_mock["mount.fstab"].assert_called_with("/etc/fstab")
 
 
-def test_bind_mount_copy_active_opts():
-    name = "/home/tmp"
-    device = "/home/tmp"
+@pytest.mark.parametrize("mount_name", ["/home/tmp", "/home/tmp with spaces"])
+def test_bind_mount_copy_active_opts(mount_name):
+    name = mount_name
+    device = name
+    active_name = name.replace(" ", "\\040")
     fstype = "none"
     opts = [
         "bind",
@@ -1307,7 +1309,7 @@ def test_bind_mount_copy_active_opts():
 
     mock_active = MagicMock(
         return_value={
-            "/home/tmp": {
+            active_name: {
                 "alt_device": "/dev/vda1",
                 "device": "/dev/vda1",
                 "device_label": None,
@@ -1318,14 +1320,14 @@ def test_bind_mount_copy_active_opts():
                 "mountid": "105",
                 "opts": ["rw", "relatime"],
                 "parentid": "25",
-                "root": "/home/tmp",
+                "root": active_name,
                 "superopts": ["rw", "discard", "errors=remount-ro"],
             },
         }
     )
     mock_read_mount_cache = MagicMock(
         return_value={
-            "device": "/home/tmp",
+            "device": device,
             "fstype": "none",
             "mkmnt": False,
             "opts": ["bind", "nodev", "noexec", "nosuid", "rw"],
@@ -1347,11 +1349,11 @@ def test_bind_mount_copy_active_opts():
         "realpath",
         MagicMock(
             side_effect=[
-                "/home/tmp",
+                name,
                 "/dev/vda1",
-                "/home/tmp",
+                name,
                 "/dev/vda1",
-                "/home/tmp",
+                name,
                 "/dev/vda1",
             ]
         ),