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

Support for alternative YPPWD_SRCDIR #374

Merged
merged 9 commits into from
Jul 24, 2023
Merged

Conversation

ancorgs
Copy link
Contributor

@ancorgs ancorgs commented Jan 14, 2023

Problem

In a NIS master server, there is a setting stored at /etc/sysconfig/ypserv and called YPPWD_SRCDIR that specifies where are the passwd, shadow and group files that NIS should export. By default this points to /etc. So in a typical NIS master server, managing the users and groups of the NIS domain is fully equivalent to managing the local users and groups in the system.

But what happens if that setting is set to a non default value?

When YaST Users runs in a system that is a NIS master server with YPPWD_SRCDIR pointing to an alternative location, it allows the user to select which set of "local users" should be managed in that execution of YaST - the one stored at /etc or the one stored at the location pointed by YPPWD_SRCDIR.

01_start_yast2_users

Unfortunately, when rewriting YaST Users to rely on the shadow tools (useradd and friends) we were not aware of that circumstance and YaST always modifies the files under /etc.

See https://bugzilla.suse.com/show_bug.cgi?id=1206627

Solution

First step. Make Linux::Writer more configurable so it can be instructed to write the changes to the files in the directory pointed by YPPWD_SRCDIR

Second step. Modify Linux::Writer and its action to execute the corresponding commands using the --prefix argument (for all commands useradd, userdel, usermod, groupadd, groupdel, groupmod, passwd, chpasswd and chage).

Note for reviewer

This involved a previous refactoring to make easier to add new values to the Linux::Writer configuration. Review commit by commit if you want to easily follow the rationale of that refactoring.

Testing

  • Tested manually.
  • Added a few unit tests.
  • Also verified by the L3 reporter (incident closed).

@ancorgs ancorgs force-pushed the upstream/yppwd_srcdir_sp4 branch 3 times, most recently from 39ec130 to 2d392ef Compare January 14, 2023 08:56
Copy link
Contributor

@joseivanlopez joseivanlopez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great improvement! Thanks.

Code wise looks quite good to me, but I will not approve it untill having a solution from the shadow tools for that commands missing --prefix option.

Copy link
Contributor

@joseivanlopez joseivanlopez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

CI failing, but the failure is unrelated to the changes in this PR.

#
# Hopefully, this mixin will disappear (maybe substituted by a similar one) once the shadow
# tools gain the ability to specify the exact location of the passwd, shadow and groups files.
module RootPath
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NP: Maybe this mixin can be part of the Action base class.

@jubalh
Copy link

jubalh commented Jun 9, 2023

I will not approve it untill having a solution from the shadow tools for that commands missing --prefix option.

The shadow PR got merged today.
Upstream plans to create a new release soonish.

Copy link
Contributor

@joseivanlopez joseivanlopez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ancorgs
Copy link
Contributor Author

ancorgs commented Jul 24, 2023

Added the changelog and version bump, since the updated shadow tools are already accepted. See https://build.suse.de/request/show/303035 and https://build.suse.de/request/show/303036

@ancorgs ancorgs merged commit 3415b5e into SLE-15-SP4 Jul 24, 2023
10 checks passed
@ancorgs ancorgs deleted the upstream/yppwd_srcdir_sp4 branch July 24, 2023 13:33
@yast-bot
Copy link
Contributor

✔️ Internal Jenkins job #8 successfully finished
✔️ Created IBS submit request #303674

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants