Ignore FactoryBot #create for cross-database modification detection

This was causing too many false positives which are not representative
of code that is actually running in production. It is very common in
tests to build a tree of related objects in FactoryBot and save them all
in one go with `save!` (this is the default behaviour in FactoryBot for
association building). When this is done it creates an outer transaction
and saves all the models within the context of this transaction. If 2 of
the models belong in a different `gitlab_schema` (database) then this
triggers our cross-database modification detection logic. While there is
a chance that this could be indicative of problems in our model code
(eg. `before_save` and so on) it is more often than not just a problem
with how our factories are building a relation tree.

A simple example is included as an RSpec test here where we create a
`ci_runner` and `project` (which in turn creates a `user`) all in one
go. This is convenient in tests but it is never going to happen in real
production code as there is no way to create a project at the same time
as creating a runner (let alone user). You would always have a project
and a user and then later create the runner for that project.

Our PreventCrossDatabaseModification detection logic is designed to find
places in our code where we are writing to 2 different databases in the
context of a transaction of one of those databases. This was introduced
because we are decomposing our database into separate databases.
Specifically we are moving all CI tables to a separate CI database. It's
important that we find and fix any places in our code where we are
opening a transaction and expecting all transaction semantics (ie.
rollbacks) to apply to all database queries within that context. When
you have 2 databases there may be places where we are only rolling back
some of the queries in that transaction which could lead to data
inconsistency bugs. As such we need to detect and restructure such
transactions so that each case is properly handled.

spec/support/database/prevent_cross_database_modification.rb

@@ -61,6 +61,7 @@ module Database
       return unless cross_database_context[:enabled]
 
       return if connection.pool.instance_of?(ActiveRecord::ConnectionAdapters::NullPool)
+      return if in_factory_bot_create?
 
       database = connection.pool.db_config.name
 
@@ -113,6 +114,15 @@ module Database
         raise Database::PreventCrossDatabaseModification::CrossDatabaseModificationAcrossUnsupportedTablesError, message
       end
     end
+
+    # We ignore execution in the #create method from FactoryBot
+    # because it is not representative of real code we run in
+    # production. There are far too many false positives caused
+    # by instantiating objects in different `gitlab_schema` in a
+    # FactoryBot `create`.
+    def self.in_factory_bot_create?
+      caller_locations.any? { |l| l.path.end_with?('lib/factory_bot/evaluation.rb') && l.label == 'create' }
+    end
   end
 end
 

spec/support_specs/database/prevent_cross_database_modification_spec.rb

@@ -127,6 +127,14 @@ RSpec.describe 'Database::PreventCrossDatabaseModification' do
               expect { run_queries }.to raise_error /Cross-database data modification/
             end
           end
+
+          context 'when the modification is inside a factory save! call' do
+            let(:runner) { create(:ci_runner, :project, projects: [build(:project)]) }
+
+            it 'does not raise an error' do
+              runner
+            end
+          end
         end
       end
     end
@@ -152,14 +160,6 @@ RSpec.describe 'Database::PreventCrossDatabaseModification' do
         end.not_to raise_error
       end
 
-      it 'raises error when complex factories are built referencing both databases' do
-        expect do
-          ApplicationRecord.transaction do
-            create(:ci_pipeline)
-          end
-        end.to raise_error /Cross-database data modification/
-      end
-
       it 'skips raising error on factory creation' do
         expect do
           Gitlab::Database.allow_cross_database_modification_within_transaction(url: 'gitlab-issue') do