diff options
author | Lamont Granquist <lamont@scriptkiddie.org> | 2016-12-21 13:36:16 -0800 |
---|---|---|
committer | Lamont Granquist <lamont@scriptkiddie.org> | 2016-12-21 13:36:16 -0800 |
commit | d5517ce1e766c096dde16e5c8a10732ee346c5dc (patch) | |
tree | 70d8ecf73c8b782cb09ef527c0ec66f1b3787ad1 | |
parent | 81e4f3ee85fc4e9bef11c3468dcd1cb8d30f4cea (diff) | |
download | chef-d5517ce1e766c096dde16e5c8a10732ee346c5dc.tar.gz |
cleanup of group providerlcg/group-provider-cleanup
- consistent use of shell_out_compact!
- remove more uses of run_command
- some other code cleanup
Signed-off-by: Lamont Granquist <lamont@scriptkiddie.org>
-rw-r--r-- | lib/chef/mixin/shell_out.rb | 70 | ||||
-rw-r--r-- | lib/chef/provider/group.rb | 87 | ||||
-rw-r--r-- | lib/chef/provider/group/aix.rb | 34 | ||||
-rw-r--r-- | lib/chef/provider/group/dscl.rb | 85 | ||||
-rw-r--r-- | lib/chef/provider/group/gpasswd.rb | 14 | ||||
-rw-r--r-- | lib/chef/provider/group/groupadd.rb | 18 | ||||
-rw-r--r-- | lib/chef/provider/group/groupmod.rb | 57 | ||||
-rw-r--r-- | lib/chef/provider/group/pw.rb | 59 | ||||
-rw-r--r-- | lib/chef/provider/group/suse.rb | 18 | ||||
-rw-r--r-- | lib/chef/provider/group/usermod.rb | 21 | ||||
-rw-r--r-- | lib/chef/provider/group/windows.rb | 38 | ||||
-rw-r--r-- | spec/unit/provider/group/dscl_spec.rb | 16 | ||||
-rw-r--r-- | spec/unit/provider/group/gpasswd_spec.rb | 20 | ||||
-rw-r--r-- | spec/unit/provider/group/groupadd_spec.rb | 16 | ||||
-rw-r--r-- | spec/unit/provider/group/groupmod_spec.rb | 32 | ||||
-rw-r--r-- | spec/unit/provider/group/pw_spec.rb | 22 | ||||
-rw-r--r-- | spec/unit/provider/group/suse_spec.rb | 10 | ||||
-rw-r--r-- | spec/unit/provider/group/usermod_spec.rb | 30 |
18 files changed, 334 insertions, 313 deletions
diff --git a/lib/chef/mixin/shell_out.rb b/lib/chef/mixin/shell_out.rb index d341e3d1ec..dfdb19821f 100644 --- a/lib/chef/mixin/shell_out.rb +++ b/lib/chef/mixin/shell_out.rb @@ -21,6 +21,60 @@ class Chef module Mixin module ShellOut + # PREFERRED APIS: + # + # shell_out_compact and shell_out_compact! flatten their array arguments and remove nils and pass + # the resultant array to shell_out. this actually eliminates spaces-in-args bugs because this: + # + # shell_out!("command #{arg}") + # + # becomes two arguments if arg has spaces and requires quotations: + # + # shell_out!("command '#{arg}'") + # + # using shell_out_compact! this becomes: + # + # shell_out_compact!("command", arg) + # + # and spaces in the arg just works and it does not become two arguments (and the shell quoting around + # the argument must actually be removed). + # + # there's also an implicit join between all the array elements, and nested arrays are flattened which + # means that odd where-do-i-put-the-spaces options handling just works, and instead of this: + # + # opts = "" # needs to be empty string for when foo and bar are both missing + # opts << " -foo" if needs_foo? # needs the leading space on both of these + # opts << " -bar" if needs_bar? + # shell_out!("cmd#{opts}") # have to think way too hard about why there's no space here + # + # becomes: + # + # opts = [] + # opts << "-foo" if needs_foo? + # opts << "-bar" if needs_bar? + # shell_out_compact!("cmd", opts) + # + # and opts can be an empty array or nil and it'll work out fine. + # + # generally its best to use shell_out_compact! in code and setup expectations on shell_out! in tests + # + + def shell_out_compact(*args, **options) + if options.empty? + shell_out(*clean_array(*args)) + else + shell_out(*clean_array(*args), **options) + end + end + + def shell_out_compact!(*args, **options) + if options.empty? + shell_out!(*clean_array(*args)) + else + shell_out!(*clean_array(*args), **options) + end + end + # shell_out! runs a command on the system and will raise an error if the command fails, which is what you want # for debugging, shell_out and shell_out! both will display command output to the tty when the log level is debug # Generally speaking, 'extend Chef::Mixin::ShellOut' in your recipes and include 'Chef::Mixin::ShellOut' in your LWRPs @@ -85,11 +139,11 @@ class Chef # @param args [String] variable number of string arguments # @return [String] nicely concatenated string or empty string def a_to_s(*args) - # FIXME: this should be cleaned up and deprecated - clean_array(*args).join(" ") + # FIXME: this should be deprecated in favor of shell_out_compact/shell_out_compact! + args.flatten.reject { |i| i.nil? || i == "" }.map(&:to_s).join(" ") end - # Helper for sublcasses to reject nil and empty strings out of an array. It allows + # Helper for sublcasses to reject nil out of an array. It allows # using the array form of shell_out (which avoids the need to surround arguments with # quote marks to deal with shells). # @@ -106,15 +160,7 @@ class Chef # @param args [String] variable number of string arguments # @return [Array] array of strings with nil and null string rejection def clean_array(*args) - args.flatten.reject { |i| i.nil? || i == "" }.map(&:to_s) - end - - def shell_out_compact(*args) - shell_out(*clean_array(*args)) - end - - def shell_out_compact!(*args) - shell_out!(*clean_array(*args)) + args.flatten.compact.map(&:to_s) end private diff --git a/lib/chef/provider/group.rb b/lib/chef/provider/group.rb index 8936bd2031..6751052ae4 100644 --- a/lib/chef/provider/group.rb +++ b/lib/chef/provider/group.rb @@ -39,38 +39,38 @@ class Chef end def load_current_resource - @current_resource = Chef::Resource::Group.new(@new_resource.name) - @current_resource.group_name(@new_resource.group_name) + @current_resource = Chef::Resource::Group.new(new_resource.name) + current_resource.group_name(new_resource.group_name) group_info = nil begin - group_info = Etc.getgrnam(@new_resource.group_name) - rescue ArgumentError => e + group_info = Etc.getgrnam(new_resource.group_name) + rescue ArgumentError @group_exists = false - Chef::Log.debug("#{@new_resource} group does not exist") + Chef::Log.debug("#{new_resource} group does not exist") end if group_info - @new_resource.gid(group_info.gid) unless @new_resource.gid - @current_resource.gid(group_info.gid) - @current_resource.members(group_info.mem) + new_resource.gid(group_info.gid) unless new_resource.gid + current_resource.gid(group_info.gid) + current_resource.members(group_info.mem) end - @current_resource + current_resource end def define_resource_requirements requirements.assert(:modify) do |a| a.assertion { @group_exists } - a.failure_message(Chef::Exceptions::Group, "Cannot modify #{@new_resource} - group does not exist!") - a.whyrun("Group #{@new_resource} does not exist. Unless it would have been created earlier in this run, this attempt to modify it would fail.") + a.failure_message(Chef::Exceptions::Group, "Cannot modify #{new_resource} - group does not exist!") + a.whyrun("Group #{new_resource} does not exist. Unless it would have been created earlier in this run, this attempt to modify it would fail.") end requirements.assert(:all_actions) do |a| # Make sure that the resource doesn't contain any common # user names in the members and exclude_members properties. - if !@new_resource.members.nil? && !@new_resource.excluded_members.nil? - common_members = @new_resource.members & @new_resource.excluded_members + if !new_resource.members.nil? && !new_resource.excluded_members.nil? + common_members = new_resource.members & new_resource.excluded_members a.assertion { common_members.empty? } a.failure_message(Chef::Exceptions::ConflictingMembersInGroup, "Attempting to both add and remove users from a group: '#{common_members.join(', ')}'") # No why-run alternative @@ -86,41 +86,39 @@ 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 - @change_desc << "change gid #{@current_resource.gid} to #{@new_resource.gid}" + if new_resource.gid.to_s != current_resource.gid.to_s + @change_desc << "change gid #{current_resource.gid} to #{new_resource.gid}" end - if @new_resource.append + if new_resource.append missing_members = [] - @new_resource.members.each do |member| + new_resource.members.each do |member| next if has_current_group_member?(member) validate_member!(member) missing_members << member end - if missing_members.length > 0 - @change_desc << "add missing member(s): #{missing_members.join(", ")}" + unless missing_members.empty? + @change_desc << "add missing member(s): #{missing_members.join(', ')}" end members_to_be_removed = [] - @new_resource.excluded_members.each do |member| + new_resource.excluded_members.each do |member| if has_current_group_member?(member) members_to_be_removed << member end end - if members_to_be_removed.length > 0 - @change_desc << "remove existing member(s): #{members_to_be_removed.join(", ")}" - end - else - if @new_resource.members != @current_resource.members - @change_desc << "replace group members with new list of members" + 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" end !@change_desc.empty? end def has_current_group_member?(member) - @current_resource.members.include?(member) + current_resource.members.include?(member) end def validate_member!(member) @@ -132,44 +130,41 @@ class Chef def action_create case @group_exists when false - converge_by("create group #{@new_resource.group_name}") do + converge_by("create group #{new_resource.group_name}") do create_group - Chef::Log.info("#{@new_resource} created") + Chef::Log.info("#{new_resource} created") end else if compare_group - converge_by(["alter group #{@new_resource.group_name}"] + change_desc) do + converge_by(["alter group #{new_resource.group_name}"] + change_desc) do manage_group - Chef::Log.info("#{@new_resource} altered") + Chef::Log.info("#{new_resource} altered") end end end end def action_remove - if @group_exists - converge_by("remove group #{@new_resource.group_name}") do - remove_group - Chef::Log.info("#{@new_resource} removed") - end + return unless @group_exists + converge_by("remove group #{new_resource.group_name}") do + remove_group + Chef::Log.info("#{new_resource} removed") end end def action_manage - if @group_exists && compare_group - converge_by(["manage group #{@new_resource.group_name}"] + change_desc) do - manage_group - Chef::Log.info("#{@new_resource} managed") - end + return unless @group_exists && compare_group + converge_by(["manage group #{new_resource.group_name}"] + change_desc) do + manage_group + Chef::Log.info("#{new_resource} managed") end end def action_modify - if compare_group - converge_by(["modify group #{@new_resource.group_name}"] + change_desc) do - manage_group - Chef::Log.info("#{@new_resource} modified") - end + return unless compare_group + converge_by(["modify group #{new_resource.group_name}"] + change_desc) do + manage_group + Chef::Log.info("#{new_resource} modified") end end diff --git a/lib/chef/provider/group/aix.rb b/lib/chef/provider/group/aix.rb index 4a02d5ef8c..30aea24464 100644 --- a/lib/chef/provider/group/aix.rb +++ b/lib/chef/provider/group/aix.rb @@ -17,7 +17,6 @@ # require "chef/provider/group/groupadd" -require "chef/mixin/shell_out" class Chef class Provider @@ -33,48 +32,39 @@ class Chef end def create_group - command = "mkgroup" - command << set_options << " #{@new_resource.group_name}" - run_command(:command => command) + shell_out_compact!("mkgroup", set_options, new_resource.group_name) modify_group_members end def manage_group - command = "chgroup" - options = set_options - #Usage: chgroup [-R load_module] "attr=value" ... group - if options.size > 0 - command << options << " #{@new_resource.group_name}" - run_command(:command => command) - end + shell_out_compact!("chgroup", set_options, options, new_resource.group_name) modify_group_members end def remove_group - run_command(:command => "rmgroup #{@new_resource.group_name}") + shell_out_compact!("rmgroup", new_resource.group_name) end def add_member(member) - shell_out!("chgrpmem -m + #{member} #{@new_resource.group_name}") + shell_out_compact!("chgrpmem", "-m", "+", member, new_resource.group_name) end def set_members(members) return if members.empty? - shell_out!("chgrpmem -m = #{members.join(',')} #{@new_resource.group_name}") + shell_out_compact!("chgrpmem", "-m", "=", members.join(","), new_resource.group_name) end def remove_member(member) - shell_out!("chgrpmem -m - #{member} #{@new_resource.group_name}") + shell_out_compact!("chgrpmem", "-m", "-", member, new_resource.group_name) end def set_options - opts = "" - { :gid => "id" }.sort { |a, b| a[0] <=> b[0] }.each do |field, option| - if @current_resource.send(field) != @new_resource.send(field) - if @new_resource.send(field) - Chef::Log.debug("#{@new_resource} setting #{field} to #{@new_resource.send(field)}") - opts << " '#{option}=#{@new_resource.send(field)}'" - end + opts = [] + { gid: "id" }.sort { |a, b| a[0] <=> b[0] }.each do |field, option| + next unless current_resource.send(field) != new_resource.send(field) + if new_resource.send(field) + Chef::Log.debug("#{new_resource} setting #{field} to #{new_resource.send(field)}") + opts << "#{option}=#{new_resource.send(field)}" end end opts diff --git a/lib/chef/provider/group/dscl.rb b/lib/chef/provider/group/dscl.rb index 00b4ce2b93..46932adc94 100644 --- a/lib/chef/provider/group/dscl.rb +++ b/lib/chef/provider/group/dscl.rb @@ -24,12 +24,15 @@ class Chef provides :group, os: "darwin" def dscl(*args) - host = "." - stdout_result = ""; stderr_result = ""; cmd = "dscl #{host} -#{args.join(' ')}" - status = shell_out(cmd) + argdup = args.dup + cmd = argdup.shift + shellcmd = [ "dscl", ".", "-#{cmd}", argdup ] + status = shell_out_compact(shellcmd) + stdout_result = "" + stderr_result = "" status.stdout.each_line { |line| stdout_result << line } status.stderr.each_line { |line| stderr_result << line } - return [cmd, status, stdout_result, stderr_result] + [shellcmd.flatten.compact.join(" "), status, stdout_result, stderr_result] end def safe_dscl(*args) @@ -37,18 +40,18 @@ class Chef return "" if ( args.first =~ /^delete/ ) && ( result[1].exitstatus != 0 ) raise(Chef::Exceptions::Group, "dscl error: #{result.inspect}") unless result[1].exitstatus == 0 raise(Chef::Exceptions::Group, "dscl error: #{result.inspect}") if result[2] =~ /No such key: / - return result[2] + result[2] end def load_current_resource - @current_resource = Chef::Resource::Group.new(@new_resource.name) - @current_resource.group_name(@new_resource.group_name) + @current_resource = Chef::Resource::Group.new(new_resource.name) + current_resource.group_name(new_resource.group_name) group_info = nil begin - group_info = safe_dscl("read /Groups/#{@new_resource.group_name}") + group_info = safe_dscl("read /Groups/#{new_resource.group_name}") rescue Chef::Exceptions::Group @group_exists = false - Chef::Log.debug("#{@new_resource} group does not exist") + Chef::Log.debug("#{new_resource} group does not exist") end if group_info @@ -57,15 +60,15 @@ class Chef val.strip! if val case key.downcase when "primarygroupid" - @new_resource.gid(val) unless @new_resource.gid - @current_resource.gid(val) + new_resource.gid(val) unless new_resource.gid + current_resource.gid(val) when "groupmembership" - @current_resource.members(val.split(" ")) + current_resource.members(val.split(" ")) end end end - @current_resource + current_resource end # get a free GID greater than 200 @@ -80,51 +83,51 @@ class Chef break end end - return gid || raise("gid not found. Exhausted. Searched #{search_limit} times") + gid || raise("gid not found. Exhausted. Searched #{search_limit} times") end def gid_used?(gid) return false unless gid groups_gids = safe_dscl("list /Groups gid") - !! ( groups_gids =~ Regexp.new("#{Regexp.escape(gid.to_s)}\n") ) + !!( groups_gids =~ Regexp.new("#{Regexp.escape(gid.to_s)}\n") ) end def set_gid - @new_resource.gid(get_free_gid) if [nil, ""].include? @new_resource.gid - raise(Chef::Exceptions::Group, "gid is already in use") if gid_used?(@new_resource.gid) - safe_dscl("create /Groups/#{@new_resource.group_name} PrimaryGroupID #{@new_resource.gid}") + new_resource.gid(get_free_gid) if [nil, ""].include? new_resource.gid + raise(Chef::Exceptions::Group, "gid is already in use") if gid_used?(new_resource.gid) + safe_dscl("create /Groups/#{new_resource.group_name} PrimaryGroupID #{new_resource.gid}") end def set_members # First reset the memberships if the append is not set - unless @new_resource.append - Chef::Log.debug("#{@new_resource} removing group members #{@current_resource.members.join(' ')}") unless @current_resource.members.empty? - safe_dscl("create /Groups/#{@new_resource.group_name} GroupMembers ''") # clear guid list - safe_dscl("create /Groups/#{@new_resource.group_name} GroupMembership ''") # clear user list - @current_resource.members([ ]) + unless new_resource.append + Chef::Log.debug("#{new_resource} removing group members #{current_resource.members.join(' ')}") unless current_resource.members.empty? + safe_dscl("create /Groups/#{new_resource.group_name} GroupMembers ''") # clear guid list + safe_dscl("create /Groups/#{new_resource.group_name} GroupMembership ''") # clear user list + current_resource.members([ ]) end # Add any members that need to be added - if @new_resource.members && !@new_resource.members.empty? + 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) + new_resource.members.each do |member| + members_to_be_added << member unless current_resource.members.include?(member) end unless members_to_be_added.empty? - Chef::Log.debug("#{@new_resource} setting group members #{members_to_be_added.join(', ')}") - safe_dscl("append /Groups/#{@new_resource.group_name} GroupMembership #{members_to_be_added.join(' ')}") + Chef::Log.debug("#{new_resource} setting group members #{members_to_be_added.join(', ')}") + safe_dscl("append /Groups/#{new_resource.group_name} GroupMembership #{members_to_be_added.join(' ')}") end end # Remove any members that need to be removed - if @new_resource.excluded_members && !@new_resource.excluded_members.empty? + 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) + 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? - Chef::Log.debug("#{@new_resource} removing group members #{members_to_be_removed.join(', ')}") - safe_dscl("delete /Groups/#{@new_resource.group_name} GroupMembership #{members_to_be_removed.join(' ')}") + Chef::Log.debug("#{new_resource} removing group members #{members_to_be_removed.join(', ')}") + safe_dscl("delete /Groups/#{new_resource.group_name} GroupMembership #{members_to_be_removed.join(' ')}") end end end @@ -132,8 +135,8 @@ class Chef def define_resource_requirements super requirements.assert(:all_actions) do |a| - a.assertion { ::File.exists?("/usr/bin/dscl") } - a.failure_message Chef::Exceptions::Group, "Could not find binary /usr/bin/dscl for #{@new_resource.name}" + a.assertion { ::File.exist?("/usr/bin/dscl") } + a.failure_message Chef::Exceptions::Group, "Could not find binary /usr/bin/dscl for #{new_resource.name}" # No whyrun alternative: this component should be available in the base install of any given system that uses it end end @@ -145,24 +148,24 @@ class Chef end def manage_group - if @new_resource.group_name && (@current_resource.group_name != @new_resource.group_name) + if new_resource.group_name && (current_resource.group_name != new_resource.group_name) dscl_create_group end - if @new_resource.gid && (@current_resource.gid != @new_resource.gid) + if new_resource.gid && (current_resource.gid != new_resource.gid) set_gid end - if @new_resource.members || @new_resource.excluded_members + if new_resource.members || new_resource.excluded_members set_members end end def dscl_create_group - safe_dscl("create /Groups/#{@new_resource.group_name}") - safe_dscl("create /Groups/#{@new_resource.group_name} Password '*'") + safe_dscl("create /Groups/#{new_resource.group_name}") + safe_dscl("create /Groups/#{new_resource.group_name} Password '*'") end def remove_group - safe_dscl("delete /Groups/#{@new_resource.group_name}") + safe_dscl("delete /Groups/#{new_resource.group_name}") end end end diff --git a/lib/chef/provider/group/gpasswd.rb b/lib/chef/provider/group/gpasswd.rb index dcf526b211..d8aff10d5b 100644 --- a/lib/chef/provider/group/gpasswd.rb +++ b/lib/chef/provider/group/gpasswd.rb @@ -31,26 +31,26 @@ class Chef def define_resource_requirements super requirements.assert(:all_actions) do |a| - a.assertion { ::File.exists?("/usr/bin/gpasswd") } - a.failure_message Chef::Exceptions::Group, "Could not find binary /usr/bin/gpasswd for #{@new_resource}" + a.assertion { ::File.exist?("/usr/bin/gpasswd") } + a.failure_message Chef::Exceptions::Group, "Could not find binary /usr/bin/gpasswd for #{new_resource}" # No whyrun alternative: this component should be available in the base install of any given system that uses it end end def set_members(members) - unless members.empty? - shell_out!("gpasswd -M #{members.join(',')} #{@new_resource.group_name}") + if members.empty? + shell_out_compact!("gpasswd", "-M", "", new_resource.group_name) else - shell_out!("gpasswd -M \"\" #{@new_resource.group_name}") + shell_out_compact!("gpasswd", "-M", members.join(","), new_resource.group_name) end end def add_member(member) - shell_out!("gpasswd -a #{member} #{@new_resource.group_name}") + shell_out_compact!("gpasswd", "-a", member, new_resource.group_name) end def remove_member(member) - shell_out!("gpasswd -d #{member} #{@new_resource.group_name}") + shell_out_compact!("gpasswd", "-d", member, new_resource.group_name) end end end diff --git a/lib/chef/provider/group/groupadd.rb b/lib/chef/provider/group/groupadd.rb index e94ed93cbd..f73c3b3be9 100644 --- a/lib/chef/provider/group/groupadd.rb +++ b/lib/chef/provider/group/groupadd.rb @@ -35,7 +35,7 @@ class Chef super required_binaries.each do |required_binary| requirements.assert(:all_actions) do |a| - a.assertion { ::File.exists?(required_binary) } + a.assertion { ::File.exist?(required_binary) } a.failure_message Chef::Exceptions::Group, "Could not find binary #{required_binary} for #{new_resource}" # No whyrun alternative: this component should be available in the base install of any given system that uses it end @@ -64,7 +64,7 @@ class Chef 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) + members_to_be_added << member unless 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}") @@ -108,14 +108,12 @@ class Chef # <string>:: A string containing the option and then the quoted value def set_options opts = [] - { :gid => "-g" }.sort { |a, b| a[0] <=> b[0] }.each do |field, option| - if current_resource.send(field) != new_resource.send(field) - if new_resource.send(field) - opts << option - opts << new_resource.send(field) - Chef::Log.debug("#{new_resource} set #{field} to #{new_resource.send(field)}") - end - end + { gid: "-g" }.sort { |a, b| a[0] <=> b[0] }.each do |field, option| + next unless current_resource.send(field) != new_resource.send(field) + next unless new_resource.send(field) + opts << option + opts << new_resource.send(field) + Chef::Log.debug("#{new_resource} set #{field} to #{new_resource.send(field)}") end opts << new_resource.group_name opts diff --git a/lib/chef/provider/group/groupmod.rb b/lib/chef/provider/group/groupmod.rb index 295d3b3425..01581e4863 100644 --- a/lib/chef/provider/group/groupmod.rb +++ b/lib/chef/provider/group/groupmod.rb @@ -26,28 +26,26 @@ class Chef def load_current_resource super %w{group user}.each do |binary| - raise Chef::Exceptions::Group, "Could not find binary /usr/sbin/#{binary} for #{@new_resource}" unless ::File.exists?("/usr/sbin/#{binary}") + raise Chef::Exceptions::Group, "Could not find binary /usr/sbin/#{binary} for #{new_resource}" unless ::File.exist?("/usr/sbin/#{binary}") end end # Create the group def create_group - command = "group add" - command << set_options - shell_out!(command) + shell_out_compact!("group", "add", set_options) - add_group_members(@new_resource.members) + add_group_members(new_resource.members) end # Manage the group when it already exists def manage_group - if @new_resource.append + if new_resource.append members_to_be_added = [ ] - if @new_resource.excluded_members && !@new_resource.excluded_members.empty? + 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) + 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? @@ -56,39 +54,39 @@ class Chef # Capture the members we need to add in # members_to_be_added to be added later on. - @current_resource.members.each do |member| + 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) + if new_resource.members && !new_resource.members.empty? + new_resource.members.each do |member| + members_to_be_added << member unless 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? + 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 # 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? - add_group_members(@new_resource.members) + Chef::Log.debug("#{new_resource} setting group members to: none") if new_resource.members.empty? + add_group_members(new_resource.members) end end # Remove the group def remove_group - shell_out!("group del #{@new_resource.group_name}") + shell_out_compact!("group", "del", new_resource.group_name) end # Adds a list of usernames to the group using `user mod` def add_group_members(members) - Chef::Log.debug("#{@new_resource} adding members #{members.join(', ')}") if !members.empty? + Chef::Log.debug("#{new_resource} adding members #{members.join(', ')}") unless members.empty? members.each do |user| - shell_out!("user mod -G #{@new_resource.group_name} #{user}") + shell_out_compact!("user", "mod", "-G", new_resource.group_name, user) end end @@ -96,15 +94,11 @@ class Chef # "<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) + shell_out_compact!("group", "mod", "-n", "#{new_resource.group_name}_bak", new_resource.group_name) - create = "group add" - create << set_options(:overwrite_gid => true) - shell_out!(create) + shell_out_compact!("group", "add", set_options(overwrite_gid: true)) - remove = "group del #{@new_resource.group_name}_bak" - shell_out!(remove) + shell_out_compact!("group", "del", "#{new_resource.group_name}_bak") end # Little bit of magic as per Adam's useradd provider to pull and assign the command line flags @@ -112,14 +106,15 @@ class Chef # ==== Returns # <string>:: A string containing the option and then the quoted value def set_options(overwrite_gid = false) - opts = "" - if overwrite_gid || @new_resource.gid && (@current_resource.gid != @new_resource.gid) - opts << " -g '#{@new_resource.gid}'" + opts = [] + if overwrite_gid || new_resource.gid && (current_resource.gid != new_resource.gid) + opts << "-g" + opts << new_resource.gid end if overwrite_gid - opts << " -o" + opts << "-o" end - opts << " #{@new_resource.group_name}" + opts << new_resource.group_name opts end end diff --git a/lib/chef/provider/group/pw.rb b/lib/chef/provider/group/pw.rb index 4fd78b6b31..115ccf3714 100644 --- a/lib/chef/provider/group/pw.rb +++ b/lib/chef/provider/group/pw.rb @@ -30,46 +30,42 @@ class Chef super requirements.assert(:all_actions) do |a| - a.assertion { ::File.exists?("/usr/sbin/pw") } - a.failure_message Chef::Exceptions::Group, "Could not find binary /usr/sbin/pw for #{@new_resource}" + a.assertion { ::File.exist?("/usr/sbin/pw") } + a.failure_message Chef::Exceptions::Group, "Could not find binary /usr/sbin/pw for #{new_resource}" # No whyrun alternative: this component should be available in the base install of any given system that uses it end end # Create the group def create_group - command = "pw groupadd" - command << set_options - - unless @new_resource.members.empty? + command = [ "pw", "groupadd", set_options ] + unless new_resource.members.empty? # pw group[add|mod] -M is used to set the full membership list on a # new or existing group. Because pw groupadd does not support the -m # and -d options used by manage_group, we treat group creation as a # special case and use -M. - Chef::Log.debug("#{@new_resource} setting group members: #{@new_resource.members.join(',')}") - command += " -M #{@new_resource.members.join(',')}" + Chef::Log.debug("#{new_resource} setting group members: #{new_resource.members.join(',')}") + command += [ "-M", new_resource.members.join(",") ] end - run_command(:command => command) + shell_out_compact!(command) end # Manage the group when it already exists def manage_group - command = "pw groupmod" - command << set_options member_options = set_members_options if member_options.empty? - run_command(:command => command) + shell_out_compact!("pw", "groupmod", set_options) else member_options.each do |option| - run_command(:command => command + option) + shell_out_compact!("pw", "groupmod", set_options, option) end end end # Remove the group def remove_group - run_command(:command => "pw groupdel #{@new_resource.group_name}") + shell_out_compact!("pw", "groupdel", new_resource.group_name) end # Little bit of magic as per Adam's useradd provider to pull and assign the command line flags @@ -77,10 +73,11 @@ class Chef # ==== Returns # <string>:: A string containing the option and then the quoted value def set_options - opts = " #{@new_resource.group_name}" - if @new_resource.gid && (@current_resource.gid != @new_resource.gid) - Chef::Log.debug("#{@new_resource}: current gid (#{@current_resource.gid}) doesnt match target gid (#{@new_resource.gid}), changing it") - opts << " -g '#{@new_resource.gid}'" + opts = [ new_resource.group_name ] + if new_resource.gid && (current_resource.gid != new_resource.gid) + Chef::Log.debug("#{new_resource}: current gid (#{current_resource.gid}) doesnt match target gid (#{new_resource.gid}), changing it") + opts << "-g" + opts << new_resource.gid end opts end @@ -91,26 +88,26 @@ class Chef members_to_be_added = [ ] members_to_be_removed = [ ] - if @new_resource.append + 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) + if new_resource.members && !new_resource.members.empty? + new_resource.members.each do |member| + members_to_be_added << member unless 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) + 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 # Append is not set so we're resetting the membership of # the group to the given members. - members_to_be_added = @new_resource.members.dup - @current_resource.members.each do |member| + members_to_be_added = new_resource.members.dup + 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 @@ -122,13 +119,13 @@ class Chef end 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(',')}" + 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(',')}" + Chef::Log.debug("#{new_resource} removing group members: #{members_to_be_removed.join(',')}") + opts << [ "-d", members_to_be_removed.join(",") ] end opts diff --git a/lib/chef/provider/group/suse.rb b/lib/chef/provider/group/suse.rb index 71336f9311..0790d2c2d9 100644 --- a/lib/chef/provider/group/suse.rb +++ b/lib/chef/provider/group/suse.rb @@ -33,21 +33,21 @@ class Chef def define_resource_requirements super requirements.assert(:all_actions) do |a| - a.assertion { ::File.exists?("/usr/sbin/groupmod") } - a.failure_message Chef::Exceptions::Group, "Could not find binary /usr/sbin/groupmod for #{@new_resource.name}" + a.assertion { ::File.exist?("/usr/sbin/groupmod") } + a.failure_message Chef::Exceptions::Group, "Could not find binary /usr/sbin/groupmod for #{new_resource.name}" # No whyrun alternative: this component should be available in the base install of any given system that uses it end requirements.assert(:create, :manage, :modify) do |a| a.assertion do begin - to_add(@new_resource.members).all? { |member| Etc.getpwnam(member) } + to_add(new_resource.members).all? { |member| Etc.getpwnam(member) } rescue false end end - a.failure_message Chef::Exceptions::Group, "Could not add users #{to_add(@new_resource.members).join(", ")} to #{@new_resource.group_name}: one of these users does not exist" - a.whyrun "Could not find one of these users: #{to_add(@new_resource.members).join(", ")}. Assuming it will be created by a prior step" + a.failure_message Chef::Exceptions::Group, "Could not add users #{to_add(new_resource.members).join(', ')} to #{new_resource.group_name}: one of these users does not exist" + a.whyrun "Could not find one of these users: #{to_add(new_resource.members).join(', ')}. Assuming it will be created by a prior step" end end @@ -62,19 +62,19 @@ class Chef end def to_add(members) - members - @current_resource.members + members - current_resource.members end def add_member(member) - shell_out!("groupmod -A #{member} #{@new_resource.group_name}") + shell_out_compact!("groupmod", "-A", member, new_resource.group_name) end def to_remove(members) - @current_resource.members - members + current_resource.members - members end def remove_member(member) - shell_out!("groupmod -R #{member} #{@new_resource.group_name}") + shell_out_compact!("groupmod", "-R", member, new_resource.group_name) end end diff --git a/lib/chef/provider/group/usermod.rb b/lib/chef/provider/group/usermod.rb index bef4b667a2..3874f7b4de 100644 --- a/lib/chef/provider/group/usermod.rb +++ b/lib/chef/provider/group/usermod.rb @@ -34,19 +34,19 @@ class Chef super requirements.assert(:all_actions) do |a| - a.assertion { ::File.exists?("/usr/sbin/usermod") } - a.failure_message Chef::Exceptions::Group, "Could not find binary /usr/sbin/usermod for #{@new_resource}" + a.assertion { ::File.exist?("/usr/sbin/usermod") } + a.failure_message Chef::Exceptions::Group, "Could not find binary /usr/sbin/usermod for #{new_resource}" # No whyrun alternative: this component should be available in the base install of any given system that uses it end requirements.assert(:modify, :manage) do |a| - a.assertion { @new_resource.members.empty? || @new_resource.append } + a.assertion { new_resource.members.empty? || new_resource.append } a.failure_message Chef::Exceptions::Group, "setting group members directly is not supported by #{self}, must set append true in group" # No whyrun alternative - this action is simply not supported. end requirements.assert(:all_actions) do |a| - a.assertion { @new_resource.excluded_members.empty? } + a.assertion { new_resource.excluded_members.empty? } a.failure_message Chef::Exceptions::Group, "excluded_members is not supported by #{self}" # No whyrun alternative - this action is simply not supported. end @@ -57,17 +57,16 @@ class Chef # 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.include?(:create) - members.each do |member| - add_member(member) - end - else + unless new_resource.action.include?(:create) raise Chef::Exceptions::UnsupportedAction, "Setting members directly is not supported by #{self}" end + members.each do |member| + add_member(member) + end end def add_member(member) - shell_out!("usermod #{append_flags} #{@new_resource.group_name} #{member}") + shell_out_compact!("usermod", append_flags, new_resource.group_name, member) end def remove_member(member) @@ -81,7 +80,7 @@ class Chef when "openbsd", "netbsd", "aix", "solaris2", "smartos", "omnios" "-G" when "solaris", "suse", "opensuse" - "-a -G" + [ "-a", "-G" ] end end diff --git a/lib/chef/provider/group/windows.rb b/lib/chef/provider/group/windows.rb index 5873e42a6b..c0026bf368 100644 --- a/lib/chef/provider/group/windows.rb +++ b/lib/chef/provider/group/windows.rb @@ -30,26 +30,26 @@ class Chef def initialize(new_resource, run_context) super - @net_group = Chef::Util::Windows::NetGroup.new(@new_resource.group_name) + @net_group = Chef::Util::Windows::NetGroup.new(new_resource.group_name) end def load_current_resource - @current_resource = Chef::Resource::Group.new(@new_resource.name) - @current_resource.group_name(@new_resource.group_name) + @current_resource = Chef::Resource::Group.new(new_resource.name) + current_resource.group_name(new_resource.group_name) members = nil begin members = @net_group.local_get_members - rescue => e + rescue @group_exists = false - Chef::Log.debug("#{@new_resource} group does not exist") + Chef::Log.debug("#{new_resource} group does not exist") end if members - @current_resource.members(members) + current_resource.members(members) end - @current_resource + current_resource end def create_group @@ -58,10 +58,10 @@ class Chef end def manage_group - if @new_resource.append + if new_resource.append members_to_be_added = [ ] - @new_resource.members.each do |member| - members_to_be_added << member if ! has_current_group_member?(member) && validate_member!(member) + new_resource.members.each do |member| + members_to_be_added << member if !has_current_group_member?(member) && validate_member!(member) end # local_add_members will raise ERROR_MEMBER_IN_ALIAS if a @@ -69,19 +69,19 @@ class Chef @net_group.local_add_members(members_to_be_added) unless members_to_be_added.empty? members_to_be_removed = [ ] - @new_resource.excluded_members.each do |member| - member_sid = lookup_account_name(member) + new_resource.excluded_members.each do |member| + lookup_account_name(member) 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 - @net_group.local_set_members(@new_resource.members) + @net_group.local_set_members(new_resource.members) end end def has_current_group_member?(member) member_sid = lookup_account_name(member) - @current_resource.members.include?(member_sid) + current_resource.members.include?(member_sid) end def remove_group @@ -97,12 +97,10 @@ class Chef end def lookup_account_name(account_name) - begin - Chef::ReservedNames::Win32::Security.lookup_account_name(locally_qualified_name(account_name))[1].to_s - rescue Chef::Exceptions::Win32APIError - Chef::Log.warn("SID for '#{locally_qualified_name(account_name)}' could not be found") - "" - end + Chef::ReservedNames::Win32::Security.lookup_account_name(locally_qualified_name(account_name))[1].to_s + rescue Chef::Exceptions::Win32APIError + Chef::Log.warn("SID for '#{locally_qualified_name(account_name)}' could not be found") + "" end end diff --git a/spec/unit/provider/group/dscl_spec.rb b/spec/unit/provider/group/dscl_spec.rb index bc69d7580e..9dc969c352 100644 --- a/spec/unit/provider/group/dscl_spec.rb +++ b/spec/unit/provider/group/dscl_spec.rb @@ -28,12 +28,12 @@ describe Chef::Provider::Group::Dscl do @provider = Chef::Provider::Group::Dscl.new(@new_resource, @run_context) @provider.current_resource = @current_resource - @status = double(:stdout => "\n", :stderr => "", :exitstatus => 0) + @status = double(stdout: "\n", stderr: "", exitstatus: 0) allow(@provider).to receive(:shell_out).and_return(@status) end it "should run shell_out with the supplied array of arguments appended to the dscl command" do - expect(@provider).to receive(:shell_out).with("dscl . -cmd /Path arg1 arg2") + expect(@provider).to receive(:shell_out).with("dscl", ".", "-cmd", "/Path", "arg1", "arg2") @provider.dscl("cmd", "/Path", "arg1", "arg2") end @@ -57,7 +57,7 @@ describe Chef::Provider::Group::Dscl do describe "with the dscl command returning a non zero exit status for a delete" do before do - @status = double("Process::Status", :exitstatus => 1) + @status = double("Process::Status", exitstatus: 1) allow(@provider).to receive(:dscl).and_return(["cmd", @status, "stdout", "stderr"]) end @@ -232,20 +232,20 @@ describe Chef::Provider::Group::Dscl do end describe "when loading the current system state" do - before (:each) do + before(:each) do @provider.action = :create @provider.load_current_resource @provider.define_resource_requirements end it "raises an error if the required binary /usr/bin/dscl doesn't exist" do - expect(File).to receive(:exists?).with("/usr/bin/dscl").and_return(false) + expect(File).to receive(:exist?).with("/usr/bin/dscl").and_return(false) expect { @provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Group) end it "doesn't raise an error if /usr/bin/dscl exists" do - allow(File).to receive(:exists?).and_return(true) + allow(File).to receive(:exist?).and_return(true) expect { @provider.process_resource_requirements }.not_to raise_error end end @@ -321,11 +321,11 @@ EOF end it "should parse gid properly" do - allow(File).to receive(:exists?).and_return(true) + allow(File).to receive(:exist?).and_return(true) expect(@current_resource.gid).to eq("999") end it "should parse members properly" do - allow(File).to receive(:exists?).and_return(true) + allow(File).to receive(:exist?).and_return(true) expect(@current_resource.members).to eq(%w{waka bar}) end end diff --git a/spec/unit/provider/group/gpasswd_spec.rb b/spec/unit/provider/group/gpasswd_spec.rb index 16e6ad15ea..add87bf008 100644 --- a/spec/unit/provider/group/gpasswd_spec.rb +++ b/spec/unit/provider/group/gpasswd_spec.rb @@ -27,11 +27,11 @@ describe Chef::Provider::Group::Gpasswd, "modify_group_members" do @new_resource.members %w{lobster rage fist} @new_resource.append false @provider = Chef::Provider::Group::Gpasswd.new(@new_resource, @run_context) - #@provider.stub(:run_command).and_return(true) + # @provider.stub(:run_command).and_return(true) end describe "when determining the current group state" do - before (:each) do + before(:each) do @provider.action = :create @provider.load_current_resource @provider.define_resource_requirements @@ -41,13 +41,13 @@ describe Chef::Provider::Group::Gpasswd, "modify_group_members" do # for Chef::Provider::Group - no need to repeat it here. We'll # include only what's specific to this provider. it "should raise an error if the required binary /usr/bin/gpasswd doesn't exist" do - allow(File).to receive(:exists?).and_return(true) - expect(File).to receive(:exists?).with("/usr/bin/gpasswd").and_return(false) + allow(File).to receive(:exist?).and_return(true) + expect(File).to receive(:exist?).with("/usr/bin/gpasswd").and_return(false) expect { @provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Group) end it "shouldn't raise an error if the required binaries exist" do - allow(File).to receive(:exists?).and_return(true) + allow(File).to receive(:exist?).and_return(true) expect { @provider.process_resource_requirements }.not_to raise_error end end @@ -66,7 +66,7 @@ describe Chef::Provider::Group::Gpasswd, "modify_group_members" do it "logs a message and sets group's members to 'none'" do expect(Chef::Log).to receive(:debug).with("group[wheel] setting group members to: none") - expect(@provider).to receive(:shell_out!).with("gpasswd -M \"\" wheel") + expect(@provider).to receive(:shell_out!).with("gpasswd", "-M", "", "wheel") @provider.modify_group_members end end @@ -91,7 +91,7 @@ describe Chef::Provider::Group::Gpasswd, "modify_group_members" do end it "should run gpasswd with the members joined by ',' followed by the target group" do - expect(@provider).to receive(:shell_out!).with("gpasswd -M lobster,rage,fist wheel") + expect(@provider).to receive(:shell_out!).with("gpasswd", "-M", "lobster,rage,fist", "wheel") @provider.modify_group_members end @@ -104,9 +104,9 @@ describe Chef::Provider::Group::Gpasswd, "modify_group_members" do it "should run gpasswd individually for each user when the append option is set" do @new_resource.append(true) - expect(@provider).to receive(:shell_out!).with("gpasswd -a lobster wheel") - expect(@provider).to receive(:shell_out!).with("gpasswd -a rage wheel") - expect(@provider).to receive(:shell_out!).with("gpasswd -a fist wheel") + expect(@provider).to receive(:shell_out!).with("gpasswd", "-a", "lobster", "wheel") + expect(@provider).to receive(:shell_out!).with("gpasswd", "-a", "rage", "wheel") + expect(@provider).to receive(:shell_out!).with("gpasswd", "-a", "fist", "wheel") @provider.modify_group_members end end diff --git a/spec/unit/provider/group/groupadd_spec.rb b/spec/unit/provider/group/groupadd_spec.rb index 70a98ff2f5..929dd00450 100644 --- a/spec/unit/provider/group/groupadd_spec.rb +++ b/spec/unit/provider/group/groupadd_spec.rb @@ -46,7 +46,7 @@ describe Chef::Provider::Group::Groupadd do describe "#set_options" do field_list = { - :gid => "-g", + gid: "-g", } field_list.each do |attribute, option| @@ -168,25 +168,25 @@ describe Chef::Provider::Group::Groupadd do end before do - allow(File).to receive(:exists?).and_return(false) + allow(File).to receive(:exist?).and_return(false) provider.define_resource_requirements end it "should raise an error if the required binary /usr/sbin/groupadd doesn't exist" do - expect(File).to receive(:exists?).with("/usr/sbin/groupadd").and_return(false) + expect(File).to receive(:exist?).with("/usr/sbin/groupadd").and_return(false) expect { provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Group) end it "should raise an error if the required binary /usr/sbin/groupmod doesn't exist" do - expect(File).to receive(:exists?).with("/usr/sbin/groupadd").and_return(true) - expect(File).to receive(:exists?).with("/usr/sbin/groupmod").and_return(false) + expect(File).to receive(:exist?).with("/usr/sbin/groupadd").and_return(true) + expect(File).to receive(:exist?).with("/usr/sbin/groupmod").and_return(false) expect { provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Group) end it "should raise an error if the required binary /usr/sbin/groupdel doesn't exist" do - expect(File).to receive(:exists?).with("/usr/sbin/groupadd").and_return(true) - expect(File).to receive(:exists?).with("/usr/sbin/groupmod").and_return(true) - expect(File).to receive(:exists?).with("/usr/sbin/groupdel").and_return(false) + expect(File).to receive(:exist?).with("/usr/sbin/groupadd").and_return(true) + expect(File).to receive(:exist?).with("/usr/sbin/groupmod").and_return(true) + expect(File).to receive(:exist?).with("/usr/sbin/groupdel").and_return(false) expect { provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Group) end diff --git a/spec/unit/provider/group/groupmod_spec.rb b/spec/unit/provider/group/groupmod_spec.rb index 21d026409e..6629b718d0 100644 --- a/spec/unit/provider/group/groupmod_spec.rb +++ b/spec/unit/provider/group/groupmod_spec.rb @@ -33,17 +33,17 @@ describe Chef::Provider::Group::Groupmod do describe "manage_group" do describe "when determining the current group state" do it "should raise an error if the required binary /usr/sbin/group doesn't exist" do - expect(File).to receive(:exists?).with("/usr/sbin/group").and_return(false) + expect(File).to receive(:exist?).with("/usr/sbin/group").and_return(false) expect { @provider.load_current_resource }.to raise_error(Chef::Exceptions::Group) end it "should raise an error if the required binary /usr/sbin/user doesn't exist" do - expect(File).to receive(:exists?).with("/usr/sbin/group").and_return(true) - expect(File).to receive(:exists?).with("/usr/sbin/user").and_return(false) + expect(File).to receive(:exist?).with("/usr/sbin/group").and_return(true) + expect(File).to receive(:exist?).with("/usr/sbin/user").and_return(false) expect { @provider.load_current_resource }.to raise_error(Chef::Exceptions::Group) end it "shouldn't raise an error if the required binaries exist" do - allow(File).to receive(:exists?).and_return(true) + allow(File).to receive(:exist?).and_return(true) expect { @provider.load_current_resource }.not_to raise_error end end @@ -62,9 +62,9 @@ describe Chef::Provider::Group::Groupmod do it "logs a message and sets group's members to 'none', then removes existing group members" do expect(Chef::Log).to receive(:debug).with("group[wheel] setting group members to: none") - expect(@provider).to receive(:shell_out!).with("group mod -n wheel_bak wheel") - expect(@provider).to receive(:shell_out!).with("group add -g '123' -o wheel") - expect(@provider).to receive(:shell_out!).with("group del wheel_bak") + expect(@provider).to receive(:shell_out!).with("group", "mod", "-n", "wheel_bak", "wheel") + expect(@provider).to receive(:shell_out!).with("group", "add", "-g", "123", "-o", "wheel") + expect(@provider).to receive(:shell_out!).with("group", "del", "wheel_bak") @provider.manage_group end end @@ -90,10 +90,10 @@ describe Chef::Provider::Group::Groupmod do it "updates group membership correctly" do allow(Chef::Log).to receive(:debug) - expect(@provider).to receive(:shell_out!).with("group mod -n wheel_bak wheel") - expect(@provider).to receive(:shell_out!).with("user mod -G wheel lobster") - expect(@provider).to receive(:shell_out!).with("group add -g '123' -o wheel") - expect(@provider).to receive(:shell_out!).with("group del wheel_bak") + expect(@provider).to receive(:shell_out!).with("group", "mod", "-n", "wheel_bak", "wheel") + expect(@provider).to receive(:shell_out!).with("user", "mod", "-G", "wheel", "lobster") + expect(@provider).to receive(:shell_out!).with("group", "add", "-g", "123", "-o", "wheel") + expect(@provider).to receive(:shell_out!).with("group", "del", "wheel_bak") @provider.manage_group end end @@ -108,10 +108,10 @@ describe Chef::Provider::Group::Groupmod do end it "should run a group add command and some user mod commands" do - expect(@provider).to receive(:shell_out!).with("group add -g '123' wheel") - expect(@provider).to receive(:shell_out!).with("user mod -G wheel lobster") - expect(@provider).to receive(:shell_out!).with("user mod -G wheel rage") - expect(@provider).to receive(:shell_out!).with("user mod -G wheel fist") + expect(@provider).to receive(:shell_out!).with("group", "add", "-g", "123", "wheel") + expect(@provider).to receive(:shell_out!).with("user", "mod", "-G", "wheel", "lobster") + expect(@provider).to receive(:shell_out!).with("user", "mod", "-G", "wheel", "rage") + expect(@provider).to receive(:shell_out!).with("user", "mod", "-G", "wheel", "fist") @provider.create_group end end @@ -125,7 +125,7 @@ describe Chef::Provider::Group::Groupmod do end it "should run a group del command" do - expect(@provider).to receive(:shell_out!).with("group del wheel") + expect(@provider).to receive(:shell_out!).with("group", "del", "wheel") @provider.remove_group end end diff --git a/spec/unit/provider/group/pw_spec.rb b/spec/unit/provider/group/pw_spec.rb index 97ffcb3c31..f109ba6ba1 100644 --- a/spec/unit/provider/group/pw_spec.rb +++ b/spec/unit/provider/group/pw_spec.rb @@ -37,19 +37,19 @@ describe Chef::Provider::Group::Pw do describe "when setting options for the pw command" do it "does not set the gid option if gids match or are unmanaged" do - expect(@provider.set_options).to eq(" wheel") + expect(@provider.set_options).to eq(["wheel"]) end it "sets the option for gid if it is not nil" do @new_resource.gid(42) - expect(@provider.set_options).to eql(" wheel -g '42'") + expect(@provider.set_options).to eql(["wheel", "-g", 42]) end end 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) - expect(@provider).to receive(:run_command).with({ :command => "pw groupadd wheel -g '23' -M root,aj" }).and_return(true) + expect(@provider).to receive(:shell_out!).with("pw", "groupadd", "wheel", "-g", "23", "-M", "root,aj").and_return(true) @provider.create_group end end @@ -59,8 +59,8 @@ describe Chef::Provider::Group::Pw do it "should run pw groupmod with the return of set_options" do @new_resource.gid(42) @new_resource.members(["someone"]) - expect(@provider).to receive(:run_command).with({ :command => "pw groupmod wheel -g '42' -m someone" }).and_return(true) - expect(@provider).to receive(:run_command).with({ :command => "pw groupmod wheel -g '42' -d root,aj" }).and_return(true) + expect(@provider).to receive(:shell_out!).with("pw", "groupmod", "wheel", "-g", "42", "-m", "someone").and_return(true) + expect(@provider).to receive(:shell_out!).with("pw", "groupmod", "wheel", "-g", "42", "-d", "root,aj").and_return(true) @provider.manage_group end @@ -68,7 +68,7 @@ describe Chef::Provider::Group::Pw do describe "when removing the group" do it "should run pw groupdel with the new resources group name" do - expect(@provider).to receive(:run_command).with({ :command => "pw groupdel wheel" }).and_return(true) + expect(@provider).to receive(:shell_out!).with("pw", "groupdel", "wheel").and_return(true) @provider.remove_group end end @@ -98,7 +98,7 @@ describe Chef::Provider::Group::Pw do end it "should set the -d option with the members joined by ','" do - expect(@provider.set_members_options).to eql([ " -d all,your,base" ]) + expect(@provider.set_members_options).to eql([ ["-d", "all,your,base"] ]) end end @@ -114,24 +114,24 @@ describe Chef::Provider::Group::Pw do end it "should set the -m option with the members joined by ','" do - expect(@provider.set_members_options).to eql([ " -m all,your,base" ]) + expect(@provider.set_members_options).to eql([[ "-m", "all,your,base" ]]) end end end describe "load_current_resource" do - before (:each) do + before(:each) do @provider.action = :create @provider.load_current_resource @provider.define_resource_requirements end it "should raise an error if the required binary /usr/sbin/pw doesn't exist" do - expect(File).to receive(:exists?).with("/usr/sbin/pw").and_return(false) + expect(File).to receive(:exist?).with("/usr/sbin/pw").and_return(false) expect { @provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Group) end it "shouldn't raise an error if /usr/sbin/pw exists" do - allow(File).to receive(:exists?).and_return(true) + allow(File).to receive(:exist?).and_return(true) expect { @provider.process_resource_requirements }.not_to raise_error end end diff --git a/spec/unit/provider/group/suse_spec.rb b/spec/unit/provider/group/suse_spec.rb index da4e8e9155..e61d865b6d 100644 --- a/spec/unit/provider/group/suse_spec.rb +++ b/spec/unit/provider/group/suse_spec.rb @@ -7,7 +7,7 @@ # you may not use this file except in compliance with the License. # You may obtain a copy of the License at # -# http://www.apache.org/licenses/LICENSE-2.0 +# http://www.apache.org/licenses/LICENSE-2.0 # # Unless required by applicable law or agreed to in writing, software # distributed under the License is distributed on an "AS IS" BASIS, @@ -47,7 +47,7 @@ describe Chef::Provider::Group::Suse do describe "when determining the current group state" do before(:each) do - allow(File).to receive(:exists?).and_return(true) + allow(File).to receive(:exist?).and_return(true) provider.action = :create provider.define_resource_requirements end @@ -56,7 +56,7 @@ describe Chef::Provider::Group::Suse do # for Chef::Provider::Group - no need to repeat it here. We'll # include only what's specific to this provider. it "should raise an error if the required binary /usr/sbin/groupmod doesn't exist" do - expect(File).to receive(:exists?).with("/usr/sbin/groupmod").and_return(false) + expect(File).to receive(:exist?).with("/usr/sbin/groupmod").and_return(false) expect { provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Group) end @@ -76,14 +76,14 @@ describe Chef::Provider::Group::Suse do describe "#add_member" do it "should call out to groupmod to add user" do - expect(provider).to receive(:shell_out!).with("groupmod -A new_user new_group") + expect(provider).to receive(:shell_out!).with("groupmod", "-A", "new_user", "new_group") provider.add_member("new_user") end end describe "#remove_member" do it "should call out to groupmod to remove user" do - expect(provider).to receive(:shell_out!).with("groupmod -R new_user new_group") + expect(provider).to receive(:shell_out!).with("groupmod", "-R", "new_user", "new_group") provider.remove_member("new_user") end end diff --git a/spec/unit/provider/group/usermod_spec.rb b/spec/unit/provider/group/usermod_spec.rb index 7bd45c4f1b..c6422b12c9 100644 --- a/spec/unit/provider/group/usermod_spec.rb +++ b/spec/unit/provider/group/usermod_spec.rb @@ -46,18 +46,18 @@ describe Chef::Provider::Group::Usermod do describe "with supplied members" do platforms = { - "openbsd" => "-G", - "netbsd" => "-G", - "solaris" => "-a -G", - "suse" => "-a -G", - "opensuse" => "-a -G", - "smartos" => "-G", - "omnios" => "-G", + "openbsd" => [ "-G" ], + "netbsd" => [ "-G" ], + "solaris" => [ "-a", "-G" ], + "suse" => [ "-a", "-G" ], + "opensuse" => [ "-a", "-G" ], + "smartos" => [ "-G" ], + "omnios" => [ "-G" ], } before do allow(@new_resource).to receive(:members).and_return(%w{all your base}) - allow(File).to receive(:exists?).and_return(true) + allow(File).to receive(:exist?).and_return(true) end it "should raise an error when setting the entire group directly" do @@ -85,9 +85,9 @@ describe Chef::Provider::Group::Usermod do @provider.current_resource = current_resource @node.automatic_attrs[:platform] = platform allow(@new_resource).to receive(:append).and_return(true) - expect(@provider).to receive(:shell_out!).with("usermod #{flags} wheel all") - expect(@provider).to receive(:shell_out!).with("usermod #{flags} wheel your") - expect(@provider).to receive(:shell_out!).with("usermod #{flags} wheel base") + expect(@provider).to receive(:shell_out!).with("usermod", *flags, "wheel", "all") + expect(@provider).to receive(:shell_out!).with("usermod", *flags, "wheel", "your") + expect(@provider).to receive(:shell_out!).with("usermod", *flags, "wheel", "base") @provider.modify_group_members end end @@ -96,19 +96,19 @@ describe Chef::Provider::Group::Usermod do describe "when loading the current resource" do before(:each) do - allow(File).to receive(:exists?).and_return(false) + allow(File).to receive(:exist?).and_return(false) @provider.action = :create @provider.define_resource_requirements end it "should raise an error if the required binary /usr/sbin/usermod doesn't exist" do - allow(File).to receive(:exists?).and_return(true) - expect(File).to receive(:exists?).with("/usr/sbin/usermod").and_return(false) + allow(File).to receive(:exist?).and_return(true) + expect(File).to receive(:exist?).with("/usr/sbin/usermod").and_return(false) expect { @provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Group) end it "shouldn't raise an error if the required binaries exist" do - allow(File).to receive(:exists?).and_return(true) + allow(File).to receive(:exist?).and_return(true) expect { @provider.process_resource_requirements }.not_to raise_error end end |