From 8df1dbf19a6389d50490abe04bcce3643f4cc3f8 Mon Sep 17 00:00:00 2001 From: Shubham Shinde Date: Wed, 15 May 2024 20:58:19 +0530 Subject: [PATCH] (PUP-12041) Handle libuser's unavailability in Fedora 40 - 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. (cherry picked from commit 6a1db9e53b721c31e6771f181fb7457a92dc86d7) --- lib/puppet/provider/group/groupadd.rb | 39 +++++++++++++++++------ spec/unit/provider/group/groupadd_spec.rb | 19 ++++++++++- 2 files changed, 48 insertions(+), 10 deletions(-) diff --git a/lib/puppet/provider/group/groupadd.rb b/lib/puppet/provider/group/groupadd.rb index cb225f9fa22..50ef6a495de 100644 --- a/lib/puppet/provider/group/groupadd.rb +++ b/lib/puppet/provider/group/groupadd.rb @@ -15,11 +15,20 @@ value.is_a? Integer end - optional_commands :localadd => "lgroupadd", :localdelete => "lgroupdel", :localmodify => "lgroupmod" - - has_feature :manages_local_users_and_groups, :manages_members if Puppet.features.libuser? - - options :members, :flag => '-M', :method => :mem + optional_commands :localadd => "lgroupadd", :localdelete => "lgroupdel", :localmodify => "lgroupmod", :purgemember => "usermod" + + has_feature :manages_local_users_and_groups if Puppet.features.libuser? + has_feature :manages_members if Puppet.features.libuser? || + (Puppet.runtime[:facter].value('os.name') == "Fedora" && + Puppet.runtime[:facter].value('os.release.major').to_i >= 40) + + # Libuser's modify command 'lgroupmod' requires '-M' flag for member additions. + # 'groupmod' command requires the '-aU' flags for it. + if Puppet.features.libuser? + options :members, :flag => '-M', :method => :mem + else + options :members, :flag => '-aU', :method => :mem + end def exists? return !!localgid if @resource.forcelocal? @@ -58,7 +67,8 @@ def create end def addcmd - if @resource.forcelocal? + # The localadd command (lgroupadd) must only be called when libuser is supported. + if Puppet.features.libuser? && @resource.forcelocal? cmd = [command(:localadd)] @custom_environment = Puppet::Util::Libuser.getenv else @@ -86,7 +96,8 @@ def validate_members(members) end def modifycmd(param, value) - if @resource.forcelocal? || @resource[:members] + # The localmodify command (lgroupmod) must only be called when libuser is supported. + if Puppet.features.libuser? && (@resource.forcelocal? || @resource[:members]) cmd = [command(:localmodify)] @custom_environment = Puppet::Util::Libuser.getenv else @@ -109,7 +120,8 @@ def modifycmd(param, value) end def deletecmd - if @resource.forcelocal? + # The localdelete command (lgroupdel) must only be called when libuser is supported. + if Puppet.features.libuser? && @resource.forcelocal? @custom_environment = Puppet::Util::Libuser.getenv [command(:localdelete), @resource[:name]] else @@ -127,7 +139,16 @@ def members_to_s(current) end def purge_members - localmodify('-m', members_to_s(members), @resource.name) + # The groupadd provider doesn't have the ability currently to remove members from a group, libuser does. + # Use libuser's lgroupmod command to achieve purging members if libuser is supported. + # Otherwise use the 'usermod' command. + if Puppet.features.libuser? + localmodify('-m', members_to_s(members), @resource.name) + else + members.each do |member| + purgemember('-rG', @resource.name, member) + end + end end private diff --git a/spec/unit/provider/group/groupadd_spec.rb b/spec/unit/provider/group/groupadd_spec.rb index 1d9f886a012..68c18588193 100644 --- a/spec/unit/provider/group/groupadd_spec.rb +++ b/spec/unit/provider/group/groupadd_spec.rb @@ -44,6 +44,10 @@ end describe "on systems with libuser" do + before do + allow(Puppet.features).to receive(:libuser?).and_return(true) + end + describe "with forcelocal=true" do before do described_class.has_feature(:manages_local_users_and_groups) @@ -71,7 +75,7 @@ describe "with a list of members" do before do members.each { |m| allow(Etc).to receive(:getpwnam).with(m).and_return(true) } - + allow(provider).to receive(:flag).and_return('-M') described_class.has_feature(:manages_members) resource[:forcelocal] = false resource[:members] = members @@ -92,6 +96,10 @@ end describe "on systems with libuser" do + before do + allow(Puppet.features).to receive(:libuser?).and_return(true) + end + describe "with forcelocal=false" do before do described_class.has_feature(:manages_local_users_and_groups) @@ -156,6 +164,7 @@ before { resource[:auth_membership] = false } it "should add to the existing users" do + allow(provider).to receive(:flag).and_return('-M') new_members = ['user1', 'user2', 'user3', 'user4'] allow(provider).to receive(:members).and_return(members) expect(provider).not_to receive(:localmodify).with('-m', members.join(','), 'mygroup') @@ -235,6 +244,10 @@ end describe "on systems with the libuser and forcelocal=false" do + before do + allow(Puppet.features).to receive(:libuser?).and_return(true) + end + before do described_class.has_feature(:manages_local_users_and_groups) resource[:forcelocal] = :false @@ -247,6 +260,10 @@ end describe "on systems with the libuser and forcelocal=true" do + before do + allow(Puppet.features).to receive(:libuser?).and_return(true) + end + before do described_class.has_feature(:manages_local_users_and_groups) resource[:forcelocal] = :true