Commit 40f9110d authored by Dylan Griffith's avatar Dylan Griffith Committed by Thong Kuah

Prevent cross joins for all while allowing existing failures

This uses a filename based approach to allowlist spec files

Add documentation about cross-database joins detection
parent 5e3d4ef8
- ./ee/spec/lib/analytics/devops_adoption/snapshot_calculator_spec.rb
......@@ -109,6 +109,10 @@ already many such examples that need to be fixed in
The following are some real examples that have resulted from this and these
patterns may apply to future cases.
[Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68620) in GitLab 14.3, any
queries detected that join across databases raises an error (except
for pre-existing queries).
#### Remove the code
The simplest solution we've seen several times now has been an existing scope
......
......@@ -33,7 +33,8 @@ module Database
tables = PgQuery.parse(sql).tables
schemas = Database::GitlabSchema.table_schemas(tables)
if schemas.many?
if schemas.include?(:gitlab_ci) && schemas.include?(:gitlab_main)
Thread.current[:has_cross_join_exception] = true
raise CrossJoinAcrossUnsupportedTablesError,
"Unsupported cross-join across '#{tables.join(", ")}' modifying '#{schemas.to_a.join(", ")}' discovered " \
"when executing query '#{sql}'"
......@@ -66,11 +67,18 @@ end
Gitlab::Database.singleton_class.prepend(
Database::PreventCrossJoins::GitlabDatabaseMixin)
ALLOW_LIST = Set.new(YAML.load_file(Rails.root.join('.cross-join-allowlist.yml'))).freeze
RSpec.configure do |config|
config.include(::Database::PreventCrossJoins::SpecHelpers)
# TODO: remove `:prevent_cross_joins` to enable the check by default
config.around(:each, :prevent_cross_joins) do |example|
with_cross_joins_prevented { example.run }
config.around do |example|
Thread.current[:has_cross_join_exception] = false
if ALLOW_LIST.include?(example.file_path)
example.run
else
with_cross_joins_prevented { example.run }
end
end
end
......@@ -3,7 +3,7 @@
require 'spec_helper'
RSpec.describe Database::PreventCrossJoins do
context 'when running in :prevent_cross_joins scope', :prevent_cross_joins do
context 'when running in a default scope' do
context 'when only non-CI tables are used' do
it 'does not raise exception' do
expect { main_only_query }.not_to raise_error
......@@ -32,14 +32,6 @@ RSpec.describe Database::PreventCrossJoins do
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
......
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