Fix useradd hard-coded relative command names

[nicholasmhughes]

What does this PR do?

This PR adds support for searching for useradd, usermod, and userdel binaries using the salt.utils.path.which function instead of solely relying on the PATH environment variable to find them.

What issues does this PR fix or reference?

Fixes: #62087

Previous Behavior

Binaries would not be found if some "sbin" locations were not in the PATH, despite being installed.

New Behavior

The salt.utils.path.which function adds some standard paths into the search locations, so the binaries will still be found if in a "standard" location like /sbin/ even if that directory isn't in the PATH.

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

• Docs
• Changelog - https://docs.saltproject.io/en/master/topics/development/changelog.html
• Tests written/updated

Commits signed with GPG?

Yes

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

[whytewolf]

Hey Nick,

Nice pr, looks like the tests will need to be skipped on a couple of operating systems.

[nicholasmhughes]

hmmm... looks like it's failing the old style tests because the which return isn't mocked. I guess I should probably ensure full pytest coverage and just remove the old unit tests.

[whytewolf]

Hey @nicholasmhughes so, my two cents on the mocking of which. I would rather know these fail on some operating systems that don't have useradd and the like. and have the tests account for that change. then to mock which and have it "work" when the module wouldn't actually work on an operating system.

but that is just my two cents. @waynew what do you think?

[nicholasmhughes]

re-run pr-opensuse-15-x86_64-py3-pytest

[nicholasmhughes]

re-run pr-fedora-34-x86_64-py3-pytest

[nicholasmhughes]

re-run pr-centos-7-x86_64-py3-tcp-pytest

[waynew]

@whytewolf when it comes to a unit test, mocking is preferable, but it wouldn't hurt at all to have a functional test that fails on Windows or other systems without useradd. Could do something like:

@pytest.mark.skipIf(..., "Should only fail on Windows/Mac")  # or whatever
def test_errors_out_when_no_useradd_exists():
    with pytest.raises(CommandExecutionError):
        ...

It might be worthwhile adding another test for successful platforms, but I think we actually do have some of those already? 🤔

[nicholasmhughes]

re-run full all

[whytewolf]

@whytewolf when it comes to a unit test, mocking is preferable, but it wouldn't hurt at all to have a functional test that fails on Windows or other systems without useradd. Could do something like:

@pytest.mark.skipIf(..., "Should only fail on Windows/Mac")  # or whatever
def test_errors_out_when_no_useradd_exists():
    with pytest.raises(CommandExecutionError):
        ...

It might be worthwhile adding another test for successful platforms, but I think we actually do have some of those already? 🤔

Woops totally didn't notice they were unit thought they were functional for some reason. You're right. unit should be mocked. the new test looks to cover my concern so approving.

[waynew]

🚀 You rock!

[nicholasmhughes]

re-run pr-arch-lts-x86_64-py3-pytest