Merge pull request #1127 from opscode/CHEF-3297-part-2

Add excluded_members support to the rest of the group providers.

19 files changed, 370 insertions, 177 deletions

diff --git a/lib/chef/provider/group.rb b/lib/chef/provider/group.rb
index a89b4a57b3..09c2a0052d 100644
--- a/lib/chef/provider/group.rb
+++ b/lib/chef/provider/group.rb
@@ -125,7 +125,7 @@ class Chef
           end
         else
           if compare_group
-            converge_by(["alter group #{@new_resource}"] + @change_desc) do
+            converge_by(["alter group #{@new_resource}"] + change_desc) do
               manage_group
               Chef::Log.info("#{@new_resource} altered")
             end
@@ -144,7 +144,7 @@ class Chef
 
       def action_manage
         if @group_exists && compare_group
-          converge_by(["manage group #{@new_resource}"] + @change_desc) do
+          converge_by(["manage group #{@new_resource}"] + change_desc) do
             manage_group
             Chef::Log.info("#{@new_resource} managed")
           end
@@ -153,7 +153,7 @@ class Chef
 
       def action_modify
         if compare_group
-          converge_by(["modify group #{@new_resource}"] + @change_desc) do
+          converge_by(["modify group #{@new_resource}"] + change_desc) do
             manage_group
             Chef::Log.info("#{@new_resource} modified")
           end
diff --git a/lib/chef/provider/group/gpasswd.rb b/lib/chef/provider/group/gpasswd.rb
index c28a3db19a..a65a7ffd53 100644
--- a/lib/chef/provider/group/gpasswd.rb
+++ b/lib/chef/provider/group/gpasswd.rb
@@ -39,40 +39,21 @@ class Chef
           end
         end
 
-        def modify_group_members
-          if(@new_resource.append)
-            if @new_resource.members && !@new_resource.members.empty?
-              members_to_be_added = [ ]
-              @new_resource.members.each do |member|
-                members_to_be_added << member if !@current_resource.members.include?(member)
-              end
-              members_to_be_added.each do |member|
-                Chef::Log.debug("#{@new_resource} appending member #{member} to group #{@new_resource.group_name}")
-                shell_out!("gpasswd -a #{member} #{@new_resource.group_name}")
-              end
-            end
-
-            if @new_resource.excluded_members && !@new_resource.excluded_members.empty?
-              members_to_be_removed = [ ]
-              @new_resource.excluded_members.each do |member|
-                members_to_be_removed << member if @current_resource.members.include?(member)
-              end
-
-              members_to_be_removed.each do |member|
-                Chef::Log.debug("#{@new_resource} removing member #{member} from group #{@new_resource.group_name}")
-                shell_out!("gpasswd -d #{member} #{@new_resource.group_name}")
-              end
-            end
+        def set_members(members)
+          unless members.empty?
+            shell_out!("gpasswd -M #{members.join(',')} #{@new_resource.group_name}")
           else
-            unless @new_resource.members.empty?
-              Chef::Log.debug("#{@new_resource} setting group members to #{@new_resource.members.join(', ')}")
-              shell_out!("gpasswd -M #{@new_resource.members.join(',')} #{@new_resource.group_name}")
-            else
-              Chef::Log.debug("#{@new_resource} setting group members to: none")
-              shell_out!("gpasswd -M \"\" #{@new_resource.group_name}")
-            end
+            shell_out!("gpasswd -M \"\" #{@new_resource.group_name}")
           end
         end
+
+        def add_member(member)
+          shell_out!("gpasswd -a #{member} #{@new_resource.group_name}")
+        end
+
+        def remove_member(member)
+          shell_out!("gpasswd -d #{member} #{@new_resource.group_name}")
+        end
       end
     end
   end
diff --git a/lib/chef/provider/group/groupadd.rb b/lib/chef/provider/group/groupadd.rb
index 45ae308612..cb480aab54 100644
--- a/lib/chef/provider/group/groupadd.rb
+++ b/lib/chef/provider/group/groupadd.rb
@@ -65,8 +65,48 @@ class Chef
         end
 
         def modify_group_members
-          raise Chef::Exceptions::Group, "you must override modify_group_members in #{self.to_s}"
+          if @new_resource.append
+            if @new_resource.members && !@new_resource.members.empty?
+              members_to_be_added = [ ]
+              @new_resource.members.each do |member|
+                members_to_be_added << member if !@current_resource.members.include?(member)
+              end
+              members_to_be_added.each do |member|
+                Chef::Log.debug("#{@new_resource} appending member #{member} to group #{@new_resource.group_name}")
+                add_member(member)
+              end
+            end
+
+            if @new_resource.excluded_members && !@new_resource.excluded_members.empty?
+              members_to_be_removed = [ ]
+              @new_resource.excluded_members.each do |member|
+                members_to_be_removed << member if @current_resource.members.include?(member)
+              end
+
+              members_to_be_removed.each do |member|
+                Chef::Log.debug("#{@new_resource} removing member #{member} from group #{@new_resource.group_name}")
+                remove_member(member)
+              end
+            end
+          else
+            members_description = @new_resource.members.empty? ? "none" : @new_resource.members.join(", ")
+            Chef::Log.debug("#{@new_resource} setting group members to: #{members_description}")
+            set_members(@new_resource.members)
+          end
+        end
+
+        def add_member(member)
+          raise Chef::Exceptions::Group, "you must override add_member in #{self.to_s}"
+        end
+
+        def remove_member(member)
+          raise Chef::Exceptions::Group, "you must override remove_member in #{self.to_s}"
+        end
+
+        def set_members(members)
+          raise Chef::Exceptions::Group, "you must override set_members in #{self.to_s}"
         end
