Commit 5ec78473 authored by Kamil Trzciński's avatar Kamil Trzciński

Allowlist the `destroy_service.rb` of `users` and `pipelines`

Document 2PC violation related to destroy of user
trying to nullify all ci_builds and pipelines.
parent 266a13d8
......@@ -12,7 +12,9 @@ module Ci
# Ci::Pipeline#destroy triggers `use_fast_destroy :job_artifacts` and
# ci_builds has ON DELETE CASCADE to ci_pipelines. The pipeline, the builds,
# job and pipeline artifacts all get destroyed here.
pipeline.reset.destroy!
::Gitlab::Database::QueryAnalyzers::PreventCrossDatabaseModification.allow_cross_database_modification_within_transaction(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/345849') do
pipeline.reset.destroy!
end
ServiceResponse.success(message: 'Pipeline not found')
rescue ActiveRecord::RecordNotFound
......
......@@ -65,7 +65,10 @@ module Users
user.destroy_dependent_associations_in_batches(exclude: [:snippets])
# Destroy the namespace after destroying the user since certain methods may depend on the namespace existing
user_data = user.destroy
user_data = nil
::Gitlab::Database::QueryAnalyzers::PreventCrossDatabaseModification.allow_cross_database_modification_within_transaction(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/345843') do
user_data = user.destroy
end
namespace.destroy
user_data
......
......@@ -91,9 +91,6 @@ module Gitlab
raise CrossDatabaseModificationAcrossUnsupportedTablesError, message
end
rescue CrossDatabaseModificationAcrossUnsupportedTablesError => e
::Gitlab::ErrorTracking.track_exception(e, { gitlab_schemas: schemas, tables: all_tables, query: parsed.sql })
raise if raise_exception?
end
# rubocop:enable Metrics/AbcSize
......@@ -105,11 +102,6 @@ module Gitlab
def self.in_factory_bot_create?
Rails.env.test? && caller_locations.any? { |l| l.path.end_with?('lib/factory_bot/evaluation.rb') && l.label == 'create' }
end
# When in test we raise exception
def self.raise_exception?
Rails.env.test?
end
end
end
end
......
......@@ -21,9 +21,8 @@ RSpec.describe Gitlab::Middleware::QueryAnalyzer, query_analyzers: false do
end
end
it 'detects cross modifications and logs them without raising exception' do
allow(Rails.env).to receive(:test?).and_return(false)
expect(::Gitlab::ErrorTracking).to receive(:track_exception)
it 'detects cross modifications and tracks exception' do
expect(::Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception)
expect { subject }.not_to raise_error
end
......@@ -34,7 +33,7 @@ RSpec.describe Gitlab::Middleware::QueryAnalyzer, query_analyzers: false do
end
it 'does not detect cross modifications' do
expect(::Gitlab::ErrorTracking).not_to receive(:track_exception)
expect(::Gitlab::ErrorTracking).not_to receive(:track_and_raise_for_dev_exception)
subject
end
......@@ -52,7 +51,7 @@ RSpec.describe Gitlab::Middleware::QueryAnalyzer, query_analyzers: false do
end
it 'does not log anything' do
expect(::Gitlab::ErrorTracking).not_to receive(:track_exception)
expect(::Gitlab::ErrorTracking).not_to receive(:track_and_raise_for_dev_exception)
subject
end
......
......@@ -23,11 +23,10 @@ RSpec.describe Gitlab::SidekiqMiddleware::QueryAnalyzer, query_analyzers: false
end
end
it 'detects cross modifications and logs them without raising exception' do
allow(Rails.env).to receive(:test?).and_return(false)
expect(::Gitlab::ErrorTracking).to receive(:track_exception)
it 'detects cross modifications and tracks exception' do
expect(::Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception)
expect { subject }.not_to raise_error
subject
end
context 'when the detect_cross_database_modification is disabled' do
......@@ -36,7 +35,7 @@ RSpec.describe Gitlab::SidekiqMiddleware::QueryAnalyzer, query_analyzers: false
end
it 'does not detect cross modifications' do
expect(::Gitlab::ErrorTracking).not_to receive(:track_exception)
expect(::Gitlab::ErrorTracking).not_to receive(:track_and_raise_for_dev_exception)
subject
end
......@@ -52,7 +51,7 @@ RSpec.describe Gitlab::SidekiqMiddleware::QueryAnalyzer, query_analyzers: false
end
it 'does not log anything' do
expect(::Gitlab::ErrorTracking).not_to receive(:track_exception)
expect(::Gitlab::ErrorTracking).not_to receive(:track_and_raise_for_dev_exception)
subject
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