1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00

Prevent polluting ENV with postgres vars

This commit is contained in:
Samuel Cochran 2021-06-24 16:14:36 +10:00
parent 93b0b30821
commit 8478b26485
No known key found for this signature in database
GPG key ID: 1A36ACA1BDECD23E
3 changed files with 59 additions and 23 deletions

View file

@ -1,3 +1,13 @@
* Prevent polluting ENV during postgresql structure dump/load
Some configuration parameters were provided to pg_dump / psql via
environment variables which persisted beyond the command being run, and may
have caused subsequent commands and connections to fail. Tasks running
across multiple postgresql databases like `rails db:test:prepare` may have
been affected.
*Samuel Cochran*
* Set precision 6 by default for `datetime` columns
By default, datetime columns will have microseconds precision instead of seconds precision.

View file

@ -47,8 +47,6 @@ module ActiveRecord
end
def structure_dump(filename, extra_flags)
set_psql_env
search_path = \
case ActiveRecord.dump_schemas
when :schema_search_path
@ -79,7 +77,6 @@ module ActiveRecord
end
def structure_load(filename, extra_flags)
set_psql_env
args = ["--set", ON_ERROR_STOP_1, "--quiet", "--no-psqlrc", "--file", filename]
args.concat(Array(extra_flags)) if extra_flags
args << db_config.database
@ -100,15 +97,17 @@ module ActiveRecord
)
end
def set_psql_env
ENV["PGHOST"] = db_config.host if db_config.host
ENV["PGPORT"] = configuration_hash[:port].to_s if configuration_hash[:port]
ENV["PGPASSWORD"] = configuration_hash[:password].to_s if configuration_hash[:password]
ENV["PGUSER"] = configuration_hash[:username].to_s if configuration_hash[:username]
def psql_env
{}.tap do |env|
env["PGHOST"] = db_config.host if db_config.host
env["PGPORT"] = configuration_hash[:port].to_s if configuration_hash[:port]
env["PGPASSWORD"] = configuration_hash[:password].to_s if configuration_hash[:password]
env["PGUSER"] = configuration_hash[:username].to_s if configuration_hash[:username]
end
end
def run_cmd(cmd, args, action)
fail run_cmd_error(cmd, args, action) unless Kernel.system(cmd, *args)
fail run_cmd_error(cmd, args, action) unless Kernel.system(psql_env, cmd, *args)
end
def run_cmd_error(cmd, args, action)

View file

