Merge pull request #9346 from shubhamshinde360/PUP-12041

Author: joshcooper

lib/puppet/provider/group/groupadd.rb

@@ -17,11 +17,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?
@@ -63,7 +72,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
@@ -91,7 +101,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
@@ -114,7 +125,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
@@ -133,7 +145,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

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