Re: Support default_middleware configurator method
On 09/13 10:34, Eric Wong wrote: > Jeremy Evans wrote: > > I was originally looking for a way to turn off default middleware > > without having to specify the -N command line option every time. > > After reading the Unicorn.builder code, I thought it may be > > possible to do this by specifying the option as an embedded option > > in the rackup file, via the following line at the top of the file: > > > > #\-N > > > > Unfortunately, while this works for many other command line options, > > because of Unicorn's internals, this doesn't work for -N, as > > embedded options are not parsed until after the check for skipping > > default middleware is made. > > That was intentional, I think. We should never be encouraging > users to pollute config.ru and make it difficult to migrate/test > other servers. > > > This patch makes the embedded -N option work, as well as adds a > > default_middleware configuration file option > > I really hate the -N vs RACK_ENV=none redundancy... Is -N still > needed for Rails or something else while keeping RACK_ENV unset? I use RACK_ENV to configure the environment for my web applications, and for a few reasons have used development/test/production as the environment names. I try as much as possible to have the development environment mirror the production environment unless there is a good reason to be different, and would like to avoid adding all of the middleware Unicorn adds in development mode. Additionally, let's say I want to add Rack::CommonLogger to both development and production environments. So in my config.ru, I add: use Rack::CommonLogger, Logger.new($stderr) Now I load up the development version. Then I have two common loggers in development, with all requests being logged twice. I currently work around this by doing something like: unless ENV['RACK_ENV'] == 'development' use Rack::CommonLogger, Logger.new($stderr) end I would like to avoid having to do that. > Worse case is we only support default_middleware as a config > option (but I prefer not to add more options). Embedded -N support > is an anti-feature which leads to lock-in. I don't really care about embedded -N support. I actually didn't even know embedded command file options were possible until I saw the comment in Unicorn.builder. I just want some way to specify not to load default middleware without requiring a command line option each time. Thanks, Jeremy -- unsubscribe: unicorn-public+unsubscr...@bogomips.org archive: https://bogomips.org/unicorn-public/
Re: Make Worker#user support different process primary group and log file group
Jeremy Evans wrote: > This patch allows Worker#user to accept the group argument as an array > of two strings, the first string being the process primary group, and > the second string being the group that owns the log files. This can > help when you have a large number of applications that use unique > primary groups, and want to have a user with the ability to read the log > files for any of the applications, especially if you are on an operating > system that only supports a small number of groups per user. Fwiw, I don't understand why there's a blurb here for an isolated patch when your commit message below is is sufficient :) Along the same lines, "--cover-letter" in git-format-patch(1) isn't necessary for single patches, either, but they help with a multi-patch series. > Subject: [PATCH] Make Worker#user support different process primary group and > log file group > > Previously, Unicorn always used the process's primary group as the > the group of the log file. However, there are reasons to use a > separate group for the log files, such as when you have many > applications where each application uses it's own user and primary > group, but you want to be able to have a user read the log files > for all applications. Some operating systems have a fairly small > limit on the number of groups per user, and it may not be feasible > to have a user be in the primary group for all applications. > a primary group Anyways, this featureseems acceptable. Pushed to master as 47fddb53aa0b7763f353ba515cf3fb5b2059f4f7 Thanks -- unsubscribe: unicorn-public+unsubscr...@bogomips.org archive: https://bogomips.org/unicorn-public/
Re: Support default_middleware configurator method
Jeremy Evans wrote: > I was originally looking for a way to turn off default middleware > without having to specify the -N command line option every time. > After reading the Unicorn.builder code, I thought it may be > possible to do this by specifying the option as an embedded option > in the rackup file, via the following line at the top of the file: > > #\-N > > Unfortunately, while this works for many other command line options, > because of Unicorn's internals, this doesn't work for -N, as > embedded options are not parsed until after the check for skipping > default middleware is made. That was intentional, I think. We should never be encouraging users to pollute config.ru and make it difficult to migrate/test other servers. > This patch makes the embedded -N option work, as well as adds a > default_middleware configuration file option I really hate the -N vs RACK_ENV=none redundancy... Is -N still needed for Rails or something else while keeping RACK_ENV unset? Worse case is we only support default_middleware as a config option (but I prefer not to add more options). Embedded -N support is an anti-feature which leads to lock-in. Thanks. -- unsubscribe: unicorn-public+unsubscr...@bogomips.org archive: https://bogomips.org/unicorn-public/
Make Worker#user support different process primary group and log file group
This patch allows Worker#user to accept the group argument as an array of two strings, the first string being the process primary group, and the second string being the group that owns the log files. This can help when you have a large number of applications that use unique primary groups, and want to have a user with the ability to read the log files for any of the applications, especially if you are on an operating system that only supports a small number of groups per user. Thanks, Jeremy >From 780bd79d2f1fd40e7daae969ff482c3fd9d58ad7 Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Thu, 13 Sep 2018 11:16:38 -0700 Subject: [PATCH] Make Worker#user support different process primary group and log file group Previously, Unicorn always used the process's primary group as the the group of the log file. However, there are reasons to use a separate group for the log files, such as when you have many applications where each application uses it's own user and primary group, but you want to be able to have a user read the log files for all applications. Some operating systems have a fairly small limit on the number of groups per user, and it may not be feasible to have a user be in the primary group for all applications. a primary group --- lib/unicorn/worker.rb | 18 -- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/lib/unicorn/worker.rb b/lib/unicorn/worker.rb index 68de17e..5ddf379 100644 --- a/lib/unicorn/worker.rb +++ b/lib/unicorn/worker.rb @@ -122,6 +122,11 @@ def close # :nodoc: # the +after_fork+ hook after any privileged functions need to be # run (e.g. to set per-worker CPU affinity, niceness, etc) # + # +group+ can be specified as a string, or as an array of two + # strings. If an array of two strings is given, the first string + # is used as the primary group of the process, and the second is + # used as the group of the log files. + # # Any and all errors raised within this method will be propagated # directly back to the caller (usually the +after_fork+ hook. # These errors commonly include ArgumentError for specifying an @@ -134,8 +139,17 @@ def user(user, group = nil, chroot = false) # insufficient because modern systems have fine-grained # capabilities. Let the caller handle any and all errors. uid = Etc.getpwnam(user).uid -gid = Etc.getgrnam(group).gid if group -Unicorn::Util.chown_logs(uid, gid) + +if group + if group.is_a?(Array) +group, log_group = group +log_gid = Etc.getgrnam(log_group).gid + end + gid = Etc.getgrnam(group).gid + log_gid ||= gid +end + +Unicorn::Util.chown_logs(uid, log_gid) if gid && Process.egid != gid Process.initgroups(user, gid) Process::GID.change_privilege(gid) -- 2.17.1 -- unsubscribe: unicorn-public+unsubscr...@bogomips.org archive: https://bogomips.org/unicorn-public/
Support default_middleware configurator method
I was originally looking for a way to turn off default middleware
without having to specify the -N command line option every time.
After reading the Unicorn.builder code, I thought it may be
possible to do this by specifying the option as an embedded option
in the rackup file, via the following line at the top of the file:
#\-N
Unfortunately, while this works for many other command line options,
because of Unicorn's internals, this doesn't work for -N, as
embedded options are not parsed until after the check for skipping
default middleware is made.
This patch makes the embedded -N option work, as well as adds a
default_middleware configuration file option. It does this by
passing the HttpServer instance to the lambda that returns the
rack application. It uses arity checking to do this, like the
previous approach, but support an arity of 2 instead of just an
arity of 0 (which is still supported for backwards compatibility).
An alternative approach would be to parse the embedded options
inside Unicorn.builder, but that results in them getting parsed
twice as well as duplicating some code. Additionally, embedded
options are less understandable than unicorn configuration file
options. If you want a patch for that simpler approach, I have one.
Thanks,
Jeremy
>From 90cca6ff773d58657a4b466a03e8a21b486c913f Mon Sep 17 00:00:00 2001
From: Jeremy Evans
Date: Thu, 13 Sep 2018 10:48:25 -0700
Subject: [PATCH] Support default_middleware configuration option
This allows for the equivalent of the
-N/--no-default_middleware command line option to be
specified in the configuration file. It
also allows the -N/--no-default_middleware embedded
configuration option in the rackup file, which should
have worked previously but did not because embedded
configuration options weren't parsed until after the
app was built.
In order to allow the configuration method to work, have
the lambda that Unicorn.builder returns accept two arguments.
Technically, only one argument is needed for the HttpServer
instance, but I'm guessing if the lambda accepts a single
argument, we expect that to be a rack application instead
of a lambda that returns a rack application.
The command line option (or rackup embedded configuration
option) to disable default middleware will take precedence
over the unicorn configuration file option if both are
present.
For backwards compatibility, if the lambda passed to
HttpServer accepts 0 arguments, then call it without
arguments.
---
lib/unicorn.rb | 8 ++--
lib/unicorn/configurator.rb | 11 +++
lib/unicorn/http_server.rb | 8 +---
3 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/lib/unicorn.rb b/lib/unicorn.rb
index b6dae36..5f2134d 100644
--- a/lib/unicorn.rb
+++ b/lib/unicorn.rb
@@ -45,12 +45,8 @@ def self.builder(ru, op)
abort "rack and Rack::Builder must be available for processing #{ru}"
end
-# Op is going to get cleared before the returned lambda is called, so
-# save this value so that it's still there when we need it:
-no_default_middleware = op[:no_default_middleware]
-
# always called after config file parsing, may be called after forking
-lambda do ||
+lambda do |_, server|
inner_app = case ru
when /\.ru$/
raw = File.read(ru)
@@ -66,7 +62,7 @@ def self.builder(ru, op)
pp({ :inner_app => inner_app })
end
- return inner_app if no_default_middleware
+ return inner_app unless server.default_middleware
middleware = { # order matters
ContentLength: nil,
diff --git a/lib/unicorn/configurator.rb b/lib/unicorn/configurator.rb
index f34d38b..9c36dfe 100644
--- a/lib/unicorn/configurator.rb
+++ b/lib/unicorn/configurator.rb
@@ -265,6 +265,14 @@ def worker_processes(nr)
set_int(:worker_processes, nr, 1)
end
+ # sets whether to add default middleware in the development and
+ # deployment RACK_ENVs.
+ #
+ # default_middleware is only available in unicorn 5.5.0+
+ def default_middleware(bool)
+set_bool(:default_middleware, bool)
+ end
+
# sets listeners to the given +addresses+, replacing or augmenting the
# current set. This is for the global listener pool shared by all
# worker processes. For per-worker listeners, see the after_fork example
@@ -706,6 +714,9 @@ def parse_rackup_file # :nodoc:
/^#\\(.*)/ =~ File.read(ru) or return
RACKUP[:optparse].parse!($1.split(/\s+/))
+if RACKUP[:no_default_middleware]
+ set[:default_middleware] = false
+end
if RACKUP[:daemonize]
# unicorn_rails wants a default pid path, (not plain 'unicorn')
diff --git a/lib/unicorn/http_server.rb b/lib/unicorn/http_server.rb
index b2bbddb..7531886 100644
--- a/lib/unicorn/http_server.rb
+++ b/lib/unicorn/http_server.rb
@@ -14,7 +14,8 @@ class Unicorn::HttpServer
attr_accessor :app, :timeout, :worker_processes,
:before_fork, :after_fork, :before_exec,
:listener_opts, :prelo