+
         # Little bit of magic as per Adam's useradd provider to pull the assign the command line flags
         #
         # ==== Returns
diff --git a/lib/chef/provider/group/groupmod.rb b/lib/chef/provider/group/groupmod.rb
index 10fc680d78..c8b6458db0 100644
--- a/lib/chef/provider/group/groupmod.rb
+++ b/lib/chef/provider/group/groupmod.rb
@@ -44,45 +44,40 @@ class Chef
         # Manage the group when it already exists
         def manage_group
           if @new_resource.append
-            to_add = @new_resource.members.dup
-            to_add.reject! { |user| @current_resource.members.include?(user) }
-
-            to_delete = Array.new
-
-            Chef::Log.debug("#{@new_resource} not changing group members, the group has no members to add") if to_add.empty?
+            members_to_be_added = [ ]
+            if @new_resource.excluded_members && !@new_resource.excluded_members.empty?
+              # First find out if any member needs to be removed
+              members_to_be_removed = [ ]
+              @new_resource.excluded_members.each do |member|
+                members_to_be_removed << member if @current_resource.members.include?(member)
+              end
+
+              unless members_to_be_removed.empty?
+                # We are using a magic trick to remove the groups.
+                reset_group_membership
+
+                # Capture the members we need to add in
+                # members_to_be_added to be added later on.
+                @current_resource.members.each do |member|
+                  members_to_be_added << member unless members_to_be_removed.include?(member)
+                end
+              end
+            end
+
+            if @new_resource.members && !@new_resource.members.empty?
+              @new_resource.members.each do |member|
+                members_to_be_added << member if !@current_resource.members.include?(member)
+              end
+            end
+
+            Chef::Log.debug("#{@new_resource} not changing group members, the group has no members to add") if members_to_be_added.empty?
+
+            add_group_members(members_to_be_added)
           else
-            to_add = @new_resource.members.dup
-            to_add.reject! { |user| @current_resource.members.include?(user) }
-
-            to_delete = @current_resource.members.dup
-            to_delete.reject! { |user| @new_resource.members.include?(user) }
-
+            # We are resetting the members of a group so use the same trick
+            reset_group_membership
             Chef::Log.debug("#{@new_resource} setting group members to: none") if @new_resource.members.empty?
-          end
-
-          if to_delete.empty?
-            # If we are only adding new members to this group, then
-            # call add_group_members with only those users
-            add_group_members(to_add)
-          else
-            Chef::Log.debug("#{@new_resource} removing members #{to_delete.join(', ')}")
-
-            # This is tricky, but works: rename the existing group to
-            # "<name>_bak", create a new group with the same GID and
-            # "<name>", then set correct members on that group
-            rename = "group mod -n #{@new_resource.group_name}_bak #{@new_resource.group_name}"
-            shell_out!(rename)
-
-            create = "group add"
-            create << set_options(:overwrite_gid => true)
-            shell_out!(create)
-
-            # Ignore to_add here, since we're replacing the group we
-            # have to add all members who should be in the group.
             add_group_members(@new_resource.members)
-
-            remove = "group del #{@new_resource.group_name}_bak"
-            shell_out!(remove)
           end
         end
 
@@ -99,6 +94,21 @@ class Chef
           end
         end
 
+        # This is tricky, but works: rename the existing group to
+        # "<name>_bak", create a new group with the same GID and
+        # "<name>", then set correct members on that group
+        def reset_group_membership
+          rename = "group mod -n #{@new_resource.group_name}_bak #{@new_resource.group_name}"
+          shell_out!(rename)
+
+          create = "group add"
+          create << set_options(:overwrite_gid => true)
+          shell_out!(create)
+
+          remove = "group del #{@new_resource.group_name}_bak"
+          shell_out!(remove)
+        end
+
         # Little bit of magic as per Adam's useradd provider to pull and assign the command line flags
         #
         # ==== Returns
diff --git a/lib/chef/provider/group/pw.rb b/lib/chef/provider/group/pw.rb
index 66da8281be..3ec6f6f668 100644
--- a/lib/chef/provider/group/pw.rb
+++ b/lib/chef/provider/group/pw.rb
@@ -39,16 +39,28 @@ class Chef
         def create_group
           command = "pw groupadd"
           command << set_options
-          command << set_members_option
-          run_command(:command => command)
+          member_options = set_members_options
+          if member_options.empty?
+            run_command(:command => command)
+          else
+            member_options.each do |option|
+              run_command(:command => command + option)
+            end
+          end
         end
 
         # Manage the group when it already exists
         def manage_group
           command = "pw groupmod"
           command << set_options
-          command << set_members_option
-          run_command(:command => command)
+          member_options = set_members_options
+          if member_options.empty?
+            run_command(:command => command)
+          else
+            member_options.each do |option|
+              run_command(:command => command + option)
+            end
+          end
         end
 
         # Remove the group
@@ -70,21 +82,52 @@ class Chef
         end
 
         # Set the membership option depending on the current resource states
