Merge branch 'master' into fix-fstab-opts-order

changelog/54907.fixed

@@ -0,0 +1 @@
+Fix spelling error for python_shell argument in dpkg_lower module

changelog/61816.fixed

@@ -0,0 +1 @@
+Made cmdmod._run[_all]_quiet work during minion startup on MacOS with runas specified (which fixed mac_service)

changelog/62502.fixed

@@ -0,0 +1 @@
+Fix user.present to allow removing groups using optional_groups parameter and enforcing idempotent group membership.

changelog/62519.fixed

@@ -0,0 +1 @@
+Fix possible tracebacks if there is a package with '------' or '======' in the description is installed on the Debian based minion.

doc/topics/releases/index.rst

@@ -20,6 +20,7 @@ Previous Releases
     :maxdepth: 1
     :glob:
 
+    3005*
     3004*
     3003*
     3002*
@@ -43,7 +44,6 @@ Upcoming Release
     :maxdepth: 1
     :glob:
 
-    3005*
     3006*
 
 .. seealso:: :ref:`Legacy salt-cloud release docs <legacy-salt-cloud-release-notes>`

salt/modules/cmdmod.py

@@ -443,12 +443,13 @@ def _run(
         # Ensure environment is correct for a newly logged-in user by running
         # the command under bash as a login shell
         try:
-            user_shell = __salt__["user.info"](runas)["shell"]
+            # Do not rely on populated __salt__ dict (ie avoid __salt__['user.info'])
+            user_shell = [x for x in pwd.getpwall() if x.pw_name == runas][0].pw_shell
             if re.search("bash$", user_shell):
                 cmd = "{shell} -l -c {cmd}".format(
                     shell=user_shell, cmd=_cmd_quote(cmd)
                 )
-        except KeyError:
+        except (AttributeError, IndexError):
             pass
 
         # Ensure the login is simulated correctly (note: su runs sh, not bash,

salt/modules/dpkg_lowpkg.py

@@ -271,14 +271,13 @@ def _get_pkg_info(*packages, **kwargs):
         "origin:${Origin}\\n"
         "homepage:${Homepage}\\n"
         "status:${db:Status-Abbrev}\\n"
-        "======\\n"
         "description:${Description}\\n"
-        "------\\n'"
+        "\\n*/~^\\\\*\\n'"
     )
     cmd += " {}".format(" ".join(packages))
     cmd = cmd.strip()
 
