homebrew_owner now tries to run homebrew as the user who owns the `brew` executable, or an optional homebrew_package resource attribute

9 files changed, 212 insertions, 132 deletions

diff --git a/lib/chef/exceptions.rb b/lib/chef/exceptions.rb
index 67429ac5a2..f08cc5eb96 100644
--- a/lib/chef/exceptions.rb
+++ b/lib/chef/exceptions.rb
@@ -121,6 +121,7 @@ class Chef
     class PowershellCmdletException < RuntimeError; end
 
     class CannotDetermineHomebrewOwner < Package; end
+    class HomebrewOwnerIsRoot < ArgumentError; end
 
     # A different version of a cookbook was added to a
     # VersionedRecipeList than the one already there.
diff --git a/lib/chef/mixin/homebrew_owner.rb b/lib/chef/mixin/homebrew_owner.rb
deleted file mode 100644
index 73bb22ddf5..0000000000
--- a/lib/chef/mixin/homebrew_owner.rb
+++ /dev/null
@@ -1,58 +0,0 @@
-#
-# Author:: Joshua Timberman (<joshua@getchef.com>)
-# Author:: Graeme Mathieson (<mathie@woss.name>)
-#
-# Copyright 2011-2013, Opscode, Inc.
-# Copyright 2014, Chef Software, Inc <legal@getchef.com>
-#
-# 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.
-#
-# Ported from the homebrew cookbook's Homebrew::Mixin owner helpers
-#
-# This lives here in Chef::Mixin because Chef's namespacing makes it
-# awkward to use modules elsewhere (e.g., chef/provider/package/homebrew/owner)
-
-class Chef
-  module Mixin
-    module HomebrewOwner
-      def homebrew_owner(node)
-        @homebrew_owner ||= calculate_owner(node)
-      end
-
-      private
-
-      def calculate_owner(node)
-        owner = homebrew_owner_attr(node) || sudo_user || current_user
-        if owner == 'root'
-          raise Chef::Exceptions::CannotDetermineHomebrewOwner,
-            'The homebrew owner is not specified and the current user is \"root\"' +
-            'Homebrew does not support root installs, please specify the homebrew' +
-            'owner by setting the attribute `node[\'homebrew\'][\'owner\']`.'
-        end
-        owner
-      end
-
-      def homebrew_owner_attr(node)
-        node['homebrew']['owner'] if node.attribute?('homebrew') && node['homebrew'].attribute?('owner')
-      end
-
-      def sudo_user
-        ENV['SUDO_USER']
-      end
-
-      def current_user
-        ENV['USER']
-      end
-    end
-  end
-end
diff --git a/lib/chef/mixin/homebrew_user.rb b/lib/chef/mixin/homebrew_user.rb
new file mode 100644
index 0000000000..05bc42f432
--- /dev/null
+++ b/lib/chef/mixin/homebrew_user.rb
@@ -0,0 +1,71 @@
+#
+# Author:: Joshua Timberman (<joshua@getchef.com>)
+# Author:: Graeme Mathieson (<mathie@woss.name>)
+#
+# Copyright 2011-2013, Opscode, Inc.
+# Copyright 2014, Chef Software, Inc <legal@getchef.com>
+#
+# 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.
+#
+# Ported from the homebrew cookbook's Homebrew::Mixin owner helpers
+#
+# This lives here in Chef::Mixin because Chef's namespacing makes it
+# awkward to use modules elsewhere (e.g., chef/provider/package/homebrew/owner)
+
+require 'chef/mixin/shell_out'
+require 'etc'
+
+class Chef
+  module Mixin
+    module HomebrewUser
+      include Chef::Mixin::ShellOut
+
+      ##
+      # This tries to find the user to execute brew as.  If a user is provided, that overrides the brew
+      # executable user.  It is an error condition if the brew executable owner is root or we cannot find
+      # the brew executable.
+      def find_homebrew_uid(provided_user)
+        # They could provide us a user name or a UID
+        unless provided_user.nil?
+          return provided_user if provided_user.is_a? Integer
+          return Etc.getpwnam(provided_user).uid
+        end
+
+        @homebrew_owner ||= calculate_owner
+        if @homebrew_owner == 0
+          raise Chef::Exceptions::HomebrewOwnerIsRoot,
+                'The homebrew owner is currently "root".  This is not suggested by the' +
+                    'homebrew maintainers.'
+        end
+        @homebrew_owner
+      end
+
+      private
+
+      def calculate_owner
+        default_brew_path = '/usr/local/bin/brew'
+        if ::File.exist?(default_brew_path)
+          # By default, this follows symlinks which is what we want
+          ::File.stat(default_brew_path).uid
+        elsif (brew_path = shell_out("which brew").stdout.strip) && !brew_path.empty?
+          ::File.stat(brew_path).uid
+        else
+          raise Chef::Exceptions::CannotDetermineHomebrewOwner,
+                'Could not find the "brew" executable in /usr/local/bin or anywhere on the ' +
+                    'path.'
+        end
+      end
+
+    end
+  end
+end
diff --git a/lib/chef/provider/package/homebrew.rb b/lib/chef/provider/package/homebrew.rb
index d964703c87..202e4d2533 100644
--- a/lib/chef/provider/package/homebrew.rb
+++ b/lib/chef/provider/package/homebrew.rb
@@ -19,13 +19,13 @@
 #
 
 require 'etc'
