From 62b941fa1d2cd41613f1131bb122e84d973b7b88 Mon Sep 17 00:00:00 2001 From: Omar Qunsul <oqunsul@gitlab.com> Date: Wed, 21 Jun 2023 15:22:00 +0300 Subject: [PATCH] Identifying all cross joins between namespaces and users Addressing Issue https://gitlab.com/gitlab-org/gitlab/-/issues/415196 Changelog: other --- .rubocop_todo/layout/line_length.yml | 1 - app/finders/events_finder.rb | 1 + app/finders/issuables/assignee_filter.rb | 2 ++ app/finders/members_finder.rb | 4 +++- app/finders/merge_requests_finder.rb | 1 + app/models/concerns/milestoneish.rb | 1 + app/models/concerns/protected_ref.rb | 7 ++++++- app/models/concerns/protected_ref_access.rb | 9 +++++++- app/models/group.rb | 21 ++++++++++++------- app/models/member.rb | 11 ++++++++-- .../boards/base_items_list_service.rb | 19 ++++++++++------- db/gitlab_schemas/gitlab_main_cell.yaml | 3 --- ee/app/finders/epics_finder.rb | 3 +++ .../merge_requests/by_approvers_finder.rb | 1 + .../finders/namespaces/billed_users_finder.rb | 9 +++++--- .../namespaces/free_user_cap/users_finder.rb | 3 ++- ee/app/models/approval_merge_request_rule.rb | 2 +- ee/app/models/approval_state.rb | 12 ++++++----- ee/app/models/approval_wrapped_rule.rb | 9 +++++--- ee/app/models/concerns/approval_rule_like.rb | 5 +++++ ee/app/models/ee/group.rb | 1 + ee/app/models/ee/project.rb | 4 +++- .../approval_rules/finalize_service.rb | 18 +++++++++------- .../ee/boards/lists/create_service.rb | 4 +++- .../fetch_policy_approvers_service.rb | 1 + ee/lib/ee/backup/repositories.rb | 7 +++++-- ee/lib/ee/gitlab/auth/ldap/sync/group.rb | 1 + lib/api/helpers/members_helpers.rb | 1 + lib/backup/repositories.rb | 7 +++++-- .../references/user_reference_filter.rb | 11 ++++++---- lib/banzai/reference_parser/user_parser.rb | 9 +++++--- lib/gitlab/group_search_results.rb | 1 + .../primary_key_batching_strategy_spec.rb | 5 +++-- spec/support/database/prevent_cross_joins.rb | 2 +- 34 files changed, 135 insertions(+), 61 deletions(-) diff --git a/.rubocop_todo/layout/line_length.yml b/.rubocop_todo/layout/line_length.yml index dc6cccb6762ed..017bc35e8b861 100644 --- a/.rubocop_todo/layout/line_length.yml +++ b/.rubocop_todo/layout/line_length.yml @@ -317,7 +317,6 @@ Layout/LineLength: - 'app/models/concerns/packages/debian/distribution.rb' - 'app/models/concerns/packages/debian/distribution_key.rb' - 'app/models/concerns/partitioned_table.rb' - - 'app/models/concerns/protected_ref.rb' - 'app/models/concerns/redis_cacheable.rb' - 'app/models/concerns/restricted_signup.rb' - 'app/models/concerns/shardable.rb' diff --git a/app/finders/events_finder.rb b/app/finders/events_finder.rb index 4ed447a90cefe..7bccfe453ab3b 100644 --- a/app/finders/events_finder.rb +++ b/app/finders/events_finder.rb @@ -61,6 +61,7 @@ def get_events def by_current_user_access(events) events.merge(Project.public_or_visible_to_user(current_user)) .joins(:project) + .allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/417462") end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/finders/issuables/assignee_filter.rb b/app/finders/issuables/assignee_filter.rb index 2e58a6b34c9e9..c97fdffd32eae 100644 --- a/app/finders/issuables/assignee_filter.rb +++ b/app/finders/issuables/assignee_filter.rb @@ -5,6 +5,8 @@ class AssigneeFilter < BaseFilter def filter(issuables) filtered = by_assignee(issuables) filtered = by_assignee_union(filtered) + # Cross Joins Fails tests in bin/rspec spec/requests/api/graphql/boards/board_list_issues_query_spec.rb + filtered = filtered.allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/417462") by_negated_assignee(filtered) end diff --git a/app/finders/members_finder.rb b/app/finders/members_finder.rb index 3c0714441b20e..6348bceb1574b 100644 --- a/app/finders/members_finder.rb +++ b/app/finders/members_finder.rb @@ -90,8 +90,10 @@ def distinct_union_of_members(union_members) # enumerate the columns here since we are enumerating them in the union and want to be immune to # column caching issues when adding/removing columns - Member.select(*Member.column_names) + members = Member.select(*Member.column_names) .includes(:user).from([Arel.sql("(#{sql}) AS #{Member.table_name}")]) # rubocop: disable CodeReuse/ActiveRecord + # The left join with the table users in the method distinct_on needs to be resolved + members.allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/417456") end def distinct_on(union) diff --git a/app/finders/merge_requests_finder.rb b/app/finders/merge_requests_finder.rb index f7ee90ab8705c..95b5b267089e9 100644 --- a/app/finders/merge_requests_finder.rb +++ b/app/finders/merge_requests_finder.rb @@ -73,6 +73,7 @@ def filter_items(_items) items = by_deployments(items) items = by_reviewer(items) items = by_source_project_id(items) + items = items.allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/417462") by_approved(items) end diff --git a/app/models/concerns/milestoneish.rb b/app/models/concerns/milestoneish.rb index 7968fe1cd86f5..3d9e09acf44e0 100644 --- a/app/models/concerns/milestoneish.rb +++ b/app/models/concerns/milestoneish.rb @@ -51,6 +51,7 @@ def issues_visible_to_user(user) def issue_participants_visible_by_user(user) User.joins(:issue_assignees) .where('issue_assignees.issue_id' => issues_visible_to_user(user).select(:id)) + .allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/417457") .distinct end diff --git a/app/models/concerns/protected_ref.rb b/app/models/concerns/protected_ref.rb index 7e1ebd1eba3a5..a87eadb93321d 100644 --- a/app/models/concerns/protected_ref.rb +++ b/app/models/concerns/protected_ref.rb @@ -32,7 +32,12 @@ def protected_ref_access_levels(*types) # to fail. has_many :"#{type}_access_levels", inverse_of: self.model_name.singular - validates :"#{type}_access_levels", length: { is: 1, message: "are restricted to a single instance per #{self.model_name.human}." }, unless: -> { allow_multiple?(type) } + validates :"#{type}_access_levels", + length: { + is: 1, + message: "are restricted to a single instance per #{self.model_name.human}." + }, + unless: -> { allow_multiple?(type) } accepts_nested_attributes_for :"#{type}_access_levels", allow_destroy: true end diff --git a/app/models/concerns/protected_ref_access.rb b/app/models/concerns/protected_ref_access.rb index ce520f946140f..f0bb1cc359b69 100644 --- a/app/models/concerns/protected_ref_access.rb +++ b/app/models/concerns/protected_ref_access.rb @@ -38,7 +38,14 @@ def non_role_types included do scope :maintainer, -> { where(access_level: Gitlab::Access::MAINTAINER) } scope :developer, -> { where(access_level: Gitlab::Access::DEVELOPER) } - scope :for_role, -> { non_role_types.present? ? where.missing(*non_role_types) : all } + scope :for_role, -> { + if non_role_types.present? + where.missing(*non_role_types) + .allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/417457") + else + all + end + } protected_ref_fk = "#{module_parent.model_name.singular}_id" validates :access_level, diff --git a/app/models/group.rb b/app/models/group.rb index 4e92bdab94ce7..4094f0a1750a2 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -510,7 +510,9 @@ def member_owners_excluding_project_bots members_with_parents(only_active_users: false) end - members_from_hiearchy.all_owners.left_outer_joins(:user).merge(User.without_project_bot) + members_from_hiearchy.all_owners.left_outer_joins(:user) + .merge(User.without_project_bot) + .allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/417455") end def ldap_synced? @@ -694,7 +696,7 @@ def direct_and_indirect_users_with_inactive .where(id: direct_and_indirect_members_with_inactive.select(:user_id)) .reorder(nil), project_users_with_descendants - ]) + ]).allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/417455") # failed in spec/tasks/gitlab/user_management_rake_spec.rb end def users_count @@ -707,6 +709,7 @@ def project_users_with_descendants User .joins(projects: :group) .where(namespaces: { id: self_and_descendants.select(:id) }) + .allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/417455") end # Return the highest access level for a user @@ -970,12 +973,14 @@ def feature_flag_enabled_for_self_or_ancestor?(feature_flag) end def max_member_access(user_ids) - Gitlab::SafeRequestLoader.execute( - resource_key: max_member_access_for_resource_key(User), - resource_ids: user_ids, - default_value: Gitlab::Access::NO_ACCESS - ) do |user_ids| - members_with_parents.where(user_id: user_ids).group(:user_id).maximum(:access_level) + ::Gitlab::Database.allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/417455") do + Gitlab::SafeRequestLoader.execute( + resource_key: max_member_access_for_resource_key(User), + resource_ids: user_ids, + default_value: Gitlab::Access::NO_ACCESS + ) do |user_ids| + members_with_parents.where(user_id: user_ids).group(:user_id).maximum(:access_level) + end end end diff --git a/app/models/member.rb b/app/models/member.rb index 0700b1a844859..f164ea244b4d5 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -66,6 +66,7 @@ class Member < ApplicationRecord scope :with_invited_user_state, -> do joins('LEFT JOIN users as invited_user ON invited_user.email = members.invite_email') .select('members.*', 'invited_user.state as invited_user_state') + .allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/417456") end scope :in_hierarchy, ->(source) do @@ -174,7 +175,10 @@ class Member < ApplicationRecord scope :by_access_level, -> (access_level) { active.where(access_level: access_level) } scope :all_by_access_level, -> (access_level) { where(access_level: access_level) } - scope :preload_user_and_notification_settings, -> { preload(user: :notification_settings) } + scope :preload_user_and_notification_settings, -> do + preload(user: :notification_settings) + .allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/417456") + end scope :with_source_id, ->(source_id) { where(source_id: source_id) } scope :including_source, -> { includes(:source) } @@ -288,7 +292,9 @@ class Member < ApplicationRecord class << self def search(query) - scope = joins(:user).merge(User.search(query, use_minimum_char_limit: false)) + scope = joins(:user) + .merge(User.search(query, use_minimum_char_limit: false)) + .allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/417456") return scope unless Gitlab::Pagination::Keyset::Order.keyset_aware?(scope) @@ -347,6 +353,7 @@ def sort_by_attribute(method) def left_join_users left_outer_joins(:user) + .allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/417456") end def access_for_user_ids(user_ids) diff --git a/app/services/boards/base_items_list_service.rb b/app/services/boards/base_items_list_service.rb index bf68aee2c1f23..a4b1be1e599f1 100644 --- a/app/services/boards/base_items_list_service.rb +++ b/app/services/boards/base_items_list_service.rb @@ -13,14 +13,17 @@ def execute # rubocop: disable CodeReuse/ActiveRecord def metadata(required_fields = [:issue_count, :total_issue_weight]) - fields = metadata_fields(required_fields) - keys = fields.keys - # TODO: eliminate need for SQL literal fragment - columns = Arel.sql(fields.values_at(*keys).join(', ')) - results = item_model.where(id: collection_ids) - results = results.select(columns) - - Hash[keys.zip(results.pluck(columns).flatten)] + # Failing tests in spec/requests/api/graphql/boards/board_lists_query_spec.rb + ::Gitlab::Database.allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/417465") do + fields = metadata_fields(required_fields) + keys = fields.keys + # TODO: eliminate need for SQL literal fragment + columns = Arel.sql(fields.values_at(*keys).join(', ')) + results = item_model.where(id: collection_ids) + results = results.select(columns) + + Hash[keys.zip(results.pluck(columns).flatten)] + end end # rubocop: enable CodeReuse/ActiveRecord diff --git a/db/gitlab_schemas/gitlab_main_cell.yaml b/db/gitlab_schemas/gitlab_main_cell.yaml index 0f85cfdf5d3b7..7bc140ffecb55 100644 --- a/db/gitlab_schemas/gitlab_main_cell.yaml +++ b/db/gitlab_schemas/gitlab_main_cell.yaml @@ -3,9 +3,6 @@ description: Schema for all Cell-local tables, ex. namespaces, projects, etc. allow_cross_joins: - gitlab_shared - gitlab_main - # Temporarily allow cross-joins between clusterwide and cell schemas - # This is to be removed once we annotate all cross-joins between those - - gitlab_main_clusterwide allow_cross_transactions: - gitlab_internal - gitlab_shared diff --git a/ee/app/finders/epics_finder.rb b/ee/app/finders/epics_finder.rb index a8c6cf6ee6d05..19e55333df274 100644 --- a/ee/app/finders/epics_finder.rb +++ b/ee/app/finders/epics_finder.rb @@ -53,6 +53,9 @@ def execute(skip_visibility_check: false) items = filter_and_search(init_collection) + # fails in ee/spec/graphql/resolvers/boards/board_list_epics_resolver_spec.rb + items = items.allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/417462") + sort(items) end diff --git a/ee/app/finders/merge_requests/by_approvers_finder.rb b/ee/app/finders/merge_requests/by_approvers_finder.rb index 00b2593eeeeb8..5631f51ea5ab2 100644 --- a/ee/app/finders/merge_requests/by_approvers_finder.rb +++ b/ee/app/finders/merge_requests/by_approvers_finder.rb @@ -75,6 +75,7 @@ def find_approvers_by_query(items, field, values) .where(users: { field => values }) .group('merge_requests.id') .having("COUNT(users.id) = ?", values.size) + .allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/417459") end def with_users_filtered_by_criteria(items) diff --git a/ee/app/finders/namespaces/billed_users_finder.rb b/ee/app/finders/namespaces/billed_users_finder.rb index 9c7b31fc8d83e..a924bb15f7692 100644 --- a/ee/app/finders/namespaces/billed_users_finder.rb +++ b/ee/app/finders/namespaces/billed_users_finder.rb @@ -28,10 +28,13 @@ def execute }.freeze def calculate_user_ids(method_name) - @ids[METHOD_KEY_MAP[method_name]] = group.public_send(method_name, exclude_guests: @exclude_guests) # rubocop:disable GitlabSecurity/PublicSend - .pluck(:id).to_set # rubocop:disable CodeReuse/ActiveRecord + cross_join_issue = "https://gitlab.com/gitlab-org/gitlab/-/issues/417464" + ::Gitlab::Database.allow_cross_joins_across_databases(url: cross_join_issue) do + @ids[METHOD_KEY_MAP[method_name]] = group.public_send(method_name, exclude_guests: @exclude_guests) # rubocop:disable GitlabSecurity/PublicSend + .pluck(:id).to_set # rubocop:disable CodeReuse/ActiveRecord - append_to_user_ids(ids[METHOD_KEY_MAP[method_name]]) + append_to_user_ids(ids[METHOD_KEY_MAP[method_name]]) + end end def append_to_user_ids(user_ids) diff --git a/ee/app/finders/namespaces/free_user_cap/users_finder.rb b/ee/app/finders/namespaces/free_user_cap/users_finder.rb index 89f5ae1fd4bc4..907595c008385 100644 --- a/ee/app/finders/namespaces/free_user_cap/users_finder.rb +++ b/ee/app/finders/namespaces/free_user_cap/users_finder.rb @@ -27,7 +27,8 @@ def calculate_user_ids(method_name) return if ids[:user_ids].count >= limit @ids[METHOD_KEY_MAP[method_name]] = group.public_send(method_name).limit(limit) # rubocop:disable GitlabSecurity/PublicSend - .pluck(:id).to_set # rubocop:disable CodeReuse/ActiveRecord + .allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/417464") + .pluck(:id).to_set # rubocop:disable CodeReuse/ActiveRecord append_to_user_ids(ids[METHOD_KEY_MAP[method_name]]) end diff --git a/ee/app/models/approval_merge_request_rule.rb b/ee/app/models/approval_merge_request_rule.rb index d6edcffb89765..f824c5430a120 100644 --- a/ee/app/models/approval_merge_request_rule.rb +++ b/ee/app/models/approval_merge_request_rule.rb @@ -105,7 +105,7 @@ def approvers next scope_or_array if project.merge_requests_author_approval? if scope_or_array.respond_to?(:where) - scope_or_array.where.not(id: merge_request.author) + scope_or_array.where.not(id: merge_request.author).allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/417459") else scope_or_array - [merge_request.author] end diff --git a/ee/app/models/approval_state.rb b/ee/app/models/approval_state.rb index 23c57add91635..a56eafdac193e 100644 --- a/ee/app/models/approval_state.rb +++ b/ee/app/models/approval_state.rb @@ -24,7 +24,7 @@ def self.filter_author(users, merge_request) return users if merge_request.target_project.merge_requests_author_approval? if users.is_a?(ActiveRecord::Relation) && !users.loaded? - users.where.not(id: merge_request.author_id) + users.where.not(id: merge_request.author_id).allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/417459") else users - [merge_request.author] end @@ -35,7 +35,7 @@ def self.filter_committers(users, merge_request) return users unless merge_request.target_project.merge_requests_disable_committers_approval? if users.is_a?(ActiveRecord::Relation) && !users.loaded? - users.where.not(id: merge_request.committers.select(:id)) + users.where.not(id: merge_request.committers.select(:id)).allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/417459") else users - merge_request.committers end @@ -141,10 +141,12 @@ def approvers # @param target [:approvers, :users] # @param unactioned [Boolean] def filtered_approvers(code_owner: true, target: :approvers, unactioned: false) - rules = user_defined_rules + report_approver_rules - rules.concat(code_owner_rules) if code_owner + ::Gitlab::Database.allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/417459") do + rules = user_defined_rules + report_approver_rules + rules.concat(code_owner_rules) if code_owner - filter_approvers(rules.flat_map(&target), unactioned: unactioned) + filter_approvers(rules.flat_map(&target), unactioned: unactioned) + end end def unactioned_approvers diff --git a/ee/app/models/approval_wrapped_rule.rb b/ee/app/models/approval_wrapped_rule.rb index ecad13f68b216..d556364555851 100644 --- a/ee/app/models/approval_wrapped_rule.rb +++ b/ee/app/models/approval_wrapped_rule.rb @@ -109,10 +109,13 @@ def allow_merge_when_invalid? # and/or allow MR authors to approve their own merge # requests (in case only one approval is needed). def approvals_left - strong_memoize(:approvals_left) do - approvals_left_count = approvals_required - approved_approvers.size + cross_join_issue = "https://gitlab.com/gitlab-org/gitlab/-/issues/417459" + ::Gitlab::Database.allow_cross_joins_across_databases(url: cross_join_issue) do + strong_memoize(:approvals_left) do + approvals_left_count = approvals_required - approved_approvers.size - [approvals_left_count, 0].max + [approvals_left_count, 0].max + end end end diff --git a/ee/app/models/concerns/approval_rule_like.rb b/ee/app/models/concerns/approval_rule_like.rb index 5aff137b99a94..0b56c86b40cb4 100644 --- a/ee/app/models/concerns/approval_rule_like.rb +++ b/ee/app/models/concerns/approval_rule_like.rb @@ -51,6 +51,10 @@ module ApprovalRuleLike end end + def group_users + super.loaded? ? super : super.allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/417457") + end + def audit_add raise NotImplementedError end @@ -125,6 +129,7 @@ def role_approvers def filter_inactive_approvers(approvers) if approvers.respond_to?(:with_state) approvers.with_state(:active) + .allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/417457") else approvers.select(&:active?) end diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb index 5ac2e55acab57..66dedd0303488 100644 --- a/ee/app/models/ee/group.rb +++ b/ee/app/models/ee/group.rb @@ -768,6 +768,7 @@ def billed_project_users(exclude_guests: false) members = members.not_banned_in(root_ancestor) users_without_bots(members).with_state(:active) + .allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/417455") end # Members belonging to Groups invited to collaborate with Groups and Subgroups diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb index ffce07df5d2dd..a5b9e3f8fb445 100644 --- a/ee/app/models/ee/project.rb +++ b/ee/app/models/ee/project.rb @@ -704,7 +704,9 @@ def approvals_before_merge def applicable_approval_rules_for_user(user_id, target_branch = nil) visible_approval_rules(target_branch: target_branch).select do |rule| - rule.approvers.pluck(:id).include?(user_id) + rule.approvers + .allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/417461") + .pluck(:id).include?(user_id) end end diff --git a/ee/app/services/approval_rules/finalize_service.rb b/ee/app/services/approval_rules/finalize_service.rb index 8769665337b5a..1006595fe279b 100644 --- a/ee/app/services/approval_rules/finalize_service.rb +++ b/ee/app/services/approval_rules/finalize_service.rb @@ -11,14 +11,18 @@ def initialize(merge_request) def execute return unless merge_request.merged? - ApplicationRecord.transaction do - if merge_request.approval_rules.regular.exists? - merge_group_members_into_users - else - copy_project_approval_rules - end + # fails ee/spec/services/approval_rules/finalize_service_spec.rb + cross_join_issue = "https://gitlab.com/gitlab-org/gitlab/-/issues/417459" + ::Gitlab::Database.allow_cross_joins_across_databases(url: cross_join_issue) do + ApplicationRecord.transaction do + if merge_request.approval_rules.regular.exists? + merge_group_members_into_users + else + copy_project_approval_rules + end - merge_request.approval_rules.each(&:sync_approved_approvers) + merge_request.approval_rules.each(&:sync_approved_approvers) + end end end diff --git a/ee/app/services/ee/boards/lists/create_service.rb b/ee/app/services/ee/boards/lists/create_service.rb index 19329c4b3b86c..f8710b597e1b3 100644 --- a/ee/app/services/ee/boards/lists/create_service.rb +++ b/ee/app/services/ee/boards/lists/create_service.rb @@ -89,7 +89,9 @@ def find_iteration(board) # rubocop: disable CodeReuse/ActiveRecord def find_user(board) user_ids = user_finder(board).execute.reselect(:user_id) - ::User.id_in(user_ids).find_by(id: params['assignee_id']) + ::User.id_in(user_ids) + .allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/417465") + .find_by(id: params['assignee_id']) end # rubocop: enable CodeReuse/ActiveRecord diff --git a/ee/app/services/security/security_orchestration_policies/fetch_policy_approvers_service.rb b/ee/app/services/security/security_orchestration_policies/fetch_policy_approvers_service.rb index 7a8a01aa2ac4b..343b918ba03e2 100644 --- a/ee/app/services/security/security_orchestration_policies/fetch_policy_approvers_service.rb +++ b/ee/app/services/security/security_orchestration_policies/fetch_policy_approvers_service.rb @@ -57,6 +57,7 @@ def authorizable_users_in_group_hierarchy_by_ids_or_usernames(user_ids, user_nam .arel .exists ) + .allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/417460") end # rubocop: enable CodeReuse/ActiveRecord diff --git a/ee/lib/ee/backup/repositories.rb b/ee/lib/ee/backup/repositories.rb index f4c5a9a0bc08a..2363072bf690c 100644 --- a/ee/lib/ee/backup/repositories.rb +++ b/ee/lib/ee/backup/repositories.rb @@ -32,8 +32,11 @@ def enqueue_consecutive end def enqueue_consecutive_groups - find_groups_in_batches do |group| - enqueue_group(group) + cross_join_issue = "https://gitlab.com/gitlab-org/gitlab/-/issues/417467" + ::Gitlab::Database.allow_cross_joins_across_databases(url: cross_join_issue) do + find_groups_in_batches do |group| + enqueue_group(group) + end end end end diff --git a/ee/lib/ee/gitlab/auth/ldap/sync/group.rb b/ee/lib/ee/gitlab/auth/ldap/sync/group.rb index 60791139947cb..316fa8529911b 100644 --- a/ee/lib/ee/gitlab/auth/ldap/sync/group.rb +++ b/ee/lib/ee/gitlab/auth/ldap/sync/group.rb @@ -162,6 +162,7 @@ def inherit_higher_access_levels(group, access_levels) .where(users: { identities: ::Identity.iwhere(extern_uid: access_levels.keys) }) .select(:id, 'identities.extern_uid AS distinguished_name', :access_level, :source_id) .references(:identities) + .allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/417455") permissions_in_ancestry.each do |member| access_levels.set([member.distinguished_name], to: member.access_level) diff --git a/lib/api/helpers/members_helpers.rb b/lib/api/helpers/members_helpers.rb index b3c7948646515..a82aed507fd2e 100644 --- a/lib/api/helpers/members_helpers.rb +++ b/lib/api/helpers/members_helpers.rb @@ -29,6 +29,7 @@ def authorize_admin_source!(source_type, source) # rubocop: disable CodeReuse/ActiveRecord def retrieve_members(source, params:, deep: false) members = deep ? find_all_members(source) : source_members(source).connected_to_user + members = members.allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/417456") members = members.includes(:user) members = members.references(:user).merge(User.search(params[:query], use_minimum_char_limit: false)) if params[:query].present? members = members.where(user_id: params[:user_ids]) if params[:user_ids].present? diff --git a/lib/backup/repositories.rb b/lib/backup/repositories.rb index 56726665d14da..199da8821d987 100644 --- a/lib/backup/repositories.rb +++ b/lib/backup/repositories.rb @@ -58,8 +58,11 @@ def enqueue_consecutive end def enqueue_consecutive_projects - project_relation.find_each(batch_size: 1000) do |project| - enqueue_project(project) + cross_join_issue = "https://gitlab.com/gitlab-org/gitlab/-/issues/417467" + ::Gitlab::Database.allow_cross_joins_across_databases(url: cross_join_issue) do + project_relation.find_each(batch_size: 1000) do |project| + enqueue_project(project) + end end end diff --git a/lib/banzai/filter/references/user_reference_filter.rb b/lib/banzai/filter/references/user_reference_filter.rb index d6b6fdb714978..a3784004087ab 100644 --- a/lib/banzai/filter/references/user_reference_filter.rb +++ b/lib/banzai/filter/references/user_reference_filter.rb @@ -65,10 +65,13 @@ def object_link_filter(text, pattern, link_content: nil, link_reference: false) # The keys of this Hash are the namespace paths, the values the # corresponding Namespace objects. def namespaces - @namespaces ||= Namespace.eager_load(:owner, :route) - .where_full_path_in(usernames) - .index_by(&:full_path) - .transform_keys(&:downcase) + cross_join_issue = "https://gitlab.com/gitlab-org/gitlab/-/issues/417466" + Gitlab::Database.allow_cross_joins_across_databases(url: cross_join_issue) do + @namespaces ||= Namespace.eager_load(:owner, :route) + .where_full_path_in(usernames) + .index_by(&:full_path) + .transform_keys(&:downcase) + end end # Returns all usernames referenced in the current document. diff --git a/lib/banzai/reference_parser/user_parser.rb b/lib/banzai/reference_parser/user_parser.rb index 48e2bcc9a116b..ec96181e7f173 100644 --- a/lib/banzai/reference_parser/user_parser.rb +++ b/lib/banzai/reference_parser/user_parser.rb @@ -97,9 +97,12 @@ def find_users(ids) def find_users_for_groups(ids) return [] if ids.empty? - User.joins(:group_members).where(members: { - source_id: Namespace.where(id: ids).where('mentions_disabled IS NOT TRUE').select(:id) - }).to_a + cross_join_issue = "https://gitlab.com/gitlab-org/gitlab/-/issues/417466" + ::Gitlab::Database.allow_cross_joins_across_databases(url: cross_join_issue) do + User.joins(:group_members).where(members: { + source_id: Namespace.where(id: ids).where('mentions_disabled IS NOT TRUE').select(:id) + }).to_a + end end def find_users_for_projects(ids) diff --git a/lib/gitlab/group_search_results.rb b/lib/gitlab/group_search_results.rb index b112740c4adee..8ca88859b22c7 100644 --- a/lib/gitlab/group_search_results.rb +++ b/lib/gitlab/group_search_results.rb @@ -13,6 +13,7 @@ def initialize(current_user, query, limit_projects = nil, group:, default_projec # rubocop:disable CodeReuse/ActiveRecord def users groups = group.self_and_hierarchy_intersecting_with_user_groups(current_user) + groups = groups.allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/417455") members = GroupMember.where(group: groups).non_invite users = super diff --git a/spec/lib/gitlab/background_migration/batching_strategies/primary_key_batching_strategy_spec.rb b/spec/lib/gitlab/background_migration/batching_strategies/primary_key_batching_strategy_spec.rb index 37fdd209622cb..6201e2c0fcc6b 100644 --- a/spec/lib/gitlab/background_migration/batching_strategies/primary_key_batching_strategy_spec.rb +++ b/spec/lib/gitlab/background_migration/batching_strategies/primary_key_batching_strategy_spec.rb @@ -2,7 +2,8 @@ require 'spec_helper' -RSpec.describe Gitlab::BackgroundMigration::BatchingStrategies::PrimaryKeyBatchingStrategy, '#next_batch' do +RSpec.describe Gitlab::BackgroundMigration::BatchingStrategies::PrimaryKeyBatchingStrategy, + '#next_batch', feature_category: :database do let(:batching_strategy) { described_class.new(connection: ActiveRecord::Base.connection) } let(:namespaces) { table(:namespaces) } @@ -64,7 +65,7 @@ context 'when scope has a join which makes the column name ambiguous' do let(:job_class) do Class.new(Gitlab::BackgroundMigration::BatchedMigrationJob) do - scope_to ->(r) { r.joins('LEFT JOIN users ON users.id = namespaces.owner_id') } + scope_to ->(r) { r.joins('LEFT JOIN namespaces as parents ON parents.id = namespaces.parent_id') } end end diff --git a/spec/support/database/prevent_cross_joins.rb b/spec/support/database/prevent_cross_joins.rb index 540c287bdad27..443216ba9df4f 100644 --- a/spec/support/database/prevent_cross_joins.rb +++ b/spec/support/database/prevent_cross_joins.rb @@ -1,7 +1,7 @@ # 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 +# This will forbid usage of tables of different gitlab_schemas # 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. -- GitLab