-        def set_members_option
-          opt = ""
-          unless @new_resource.members.empty?
-            opt << " -M #{@new_resource.members.join(',')}"
-            Chef::Log.debug("#{@new_resource} setting group members to #{@new_resource.members.join(', ')}")
+        def set_members_options
+          opts = [ ]
+          members_to_be_added = [ ]
+          members_to_be_removed = [ ]
+
+          if @new_resource.append
+            # Append is set so we will only add members given in the
+            # members list and remove members given in the
+            # excluded_members list.
+            if @new_resource.members && !@new_resource.members.empty?
+              @new_resource.members.each do |member|
+                members_to_be_added << member if !@current_resource.members.include?(member)
+              end
+            end
+
+            if @new_resource.excluded_members && !@new_resource.excluded_members.empty?
+              @new_resource.excluded_members.each do |member|
+                members_to_be_removed << member if @current_resource.members.include?(member)
+              end
+            end
           else
-            # New member list is empty so we should delete any old group members
-            unless @current_resource.members.empty?
-              opt << " -d #{@current_resource.members.join(',')}"
-              Chef::Log.debug("#{@new_resource} removing group members #{@current_resource.members.join(', ')}")
-            else
-              Chef::Log.debug("#{@new_resource} not changing group members, the group has no members")
+            # Append is not set so we're resetting the membership of
+            # the group to the given members.
+            members_to_be_added = @new_resource.members
+            @current_resource.members.each do |member|
+              # No need to re-add a member if it's present in the new
+              # list of members
+              if members_to_be_added.include? member
+                members_to_be_added.delete member
+              else
+                members_to_be_removed << member
+              end
             end
           end
-          opt
+
+          unless members_to_be_added.empty?
+            Chef::Log.debug("#{@new_resource} adding group members: #{members_to_be_added.join(',')}")
+            opts << " -m #{members_to_be_added.join(',')}"
+          end
+
+          unless members_to_be_removed.empty?
+            Chef::Log.debug("#{@new_resource} removing group members: #{members_to_be_removed.join(',')}")
+            opts << " -d #{members_to_be_removed.join(',')}"
+          end
+
+          opts
         end
 
       end
diff --git a/lib/chef/provider/group/suse.rb b/lib/chef/provider/group/suse.rb
index 4c343bddf9..14380f1705 100644
--- a/lib/chef/provider/group/suse.rb
+++ b/lib/chef/provider/group/suse.rb
@@ -39,21 +39,24 @@ class Chef
           end
         end
 
-        def modify_group_members
-          unless @new_resource.members.empty?
-            if(@new_resource.append)
-              @new_resource.members.each do |member|
-                Chef::Log.debug("#{@new_resource} appending member #{member} to group #{@new_resource.group_name}")
-                shell_out!("groupmod -A #{member} #{@new_resource.group_name}")
-              end
-            else
-              Chef::Log.debug("#{@new_resource} setting group members to #{@new_resource.members.join(', ')}")
-              shell_out!("groupmod -A #{@new_resource.members.join(',')} #{@new_resource.group_name}")
-            end
-          else
-            Chef::Log.debug("#{@new_resource} not changing group members, the group has no members")
+        def set_members(members)
+          unless @current_resource.members.empty?
+            shell_out!("groupmod -R #{@current_resource.members.join(',')} #{@new_resource.group_name}")
           end
+
+          unless members.empty?
+            shell_out!("groupmod -A #{members.join(',')} #{@new_resource.group_name}")
+          end
+        end
+
+        def add_member(member)
+          shell_out!("groupmod -A #{member} #{@new_resource.group_name}")
         end
+
+        def remove_member(member)
+          shell_out!("groupmod -R #{member} #{@new_resource.group_name}")
+        end
+
       end
     end
   end
diff --git a/lib/chef/provider/group/usermod.rb b/lib/chef/provider/group/usermod.rb
index 5788ac8fad..8323c3e9ba 100644
--- a/lib/chef/provider/group/usermod.rb
+++ b/lib/chef/provider/group/usermod.rb
@@ -17,12 +17,15 @@
 #
 
 require 'chef/provider/group/groupadd'
+require 'chef/mixin/shell_out'
 
 class Chef
   class Provider
     class Group
       class Usermod < Chef::Provider::Group::Groupadd
 
+        include Chef::Mixin::ShellOut
+
         def load_current_resource
           super
         end
@@ -36,32 +39,52 @@ class Chef
             # No whyrun alternative: this component should be available in the base install of any given system that uses it
           end
 
-          requirements.assert(:modify, :create) do |a|
+          requirements.assert(:modify, :manage) do |a|
             a.assertion { @new_resource.members.empty? || @new_resource.append }
             a.failure_message Chef::Exceptions::Group, "setting group members directly is not supported by #{self.to_s}, must set append true in group"
             # No whyrun alternative - this action is simply not supported.
           end
-        end
 
-        def modify_group_members
-          case node[:platform]
-          when "openbsd", "netbsd", "aix", "solaris2", "smartos"
-            append_flags = "-G"
-          when "solaris", "suse", "opensuse"
-            append_flags = "-a -G"
+          requirements.assert(:all_actions) do |a|
+            a.assertion { @new_resource.excluded_members.empty? }
+            a.failure_message Chef::Exceptions::Group, "excluded_members is not supported by #{self.to_s}"
+            # No whyrun alternative - this action is simply not supported.
           end
+        end
 
-          unless @new_resource.members.empty?
-            if(@new_resource.append)
-              @new_resource.members.each do |member|
-                Chef::Log.debug("#{@new_resource} appending member #{member} to group #{@new_resource.group_name}")
-                run_command(:command => "usermod #{append_flags} #{@new_resource.group_name} #{member}" )
-              end
+        def set_members(members)
+          return if members.empty?
+          # This provider only supports adding members with
+          # append. Only if the action is create we will go
+          # ahead and add members.
+          if @new_resource.action == :create
+            members.each do |member|
+              add_member(member)
             end
           else
