Skip to content

Commit 17d5563

Browse files
committed
fix protected env check for multi-db apps
Similarly to rails#46336, this change brings the `db:check_protected_environments` task up to parity with the rest of tasks that support multi-database applications. Today, misconfiguration of non-primary databases in multi-db applications is not checked against the `ar_internal_metadata` table's `environment` row, posing a risk to production environments. Similarly to my other PRs (rails#46097, rails#46336), we iterate through each of the relevant database configurations and re-establish `ActiveRecord::Base` using each configuration. This is safe to do since the `check_protected_environments!` method is only ever called from the `databases.rake` file/tasks. **A hidden change, but critical (for the purposes that I'm working on), that is happening here** is that database configurations with `database_tasks: false` will be excluded from the logic in this task – due to the usage of the `ActiveRecord::Base.configurations.configs_for` method, which by default has `include_hidden: false`!
1 parent 656118b commit 17d5563

File tree

4 files changed

+119
-19
lines changed

4 files changed

+119
-19
lines changed

activerecord/lib/active_record/tasks/database_tasks.rb

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -60,20 +60,16 @@ module DatabaseTasks
6060

6161
LOCAL_HOSTS = ["127.0.0.1", "localhost"]
6262

63-
def check_protected_environments!
64-
unless ENV["DISABLE_DATABASE_ENVIRONMENT_CHECK"]
65-
current = ActiveRecord::Base.connection.migration_context.current_environment
66-
stored = ActiveRecord::Base.connection.migration_context.last_stored_environment
63+
def check_protected_environments!(environment = env)
64+
return if ENV["DISABLE_DATABASE_ENVIRONMENT_CHECK"]
6765

68-
if ActiveRecord::Base.connection.migration_context.protected_environment?
69-
raise ActiveRecord::ProtectedEnvironmentError.new(stored)
70-
end
71-
72-
if stored && stored != current
73-
raise ActiveRecord::EnvironmentMismatchError.new(current: current, stored: stored)
74-
end
66+
original_db_config = ActiveRecord::Base.connection_db_config
67+
configs_for(env_name: environment).each do |db_config|
68+
ActiveRecord::Base.establish_connection(db_config)
69+
check_current_protected_environment!
7570
end
76-
rescue ActiveRecord::NoDatabaseError
71+
ensure
72+
ActiveRecord::Base.establish_connection(original_db_config)
7773
end
7874

7975
def register_task(pattern, task)
@@ -589,6 +585,20 @@ def structure_load_flags_for(adapter)
589585
structure_load_flags
590586
end
591587
end
588+
589+
def check_current_protected_environment!
590+
current = ActiveRecord::Base.connection.migration_context.current_environment
591+
stored = ActiveRecord::Base.connection.migration_context.last_stored_environment
592+
593+
if ActiveRecord::Base.connection.migration_context.protected_environment?
594+
raise ActiveRecord::ProtectedEnvironmentError.new(stored)
595+
end
596+
597+
if stored && stored != current
598+
raise ActiveRecord::EnvironmentMismatchError.new(current: current, stored: stored)
599+
end
600+
rescue ActiveRecord::NoDatabaseError
601+
end
592602
end
593603
end
594604
end

activerecord/lib/active_record/tasks/sqlite_database_tasks.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ def drop
3030
end
3131

3232
def purge
33-
drop
3433
connection.disconnect!
34+
drop
3535
rescue NoDatabaseError
3636
ensure
3737
create

activerecord/test/cases/tasks/database_tasks_test.rb

Lines changed: 93 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,17 @@ def assert_called_for_configs(method_name, configs, &block)
5555
sqlite3: :sqlite_tasks
5656
}
5757

58-
class DatabaseTasksUtilsTask < ActiveRecord::TestCase
58+
class DatabaseTasksCheckProtectedEnvironmentsTest < ActiveRecord::TestCase
59+
self.use_transactional_tests = false
60+
61+
def setup
62+
recreate_metadata_tables
63+
end
64+
65+
def teardown
66+
recreate_metadata_tables
67+
end
68+
5969
def test_raises_an_error_when_called_with_protected_environment
6070
protected_environments = ActiveRecord::Base.protected_environments
6171
current_env = ActiveRecord::Base.connection.migration_context.current_environment
@@ -70,12 +80,12 @@ def test_raises_an_error_when_called_with_protected_environment
7080
) do
7181
assert_not_includes protected_environments, current_env
7282
# Assert no error
73-
ActiveRecord::Tasks::DatabaseTasks.check_protected_environments!
83+
ActiveRecord::Tasks::DatabaseTasks.check_protected_environments!("arunit")
7484

7585
ActiveRecord::Base.protected_environments = [current_env]
7686

7787
assert_raise(ActiveRecord::ProtectedEnvironmentError) do
78-
ActiveRecord::Tasks::DatabaseTasks.check_protected_environments!
88+
ActiveRecord::Tasks::DatabaseTasks.check_protected_environments!("arunit")
7989
end
8090
end
8191
ensure
@@ -96,11 +106,11 @@ def test_raises_an_error_when_called_with_protected_environment_which_name_is_a_
96106
) do
97107
assert_not_includes protected_environments, current_env
98108
# Assert no error
99-
ActiveRecord::Tasks::DatabaseTasks.check_protected_environments!
109+
ActiveRecord::Tasks::DatabaseTasks.check_protected_environments!("arunit")
100110

