Commit e0cbb1f8 authored by Stan Hu's avatar Stan Hu

Skip Rack Attack rate limiting for container registry event API

As seen in https://gitlab.com/gitlab-org/gitlab/-/issues/327416,
previously Rack Attack throttled
`/api/v4/container_registry_event/events`, which then caused Rack Attack
to rate limit on other unauthenticated requests, such as fetches to
remote CI YAML files.

Since this is a trusted request from the Docker API, we can exempt this
from our rate limiting. We now skip the request if the path starts with
`/api/v4/container_registry_event/`.  We now skip the request if the
path starts with `/api/v4/container_registry_event/`.

Relates to https://gitlab.com/gitlab-org/gitlab/-/issues/327416
parent bff31fff
---
title: Skip Rack Attack rate limiting for container registry event API
merge_request: 59085
author:
type: fixed
...@@ -34,12 +34,16 @@ module Gitlab ...@@ -34,12 +34,16 @@ module Gitlab
path =~ %r{^/-/(health|liveness|readiness|metrics)} path =~ %r{^/-/(health|liveness|readiness|metrics)}
end end
def container_registry_event?
path =~ %r{^/api/v\d+/container_registry_event/}
end
def product_analytics_collector_request? def product_analytics_collector_request?
path.start_with?('/-/collector/i') path.start_with?('/-/collector/i')
end end
def should_be_skipped? def should_be_skipped?
api_internal_request? || health_check_request? api_internal_request? || health_check_request? || container_registry_event?
end end
def web_request? def web_request?
......
...@@ -143,6 +143,31 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac ...@@ -143,6 +143,31 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac
end end
end end
context 'when the request is to a container registry notification endpoint' do
let(:secret_token) { 'secret_token' }
let(:events) { [{ action: 'push' }] }
let(:registry_endpoint) { '/api/v4/container_registry_event/events' }
let(:registry_headers) { { 'Content-Type' => ::API::ContainerRegistryEvent::DOCKER_DISTRIBUTION_EVENTS_V1_JSON } }
before do
allow(Gitlab.config.registry).to receive(:notification_secret) { secret_token }
event = spy(:event)
allow(::ContainerRegistry::Event).to receive(:new).and_return(event)
allow(event).to receive(:supported?).and_return(true)
end
it 'does not throttle the requests' do
(1 + requests_per_period).times do
post registry_endpoint,
params: { events: events }.to_json,
headers: registry_headers.merge('Authorization' => secret_token)
expect(response).to have_gitlab_http_status(:ok)
end
end
end
it 'logs RackAttack info into structured logs' do it 'logs RackAttack info into structured logs' do
requests_per_period.times do requests_per_period.times do
get url_that_does_not_require_authentication get url_that_does_not_require_authentication
......
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