@ -361,7 +361,7 @@ if current_adapter?(:PostgreSQLAdapter)
assert_called_with(
Kernel,
:system,
["pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--file", @filename, "my-app-db"],
[{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--file", @filename, "my-app-db"],
returns: true
) do
ActiveRecord::Tasks::DatabaseTasks.structure_dump(@configuration, @filename)
@ -377,8 +377,20 @@ if current_adapter?(:PostgreSQLAdapter)
end
end
def test_structure_dump_with_env
expected_env = { "PGHOST" => "my.server.tld", "PGPORT" => "2345", "PGUSER" => "jane", "PGPASSWORD" => "s3cr3t" }
expected_command = [expected_env, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--file", @filename, "my-app-db"]
assert_called_with(Kernel, :system, expected_command, returns: true) do
ActiveRecord::Tasks::DatabaseTasks.structure_dump(
@configuration.merge(host: "my.server.tld", port: 2345, username: "jane", password: "s3cr3t"),
@filename
)
end
end
def test_structure_dump_with_extra_flags
expected_command = ["pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--file", @filename, "--noop", "my-app-db"]
expected_command = [{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--file", @filename, "--noop", "my-app-db"]
assert_called_with(Kernel, :system, expected_command, returns: true) do
with_structure_dump_flags(["--noop"]) do
@ -388,7 +400,7 @@ if current_adapter?(:PostgreSQLAdapter)
end
def test_structure_dump_with_hash_extra_flags_for_a_different_driver
expected_command = ["pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--file", @filename, "my-app-db"]
expected_command = [{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--file", @filename, "my-app-db"]
assert_called_with(Kernel, :system, expected_command, returns: true) do
with_structure_dump_flags({ mysql2: ["--noop"] }) do
@ -398,7 +410,7 @@ if current_adapter?(:PostgreSQLAdapter)
end
def test_structure_dump_with_hash_extra_flags_for_the_correct_driver
expected_command = ["pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--file", @filename, "--noop", "my-app-db"]
expected_command = [{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--file", @filename, "--noop", "my-app-db"]
assert_called_with(Kernel, :system, expected_command, returns: true) do
with_structure_dump_flags({ postgresql: ["--noop"] }) do
@ -416,7 +428,7 @@ if current_adapter?(:PostgreSQLAdapter)
assert_called_with(
Kernel,
:system,
["pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--file", @filename, "-T", "foo", "-T", "bar", "my-app-db"],
[{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--file", @filename, "-T", "foo", "-T", "bar", "my-app-db"],
returns: true
) do
ActiveRecord::Tasks::DatabaseTasks.structure_dump(@configuration, @filename)
@ -430,7 +442,7 @@ if current_adapter?(:PostgreSQLAdapter)
assert_called_with(
Kernel,
:system,
["pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--file", @filename, "--schema=foo", "--schema=bar", "my-app-db"],
[{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--file", @filename, "--schema=foo", "--schema=bar", "my-app-db"],
returns: true
) do
ActiveRecord::Tasks::DatabaseTasks.structure_dump(@configuration, @filename)
@ -443,7 +455,7 @@ if current_adapter?(:PostgreSQLAdapter)
assert_called_with(
Kernel,
:system,
["pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--file", @filename, "my-app-db"],
[{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--file", @filename, "my-app-db"],
returns: true
) do
with_dump_schemas(:all) do
@ -456,7 +468,7 @@ if current_adapter?(:PostgreSQLAdapter)
assert_called_with(
Kernel,
:system,
["pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--file", @filename, "--schema=foo", "--schema=bar", "my-app-db"],
[{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--file", @filename, "--schema=foo", "--schema=bar", "my-app-db"],
returns: true
) do
with_dump_schemas("foo,bar") do
@ -470,7 +482,7 @@ if current_adapter?(:PostgreSQLAdapter)
assert_called_with(
Kernel,
:system,
["pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--file", filename, "my-app-db"],
[{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--file", filename, "my-app-db"],
returns: nil
) do
e = assert_raise(RuntimeError) do
@ -511,7 +523,7 @@ if current_adapter?(:PostgreSQLAdapter)
assert_called_with(
Kernel,
:system,
["psql", "--set", "ON_ERROR_STOP=1", "--quiet", "--no-psqlrc", "--file", filename, @configuration["database"]],
[{}, "psql", "--set", "ON_ERROR_STOP=1", "--quiet", "--no-psqlrc", "--file", filename, @configuration["database"]],
returns: true
) do
ActiveRecord::Tasks::DatabaseTasks.structure_load(@configuration, filename)
@ -520,7 +532,7 @@ if current_adapter?(:PostgreSQLAdapter)
def test_structure_load_with_extra_flags
filename = "awesome-file.sql"
expected_command = ["psql", "--set", "ON_ERROR_STOP=1", "--quiet", "--no-psqlrc", "--file", filename, "--noop", @configuration["database"]]
expected_command = [{}, "psql", "--set", "ON_ERROR_STOP=1", "--quiet", "--no-psqlrc", "--file", filename, "--noop", @configuration["database"]]
assert_called_with(Kernel, :system, expected_command, returns: true) do
with_structure_load_flags(["--noop"]) do
@ -529,9 +541,24 @@ if current_adapter?(:PostgreSQLAdapter)
end
end
def test_structure_load_with_env
filename = "awesome-file.sql"
expected_env = { "PGHOST" => "my.server.tld", "PGPORT" => "2345", "PGUSER" => "jane", "PGPASSWORD" => "s3cr3t" }
expected_command = [expected_env, "psql", "--set", "ON_ERROR_STOP=1", "--quiet", "--no-psqlrc", "--file", filename, "--noop", @configuration["database"]]
assert_called_with(Kernel, :system, expected_command, returns: true) do
with_structure_load_flags(["--noop"]) do
ActiveRecord::Tasks::DatabaseTasks.structure_load(
@configuration.merge(host: "my.server.tld", port: 2345, username: "jane", password: "s3cr3t"),
filename
)
end
end
end
def test_structure_load_with_hash_extra_flags_for_a_different_driver
filename = "awesome-file.sql"
expected_command = ["psql", "--set", "ON_ERROR_STOP=1", "--quiet", "--no-psqlrc", "--file", filename, @configuration["database"]]
expected_command = [{}, "psql", "--set", "ON_ERROR_STOP=1", "--quiet", "--no-psqlrc", "--file", filename, @configuration["database"]]
assert_called_with(Kernel, :system, expected_command, returns: true) do
with_structure_load_flags({ mysql2: ["--noop"] }) do
@ -542,7 +569,7 @@ if current_adapter?(:PostgreSQLAdapter)
def test_structure_load_with_hash_extra_flags_for_the_correct_driver
filename = "awesome-file.sql"
expected_command = ["psql", "--set", "ON_ERROR_STOP=1", "--quiet", "--no-psqlrc", "--file", filename, "--noop", @configuration["database"]]
expected_command = [{}, "psql", "--set", "ON_ERROR_STOP=1", "--quiet", "--no-psqlrc", "--file", filename, "--noop", @configuration["database"]]
assert_called_with(Kernel, :system, expected_command, returns: true) do
with_structure_load_flags({ postgresql: ["--noop"] }) do
@ -556,7 +583,7 @@ if current_adapter?(:PostgreSQLAdapter)
assert_called_with(
Kernel,
:system,
["psql", "--set", "ON_ERROR_STOP=1", "--quiet", "--no-psqlrc", "--file", filename, @configuration["database"]],
[{}, "psql", "--set", "ON_ERROR_STOP=1", "--quiet", "--no-psqlrc", "--file", filename, @configuration["database"]],
returns: true
) do
ActiveRecord::Tasks::DatabaseTasks.structure_load(@configuration, filename)