From 0534013c522dbaaf5cc1f480df8119666bf5bd42 Mon Sep 17 00:00:00 2001 From: Thong Kuah <tkuah@gitlab.com> Date: Wed, 11 May 2022 13:34:21 +1200 Subject: [PATCH] Allowlist Database/MultipleDatabases offenses - config/initializers/sidekiq.rb this method is safe for multiple databases - lib/gitlab/database.rb is a legitmate use - lib/gitlab/database/load_balancing/load_balancer.rb uses methods that are safe with multiple databases - lib/gitlab/database/migrations/observers/query_log.rb also uses a method that is safe with multiple databases - lib/tasks/gitlab/db/validate_config.rake is OK because it's executed in the context of looping through each database. --- .rubocop_todo/database/multiple_databases.yml | 5 ----- config/initializers/sidekiq.rb | 2 +- lib/gitlab/database.rb | 2 +- lib/gitlab/database/load_balancing/load_balancer.rb | 2 ++ lib/gitlab/database/migrations/observers/query_log.rb | 2 +- lib/tasks/gitlab/db/validate_config.rake | 4 +++- 6 files changed, 8 insertions(+), 9 deletions(-) diff --git a/.rubocop_todo/database/multiple_databases.yml b/.rubocop_todo/database/multiple_databases.yml index 08ce72f7f51e..1d0085e1ba65 100644 --- a/.rubocop_todo/database/multiple_databases.yml +++ b/.rubocop_todo/database/multiple_databases.yml @@ -2,7 +2,6 @@ Database/MultipleDatabases: Exclude: - 'config/initializers/active_record_data_types.rb' - - 'config/initializers/sidekiq.rb' - 'db/post_migrate/20210317104032_set_iteration_cadence_automatic_to_false.rb' - 'db/post_migrate/20210811122206_update_external_project_bots.rb' - 'db/post_migrate/20210812013042_remove_duplicate_project_authorizations.rb' @@ -10,10 +9,6 @@ Database/MultipleDatabases: - 'lib/backup/manager.rb' - 'lib/gitlab/background_migration/backfill_projects_with_coverage.rb' - 'lib/gitlab/background_migration/copy_ci_builds_columns_to_security_scans.rb' - - 'lib/gitlab/database.rb' - - 'lib/gitlab/database/load_balancing/load_balancer.rb' - - 'lib/gitlab/database/migrations/observers/query_log.rb' - - 'lib/tasks/gitlab/db/validate_config.rake' - 'spec/db/schema_spec.rb' - 'spec/initializers/database_config_spec.rb' - 'spec/lib/backup/manager_spec.rb' diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb index b3c7d9cbad54..29df6da6ef1f 100644 --- a/config/initializers/sidekiq.rb +++ b/config/initializers/sidekiq.rb @@ -56,7 +56,7 @@ def enable_semi_reliable_fetch_mode? config.on :startup do # Clear any connections that might have been obtained before starting # Sidekiq (e.g. in an initializer). - ActiveRecord::Base.clear_all_connections! + ActiveRecord::Base.clear_all_connections! # rubocop:disable Database/MultipleDatabases # Start monitor to track running jobs. By default, cancel job is not enabled # To cancel job, it requires `SIDEKIQ_MONITOR_WORKER=1` to enable notification channel diff --git a/lib/gitlab/database.rb b/lib/gitlab/database.rb index 1895f0fab321..d5cd22b94b1a 100644 --- a/lib/gitlab/database.rb +++ b/lib/gitlab/database.rb @@ -49,7 +49,7 @@ module Database # It does not include the default public schema EXTRA_SCHEMAS = [DYNAMIC_PARTITIONS_SCHEMA, STATIC_PARTITIONS_SCHEMA].freeze - PRIMARY_DATABASE_NAME = ActiveRecord::Base.connection_db_config.name.to_sym + PRIMARY_DATABASE_NAME = ActiveRecord::Base.connection_db_config.name.to_sym # rubocop:disable Database/MultipleDatabases def self.database_base_models @database_base_models ||= { diff --git a/lib/gitlab/database/load_balancing/load_balancer.rb b/lib/gitlab/database/load_balancing/load_balancer.rb index 1e27bcfc55dc..191ebe18b8ac 100644 --- a/lib/gitlab/database/load_balancing/load_balancer.rb +++ b/lib/gitlab/database/load_balancing/load_balancer.rb @@ -255,6 +255,7 @@ def create_replica_connection_pool(pool_size, host = nil, port = nil) # ActiveRecord::ConnectionAdapters::ConnectionHandler handles fetching, # and caching for connections pools for each "connection", so we # leverage that. + # rubocop:disable Database/MultipleDatabases def pool ActiveRecord::Base.connection_handler.retrieve_connection_pool( @configuration.primary_connection_specification_name, @@ -262,6 +263,7 @@ def pool shard: ActiveRecord::Base.default_shard ) || raise(::ActiveRecord::ConnectionNotEstablished) end + # rubocop:enable Database/MultipleDatabases def wal_diff(location1, location2) read_write do |connection| diff --git a/lib/gitlab/database/migrations/observers/query_log.rb b/lib/gitlab/database/migrations/observers/query_log.rb index 8ca57bb7df94..543e6b8e3023 100644 --- a/lib/gitlab/database/migrations/observers/query_log.rb +++ b/lib/gitlab/database/migrations/observers/query_log.rb @@ -6,7 +6,7 @@ module Migrations module Observers class QueryLog < MigrationObserver def before - @logger_was = ActiveRecord::Base.logger + @logger_was = ActiveRecord::Base.logger # rubocop:disable Database/MultipleDatabases file_path = File.join(output_dir, "migration.log") @logger = Logger.new(file_path) ActiveRecord::Base.logger = @logger diff --git a/lib/tasks/gitlab/db/validate_config.rake b/lib/tasks/gitlab/db/validate_config.rake index cc5f6bb6e097..66aa949cc942 100644 --- a/lib/tasks/gitlab/db/validate_config.rake +++ b/lib/tasks/gitlab/db/validate_config.rake @@ -6,7 +6,7 @@ namespace :gitlab do namespace :db do desc 'Validates `config/database.yml` to ensure a correct behavior is configured' task validate_config: :environment do - original_db_config = ActiveRecord::Base.connection_db_config + original_db_config = ActiveRecord::Base.connection_db_config # rubocop:disable Database/MultipleDatabases # The include_replicas: is a legacy name to fetch all hidden entries (replica: true or database_tasks: false) # Once we upgrade to Rails 7.x this should be changed to `include_hidden: true` @@ -15,6 +15,7 @@ namespace :gitlab do db_configs = db_configs.reject(&:replica?) # Map each database connection into unique identifier of system+database + # rubocop:disable Database/MultipleDatabases all_connections = db_configs.map do |db_config| identifier = begin @@ -32,6 +33,7 @@ namespace :gitlab do identifier: identifier } end.compact + # rubocop:enable Database/MultipleDatabases unique_connections = all_connections.group_by { |connection| connection[:identifier] } primary_connection = all_connections.find { |connection| ActiveRecord::Base.configurations.primary?(connection[:name]) } -- GitLab