Geo migrations settings are included by default

Changelog: changed
EE: true

config/application.rb

@@ -69,18 +69,19 @@ module Gitlab
     require_dependency Rails.root.join('lib/gitlab/middleware/handle_malformed_strings')
     require_dependency Rails.root.join('lib/gitlab/middleware/rack_multipart_tempfile_factory')
     require_dependency Rails.root.join('lib/gitlab/runtime')
-    require_dependency Rails.root.join('lib/gitlab/patch/legacy_database_config')
+    require_dependency Rails.root.join('lib/gitlab/patch/database_config')
     require_dependency Rails.root.join('lib/gitlab/exceptions_app')
 
     config.exceptions_app = Gitlab::ExceptionsApp.new(Rails.public_path)
 
-    # To be removed in 15.0
-    # 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)
-      config.class.prepend(::Gitlab::Patch::LegacyDatabaseConfig)
-    end
+    # This preload is required to:
+    #
+    # 1. Convert legacy `database.yml`;
+    # 2. Include Geo post-deployment migrations settings;
+    #
+    # TODO: In 15.0, this preload can be wrapped in a Gitlab.ee block
+    #       since we don't need to convert legacy `database.yml` anymore.
+    config.class.prepend(::Gitlab::Patch::DatabaseConfig)
 
     # Settings in config/environments/* take precedence over those specified here.
     # Application configuration should go into files in config/initializers

ee/spec/lib/gitlab/patch/legacy_database_config_spec.rb → ee/spec/lib/gitlab/patch/database_config_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::Patch::LegacyDatabaseConfig do
+RSpec.describe Gitlab::Patch::DatabaseConfig do
   describe '#load_geo_database_yaml' do
     let(:configuration) { Rails::Application::Configuration.new(Rails.root) }
 
@@ -35,7 +35,7 @@ RSpec.describe Gitlab::Patch::LegacyDatabaseConfig do
 
         allow(Pathname)
           .to receive(:new).with(Rails.root.join('config/database_geo.yml'))
-          .and_return(double(read: database_geo_yml))
+          .and_return(instance_double('Pathname', read: database_geo_yml))
       end
 
       context 'when config/database_geo.yml use a new syntax' do
@@ -129,7 +129,6 @@ RSpec.describe Gitlab::Patch::LegacyDatabaseConfig do
       # rubocop:enable RSpec/AnyInstanceOf
     end
 
-    context 'when config/database_geo.yml does not exist' do
     shared_examples 'hash containing main: connection name' do
       it 'returns a hash containing only main:' do
         database_configuration = configuration.database_configuration
@@ -142,12 +141,42 @@ RSpec.describe Gitlab::Patch::LegacyDatabaseConfig do
       end
     end
 
+    shared_examples 'hash containing both main: and geo: connection names' do
+      it 'returns a hash containing both main: and geo:' do
+        database_configuration = configuration.database_configuration
+
+        expect(database_configuration).to match(
+          "production" => { "main" => a_hash_including("adapter"), "geo" => a_hash_including("adapter", "migrations_paths" => ["ee/db/geo/migrate", "ee/db/geo/post_migrate"], "schema_migrations_path" => "ee/db/geo/schema_migrations") },
+          "development" => { "main" => a_hash_including("adapter"), "geo" => a_hash_including("adapter" => "postgresql", "migrations_paths" => ["ee/db/geo/migrate", "ee/db/geo/post_migrate"], "schema_migrations_path" => "ee/db/geo/schema_migrations") },
+          "test" => { "main" => a_hash_including("adapter"), "geo" => a_hash_including("adapter" => "postgresql", "migrations_paths" => ["ee/db/geo/migrate", "ee/db/geo/post_migrate"], "schema_migrations_path" => "ee/db/geo/schema_migrations") }
+        )
+      end
+
+      context 'when SKIP_POST_DEPLOYMENT_MIGRATIONS environment variable set' do
+        before do
+          stub_env('SKIP_POST_DEPLOYMENT_MIGRATIONS', 'true')
+        end
+
+        it 'does not include Geo post deployment migrations path' do
+          database_configuration = configuration.database_configuration
+
+          expect(database_configuration).to match(
+            "production" => { "main" => a_hash_including("adapter"), "geo" => a_hash_including("adapter", "migrations_paths" => ["ee/db/geo/migrate"], "schema_migrations_path" => "ee/db/geo/schema_migrations") },
+            "development" => { "main" => a_hash_including("adapter"), "geo" => a_hash_including("adapter" => "postgresql", "migrations_paths" => ["ee/db/geo/migrate"], "schema_migrations_path" => "ee/db/geo/schema_migrations") },
+            "test" => { "main" => a_hash_including("adapter"), "geo" => a_hash_including("adapter" => "postgresql", "migrations_paths" => ["ee/db/geo/migrate"], "schema_migrations_path" => "ee/db/geo/schema_migrations") }
+          )
+        end
+      end
+    end
+
+    context 'when config/database_geo.yml does not exist' do
       before do
         allow(File).to receive(:exist?).and_call_original
         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
               production:
@@ -187,6 +216,73 @@ RSpec.describe Gitlab::Patch::LegacyDatabaseConfig do
           include_examples 'hash containing main: connection name'
         end
 
+        context 'contains Geo settings' do
+          let(:database_yml) do
+            <<-EOS
+              production:
+                main:
+                  adapter: postgresql
+                  encoding: unicode
+                  database: gitlabhq_production
+                  username: git
+                  password: "secure password"
+                  host: localhost
+                geo:
+                  adapter: postgresql
+                  encoding: unicode
+                  database: gitlabhq_geo_production
+                  username: git
+                  password: "secure password"
+                  host: localhost
+
+              development:
+                main:
+                  adapter: postgresql
+                  encoding: unicode
+                  database: gitlabhq_development
+                  username: postgres
+                  password: "secure password"
+                  host: localhost
+                  variables:
+                    statement_timeout: 15s
+                geo:
+                  adapter: postgresql
+                  encoding: unicode
+                  database: gitlabhq_geo_development
+                  username: postgres
+                  password: "secure password"
+                  host: localhost
+                  variables:
+                    statement_timeout: 15s
+
+              test: &test
+                main:
+                  adapter: postgresql
+                  encoding: unicode
+                  database: gitlabhq_test
+                  username: postgres
+                  password:
+                  host: localhost
+                  prepared_statements: false
+                  variables:
+                    statement_timeout: 15s
+                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 both main: and geo: connection names'
+        end
+      end
+
       context 'when config/database.yml use a legacy syntax' do
         let(:database_yml) do
           <<-EOS
@@ -268,18 +364,6 @@ RSpec.describe Gitlab::Patch::LegacyDatabaseConfig do
         EOS
       end
 
-      shared_examples 'hash containing both main: and geo: connection names' do
-        it 'returns a hash containing both main: and geo:' do
-          database_configuration = configuration.database_configuration
-
-          expect(database_configuration).to match(
-            "production" => { "main" => a_hash_including("adapter"), "geo" => a_hash_including("adapter") },
-            "development" => { "main" => a_hash_including("adapter"), "geo" => a_hash_including("adapter" => "postgresql") },
-            "test" => { "main" => a_hash_including("adapter"), "geo" => a_hash_including("adapter" => "postgresql") }
-          )
-        end
-      end
-
       before do
         # The `AS::ConfigurationFile` calls `read` in `def initialize`
         # thus we cannot use `allow_next_instance_of`
@@ -290,6 +374,7 @@ RSpec.describe Gitlab::Patch::LegacyDatabaseConfig do
       end
 
       context 'when config/database.yml use a new syntax' do
+        context 'and does not contain Geo setting' do
           let(:database_yml) do
             <<-EOS
               production:
@@ -329,6 +414,73 @@ RSpec.describe Gitlab::Patch::LegacyDatabaseConfig do
           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
+                  encoding: unicode
+                  database: gitlabhq_production
+                  username: git
+                  password: "secure password"
+                  host: localhost
+                geo:
+                  adapter: postgresql
+                  encoding: unicode
+                  database: gitlabhq_geo_production
+                  username: git
+                  password: "secure password"
+                  host: localhost
+
+              development:
+                main:
+                  adapter: postgresql
+                  encoding: unicode
+                  database: gitlabhq_development
+                  username: postgres
+                  password: "secure password"
+                  host: localhost
+                  variables:
+                    statement_timeout: 15s
+                geo:
+                  adapter: postgresql
+                  encoding: unicode
+                  database: gitlabhq_geo_development
+                  username: postgres
+                  password: "secure password"
+                  host: localhost
+                  variables:
+                    statement_timeout: 15s
+
+              test: &test
+                main:
+                  adapter: postgresql
+                  encoding: unicode
+                  database: gitlabhq_test
+                  username: postgres
+                  password:
+                  host: localhost
+                  prepared_statements: false
+                  variables:
+                    statement_timeout: 15s
+                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 both main: and geo: connection names'
+        end
+      end
+
       context 'when config/database.yml use a legacy syntax' do
         let(:database_yml) do
           <<-EOS

lib/gitlab/patch/legacy_database_config.rb → lib/gitlab/patch/database_config.rb

@@ -28,7 +28,7 @@
 
 module Gitlab
   module Patch
-    module LegacyDatabaseConfig
+    module DatabaseConfig
       extend ActiveSupport::Concern
 
       prepended do
@@ -73,6 +73,11 @@ module Gitlab
         @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
@@ -81,15 +86,21 @@ module Gitlab
 
               @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
+            end
 
-          if Gitlab.ee? && File.exist?(Rails.root.join("config/database_geo.yml"))
-            migrations_paths = ["ee/db/geo/migrate"]
+            if configs.key?("geo")
+              migrations_paths = Array(configs["geo"]["migrations_paths"])
+              migrations_paths << "ee/db/geo/migrate" if migrations_paths.empty?
               migrations_paths << "ee/db/geo/post_migrate" unless ENV['SKIP_POST_DEPLOYMENT_MIGRATIONS']
 
-            configs["geo"] =
-              Rails.application.config_for(:database_geo)
-                .merge(migrations_paths: migrations_paths, schema_migrations_path: "ee/db/geo/schema_migrations")
-                .stringify_keys
+              configs["geo"]["migrations_paths"] = migrations_paths.uniq
+              configs["geo"]["schema_migrations_path"] = "ee/db/geo/schema_migrations" if configs["geo"]["schema_migrations_path"].blank?
+            end
           end
 
           [env, configs]

spec/lib/gitlab/patch/legacy_database_config_spec.rb → spec/lib/gitlab/patch/database_config_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::Patch::LegacyDatabaseConfig do
+RSpec.describe Gitlab::Patch::DatabaseConfig do
   it 'module is included' do
     expect(Rails::Application::Configuration).to include(described_class)
   end