Identifying all cross joins between namespaces and users

Addressing Issue #415196

Changelog: other

.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'

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
 

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
 

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)

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

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
 

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

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,

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
 

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)

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
 

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

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
 

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)

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)

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

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

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

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
 

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