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

(PUP-12041) Handle libuser's unavailability in Fedora 40 #9346

Merged
merged 1 commit into from
May 21, 2024

Conversation

shubhamshinde360
Copy link
Contributor

@shubhamshinde360 shubhamshinde360 commented May 13, 2024

  • Starting from version 40, Fedora does not have the lgroup* commands available due to deprecation of libuser: https://fedoraproject.org/wiki/Changes/LibuserDeprecation.
  • groupadd relies on libuser to add/purge members to/from groups.
  • Add manages_members feature to Fedora 40 and above since groupmod can add members to groups now. Historically, it was unable to do so that's why puppet used lgroupmod for it.
  • Handle flags for lgroupmod (-M) and groupmod (-aU) commands properly.
  • Only use lgroup* commands if libuser is supported.
  • When libuser is not supported, members should be purged using the usermod command. Since usermod does not support comma separated list of users they should be removed one by one.

@shubhamshinde360 shubhamshinde360 force-pushed the PUP-12041 branch 3 times, most recently from 8256d3b to dae7ca8 Compare May 14, 2024 07:35
@joshcooper
Copy link
Contributor

Could you add a link to https://fedoraproject.org/wiki/Changes/LibuserDeprecation in your commit message for future reference? Also please squash the code and test changes so that future git bisect commands don't generate false negatives (on the commit that modified the code, but not the tests).

Also found a good background thread, including a reference to Puppet https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/M6UATQKVB7IQKJ6EIOAVPYGJB5KQM7VN/ One thing I didn't realize is that libuser can still be installed from epel (since it's deprecated, but not removed yet).

One thing that doesn't seem right. Puppet can either manage the complete list of group members or a partial list. For example, you may want to ensure the wheel group only contains root and nothing more. But in other cases you may want to ensure a group contains at least joe.

To differentiate between these cases, the group type has an auth_membership parameter. If it's true, then the group provider will purge members that aren't listed in the manifest. It seems your PR still calls lgroupmod

First create a group, two users and add them to the group:

# bundle exec puppet resource group staff ensure=present
group { 'staff':
  ensure   => 'present',
  provider => 'groupadd',
}
# bundle exec puppet resource user josh ensure=present
Notice: /User[josh]/ensure: created
user { 'josh':
  ensure   => 'present',
  provider => 'useradd',
}
# bundle exec puppet resource user notjosh ensure=present
Notice: /User[notjosh]/ensure: created
user { 'notjosh':
  ensure   => 'present',
  provider => 'useradd',
}
# bundle exec puppet resource group staff ensure=present members=josh,notjosh
Notice: /Group[staff]/members: members changed root to josh,notjosh,root
group { 'staff':
  ensure   => 'present',
  members  => ['root', 'josh', 'notjosh'],
  provider => 'groupadd',
}

Notice if we remove one of the users, puppet doesn't do anything:

# bundle exec puppet resource group staff ensure=present members=josh
group { 'staff':
  ensure   => 'present',
  members  => ['root', 'josh', 'notjosh'],
  provider => 'groupadd',
}

But if we specify auth_membership it will try to purge the user and fail:

# bundle exec puppet resource group staff ensure=present members=josh auth_membership=true
Error: Command localmodify is missing
Error: /Group[staff]/members: change from root,josh,notjosh to josh failed: Command localmodify is missing
group { 'staff':
  ensure   => 'present',
  members  => ['root', 'josh', 'notjosh'],
  provider => 'groupadd',
}

Based on the thread, we may be able to purge specific users with usermod -r -G somegroup someuser instead.

 - Starting from version 40, Fedora does not have the lgroup* commands available due to deprecation of libuser: https://fedoraproject.org/wiki/Changes/LibuserDeprecation.
 - groupadd relies on libuser to add/purge members to/from groups.
 - Add manages_members feature to Fedora 40 and above since groupmod can add members to groups now. Historically, it was unable to do so that's why puppet used lgroupmod for it.
 - Handle flags for lgroupmod (-M) and groupmod (-aU) commands properly.
 - Only use lgroup* commands if libuser is supported.
 - When libuser is not supported, members should be purged using the usermod command. Since usermod does not support comma separated list of users they should be removed one by one.
@shubhamshinde360
Copy link
Contributor Author

Thanks @joshcooper for pointing this out,

I have added a way to work around the purge_members issue with the 'usermod -rG' command. Since usermod is not allowing a list of comma separated list of members, I had to loop on the members and remove them one by one. Please let me know your thoughts on this.

@shubhamshinde360
Copy link
Contributor Author

Also, not sure if we should also look at the direction of installing libuser from epel for Fedora 40. I guess puppet could try to look at the EPEL repo and try to find libuser in case it is missing. But not sure if thats a good way to go. It would be great if you could share your thoughts on this too. Thanks!

@shubhamshinde360
Copy link
Contributor Author

shubhamshinde360 commented May 15, 2024

Did a quick sanity check on RHEL for any impact. It works fine without any issues.

@shubhamshinde360 shubhamshinde360 marked this pull request as ready for review May 15, 2024 17:59
@shubhamshinde360 shubhamshinde360 requested a review from a team as a code owner May 15, 2024 17:59
@shubhamshinde360
Copy link
Contributor Author

Hey @joshcooper, @cthorn42, @mhashizume
Could you please take a look at this if this looks good now. Thanks!

@shubhamshinde360
Copy link
Contributor Author

shubhamshinde360 commented May 20, 2024

Picked up several linux based platforms with puppet component pointed to these changes, all platforms ran the puppet tests successfully
https://jenkins-platform.delivery.puppetlabs.net/view/puppet-agent/view/ad-hoc/job/platform_puppet-agent-extra_puppet-agent-integration-suite_adhoc-ad_hoc/1331/

@joshcooper joshcooper merged commit b0d63e5 into puppetlabs:main May 21, 2024
9 checks passed
@shubhamshinde360 shubhamshinde360 added the backport 7.x Generate a backport PR to 7.x label May 21, 2024
Copy link

Successfully created backport PR for 7.x:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 7.x Generate a backport PR to 7.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants