Skip to content
Snippets Groups Projects
Commit a1c2283b authored by Etienne Baqué's avatar Etienne Baqué :red_circle: Committed by Matt Kasa
Browse files

Added ApplicationRateLimit for Namespace update API

This new limit will throttle to async update of an Order object in
CustomersDot, upon Group object update in GitLab

Changelog: changed
parent 041611aa
No related branches found
No related tags found
1 merge request!132053Add ApplicationRateLimit for Group update API
# frozen_string_literal: true
class AddUpdateNamespaceNameToApplicationSettings < Gitlab::Database::Migration[2.2]
milestone '16.6'
def change
add_column :application_settings, :update_namespace_name_rate_limit, :smallint, default: 120, null: false
end
end
72f0dde010df3c7bd9f8e5f44510f9d9eae275d1f6c4c3a72fa5813a2d9f3992
\ No newline at end of file
......@@ -12121,6 +12121,7 @@ CREATE TABLE application_settings (
service_access_tokens_expiration_enforced boolean DEFAULT true NOT NULL,
enable_artifact_external_redirect_warning_page boolean DEFAULT true NOT NULL,
allow_project_creation_for_guest_and_below boolean DEFAULT true NOT NULL,
update_namespace_name_rate_limit smallint DEFAULT 120 NOT NULL,
CONSTRAINT app_settings_container_reg_cleanup_tags_max_list_size_positive CHECK ((container_registry_cleanup_tags_service_max_list_size >= 0)),
CONSTRAINT app_settings_container_registry_pre_import_tags_rate_positive CHECK ((container_registry_pre_import_tags_rate >= (0)::numeric)),
CONSTRAINT app_settings_dep_proxy_ttl_policies_worker_capacity_positive CHECK ((dependency_proxy_ttl_group_policy_worker_capacity >= 0)),
......@@ -591,6 +591,7 @@ def sync_name_with_customers_dot
return unless ::Gitlab.com?
return if user_namespace? && owner.privatized_by_abuse_automation?
return unless root? && (trial? || actual_plan&.paid?)
return if update_to_customerdot_blocked?
::Namespaces::SyncNamespaceNameWorker.perform_async(id)
end
......@@ -673,5 +674,9 @@ def update_within_same_minute?
minute_time_was == minute_time_is
end
def update_to_customerdot_blocked?
::Gitlab::ApplicationRateLimiter.peek(:update_namespace_name, scope: self)
end
end
end
......@@ -500,6 +500,7 @@
let(:owner) { create(:user) }
let(:namespace) { create(:group) }
let(:privatized_by_abuse_automation) { false }
let(:block_namespace_name_update) { false }
subject(:update_namespace) { namespace.update!(attributes) }
......@@ -507,6 +508,10 @@
allow(Gitlab).to receive(:com?).and_return(true)
allow(owner).to receive(:privatized_by_abuse_automation?)
.and_return(privatized_by_abuse_automation)
allow(::Gitlab::ApplicationRateLimiter).to receive(:peek).and_call_original
allow(::Gitlab::ApplicationRateLimiter).to receive(:peek)
.with(:update_namespace_name, scope: namespace)
.and_return(block_namespace_name_update)
end
shared_examples 'no sync' do
......@@ -621,6 +626,12 @@
context 'when the owner is not privatized by abuse automation' do
include_examples 'sync'
context 'when the update to CustomersDot is blocked ay throttle' do
let(:block_namespace_name_update) { true }
include_examples 'no sync'
end
end
end
......
......@@ -254,6 +254,7 @@ def check_subscription!(group)
group = find_group!(params[:id])
group.preload_shared_group_links
mark_throttle! :update_namespace_name, scope: group if params.key?(:name) && params[:name].present?
authorize! :admin_group, group
group.remove_avatar! if params.key?(:avatar) && params[:avatar].nil?
......
......@@ -18,6 +18,10 @@ def check_rate_limit!(key, scope:, **options)
render_api_error!({ error: _('This endpoint has been requested too many times. Try again later.') }, 429)
end
def mark_throttle!(key, scope:)
Gitlab::ApplicationRateLimiter.throttled?(key, scope: scope)
end
end
end
end
......@@ -55,6 +55,7 @@ def rate_limits # rubocop:disable Metrics/AbcSize
phone_verification_send_code: { threshold: 10, interval: 1.hour },
phone_verification_verify_code: { threshold: 10, interval: 10.minutes },
namespace_exists: { threshold: 20, interval: 1.minute },
update_namespace_name: { threshold: -> { application_settings.update_namespace_name_rate_limit }, interval: 1.hour },
fetch_google_ip_list: { threshold: 10, interval: 1.minute },
project_fork_sync: { threshold: 10, interval: 30.minutes },
ai_action: { threshold: 160, interval: 8.hours },
......
......@@ -69,4 +69,12 @@ def render_api_error!(**args); end
end
end
end
describe '#mark_throttle!' do
it 'calls ApplicationRateLimiter#throttle?' do
expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(key, scope: scope).and_return(false)
subject.mark_throttle!(key, scope: scope)
end
end
end
......@@ -871,6 +871,38 @@ def make_upload_request
end
end
before do
stub_application_setting(update_namespace_name_rate_limit: 1)
end
it 'increments the update_namespace_name rate limit' do
put api("/groups/#{group1.id}", user1), params: { name: "#{new_group_name}_1" }
expect(::Gitlab::ApplicationRateLimiter.peek(:update_namespace_name, scope: group1)).to be_falsey
put api("/groups/#{group1.id}", user1), params: { name: "#{new_group_name}_2" }
expect(::Gitlab::ApplicationRateLimiter.peek(:update_namespace_name, scope: group1)).to be_truthy
expect(response).to have_gitlab_http_status(:ok)
expect(group1.reload.name).to eq("#{new_group_name}_2")
end
context 'a name is not passed in' do
it 'does not mark name update throttling' do
expect(::Gitlab::ApplicationRateLimiter).not_to receive(:throttled?)
put api("/groups/#{group1.id}", user1), params: { path: 'another/path' }
end
end
context 'an empty name is passed in' do
it 'does not mark name update throttling' do
expect(::Gitlab::ApplicationRateLimiter).not_to receive(:throttled?)
put api("/groups/#{group1.id}", user1), params: { name: '' }
end
end
context 'when authenticated as the group owner' do
it 'updates the group', :aggregate_failures do
workhorse_form_with_file(
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment