From e085626854606740666b971867c6a88c9b4e79ea Mon Sep 17 00:00:00 2001 From: Dylan Griffith <dyl.griffith@gmail.com> Date: Thu, 21 Apr 2022 03:17:06 +0000 Subject: [PATCH] Remove support for uses_legacy_database_config This was planned to to be removed in `15.0`. This setting in `config/database.yml` (or inferred from the structure of `config/database.yml`) allowed GitLab installations (from source) to stick to the old "flat" structure. Since we've already updated Omnibus installations and GDK to no longer generate this flat structure there would only be installations from source that are affected by this change and they should have already been receiving deprecation warnings for some months. This change will ensure they see an error if they don't have a top level `main` key in their `config/database.yml` and all they need to do is nest the config under `main` to fix the problem. Changelog: removed --- .../initializers/validate_database_config.rb | 6 - .../lib/gitlab/patch/database_config_spec.rb | 187 +++--------------- lib/gitlab/patch/database_config.rb | 21 -- .../validate_database_config_spec.rb | 40 +--- spec/lib/gitlab/patch/database_config_spec.rb | 59 +----- 5 files changed, 44 insertions(+), 269 deletions(-) diff --git a/config/initializers/validate_database_config.rb b/config/initializers/validate_database_config.rb index d5e73cdc1ee73..d381dbac2ed4a 100644 --- a/config/initializers/validate_database_config.rb +++ b/config/initializers/validate_database_config.rb @@ -4,12 +4,6 @@ return end -if Rails.application.config.uses_legacy_database_config - warn "WARNING: This installation of GitLab uses a deprecated syntax for 'config/database.yml'. " \ - "The support for this syntax will be removed in 15.0. " \ - "More information can be found here: https://gitlab.com/gitlab-org/gitlab/-/issues/338182" -end - if configurations = ActiveRecord::Base.configurations.configurations if configurations.first.name != Gitlab::Database::MAIN_DATABASE_NAME raise "ERROR: This installation of GitLab uses unsupported 'config/database.yml'. " \ diff --git a/ee/spec/lib/gitlab/patch/database_config_spec.rb b/ee/spec/lib/gitlab/patch/database_config_spec.rb index 5095a20d4b69c..34223cf07b05b 100644 --- a/ee/spec/lib/gitlab/patch/database_config_spec.rb +++ b/ee/spec/lib/gitlab/patch/database_config_spec.rb @@ -38,50 +38,10 @@ .and_return(instance_double('Pathname', read: database_geo_yml)) end - context 'when config/database_geo.yml use a new syntax' do - let(:database_geo_yml) do - <<-EOS - production: - geo: - adapter: postgresql - encoding: unicode - database: gitlabhq_geo_production - username: git - password: "secure password" - host: localhost - - development: - geo: - adapter: postgresql - encoding: unicode - database: gitlabhq_geo_development - username: postgres - password: "secure password" - host: localhost - variables: - statement_timeout: 15s - - test: &test - geo: - adapter: postgresql - encoding: unicode - database: gitlabhq_geo_test - username: postgres - password: - host: localhost - prepared_statements: false - variables: - statement_timeout: 15s - EOS - end - - include_examples 'hash containing geo: connection name' - end - - context 'when config/database_geo.yml use a legacy syntax' do - let(:database_geo_yml) do - <<-EOS - production: + let(:database_geo_yml) do + <<-EOS + production: + geo: adapter: postgresql encoding: unicode database: gitlabhq_geo_production @@ -89,7 +49,8 @@ password: "secure password" host: localhost - development: + development: + geo: adapter: postgresql encoding: unicode database: gitlabhq_geo_development @@ -99,7 +60,8 @@ variables: statement_timeout: 15s - test: &test + test: &test + geo: adapter: postgresql encoding: unicode database: gitlabhq_geo_test @@ -109,11 +71,10 @@ prepared_statements: false variables: statement_timeout: 15s - EOS - end - - include_examples 'hash containing geo: connection name' + EOS end + + include_examples 'hash containing geo: connection name' end end @@ -175,10 +136,9 @@ allow(File).to receive(:exist?).with(Rails.root.join("config/database_geo.yml")).and_return(false) end - context 'when config/database.yml use a new syntax' do - context 'and does not contain Geo settings' do - let(:database_yml) do - <<-EOS + context 'and does not contain Geo settings' do + let(:database_yml) do + <<-EOS production: main: adapter: postgresql @@ -210,15 +170,15 @@ prepared_statements: false variables: statement_timeout: 15s - EOS - end - - include_examples 'hash containing main: connection name' + EOS end - context 'contains Geo settings' do - let(:database_yml) do - <<-EOS + include_examples 'hash containing main: connection name' + end + + context 'contains Geo settings' do + let(:database_yml) do + <<-EOS production: main: adapter: postgresql @@ -276,54 +236,10 @@ prepared_statements: false variables: statement_timeout: 15s - EOS - end - - include_examples 'hash containing both main: and geo: connection names' - end - end - - context 'when config/database.yml use a legacy syntax' do - let(:database_yml) do - <<-EOS - production: - adapter: postgresql - encoding: unicode - database: gitlabhq_production - username: git - password: "secure password" - host: localhost - - development: - adapter: postgresql - encoding: unicode - database: gitlabhq_development - username: postgres - password: "secure password" - host: localhost - variables: - statement_timeout: 15s - - test: &test - adapter: postgresql - encoding: unicode - database: gitlabhq_test - username: postgres - password: - host: localhost - prepared_statements: false - variables: - statement_timeout: 15s EOS end - include_examples 'hash containing main: connection name' - - it 'configuration is legacy' do - configuration.database_configuration - - expect(configuration.uses_legacy_database_config).to eq(true) - end + include_examples 'hash containing both main: and geo: connection names' end end @@ -373,10 +289,9 @@ # rubocop:enable RSpec/AnyInstanceOf end - context 'when config/database.yml use a new syntax' do - context 'and does not contain Geo setting' do - let(:database_yml) do - <<-EOS + context 'and does not contain Geo setting' do + let(:database_yml) do + <<-EOS production: main: adapter: postgresql @@ -408,15 +323,15 @@ prepared_statements: false variables: statement_timeout: 15s - EOS - end - - include_examples 'hash containing both main: and geo: connection names' + EOS end - context 'contains Geo setting' do - let(:database_yml) do - <<-EOS + include_examples 'hash containing both main: and geo: connection names' + end + + context 'contains Geo setting' do + let(:database_yml) do + <<-EOS production: main: adapter: postgresql @@ -474,44 +389,6 @@ prepared_statements: false variables: statement_timeout: 15s - EOS - end - - include_examples 'hash containing both main: and geo: connection names' - end - end - - context 'when config/database.yml use a legacy syntax' do - let(:database_yml) do - <<-EOS - production: - adapter: postgresql - encoding: unicode - database: gitlabhq_production - username: git - password: "secure password" - host: localhost - - development: - adapter: postgresql - encoding: unicode - database: gitlabhq_development - username: postgres - password: "secure password" - host: localhost - variables: - statement_timeout: 15s - - test: &test - adapter: postgresql - encoding: unicode - database: gitlabhq_test - username: postgres - password: - host: localhost - prepared_statements: false - variables: - statement_timeout: 15s EOS end diff --git a/lib/gitlab/patch/database_config.rb b/lib/gitlab/patch/database_config.rb index 702e8d404b128..c5c73d505187b 100644 --- a/lib/gitlab/patch/database_config.rb +++ b/lib/gitlab/patch/database_config.rb @@ -31,10 +31,6 @@ module Patch module DatabaseConfig extend ActiveSupport::Concern - prepended do - attr_reader :uses_legacy_database_config - end - def load_database_yaml return super unless Gitlab.ee? @@ -70,24 +66,7 @@ def load_geo_database_yaml end def database_configuration - @uses_legacy_database_config = false # rubocop:disable Gitlab/ModuleWithInstanceVariables - super.to_h do |env, configs| - # TODO: To be removed in 15.0. See https://gitlab.com/gitlab-org/gitlab/-/issues/338182 - # This preload is needed to convert legacy `database.yml` - # from `production: adapter: postgresql` - # into a `production: main: adapter: postgresql` - unless Gitlab::Utils.to_boolean(ENV['SKIP_DATABASE_CONFIG_VALIDATION'], default: false) - # This check is taken from Rails where the transformation - # of a flat database.yml is done into `primary:` - # https://github.com/rails/rails/blob/v6.1.4/activerecord/lib/active_record/database_configurations.rb#L169 - if configs.is_a?(Hash) && !configs.all? { |_, v| v.is_a?(Hash) } - configs = { "main" => configs } - - @uses_legacy_database_config = true # rubocop:disable Gitlab/ModuleWithInstanceVariables - end - end - if Gitlab.ee? if !configs.key?("geo") && File.exist?(Rails.root.join("config/database_geo.yml")) configs["geo"] = Rails.application.config_for(:database_geo).stringify_keys diff --git a/spec/initializers/validate_database_config_spec.rb b/spec/initializers/validate_database_config_spec.rb index 209d96913507b..5f3f950a852cc 100644 --- a/spec/initializers/validate_database_config_spec.rb +++ b/spec/initializers/validate_database_config_spec.rb @@ -39,47 +39,23 @@ end context 'when config/database.yml is valid' do - context 'uses legacy syntax' do - let(:database_yml) do - <<-EOS - production: + let(:database_yml) do + <<-EOS + production: + main: adapter: postgresql encoding: unicode database: gitlabhq_production username: git password: "secure password" host: localhost - EOS - end - - it 'validates configuration with a warning' do - expect(main_object).to receive(:warn).with /uses a deprecated syntax for/ - - expect { subject }.not_to raise_error - end - - it_behaves_like 'with SKIP_DATABASE_CONFIG_VALIDATION=true' + EOS end - context 'uses new syntax' do - let(:database_yml) do - <<-EOS - production: - main: - adapter: postgresql - encoding: unicode - database: gitlabhq_production - username: git - password: "secure password" - host: localhost - EOS - end + it 'validates configuration without errors and warnings' do + expect(main_object).not_to receive(:warn) - it 'validates configuration without errors and warnings' do - expect(main_object).not_to receive(:warn) - - expect { subject }.not_to raise_error - end + expect { subject }.not_to raise_error end end diff --git a/spec/lib/gitlab/patch/database_config_spec.rb b/spec/lib/gitlab/patch/database_config_spec.rb index d6f36ab86d500..73dc84bb2ef29 100644 --- a/spec/lib/gitlab/patch/database_config_spec.rb +++ b/spec/lib/gitlab/patch/database_config_spec.rb @@ -34,9 +34,8 @@ end end - context 'when a new syntax is used' do - let(:database_yml) do - <<-EOS + let(:database_yml) do + <<-EOS production: main: adapter: postgresql @@ -68,59 +67,9 @@ prepared_statements: false variables: statement_timeout: 15s - EOS - end - - include_examples 'hash containing main: connection name' - - it 'configuration is not legacy one' do - configuration.database_configuration - - expect(configuration.uses_legacy_database_config).to eq(false) - end + EOS end - context 'when a legacy syntax is used' do - let(:database_yml) do - <<-EOS - production: - adapter: postgresql - encoding: unicode - database: gitlabhq_production - username: git - password: "secure password" - host: localhost - - development: - adapter: postgresql - encoding: unicode - database: gitlabhq_development - username: postgres - password: "secure password" - host: localhost - variables: - statement_timeout: 15s - - test: &test - adapter: postgresql - encoding: unicode - database: gitlabhq_test - username: postgres - password: - host: localhost - prepared_statements: false - variables: - statement_timeout: 15s - EOS - end - - include_examples 'hash containing main: connection name' - - it 'configuration is legacy' do - configuration.database_configuration - - expect(configuration.uses_legacy_database_config).to eq(true) - end - end + include_examples 'hash containing main: connection name' end end -- GitLab