Skip to content

Use ActiveSupport::Cache::RedisCacheStore as session store

Gregorius Marco requested to merge mg-redis-cache-store-as-session-store into master

What does this MR do and why?

This MR Is broken up from !175735, where we'll be migrating Redis Sessions to a Redis Cluster mode in response to CPU saturation warning.

To migrate sessions workload, we need to wrap Gitlab::Redis::Sessions with MultiStore which handles dual-writing to both the old and new stores. This works for callsites within GitLab codebase, mainly from Gitlab::Redis::Sessions.with. However, sessions management from the gem ActionDispatch::Session::RedisStore uses redis-store and has its own function signatures, ie the sets expect different arguments than MultiStore which uses redis-rb gem. We also need to pass in the MultiStore connection pool to the session store so we could override the .with behavior in the gem.

The workaround is to replace the config.session_store with ActionDispatch::Session::CacheStore as suggested in https://github.com/redis/redis-rb/issues/1280#issuecomment-2111327985.

The main difference between both stores lie in the format of the session data. RedisStore stores a marshal of the session hash, whereas CacheStore first wraps the session hash in ActiveSupport::Cache::Entry then being marshalled. This MR also supports CacheStore to read the format of RedisStore session by passing a custom coder in the ActiveSupport::Cache::RedisCacheStore initialization.

Comparison of the session data in Redis:

--- this branch
redis /Users/gregoriusmarco/gdk/redis/redis.socket[5]> get session:gitlab:2::40202a340d5d7b10dc728367de75595c12a85e4e1f05658b4fb98292e21584f6
"\x04\bo: ActiveSupport::Cache::Entry\t:\x0b@value{\bI\"\x19warden.user.user.key\x06:\x06ET[\a[\x06i\x06I\"\"$2a$10$y7/L42WUBppyY8sKToazCu\x06;\aTI\" ask_for_usage_stats_consent\x06;\aFFI\"\x10_csrf_token\x06;\aFI\"0CQs4KCq7TJmUGtYRiXVHTacWnMdoXx-NdHgwiA_vZHM\x06;\aF:\r@version0:\x10@created_atf\x060:\x10@expires_inf\x161736824501.133667"

--- master
redis /Users/gregoriusmarco/gdk/redis/redis.socket[5]> get session:gitlab:2::8a5bda9738eb9eb4a5d3741501e85ecb23bc2892e9a797032d7cd65d5c51d47d
"\x04\b{\bI\"\x19warden.user.user.key\x06:\x06ET[\a[\x06i\x06I\"\"$2a$10$y7/L42WUBppyY8sKToazCu\x06;\x00TI\" ask_for_usage_stats_consent\x06;\x00FFI\"\x10_csrf_token\x06;\x00FI\"0vfgp0tAfNdGAffYAnxy-bi4R6TeSUK7GZLcWvxf-r5Y\x06;\x00F"

Rolling back

In case unexpected things occur once we've rolled forward, the rollback is already supported in !176876 (merged). The Gitlab::Sessions::RedisStore is able to read Entry object introduced in this MR.

Test gap

I'd like to add specs (either request/feature specs) to test old sessions are still working with the new store, and vice versa - new sessions working with the old store. However it's pretty difficult to write one since the stores are initialized in initializers, and it's not possible to rerun the initializer.

Another alternative I've tried is stubbing the session similar to https://gitlab.com/gitlab-org/gitlab/-/blob/0bcde28a5700833dc3d97c577670e93f6f057079/spec/support/helpers/session_helpers.rb#L6-16, then attach the cookie to the request header without signing in, but the request is returning 302.

Click to expand 
# frozen_string_literal: true

require 'spec_helper'

RSpec.describe 'Session store integration test', :clean_gitlab_redis_sessions, type: :request, feature_category: :system_access do
  include SessionHelpers
  let_it_be(:user) { create(:user) }
  let(:cookies) { {} }

  def stub_redis_store_session(session_data:, user_id: nil)
    session_id = Rack::Session::SessionId.new(SecureRandom.hex)

    Gitlab::Redis::Sessions.with do |redis|
      redis.set("session:gitlab:#{session_id.private_id}", Marshal.dump(session_data))
      redis.sadd("session:lookup:user:gitlab:#{user_id}", [session_id.private_id]) if user_id
    end

    defined?(cookies) && cookies[Gitlab::Application.config.session_options[:key]] = session_id.public_id
  end

  before do
    stub_redis_store_session(session_data: { 'warden.user.user.key' => [[user.id], user.authenticatable_salt] }, user_id: user.id)
  end

  context 'when old session from Gitlab::Sessions::RedisStore exists' do
    it 'session still works' do
      key = Gitlab::Application.config.session_options[:key]
      get root_path, headers: { "Cookies": "#{key}=cell-1-#{cookies[key]}" }

      expect(response).not_to be_redirect # fails here, getting a 302 redirect
    end
  end
end

For now, we could only rely on the local validations below.

References

Please include cross links to any resources that are relevant to this MR. This will give reviewers and future readers helpful context to give an efficient review of the changes introduced.

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

Before After

How to set up and validate locally

