Merge branch 'CHEF-2681'

chef/lib/chef/client.rb

@@ -20,6 +20,7 @@
 
 require 'chef/config'
 require 'chef/mixin/params_validate'
+require 'chef/mixin/path_sanity'
 require 'chef/log'
 require 'chef/rest'
 require 'chef/api_client'
@@ -36,14 +37,14 @@
 require 'chef/cookbook/remote_file_vendor'
 require 'chef/version'
 require 'ohai'
+require 'rbconfig'
 
 class Chef
   # == Chef::Client
   # The main object in a Chef run. Preps a Chef::Node and Chef::RunContext,
   # syncs cookbooks if necessary, and triggers convergence.
   class Client
-
-    SANE_PATHS = %w[/usr/local/sbin /usr/local/bin /usr/sbin /usr/bin /sbin /bin]
+    include Chef::Mixin::PathSanity
 
     # Clears all notifications for client run status events.
     # Primarily for testing purposes.
@@ -313,20 +314,6 @@ def converge(run_context)
       true
     end
 
-    def enforce_path_sanity(env=ENV)
-      if Chef::Config[:enforce_path_sanity] && RUBY_PLATFORM !~ /mswin|mingw32|windows/
-        existing_paths = env["PATH"].split(':')
-        SANE_PATHS.each do |sane_path|
-          unless existing_paths.include?(sane_path)
-            env_path = env["PATH"].dup
-            env_path << ':' unless env["PATH"].empty?
-            env_path << sane_path
-            env["PATH"] = env_path
-          end
-        end
-      end
-    end
-
     private
 
     def directory_not_empty?(path)

chef/lib/chef/knife.rb

@@ -21,6 +21,7 @@
 require 'chef/version'
 require 'mixlib/cli'
 require 'chef/mixin/convert_to_class_name'
+require 'chef/mixin/path_sanity'
 require 'chef/knife/core/subcommand_loader'
 require 'chef/knife/core/ui'
 require 'chef/rest'
@@ -32,6 +33,7 @@ class Knife
     Chef::REST::RESTRequest.user_agent = "Chef Knife#{Chef::REST::RESTRequest::UA_COMMON}"
 
     include Mixlib::CLI
+    include Chef::Mixin::PathSanity
     extend Chef::Mixin::ConvertToClassName
     extend Forwardable
 
@@ -388,6 +390,7 @@ def run_with_pretty_exceptions
       unless self.respond_to?(:run)
         ui.error "You need to add a #run method to your knife command before you can use it"
       end
+      enforce_path_sanity
       run
     rescue Exception => e
       raise if config[:verbosity] == 2

chef/lib/chef/mixin/path_sanity.rb

@@ -0,0 +1,66 @@
+#
+# Author:: Seth Chisamore (<schisamo@opscode.com>)
+# Copyright:: Copyright (c) 2011 Opscode, Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# 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
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+class Chef
+  module Mixin
+    module PathSanity
+
+      def enforce_path_sanity(env=ENV)
+        if Chef::Config[:enforce_path_sanity]
+          path_separator = RbConfig::CONFIG['host_os'] =~ /mswin|mingw|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
+            end
+          end
+        end
+      end
+
+      private
+
+      def sane_paths
+        @sane_paths ||= begin
+          if RbConfig::CONFIG['host_os'] =~ /mswin|mingw|windows/
+            %w[]
+          else
+            %w[/usr/local/sbin /usr/local/bin /usr/sbin /usr/bin /sbin /bin]
+          end
+        end
+      end
+
+      def ruby_bindir
+        RbConfig::CONFIG['bindir']
+      end
+
+      def gem_bindir
+        Gem.bindir
+      end
+
+    end
+  end
+end

chef/lib/chef/mixins.rb

@@ -10,6 +10,7 @@
 require 'chef/mixin/language_include_attribute'
 require 'chef/mixin/language_include_recipe'
 require 'chef/mixin/params_validate'
