From 34d18d082ae446eccc537922d4e45126e4f486d4 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Fri, 18 Oct 2024 14:43:21 +0200 Subject: [PATCH 1/3] Make load_configuration pure side effect free methods This refactors load_configuration to return a simple hash and do the caching in the method configuration. --- definitions/features/candlepin_database.rb | 4 ++-- definitions/features/foreman_database.rb | 7 +++---- definitions/features/pulpcore_database.rb | 18 +++++++++--------- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/definitions/features/candlepin_database.rb b/definitions/features/candlepin_database.rb index 34374298b..3916d8009 100644 --- a/definitions/features/candlepin_database.rb +++ b/definitions/features/candlepin_database.rb @@ -19,7 +19,7 @@ def services end def configuration - @configuration || load_configuration + @configuration ||= load_configuration end def check_option_using_cpdb_help(option_name, parent_cmd = '') @@ -36,7 +36,7 @@ def load_configuration uri_regexp = %r{://(([^/:]*):?([^/]*))/([^?]*)\??(ssl=([^&]*))?} url = full_config['jpa.config.hibernate.connection.url'] uri = uri_regexp.match(url) - @configuration = { + { 'username' => full_config['jpa.config.hibernate.connection.username'], 'password' => full_config['jpa.config.hibernate.connection.password'], 'database' => uri[4], diff --git a/definitions/features/foreman_database.rb b/definitions/features/foreman_database.rb index a37df8a68..4a60f4358 100644 --- a/definitions/features/foreman_database.rb +++ b/definitions/features/foreman_database.rb @@ -12,7 +12,7 @@ class Features::ForemanDatabase < ForemanMaintain::Feature end def configuration - @configuration || load_configuration + @configuration ||= load_configuration end def config_files @@ -41,8 +41,7 @@ def load_configuration { 'production' => {} } end - @configuration = config['production'] - @configuration['host'] ||= 'localhost' - @configuration + config['production']['host'] ||= 'localhost' + config['production'] end end diff --git a/definitions/features/pulpcore_database.rb b/definitions/features/pulpcore_database.rb index 46c52eef3..24a1c5ab7 100644 --- a/definitions/features/pulpcore_database.rb +++ b/definitions/features/pulpcore_database.rb @@ -14,7 +14,7 @@ class Features::PulpcoreDatabase < ForemanMaintain::Feature end def configuration - @configuration || load_configuration + @configuration ||= load_configuration end def services @@ -34,13 +34,13 @@ def load_configuration manager_result = execute!(manager_command) db_config = JSON.parse(manager_result) - @configuration = {} - @configuration['adapter'] = 'postgresql' - @configuration['host'] = db_config['HOST'] - @configuration['port'] = db_config['PORT'] - @configuration['database'] = db_config['NAME'] - @configuration['username'] = db_config['USER'] - @configuration['password'] = db_config['PASSWORD'] - @configuration + { + 'adapter' => 'postgresql', + 'host' => db_config['HOST'], + 'port' => db_config['PORT'], + 'database' => db_config['NAME'], + 'username' => db_config['USER'], + 'password' => db_config['PASSWORD'], + } end end From 4c86ebb2a029668626e38b7b17281da093ff9ac9 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Fri, 18 Oct 2024 19:12:53 +0200 Subject: [PATCH 2/3] Refactor candlepin_database_test to be more explicit in testing --- definitions/features/candlepin_database.rb | 5 ++- .../features/candlepin_database_test.rb | 40 +++++++++---------- 2 files changed, 24 insertions(+), 21 deletions(-) diff --git a/definitions/features/candlepin_database.rb b/definitions/features/candlepin_database.rb index 3916d8009..aacc5c160 100644 --- a/definitions/features/candlepin_database.rb +++ b/definitions/features/candlepin_database.rb @@ -30,8 +30,11 @@ def check_option_using_cpdb_help(option_name, parent_cmd = '') private + def raw_config + File.read(CANDLEPIN_DB_CONFIG) + end + def load_configuration - raw_config = File.read(CANDLEPIN_DB_CONFIG) full_config = Hash[raw_config.scan(/(^[^#\n][^=]*)=(.*)/)] uri_regexp = %r{://(([^/:]*):?([^/]*))/([^?]*)\??(ssl=([^&]*))?} url = full_config['jpa.config.hibernate.connection.url'] diff --git a/test/definitions/features/candlepin_database_test.rb b/test/definitions/features/candlepin_database_test.rb index aa875d862..9c394a7a2 100644 --- a/test/definitions/features/candlepin_database_test.rb +++ b/test/definitions/features/candlepin_database_test.rb @@ -1,39 +1,39 @@ require 'test_helper' -require 'minitest/stub_const' describe Features::CandlepinDatabase do include DefinitionsTestHelper subject { Features::CandlepinDatabase.new } - let(:subject_ins) { Features::CandlepinDatabase.any_instance } let(:cp_config_dir) do File.expand_path('../../support', __dir__) end - def stub_with_ssl_config(&block) - Features::CandlepinDatabase.stub_const(:CANDLEPIN_DB_CONFIG, - cp_config_dir + '/candlepin_with_ssl.conf', &block) - end - - def stub_without_ssl_config(&block) - Features::CandlepinDatabase.stub_const(:CANDLEPIN_DB_CONFIG, - cp_config_dir + '/candlepin_without_ssl.conf', &block) + def stub_config(&block) + subject.stub(:raw_config, File.read(File.join(cp_config_dir, config)), &block) end describe '.configuration' do - it 'The url includes ssl attributes when ssl is enabled' do - stub_with_ssl_config do - url = subject.configuration['url'] - assert_includes url, 'ssl=true' - assert_includes url, 'sslrootcert=/usr/share/foreman/root.crt' + let(:configuration) { subject.configuration } + + describe 'with ssl' do + let(:config) { 'candlepin_with_ssl.conf' } + + it 'sets ssl to true' do + stub_config do + assert_includes configuration['url'], 'ssl=true' + assert configuration['ssl'] + end end end - it 'The url does not include ssl attributes when ssl is disabled' do - stub_without_ssl_config do - url = subject.configuration['url'] - refute_includes url, 'ssl=true' - refute_includes url, 'sslrootcert=/usr/share/foreman/root.crt' + describe 'without ssl' do + let(:config) { 'candlepin_without_ssl.conf' } + + it 'sets ssl to false' do + stub_config do + refute_includes configuration['url'], 'ssl=true' + refute configuration['ssl'] + end end end end From 2b6a059f01042cbfb76723c06c6bbdddeebe2d35 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Fri, 18 Oct 2024 14:50:00 +0200 Subject: [PATCH 3/3] Use URI and CGI to parse the Candlepin DB URL --- definitions/features/candlepin_database.rb | 24 +++++++++------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/definitions/features/candlepin_database.rb b/definitions/features/candlepin_database.rb index aacc5c160..2d5394f57 100644 --- a/definitions/features/candlepin_database.rb +++ b/definitions/features/candlepin_database.rb @@ -1,3 +1,6 @@ +require 'uri' +require 'cgi' + class Features::CandlepinDatabase < ForemanMaintain::Feature CANDLEPIN_DB_CONFIG = '/etc/candlepin/candlepin.conf'.freeze @@ -36,17 +39,17 @@ def raw_config def load_configuration full_config = Hash[raw_config.scan(/(^[^#\n][^=]*)=(.*)/)] - uri_regexp = %r{://(([^/:]*):?([^/]*))/([^?]*)\??(ssl=([^&]*))?} url = full_config['jpa.config.hibernate.connection.url'] - uri = uri_regexp.match(url) + uri = URI.parse(url.delete_prefix('jdbc:')) + query = uri.query ? CGI.parse(uri.query) : {} { 'username' => full_config['jpa.config.hibernate.connection.username'], 'password' => full_config['jpa.config.hibernate.connection.password'], - 'database' => uri[4], - 'host' => uri[2], - 'port' => uri[3] || '5432', - 'ssl' => (fetch_extra_param(url, 'ssl') == 'true'), - 'sslfactory' => fetch_extra_param(url, 'sslfactory'), + 'database' => uri.path, + 'host' => uri.host, + 'port' => uri.port || '5432', + 'ssl' => query['ssl']&.first == 'true', + 'sslfactory' => query['sslfactory']&.first, 'driver_class' => full_config['jpa.config.hibernate.connection.driver_class'], 'url' => url, } @@ -60,11 +63,4 @@ def extend_with_db_options end db_options end - - def fetch_extra_param(url, key_name) - query_string = url.split('?')[1] - return nil unless query_string - output = /#{key_name}=([^&]*)?/.match(query_string) - output[1] if output - end end