sous-chefs / homebrew

Development repository for the homebrew cookbook
https://supermarket.chef.io/cookbooks/homebrew
Apache License 2.0
151 stars 136 forks source link

Cannot create directory[/opt/homebrew-cask] when running chef-client as a non-root user #70

Closed glevine closed 8 years ago

glevine commented 9 years ago

When ensuring directories, the cask recipe makes the assumption that chef-client has been run either as root or with elevated privileges. If running chef-client in local mode as the current user, I run into the following error.

================================================================================
Error executing action `create` on resource 'directory[/opt/homebrew-cask]'
================================================================================

Chef::Exceptions::InsufficientPermissions
-----------------------------------------
Cannot create directory[/opt/homebrew-cask] at /opt/homebrew-cask due to insufficient permissions

Resource Declaration:
---------------------
# In /Users/glevine/.chef/local-mode-cache/cache/cookbooks/homebrew/recipes/cask.rb

 31: directory '/opt/homebrew-cask' do
 32:   owner node['homebrew']['owner'] || homebrew_owner
 33:   mode 00775
 34: end
 35: 

Compiled Resource:
------------------
# Declared in /Users/glevine/.chef/local-mode-cache/cache/cookbooks/homebrew/recipes/cask.rb:31:in `from_file'

directory("/opt/homebrew-cask") do
  action :create
  retries 0
  retry_delay 2
  default_guard_interpreter :default
  path "/opt/homebrew-cask"
  declared_type :directory
  cookbook_name "homebrew"
  recipe_name "cask"
  homebrew_owner 501
  owner 501
  mode 509
end

[2015-04-28T23:06:01-04:00] INFO: Running queued delayed notifications before re-raising exception
[2015-04-28T23:06:01-04:00] DEBUG: Re-raising exception: Chef::Exceptions::InsufficientPermissions - directory[/opt/homebrew-cask] (homebrew::cask line 31) had an error: Chef::Exceptions::InsufficientPermissions: Cannot create directory[/opt/homebrew-cask] at /opt/homebrew-cask due to insufficient permissions

The java cookbook includes the homebrew::cask recipe in its homebrew recipe (https://github.com/agileorbit-cookbooks/java/blob/master/recipes/homebrew.rb). So when installing Java using Homebrew, it seems like it is a requirement to run chef-client as root. This has two smells to me:

  1. The user is effectively required to install Java as root, which runs counter to how Homebrew is designed.
  2. Even if I wrote a wrapper script to create the directory before calling chef-client, then I would still run into this issue anytime I tried to use the java community cookbook.

The java cookbook is just an example, though. This could be true for any dependent cookbook. But it is especially difficult to work around when trying to use community cookbooks.

Maybe this could be passed off as an issue with the java cookbook. But it just feels like cookbooks dependent on the homebrew cookbook are not expecting this cookbook to behave this way -- if being permission agnostic is a goal.

jtimberman commented 9 years ago

If running chef-client in local mode as the current user, I run into the following error.

Yes. This is actually expected, as /opt is not writable by the normal user. A thing that Homebrew and Homebrew Cask do is shell out with sudo to create the directories they need.

When running these recipes with Chef, it's possible that sudo is executed transparently when they're installing, or it might prompt for a password. Either way, while homebrew recommends against doing things with sudo, the fact is you still need to do certain things on your system with elevated privileges. For example, when you install software from a .pkg, and you get prompted for your password by OS X, that's a sudo prompt that it's going to use behind the GUI to perform the installation.

glevine commented 9 years ago

In my case, I already have Homebrew and Cask installed. So when testing a recipe, I wasn't expecting this failure. If I run chef-client in local mode as myself and /opt/homebrew-cask already exists (and is owned by me), then I would expect everything to run just fine. If that directory didn't already exist in such a state, then I would totally understand the error.

The Cask installer requires sudo to create its directories, but then it modifies the permissions of everything except /opt so that sudo isn't necessary for installations of software. My point about it being counter to Homebrew's recommendations was only that after installation of the tool, sudo should no longer be necessary.

So I guess the real issue is that the cask recipe isn't taking into account whether or not those directories already exist and are in the correct state. The default recipe uses a not_if (https://github.com/opscode-cookbooks/homebrew/blob/master/recipes/default.rb#L37) to prevent a second attempt at installing Homebrew. It may not be as trivial, but I feel like the the cask recipe should avoid repeating installation steps, too.

Maybe the root cause is in the way that directory works and is thus an issue with chef itself? It appears to attempt to make the directory as opposed to testing its state first. Although, that is just speculation on my part.

kbruner commented 9 years ago

+1 Same problem here. /opt/homebrew-cask already exists, is already owned by the homebrew user. And yes, it's because the chef directory resource provider checks permissions of the parent directory before it checks whether it even needs to take action.

glevine commented 9 years ago

Is it best to open an issue against chef for the directory resource provider? I just don't know enough about the existing use cases of that resource provider to know if it would open a can of worms.

glevine commented 9 years ago

@jtimberman Do you have an opinion about how best to handle this? Thanks.

seanfisk commented 8 years ago

I have been having this same problem as well. Do we even need the following code? Doesn't the Cask installer create these directories if they don't exist?

directory '/Library/Caches/Homebrew/Casks' do
  owner homebrew_owner
  mode 00775
  only_if { ::Dir.exist?('/Library/Caches/Homebrew') }
end

directory '/opt/homebrew-cask' do
  owner homebrew_owner
  mode 00775
  recursive true
end

directory '/opt/homebrew-cask/Caskroom' do
  owner homebrew_owner
  mode 00775
end
aramprice commented 8 years ago

Relatedly the default Caskroom location has moved per chef-cookbooks/homebrew#100 and caskroom/homebrew-cask#21603

tas50 commented 8 years ago

We no longer manage this directory in the latest version so I'm going to close this out