Skip to content

Commit

Permalink
Merge pull request #14153 from chef/jfm/chef17-backport-homebrew-prs
Browse files Browse the repository at this point in the history
[chef17] backport homebrew prs
  • Loading branch information
johnmccrae committed Dec 29, 2023
2 parents d0d70ea + 86fc924 commit 4f10910
Show file tree
Hide file tree
Showing 7 changed files with 61 additions and 30 deletions.
6 changes: 2 additions & 4 deletions .github/workflows/kitchen.yml
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ jobs:
/opt/chef/bin/chef-client -v
/opt/chef/bin/ohai -v
/opt/chef/embedded/bin/rake --version
/opt/chef/embedded/bin/bundle -v
- name: 'Upgrade Chef/Ohai via Appbundler'
id: upgrade
run: |
Expand All @@ -122,9 +121,8 @@ jobs:
sudo /opt/chef/embedded/bin/bundle config set --local path 'vendor/bundle'
sudo /opt/chef/embedded/bin/bundle install --jobs=3 --retry=3
sudo rm -f /opt/chef/embedded/bin/{htmldiff,ldiff}
gem install berkshelf
sudo berks vendor cookbooks
sudo /opt/chef/embedded/bin/gem install berkshelf --no-doc
sudo /opt/chef/embedded/bin/berks vendor cookbooks
sudo /opt/chef/bin/chef-client -z -o end_to_end --chef-license accept-no-persist
linux:
Expand Down
1 change: 1 addition & 0 deletions cspell.json
Original file line number Diff line number Diff line change
Expand Up @@ -611,6 +611,7 @@
"lgrpi",
"LGRPID",
"LHND",
"linuxbrew",
"linuxmint",
"LISTBOX",
"listprop",
Expand Down
20 changes: 15 additions & 5 deletions lib/chef/mixin/homebrew_user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,18 +57,28 @@ def find_homebrew_username(provided_user = nil)
@homebrew_owner_username
end

def homebrew_bin_path(brew_bin_path = nil)
if brew_bin_path && ::File.exist?(brew_bin_path)
brew_bin_path
else
[which("brew"), "/opt/homebrew/bin/brew", "/usr/local/bin/brew", "/home/linuxbrew/.linuxbrew/bin/brew"].uniq.select do |x|
next if x == false

::File.exist?(x) && ::File.executable?(x)
end.first || nil
end
end

private

def calculate_owner
default_brew_path = "/usr/local/bin/brew"
if ::File.exist?(default_brew_path)
brew_path = homebrew_bin_path
if brew_path
# By default, this follows symlinks which is what we want
owner = ::File.stat(default_brew_path).uid
elsif (brew_path = shell_out("which brew").stdout.strip) && !brew_path.empty?
owner = ::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.'
'Could not find the "brew" executable anywhere on the path.'
end

Chef::Log.debug "Found Homebrew owner #{Etc.getpwuid(owner).name}; executing `brew` commands as them"
Expand Down
15 changes: 7 additions & 8 deletions lib/chef/resource/homebrew_cask.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,25 +46,24 @@ class HomebrewCask < Chef::Resource
default: true

property :homebrew_path, String,
description: "The path to the homebrew binary.",
default: "/usr/local/bin/brew"
description: "The path to the homebrew binary."

property :owner, [String, Integer],
description: "The owner of the Homebrew installation.",
default: lazy { find_homebrew_username },
default_description: "Calculated default username"\
default_description: "Calculated default username"

action :install, description: "Install an application that is packaged as a Homebrew cask." do
if new_resource.install_cask
homebrew_tap "homebrew/cask" do
homebrew_path new_resource.homebrew_path
homebrew_path homebrew_bin_path(new_resource.homebrew_path)
owner new_resource.owner
end
end

