Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: sudosu not working on some BSD machines #8214

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lekoOwO
Copy link

@lekoOwO lekoOwO commented Apr 11, 2024

SUMMARY

The original sudosu does not work on some BSD machines and some versions of linux machine as the argument of su differs .

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

become.sudosu

ADDITIONAL INFORMATION

@ansibullbot
Copy link
Collaborator

@ansibullbot ansibullbot added become become plugin bug This issue/PR relates to a bug new_contributor Help guide this first time contributor plugins plugin (any type) small_patch Hopefully easy to review needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Apr 11, 2024
@lekoOwO
Copy link
Author

lekoOwO commented Apr 11, 2024

BTW, if /bin/sh is prefered, we should pass -s /bin/sh to su.

The original version passes /bin/sh without -s, thus on many su it does not work.

@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-7 Automatically create a backport for the stable-7 branch backport-8 Automatically create a backport for the stable-8 branch labels Apr 11, 2024
@felixfontein
Copy link
Collaborator

Thanks for your contribution! Please note that you need to add a changelog fragment, and that the unit tests are failing (https://github.com/ansible-collections/community.general/blob/main/tests/unit/plugins/become/test_sudosu.py).

Also it seems to me that this is potentially a larger change that might break some use-cases on other systems. I'm wondering whether the switch between the current and the new behavior should maybe be controlled by a toggle?

(The /bin/sh comes from the currently selected shell plugin, so hardcoding that is out of the question. That would break other mechanisms.)

@lekoOwO
Copy link
Author

lekoOwO commented Apr 16, 2024

ummmm the unit test is failing because it is asserting the original command which is not working on all system... Maybe we should change the unit test?

@lekoOwO
Copy link
Author

lekoOwO commented Apr 16, 2024

I'm wondering whether the switch between the current and the new behavior should maybe be controlled by a toggle?

That sounds good to me.

@ansibullbot ansibullbot added the stale_ci CI is older than 7 days, rerun before merging label Apr 24, 2024
@felixfontein felixfontein added backport-9 Automatically create a backport for the stable-9 branch and removed backport-7 Automatically create a backport for the stable-7 branch labels May 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-8 Automatically create a backport for the stable-8 branch backport-9 Automatically create a backport for the stable-9 branch become become plugin bug This issue/PR relates to a bug check-before-release PR will be looked at again shortly before release and merged if possible. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR new_contributor Help guide this first time contributor plugins plugin (any type) small_patch Hopefully easy to review stale_ci CI is older than 7 days, rerun before merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants