Commit 5cf0ce43 authored by Lin Jen-Shin's avatar Lin Jen-Shin

Merge branch 'sh-skip-findcommit-lookup-rate-limit' into 'master'

Avoid Gitaly RPCs in rate-limited raw blob requests

Closes gitlab-com/gl-infra/scalability#77

See merge request gitlab-org/gitlab!22123
parents 888aa6fd 5200d89d
...@@ -9,9 +9,9 @@ class Projects::RawController < Projects::ApplicationController ...@@ -9,9 +9,9 @@ class Projects::RawController < Projects::ApplicationController
prepend_before_action(only: [:show]) { authenticate_sessionless_user!(:blob) } prepend_before_action(only: [:show]) { authenticate_sessionless_user!(:blob) }
before_action :require_non_empty_project before_action :require_non_empty_project
before_action :assign_ref_vars
before_action :authorize_download_code! before_action :authorize_download_code!
before_action :show_rate_limit, only: [:show], unless: :external_storage_request? before_action :show_rate_limit, only: [:show], unless: :external_storage_request?
before_action :assign_ref_vars
before_action :redirect_to_external_storage, only: :show, if: :static_objects_external_storage_enabled? before_action :redirect_to_external_storage, only: :show, if: :static_objects_external_storage_enabled?
def show def show
...@@ -23,11 +23,15 @@ class Projects::RawController < Projects::ApplicationController ...@@ -23,11 +23,15 @@ class Projects::RawController < Projects::ApplicationController
private private
def show_rate_limit def show_rate_limit
if rate_limiter.throttled?(:show_raw_controller, scope: [@project, @commit, @path], threshold: raw_blob_request_limit) # This bypasses assign_ref_vars to avoid a Gitaly FindCommit lookup.
# When rate limiting, we really don't care if a different commit is
# being requested.
_ref, path = extract_ref(get_id)
if rate_limiter.throttled?(:show_raw_controller, scope: [@project, path], threshold: raw_blob_request_limit)
rate_limiter.log_request(request, :raw_blob_request_limit, current_user) rate_limiter.log_request(request, :raw_blob_request_limit, current_user)
flash[:alert] = _('You cannot access the raw file. Please wait a minute.') render plain: _('You cannot access the raw file. Please wait a minute.'), status: :too_many_requests
redirect_to project_blob_path(@project, File.join(@ref, @path)), status: :too_many_requests
end end
end end
......
---
title: Avoid Gitaly RPCs in rate-limited raw blob requests
merge_request: 22123
author:
type: performance
...@@ -56,10 +56,13 @@ describe Projects::RawController do ...@@ -56,10 +56,13 @@ describe Projects::RawController do
stub_application_setting(raw_blob_request_limit: 5) stub_application_setting(raw_blob_request_limit: 5)
end end
it 'prevents from accessing the raw file' do it 'prevents from accessing the raw file', :request_store do
execute_raw_requests(requests: 6, project: project, file_path: file_path) execute_raw_requests(requests: 5, project: project, file_path: file_path)
expect { execute_raw_requests(requests: 1, project: project, file_path: file_path) }
.to change { Gitlab::GitalyClient.get_request_count }.by(0)
expect(flash[:alert]).to eq(_('You cannot access the raw file. Please wait a minute.')) expect(response.body).to eq(_('You cannot access the raw file. Please wait a minute.'))
expect(response).to have_gitlab_http_status(429) expect(response).to have_gitlab_http_status(429)
end end
...@@ -109,7 +112,7 @@ describe Projects::RawController do ...@@ -109,7 +112,7 @@ describe Projects::RawController do
execute_raw_requests(requests: 3, project: project, file_path: modified_path) execute_raw_requests(requests: 3, project: project, file_path: modified_path)
expect(flash[:alert]).to eq(_('You cannot access the raw file. Please wait a minute.')) expect(response.body).to eq(_('You cannot access the raw file. Please wait a minute.'))
expect(response).to have_gitlab_http_status(429) expect(response).to have_gitlab_http_status(429)
end end
end end
...@@ -137,7 +140,7 @@ describe Projects::RawController do ...@@ -137,7 +140,7 @@ describe Projects::RawController do
# Accessing downcase version of readme # Accessing downcase version of readme
execute_raw_requests(requests: 6, project: project, file_path: file_path) execute_raw_requests(requests: 6, project: project, file_path: file_path)
expect(flash[:alert]).to eq(_('You cannot access the raw file. Please wait a minute.')) expect(response.body).to eq(_('You cannot access the raw file. Please wait a minute.'))
expect(response).to have_gitlab_http_status(429) expect(response).to have_gitlab_http_status(429)
# Accessing upcase version of readme # Accessing upcase version of readme
......
...@@ -31,8 +31,6 @@ describe 'Projects > Raw > User interacts with raw endpoint' do ...@@ -31,8 +31,6 @@ describe 'Projects > Raw > User interacts with raw endpoint' do
visit project_raw_url(project, file_path) visit project_raw_url(project, file_path)
end end
expect(source).to have_content('You are being redirected')
click_link('redirected')
expect(page).to have_content('You cannot access the raw file. Please wait a minute.') expect(page).to have_content('You cannot access the raw file. Please wait a minute.')
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