+require 'chef/mixin/path_sanity'
 require 'chef/mixin/recipe_definition_dsl_core'
 require 'chef/mixin/template'
 require 'chef/mixin/xml_escape'

chef/spec/unit/client_spec.rb

@@ -22,6 +22,7 @@
 
 require 'chef/run_context'
 require 'chef/rest'
+require 'rbconfig'
 
 describe Chef::Client do
   before do
@@ -50,24 +51,6 @@
     @client.node = @node
   end
 
-  describe "when enforcing path sanity" do
-    before do
-      Chef::Config[:enforce_path_sanity] = true
-    end
-
-    it "adds all useful PATHs that are not yet in PATH to PATH" do
-      env = {"PATH" => ""}
-      @client.enforce_path_sanity(env)
-      env["PATH"].should == "/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"}
-      @client.enforce_path_sanity(env)
-      env["PATH"].should == "/usr/bin:/sbin:/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin"
-    end
-  end
-
   describe "run" do
     it "should identify the node and run ohai, then register the client" do
       mock_chef_rest_for_node = mock("Chef::REST (node)")

chef/spec/unit/mixin/path_sanity_spec.rb

@@ -0,0 +1,80 @@
+#
+# Author:: Seth Chisamore (<schisamo@opscode.com>)
+# Copyright:: Copyright (c) 2011 Opscode, Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# 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
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+require File.expand_path(File.join(File.dirname(__FILE__), "..", "..", "spec_helper"))
+
+class PathSanityTestHarness
+  include Chef::Mixin::PathSanity
+end
+
+describe Chef::Mixin::PathSanity do
+
+  before do
+    @sanity = PathSanityTestHarness.new
+  end
+
+  describe "when enforcing path sanity" do
+    before do
+      Chef::Config[:enforce_path_sanity] = true
+      @ruby_bindir = '/some/ruby/bin'
+      @gem_bindir = '/some/gem/bin'
+      Gem.stub!(:bindir).and_return(@gem_bindir)
+      RbConfig::CONFIG.stub!(:[]).with('bindir').and_return(@ruby_bindir)
+      RbConfig::CONFIG.stub!(:[]).with('host_os').and_return('darwin11.1.0')
+    end
+
+    it "adds all useful PATHs that are not yet in PATH to PATH" do
+      env = {"PATH" => ""}
+      @sanity.enforce_path_sanity(env)
+      env["PATH"].should == "#{@ruby_bindir}:#{@gem_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)
+      env["PATH"].should == "/usr/bin:/sbin:/bin:#{@ruby_bindir}:#{@gem_bindir}:/usr/local/sbin:/usr/local/bin:/usr/sbin"
+    end
+
+    it "adds the current executing Ruby's bindir and Gem bindir to the PATH" do
+      env = {"PATH" => ""}
+      @sanity.enforce_path_sanity(env)
+      env["PATH"].should == "#{@ruby_bindir}:#{@gem_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
+      ruby_bindir = '/usr/bin'
+      gem_bindir = '/yo/gabba/gabba'
+      Gem.stub!(:bindir).and_return(gem_bindir)
+      RbConfig::CONFIG.stub!(:[]).with('bindir').and_return(ruby_bindir)
+      env = {"PATH" => gem_bindir}
+      @sanity.enforce_path_sanity(env)
+      env["PATH"].should == "/yo/gabba/gabba:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
+    end
+
+    it "builds a valid windows path" do
+      ruby_bindir = 'C:\ruby\bin'
+      gem_bindir = 'C:\gems\bin'
+      Gem.stub!(:bindir).and_return(gem_bindir)
+      RbConfig::CONFIG.stub!(:[]).with('bindir').and_return(ruby_bindir)
+      RbConfig::CONFIG.stub!(:[]).with('host_os').and_return('mswin')
+      env = {"PATH" => 'C:\Windows\system32;C:\mr\softie'}
+      @sanity.enforce_path_sanity(env)
+      env["PATH"].should == "C:\\Windows\\system32;C:\\mr\\softie;#{ruby_bindir};#{gem_bindir}"
+    end
+  end
+end