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

Fixes #25293 - Support Puma #774

Draft
wants to merge 4 commits into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions bin/smart-proxy
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@
$LOAD_PATH.unshift(*Dir[File.expand_path('../lib', __dir__), File.expand_path('../modules', __dir__)])

require 'smart_proxy_main'
require 'proxy/launcher'
Proxy::Launcher.new.launch
8 changes: 5 additions & 3 deletions config.ru
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
$LOAD_PATH.unshift *Dir[File.expand_path('lib', __dir__), File.expand_path('modules', __dir__)]
$LOAD_PATH.unshift(*Dir[File.expand_path('lib', __dir__), File.expand_path('modules', __dir__)])

require 'smart_proxy_main'
::Proxy::PluginInitializer.new(::Proxy::Plugins.instance).initialize_plugins
::Proxy::Plugins.instance.select { |p| p[:state] == :running && p[:https_enabled] }.each { |p| instance_eval(p[:class].https_rackup) }
require 'proxy/app'
plugins = ::Proxy::Plugins.instance
::Proxy::PluginInitializer.new(plugins).initialize_plugins
run ::Proxy::App.new(plugins)
29 changes: 29 additions & 0 deletions lib/proxy/app.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
module Proxy
class App
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For naming, perhaps this is more of a Router than an app?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't mind App here. No big deal for me.

def initialize(plugins)
@apps = {}

http_plugins = plugins.select { |p| p[:state] == :running && p[:http_enabled] }
if http_plugins.any?
@apps['http'] = Rack::Builder.new do
http_plugins.each { |p| instance_eval(p[:class].http_rackup) }
end
end

https_plugins = plugins.select { |p| p[:state] == :running && p[:https_enabled] }
if https_plugins.any?
@apps['https'] = Rack::Builder.new do
https_plugins.each { |p| instance_eval(p[:class].https_rackup) }
end
end
end

def call(env)
# TODO: Respect X-Forwarded-Proto?
scheme = env['rack.url_scheme']
app = @apps[scheme]
fail "Unsupported URL scheme #{scheme}" unless app
app.call(env)
end
end
end
106 changes: 106 additions & 0 deletions lib/proxy/launcher.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
require 'proxy/log'
require 'proxy/settings'
require 'proxy/signal_handler'
require 'proxy/log_buffer/trace_decorator'

CIPHERS = ['ECDHE-RSA-AES128-GCM-SHA256', 'ECDHE-RSA-AES256-GCM-SHA384',
'AES128-GCM-SHA256', 'AES256-GCM-SHA384', 'AES128-SHA256',
'AES256-SHA256', 'AES128-SHA', 'AES256-SHA'].freeze

module Proxy
class Launcher
include ::Proxy::Log

attr_reader :settings

def initialize(settings = ::Proxy::SETTINGS)
@settings = settings
end

def pid_path
settings.daemon_pid
end

def http_enabled?
!settings.http_port.nil?
end

def https_enabled?
settings.ssl_private_key && settings.ssl_certificate && settings.ssl_ca_file
end

def plugins
::Proxy::Plugins.instance
end

def pid_status
return :exited unless File.exist?(pid_path)
pid = ::File.read(pid_path).to_i
return :dead if pid == 0
Process.kill(0, pid)
:running
rescue Errno::ESRCH
:dead
rescue Errno::EPERM
:not_owned
end

def check_pid
case pid_status
when :running, :not_owned
logger.error "A server is already running. Check #{pid_path}"
exit(2)
when :dead
File.delete(pid_path)
end
end

def write_pid
FileUtils.mkdir_p(File.dirname(pid_path)) unless File.exist?(pid_path)
File.open(pid_path, ::File::CREAT | ::File::EXCL | ::File::WRONLY) { |f| f.write(Process.pid.to_s) }
at_exit { File.delete(pid_path) if File.exist?(pid_path) }
rescue Errno::EEXIST
check_pid
retry
end

def ciphers
CIPHERS - settings.ssl_disabled_ciphers
end

def launch
raise Exception.new("Both http and https are disabled, unable to start.") unless http_enabled? || https_enabled?

if settings.daemon
check_pid
Process.daemon
write_pid
end

::Proxy::PluginInitializer.new(plugins).initialize_plugins

