fixes saltstack/salt#39292 fix copying superopts to bind mounts

changelog/39292.fixed

@@ -0,0 +1 @@
+Fix mounted bind mounts getting active mount options added

salt/states/mount.py

@@ -70,6 +70,7 @@ def mounted(
     extra_mount_ignore_fs_keys=None,
     extra_mount_translate_options=None,
     hidden_opts=None,
+    bind_mount_copy_active_opts=True,
     **kwargs
 ):
     """
@@ -186,6 +187,12 @@ def mounted(
         as part of the state application
 
         .. versionadded:: 2015.8.2
+
+    bind_mount_copy_active_opts
+        If set to ``False``, this option disables the default behavior of
+        copying the options from the bind mount if it was found to be active.
+
+        .. versionadded:: 3006.0
     """
     ret = {"name": name, "changes": {}, "result": True, "comment": ""}
 
@@ -242,13 +249,14 @@ def mounted(
                 while True:
                     if _device in active:
                         _real_device = active[_device]["device"]
-                        opts = list(
+                        if bind_mount_copy_active_opts:
-                            set(
+                            opts = sorted(
-                                opts
+                                set(
-                                + active[_device]["opts"]
+                                    opts
-                                + active[_device]["superopts"]
+                                    + active[_device]["opts"]
+                                    + active[_device]["superopts"]
+                                )
                             )
-                        )
                         active[real_name]["opts"].append("bind")
                         break
                     _device = os.path.dirname(_device)
@@ -256,13 +264,14 @@ def mounted(
             else:
                 # Remote file systems act differently.
                 if _device in active:
-                    opts = list(
+                    if bind_mount_copy_active_opts:
-                        set(
+                        opts = sorted(
-                            opts
+                            set(
-                            + active[_device]["opts"]
+                                opts
-                            + active[_device]["superopts"]
+                                + active[_device]["opts"]
+                                + active[_device]["superopts"]
+                            )
                         )
-                    )
                     active[real_name]["opts"].append("bind")
                 real_device = active[real_name]["device"]
         else:
@@ -407,6 +416,7 @@ def mounted(
                 if extra_mount_translate_options:
                     mount_translate_options.update(extra_mount_translate_options)
 
+                trigger_remount = []
                 for opt in opts:
                     if opt in mount_translate_options:
                         opt = mount_translate_options[opt]
@@ -460,58 +470,65 @@ def mounted(
                         and opt not in mount_ignore_fs_keys.get(fstype, [])
                         and opt not in mount_invisible_keys
                     ):
-                        if __opts__["test"]:
+                        trigger_remount.append(opt)
-                            ret["result"] = None
+
-                            ret[
+                if trigger_remount:
-                                "comment"
+                    if __opts__["test"]:
-                            ] = "Remount would be forced because options ({}) changed".format(
+                        ret["result"] = None
-                                opt
+                        ret[
+                            "comment"
+                        ] = "Remount would be forced because options ({}) changed".format(
+                            ",".join(sorted(trigger_remount))
+                        )
+                        return ret
+                    else:
+                        # Some file systems require umounting and mounting if options change
+                        # add others to list that require similiar functionality
+                        if fstype in ["nfs", "cvfs"] or fstype.startswith("fuse"):
+                            ret["changes"]["umount"] = (
+                                "Forced unmount and mount because "
+                                + "options ({}) changed".format(
+                                    ",".join(sorted(trigger_remount))
+                                )
                             )
-                            return ret
+                            unmount_result = __salt__["mount.umount"](real_name)
-                        else:
+                            if unmount_result is True:
-                            # Some file systems require umounting and mounting if options change
+                                mount_result = __salt__["mount.mount"](
-                            # add others to list that require similiar functionality
-                            if fstype in ["nfs", "cvfs"] or fstype.startswith("fuse"):
-                                ret["changes"]["umount"] = (
-                                    "Forced unmount and mount because "
-                                    + "options ({}) changed".format(opt)
-                                )
-                                unmount_result = __salt__["mount.umount"](real_name)
-                                if unmount_result is True:
-                                    mount_result = __salt__["mount.mount"](
-                                        real_name,
-                                        device,
-                                        mkmnt=mkmnt,
-                                        fstype=fstype,
-                                        opts=opts,
-                                    )
-                                    ret["result"] = mount_result
-                                else:
-                                    ret["result"] = False
-                                    ret["comment"] = "Unable to unmount {}: {}.".format(
-                                        real_name, unmount_result
-                                    )
-                                    return ret
-                            else:
-                                ret["changes"]["umount"] = (
-                                    "Forced remount because "
-                                    + "options ({}) changed".format(opt)
-                                )
-                                remount_result = __salt__["mount.remount"](
                                     real_name,
                                     device,
                                     mkmnt=mkmnt,
                                     fstype=fstype,
                                     opts=opts,
                                 )
-                                ret["result"] = remount_result
+                                ret["result"] = mount_result
-                                # Cleanup after the remount, so we
+                            else:
-                                # don't write remount into fstab
+                                ret["result"] = False
-                                if "remount" in opts:
+                                ret["comment"] = "Unable to unmount {}: {}.".format(
-                                    opts.remove("remount")
+                                    real_name, unmount_result
+                                )
+                                return ret
+                        else:
+                            ret["changes"]["umount"] = (
+                                "Forced remount because "
+                                + "options ({}) changed".format(
+                                    ",".join(sorted(trigger_remount))
+                                )
+                            )
+                            remount_result = __salt__["mount.remount"](
+                                real_name,
+                                device,
+                                mkmnt=mkmnt,
+                                fstype=fstype,
+                                opts=opts,
+                            )
+                            ret["result"] = remount_result
+                            # Cleanup after the remount, so we
+                            # don't write remount into fstab
+                            if "remount" in opts:
+                                opts.remove("remount")
 
-                            # Update the cache
+                        # Update the cache
-                            update_mount_cache = True
+                        update_mount_cache = True
 
                 mount_cache = __salt__["mount.read_mount_cache"](real_name)
                 if "opts" in mount_cache:

tests/pytests/unit/states/test_mount.py

@@ -1289,3 +1289,143 @@ def test_fstab_absent_absent():
     ), patch.dict(mount.__salt__, salt_mock):
         assert mount.fstab_absent("/dev/sda1", "/home") == ret
         salt_mock["mount.fstab"].assert_called_with("/etc/fstab")
+
+
+def test_bind_mount_copy_active_opts():
+    name = "/home/tmp"
+    device = "/home/tmp"
+    fstype = "none"
+    opts = [
+        "bind",
+        "nodev",
+        "noexec",
+        "nosuid",
+        "rw",
+    ]
+
+    ret = {"name": name, "result": None, "comment": "", "changes": {}}
+
+    mock_active = MagicMock(
+        return_value={
+            "/home/tmp": {
+                "alt_device": "/dev/vda1",
+                "device": "/dev/vda1",
+                "device_label": None,
+                "device_uuid": "b4e712d1-cd94-4b7c-97cd-294d3db80ec6",
+                "fstype": "ext4",
+                "major": "254",
+                "minor": "1",
+                "mountid": "105",
+                "opts": ["rw", "relatime"],
+                "parentid": "25",
+                "root": "/home/tmp",
+                "superopts": ["rw", "discard", "errors=remount-ro"],
+            },
+        }
+    )
+    mock_read_mount_cache = MagicMock(
+        return_value={
+            "device": "/home/tmp",
+            "fstype": "none",
+            "mkmnt": False,
+            "opts": ["bind", "nodev", "noexec", "nosuid", "rw"],
+        }
+    )
+    mock_set_fstab = MagicMock(return_value="new")
+
+    with patch.dict(mount.__grains__, {"os": "CentOS"}), patch.dict(
+        mount.__salt__,
+        {
+            "mount.active": mock_active,
+            "mount.read_mount_cache": mock_read_mount_cache,
+            "mount.remount": MagicMock(return_value=True),
+            "mount.set_fstab": mock_set_fstab,
+            "mount.write_mount_cache": MagicMock(return_value=True),
+        },
+    ), patch.object(
+        os.path,
+        "realpath",
+        MagicMock(
+            side_effect=[
+                "/home/tmp",
+                "/dev/vda1",
+                "/home/tmp",
+                "/dev/vda1",
+                "/home/tmp",
+                "/dev/vda1",
+            ]
+        ),
+    ):
+        with patch.dict(mount.__opts__, {"test": True}):
+            ret[
+                "comment"
+            ] = "Remount would be forced because options (nodev,noexec,nosuid) changed"
+            result = mount.mounted(
+                name=name,
+                device=device,
+                fstype=fstype,
+                opts=opts,
+                persist=True,
+                bind_mount_copy_active_opts=False,
+            )
+            assert result == ret
+
+        with patch.dict(mount.__opts__, {"test": False}):
+            ret["comment"] = "Target was already mounted. Added new entry to the fstab."
+            ret["changes"] = {
+                "persist": "new",
+                "umount": "Forced remount because options (nodev,noexec,nosuid) changed",
+            }
+            ret["result"] = True
+
+            # bind_mount_copy_active_opts is off
+            result = mount.mounted(
+                name=name,
+                device=device,
+                fstype=fstype,
+                opts=opts,
+                persist=True,
+                bind_mount_copy_active_opts=False,
+            )
+            assert result == ret
+
+            mock_set_fstab.assert_called_with(
+                name,
+                device,
+                fstype,
+                ["bind", "nodev", "noexec", "nosuid", "rw"],
+                0,
+                0,
+                "/etc/fstab",
+                match_on="auto",
+            )
+
+            # bind_mount_copy_active_opts is on (default)
+            result = mount.mounted(
+                name=name,
+                device=device,
+                fstype=fstype,
+                opts=opts,
+                persist=True,
+            )
+            assert result == ret
+
+            mock_set_fstab.assert_called_with(
+                name,
+                device,
+                fstype,
+                [
+                    "bind",
+                    "discard",
+                    "errors=remount-ro",
+                    "nodev",
+                    "noexec",
+                    "nosuid",
+                    "relatime",
+                    "rw",
+                ],
+                0,
+                0,
+                "/etc/fstab",
+                match_on="auto",
+            )