diff options
author | Lamont Granquist <lamont@scriptkiddie.org> | 2016-12-21 09:25:50 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2016-12-21 09:25:50 -0800 |
commit | 9c38e31108d55c39e097c4dd6a6fea2f26bcfd48 (patch) | |
tree | aa263b89b50afb306911244d2fc5f1b5d6aa18cc | |
parent | a52e4e48e66254e89a7bbeed67cda8efc6fc3548 (diff) | |
parent | 1c9b81cab7461b1fc687e327a90eed1f3b446989 (diff) | |
download | chef-9c38e31108d55c39e097c4dd6a6fea2f26bcfd48.tar.gz |
Merge pull request #5666 from chef/lcg/deprecate-run-command
formally deprecate run_command
-rw-r--r-- | lib/chef/deprecated.rb | 10 | ||||
-rw-r--r-- | lib/chef/mixin/command.rb | 1 | ||||
-rw-r--r-- | lib/chef/mixin/shell_out.rb | 9 | ||||
-rw-r--r-- | lib/chef/provider/group/groupadd.rb | 57 | ||||
-rw-r--r-- | spec/unit/provider/group/groupadd_spec.rb | 45 |
5 files changed, 70 insertions, 52 deletions
diff --git a/lib/chef/deprecated.rb b/lib/chef/deprecated.rb index 3a988fdfa3..5b0bac552e 100644 --- a/lib/chef/deprecated.rb +++ b/lib/chef/deprecated.rb @@ -166,6 +166,16 @@ class Chef end end + class RunCommand < Base + def id + 14 + end + + def target + "run_command.html" + end + end + class ResourceCloning < Base def id 3694 diff --git a/lib/chef/mixin/command.rb b/lib/chef/mixin/command.rb index 0cc3143ec7..269f1d1194 100644 --- a/lib/chef/mixin/command.rb +++ b/lib/chef/mixin/command.rb @@ -107,6 +107,7 @@ class Chef end def output_of_command(command, args) + Chef.deprecated(:run_command, "Chef::Mixin::Command.run_command is deprecated, please use shell_out") Chef::Log.debug("Executing #{command}") stderr_string, stdout_string, status = "", "", nil diff --git a/lib/chef/mixin/shell_out.rb b/lib/chef/mixin/shell_out.rb index cc2bf085e9..d341e3d1ec 100644 --- a/lib/chef/mixin/shell_out.rb +++ b/lib/chef/mixin/shell_out.rb @@ -85,6 +85,7 @@ 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(" ") end @@ -108,6 +109,14 @@ class Chef 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)) + end + private def shell_out_command(*command_args) diff --git a/lib/chef/provider/group/groupadd.rb b/lib/chef/provider/group/groupadd.rb index 162875f9f1..e94ed93cbd 100644 --- a/lib/chef/provider/group/groupadd.rb +++ b/lib/chef/provider/group/groupadd.rb @@ -36,7 +36,7 @@ class Chef required_binaries.each do |required_binary| requirements.assert(:all_actions) do |a| a.assertion { ::File.exists?(required_binary) } - a.failure_message Chef::Exceptions::Group, "Could not find binary #{required_binary} for #{@new_resource}" + 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 end @@ -44,54 +44,49 @@ class Chef # Create the group def create_group - command = "groupadd" - command << set_options - command << groupadd_options - run_command(:command => command) + shell_out_compact!("groupadd", set_options, groupadd_options) modify_group_members end # Manage the group when it already exists def manage_group - command = "groupmod" - command << set_options - run_command(:command => command) + shell_out_compact!("groupmod", set_options) modify_group_members end # Remove the group def remove_group - run_command(:command => "groupdel #{@new_resource.group_name}") + shell_out_compact!("groupdel", new_resource.group_name) end def modify_group_members - if @new_resource.append - if @new_resource.members && !@new_resource.members.empty? + 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) + 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}") + 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? + 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 members_to_be_removed.each do |member| - Chef::Log.debug("#{@new_resource} removing member #{member} from group #{@new_resource.group_name}") + 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) + 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 @@ -112,23 +107,25 @@ class Chef # ==== Returns # <string>:: A string containing the option and then the quoted value def set_options - opts = "" + 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} '#{@new_resource.send(field)}'" - Chef::Log.debug("#{@new_resource} set #{field} to #{@new_resource.send(field)}") + 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 end - opts << " #{@new_resource.group_name}" + opts << new_resource.group_name + opts end def groupadd_options - opts = "" + opts = [] # Solaris doesn't support system groups. - opts << " -r" if @new_resource.system && !node.platform?("solaris2") - opts << " -o" if @new_resource.non_unique + opts << "-r" if new_resource.system && !node.platform?("solaris2") + opts << "-o" if new_resource.non_unique opts end diff --git a/spec/unit/provider/group/groupadd_spec.rb b/spec/unit/provider/group/groupadd_spec.rb index 054d953414..70a98ff2f5 100644 --- a/spec/unit/provider/group/groupadd_spec.rb +++ b/spec/unit/provider/group/groupadd_spec.rb @@ -58,36 +58,37 @@ describe Chef::Provider::Group::Groupadd do it "should set the option for #{attribute} if the new resources #{attribute} is not null" do allow(new_resource).to receive(attribute).and_return("wowaweea") - expect(provider.set_options).to eql(" #{option} '#{new_resource.send(attribute)}' #{new_resource.group_name}") + expect(provider.set_options).to eql([ option, new_resource.send(attribute), new_resource.group_name]) end end it "should combine all the possible options" do - match_string = "" + match_array = [] field_list.sort { |a, b| a[0] <=> b[0] }.each do |attribute, option| allow(new_resource).to receive(attribute).and_return("hola") - match_string << " #{option} 'hola'" + match_array << option + match_array << "hola" end - match_string << " aj" - expect(provider.set_options).to eql(match_string) + match_array << "aj" + expect(provider.set_options).to eql(match_array) end describe "when we want to create a system group" do it "should not set groupadd_options '-r' when system is false" do new_resource.system(false) - expect(provider.groupadd_options).not_to match(/-r/) + expect(provider.groupadd_options).to eq([]) end it "should set groupadd -r if system is true" do new_resource.system(true) - expect(provider.groupadd_options).to eq(" -r") + expect(provider.groupadd_options).to eq(["-r"]) end context "on Solaris" do before { node.automatic["platform"] = "solaris2" } it "should not set groupadd -r if system is true" do new_resource.system(true) - expect(provider.groupadd_options).not_to match(/-r/) + expect(provider.groupadd_options).to eql([]) end end end @@ -95,26 +96,26 @@ describe Chef::Provider::Group::Groupadd do describe "when we want to create a non_unique gid group" do it "should not set groupadd_options '-o' when non_unique is false" do new_resource.non_unique(false) - expect(provider.groupadd_options).not_to match(/-o/) + expect(provider.groupadd_options).to eq([]) end it "should set groupadd -o if non_unique is true" do new_resource.non_unique(true) - expect(provider.groupadd_options).to eq(" -o") + expect(provider.groupadd_options).to eq(["-o"]) end end end describe "#create_group" do before do - allow(provider).to receive(:run_command).and_return(true) - allow(provider).to receive(:set_options).and_return(" monkey") - allow(provider).to receive(:groupadd_options).and_return("") + allow(provider).to receive(:shell_out!).and_return(true) + allow(provider).to receive(:set_options).and_return("monkey") + allow(provider).to receive(:groupadd_options).and_return([]) allow(provider).to receive(:modify_group_members).and_return(true) end it "should run groupadd with the return of set_options" do - expect(provider).to receive(:run_command).with({ :command => "groupadd monkey" }).and_return(true) + expect(provider).to receive(:shell_out!).with("groupadd", "monkey").and_return(true) provider.create_group end @@ -126,13 +127,13 @@ describe Chef::Provider::Group::Groupadd do describe "#manage_group" do before do - allow(provider).to receive(:run_command).and_return(true) - allow(provider).to receive(:set_options).and_return(" monkey") + allow(provider).to receive(:shell_out!).and_return(true) + allow(provider).to receive(:set_options).and_return("monkey") end it "should run groupmod with the return of set_options" do allow(provider).to receive(:modify_group_members).and_return(true) - expect(provider).to receive(:run_command).with({ :command => "groupmod monkey" }).and_return(true) + expect(provider).to receive(:shell_out!).with("groupmod", "monkey").and_return(true) provider.manage_group end @@ -144,12 +145,12 @@ describe Chef::Provider::Group::Groupadd do describe "#remove_group" do before do - allow(provider).to receive(:run_command).and_return(true) - allow(provider).to receive(:set_options).and_return(" monkey") + allow(provider).to receive(:shell_out!).and_return(true) + allow(provider).to receive(:set_options).and_return("monkey") end it "should run groupdel with the new resources group name" do - expect(provider).to receive(:run_command).with({ :command => "groupdel aj" }).and_return(true) + expect(provider).to receive(:shell_out!).with("groupdel", "aj").and_return(true) provider.remove_group end end @@ -162,8 +163,8 @@ describe Chef::Provider::Group::Groupadd do describe "#load_current_resource" do before do - allow(provider).to receive(:run_command).and_return(true) - allow(provider).to receive(:set_options).and_return(" monkey") + allow(provider).to receive(:shell_out!).and_return(true) + allow(provider).to receive(:set_options).and_return("monkey") end before do |