case settings.http_server_type
when 'webrick'
require 'proxy/launcher/webrick'
launcher = ::Launcher::Webrick.new(self)
when 'puma'
require 'proxy/launcher/puma'
launcher = ::Launcher::Puma.new(self)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed? Aren't we actually switching from using our own startup binary to calling "puma" directly?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a bit of my confusion as well trying to understand the layers and I think @lzap worded the question better than I was going to. I attempted to read up on the Rack pattern and it's uses in places and apply it to here. I'm going to attempt to write out what I think I understand (and am probably wrong about in some way):

Puma  -> Rack Middleware -> Smart Proxy Plugins

Where Puma serves as the application server handling spawning processes and threads to handle incoming requests.
Rack middleware (Proxy::App) handles mounting each Smart Proxy plugin as a Rack app as an http, https or both endpoint.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed? Aren't we actually switching from using our own startup binary to calling "puma" directly?

Both are valid. The Puma launcher keeps everything compatible from a packaging perspective: no need to change all config files and service files.

Puma -> Rack Middleware -> Smart Proxy Plugins

This is slightly incorrect. More correct would be:

Application server -> Rack application

Passenger would also be a valid application server since there is a config.ru file. Any Rack application can also apply middleware (we don't do this today).

If you note, previously it also built a Rack application here:

app = Rack::Builder.new do
plugins.each { |p| instance_eval(p.http_rackup) }
end

And the HTTPS one here:

app = Rack::Builder.new do
plugins.each { |p| instance_eval(p.https_rackup) }
end

So the solution I chose was to write a third Rack application that embeds boths those and switches it based on the received protocol.

Note that a Rack application is defined as a class that has a call(env) method. Env is a simple Ruby hash. To get a request, you can call Rack::Request.new(env). It's surprisingly simple.

https://github.com/rack/rack/blob/master/lib/rack/lobster.rb is the example they ship.

else
fail "Unknown http_server_type: #{settings.http_server_type}"
end

launcher.launch
rescue SignalException => e
logger.debug("Caught #{e}. Exiting")
raise
rescue SystemExit
# do nothing. This is to prevent the exception handler below from catching SystemExit exceptions.
raise
rescue Exception => e
logger.error "Error during startup, terminating", e
puts "Errors detected on startup, see log for details. Exiting: #{e}"
exit(1)
end
end
end
87 changes: 87 additions & 0 deletions lib/proxy/launcher/puma.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
require 'puma'
require 'puma/configuration'
require 'proxy/app'

module Launcher
class Puma
attr_reader :launcher

def initialize(launcher)
@launcher = launcher
end

def launch
::Puma::Launcher.new(conf).run
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Offtopic: I noticed that the spawned thread in DHCP plugin does not have any shutdown implementation. It provides stop method however it's never called except tests (?), I tested this using pry (DHCP plugin must be enabled and working). This can be probably workarounded using at_exit block with calling stop and waiting until the thread joins.

Anyway, this is not relevant, I was just investigating how shutdown works in puma - it performs graceful shutdown of the Puma's resources and that's about it. Although I had impression we do some thread joining for webrick case, we only do this for server threads (not background plugin DHCP thread).

end

private

def conf
::Puma::Configuration.new do |user_config|
user_config.environment('production')
user_config.app(app)

if launcher.http_enabled?
bind_hosts do |host|
user_config.bind "tcp://#{host}:#{settings.http_port}"
end
end

if launcher.https_enabled?
ssl_options = {
key: settings.ssl_private_key,
cert: settings.ssl_certificate,
ca: settings.ssl_ca_file,
ssl_cipher_filter: launcher.ciphers.join(':'),
verify_mode: 'peer',
no_tlsv1: true,
no_tlsv1_1: true,
}

bind_hosts do |host|
user_config.ssl_bind(host, settings.https_port, ssl_options)
end
end

user_config.on_restart do
::Proxy::LogBuffer::Decorator.instance.roll_log = true
end

begin
user_config.plugin('systemd')
rescue ::Puma::UnknownPlugin
end
end
end

def app
::Proxy::App.new(launcher.plugins)
end

def binds
end

def bind_hosts
settings.bind_host.each do |host|
if host == '*'
yield ipv6_enabled? ? '[::]' : '0.0.0.0'
else
begin
addr = IPAddr.new(host)
yield addr.ipv6? ? "[#{addr}]" : addr.to_s
rescue IPAddr::InvalidAddressError
yield host
end
end
end
end

def settings
launcher.settings
end

def ipv6_enabled?
File.exist?('/proc/net/if_inet6') || (RUBY_PLATFORM =~ /cygwin|mswin|mingw|bccwin|wince|emx/)
end
end
end
Loading