Browsing around with new sessions

  1. Check out this branch
  2. Restart gdk restart rails-web since the store is initialized on load time.
  3. Sign in, browse around, submit some forms. Everything should work normally.

Simulating rolling forward from existing sessions

  1. Check out master branch

  2. Flush sessions by gdk redis-cli -n 5 flushdb

  3. Restart gdk restart rails-web

  4. Sign in, ensure the session exists in Redis

    redis /Users/gregoriusmarco/gdk/redis/redis.socket[5]> keys *
1) "session:lookup:user:gitlab:1"
2) "session:user:gitlab::v2:1:2::4c2a2bb7385f73a0d13461dcff6ff9b94de9e1224f73e94e296c5faf509d7f2b"
3) "session:gitlab:2::4c2a2bb7385f73a0d13461dcff6ff9b94de9e1224f73e94e296c5faf509d7f2b"
redis /Users/gregoriusmarco/gdk/redis/redis.socket[5]> get session:gitlab:2::4c2a2bb7385f73a0d13461dcff6ff9b94de9e1224f73e94e296c5faf509d7f2b
"\x04\b{\bI\"\x19warden.user.user.key\x06:\x06ET[\a[\x06i\x06I\"\"$2a$10$y7/L42WUBppyY8sKToazCu\x06;\x00TI\" ask_for_usage_stats_consent\x06;\x00FFI\"\x10_csrf_token\x06;\x00FI\"0DcNX9jZzewvW6knMmlVKwS-zmdNX5PYaDsRdjVdMFUo\x06;\x00F"

  5. Check out this branch

  6. Restart gdk restart rails-web

  7. Refresh your browser, you should still be logged in. Browse around, make form submissions, everything should work normally.

  8. Check redis, the session is overwritten with ActiveSupport::Cache::Entry:

     redis /Users/gregoriusmarco/gdk/redis/redis.socket[5]> get session:gitlab:2::d64aa9cf9ea0dc0aad17ab8033fe93ca4711f99fd3934989f700632ae4b841eb
 "\x04\bo: ActiveSupport::Cache::Entry\t:\x0b@value{\bI\"\x19warden.user.user.key\x06:\x06ET[\a[\x06i\x06I\"\"$2a$10$y7/L42WUBppyY8sKToazCu\x06;\aTI\" ask_for_usage_stats_consent\x06;\aFFI\"\x10_csrf_token\x06;\aFI\"0lkdgevlocYAO03xtXrqY-sBQDS70eGUj3crG31LNup8\x06;\aF:\r@version0:\x10@created_atf\x060:\x10@expires_inf\x161736834265.350129"

Rolling back from new sessions to old sessions

This is to test the functionality in !176876 (comment 2283975076)

  1. While still logged in on this branch, confirm the current session is of ActiveSupport::Cache::Entry instance:

     redis /Users/gregoriusmarco/gdk/redis/redis.socket[5]> get session:gitlab:2::d64aa9cf9ea0dc0aad17ab8033fe93ca4711f99fd3934989f700632ae4b841eb
 "\x04\bo: ActiveSupport::Cache::Entry\t:\x0b@value{\bI\"\x19warden.user.user.key\x06:\x06ET[\a[\x06i\x06I\"\"$2a$10$y7/L42WUBppyY8sKToazCu\x06;\aTI\" ask_for_usage_stats_consent\x06;\aFFI\"\x10_csrf_token\x06;\aFI\"0lkdgevlocYAO03xtXrqY-sBQDS70eGUj3crG31LNup8\x06;\aF:\r@version0:\x10@created_atf\x060:\x10@expires_inf\x161736834265.350129"

  2. Check out to master branch, or comment out below line:

    # ENV['USE_REDIS_CACHE_STORE_AS_SESSION_STORE'] = 'true' if Rails.env.test? || Rails.env.development?

  3. Restart rails gdk restart rails-web

  4. Refresh the browser, you should still be logged in.

  5. Redis now contains the old session in hash

    redis /Users/gregoriusmarco/gdk/redis/redis.socket[5]> get session:gitlab:2::d64aa9cf9ea0dc0aad17ab8033fe93ca4711f99fd3934989f700632ae4b841eb
"\x04\b{\bI\"\x19warden.user.user.key\x06:\x06ET[\a[\x06i\x06I\"\"$2a$10$y7/L42WUBppyY8sKToazCu\x06;\x00TI\" ask_for_usage_stats_consent\x06;\x00FFI\"\x10_csrf_token\x06;\x00FI\"0lkdgevlocYAO03xtXrqY-sBQDS70eGUj3crG31LNup8\x06;\x00F"

  6. Or from console:

    [35] pry(main)> Marshal.load(Gitlab::Redis::Sessions.with { |c| c.get("session:gitlab:2::d64aa9cf9ea0dc0aad17ab8033fe93ca4711f99fd3934989f700632ae4b841eb") })
=> {"warden.user.user.key"=>[[1], "$2a$10$y7/L42WUBppyY8sKToazCu"], "ask_for_usage_stats_consent"=>false, "_csrf_token"=>"lkdgevlocYAO03xtXrqY-sBQDS70eGUj3crG31LNup8"}
Edited by Gregorius Marco

Merge request reports