Problems with using standandalone pubsubstub with rails 6 redis session store

[ohsabry]

We recently upgraded an instance of shipit-engine to 0.31 (and now running shipit-engine master)

When running pubsubstub in a separate app, we don't want to leave the /events endpoint exposed, so our goal is to return 403s and short circuit the pubsubstub server with a middleware if we get a request that doesn't map to the session that was created from the rails app.

Unfortunately, we are unable to use the same session cookie created from the rails server in the pubsubstub server

we ran into two problems so far:

1. Doing request.session[:init] = true in the pubsubstub app was blowing up with an error. I did some digging around and found that this was an issue related to an incompatibility between redis-rb 4.2 and redis-store. This was fixed by @casperisfine very recently here: https://github.com/redis-store/redis-store/pull/334. We are running this patch gem 'redis-store', github: 'redis-store/redis-store' to get around this. I think this is needed because the session is uninitialized in rack without it because of some lazy loading.

2. It seems like rack/session/redis is creating a new session with nothing in it as opposed to reading rails session. session[:user_id] is not defined when I try to print it from the pubsubstub application after the Rack::Session::Redismiddleware executed.

@casperisfine i hate to bug you again but you know probably redis-store, pubsubstub and shipit better than anyone - can you please share how you would implement UserRequiredMiddleware today if we are storing sessions from the rails side using the redis_cache_store as the `session_store?

We run pubsubstub in its own process/container like this:

bundle exec puma --config pubsubstub/puma_config.rb pubsubstub/config.ru --port 3000 environment $RAILS_ENV

pubsubstub/puma_config.rb:

#!/usr/bin/env puma

# Tells puma to not wait on clients to sutdown
force_shutdown_after 0

# Number of processes
workers 2

port ENV.fetch("PORT") { 3000 }

# min, max threads per workers
threads 32, 256

This is how we are setting up the session store for the rails application:

# config/initializers/session_store.rb
Shipit::Application.config.session_store :cache_store,
    servers: ["#{ENV['REDIS_URL']}/0/session"],
    key: '_shipit_session',
    expires_in: 60.minutes,
    threadsafe: false,
    signed: true,
    secure: true,
    http_only: true

How the cache store is set up:

# config/environments.production.rb
config.cache_store = :redis_cache_store, { url: Shipit.redis_url.to_s, expires_in: 90.minutes }

pubsubstub/config.ru

require 'rack'
require 'pubsubstub'
require 'rack/session/redis'

class HealthCheck
  def initialize(app)
    @app = app
  end

  def call(env)
    if env['PATH_INFO'] == '/events/healthcheck'
      return [200, {'Content-Type' => 'text/plain'}, %w(OK)]
    end

    return @app.call(env) 
end

class UserRequiredMiddleware
  def initialize(app)
    @app = app
  end

  def call(env)
    request = Rack::Request.new(env)
    # this was throwing before the redis-store patch
    request.session[:init] = true 

    # this condition is never true 
    if request.session[:user_id]
      return @app.call(env) 
    end

    [403, {"Content-Type" => "text/html"}, ["Log into Shipit first"]]
  rescue
    [500, {"Content-Type" => "text/html"}, ["Server Error"]]
  end
end

Pubsubstub.redis_url = ENV['REDIS_URL']

use HealthCheck
use Rack::Session::Redis, {redis_server: "#{ENV['REDIS_URL']}/0/session", key: "_shipit_session"}
use UserRequiredMiddleware
run Pubsubstub::StreamAction.new

[ohsabry]

@casperisfine does Shopify still use pubsubstub in a separate process, or are you using the polling mode from the rails application? I see that Pubsubstub.use_persistent_connections = false is now the default in the shipit-engine template.rb.

(Unfortunately, I couldn't get polling mode to work on latest shipit either: https://github.com/Shopify/shipit-engine/issues/1082)

If running it separately is still the recommended approach - I would be interested in submitting a PR with a conventional implementation of pubsubstub/config.ru and some doc updates if you'd take it. Gotta figure out how to implement it well first of course 😁

[casperisfine]

does Shopify still use pubsubstub in a separate process

Yes. We haven't changed it one bit.

I see that Pubsubstub.use_persistent_connections = false is now the default in the shipit-engine template.rb.

This was done so that it works out of the box. And if you don't have many users it's actually just fine.

If running it separately is still the recommended approach

It is if you have a sizeable amount of users, or would like to have a slightly more reactive UI.

can you please share how you would implement UserRequiredMiddleware today if we are storing sessions from the rails side using the redis_cache_store as the `session_store?

I'd still do it the same way. Except we moved to memcache for sessions:

# config.ru
use ActionDispatch::Session::MemCacheStore, {
  memcache_server: Shipit::Config.sessions_url,
  key: Shipit::Config::SESSION_KEY,
  pool_size: 5,
}

use UserRequiredMiddleware
run Pubsubstub::StreamAction.new

class UserRequiredMiddleware
  def initialize(app)
    @app = app
  end

  def call(env)
    env['rack.session'][:init] = true
    if env['rack.session'][:authenticated] || ENV.key?('SHIPIT_DISABLE_AUTH')
      @app.call(env)
    else
      [403, {"Content-Type" => "text/html"}, ["Log into Shipit first"]]
    end
  end
end

Then from our auth controller:

session[:authenticated] = true

We also use the same midleware to protect /sidekiq:

# routes.rb
mount UserRequiredMiddleware.new(Sidekiq::Web.new), at: '/sidekiq'

It seems like rack/session/redis is creating a new session with nothing in it as opposed to reading rails session.

I can see a few possible reasons:

• Make sure they use the same cookie name
• Make sure they look for the same key in the same redis db.

I would be interested in submitting a PR with a conventional implementation of pubsubstub/config.ru and some doc updates if you'd take it.

I am, but the goal is to make the steup as simple and as little error prone as possible. So I'd probably leave the integrated polling method as the default. But I'd definitely be open for a nice doc on when and how to setup the external stream server.