diff --git a/app/jobs/reports/base_report.rb b/app/jobs/reports/base_report.rb index 7d0f7c7cb1b..7fdd86877fd 100644 --- a/app/jobs/reports/base_report.rb +++ b/app/jobs/reports/base_report.rb @@ -31,8 +31,12 @@ def report_timeout IdentityConfig.store.report_timeout end - def transaction_with_timeout - Db::EstablishConnection::ReadReplicaConnection.new.call do + def transaction_with_timeout(rails_env = Rails.env) + # rspec-rails's use_transactional_tests does not seem to act as expected when switching + # connections mid-test, so we just skip for now :[ + return yield if rails_env.test? + + ApplicationRecord.connected_to(role: :reading, shard: :read_replica) do ActiveRecord::Base.transaction do quoted_timeout = ActiveRecord::Base.connection.quote(report_timeout) ActiveRecord::Base.connection.execute("SET LOCAL statement_timeout = #{quoted_timeout}") diff --git a/app/models/application_record.rb b/app/models/application_record.rb index 10a4cba84df..80a8b8c1f47 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -1,3 +1,12 @@ class ApplicationRecord < ActiveRecord::Base self.abstract_class = true + + connects_to shards: { + default: { writing: :primary, reading: :primary }, + read_replica: { + # writing to the read_replica won't work, but AR needs to have something here + writing: :read_replica, + reading: :read_replica, + }, + } end diff --git a/app/services/db/establish_connection/read_replica_connection.rb b/app/services/db/establish_connection/read_replica_connection.rb deleted file mode 100644 index 1fefbf7bd0a..00000000000 --- a/app/services/db/establish_connection/read_replica_connection.rb +++ /dev/null @@ -1,45 +0,0 @@ -module Db - module EstablishConnection - class ReadReplicaConnection - def call - return yield if Rails.env.test? - begin - ActiveRecord::Base.establish_connection(read_replica_connection_params) - yield - ensure - ActiveRecord::Base.establish_connection(primary_connection_params) - end - end - - private - - def read_replica_connection_params - { - adapter: 'postgresql', - database: database_name, - host: IdentityConfig.store.database_read_replica_host, - username: IdentityConfig.store.database_readonly_username, - password: IdentityConfig.store.database_readonly_password, - } - end - - def primary_connection_params - { - adapter: 'postgresql', - database: database_name, - host: IdentityConfig.store.database_host, - username: IdentityConfig.store.database_username, - password: IdentityConfig.store.database_password, - } - end - - def database_name - if Rails.env.production? - IdentityConfig.store.database_name - else - "upaya_#{Rails.env}" - end - end - end - end -end diff --git a/config/application.rb b/config/application.rb index f48612117c0..8e0824f9446 100644 --- a/config/application.rb +++ b/config/application.rb @@ -23,8 +23,23 @@ class Application < Rails::Application ) IdentityConfig.build_store(configuration) + console do + if ENV['ALLOW_CONSOLE_DB_WRITE_ACCESS'] != 'true' && + IdentityConfig.store.database_readonly_username.present? && + IdentityConfig.store.database_readonly_password.present? + warn <<-EOS.squish + WARNING: Loading database a configuration with the readonly database user. + If you wish to make changes to records in the database set + ALLOW_CONSOLE_DB_WRITE_ACCESS to "true" in the environment + EOS + + ActiveRecord::Base.establish_connection :read_replica + end + end + config.load_defaults '6.1' config.active_record.belongs_to_required_by_default = false + config.active_record.legacy_connection_handling = false config.assets.unknown_asset_fallback = true if IdentityConfig.store.ruby_workers_enabled diff --git a/config/database.yml b/config/database.yml index ffe46a4fbba..ace8021f68e 100644 --- a/config/database.yml +++ b/config/database.yml @@ -1,5 +1,3 @@ -<% require 'production_database_configuration' %> - postgresql: &postgresql adapter: postgresql encoding: utf8 @@ -24,22 +22,49 @@ defaults: &defaults statement_timeout: <%= IdentityConfig.store.database_statement_timeout %> development: - <<: *defaults + primary: + <<: *defaults + read_replica: + <<: *defaults + replica: true test: - <<: *defaults - pool: 10 - checkout_timeout: 10 - database: <%= ENV['POSTGRES_DB'] || "upaya_test#{ENV['TEST_ENV_NUMBER']}" %> - user: <%= ENV['POSTGRES_USER'] %> - password: <%= ENV['POSTGRES_PASSWORD'] %> + primary: &test + <<: *defaults + pool: 10 + checkout_timeout: 10 + database: <%= ENV['POSTGRES_DB'] || "upaya_test#{ENV['TEST_ENV_NUMBER']}" %> + user: <%= ENV['POSTGRES_USER'] %> + password: <%= ENV['POSTGRES_PASSWORD'] %> + read_replica: + <<: *test + replica: true + +<% + pool = if Identity::Hostdata.instance_role == 'worker' + IdentityConfig.store.good_job_max_threads + IdentityConfig.store.database_pool_idp + else + IdentityConfig.store.database_pool_idp + end +%> production: - <<: *defaults - database: <%= IdentityConfig.store.database_name %> - username: <%= ProductionDatabaseConfiguration.username %> - host: <%= ProductionDatabaseConfiguration.host %> - password: <%= ProductionDatabaseConfiguration.password %> - pool: <%= ProductionDatabaseConfiguration.pool %> - sslmode: 'verify-full' - sslrootcert: '/usr/local/share/aws/rds-combined-ca-bundle.pem' + primary: + <<: *defaults + database: <%= IdentityConfig.store.database_name %> + username: <%= IdentityConfig.store.database_username %> + host: <%= IdentityConfig.store.database_host %> + password: <%= IdentityConfig.store.database_password %> + pool: <%= pool %> + sslmode: 'verify-full' + sslrootcert: '/usr/local/share/aws/rds-combined-ca-bundle.pem' + read_replica: + <<: *defaults + database: <%= IdentityConfig.store.database_name %> + username: <%= IdentityConfig.store.database_readonly_username %> + host: <%= IdentityConfig.store.database_read_replica_host %> + password: <%= IdentityConfig.store.database_readonly_password %> + pool: <%= pool %> + sslmode: 'verify-full' + sslrootcert: '/usr/local/share/aws/rds-combined-ca-bundle.pem' + replica: true diff --git a/lib/production_database_configuration.rb b/lib/production_database_configuration.rb deleted file mode 100644 index 821d2c715e6..00000000000 --- a/lib/production_database_configuration.rb +++ /dev/null @@ -1,56 +0,0 @@ -class ProductionDatabaseConfiguration - READONLY_WARNING_MESSAGE = ' - WARNING: Loading database a configuration with the readonly database user. - If you wish to make changes to records in the database set - ALLOW_CONSOLE_DB_WRITE_ACCESS to "true" in the environment - '.freeze.gsub(/^\s+/, '') - - def self.host - if readonly_mode? - raise if IdentityConfig.store.database_read_replica_host.blank? - IdentityConfig.store.database_read_replica_host - else - IdentityConfig.store.database_host - end - end - - def self.username - if readonly_mode? - raise if IdentityConfig.store.database_readonly_username.blank? - IdentityConfig.store.database_readonly_username - else - IdentityConfig.store.database_username - end - end - - def self.password - if readonly_mode? - raise if IdentityConfig.store.database_readonly_password.blank? - IdentityConfig.store.database_readonly_password - else - IdentityConfig.store.database_password - end - end - - def self.pool - IdentityConfig.store.database_pool_idp - end - - private_class_method def self.readonly_mode? - return false unless defined?(Rails::Console) - return false unless readonly_credentials_present? - return false if ENV['ALLOW_CONSOLE_DB_WRITE_ACCESS'] == 'true' - print_readonly_warning - true - end - - private_class_method def self.readonly_credentials_present? - IdentityConfig.store.database_readonly_username.present? && - IdentityConfig.store.database_readonly_password.present? - end - - private_class_method def self.print_readonly_warning - return if @readonly_warning.present? - warn @readonly_warning ||= READONLY_WARNING_MESSAGE - end -end diff --git a/spec/jobs/reports/base_report_spec.rb b/spec/jobs/reports/base_report_spec.rb new file mode 100644 index 00000000000..4e406d3347d --- /dev/null +++ b/spec/jobs/reports/base_report_spec.rb @@ -0,0 +1,22 @@ +require 'rails_helper' + +RSpec.describe Reports::BaseReport do + subject(:report) { described_class.new } + + describe '#transaction_with_timeout' do + let(:rails_env) { ActiveSupport::StringInquirer.new('production') } + let(:report_timeout) { 999 } + + before do + allow(IdentityConfig.store).to receive(:report_timeout).and_return(report_timeout) + end + + it 'sets the statement_timeout inside a transaction' do + result = report.send(:transaction_with_timeout, rails_env) do + ActiveRecord::Base.connection.execute('SHOW statement_timeout') + end + + expect(result.first['statement_timeout']).to eq("#{report_timeout}ms") + end + end +end diff --git a/spec/lib/production_database_configuration_spec.rb b/spec/lib/production_database_configuration_spec.rb deleted file mode 100644 index bae527181dd..00000000000 --- a/spec/lib/production_database_configuration_spec.rb +++ /dev/null @@ -1,178 +0,0 @@ -require 'rails_helper' - -describe ProductionDatabaseConfiguration do - let(:database_username) { 'db_user' } - let(:database_password) { 'db_pass' } - let(:database_readonly_username) { 'db_readonly_user' } - let(:database_readonly_password) { 'db_readonly_pass' } - let(:database_read_replica_host) { 'read_only_host' } - let(:database_host) { 'read_write_host' } - - before do - allow(IdentityConfig.store).to receive(:database_username).and_return(database_username) - allow(IdentityConfig.store).to receive(:database_password).and_return(database_password) - allow(IdentityConfig.store).to receive(:database_readonly_username).and_return( - database_readonly_username, - ) - allow(IdentityConfig.store).to receive(:database_readonly_password).and_return( - database_readonly_password, - ) - allow(ProductionDatabaseConfiguration).to receive(:warn) - allow(IdentityConfig.store).to receive(:database_read_replica_host).and_return( - database_read_replica_host, - ) - allow(IdentityConfig.store).to receive(:database_host).and_return(database_host) - end - - describe '.username' do - context 'when app is running in a console' do - before { stub_rails_console } - - it 'returns the readonly username' do - expect(ProductionDatabaseConfiguration.username).to eq(database_readonly_username) - end - end - - context 'when the app is running in a console without readonly user credentials' do - it 'returns the read/write username' do - allow(IdentityConfig.store).to receive(:database_readonly_username).and_return('') - allow(IdentityConfig.store).to receive(:database_readonly_password).and_return('') - - expect(ProductionDatabaseConfiguration.username).to eq(database_username) - end - end - - context 'when the app is running in a console with the write access flag' do - before do - stub_rails_console - stub_environment_write_access_flag - end - - it 'returns the read/write username' do - expect(ProductionDatabaseConfiguration.username).to eq(database_username) - end - end - - context 'when the app is not running in a console' do - it 'returns the read/write username' do - expect(ProductionDatabaseConfiguration.username).to eq(database_username) - end - end - end - - describe '.host' do - context 'when app is running in a console' do - before { stub_rails_console } - - it 'returns the readonly host' do - expect(ProductionDatabaseConfiguration.host).to eq(database_read_replica_host) - end - end - - context 'when the app is running in a console without readonly user credentials' do - it 'returns the read/write host' do - allow(IdentityConfig.store).to receive(:database_readonly_username).and_return('') - allow(IdentityConfig.store).to receive(:database_readonly_password).and_return('') - - expect(ProductionDatabaseConfiguration.host).to eq(database_host) - end - end - - context 'when the app is running in a console with the write access flag' do - before do - stub_rails_console - stub_environment_write_access_flag - end - - it 'returns the read/write host' do - expect(ProductionDatabaseConfiguration.host).to eq(database_host) - end - end - - context 'when the app is not running in a console' do - it 'returns the read/write host' do - expect(ProductionDatabaseConfiguration.host).to eq(database_host) - end - end - end - - describe '.password' do - context 'when app is running in a console' do - before { stub_rails_console } - - it 'returns the readonly password' do - expect(ProductionDatabaseConfiguration.password).to eq(database_readonly_password) - end - end - - context 'when the app is running in a console without readonly user credentials' do - it 'returns the read/write username' do - allow(IdentityConfig.store).to receive(:database_readonly_username).and_return('') - allow(IdentityConfig.store).to receive(:database_readonly_password).and_return('') - - expect(ProductionDatabaseConfiguration.password).to eq(database_password) - end - end - - context 'when the app is running in a console with the write access flag' do - before do - stub_rails_console - stub_environment_write_access_flag - end - - it 'returns the read/write password' do - expect(ProductionDatabaseConfiguration.password).to eq(database_password) - end - end - - context 'when the app is not running in a console' do - it 'returns the read/write password' do - expect(ProductionDatabaseConfiguration.password).to eq(database_password) - end - end - end - - describe '.pool' do - context 'when the app is running on an idp host' do - before { stub_role_config('idp') } - - it 'returns the idp pool size' do - allow(IdentityConfig.store).to receive(:database_pool_idp).and_return(7) - - expect(ProductionDatabaseConfiguration.pool).to eq(7) - end - end - - context 'when the app is running on an host with an ambigous role' do - before { stub_role_config('fake') } - - it 'returns a default of 5' do - expect(ProductionDatabaseConfiguration.pool).to eq(5) - end - end - - context 'when the app is running on a host without a role config file' do - before do - allow(File).to receive(:exist?).with('/etc/login.gov/info/role').and_return(false) - end - - it 'returns 5 and does not read the role config' do - expect(File).to_not receive(:read) - expect(ProductionDatabaseConfiguration.pool).to eq(5) - end - end - end - - def stub_rails_console - stub_const('Rails::Console', Object) - end - - def stub_environment_write_access_flag - allow(ENV).to receive(:[]).with('ALLOW_CONSOLE_DB_WRITE_ACCESS').and_return('true') - end - - def stub_role_config(role) - allow(File).to receive(:exist?).with('/etc/login.gov/info/role').and_return(true) - allow(File).to receive(:read).with('/etc/login.gov/info/role').and_return(role) - end -end