-    call = __salt__["cmd.run_all"](cmd, python_chell=False)
+    call = __salt__["cmd.run_all"](cmd, python_shell=False)
     if call["retcode"]:
         if failhard:
             raise CommandExecutionError(
@@ -287,9 +286,13 @@ def _get_pkg_info(*packages, **kwargs):
         else:
             return ret
 
-    for pkg_info in [elm for elm in re.split(r"------", call["stdout"]) if elm.strip()]:
+    for pkg_info in [
+        elm
+        for elm in re.split(r"\r?\n\*/~\^\\\*(\r?\n|)", call["stdout"])
+        if elm.strip()
+    ]:
         pkg_data = {}
-        pkg_info, pkg_descr = re.split(r"======", pkg_info)
+        pkg_info, pkg_descr = pkg_info.split("\ndescription:", 1)
         for pkg_info_line in [
             el.strip() for el in pkg_info.split(os.linesep) if el.strip()
         ]:
@@ -299,7 +302,7 @@ def _get_pkg_info(*packages, **kwargs):
             install_date = _get_pkg_install_time(pkg_data.get("package"))
             if install_date:
                 pkg_data["install_date"] = install_date
-        pkg_data["description"] = pkg_descr.split(":", 1)[-1]
+        pkg_data["description"] = pkg_descr
         ret.append(pkg_data)
 
     return ret

salt/states/user.py

@@ -98,9 +98,9 @@ def _changes(
         return False
 
     change = {}
-    if groups is None:
-        groups = lusr["groups"]
     wanted_groups = sorted(set((groups or []) + (optional_groups or [])))
+    if not remove_groups:
+        wanted_groups = sorted(set(wanted_groups + lusr["groups"]))
     if uid and lusr["uid"] != uid:
         change["uid"] = uid
     if gid is not None and lusr["gid"] not in (gid, __salt__["file.group_to_gid"](gid)):

tests/pytests/functional/states/test_user.py

@@ -52,10 +52,14 @@ def user_home(username, tmp_path):
 
 
 @pytest.fixture
-def group_1(username):
+def group_1(username, grains):
     groupname = username
     if salt.utils.platform.is_darwin():
         groupname = "staff"
+    elif salt.utils.platform.is_photonos():
+        groupname = "users"
+    elif grains["os_family"] in ("Suse",):
+        groupname = "users"
     with pytest.helpers.create_group(name=groupname) as group:
         yield group
 
@@ -110,7 +114,9 @@ def test_user_present_when_home_dir_does_not_18843(states, existing_account):
     """
     shutil.rmtree(existing_account.info.home)
     ret = states.user.present(
-        name=existing_account.username, home=existing_account.info.home
+        name=existing_account.username,
+        home=existing_account.info.home,
+        remove_groups=False,
     )
     assert ret.result is True
     assert pathlib.Path(existing_account.info.home).is_dir()
@@ -213,6 +219,7 @@ def test_user_present_unicode(states, username, subtests):
             roomnumber="①②③",
             workphone="١٢٣٤",
             homephone="६७८",
+            remove_groups=False,
         )
         assert ret.result is True
 
@@ -363,3 +370,53 @@ def test_user_present_existing(states, username):
     assert ret.changes["profile"] == win_profile
     assert "description" in ret.changes
     assert ret.changes["description"] == win_description
+
+
+@pytest.mark.skip_unless_on_linux(reason="underlying functionality only runs on Linux")
+def test_user_present_change_groups(modules, states, username, group_1, group_2):
+    ret = states.user.present(
+        name=username,
+        groups=[group_1.name, group_2.name],
+    )
+    assert ret.result is True
+
+    user_info = modules.user.info(username)
+    assert user_info
+    assert user_info["groups"] == [group_2.name, group_1.name]
+
+    # run again and remove group_2
+    ret = states.user.present(
+        name=username,
+        groups=[group_1.name],
+    )
+    assert ret.result is True
+
+    user_info = modules.user.info(username)
+    assert user_info
+    assert user_info["groups"] == [group_1.name]
+
+
+@pytest.mark.skip_unless_on_linux(reason="underlying functionality only runs on Linux")
+def test_user_present_change_optional_groups(
+    modules, states, username, group_1, group_2
+):
+    ret = states.user.present(
+        name=username,
+        optional_groups=[group_1.name, group_2.name],
+    )
+    assert ret.result is True
+
+    user_info = modules.user.info(username)
+    assert user_info
+    assert user_info["groups"] == [group_2.name, group_1.name]
+
+    # run again and remove group_2
+    ret = states.user.present(
+        name=username,
+        optional_groups=[group_1.name],
+    )
+    assert ret.result is True
+
+    user_info = modules.user.info(username)
+    assert user_info
+    assert user_info["groups"] == [group_1.name]

tests/pytests/unit/modules/test_cmdmod.py

@@ -455,11 +455,12 @@ def mock_proc(__cmd__, **kwargs):
 
             # User default shell is '/usr/local/bin/bash'
             user_default_shell = "/usr/local/bin/bash"
-            with patch.dict(
-                cmdmod.__salt__,
-                {"user.info": MagicMock(return_value={"shell": user_default_shell})},
+            with patch(
+                "pwd.getpwall",
+                Mock(
+                    return_value=[Mock(pw_shell=user_default_shell, pw_name="foobar")]
+                ),
             ):
-
                 cmd_handler.clear()
                 cmdmod._run(
                     "ls", cwd=tempfile.gettempdir(), runas="foobar", use_vt=False
@@ -471,9 +472,11 @@ def mock_proc(__cmd__, **kwargs):
 
             # User default shell is '/bin/zsh'
             user_default_shell = "/bin/zsh"
-            with patch.dict(
-                cmdmod.__salt__,
-                {"user.info": MagicMock(return_value={"shell": user_default_shell})},
+            with patch(
+                "pwd.getpwall",
+                Mock(
+                    return_value=[Mock(pw_shell=user_default_shell, pw_name="foobar")]
+                ),
             ):
 
                 cmd_handler.clear()
@@ -486,6 +489,37 @@ def mock_proc(__cmd__, **kwargs):
                 ), "cmd does not invoke user shell on macOS"
 
 
+@pytest.mark.skip_on_windows
+def test_run_all_quiet_does_not_depend_on_salt_dunder():
+    """
+    `cmdmod._run_all_quiet` should not depend on availability
+    of __salt__ dictionary (issue #61816).
+
+    This test checks for __salt__ specifically and will still
+    pass if other dunders, especially __grains__, are referenced.
+    This is the case on UNIX systems other than MacOS when
+    `sudo` could not be found.
+    """
+
+    proc = MagicMock(return_value=MockTimedProc(stdout=b"success", stderr=None))
+    runas = getpass.getuser()
+
+    with patch.dict(cmdmod.__grains__, {"os": "Darwin", "os_family": "Solaris"}):
+        with patch("salt.utils.timed_subprocess.TimedProc", proc):
+            salt_dunder_mock = MagicMock(spec_set=dict)
+            salt_dunder_mock.__getitem__.side_effect = NameError(
+                "__salt__ might not be defined"
+            )
+
+            with patch.object(cmdmod, "__salt__", salt_dunder_mock):
+                ret = cmdmod._run_all_quiet("foo")
+                assert ret["stdout"] == "success"
+                assert salt_dunder_mock.__getitem__.call_count == 0
+                ret = cmdmod._run_all_quiet("foo", runas=runas)
+                assert ret["stdout"] == "success"
+                assert salt_dunder_mock.__getitem__.call_count == 0
+
+
 def test_run_cwd_doesnt_exist_issue_7154():
     """
     cmd.run should fail and raise

tests/pytests/unit/states/test_user.py

@@ -178,8 +178,10 @@ def test_present_uid_gid_change():
     # get the before/after for the changes dict, and one last time to
     # confirm that no changes still need to be made.
     mock_info = MagicMock(side_effect=[before, before, after, after])
-    mock_group_to_gid = MagicMock(side_effect=["foo", "othergroup"])
-    mock_gid_to_group = MagicMock(side_effect=[5000, 5000, 5001, 5001])
+    mock_group_to_gid = MagicMock(side_effect=[5000, 5001])
+    mock_gid_to_group = MagicMock(
+        side_effect=["othergroup", "foo", "othergroup", "othergroup"]
+    )
     dunder_salt = {
         "user.info": mock_info,
         "user.chuid": Mock(),
@@ -197,7 +199,7 @@ def test_present_uid_gid_change():
         )
         assert ret == {
             "comment": "Updated user foo",
-            "changes": {"gid": 5001, "uid": 5001, "groups": ["othergroup"]},
+            "changes": {"gid": 5001, "uid": 5001, "groups": []},
             "name": "foo",
             "result": True,
         }

tests/unit/modules/test_dpkg_lowpkg.py

@@ -290,7 +290,6 @@ def test_info(self):
                         "origin:",
                         "homepage:http://tiswww.case.edu/php/chet/bash/bashtop.html",
                         "status:ii ",
-                        "======",
                         "description:GNU Bourne Again SHell",
                         " Bash is an sh-compatible command language interpreter that"
                         " executes",
@@ -307,7 +306,8 @@ def test_info(self):
                         " The Programmable Completion Code, by Ian Macdonald, is now"
                         " found in",
                         " the bash-completion package.",
-                        "------",
+                        "",
+                        "*/~^\\*",  # pylint: disable=W1401
                     ]
                 ),
             }