Fix group output and windows support

Fixes both #10640 and #10641.

* Take the description we build for doc-formatters and put them in the
logs
* Extract out parts of `compare_group` that need to be different on
Windows and implement them in the subclass
* Lookup member SIDs on windows when comparing
* don't compare GIDs on windows.

Signed-off-by: Phil Dibowitz <phil@ipom.com>

2 files changed, 24 insertions, 5 deletions

diff --git a/lib/chef/provider/group.rb b/lib/chef/provider/group.rb
index 905a22dc33..2615d1eb5b 100644
--- a/lib/chef/provider/group.rb
+++ b/lib/chef/provider/group.rb
@@ -78,7 +78,7 @@ class Chef
       # <false>:: If a change is not required
       def compare_group
         @change_desc = [ ]
-        if new_resource.gid.to_s != current_resource.gid.to_s
+        unless group_gid_match?
           @change_desc << "change gid #{current_resource.gid} to #{new_resource.gid}"
         end
 
@@ -103,13 +103,21 @@ class Chef
           unless members_to_be_removed.empty?
             @change_desc << "remove existing member(s): #{members_to_be_removed.join(", ")}"
           end
-        elsif new_resource.members != current_resource.members
-          @change_desc << "replace group members with new list of members"
+        elsif !group_members_match?
+          @change_desc << "replace group members with new list of members: #{new_resource.members}"
         end
 
         !@change_desc.empty?
       end
 
+      def group_gid_match?
+        new_resource.gid.to_s == current_resource.gid.to_s
+      end
+
+      def group_members_match?
+        new_resource.members.sort == current_resource.members.sort
+      end
+
       def has_current_group_member?(member)
         current_resource.members.include?(member)
       end
@@ -131,7 +139,7 @@ class Chef
           if compare_group
             converge_by(["alter group #{new_resource.group_name}"] + change_desc) do
               manage_group
-              logger.info("#{new_resource} altered")
+              logger.info("#{new_resource} altered: #{change_desc.join(', ')}")
             end
           end
         end
diff --git a/lib/chef/provider/group/windows.rb b/lib/chef/provider/group/windows.rb
index 45914b16e3..dacfc348f7 100644
--- a/lib/chef/provider/group/windows.rb
+++ b/lib/chef/provider/group/windows.rb
@@ -33,6 +33,17 @@ class Chef
           @net_group = Chef::Util::Windows::NetGroup.new(new_resource.group_name)
         end
 
+        def group_members_match?
+          sorted_members_sids = new_resource.members.map { |x| lookup_account_name(x) }.sort
+          sorted_current_sids = current_resource.members.sort
+          Chef::Log.debug("#{new_resource.name}: current_members: #{sorted_current_sids} vs new_members #{sorted_members_sids}")
+          sorted_members_sids == sorted_current_sids
+        end
+
+        def group_gid_match?
+          true
+        end
+
         def load_current_resource
           @current_resource = Chef::Resource::Group.new(new_resource.name)
           current_resource.group_name(new_resource.group_name)
@@ -74,7 +85,7 @@ class Chef
               members_to_be_removed << member if has_current_group_member?(member)
             end
             @net_group.local_delete_members(members_to_be_removed) unless members_to_be_removed.empty?
-          else
+          elsif !group_members_match?
             @net_group.local_set_members(new_resource.members)
           end
           @net_group.local_group_set_info(new_resource.comment) if new_resource.comment