From 6e1277ae4c6000de0516dc0c66f67cf31a0fd12e Mon Sep 17 00:00:00 2001 From: nicholasmhughes Date: Mon, 29 Aug 2022 12:32:33 -0400 Subject: [PATCH 1/6] fixes saltstack/salt#62556 mount.mounted options are order specific for persist --- changelog/62556.fixed | 1 + salt/modules/mount.py | 24 +++++++++++++++++++++--- salt/states/mount.py | 4 ---- tests/pytests/unit/modules/test_mount.py | 12 ++++++++++++ 4 files changed, 34 insertions(+), 7 deletions(-) create mode 100644 changelog/62556.fixed diff --git a/changelog/62556.fixed b/changelog/62556.fixed new file mode 100644 index 000000000000..0dc3a7933ec2 --- /dev/null +++ b/changelog/62556.fixed @@ -0,0 +1 @@ +Fix order specific mount.mounted options for persist diff --git a/salt/modules/mount.py b/salt/modules/mount.py index af1135a3fdad..54a8f6cb84f3 100644 --- a/salt/modules/mount.py +++ b/salt/modules/mount.py @@ -368,7 +368,12 @@ def match(self, line): """ entry = self.dict_from_line(line) for key, value in self.criteria.items(): - if entry[key] != value: + if key == "opts": + ex_opts = sorted(entry.get(key, "").split(",")) + cr_opts = sorted(value.split(",")) + if ex_opts != cr_opts: + return False + elif entry[key] != value: return False return True @@ -467,7 +472,12 @@ def match(self, line): """ entry = self.dict_from_line(line) for key, value in self.criteria.items(): - if entry[key] != value: + if key == "opts": + ex_opts = sorted(entry.get(key, "").split(",")) + cr_opts = sorted(value.split(",")) + if ex_opts != cr_opts: + return False + elif entry[key] != value: return False return True @@ -628,7 +638,12 @@ def match(self, fsys_view): evalue_dict = fsys_view[1] for key, value in self.criteria.items(): if key in evalue_dict: - if evalue_dict[key] != value: + if key == "opts": + ex_opts = sorted(evalue_dict.get(key, "").split(",")) + cr_opts = sorted(value.split(",")) + if ex_opts != cr_opts: + return False + elif evalue_dict[key] != value: return False else: return False @@ -867,6 +882,9 @@ def set_fstab( line = salt.utils.stringutils.to_unicode(line) try: if criteria.match(line): + log.debug( + "Checking ( %s ) against current line ( %s )", entry, line + ) # Note: If ret isn't None here, # we've matched multiple lines ret = "present" diff --git a/salt/states/mount.py b/salt/states/mount.py index a7af284c6790..c6657a801148 100644 --- a/salt/states/mount.py +++ b/salt/states/mount.py @@ -222,8 +222,6 @@ def mounted( # string if isinstance(opts, str): opts = opts.split(",") - if opts: - opts.sort() if isinstance(hidden_opts, str): hidden_opts = hidden_opts.split(",") @@ -345,8 +343,6 @@ def mounted( if label_device and label_device not in device_list: device_list.append(label_device) if opts: - opts.sort() - mount_invisible_options = [ "_netdev", "actimeo", diff --git a/tests/pytests/unit/modules/test_mount.py b/tests/pytests/unit/modules/test_mount.py index 4e233ec63c6b..784b6b411dd2 100644 --- a/tests/pytests/unit/modules/test_mount.py +++ b/tests/pytests/unit/modules/test_mount.py @@ -137,6 +137,18 @@ def test_active(): assert mount.active() == {} +def test_fstab_entry_ignores_opt_ordering(): + entry = mount._fstab_entry( + name="/tmp", + device="tmpfs", + fstype="tmpfs", + opts="defaults,nodev,noexec", + dump=0, + pass_num=0, + ) + assert entry.match("tmpfs\t\t/tmp\ttmpfs\tnodev,defaults,noexec\t0 0\n") + + def test_fstab(): """ List the content of the fstab From 726615887abf52dd87e042ec53d846d2569cc983 Mon Sep 17 00:00:00 2001 From: nicholasmhughes Date: Mon, 29 Aug 2022 13:02:06 -0400 Subject: [PATCH 2/6] fix indent on test --- tests/pytests/unit/modules/test_mount.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/pytests/unit/modules/test_mount.py b/tests/pytests/unit/modules/test_mount.py index 784b6b411dd2..c48886c24b7b 100644 --- a/tests/pytests/unit/modules/test_mount.py +++ b/tests/pytests/unit/modules/test_mount.py @@ -138,15 +138,15 @@ def test_active(): def test_fstab_entry_ignores_opt_ordering(): - entry = mount._fstab_entry( - name="/tmp", - device="tmpfs", - fstype="tmpfs", - opts="defaults,nodev,noexec", - dump=0, - pass_num=0, - ) - assert entry.match("tmpfs\t\t/tmp\ttmpfs\tnodev,defaults,noexec\t0 0\n") + entry = mount._fstab_entry( + name="/tmp", + device="tmpfs", + fstype="tmpfs", + opts="defaults,nodev,noexec", + dump=0, + pass_num=0, + ) + assert entry.match("tmpfs\t\t/tmp\ttmpfs\tnodev,defaults,noexec\t0 0\n") def test_fstab(): From 05fa1e070e08851208e30d8c0c5dd521f24b9dcf Mon Sep 17 00:00:00 2001 From: nicholasmhughes Date: Mon, 29 Aug 2022 13:57:27 -0400 Subject: [PATCH 3/6] fix test based on old ordering --- tests/unit/states/test_mount.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/states/test_mount.py b/tests/unit/states/test_mount.py index 53ddd4aa686d..d770693484b4 100644 --- a/tests/unit/states/test_mount.py +++ b/tests/unit/states/test_mount.py @@ -258,12 +258,12 @@ def test_mounted(self): ), ret, ) - # Test to check the options order #57520 + # Test to check the options order #57520, reverted in #62557 set_fstab_mock.assert_called_with( name2, "//SERVER/SHARE/", "cifs", - ["gid=group1", "uid=user1"], + ["uid=user1", "gid=group1"], 0, 0, "/etc/fstab", From be5d9f6a2d3dbd6ea31c96bc5881fa8f36dae516 Mon Sep 17 00:00:00 2001 From: nicholasmhughes Date: Fri, 2 Sep 2022 08:14:44 -0400 Subject: [PATCH 4/6] roll back test change to resolve conflict --- tests/unit/states/test_mount.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/states/test_mount.py b/tests/unit/states/test_mount.py index d770693484b4..53ddd4aa686d 100644 --- a/tests/unit/states/test_mount.py +++ b/tests/unit/states/test_mount.py @@ -258,12 +258,12 @@ def test_mounted(self): ), ret, ) - # Test to check the options order #57520, reverted in #62557 + # Test to check the options order #57520 set_fstab_mock.assert_called_with( name2, "//SERVER/SHARE/", "cifs", - ["uid=user1", "gid=group1"], + ["gid=group1", "uid=user1"], 0, 0, "/etc/fstab", From a68ea62da1d7adb47157c068a26fa55661837636 Mon Sep 17 00:00:00 2001 From: nicholasmhughes Date: Fri, 2 Sep 2022 08:17:01 -0400 Subject: [PATCH 5/6] transfer test change to new location --- tests/pytests/unit/states/test_mount.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/pytests/unit/states/test_mount.py b/tests/pytests/unit/states/test_mount.py index 5f27b8b81533..f17541308add 100644 --- a/tests/pytests/unit/states/test_mount.py +++ b/tests/pytests/unit/states/test_mount.py @@ -238,12 +238,12 @@ def test_mounted(): ) == ret ) - # Test to check the options order #57520 + # Test to check the options order #57520, reverted in #62557 set_fstab_mock.assert_called_with( name2, "//SERVER/SHARE/", "cifs", - ["gid=group1", "uid=user1"], + ["uid=user1", "gid=group1"], 0, 0, "/etc/fstab", From 9aa7aaec8cfbc36d833efd23671a1ce8e97bfe22 Mon Sep 17 00:00:00 2001 From: nicholasmhughes Date: Wed, 7 Sep 2022 16:50:03 -0400 Subject: [PATCH 6/6] remove log line which could contain sensitive information --- salt/modules/mount.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/salt/modules/mount.py b/salt/modules/mount.py index 54a8f6cb84f3..76a52b89ea61 100644 --- a/salt/modules/mount.py +++ b/salt/modules/mount.py @@ -882,9 +882,6 @@ def set_fstab( line = salt.utils.stringutils.to_unicode(line) try: if criteria.match(line): - log.debug( - "Checking ( %s ) against current line ( %s )", entry, line - ) # Note: If ret isn't None here, # we've matched multiple lines ret = "present"