-            Chef::Log.debug("#{@new_resource} not changing group members, the group has no members")
+            raise Chef::Exceptions::UnsupportedAction, "Setting members directly is not supported by #{self.to_s}"
           end
         end
+
+        def add_member(member)
+          shell_out!("usermod #{append_flags} #{@new_resource.group_name} #{member}")
+        end
+
+        def remove_member(member)
+          # This provider only supports adding members with
+          # append. This function should never be called.
+          raise Chef::Exceptions::UnsupportedAction, "Removing members members is not supported by #{self.to_s}"
+        end
+
+        def append_flags
+          case node[:platform]
+          when "openbsd", "netbsd", "aix", "solaris2", "smartos"
+            "-G"
+          when "solaris", "suse", "opensuse"
+            "-a -G"
+          end
+        end
+
       end
     end
   end
diff --git a/lib/chef/resource/group.rb b/lib/chef/resource/group.rb
index 707def9e39..daf851fac6 100644
--- a/lib/chef/resource/group.rb
+++ b/lib/chef/resource/group.rb
@@ -31,6 +31,7 @@ class Chef
         @group_name = name
         @gid = nil
         @members = []
+        @excluded_members = []
         @action = :create
         @append = false
         @non_unique = false
diff --git a/spec/functional/resource/group_spec.rb b/spec/functional/resource/group_spec.rb
index 98ec77a01b..b8369d087b 100644
--- a/spec/functional/resource/group_spec.rb
+++ b/spec/functional/resource/group_spec.rb
@@ -20,7 +20,11 @@
 require 'spec_helper'
 require 'functional/resource/base'
 
-describe Chef::Resource::Group, :requires_root_or_running_windows do
+# Chef::Resource::Group are turned off on Mac OS X 10.6 due to caching
+# issues around Etc.getgrnam() not picking up the group membership
+# changes that are done on the system. Etc.endgrent is not functioning
+# correctly on certain 10.6 boxes.
+describe Chef::Resource::Group, :requires_root_or_running_windows, :not_supported_on_mac_osx_106 do
   def group_should_exist(group)
     case ohai[:platform_family]
     when "debian", "fedora", "rhel", "suse", "gentoo", "slackware", "arch"
@@ -55,6 +59,9 @@ describe Chef::Resource::Group, :requires_root_or_running_windows do
 
   def user(username)
     usr = Chef::Resource::User.new("#{username}", run_context)
+    if ohai[:platform_family] == "windows"
+      usr.password("ComplexPass11!")
+    end
     usr
   end
 
@@ -101,24 +108,24 @@ describe Chef::Resource::Group, :requires_root_or_running_windows do
     end
 
     describe "when append is not set" do
-      let(:included_members) { ["group-spec-Eric"] }
+      let(:included_members) { ["spec-Eric"] }
 
       before do
-        create_user("group-spec-Eric")
-        create_user("group-spec-Gordon")
-        add_members_to_group(["group-spec-Gordon"])
+        create_user("spec-Eric")
+        create_user("spec-Gordon")
+        add_members_to_group(["spec-Gordon"])
       end
 
       after do
-        remove_user("group-spec-Eric")
-        remove_user("group-spec-Gordon")
+        remove_user("spec-Eric")
+        remove_user("spec-Gordon")
       end
 
       it "should remove the existing users and add the new users to the group" do
         group_resource.run_action(tested_action)
 
-        user_exist_in_group?("group-spec-Eric").should == true
-        user_exist_in_group?("group-spec-Gordon").should == false
+        user_exist_in_group?("spec-Eric").should == true
+        user_exist_in_group?("spec-Gordon").should == false
       end
     end
 
@@ -153,7 +160,7 @@ describe Chef::Resource::Group, :requires_root_or_running_windows do
 
         describe "when group contains some users" do
           before(:each) do
-            add_members_to_group([ "group-spec-Gordon", "group-spec-Anthony" ])
+            add_members_to_group([ "spec-Gordon", "spec-Anthony" ])
           end
 
           it "should add the included users and remove excluded users" do
@@ -185,7 +192,7 @@ describe Chef::Resource::Group, :requires_root_or_running_windows do
     end
   end
 
