Merge branch 'allow-cross-joins' into 'master'

Prevent cross-joins across databases

See merge request gitlab-org/gitlab!67019

lib/gitlab/database.rb

@@ -142,6 +142,11 @@ module Gitlab
       MAX_TIMESTAMP_VALUE > timestamp ? timestamp : MAX_TIMESTAMP_VALUE.dup
     end
 
+    def self.allow_cross_joins_across_databases(url:)
+      # this method is implemented in:
+      # spec/support/database/prevent_cross_joins.rb
+    end
+
     def self.add_post_migrate_path_to_rails(force: false)
       return if ENV['SKIP_POST_DEPLOYMENT_MIGRATIONS'] && !force
 

spec/support/database/prevent_cross_joins.rb

+# frozen_string_literal: true
+
+# This module tries to discover and prevent cross-joins across tables
+# This will forbid usage of tables between CI and main database
+# on a same query unless explicitly allowed by. This will change execution
+# from a given point to allow cross-joins. The state will be cleared
+# on a next test run.
+#
+# This method should be used to mark METHOD introducing cross-join
+# not a test using the cross-join.
+#
+# class User
+#   def ci_owned_runners
+#     ::Gitlab::Database.allow_cross_joins_across_databases!(url: link-to-issue-url)
+#
+#     ...
+#   end
+# end
+
+module Database
+  module PreventCrossJoins
+    CrossJoinAcrossUnsupportedTablesError = Class.new(StandardError)
+
+    def self.validate_cross_joins!(sql)
+      return if Thread.current[:allow_cross_joins_across_databases]
+
+      # PgQuery might fail in some cases due to limited nesting:
+      # https://github.com/pganalyze/pg_query/issues/209
+      tables = PgQuery.parse(sql).tables
+
+      unless only_ci_or_only_main?(tables)
+        raise CrossJoinAcrossUnsupportedTablesError,
+          "Unsupported cross-join across '#{tables.join(", ")}' discovered " \
+          "when executing query '#{sql}'"
+      end
+    end
+
+    # Returns true if a set includes only CI tables, or includes only non-CI tables
+    def self.only_ci_or_only_main?(tables)
+      tables.all? { |table| ci_table_name?(table) } ||
+        tables.none? { |table| ci_table_name?(table) }
+    end
+
+    def self.ci_table_name?(name)
+      ci_tables.include?(name)
+    end
+
+    def self.ci_tables
+      @@ci_tables ||= Set.new.tap do |tables| # rubocop:disable Style/ClassVars
+        tables.merge(Ci::ApplicationRecord.descendants.map(&:table_name).compact)
+
+        # It was decided that taggings/tags are best placed with CI
+        # https://gitlab.com/gitlab-org/gitlab/-/issues/333413
+        tables.add('taggings')
+        tables.add('tags')
+      end
+    end
+
+    module GitlabDatabaseMixin
+      def allow_cross_joins_across_databases(url:)
+        Thread.current[:allow_cross_joins_across_databases] = true
+        super
+      end
+    end
+  end
+end
+
+Gitlab::Database.singleton_class.prepend(
+  Database::PreventCrossJoins::GitlabDatabaseMixin)
+
+RSpec.configure do |config|
+  # TODO: remove `:prevent_cross_joins` to enable the check by default
+  config.around(:each, :prevent_cross_joins) do |example|
+    subscriber = ActiveSupport::Notifications.subscribe('sql.active_record') do |event|
+      ::Database::PreventCrossJoins.validate_cross_joins!(event.payload[:sql])
+    end
+
+    Thread.current[:allow_cross_joins_across_databases] = false
+
+    example.run
+  ensure
+    ActiveSupport::Notifications.unsubscribe(subscriber) if subscriber
+  end
+end

spec/support_specs/database/prevent_cross_joins_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Database::PreventCrossJoins do
+  context 'when running in :prevent_cross_joins scope', :prevent_cross_joins do
+    context 'when only non-CI tables are used' do
+      it 'does not raise exception' do
+        expect { main_only_query }.not_to raise_error
+      end
+    end
+
+    context 'when only CI tables are used' do
+      it 'does not raise exception' do
+        expect { ci_only_query }.not_to raise_error
+      end
+    end
+
+    context 'when CI and non-CI tables are used' do
+      it 'raises exception' do
+        expect { main_and_ci_query }.to raise_error(
+          described_class::CrossJoinAcrossUnsupportedTablesError)
+      end
+
+      context 'when allow_cross_joins_across_databases is used' do
+        it 'does not raise exception' do
+          Gitlab::Database.allow_cross_joins_across_databases(url: 'http://issue-url')
+
+          expect { main_and_ci_query }.not_to raise_error
+        end
+      end
+    end
+  end
+
+  context 'when running in a default scope' do
+    context 'when CI and non-CI tables are used' do
+      it 'does not raise exception' do
+        expect { main_and_ci_query }.not_to raise_error
+      end
+    end
+  end
+
+  private
+
+  def main_only_query
+    Issue.joins(:project).last
+  end
+
+  def ci_only_query
+    Ci::Build.joins(:pipeline).last
+  end
+
+  def main_and_ci_query
+    Ci::Build.joins(:project).last
+  end
+end