Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[chef17] backport homebrew prs #14153

Merged
merged 2 commits into from
Dec 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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