Commit 1c3b893f authored by Sean McGivern's avatar Sean McGivern

Use rate limiting Redis for Rack::Attack

To control this, we use an environment variable. This is because
Rack::Attack is configured in an initialiser, so a feature flag is of
minimal value - it would still need a restart to take effect.

We could rewrite the InstrumentedCacheStore to allow changing the store
on a per-operation basis, but that adds more risk for what should be a
quick migration. It would also add a feature flag check in a very hot
code path (rate limiting checks happen multiple times on every request).
parent 1ded4b19
...@@ -2,9 +2,10 @@ ...@@ -2,9 +2,10 @@
module Gitlab module Gitlab
module RackAttack module RackAttack
# This class is a proxy for all Redis calls made by RackAttack. All the # This class is a proxy for all Redis calls made by RackAttack. All
# calls are instrumented, then redirected to ::Rails.cache. This class # the calls are instrumented, then redirected to the underlying
# instruments the standard interfaces of ActiveRecord::Cache defined in # store (in `.store). This class instruments the standard interfaces
# of ActiveRecord::Cache defined in
# https://github.com/rails/rails/blob/v6.0.3.1/activesupport/lib/active_support/cache.rb#L315 # https://github.com/rails/rails/blob/v6.0.3.1/activesupport/lib/active_support/cache.rb#L315
# #
# For more information, please see # For more information, please see
...@@ -14,7 +15,18 @@ module Gitlab ...@@ -14,7 +15,18 @@ module Gitlab
delegate :silence!, :mute, to: :@upstream_store delegate :silence!, :mute, to: :@upstream_store
def initialize(upstream_store: ::Rails.cache, notifier: ActiveSupport::Notifications) # Clean up in https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/1249
def self.store
if ENV['USE_RATE_LIMITING_STORE_FOR_RACK_ATTACK'] == '1'
Gitlab::AuthLogger.info(message: 'Rack::Attack using rate limiting store')
::Gitlab::Redis::RateLimiting.cache_store
else
Gitlab::AuthLogger.info(message: 'Rack::Attack using cache store')
::Rails.cache
end
end
def initialize(upstream_store: self.class.store, notifier: ActiveSupport::Notifications)
@upstream_store = upstream_store @upstream_store = upstream_store
@notifier = notifier @notifier = notifier
end end
......
...@@ -7,6 +7,10 @@ module Gitlab ...@@ -7,6 +7,10 @@ module Gitlab
def self.config_fallback def self.config_fallback
Cache Cache
end end
def self.cache_store
@cache_store ||= ActiveSupport::Cache::RedisCacheStore.new(redis: pool, namespace: Cache::CACHE_NAMESPACE)
end
end end
end end
end end
...@@ -86,4 +86,17 @@ RSpec.describe Gitlab::RackAttack::InstrumentedCacheStore do ...@@ -86,4 +86,17 @@ RSpec.describe Gitlab::RackAttack::InstrumentedCacheStore do
test_proc.call(subject) test_proc.call(subject)
end end
end end
# Remove in https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/1249
describe '.store' do
it 'uses the rate limiting store when USE_RATE_LIMITING_STORE_FOR_RACK_ATTACK is set' do
stub_env('USE_RATE_LIMITING_STORE_FOR_RACK_ATTACK', '1')
expect(described_class.store).to eq(Gitlab::Redis::RateLimiting.cache_store)
end
it 'uses the cache store' do
expect(described_class.store).to eq(Rails.cache)
end
end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment