diff --git a/changelog/68110.fixed.md b/changelog/68110.fixed.md new file mode 100644 index 00000000000..3e456539619 --- /dev/null +++ b/changelog/68110.fixed.md @@ -0,0 +1 @@ +Fixed `user.present` to not fail with `result: False` in test mode when a referenced group does not yet exist; the state now reports the pending changes so users can preview states that depend on groups created by a `group.present` requisite in the same run. diff --git a/salt/states/user.py b/salt/states/user.py index 503451768af..01d5b14e026 100644 --- a/salt/states/user.py +++ b/salt/states/user.py @@ -597,14 +597,25 @@ def present( ret["result"] = False return ret + missing_groups = [] if groups: missing_groups = [x for x in groups if not __salt__["group.info"](x)] if missing_groups: - ret["comment"] = "The following group(s) are not present: {}".format( - ",".join(missing_groups) - ) - ret["result"] = False - return ret + if __opts__.get("test"): + # In test mode, a missing group is not necessarily an error: + # a `group.present` requisite may create it during the real + # run. Note the missing groups in the result, drop them from + # the membership check below (they cannot be diffed against + # the user's current groups since they do not yet exist), + # and let the rest of the function report whatever else + # would change. Issue #68110. + groups = [x for x in groups if x not in missing_groups] + else: + ret["comment"] = "The following group(s) are not present: {}".format( + ",".join(missing_groups) + ) + ret["result"] = False + return ret if optional_groups: present_optgroups = [x for x in optional_groups if __salt__["group.info"](x)] @@ -686,6 +697,10 @@ def present( elif key == "group" and not remove_groups: key = "ensure groups" ret["comment"] += f"{key}: {val}\n" + if missing_groups: + ret["comment"] += "groups (pending creation): {}\n".format( + ",".join(missing_groups) + ) return ret # The user is present if "shadow.info" in __salt__: @@ -870,6 +885,10 @@ def _change_homedir(name, val): if __opts__["test"]: ret["result"] = None ret["comment"] = f"User {name} set to be added" + if missing_groups: + ret["comment"] += " (pending groups: {})".format( + ",".join(missing_groups) + ) return ret if groups and present_optgroups: groups.extend(present_optgroups) diff --git a/tests/pytests/unit/states/test_user.py b/tests/pytests/unit/states/test_user.py index ffbd2d7d4d1..4ae8c01ef70 100644 --- a/tests/pytests/unit/states/test_user.py +++ b/tests/pytests/unit/states/test_user.py @@ -462,6 +462,66 @@ def test_present_password_unlock(): unlock_account.assert_not_called() +def test_present_test_mode_missing_group_new_user(): + """ + Regression test for #68110: user.present should not fail in test mode + when a referenced group does not yet exist (e.g. it will be created by + a group.present requisite during the actual run). When the user also + does not yet exist, the state should report it would be added and that + the missing groups are pending. + """ + mock_false = MagicMock(return_value=False) + mock_empty_list = MagicMock(return_value=[]) + with patch.dict(user.__grains__, {"kernel": "Linux"}), patch.dict( + user.__salt__, + { + "group.info": mock_false, + "user.info": mock_empty_list, + "user.chkey": mock_empty_list, + "user.add": mock_false, + }, + ), patch.dict(user.__opts__, {"test": True}): + ret = user.present("foo", groups=["foo"]) + assert ret["result"] is None, ret + assert "set to be added" in ret["comment"], ret + assert "foo" in ret["comment"], ret + + +def test_present_test_mode_missing_group_existing_user(): + """ + Regression test for #68110: when the user already exists, a missing + required group should not cause a hard failure in test mode. The + pending group(s) should appear in the test-mode comment. + """ + mock_info = MagicMock( + return_value={ + "uid": 5000, + "gid": 5000, + "groups": ["bar"], + "home": "/home/baz", + "fullname": "Baz", + } + ) + # group.info: "foo" missing, "bar" exists. + group_info = MagicMock( + side_effect=lambda name: False if name == "foo" else {"gid": 5000} + ) + dunder_salt = { + "group.info": group_info, + "user.info": mock_info, + "file.group_to_gid": MagicMock(return_value=5000), + "file.gid_to_group": MagicMock(return_value="bar"), + } + with patch.dict(user.__grains__, {"kernel": "Linux"}), patch.dict( + user.__salt__, dunder_salt + ), patch.dict(user.__opts__, {"test": True}): + ret = user.present("baz", groups=["foo", "bar"]) + assert ret["result"] is not False, ret + assert "not present" not in ret["comment"], ret + # Pending group should be reported somewhere in the comment. + assert "foo" in ret["comment"], ret + + @pytest.mark.parametrize( "current,wanted,remove,return_value,expected", [