Fallback to a temp dir when the home directory is not usable #4951

segiddins · 2016-09-06T18:23:34Z

Closes #4778
This is an alternative to #4886
\c @allenzhao @indirect

ixti · 2016-09-08T11:54:27Z

lib/bundler.rb

+        elsif !File.writable?(home)
+          warning += "\n * `#{home}` is not writable"
+        else
+          return Pathname.new(home)


This will interrupt begin block, return Pathname.new(home) and will leave @user_home.nil?

In other words, in case of happy path @user_home will remain nil and lines 149..159 will be executed every-time on #user_home call.

ixti · 2016-09-08T14:16:19Z

lib/bundler.rb

    def user_bundle_path
-      Pathname.new(Bundler.rubygems.user_home).join(".bundle")
+      Pathname.new(user_home).join(".bundle")


user_name is Pathname already. So it should be safe to leave user_home.join(".bundle")

indirect · 2016-09-10T20:59:39Z

@segiddins ping

segiddins · 2016-09-10T21:11:57Z

I took a day off of working, I'll answer tomorrow

ixti · 2016-09-11T10:10:08Z

lib/bundler/ui/silent.rb

@@ -13,6 +17,7 @@ def confirm(message, newline = nil)
      end

      def warn(message, newline = nil)
+        @warnings |= [message]


Sorry, I accidentally removed @indirect's comment. Original comment was saying that (recovering from memories):

Seems like this means message will never be garbage collected.

until the Silent instance is deallocated, yes

How do you feel about making this a Set instead of using the (esoteric and relatively confusing) |= operator?

How do you feel about using a Set instead of the (esoteric and relatively confusing) |= operator?

That's fine, I think I was just copying what the Shell class did

indirect · 2016-09-20T06:07:51Z

This is my preferred implementation of a fix for #4778, once the comments are addressed. Thanks! 👍

indirect · 2016-09-30T08:11:04Z

Looks good. I didn't realize that Dir.tmpdir returned the same directory across processes! 👍
@homu r+

homu · 2016-09-30T08:11:05Z

📌 Commit 2ac0c51 has been approved by indirect

homu · 2016-09-30T08:11:23Z

⌛ Testing commit 2ac0c51 with merge c899153...

@allenzhao

Fallback to a temp dir when the home directory is not usable Closes #4778 This is an alternative to #4886 \c @allenzhao @indirect

homu · 2016-09-30T09:24:20Z

☀️ Test successful - status