-require 'chef/mixin/homebrew_owner'
+require 'chef/mixin/homebrew_user'
 
 class Chef
   class Provider
     class Package
       class Homebrew < Chef::Provider::Package
-        include Chef::Mixin::HomebrewOwner
+        include Chef::Mixin::HomebrewUser
         def load_current_resource
           self.current_resource = Chef::Resource::Package.new(new_resource.name)
           current_resource.package_name(new_resource.package_name)
@@ -109,12 +109,14 @@ class Chef
         private
 
         def get_response_from_command(command)
-          home_dir = Etc.getpwnam(homebrew_owner(node)).dir
+          homebrew_uid = find_homebrew_uid(new_resource.homebrew_user)
+          homebrew_user = Etc.getpwuid(homebrew_uid)
 
-          Chef::Log.debug "Executing '#{command}' as user '#{homebrew_owner(node)}'"
-          output = shell_out!(command, :timeout => 1800, :user => homebrew_owner(node), :environment => { 'HOME' => home_dir, 'RUBYOPT' => nil })
+          Chef::Log.debug "Executing '#{command}' as user '#{homebrew_user.name}'"
+          output = shell_out!(command, :timeout => 1800, :user => homebrew_uid, :environment => { 'HOME' => homebrew_user.dir, 'RUBYOPT' => nil })
           output.stdout.chomp
         end
+
       end
     end
   end
diff --git a/lib/chef/resource/homebrew_package.rb b/lib/chef/resource/homebrew_package.rb
index c3fa9ddffc..e1d50c1739 100644
--- a/lib/chef/resource/homebrew_package.rb
+++ b/lib/chef/resource/homebrew_package.rb
@@ -24,11 +24,22 @@ require 'chef/resource/package'
 class Chef
   class Resource
     class HomebrewPackage < Chef::Resource::Package
+
       def initialize(name, run_context=nil)
         super
         @resource_name = :homebrew_package
         @provider = Chef::Provider::Package::Homebrew
+        @homebrew_user = nil
+      end
+
+      def homebrew_user(arg=nil)
+        set_or_return(
+            :homebrew_user,
+            arg,
+            :kind_of => [ String, Integer ]
+        )
       end
+
     end
   end
 end
