Ensure that changes are made in predicable order

This commit is contained in:
Erik Johnson 2020-04-30 21:14:25 -05:00 committed by Daniel Wozniak
parent fd8ed64b17
commit f83663c3c0
2 changed files with 144 additions and 87 deletions

View file

@ -37,7 +37,7 @@ import salt.utils.user
from salt.exceptions import CommandExecutionError
# Import 3rd-party libs
from salt.ext.six import iteritems, string_types
from salt.ext import six
log = logging.getLogger(__name__)
@ -526,7 +526,7 @@ def present(
# the comma is used to separate field in GECOS, thus resulting into
# salt adding the end of fullname each time this function is called
for gecos_field in [fullname, roomnumber, workphone]:
if isinstance(gecos_field, string_types) and "," in gecos_field:
if isinstance(gecos_field, six.string_types) and "," in gecos_field:
ret["comment"] = "Unsupported char ',' in {0}".format(gecos_field)
ret["result"] = False
return ret
@ -614,7 +614,7 @@ def present(
if __opts__["test"]:
ret["result"] = None
ret["comment"] = "The following user attributes are set to be " "changed:\n"
for key, val in iteritems(changes):
for key, val in six.iteritems(changes):
if key == "passwd":
val = "XXX-REDACTED-XXX"
elif key == "group" and not remove_groups:
@ -627,65 +627,103 @@ def present(
if __grains__["kernel"] in ("OpenBSD", "FreeBSD"):
lcpre = __salt__["user.get_loginclass"](name)
pre = __salt__["user.info"](name)
for key, val in iteritems(changes):
if key == "passwd" and not empty_password:
# Make changes
if "passwd" in changes:
del changes["passwd"]
if not empty_password:
__salt__["shadow.set_password"](name, password)
continue
if key == "passwd" and empty_password:
log.warning("No password will be set when empty_password=True")
continue
if key == "empty_password" and val:
__salt__["shadow.del_password"](name)
continue
if key == "date":
__salt__["shadow.set_date"](name, date)
continue
# run chhome once to avoid any possible bad side-effect
if key == "home" and "homeDoesNotExist" not in changes:
if __grains__["kernel"] in ("Darwin", "Windows"):
__salt__["user.chhome"](name, val)
else:
__salt__["user.chhome"](name, val, persist=False)
continue
if key == "homeDoesNotExist":
if __grains__["kernel"] in ("Darwin", "Windows"):
__salt__["user.chhome"](name, val)
else:
__salt__["user.chhome"](name, val, persist=True)
if not os.path.isdir(val):
__salt__["file.mkdir"](val, pre["uid"], pre["gid"], 0o755)
continue
if key == "mindays":
__salt__["shadow.set_mindays"](name, mindays)
continue
if key == "maxdays":
__salt__["shadow.set_maxdays"](name, maxdays)
continue
if key == "inactdays":
__salt__["shadow.set_inactdays"](name, inactdays)
continue
if key == "warndays":
__salt__["shadow.set_warndays"](name, warndays)
continue
if key == "expire":
__salt__["shadow.set_expire"](name, expire)
continue
if key == "win_homedrive":
__salt__["user.update"](name=name, homedrive=val)
continue
if key == "win_profile":
__salt__["user.update"](name=name, profile=val)
continue
if key == "win_logonscript":
__salt__["user.update"](name=name, logonscript=val)
continue
if key == "win_description":
__salt__["user.update"](name=name, description=val)
continue
if key == "groups":
__salt__["user.ch{0}".format(key)](name, val, not remove_groups)
else:
__salt__["user.ch{0}".format(key)](name, val)
log.warning("No password will be set when empty_password=True")
if changes.pop("empty_password", False) is True:
__salt__["shadow.del_password"](name)
if "date" in changes:
del changes["date"]
__salt__["shadow.set_date"](name, date)
def _change_homedir(name, val):
if __grains__["kernel"] in ("Darwin", "Windows"):
__salt__["user.chhome"](name, val)
else:
__salt__["user.chhome"](name, val, persist=False)
_homedir_changed = False
if "home" in changes:
val = changes.pop("home")
if "homeDoesNotExist" not in changes:
_change_homedir(name, val)
_homedir_changed = True
if "homeDoesNotExist" in changes:
val = changes.pop("homeDoesNotExist")
if not _homedir_changed:
_change_homedir(name, val)
if not os.path.isdir(val):
__salt__["file.mkdir"](val, pre["uid"], pre["gid"], 0o755)
if "mindays" in changes:
del changes["mindays"]
__salt__["shadow.set_mindays"](name, mindays)
if "maxdays" in changes:
del changes["maxdays"]
__salt__["shadow.set_maxdays"](name, maxdays)
if "inactdays" in changes:
del changes["inactdays"]
__salt__["shadow.set_inactdays"](name, inactdays)
if "warndays" in changes:
del changes["warndays"]
__salt__["shadow.set_warndays"](name, warndays)
if "expire" in changes:
del changes["expire"]
__salt__["shadow.set_expire"](name, expire)
if "win_homedrive" in changes:
del changes["win_homedrive"]
__salt__["user.update"](name=name, homedrive=changes.pop("win_homedrive"))
if "win_profile" in changes:
del changes["win_profile"]
__salt__["user.update"](name=name, profile=changes.pop("win_profile"))
if "win_logonscript" in changes:
del changes["win_logonscript"]
__salt__["user.update"](
name=name, logonscript=changes.pop("win_logonscript")
)
if "win_description" in changes:
del changes["win_description"]
__salt__["user.update"](
name=name, description=changes.pop("win_description")
)
# Do the changes that have "ch" functions for them, but skip changing
# groups for now. Changing groups before changing the chgid could cause
# unpredictable results, including failure to set the proper groups.
# NOTE: list(changes) required here to avoid modifying dictionary
# during iteration.
for key in [
x
for x in list(changes)
if x != "groups" and "user.ch{0}".format(x) in __salt__
]:
__salt__["user.ch{0}".format(key)](name, changes.pop(key))
# Do group changes last
if "groups" in changes:
__salt__["user.chgroups"](name, changes.pop("groups"), not remove_groups)
if changes:
ret.get("warnings", []).append(
"Unhandled changes: {0}".format(", ".join(changes))
)
post = __salt__["user.info"](name)
spost = {}

View file

@ -314,38 +314,57 @@ class UserTest(ModuleCase, SaltReturnAssertsMixin):
This tests the case in which the default group is changed at the same
time as it is also moved into the "groups" list.
"""
# Add the groups
ret = self.run_state("group.present", name=self.user_name)
self.assertSaltTrueReturn(ret)
ret = self.run_state("group.present", name=self.alt_group)
self.assertSaltTrueReturn(ret)
try:
# Add the groups
ret = self.run_state("group.present", name=self.user_name)
self.assertSaltTrueReturn(ret)
ret = self.run_state("group.present", name=self.alt_group)
self.assertSaltTrueReturn(ret)
# Add the user
ret = self.run_state("user.present", name=self.user_name, gid=self.alt_group,)
self.assertSaltTrueReturn(ret)
user_name_gid = self.run_function("file.group_to_gid", [self.user_name])
alt_group_gid = self.run_function("file.group_to_gid", [self.alt_group])
# Now change the gid and move alt_group to the groups list
ret = self.run_state(
"user.present",
name=self.user_name,
gid=self.user_name,
groups=[self.alt_group],
allow_gid_change=True,
)
self.assertSaltTrueReturn(ret)
# Add the user
ret = self.run_state("user.present", name=self.user_name, gid=alt_group_gid)
# Be extra sure that we did what we intended
gid = self.run_function("file.group_to_gid", [self.user_name])
uinfo = self.run_function("user.info", [self.user_name])
# Check that the initial user addition set the gid and groups as
# expected.
new_gid = self.run_function("file.group_to_gid", [self.user_name])
uinfo = self.run_function("user.info", [self.user_name])
assert uinfo["gid"] == gid, uinfo["gid"]
assert uinfo["groups"] == [self.user_name, self.alt_group], uinfo["groups"]
assert uinfo["gid"] == alt_group_gid, uinfo["gid"]
assert uinfo["groups"] == [self.alt_group], uinfo["groups"]
# Do the cleanup here so we don't have to put all of this in the
# tearDown to be executed after each test.
self.assertSaltTrueReturn(self.run_state("user.absent", name=self.user_name))
self.assertSaltTrueReturn(self.run_state("group.absent", name=self.user_name))
self.assertSaltTrueReturn(self.run_state("group.absent", name=self.user_name))
# Now change the gid and move alt_group to the groups list in the
# same salt run.
ret = self.run_state(
"user.present",
name=self.user_name,
gid=user_name_gid,
groups=[self.alt_group],
allow_gid_change=True,
)
self.assertSaltTrueReturn(ret)
# Be sure that we did what we intended
new_gid = self.run_function("file.group_to_gid", [self.user_name])
uinfo = self.run_function("user.info", [self.user_name])
assert uinfo["gid"] == new_gid, uinfo["gid"]
assert uinfo["groups"] == [self.user_name, self.alt_group], uinfo["groups"]
finally:
# Do the cleanup here so we don't have to put all of this in the
# tearDown to be executed after each test.
self.assertSaltTrueReturn(
self.run_state("user.absent", name=self.user_name)
)
self.assertSaltTrueReturn(
self.run_state("group.absent", name=self.user_name)
)
self.assertSaltTrueReturn(
self.run_state("group.absent", name=self.alt_group)
)
def tearDown(self):
if salt.utils.platform.is_darwin():