In Bundler 1.14, there was a [merged pull request](rubygems/bundler#4951) that introduced new functionality. When running `bundle` if the location specified in `Bundler.rubygems.user_home` has a problem (e.g. does not exist, is not writable, is not a directory), it prints out a warning message to STDOUT. You can see an example by running in bash: ``` gem install bundler --version 1.14.3 echo "source 'https://rubygems.org';\ngem 'wkhtmltopdf-binary-edge'" > Gemfile bundle install --path vendor/bundle HOME=/no/directory/found bundle exec which wkhtmltopdf ``` When run, you can see the output looks something like: ``` Your home directory is not set properly: * `/no/directory/found` is not a directory Bundler will use `/var/folders/wj/q7cqmwds75z3ndcdxrmpd9gh0000gn/T/bundler/home/rdimarco` as your home directory temporarily ``` The problem is that the [output from `bundle exec which wkhtmltopdf`] (https://github.com/pdfkit/pdfkit/blob/master/lib/pdfkit/configuration.rb#L26) is used to determine the path to the default `wkhtmltopdf` executable. So since there is the warning included in the output, the executable path contains the warning messages and is not valid. This commit will look at the lines returned by the `witch` statement and will return the first file path that it finds, ignoring the warning messages.

In bundler 1.14.0 a feature was added to fallback to a /tmp dir if the user's home dir is not writable (see: rubygems/bundler#4951). As part of this feature bundler outputs a warning that it is doing this. When our check_uricorn_workers script is run by the nagios user it runs a ruby script via bundler using an empty environment which means it has no $HOME env var set. This triggers the warning, which unfortunately goes to stdout and interferes with our script which expects the output to be a number and treats anything else as an error state. If we specify a HOME var when running the ruby script we avoid the problem. We follow the example for setting GOVUK_APP_LOGROOT to /tmp and do the same as this will be globally writable, and hopefully we don't actually have to write anything to run the short ruby script.

When home directory is not writable, but the required .gem and .bundle are, we should use then instead of falling back to use tempdirs. This creates a workaround for more restrictive setups using Omnibus Docker or any hardened setup, to overcome the annoyances introduced by rubygems#4951.

When home directory is not writable, but the required .gem and .bundle are, we should use them instead of falling back to use tempdirs. This creates a workaround for more restrictive setups using Omnibus Docker or any hardened setup, to overcome the annoyances introduced by rubygems#4951.

brodock · 2018-05-23T13:11:16Z

I would love to have an initial review here #6550

It implements a workaround for more hardened installations like omnibus bundled software, where home might not be owned or writable by the user, but would still have required .bundle and .gem folders with correct permission.

When home directory is not writable, but the required .gem and .bundle are, we should use them instead of falling back to use tempdirs. This creates a workaround for more restrictive setups using Omnibus Docker or any hardened setup, to overcome the annoyances introduced by rubygems#4951.

When home directory is not writable, but the required .bundle is, we should use it instead of falling back to use tempdirs. This creates a workaround for more restrictive setups using Omnibus Docker or any hardened setup, to overcome the annoyances introduced by rubygems#4951.

…dale Don't fallback to tempdir when required directories exist. ### What was the end-user problem that led to this PR? When running Omnibus packaged software with updated bundler, a warning is displayed because the home folder is not owned by the user: ``` `/var/opt/gitlab` is not writable. Bundler will use `/tmp/bundler/home/root' as your home directory temporarily. ``` There are valid reasons why this is desired, and I don't have control over it. What I can do is create the required folders used by bundler and provide them with the right permissions. See #6546 ### What was your diagnosis of the problem? In practice instead of asking for permission on a higher level, if required folders are present and they have the right permissions, we shouldn't fallback to warning + temp directory, we should just use what is provided. ### What is your fix for the problem, implemented in this PR? When home directory is not writable, but the required .gem and .bundle are, we should use them instead of falling back to use tempdirs. This creates a workaround for more restrictive setups using Omnibus Docker or any hardened setup, to overcome the annoyances introduced by #4951. ### Why did you choose this fix out of the possible options? This allows for distributions, package maintainers, etc to provide an alternative while keeping their hardenings requirements. When provided the required folders with the required ownership/permission, we should not bother by not having any write permissions on the `$HOME` directory.

…dale Don't fallback to tempdir when required directories exist. ### What was the end-user problem that led to this PR? When running Omnibus packaged software with updated bundler, a warning is displayed because the home folder is not owned by the user: ``` `/var/opt/gitlab` is not writable. Bundler will use `/tmp/bundler/home/root' as your home directory temporarily. ``` There are valid reasons why this is desired, and I don't have control over it. What I can do is create the required folders used by bundler and provide them with the right permissions. See #6546 ### What was your diagnosis of the problem? In practice instead of asking for permission on a higher level, if required folders are present and they have the right permissions, we shouldn't fallback to warning + temp directory, we should just use what is provided. ### What is your fix for the problem, implemented in this PR? When home directory is not writable, but the required .gem and .bundle are, we should use them instead of falling back to use tempdirs. This creates a workaround for more restrictive setups using Omnibus Docker or any hardened setup, to overcome the annoyances introduced by #4951. ### Why did you choose this fix out of the possible options? This allows for distributions, package maintainers, etc to provide an alternative while keeping their hardenings requirements. When provided the required folders with the required ownership/permission, we should not bother by not having any write permissions on the `$HOME` directory. (cherry picked from commit 31b53cf)

segiddins added 3 commits September 7, 2016 14:22

Add an inline spec for eval’ing two gemfiles

002939b

Fallback to a temp dir when the home directory is not usable

02e7f67

Stop stubbing File.expand_path in SharedHelper specs

8246923

segiddins force-pushed the seg-no-writable-home branch from b5bbad4 to 8246923 Compare September 7, 2016 12:22

Require tmpdir since old RubyGems does not

3071a0b

ixti reviewed Sep 8, 2016
View reviewed changes

Ensure that user_home is always cached

2ac0c51

ixti reviewed Sep 8, 2016
View reviewed changes

ixti reviewed Sep 11, 2016
View reviewed changes

homu added a commit that referenced this pull request Sep 30, 2016

Auto merge of #4951 - bundler:seg-no-writable-home, r=indirect

c899153

Fallback to a temp dir when the home directory is not usable Closes #4778 This is an alternative to #4886 \c @allenzhao @indirect

homu merged commit 2ac0c51 into master Sep 30, 2016

coilysiren modified the milestone: 1.13 - Release 1.13.2 Sep 30, 2016

coilysiren modified the milestone: 1.14 - Better Platform Support - Minor 1.14.0 Oct 9, 2016

coilysiren mentioned this pull request Nov 12, 2016

Bundle run as user with no home directory fails #4481

Closed

segiddins deleted the seg-no-writable-home branch December 27, 2016 05:28

This was referenced Jan 26, 2017

Bundler 1.14.x fails on read only docker root #5371

Closed

handle read only filesystem #5385

Closed

robdimarco mentioned this pull request Feb 3, 2017

Ignore Bundler 1.14.* warning messages on a non-writable home directory pdfkit/pdfkit#388

Closed

h-lame mentioned this pull request Mar 7, 2017

Specify an explicit $HOME for nagios scripts that use bundler alphagov/govuk-puppet#5583

Merged

glossawy mentioned this pull request Mar 20, 2018

Failure to locate wkhtmltopdf binary in certain environments using bundler mileszs/wicked_pdf#730

Closed

brodock mentioned this pull request May 22, 2018

Alternative to Fallback to a temp dir when the home directory is not usable #6546

Closed

brodock mentioned this pull request May 23, 2018

Don't fallback to tempdir when required directories exist. #6550

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fallback to a temp dir when the home directory is not usable #4951

Fallback to a temp dir when the home directory is not usable #4951

segiddins commented Sep 6, 2016

ixti Sep 8, 2016

ixti Sep 8, 2016

ixti Sep 8, 2016

indirect commented Sep 10, 2016

segiddins commented Sep 10, 2016

ixti Sep 11, 2016

segiddins Sep 11, 2016

indirect Sep 20, 2016

indirect Sep 20, 2016

segiddins Sep 20, 2016

indirect commented Sep 20, 2016

indirect commented Sep 30, 2016

homu commented Sep 30, 2016

homu commented Sep 30, 2016

homu commented Sep 30, 2016

brodock commented May 23, 2018

Fallback to a temp dir when the home directory is not usable #4951

Fallback to a temp dir when the home directory is not usable #4951

Conversation

segiddins commented Sep 6, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

indirect commented Sep 10, 2016

segiddins commented Sep 10, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

indirect commented Sep 20, 2016

indirect commented Sep 30, 2016

homu commented Sep 30, 2016

homu commented Sep 30, 2016

homu commented Sep 30, 2016

brodock commented May 23, 2018