diff options
| -rw-r--r-- | chef-config/lib/chef-config/config.rb | 2 | ||||
| -rw-r--r-- | lib/chef/mixin/path_sanity.rb | 36 | ||||
| -rw-r--r-- | lib/chef/mixin/shell_out.rb | 32 | ||||
| -rw-r--r-- | spec/unit/mixin/path_sanity_spec.rb | 14 | ||||
| -rw-r--r-- | spec/unit/mixin/shell_out_spec.rb | 100 |
5 files changed, 42 insertions, 142 deletions
diff --git a/chef-config/lib/chef-config/config.rb b/chef-config/lib/chef-config/config.rb index c63098930b..0666dd869d 100644 --- a/chef-config/lib/chef-config/config.rb +++ b/chef-config/lib/chef-config/config.rb @@ -250,7 +250,7 @@ module ChefConfig default(:policy_path) { derive_path_from_chef_repo_path("policies") } # Turn on "path sanity" by default. See also: http://wiki.opscode.com/display/chef/User+Environment+PATH+Sanity - default :enforce_path_sanity, true + default :enforce_path_sanity, false # Formatted Chef Client output is a beta feature, disabled by default: default :formatter, "null" diff --git a/lib/chef/mixin/path_sanity.rb b/lib/chef/mixin/path_sanity.rb index 6a8e017bcd..c441d0770a 100644 --- a/lib/chef/mixin/path_sanity.rb +++ b/lib/chef/mixin/path_sanity.rb @@ -1,6 +1,6 @@ # # Author:: Seth Chisamore (<schisamo@chef.io>) -# Copyright:: Copyright 2011-2016, Chef Software Inc. +# Copyright:: Copyright 2011-2017, Chef Software Inc. # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -22,25 +22,23 @@ class Chef def enforce_path_sanity(env = ENV) if Chef::Config[:enforce_path_sanity] - env["PATH"] = "" if env["PATH"].nil? - path_separator = Chef::Platform.windows? ? ";" : ":" - existing_paths = env["PATH"].split(path_separator) - # ensure the Ruby and Gem bindirs are included - # mainly for 'full-stack' Chef installs - paths_to_add = [] - paths_to_add << ruby_bindir unless sane_paths.include?(ruby_bindir) - paths_to_add << gem_bindir unless sane_paths.include?(gem_bindir) - paths_to_add << sane_paths if sane_paths - paths_to_add.flatten!.compact! - paths_to_add.each do |sane_path| - unless existing_paths.include?(sane_path) - env_path = env["PATH"].dup - env_path << path_separator unless env["PATH"].empty? - env_path << sane_path - env["PATH"] = env_path.encode("utf-8", invalid: :replace, undef: :replace) - end - end + env["PATH"] = sanitized_path(env) + end + end + + def sanitized_path(env = ENV) + env_path = env["PATH"].nil? ? "" : env["PATH"].dup + path_separator = Chef::Platform.windows? ? ";" : ":" + # ensure the Ruby and Gem bindirs are included + # mainly for 'full-stack' Chef installs + new_paths = env_path.split(path_separator) + [ ruby_bindir, gem_bindir ].compact.each do |path| + new_paths = [ path ] + new_paths unless new_paths.include?(path) + end + sane_paths.each do |path| + new_paths << path unless new_paths.include?(path) end + new_paths.join(path_separator).encode("utf-8", invalid: :replace, undef: :replace) end private diff --git a/lib/chef/mixin/shell_out.rb b/lib/chef/mixin/shell_out.rb index 18bd067989..1bcf6d8861 100644 --- a/lib/chef/mixin/shell_out.rb +++ b/lib/chef/mixin/shell_out.rb @@ -16,11 +16,12 @@ # limitations under the License. require "mixlib/shellout" -require "chef/deprecated" +require "chef/mixin/path_sanity" class Chef module Mixin module ShellOut + include Chef::Mixin::PathSanity # PREFERRED APIS: # @@ -108,6 +109,7 @@ class Chef "LC_ALL" => Chef::Config[:internal_locale], "LANGUAGE" => Chef::Config[:internal_locale], "LANG" => Chef::Config[:internal_locale], + "PATH" => sanitized_path, }.update(options[env_key] || {}) shell_out_command(*args, **options) end @@ -129,28 +131,6 @@ class Chef cmd end - DEPRECATED_OPTIONS = - [ [:command_log_level, :log_level], - [:command_log_prepend, :log_tag] ] - - # CHEF-3090: Deprecate command_log_level and command_log_prepend - # Patterned after https://github.com/chef/chef/commit/e1509990b559984b43e428d4d801c394e970f432 - def run_command_compatible_options(command_args) - return command_args unless command_args.last.is_a?(Hash) - - my_command_args = command_args.dup - my_options = my_command_args.last - - DEPRECATED_OPTIONS.each do |old_option, new_option| - # Edge case: someone specifies :command_log_level and 'command_log_level' in the option hash - next unless value = my_options.delete(old_option) || my_options.delete(old_option.to_s) - deprecate_option old_option, new_option - my_options[new_option] = value - end - - my_command_args - end - # Helper for sublcasses to convert an array of string args into a string. It # will compact nil or empty strings in the array and will join the array elements # with spaces, without introducing any double spaces for nil/empty elements. @@ -186,16 +166,12 @@ class Chef private def shell_out_command(*command_args) - cmd = Mixlib::ShellOut.new(*run_command_compatible_options(command_args)) + cmd = Mixlib::ShellOut.new(*command_args) cmd.live_stream ||= io_for_live_stream cmd.run_command cmd end - def deprecate_option(old_option, new_option) - Chef.deprecated :internal_api, "Chef::Mixin::ShellOut option :#{old_option} is deprecated. Use :#{new_option}" - end - def io_for_live_stream if STDOUT.tty? && !Chef::Config[:daemon] && Chef::Log.debug? STDOUT diff --git a/spec/unit/mixin/path_sanity_spec.rb b/spec/unit/mixin/path_sanity_spec.rb index 675b5722be..c9ed4e2595 100644 --- a/spec/unit/mixin/path_sanity_spec.rb +++ b/spec/unit/mixin/path_sanity_spec.rb @@ -1,6 +1,6 @@ # # Author:: Seth Chisamore (<schisamo@chef.io>) -# Copyright:: Copyright 2011-2016, Chef Software Inc. +# Copyright:: Copyright 2011-2017, Chef Software Inc. # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -41,19 +41,19 @@ describe Chef::Mixin::PathSanity do it "adds all useful PATHs even if environment is an empty hash" do env = {} @sanity.enforce_path_sanity(env) - expect(env["PATH"]).to eq("#{@ruby_bindir}:#{@gem_bindir}:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin") + expect(env["PATH"]).to eq("#{@gem_bindir}:#{@ruby_bindir}:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin") end it "adds all useful PATHs that are not yet in PATH to PATH" do env = { "PATH" => "" } @sanity.enforce_path_sanity(env) - expect(env["PATH"]).to eq("#{@ruby_bindir}:#{@gem_bindir}:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin") + expect(env["PATH"]).to eq("#{@gem_bindir}:#{@ruby_bindir}:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin") end it "does not re-add paths that already exist in PATH" do env = { "PATH" => "/usr/bin:/sbin:/bin" } @sanity.enforce_path_sanity(env) - expect(env["PATH"]).to eq("/usr/bin:/sbin:/bin:#{@ruby_bindir}:#{@gem_bindir}:/usr/local/sbin:/usr/local/bin:/usr/sbin") + expect(env["PATH"]).to eq("#{@gem_bindir}:#{@ruby_bindir}:/usr/bin:/sbin:/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin") end it "creates path with utf-8 encoding" do @@ -65,7 +65,7 @@ describe Chef::Mixin::PathSanity do it "adds the current executing Ruby's bindir and Gem bindir to the PATH" do env = { "PATH" => "" } @sanity.enforce_path_sanity(env) - expect(env["PATH"]).to eq("#{@ruby_bindir}:#{@gem_bindir}:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin") + expect(env["PATH"]).to eq("#{@gem_bindir}:#{@ruby_bindir}:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin") end it "does not create entries for Ruby/Gem bindirs if they exist in SANE_PATH or PATH" do @@ -75,7 +75,7 @@ describe Chef::Mixin::PathSanity do allow(RbConfig::CONFIG).to receive(:[]).with("bindir").and_return(ruby_bindir) env = { "PATH" => gem_bindir } @sanity.enforce_path_sanity(env) - expect(env["PATH"]).to eq("/yo/gabba/gabba:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin") + expect(env["PATH"]).to eq("/usr/bin:/yo/gabba/gabba:/usr/local/sbin:/usr/local/bin:/usr/sbin:/sbin:/bin") end it "builds a valid windows path" do @@ -86,7 +86,7 @@ describe Chef::Mixin::PathSanity do allow(ChefConfig).to receive(:windows?).and_return(true) env = { "PATH" => 'C:\Windows\system32;C:\mr\softie' } @sanity.enforce_path_sanity(env) - expect(env["PATH"]).to eq("C:\\Windows\\system32;C:\\mr\\softie;#{ruby_bindir};#{gem_bindir}") + expect(env["PATH"]).to eq("#{gem_bindir};#{ruby_bindir};C:\\Windows\\system32;C:\\mr\\softie") end end end diff --git a/spec/unit/mixin/shell_out_spec.rb b/spec/unit/mixin/shell_out_spec.rb index aa38639c1c..1ea7db87a4 100644 --- a/spec/unit/mixin/shell_out_spec.rb +++ b/spec/unit/mixin/shell_out_spec.rb @@ -21,93 +21,13 @@ # require "spec_helper" +require "chef/mixin/path_sanity" describe Chef::Mixin::ShellOut do + include Chef::Mixin::PathSanity + let(:shell_out_class) { Class.new { include Chef::Mixin::ShellOut } } subject(:shell_out_obj) { shell_out_class.new } - describe "#run_command_compatible_options" do - subject { shell_out_obj.run_command_compatible_options(command_args) } - let(:command_args) { [ cmd, options ] } - let(:cmd) { "echo '#{rand(1000)}'" } - - let(:output) { StringIO.new } - let!(:capture_log_output) { Chef::Log.logger = Logger.new(output) } - let(:assume_deprecation_log_level) { allow(Chef::Log).to receive(:level).and_return(:warn) } - - before do - Chef::Config[:treat_deprecation_warnings_as_errors] = false - end - - context "without options" do - let(:command_args) { [ cmd ] } - - it "should not edit command args" do - is_expected.to eql(command_args) - end - end - - context "without deprecated options" do - let(:options) { { :environment => environment } } - let(:environment) { { "LC_ALL" => "C", "LANG" => "C", "LANGUAGE" => "C" } } - - it "should not edit command args" do - is_expected.to eql(command_args) - end - end - - def self.should_emit_deprecation_warning_about(old_option, new_option) - it "should emit a deprecation warning" do - assume_deprecation_log_level && capture_log_output - subject - expect(output.string).to match Regexp.escape(old_option.to_s) - expect(output.string).to match Regexp.escape(new_option.to_s) - end - end - - context "with :command_log_level option" do - let(:options) { { :command_log_level => command_log_level } } - let(:command_log_level) { :warn } - - it "should convert :command_log_level to :log_level" do - is_expected.to eql [ cmd, { :log_level => command_log_level } ] - end - - should_emit_deprecation_warning_about :command_log_level, :log_level - end - - context "with :command_log_prepend option" do - let(:options) { { :command_log_prepend => command_log_prepend } } - let(:command_log_prepend) { "PROVIDER:" } - - it "should convert :command_log_prepend to :log_tag" do - is_expected.to eql [ cmd, { :log_tag => command_log_prepend } ] - end - - should_emit_deprecation_warning_about :command_log_prepend, :log_tag - end - - context "with 'command_log_level' option" do - let(:options) { { "command_log_level" => command_log_level } } - let(:command_log_level) { :warn } - - it "should convert 'command_log_level' to :log_level" do - is_expected.to eql [ cmd, { :log_level => command_log_level } ] - end - - should_emit_deprecation_warning_about :command_log_level, :log_level - end - - context "with 'command_log_prepend' option" do - let(:options) { { "command_log_prepend" => command_log_prepend } } - let(:command_log_prepend) { "PROVIDER:" } - - it "should convert 'command_log_prepend' to :log_tag" do - is_expected.to eql [ cmd, { :log_tag => command_log_prepend } ] - end - - should_emit_deprecation_warning_about :command_log_prepend, :log_tag - end - end context "when testing individual methods" do before(:each) do @@ -127,13 +47,13 @@ describe Chef::Mixin::ShellOut do describe "when the last argument is a Hash" do describe "and environment is an option" do it "should not change environment language settings when they are set to nil" do - options = { :environment => { "LC_ALL" => nil, "LANGUAGE" => nil, "LANG" => nil } } + options = { :environment => { "LC_ALL" => nil, "LANGUAGE" => nil, "LANG" => nil, "PATH" => nil } } expect(shell_out_obj).to receive(:shell_out_command).with(cmd, options).and_return(true) shell_out_obj.shell_out(cmd, options) end it "should not change environment language settings when they are set to non-nil" do - options = { :environment => { "LC_ALL" => "en_US.UTF-8", "LANGUAGE" => "en_US.UTF-8", "LANG" => "en_US.UTF-8" } } + options = { :environment => { "LC_ALL" => "en_US.UTF-8", "LANGUAGE" => "en_US.UTF-8", "LANG" => "en_US.UTF-8", "PATH" => "foo:bar:baz" } } expect(shell_out_obj).to receive(:shell_out_command).with(cmd, options).and_return(true) shell_out_obj.shell_out(cmd, options) end @@ -146,6 +66,7 @@ describe Chef::Mixin::ShellOut do "LC_ALL" => Chef::Config[:internal_locale], "LANG" => Chef::Config[:internal_locale], "LANGUAGE" => Chef::Config[:internal_locale], + "PATH" => sanitized_path, }, }).and_return(true) shell_out_obj.shell_out(cmd, options) @@ -159,6 +80,7 @@ describe Chef::Mixin::ShellOut do "LC_ALL" => Chef::Config[:internal_locale], "LANG" => Chef::Config[:internal_locale], "LANGUAGE" => Chef::Config[:internal_locale], + "PATH" => sanitized_path, }, }).and_return(true) shell_out_obj.shell_out(cmd, options) @@ -168,13 +90,13 @@ describe Chef::Mixin::ShellOut do describe "and env is an option" do it "should not change env when langauge options are set to nil" do - options = { :env => { "LC_ALL" => nil, "LANG" => nil, "LANGUAGE" => nil } } + options = { :env => { "LC_ALL" => nil, "LANG" => nil, "LANGUAGE" => nil, "PATH" => nil } } expect(shell_out_obj).to receive(:shell_out_command).with(cmd, options).and_return(true) shell_out_obj.shell_out(cmd, options) end it "should not change env when language options are set to non-nil" do - options = { :env => { "LC_ALL" => "de_DE.UTF-8", "LANG" => "de_DE.UTF-8", "LANGUAGE" => "de_DE.UTF-8" } } + options = { :env => { "LC_ALL" => "de_DE.UTF-8", "LANG" => "de_DE.UTF-8", "LANGUAGE" => "de_DE.UTF-8", "PATH" => "foo:bar:baz" } } expect(shell_out_obj).to receive(:shell_out_command).with(cmd, options).and_return(true) shell_out_obj.shell_out(cmd, options) end @@ -187,6 +109,7 @@ describe Chef::Mixin::ShellOut do "LC_ALL" => Chef::Config[:internal_locale], "LANG" => Chef::Config[:internal_locale], "LANGUAGE" => Chef::Config[:internal_locale], + "PATH" => sanitized_path, }, }).and_return(true) shell_out_obj.shell_out(cmd, options) @@ -200,6 +123,7 @@ describe Chef::Mixin::ShellOut do "LC_ALL" => Chef::Config[:internal_locale], "LANG" => Chef::Config[:internal_locale], "LANGUAGE" => Chef::Config[:internal_locale], + "PATH" => sanitized_path, }, }).and_return(true) shell_out_obj.shell_out(cmd, options) @@ -216,6 +140,7 @@ describe Chef::Mixin::ShellOut do "LC_ALL" => Chef::Config[:internal_locale], "LANG" => Chef::Config[:internal_locale], "LANGUAGE" => Chef::Config[:internal_locale], + "PATH" => sanitized_path, }, }).and_return(true) shell_out_obj.shell_out(cmd, options) @@ -230,6 +155,7 @@ describe Chef::Mixin::ShellOut do "LC_ALL" => Chef::Config[:internal_locale], "LANG" => Chef::Config[:internal_locale], "LANGUAGE" => Chef::Config[:internal_locale], + "PATH" => sanitized_path, }, }).and_return(true) shell_out_obj.shell_out(cmd) |