diff --git a/spec/unit/mixin/homebrew_owner_spec.rb b/spec/unit/mixin/homebrew_owner_spec.rb
deleted file mode 100644
index 428cd827d9..0000000000
--- a/spec/unit/mixin/homebrew_owner_spec.rb
+++ /dev/null
@@ -1,65 +0,0 @@
-#
-# Author:: Joshua Timberman (<joshua@getchef.com>)
-#
-# Copyright 2014, Chef Software, Inc <legal@getchef.com>
-#
-# 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 'spec_helper'
-require 'chef/mixin/homebrew_owner'
-
-class ExampleHomebrewOwner
-  include Chef::Mixin::HomebrewOwner
-end
-
-describe Chef::Mixin::HomebrewOwner do
-  before(:each) do
-    node.default['homebrew']['owner'] = nil
-  end
-
-  let(:homebrew_owner) { ExampleHomebrewOwner.new }
-  let(:node) { Chef::Node.new }
-
-  describe 'when the homebrew owner node attribute is set' do
-    it 'raises an exception if the owner is root' do
-      node.default['homebrew']['owner'] = 'root'
-      expect { homebrew_owner.homebrew_owner(node) }.to raise_exception(Chef::Exceptions::CannotDetermineHomebrewOwner)
-    end
-
-    it 'returns the owner set by attribute' do
-      node.default['homebrew']['owner'] = 'siouxsie'
-      expect(homebrew_owner.homebrew_owner(node)).to eql('siouxsie')
-    end
-  end
-
-  describe 'when the owner attribute is not set and we use sudo' do
-    before(:each) do
-      ENV.stub(:[]).with('SUDO_USER').and_return('john_lydon')
-    end
-
-    it 'uses the SUDO_USER environment variable' do
-      expect(homebrew_owner.homebrew_owner(node)).to eql('john_lydon')
-    end
-  end
-
-  describe 'when the owner attribute is not set and we arent using sudo' do
-    before(:each) do
-      ENV.stub(:[]).with('USER').and_return('sid_vicious')
-      ENV.stub(:[]).with('SUDO_USER').and_return(nil)
-    end
-
-    it 'uses the current user' do
-      expect(homebrew_owner.homebrew_owner(node)).to eql('sid_vicious')
-    end
-  end
-end
diff --git a/spec/unit/mixin/homebrew_user_spec.rb b/spec/unit/mixin/homebrew_user_spec.rb
new file mode 100644
index 0000000000..a27a6a0460
--- /dev/null
+++ b/spec/unit/mixin/homebrew_user_spec.rb
@@ -0,0 +1,91 @@
+#
+# Author:: Joshua Timberman (<joshua@getchef.com>)
+#
+# Copyright 2014, Chef Software, Inc <legal@getchef.com>
+#
+# 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 'spec_helper'
+require 'chef/mixin/homebrew_user'
+
+class ExampleHomebrewUser
+  include Chef::Mixin::HomebrewUser
+end
+
+describe Chef::Mixin::HomebrewUser do
+  before(:each) do
+    node.default['homebrew']['owner'] = nil
+  end
+
+  let(:homebrew_user) { ExampleHomebrewUser.new }
+  let(:node) { Chef::Node.new }
+
+  describe 'when the homebrew user is provided' do
+    let(:uid) { 1001 }
+    let(:user) { "foo" }
+
+    it 'returns the homebrew user without looking at the file when uid is provided' do
+      expect(File).to receive(:exist?).exactly(0).times
+      expect(homebrew_user.find_homebrew_uid(uid)).to eq(uid)
+    end
+
+    it 'returns the homebrew user without looking at the file when name is provided' do
+      expect(File).to receive(:exist?).exactly(0).times
+      Etc.stub_chain(:getpwnam, :uid).and_return(uid)
+      expect(homebrew_user.find_homebrew_uid(user)).to eq(uid)
+    end
+
+  end
+
+  describe 'when the homebrew user is not provided' do
+    let(:user) { nil }
+    let(:brew_owner) { 2001 }
+    let(:default_brew_path) { '/usr/local/bin/brew' }
+    let(:stat_double) {
+      d = double()
+      expect(d).to receive(:uid).and_return(brew_owner)
+      d
+    }
+
+    it 'returns the owner of the brew executable when it is at a default location' do
+      expect(File).to receive(:exist?).with(default_brew_path).and_return(true)
+      expect(File).to receive(:stat).with(default_brew_path).and_return(stat_double)
+      expect(homebrew_user.find_homebrew_uid(user)).to eq(brew_owner)
+    end
+
+    it 'returns the owner of the brew executable when it is not at a default location' do
+      expect(File).to receive(:exist?).with(default_brew_path).and_return(false)
+      homebrew_user.stub_chain(:shell_out, :stdout, :strip).and_return("/foo")
+      expect(File).to receive(:stat).with("/foo").and_return(stat_double)
+      expect(homebrew_user.find_homebrew_uid(user)).to eq(brew_owner)
+    end
+
+    it 'raises an error if no executable is found' do
+      expect(File).to receive(:exist?).with(default_brew_path).and_return(false)
+      homebrew_user.stub_chain(:shell_out, :stdout, :strip).and_return("")
+      expect { homebrew_user.find_homebrew_uid(user) }.to raise_error(Chef::Exceptions::CannotDetermineHomebrewOwner)
+    end
+
+    describe "the executable is owned by root" do
+      let(:brew_owner) { 0 }
+
+      it 'raises an error' do
+        expect(File).to receive(:exist?).with(default_brew_path).and_return(true)
+        expect(File).to receive(:stat).with(default_brew_path).and_return(stat_double)
+        expect { homebrew_user.find_homebrew_uid(user) }.to raise_error(Chef::Exceptions::HomebrewOwnerIsRoot)
+      end
+    end
+
+  end
+
+end
diff --git a/spec/unit/provider/package/homebrew_spec.rb b/spec/unit/provider/package/homebrew_spec.rb
index 9f105c13b8..d38458546d 100644
--- a/spec/unit/provider/package/homebrew_spec.rb
+++ b/spec/unit/provider/package/homebrew_spec.rb
@@ -28,6 +28,8 @@ describe Chef::Provider::Package::Homebrew do
     Chef::Provider::Package::Homebrew.new(new_resource, run_context)
   end
 