101111
ActiveRecord::Base.protected_environments = [current_env.to_sym]
102112
assert_raise(ActiveRecord::ProtectedEnvironmentError) do
103-
ActiveRecord::Tasks::DatabaseTasks.check_protected_environments!
113+
ActiveRecord::Tasks::DatabaseTasks.check_protected_environments!("arunit")
104114
end
105115
end
106116
ensure
@@ -119,12 +129,89 @@ def test_raises_an_error_if_no_migrations_have_been_made
119129
assert_not_predicate internal_metadata, :table_exists?
120130

121131
assert_raises(ActiveRecord::NoEnvironmentInSchemaError) do
122-
ActiveRecord::Tasks::DatabaseTasks.check_protected_environments!
132+
ActiveRecord::Tasks::DatabaseTasks.check_protected_environments!("arunit")
123133
end
124134
ensure
125135
schema_migration.delete_version("1")
126136
internal_metadata.create_table
127137
end
138+
139+
private
140+
def recreate_metadata_tables
141+
schema_migration = ActiveRecord::Base.connection.schema_migration
142+
schema_migration.drop_table
143+
schema_migration.create_table
144+
145+
internal_metadata = ActiveRecord::Base.connection.internal_metadata
146+
internal_metadata.drop_table
147+
internal_metadata.create_table
148+
end
149+
end
150+
151+
if current_adapter?(:SQLite3Adapter) && !in_memory_db?
152+
class DatabaseTasksCheckProtectedEnvironmentsMultiDatabaseTest < ActiveRecord::TestCase
153+
self.use_transactional_tests = false
154+
155+
def test_with_multiple_databases
156+
env = ActiveRecord::ConnectionHandling::DEFAULT_ENV.call
157+
with_multi_db_configurations(env) do
158+
protected_environments = ActiveRecord::Base.protected_environments
159+
current_env = ActiveRecord::Base.connection.migration_context.current_environment
160+
assert_equal current_env, env
161+
162+
ActiveRecord::Base.establish_connection(:primary)
163+
ActiveRecord::Base.connection.internal_metadata.create_table_and_set_flags(current_env)
164+
165+
ActiveRecord::Base.establish_connection(:secondary)
166+
ActiveRecord::Base.connection.internal_metadata.create_table_and_set_flags(current_env)
167+
168+
assert_not_includes protected_environments, current_env
169+
# Assert not raises
170+
ActiveRecord::Tasks::DatabaseTasks.check_protected_environments!("test")
171+
172+
ActiveRecord::Base.establish_connection(:secondary)
173+
connection = ActiveRecord::Base.connection
174+
schema_migration = connection.schema_migration
175+
schema_migration.create_table
176+
schema_migration.create_version("1")
177+
178+
ActiveRecord::Base.protected_environments = [current_env.to_sym]
179+
assert_raise(ActiveRecord::ProtectedEnvironmentError) do
180+
ActiveRecord::Tasks::DatabaseTasks.check_protected_environments!("test")
181+
end
182+
ensure
183+
ActiveRecord::Base.protected_environments = protected_environments
184+
end
185+
end
186+
187+
private
188+
def with_multi_db_configurations(env)
189+
old_configurations = ActiveRecord::Base.configurations
190+
ActiveRecord::Base.configurations = {
191+
env => {
192+
primary: {
193+
adapter: "sqlite3",
194+
database: "test/fixtures/fixture_database.sqlite3",
195+
},
196+
secondary: {
197+
adapter: "sqlite3",
198+
database: "test/fixtures/fixture_database_2.sqlite3",
199+
}
200+
}
201+
}
202+
203+
ActiveRecord::Base.establish_connection(:primary)
204+
yield
205+
ensure
206+
[:primary, :secondary].each do |db|
207+
ActiveRecord::Base.establish_connection(db)
208+
ActiveRecord::Base.connection.schema_migration.drop_table
209+
ActiveRecord::Base.connection.internal_metadata.drop_table
210+
end
211+
ActiveRecord::Base.configurations = old_configurations
212+
ActiveRecord::Base.establish_connection(:arunit)
213+
end
214+
end
128215
end
129216

130217
class DatabaseTasksRegisterTask < ActiveRecord::TestCase

railties/test/application/rake/multi_dbs_test.rb

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -443,6 +443,9 @@ def generate_models_for_animals
443443
require "#{app_path}/config/environment"
444444
ActiveRecord::Base.configurations.configs_for(env_name: Rails.env).each do |db_config|
445445
db_create_and_drop_namespace db_config.name, db_config.database
446+
ensure
447+
# secondary databases might have been created by check_protected_environments task
448+
rails("db:drop:all")
446449
end
447450
end
448451

0 commit comments

Comments
 (0)