-  let(:group_name) { "chef-rspec-test-#{SecureRandom.random_number(9999)}" }
+  let(:group_name) { "cheftest-#{SecureRandom.random_number(9999)}" }
   let(:included_members) { nil }
   let(:excluded_members) { nil }
   let(:group_resource) {
@@ -234,7 +241,8 @@ downthestreetalwayshadagoodsmileonhisfacetheoldmanwalkingdownthestreeQQQQQQ" }
       end
     end
 
-    describe "should raise an error when same member is included in the members and excluded_members" do
+    # not_supported_on_solaris because of the use of excluded_members
+    describe "should raise an error when same member is included in the members and excluded_members", :not_supported_on_solaris do
       it "should raise an error" do
         invalid_resource = group_resource.dup
         invalid_resource.members(["Jack"])
@@ -265,9 +273,9 @@ downthestreetalwayshadagoodsmileonhisfacetheoldmanwalkingdownthestreeQQQQQQ" }
     end
   end
 
-  describe "group modify action" do
-    let(:included_members) { ["group-spec-Gordon", "group-spec-Eric"] }
-    let(:excluded_members) { ["group-spec-Anthony"] }
+  describe "group modify action", :not_supported_on_solaris do
+    let(:included_members) { ["spec-Gordon", "spec-Eric"] }
+    let(:excluded_members) { ["spec-Anthony"] }
     let(:tested_action) { :modify }
 
     describe "when there is no group" do
@@ -281,9 +289,9 @@ downthestreetalwayshadagoodsmileonhisfacetheoldmanwalkingdownthestreeQQQQQQ" }
     end
   end
 
-  describe "group manage action" do
-    let(:included_members) { ["group-spec-Gordon", "group-spec-Eric"] }
-    let(:excluded_members) { ["group-spec-Anthony"] }
+  describe "group manage action", :not_supported_on_solaris do
+    let(:included_members) { ["spec-Gordon", "spec-Eric"] }
+    let(:excluded_members) { ["spec-Anthony"] }
     let(:tested_action) { :manage }
 
     describe "when there is no group" do
@@ -297,5 +305,39 @@ downthestreetalwayshadagoodsmileonhisfacetheoldmanwalkingdownthestreeQQQQQQ" }
       it_behaves_like "correct group management"
     end
   end
+
+  describe "group resource with Usermod provider", :solaris_only do
+    describe "when excluded_members is set" do
+      let(:excluded_members) { ["spec-Anthony"] }
+
+      it ":manage should raise an error" do
+        lambda {group_resource.run_action(:manage) }.should raise_error
+      end
+
+      it ":modify should raise an error" do
+        lambda {group_resource.run_action(:modify) }.should raise_error
+      end
+
+      it ":create should raise an error" do
+        lambda {group_resource.run_action(:create) }.should raise_error
+      end
+    end
+
+    describe "when append is not set" do
+      let(:included_members) { ["spec-Gordon", "spec-Eric"] }
+
+      before(:each) do
+        group_resource.append(false)
+      end
+
+      it ":manage should raise an error" do
+        lambda {group_resource.run_action(:manage) }.should raise_error
+      end
+
+      it ":modify should raise an error" do
+        lambda {group_resource.run_action(:modify) }.should raise_error
+      end
+    end
+  end
 end
 
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 3b75e51214..e60bdfd434 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -102,11 +102,13 @@ RSpec.configure do |config|
 
   # Add jruby filters here
   config.filter_run_excluding :windows_only => true unless windows?
+  config.filter_run_excluding :not_supported_on_mac_osx_106 => true if mac_osx_106?
   config.filter_run_excluding :not_supported_on_win2k3 => true if windows_win2k3?
   config.filter_run_excluding :not_supported_on_solaris => true if solaris?
   config.filter_run_excluding :win2k3_only => true unless windows_win2k3?
   config.filter_run_excluding :windows64_only => true unless windows64?
   config.filter_run_excluding :windows32_only => true unless windows32?
+  config.filter_run_excluding :solaris_only => true unless solaris?
   config.filter_run_excluding :system_windows_service_gem_only => true unless system_windows_service_gem?
   config.filter_run_excluding :unix_only => true unless unix?
   config.filter_run_excluding :supports_cloexec => true unless supports_cloexec?
diff --git a/spec/support/platform_helpers.rb b/spec/support/platform_helpers.rb
index b9c17ec4d6..66f1fd4b25 100644
--- a/spec/support/platform_helpers.rb
+++ b/spec/support/platform_helpers.rb
@@ -1,4 +1,7 @@
 require 'fcntl'
+require 'chef/mixin/shell_out'
+
+include Chef::Mixin::ShellOut
 
 def ruby_gte_20?
   RUBY_VERSION.to_f >= 2.0
@@ -32,6 +35,19 @@ def windows_win2k3?
   (host.version && host.version.start_with?("5.2"))
 end
 
+def mac_osx_106?
+  if File.exists? "/usr/bin/sw_vers"
+    result = shell_out("/usr/bin/sw_vers")
+    result.stdout.each_line do |line|
+      if line =~ /^ProductVersion:\s10.6.*$/
+        return true
+      end
+    end
+  end
+
+  false
+end
+
 # detects if the hardware is 64-bit (evaluates to true in "WOW64" mode in a 32-bit app on a 64-bit system)
 def windows64?
   windows? && ( ENV['PROCESSOR_ARCHITECTURE'] == 'AMD64' || ENV['PROCESSOR_ARCHITEW6432'] == 'AMD64' )
diff --git a/spec/unit/provider/group/dscl_spec.rb b/spec/unit/provider/group/dscl_spec.rb
index 28d78cbdbd..360a799b5f 100644
--- a/spec/unit/provider/group/dscl_spec.rb
+++ b/spec/unit/provider/group/dscl_spec.rb
@@ -265,6 +265,7 @@ describe Chef::Provider::Group::Dscl do
     it "should manage the group_name if it changed and the new resources group_name is not null" do
       @current_resource.group_name("oldval")
       @new_resource.group_name("newname")
+      @provider.should_receive(:set_members).and_return(true)
       @provider.should_receive(:safe_dscl).with("create /Groups/newname")
       @provider.should_receive(:safe_dscl).with("create /Groups/newname Password '*'")
       @provider.manage_group
diff --git a/spec/unit/provider/group/gpasswd_spec.rb b/spec/unit/provider/group/gpasswd_spec.rb
index 8889ba322e..f765667300 100644
--- a/spec/unit/provider/group/gpasswd_spec.rb
+++ b/spec/unit/provider/group/gpasswd_spec.rb
@@ -76,8 +76,7 @@ describe Chef::Provider::Group::Gpasswd, "modify_group_members" do
         @new_resource.members([])
       end
 
-      it "logs a message and does not modify group membership" do
-        Chef::Log.should_receive(:debug).with("group[wheel] not changing group members, the group has no members to add")
+      it "does not modify group membership" do
         @provider.should_not_receive(:shell_out!)
         @provider.modify_group_members
       end
@@ -85,7 +84,7 @@ describe Chef::Provider::Group::Gpasswd, "modify_group_members" do
 
     describe "when the resource specifies group members" do
       it "should log an appropriate debug message" do
-        Chef::Log.should_receive(:debug).with("group[wheel] setting group members to lobster, rage, fist")
+        Chef::Log.should_receive(:debug).with("group[wheel] setting group members to: lobster, rage, fist")
         @provider.stub!(:shell_out!)
         @provider.modify_group_members
       end
@@ -95,12 +94,20 @@ describe Chef::Provider::Group::Gpasswd, "modify_group_members" do
         @provider.modify_group_members
       end
 
-      it "should run gpasswd individually for each user when the append option is set" do
-        @new_resource.append(true)
-        @provider.should_receive(:shell_out!).with("gpasswd -a lobster wheel")
-        @provider.should_receive(:shell_out!).with("gpasswd -a rage wheel")
-        @provider.should_receive(:shell_out!).with("gpasswd -a fist wheel")
-        @provider.modify_group_members
+      describe "when no user exists in the system" do
+        before do
+          current_resource = @new_resource.dup
+          current_resource.members([ ])
+          @provider.current_resource = current_resource
+        end
+
+        it "should run gpasswd individually for each user when the append option is set" do
+          @new_resource.append(true)
+          @provider.should_receive(:shell_out!).with("gpasswd -a lobster wheel")
+          @provider.should_receive(:shell_out!).with("gpasswd -a rage wheel")
+          @provider.should_receive(:shell_out!).with("gpasswd -a fist wheel")
+          @provider.modify_group_members
+        end
       end
 
     end
diff --git a/spec/unit/provider/group/groupadd_spec.rb b/spec/unit/provider/group/groupadd_spec.rb
index 0cc1167142..91073727c8 100644
--- a/spec/unit/provider/group/groupadd_spec.rb
+++ b/spec/unit/provider/group/groupadd_spec.rb
@@ -143,10 +143,9 @@ describe Chef::Provider::Group::Groupadd do
     end
   end
 
-  describe "modify_group_members" do
-
-    it "should raise an error when calling modify_group_members" do
-      lambda { @provider.modify_group_members }.should raise_error(Chef::Exceptions::Group, "you must override modify_group_members in #{@provider.to_s}")
+  [:add_member, :remove_member, :set_members].each do |m|
+    it "should raise an error when calling #{m}" do
+      lambda { @provider.send(m, [ ]) }.should raise_error(Chef::Exceptions::Group, "you must override #{m} in #{@provider.to_s}")
     end
   end
 
diff --git a/spec/unit/provider/group/groupmod_spec.rb b/spec/unit/provider/group/groupmod_spec.rb
index 69b96c6edb..bfacfa5c26 100644
--- a/spec/unit/provider/group/groupmod_spec.rb
+++ b/spec/unit/provider/group/groupmod_spec.rb
@@ -62,7 +62,6 @@ describe Chef::Provider::Group::Groupmod do
 
         it "logs a message and sets group's members to 'none', then removes existing group members" do
           Chef::Log.should_receive(:debug).with("group[wheel] setting group members to: none")
-          Chef::Log.should_receive(:debug).with("group[wheel] removing members lobster, rage, fist")
           @provider.should_receive(:shell_out!).with("group mod -n wheel_bak wheel")
           @provider.should_receive(:shell_out!).with("group add -g '123' -o wheel")
           @provider.should_receive(:shell_out!).with("group del wheel_bak")
diff --git a/spec/unit/provider/group/pw_spec.rb b/spec/unit/provider/group/pw_spec.rb
index e7c38f9555..a545f8ee70 100644
--- a/spec/unit/provider/group/pw_spec.rb
+++ b/spec/unit/provider/group/pw_spec.rb
@@ -49,7 +49,7 @@ describe Chef::Provider::Group::Pw do
   describe "when creating a group" do
     it "should run pw groupadd with the return of set_options and set_members_option" do
       @new_resource.gid(23)
-      @provider.should_receive(:run_command).with({ :command => "pw groupadd wheel -g '23' -M root,aj" }).and_return(true)
+      @provider.should_receive(:run_command).with({ :command => "pw groupadd wheel -g '23'" }).and_return(true)
       @provider.create_group
     end
   end
@@ -58,7 +58,9 @@ describe Chef::Provider::Group::Pw do
 
     it "should run pw groupmod with the return of set_options" do
       @new_resource.gid(42)
-      @provider.should_receive(:run_command).with({ :command => "pw groupmod wheel -g '42' -M root,aj" }).and_return(true)
+      @new_resource.members(["someone"])
+      @provider.should_receive(:run_command).with({ :command => "pw groupmod wheel -g '42' -m someone" }).and_return(true)
+      @provider.should_receive(:run_command).with({ :command => "pw groupmod wheel -g '42' -d root,aj" }).and_return(true)
       @provider.manage_group
     end
 
@@ -79,13 +81,8 @@ describe Chef::Provider::Group::Pw do
         @current_resource.stub!(:members).and_return([])
       end
 
-      it "should log an appropriate message" do
-        Chef::Log.should_receive(:debug).with("group[wheel] not changing group members, the group has no members")
-        @provider.set_members_option
-      end
-
       it "should set no options" do
-        @provider.set_members_option.should eql("")
+        @provider.set_members_options.should eql([ ])
       end
     end
 
@@ -96,12 +93,12 @@ describe Chef::Provider::Group::Pw do
       end
 
       it "should log an appropriate message" do
-        Chef::Log.should_receive(:debug).with("group[wheel] removing group members all, your, base")
-        @provider.set_members_option
+        Chef::Log.should_receive(:debug).with("group[wheel] removing group members: all,your,base")
+        @provider.set_members_options
       end
 
       it "should set the -d option with the members joined by ','" do
-        @provider.set_members_option.should eql(" -d all,your,base")
+        @provider.set_members_options.should eql([ " -d all,your,base" ])
       end
     end
 
@@ -112,12 +109,12 @@ describe Chef::Provider::Group::Pw do
       end
 
       it "should log an appropriate debug message" do
-        Chef::Log.should_receive(:debug).with("group[wheel] setting group members to all, your, base")
-        @provider.set_members_option
+        Chef::Log.should_receive(:debug).with("group[wheel] adding group members: all,your,base")
+        @provider.set_members_options
       end
 
       it "should set the -M option with the members joined by ','" do
-        @provider.set_members_option.should eql(" -M all,your,base")
+        @provider.set_members_options.should eql([ " -m all,your,base" ])
       end
     end
   end
diff --git a/spec/unit/provider/group/usermod_spec.rb b/spec/unit/provider/group/usermod_spec.rb
index 7f2931f2e2..1687cc0b65 100644
--- a/spec/unit/provider/group/usermod_spec.rb
+++ b/spec/unit/provider/group/usermod_spec.rb
@@ -25,6 +25,7 @@ describe Chef::Provider::Group::Usermod do
     @run_context = Chef::RunContext.new(@node, {}, @events)
     @new_resource = Chef::Resource::Group.new("wheel")
     @new_resource.members [ "all", "your", "base" ]
+    @new_resource.excluded_members [ ]
     @provider = Chef::Provider::Group::Usermod.new(@new_resource, @run_context)
     @provider.stub!(:run_command)
   end
@@ -33,11 +34,12 @@ describe Chef::Provider::Group::Usermod do
 
     describe "with an empty members array" do
       before do
+        @new_resource.stub!(:append).and_return(true)
         @new_resource.stub!(:members).and_return([])
       end
 
       it "should log an appropriate message" do
-        Chef::Log.should_receive(:debug).with("group[wheel] not changing group members, the group has no members")
+        @provider.should_not_receive(:shell_out!)
         @provider.modify_group_members
       end
     end
@@ -65,13 +67,26 @@ describe Chef::Provider::Group::Usermod do
         lambda { @provider.run_action(@provider.process_resource_requirements) }.should raise_error(Chef::Exceptions::Group, "setting group members directly is not supported by #{@provider.to_s}, must set append true in group")
       end
 
+      it "should raise an error when excluded_members are set" do
+        @provider.define_resource_requirements
+        @provider.load_current_resource
+        @provider.instance_variable_set("@group_exists", true)
+        @provider.action = :modify
+        @new_resource.stub!(:append).and_return(true)
+        @new_resource.stub!(:excluded_members).and_return(["someone"])
+        lambda { @provider.run_action(@provider.process_resource_requirements) }.should raise_error(Chef::Exceptions::Group, "excluded_members is not supported by #{@provider.to_s}")
+      end
+
       platforms.each do |platform, flags|
         it "should usermod each user when the append option is set on #{platform}" do
+          current_resource = @new_resource.dup
+          current_resource.members([ ])
+          @provider.current_resource = current_resource
           @node.automatic_attrs[:platform] = platform
           @new_resource.stub!(:append).and_return(true)
-          @provider.should_receive(:run_command).with({:command => "usermod #{flags} wheel all"})
-          @provider.should_receive(:run_command).with({:command => "usermod #{flags} wheel your"})
-          @provider.should_receive(:run_command).with({:command => "usermod #{flags} wheel base"})
+          @provider.should_receive(:shell_out!).with("usermod #{flags} wheel all")
+          @provider.should_receive(:shell_out!).with("usermod #{flags} wheel your")
+          @provider.should_receive(:shell_out!).with("usermod #{flags} wheel base")
           @provider.modify_group_members
         end
       end
diff --git a/spec/unit/provider/group/windows_spec.rb b/spec/unit/provider/group/windows_spec.rb
index a65cfc241d..14a4f4bb40 100644
--- a/spec/unit/provider/group/windows_spec.rb
+++ b/spec/unit/provider/group/windows_spec.rb
@@ -70,14 +70,6 @@ describe Chef::Provider::Group::Windows do
       @provider.manage_group
     end
 
-    it "should call @net_group.local_set_members if append fails" do
-      @new_resource.stub!(:append).and_return(true)
-      @net_group.stub!(:local_add_members).and_raise(ArgumentError)
-      @net_group.should_receive(:local_add_members).with(@new_resource.members)
-      @net_group.should_receive(:local_set_members).with(@new_resource.members + @current_resource.members)
-      @provider.manage_group
-    end
-
   end
 
   describe "remove_group" do
diff --git a/spec/unit/provider/group_spec.rb b/spec/unit/provider/group_spec.rb
index caad9878b6..a076593511 100644
--- a/spec/unit/provider/group_spec.rb
+++ b/spec/unit/provider/group_spec.rb
@@ -108,6 +108,19 @@ describe Chef::Provider::User do
       @provider.compare_group.should be_true
     end
 
+    it "should return false if append is true and excluded_members include a non existing member" do
+      @new_resource.excluded_members << "extra_user"
+      @new_resource.stub!(:append).and_return(true)
+      @provider.compare_group.should be_false
+    end
+
+    it "should return true if the append is true and excluded_members include an existing user" do
+      @new_resource.members.each {|m| @new_resource.excluded_members << m }
+      @new_resource.members.clear
+      @new_resource.stub!(:append).and_return(true)
+      @provider.compare_group.should be_true
+    end
+
   end
 
   describe "when creating a group" do
@@ -127,6 +140,7 @@ describe Chef::Provider::User do
     it "should check to see if the group has mismatched attributes if the group exists" do
       @provider.group_exists = true
       @provider.stub!(:compare_group).and_return(false)
+      @provider.stub!(:change_desc).and_return([ ])
       @provider.run_action(:create)
       @provider.new_resource.should_not be_updated
     end
@@ -134,6 +148,7 @@ describe Chef::Provider::User do
     it "should call manage_group if the group exists and has mismatched attributes" do
       @provider.group_exists = true
       @provider.stub!(:compare_group).and_return(true)
+      @provider.stub!(:change_desc).and_return([ ])
       @provider.should_receive(:manage_group).and_return(true)
       @provider.run_action(:create)
     end
@@ -141,6 +156,7 @@ describe Chef::Provider::User do
     it "should set the new_resources updated flag when it creates the group if we call manage_group" do
       @provider.group_exists = true
       @provider.stub!(:compare_group).and_return(true)
+      @provider.stub!(:change_desc).and_return(["Some changes are going to be done."])
       @provider.stub!(:manage_group).and_return(true)
       @provider.run_action(:create)
       @new_resource.should be_updated
@@ -172,12 +188,14 @@ describe Chef::Provider::User do
 
     it "should run manage_group if the group exists and has mismatched attributes" do
       @provider.should_receive(:compare_group).and_return(true)
+      @provider.stub!(:change_desc).and_return(["Some changes are going to be done."])
       @provider.should_receive(:manage_group).and_return(true)
       @provider.run_action(:manage)
     end
 
     it "should set the new resources updated flag to true if manage_group is called" do
       @provider.stub!(:compare_group).and_return(true)
+      @provider.stub!(:change_desc).and_return(["Some changes are going to be done."])
       @provider.stub!(:manage_group).and_return(true)
       @provider.run_action(:manage)
       @new_resource.should be_updated
@@ -191,6 +209,7 @@ describe Chef::Provider::User do
 
     it "should not run manage_group if the group exists but has no differing attributes" do
       @provider.should_receive(:compare_group).and_return(false)
+      @provider.stub!(:change_desc).and_return(["Some changes are going to be done."])
       @provider.should_not_receive(:manage_group)
       @provider.run_action(:manage)
     end
@@ -204,12 +223,14 @@ describe Chef::Provider::User do
 
     it "should run manage_group if the group exists and has mismatched attributes" do
       @provider.should_receive(:compare_group).and_return(true)
+      @provider.stub!(:change_desc).and_return(["Some changes are going to be done."])
       @provider.should_receive(:manage_group).and_return(true)
       @provider.run_action(:modify)
     end
 
     it "should set the new resources updated flag to true if manage_group is called" do
       @provider.stub!(:compare_group).and_return(true)
+      @provider.stub!(:change_desc).and_return(["Some changes are going to be done."])
       @provider.stub!(:manage_group).and_return(true)
       @provider.run_action(:modify)
       @new_resource.should be_updated
@@ -217,6 +238,7 @@ describe Chef::Provider::User do
 
     it "should not run manage_group if the group exists but has no differing attributes" do
       @provider.should_receive(:compare_group).and_return(false)
+      @provider.stub!(:change_desc).and_return(["Some changes are going to be done."])
       @provider.should_not_receive(:manage_group)
       @provider.run_action(:modify)
     end
@@ -233,26 +255,26 @@ describe Chef::Provider::User do
        @new_resource.members << "user2"
        @new_resource.stub!(:append).and_return true
        @provider.compare_group.should be_true
-       @provider.change_desc.should == "add missing member(s): user1, user2"
+       @provider.change_desc.should == [ "add missing member(s): user1, user2" ]
     end
 
     it "should report that the group members will be overwritten if not appending" do
        @new_resource.members << "user1"
        @new_resource.stub!(:append).and_return false
        @provider.compare_group.should be_true
-       @provider.change_desc.should == "replace group members with new list of members"
+       @provider.change_desc.should == [ "replace group members with new list of members" ]
     end
 
     it "should report the gid will be changed when it does not match" do
       @current_resource.stub!(:gid).and_return("BADF00D")
       @provider.compare_group.should be_true
-      @provider.change_desc.should == "change gid #{@current_resource.gid} to #{@new_resource.gid}"
+      @provider.change_desc.should == [ "change gid #{@current_resource.gid} to #{@new_resource.gid}" ]
 
     end
 
     it "should report no change reason when no change is required" do
       @provider.compare_group.should be_false
-      @provider.change_desc.should == nil
+      @provider.change_desc.should == [ ]
     end
   end