unless casked?
converge_by("install cask #{new_resource.cask_name} #{new_resource.options}") do
shell_out!("#{new_resource.homebrew_path} install --cask #{new_resource.cask_name} #{new_resource.options}",
shell_out!("#{homebrew_bin_path(new_resource.homebrew_path)} install --cask #{new_resource.cask_name} #{new_resource.options}",
user: new_resource.owner,
env: { "HOME" => ::Dir.home(new_resource.owner), "USER" => new_resource.owner },
cwd: ::Dir.home(new_resource.owner))
Expand All @@ -75,14 +74,14 @@ class HomebrewCask < Chef::Resource
action :remove, description: "Remove an application that is packaged as a Homebrew cask." do
if new_resource.install_cask
homebrew_tap "homebrew/cask" do
homebrew_path new_resource.homebrew_path
homebrew_path homebrew_bin_path(new_resource.homebrew_path)
owner new_resource.owner
end
end

if casked?
converge_by("uninstall cask #{new_resource.cask_name}") do
shell_out!("#{new_resource.homebrew_path} uninstall --cask #{new_resource.cask_name}",
shell_out!("#{homebrew_bin_path(new_resource.homebrew_path)} uninstall --cask #{new_resource.cask_name}",
user: new_resource.owner,
env: { "HOME" => ::Dir.home(new_resource.owner), "USER" => new_resource.owner },
cwd: ::Dir.home(new_resource.owner))
Expand All @@ -100,7 +99,7 @@ class HomebrewCask < Chef::Resource
# @return [Boolean]
def casked?
unscoped_name = new_resource.cask_name.split("/").last
shell_out!("#{new_resource.homebrew_path} list --cask 2>/dev/null",
shell_out!("#{homebrew_bin_path(new_resource.homebrew_path)} list --cask 2>/dev/null",
user: new_resource.owner,
env: { "HOME" => ::Dir.home(new_resource.owner), "USER" => new_resource.owner },
cwd: ::Dir.home(new_resource.owner)).stdout.split.include?(unscoped_name)
Expand Down
2 changes: 1 addition & 1 deletion lib/chef/resource/homebrew_package.rb
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ class HomebrewPackage < Chef::Resource::Package
DOC

property :homebrew_user, [ String, Integer ],
description: "The name or uid of the Homebrew owner to be used by #{ChefUtils::Dist::Infra::PRODUCT} when executing a command.\n\n#{ChefUtils::Dist::Infra::PRODUCT}, by default, will attempt to execute a Homebrew command as the owner of the `/usr/local/bin/brew` executable. If that executable does not exist, #{ChefUtils::Dist::Infra::PRODUCT} will attempt to find the user by executing `which brew`. If that executable cannot be found, #{ChefUtils::Dist::Infra::PRODUCT} will print an error message: `Could not find the 'brew' executable in /usr/local/bin or anywhere on the path.`.\n\nSet this property to specify the Homebrew owner for situations where Chef Infra Client cannot automatically detect the correct owner.'"
description: "The name or uid of the Homebrew owner to be used by #{ChefUtils::Dist::Infra::PRODUCT} when executing a command.\n\n#{ChefUtils::Dist::Infra::PRODUCT}, by default, will attempt to execute a Homebrew command as the owner of the `/usr/local/bin/brew` executable on x86_64 machines or `/opt/homebrew/bin/brew` executable on arm64 machines. If that executable does not exist, #{ChefUtils::Dist::Infra::PRODUCT} will attempt to find the user by executing `which brew`. If that executable cannot be found, #{ChefUtils::Dist::Infra::PRODUCT} will print an error message: `Could not find the 'brew' executable in /usr/local/bin, /opt/homebrew/bin, or anywhere on the path.`.\n\nSet this property to specify the Homebrew owner for situations where Chef Infra Client cannot automatically detect the correct owner.'"

end
end
Expand Down
10 changes: 5 additions & 5 deletions lib/chef/resource/homebrew_tap.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@ class HomebrewTap < Chef::Resource
description: "The URL of the tap."

property :homebrew_path, String,
description: "The path to the Homebrew binary.",
default: "/usr/local/bin/brew"
description: "The path to the Homebrew binary."

property :owner, String,
description: "The owner of the Homebrew installation.",
Expand All @@ -53,7 +52,7 @@ class HomebrewTap < Chef::Resource
action :tap, description: "Add a Homebrew tap." do
unless tapped?(new_resource.tap_name)
converge_by("tap #{new_resource.tap_name}") do
shell_out!("#{new_resource.homebrew_path} tap #{new_resource.tap_name} #{new_resource.url || ""}",
shell_out!("#{homebrew_bin_path(new_resource.homebrew_path)} tap #{new_resource.tap_name} #{new_resource.url || ""}",
user: new_resource.owner,
env: { "HOME" => ::Dir.home(new_resource.owner), "USER" => new_resource.owner },
cwd: ::Dir.home(new_resource.owner))
Expand All @@ -64,7 +63,7 @@ class HomebrewTap < Chef::Resource
action :untap, description: "Remove a Homebrew tap." do
if tapped?(new_resource.tap_name)
converge_by("untap #{new_resource.tap_name}") do
shell_out!("#{new_resource.homebrew_path} untap #{new_resource.tap_name}",
shell_out!("#{homebrew_bin_path(new_resource.homebrew_path)} untap #{new_resource.tap_name}",
user: new_resource.owner,
env: { "HOME" => ::Dir.home(new_resource.owner), "USER" => new_resource.owner },
cwd: ::Dir.home(new_resource.owner))
Expand All @@ -76,8 +75,9 @@ class HomebrewTap < Chef::Resource
#
# @return [Boolean]
def tapped?(name)
base_path = ["#{::File.dirname(which("brew"))}/../homebrew", "#{::File.dirname(which("brew"))}/../Homebrew", "/opt/homebrew", "/usr/local/Homebrew", "/home/linuxbrew/.linuxbrew"].uniq.select { |x| Dir.exist?(x) }.first
tap_dir = name.gsub("/", "/homebrew-")
::File.directory?("/usr/local/Homebrew/Library/Taps/#{tap_dir}")
::File.directory?("#{base_path}/Library/Taps/#{tap_dir}")
end
end
end
Expand Down
37 changes: 30 additions & 7 deletions spec/unit/mixin/homebrew_user_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ class ExampleHomebrewUser
let(:user) { nil }
let(:brew_owner) { 2001 }
let(:default_brew_path) { "/usr/local/bin/brew" }
let(:default_brew_path_arm) { "/opt/homebrew/bin/brew" }
let(:default_brew_path_linux) { "/home/linuxbrew/.linuxbrew/bin/brew" }
let(:stat_double) do
d = double
expect(d).to receive(:uid).and_return(brew_owner)
Expand All @@ -59,16 +61,38 @@ class ExampleHomebrewUser
expect(Etc).to receive(:getpwuid).with(brew_owner).and_return(OpenStruct.new(name: "name"))
end

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)
def false_unless_specific_value(object, method, value)
allow(object).to receive(method).and_return(false)
allow(object).to receive(method).with(value).and_return(true)
end

it "returns the owner of the brew executable when it is at a default location for x86_64 machines" do
false_unless_specific_value(File, :exist?, default_brew_path)
false_unless_specific_value(File, :executable?, default_brew_path)
allow(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 at a default location for arm machines" do
false_unless_specific_value(File, :exist?, default_brew_path_arm)
false_unless_specific_value(File, :executable?, default_brew_path_arm)
allow(File).to receive(:stat).with(default_brew_path_arm).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 at a default location for linux machines" do
false_unless_specific_value(File, :exist?, default_brew_path_linux)
false_unless_specific_value(File, :executable?, default_brew_path_linux)
allow(File).to receive(:stat).with(default_brew_path_linux).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)
allow_any_instance_of(ExampleHomebrewUser).to receive(:which).and_return("/foo")
false_unless_specific_value(File, :exist?, "/foo")
false_unless_specific_value(File, :executable?, "/foo")
allow(homebrew_user).to receive_message_chain(:shell_out, :stdout, :strip).and_return("/foo")
expect(File).to receive(:stat).with("/foo").and_return(stat_double)
allow(File).to receive(:stat).with("/foo").and_return(stat_double)
expect(homebrew_user.find_homebrew_uid(user)).to eq(brew_owner)
end

Expand All @@ -78,8 +102,7 @@ class ExampleHomebrewUser
describe "when the homebrew user is not provided" do

it "raises an error if no executable is found" do
expect(File).to receive(:exist?).with(default_brew_path).and_return(false)
allow(homebrew_user).to receive_message_chain(:shell_out, :stdout, :strip).and_return("")
expect(File).to receive(:exist?).and_return(nil).at_least(:once)
expect { homebrew_user.find_homebrew_uid(user) }.to raise_error(Chef::Exceptions::CannotDetermineHomebrewOwner)
end

Expand Down

0 comments on commit 4f10910

Please sign in to comment.