+  let(:homebrew_uid) { 1001 }
+
   let(:uninstalled_brew_info) do
     {
       'name' => 'emacs',
@@ -92,8 +94,7 @@ describe Chef::Provider::Package::Homebrew do
   end
 
   before(:each) do
-    node.default['homebrew']['owner'] = 'sid_vicious'
-    allow(Etc).to receive(:getpwnam).with('sid_vicious').and_return('/Users/sid_vicious')
+
   end
 
   describe 'load_current_resource' do
@@ -143,13 +144,18 @@ describe Chef::Provider::Package::Homebrew do
   end
 
   describe 'brew' do
+    before do
+      expect(provider).to receive(:find_homebrew_uid).and_return(homebrew_uid)
+      expect(Etc).to receive(:getpwuid).with(homebrew_uid).and_return(OpenStruct.new(:name => "name", :dir => "/"))
+    end
+
     it 'passes a single to the brew command and return stdout' do
-      allow(provider).to receive(:get_response_from_command).and_return('zombo')
+      allow(provider).to receive(:shell_out!).and_return(OpenStruct.new(:stdout => 'zombo'))
       expect(provider.brew).to eql('zombo')
     end
 
     it 'takes multiple arguments as an array' do
-      allow(provider).to receive(:get_response_from_command).and_return('homestarrunner')
+      allow(provider).to receive(:shell_out!).and_return(OpenStruct.new(:stdout => 'homestarrunner'))
       expect(provider.brew('info', 'opts', 'bananas')).to eql('homestarrunner')
     end
   end
diff --git a/spec/unit/resource/homebrew_package_spec.rb b/spec/unit/resource/homebrew_package_spec.rb
index 4b4f9afe5e..bb657607b7 100644
--- a/spec/unit/resource/homebrew_package_spec.rb
+++ b/spec/unit/resource/homebrew_package_spec.rb
@@ -33,4 +33,25 @@ describe Chef::Resource::HomebrewPackage, 'initialize' do
     expect(resource.provider).to eql(Chef::Provider::Package::Homebrew)
   end
 
+  it 'sets the homebrew_user to nil' do
+    expect(resource.homebrew_user).to eql(nil)
+  end
+
+  shared_examples 'home_brew user set and returned' do
+    it 'returns the configured homebrew_user' do
+      resource.homebrew_user user
+      expect(resource.homebrew_user).to eql(user)
+    end
+  end
+
+  context 'homebrew_user is set' do
+    let(:user) { 'Captain Picard' }
+    include_examples 'home_brew user set and returned'
+
+    context 'as an integer' do
+      let(:user) { 1001 }
+      include_examples 'home_brew user set and returned'
+    end
+  end
+
 end