From 470a5eaf29bdaf8fc20d06b3830ebc58b3fc509a Mon Sep 17 00:00:00 2001
From: Evgeni Golov <evgeni@golov.de>
Date: Tue, 15 Oct 2024 12:13:00 +0200
Subject: [PATCH 01/14] allow passing env to CommandRunner

---
 lib/foreman_maintain/utils/command_runner.rb | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/lib/foreman_maintain/utils/command_runner.rb b/lib/foreman_maintain/utils/command_runner.rb
index 27a8ae19b..10ed2f041 100644
--- a/lib/foreman_maintain/utils/command_runner.rb
+++ b/lib/foreman_maintain/utils/command_runner.rb
@@ -8,8 +8,10 @@ class CommandRunner
       attr_reader :logger, :command
 
       def initialize(logger, command, options)
-        options.validate_options!(:stdin, :hidden_patterns, :interactive, :valid_exit_statuses)
+        options.validate_options!(:stdin, :hidden_patterns, :interactive, :valid_exit_statuses,
+          :env)
         options[:valid_exit_statuses] ||= [0]
+        options[:env] ||= {}
         @logger = logger
         @command = command
         @stdin = options[:stdin]
@@ -17,6 +19,7 @@ def initialize(logger, command, options)
         @interactive = options[:interactive]
         @options = options
         @valid_exit_statuses = options[:valid_exit_statuses]
+        @env = options[:env]
         raise ArgumentError, 'Can not pass stdin for interactive command' if @interactive && @stdin
       end
 
@@ -81,7 +84,7 @@ def run_interactively
       end
 
       def run_non_interactively
-        IO.popen(full_command, 'r+') do |f|
+        IO.popen(@env, full_command, 'r+') do |f|
           if @stdin
             f.puts(@stdin)
             f.close_write

From 747e89e0ff2fd985a1149ed75e0d2bbfcab7f147 Mon Sep 17 00:00:00 2001
From: Ewoud Kohl van Wijngaarden <ewoud@kohlvanwijngaarden.nl>
Date: Thu, 17 Oct 2024 14:32:29 +0200
Subject: [PATCH 02/14] Drop config parameter on database instance methods

The configuration is an instance level setting that is never passed in.
This simplifies the code by avoiding the need to pass it around.
---
 .../concerns/base_database.rb                 | 66 +++++++++----------
 1 file changed, 33 insertions(+), 33 deletions(-)

diff --git a/lib/foreman_maintain/concerns/base_database.rb b/lib/foreman_maintain/concerns/base_database.rb
index 18879b9c1..3570272b0 100644
--- a/lib/foreman_maintain/concerns/base_database.rb
+++ b/lib/foreman_maintain/concerns/base_database.rb
@@ -41,49 +41,49 @@ def configuration
         raise NotImplementedError
       end
 
-      def local?(config = configuration)
-        ['localhost', '127.0.0.1', `hostname`.strip].include?(config['host'])
+      def local?
+        ['localhost', '127.0.0.1', `hostname`.strip].include?(configuration['host'])
       end
 
-      def query(sql, config = configuration)
-        parse_csv(query_csv(sql, config))
+      def query(sql)
+        parse_csv(query_csv(sql))
       end
 
-      def query_csv(sql, config = configuration)
-        psql(%{COPY (#{sql}) TO STDOUT WITH CSV HEADER}, config)
+      def query_csv(sql)
+        psql(%{COPY (#{sql}) TO STDOUT WITH CSV HEADER})
       end
 
-      def psql(query, config = configuration)
-        if ping(config)
-          execute(psql_command(config),
+      def psql(query)
+        if ping
+          execute(psql_command,
             :stdin => query,
-            :hidden_patterns => [config['password']])
+            :hidden_patterns => [configuration['password']])
         else
           raise_service_error
         end
       end
 
-      def ping(config = configuration)
-        execute?(psql_command(config),
+      def ping
+        execute?(psql_command,
           :stdin => 'SELECT 1 as ping',
-          :hidden_patterns => [config['password']])
+          :hidden_patterns => [configuration['password']])
       end
 
-      def dump_db(file, config = configuration)
-        execute!(dump_command(config) + " > #{file}", :hidden_patterns => [config['password']])
+      def dump_db(file)
+        execute!(dump_command + " > #{file}", :hidden_patterns => [configuration['password']])
       end
 
-      def restore_dump(file, localdb, config = configuration)
+      def restore_dump(file, localdb)
         if localdb
           dump_cmd = "runuser - postgres -c 'pg_restore -C -d postgres #{file}'"
           execute!(dump_cmd)
         else
           # TODO: figure out how to completely ignore errors. Currently this
           # sometimes exits with 1 even though errors are ignored by pg_restore
-          dump_cmd = base_command(config, 'pg_restore') +
+          dump_cmd = base_command('pg_restore') +
                      ' --no-privileges --clean --disable-triggers -n public ' \
-                     "-d #{config['database']} #{file}"
-          execute!(dump_cmd, :hidden_patterns => [config['password']],
+                     "-d #{configuration['database']} #{file}"
+          execute!(dump_cmd, :hidden_patterns => [configuration['password']],
             :valid_exit_statuses => [0, 1])
         end
       end
@@ -109,9 +109,9 @@ def backup_dir
         @backup_dir ||= File.expand_path(ForemanMaintain.config.db_backup_dir)
       end
 
-      def dropdb(config = configuration)
+      def dropdb
         if local?
-          execute!("runuser - postgres -c 'dropdb #{config['database']}'")
+          execute!("runuser - postgres -c 'dropdb #{configuration['database']}'")
         else
           delete_statement = psql(<<-SQL)
             select string_agg('drop table if exists \"' || tablename || '\" cascade;', '')
@@ -122,11 +122,11 @@ def dropdb(config = configuration)
         end
       end
 
-      def db_version(config = configuration)
-        if ping(config)
+      def db_version
+        if ping
           # Note - t removes headers, -A removes alignment whitespace
-          server_version_cmd = psql_command(config) + ' -c "SHOW server_version" -t -A'
-          version_string = execute!(server_version_cmd, :hidden_patterns => [config['password']])
+          server_version_cmd = psql_command + ' -c "SHOW server_version" -t -A'
+          version_string = execute!(server_version_cmd, :hidden_patterns => [configuration['password']])
           version(version_string)
         else
           raise_service_error
@@ -145,18 +145,18 @@ def raise_psql_missing_error
 
       private
 
-      def base_command(config, command = 'psql')
-        "PGPASSWORD='#{config[%(password)]}' "\
-        "#{command} -h #{config['host'] || 'localhost'} "\
-        " -p #{config['port'] || '5432'} -U #{config['username']}"
+      def base_command(command = 'psql')
+        "PGPASSWORD='#{configuration[%(password)]}' "\
+        "#{command} -h #{configuration['host'] || 'localhost'} "\
+        " -p #{configuration['port'] || '5432'} -U #{configuration['username']}"
       end
 
-      def psql_command(config)
-        base_command(config, 'psql') + " -d #{config['database']}"
+      def psql_command
+        base_command('psql') + " -d #{configuration['database']}"
       end
 
-      def dump_command(config)
-        base_command(config, 'pg_dump') + " -Fc #{config['database']}"
+      def dump_command
+        base_command('pg_dump') + " -Fc #{configuration['database']}"
       end
 
       def raise_service_error

From d75f89b0a2c68d15ea4ba428471ed169d3048a54 Mon Sep 17 00:00:00 2001
From: Ewoud Kohl van Wijngaarden <ewoud@kohlvanwijngaarden.nl>
Date: Thu, 17 Oct 2024 14:53:15 +0200
Subject: [PATCH 03/14] Remove dump_command helper method that's only used once

---
 lib/foreman_maintain/concerns/base_database.rb | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/lib/foreman_maintain/concerns/base_database.rb b/lib/foreman_maintain/concerns/base_database.rb
index 3570272b0..82fa92bb0 100644
--- a/lib/foreman_maintain/concerns/base_database.rb
+++ b/lib/foreman_maintain/concerns/base_database.rb
@@ -70,6 +70,7 @@ def ping
       end
 
       def dump_db(file)
+        dump_command = base_command('pg_dump') + " -Fc #{configuration['database']}"
         execute!(dump_command + " > #{file}", :hidden_patterns => [configuration['password']])
       end
 
@@ -155,10 +156,6 @@ def psql_command
         base_command('psql') + " -d #{configuration['database']}"
       end
 
-      def dump_command
-        base_command('pg_dump') + " -Fc #{configuration['database']}"
-      end
-
       def raise_service_error
         raise Error::Fail, 'Please check whether database service is up & running state.'
       end

From 66cf3f72af782eccffcf0ec77be4e1d5c396dfc5 Mon Sep 17 00:00:00 2001
From: Ewoud Kohl van Wijngaarden <ewoud@kohlvanwijngaarden.nl>
Date: Thu, 17 Oct 2024 14:53:51 +0200
Subject: [PATCH 04/14] Use -f parameter for pg_dump

This avoids needing to use a shell.
---
 lib/foreman_maintain/concerns/base_database.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/foreman_maintain/concerns/base_database.rb b/lib/foreman_maintain/concerns/base_database.rb
index 82fa92bb0..cad95452e 100644
--- a/lib/foreman_maintain/concerns/base_database.rb
+++ b/lib/foreman_maintain/concerns/base_database.rb
@@ -70,8 +70,8 @@ def ping
       end
 
       def dump_db(file)
-        dump_command = base_command('pg_dump') + " -Fc #{configuration['database']}"
-        execute!(dump_command + " > #{file}", :hidden_patterns => [configuration['password']])
+        dump_command = base_command('pg_dump') + " -Fc #{configuration['database']} -f #{file}"
+        execute!(dump_command, :hidden_patterns => [configuration['password']])
       end
 
       def restore_dump(file, localdb)

From f701d8898ebd2c0b1519b47477fcad7b2b849ba8 Mon Sep 17 00:00:00 2001
From: Ewoud Kohl van Wijngaarden <ewoud@kohlvanwijngaarden.nl>
Date: Thu, 17 Oct 2024 15:00:42 +0200
Subject: [PATCH 05/14] Pass PostgreSQL password via environment variables

---
 lib/foreman_maintain/concerns/base_database.rb | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/lib/foreman_maintain/concerns/base_database.rb b/lib/foreman_maintain/concerns/base_database.rb
index cad95452e..ca0281adc 100644
--- a/lib/foreman_maintain/concerns/base_database.rb
+++ b/lib/foreman_maintain/concerns/base_database.rb
@@ -57,7 +57,7 @@ def psql(query)
         if ping
           execute(psql_command,
             :stdin => query,
-            :hidden_patterns => [configuration['password']])
+            :env => base_env)
         else
           raise_service_error
         end
@@ -66,12 +66,12 @@ def psql(query)
       def ping
         execute?(psql_command,
           :stdin => 'SELECT 1 as ping',
-          :hidden_patterns => [configuration['password']])
+          :env => base_env)
       end
 
       def dump_db(file)
         dump_command = base_command('pg_dump') + " -Fc #{configuration['database']} -f #{file}"
-        execute!(dump_command, :hidden_patterns => [configuration['password']])
+        execute!(dump_command, :env => base_env)
       end
 
       def restore_dump(file, localdb)
@@ -84,7 +84,7 @@ def restore_dump(file, localdb)
           dump_cmd = base_command('pg_restore') +
                      ' --no-privileges --clean --disable-triggers -n public ' \
                      "-d #{configuration['database']} #{file}"
-          execute!(dump_cmd, :hidden_patterns => [configuration['password']],
+          execute!(dump_cmd, :env => base_env,
             :valid_exit_statuses => [0, 1])
         end
       end
@@ -127,7 +127,7 @@ def db_version
         if ping
           # Note - t removes headers, -A removes alignment whitespace
           server_version_cmd = psql_command + ' -c "SHOW server_version" -t -A'
-          version_string = execute!(server_version_cmd, :hidden_patterns => [configuration['password']])
+          version_string = execute!(server_version_cmd, :env => base_env)
           version(version_string)
         else
           raise_service_error
@@ -146,8 +146,13 @@ def raise_psql_missing_error
 
       private
 
+      def base_env
+        {
+          'PGPASSWORD' => configuration['password'],
+        }
+      end
+
       def base_command(command = 'psql')
-        "PGPASSWORD='#{configuration[%(password)]}' "\
         "#{command} -h #{configuration['host'] || 'localhost'} "\
         " -p #{configuration['port'] || '5432'} -U #{configuration['username']}"
       end

From 4325b1091eaa929f7926429a7b69d7578f39b7bf Mon Sep 17 00:00:00 2001
From: Ewoud Kohl van Wijngaarden <ewoud@kohlvanwijngaarden.nl>
Date: Thu, 17 Oct 2024 15:02:33 +0200
Subject: [PATCH 06/14] Pass in PostgreSQL credentials via environment
 variables

This avoids the need to build a complex command. On the plus side, it
avoids the risk of leaking anything about the database access. On the
downside, that can make debugging harder.
---
 lib/foreman_maintain/concerns/base_database.rb | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/lib/foreman_maintain/concerns/base_database.rb b/lib/foreman_maintain/concerns/base_database.rb
index ca0281adc..2e7e52872 100644
--- a/lib/foreman_maintain/concerns/base_database.rb
+++ b/lib/foreman_maintain/concerns/base_database.rb
@@ -70,7 +70,7 @@ def ping
       end
 
       def dump_db(file)
-        dump_command = base_command('pg_dump') + " -Fc #{configuration['database']} -f #{file}"
+        dump_command = "pg_dump -Fc #{configuration['database']} -f #{file}"
         execute!(dump_command, :env => base_env)
       end
 
@@ -81,8 +81,7 @@ def restore_dump(file, localdb)
         else
           # TODO: figure out how to completely ignore errors. Currently this
           # sometimes exits with 1 even though errors are ignored by pg_restore
-          dump_cmd = base_command('pg_restore') +
-                     ' --no-privileges --clean --disable-triggers -n public ' \
+          dump_cmd = 'pg_restore --no-privileges --clean --disable-triggers -n public ' \
                      "-d #{configuration['database']} #{file}"
           execute!(dump_cmd, :env => base_env,
             :valid_exit_statuses => [0, 1])
@@ -148,17 +147,15 @@ def raise_psql_missing_error
 
       def base_env
         {
+          'PGHOST' => configuration.fetch('host', 'localhost'),
+          'PGPORT' => configuration['port']&.to_s,
+          'PGUSER' => configuration['username'],
           'PGPASSWORD' => configuration['password'],
         }
       end
 
-      def base_command(command = 'psql')
-        "#{command} -h #{configuration['host'] || 'localhost'} "\
-        " -p #{configuration['port'] || '5432'} -U #{configuration['username']}"
-      end
-
       def psql_command
-        base_command('psql') + " -d #{configuration['database']}"
+        "psql -d #{configuration['database']}"
       end
 
       def raise_service_error

From bb1681fac84b6a2375d3006a339567a90efc392d Mon Sep 17 00:00:00 2001
From: Ewoud Kohl van Wijngaarden <ewoud@kohlvanwijngaarden.nl>
Date: Thu, 17 Oct 2024 15:11:42 +0200
Subject: [PATCH 07/14] Also pass in the database name via an environment
 variable

This avoids the need for another helper command and simplifies the
commands.
---
 lib/foreman_maintain/concerns/base_database.rb | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/lib/foreman_maintain/concerns/base_database.rb b/lib/foreman_maintain/concerns/base_database.rb
index 2e7e52872..6239f6a49 100644
--- a/lib/foreman_maintain/concerns/base_database.rb
+++ b/lib/foreman_maintain/concerns/base_database.rb
@@ -55,7 +55,7 @@ def query_csv(sql)
 
       def psql(query)
         if ping
-          execute(psql_command,
+          execute('psql',
             :stdin => query,
             :env => base_env)
         else
@@ -64,13 +64,13 @@ def psql(query)
       end
 
       def ping
-        execute?(psql_command,
+        execute?('psql',
           :stdin => 'SELECT 1 as ping',
           :env => base_env)
       end
 
       def dump_db(file)
-        dump_command = "pg_dump -Fc #{configuration['database']} -f #{file}"
+        dump_command = "pg_dump -Fc -f #{file}"
         execute!(dump_command, :env => base_env)
       end
 
@@ -125,7 +125,7 @@ def dropdb
       def db_version
         if ping
           # Note - t removes headers, -A removes alignment whitespace
-          server_version_cmd = psql_command + ' -c "SHOW server_version" -t -A'
+          server_version_cmd = 'psql -c "SHOW server_version" -t -A'
           version_string = execute!(server_version_cmd, :env => base_env)
           version(version_string)
         else
@@ -151,13 +151,10 @@ def base_env
           'PGPORT' => configuration['port']&.to_s,
           'PGUSER' => configuration['username'],
           'PGPASSWORD' => configuration['password'],
+          'PGDATABASE' => configuration['database'],
         }
       end
 
-      def psql_command
-        "psql -d #{configuration['database']}"
-      end
-
       def raise_service_error
         raise Error::Fail, 'Please check whether database service is up & running state.'
       end

From de3110bafaaec100f6225e205aaed35060b57ddb Mon Sep 17 00:00:00 2001
From: Ewoud Kohl van Wijngaarden <ewoud@kohlvanwijngaarden.nl>
Date: Thu, 17 Oct 2024 15:12:50 +0200
Subject: [PATCH 08/14] Pass in the version query via stdin

This matches other queries.
---
 lib/foreman_maintain/concerns/base_database.rb | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/foreman_maintain/concerns/base_database.rb b/lib/foreman_maintain/concerns/base_database.rb
index 6239f6a49..e7a8c1e6e 100644
--- a/lib/foreman_maintain/concerns/base_database.rb
+++ b/lib/foreman_maintain/concerns/base_database.rb
@@ -124,9 +124,10 @@ def dropdb
 
       def db_version
         if ping
+          query = 'SHOW server_version'
           # Note - t removes headers, -A removes alignment whitespace
-          server_version_cmd = 'psql -c "SHOW server_version" -t -A'
-          version_string = execute!(server_version_cmd, :env => base_env)
+          server_version_cmd = 'psql -t -A'
+          version_string = execute!(server_version_cmd, :stdin => query, :env => base_env)
           version(version_string)
         else
           raise_service_error

From 1e34fbad5dc5cd621ff26eb9cc294cc56c273b20 Mon Sep 17 00:00:00 2001
From: Ewoud Kohl van Wijngaarden <ewoud@kohlvanwijngaarden.nl>
Date: Thu, 17 Oct 2024 15:13:35 +0200
Subject: [PATCH 09/14] Use long options to make a command self documenting

---
 lib/foreman_maintain/concerns/base_database.rb | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/foreman_maintain/concerns/base_database.rb b/lib/foreman_maintain/concerns/base_database.rb
index e7a8c1e6e..86f6a9b32 100644
--- a/lib/foreman_maintain/concerns/base_database.rb
+++ b/lib/foreman_maintain/concerns/base_database.rb
@@ -125,8 +125,7 @@ def dropdb
       def db_version
         if ping
           query = 'SHOW server_version'
-          # Note - t removes headers, -A removes alignment whitespace
-          server_version_cmd = 'psql -t -A'
+          server_version_cmd = 'psql --tuples-only --no-align'
           version_string = execute!(server_version_cmd, :stdin => query, :env => base_env)
           version(version_string)
         else

From 4aa6f957d1e58afc91ad1ab887532010950b0253 Mon Sep 17 00:00:00 2001
From: Ewoud Kohl van Wijngaarden <ewoud@kohlvanwijngaarden.nl>
Date: Thu, 17 Oct 2024 15:18:31 +0200
Subject: [PATCH 10/14] Introduce a ping! helper that raises an exception

---
 .../concerns/base_database.rb                 | 32 +++++++++----------
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/lib/foreman_maintain/concerns/base_database.rb b/lib/foreman_maintain/concerns/base_database.rb
index 86f6a9b32..5d177e379 100644
--- a/lib/foreman_maintain/concerns/base_database.rb
+++ b/lib/foreman_maintain/concerns/base_database.rb
@@ -54,13 +54,11 @@ def query_csv(sql)
       end
 
       def psql(query)
-        if ping
-          execute('psql',
-            :stdin => query,
-            :env => base_env)
-        else
-          raise_service_error
-        end
+        ping!
+
+        execute('psql',
+          :stdin => query,
+          :env => base_env)
       end
 
       def ping
@@ -123,14 +121,12 @@ def dropdb
       end
 
       def db_version
-        if ping
-          query = 'SHOW server_version'
-          server_version_cmd = 'psql --tuples-only --no-align'
-          version_string = execute!(server_version_cmd, :stdin => query, :env => base_env)
-          version(version_string)
-        else
-          raise_service_error
-        end
+        ping!
+
+        query = 'SHOW server_version'
+        server_version_cmd = 'psql --tuples-only --no-align'
+        version_string = execute!(server_version_cmd, :stdin => query, :env => base_env)
+        version(version_string)
       end
 
       def psql_cmd_available?
@@ -155,8 +151,10 @@ def base_env
         }
       end
 
-      def raise_service_error
-        raise Error::Fail, 'Please check whether database service is up & running state.'
+      def ping!
+        unless ping
+          raise Error::Fail, 'Please check whether database service is up & running state.'
+        end
       end
     end
   end

From d3020de7ad0262f8d6e6298c02a6d645a1544459 Mon Sep 17 00:00:00 2001
From: Ewoud Kohl van Wijngaarden <ewoud@kohlvanwijngaarden.nl>
Date: Thu, 17 Oct 2024 15:19:57 +0200
Subject: [PATCH 11/14] Completely cache the Debian PostgreSQL versions

This uses a block that won't run if @deb_postgresql_versions is already
set.
---
 lib/foreman_maintain/concerns/base_database.rb | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/lib/foreman_maintain/concerns/base_database.rb b/lib/foreman_maintain/concerns/base_database.rb
index 5d177e379..54e1e17ee 100644
--- a/lib/foreman_maintain/concerns/base_database.rb
+++ b/lib/foreman_maintain/concerns/base_database.rb
@@ -16,11 +16,12 @@ def deb_postgresql_data_dir
       end
 
       def deb_postgresql_versions
-        installed_pkgs = package_manager.list_installed_packages('${binary:Package}\n')
-        @deb_postgresql_versions ||= installed_pkgs.grep(/^postgresql-\d+$/).map do |name|
-          name.split('-').last
+        @deb_postgresql_versions ||= begin
+          installed_pkgs = package_manager.list_installed_packages('${binary:Package}\n')
+          installed_pkgs.grep(/^postgresql-\d+$/).map do |name|
+            name.split('-').last
+          end
         end
-        @deb_postgresql_versions
       end
 
       def postgresql_conf

From 31ac520b18b11279e46da892a78e186b37040dc9 Mon Sep 17 00:00:00 2001
From: Evgeni Golov <evgeni@golov.de>
Date: Tue, 15 Oct 2024 14:52:10 +0200
Subject: [PATCH 12/14] drop hidden_patterns

---
 lib/foreman_maintain/utils/command_runner.rb | 21 ++++++--------------
 test/lib/utils/command_runner_test.rb        | 16 ---------------
 2 files changed, 6 insertions(+), 31 deletions(-)
 delete mode 100644 test/lib/utils/command_runner_test.rb

diff --git a/lib/foreman_maintain/utils/command_runner.rb b/lib/foreman_maintain/utils/command_runner.rb
index 10ed2f041..762623b2e 100644
--- a/lib/foreman_maintain/utils/command_runner.rb
+++ b/lib/foreman_maintain/utils/command_runner.rb
@@ -8,14 +8,12 @@ class CommandRunner
       attr_reader :logger, :command
 
       def initialize(logger, command, options)
-        options.validate_options!(:stdin, :hidden_patterns, :interactive, :valid_exit_statuses,
-          :env)
+        options.validate_options!(:stdin, :interactive, :valid_exit_statuses, :env)
         options[:valid_exit_statuses] ||= [0]
         options[:env] ||= {}
         @logger = logger
         @command = command
         @stdin = options[:stdin]
-        @hidden_patterns = Array(options[:hidden_patterns]).compact
         @interactive = options[:interactive]
         @options = options
         @valid_exit_statuses = options[:valid_exit_statuses]
@@ -24,13 +22,13 @@ def initialize(logger, command, options)
       end
 
       def run
-        logger&.debug(hide_strings("Running command #{@command} with stdin #{@stdin.inspect}"))
+        logger&.debug("Running command #{@command} with stdin #{@stdin.inspect}")
         if @interactive
           run_interactively
         else
           run_non_interactively
         end
-        logger&.debug("output of the command:\n #{hide_strings(output)}")
+        logger&.debug("output of the command:\n #{output}")
       end
 
       def interactive?
@@ -52,10 +50,10 @@ def success?
       end
 
       def execution_error
-        raise Error::ExecutionError.new(hide_strings(@command),
+        raise Error::ExecutionError.new(@command,
           exit_status,
-          hide_strings(@stdin),
-          @interactive ? nil : hide_strings(@output))
+          @stdin,
+          @interactive ? nil : @output)
       end
 
       private
@@ -97,13 +95,6 @@ def run_non_interactively
       def full_command
         "#{@command} 2>&1"
       end
-
-      def hide_strings(string)
-        return unless string
-        @hidden_patterns.reduce(string) do |result, hidden_pattern|
-          result.gsub(hidden_pattern, '[FILTERED]')
-        end
-      end
     end
   end
 end
diff --git a/test/lib/utils/command_runner_test.rb b/test/lib/utils/command_runner_test.rb
deleted file mode 100644
index 42a1e304c..000000000
--- a/test/lib/utils/command_runner_test.rb
+++ /dev/null
@@ -1,16 +0,0 @@
-require 'test_helper'
-
-module ForemanMaintain
-  describe Utils::CommandRunner do
-    let(:log) { StringIO.new }
-    let(:logger) { Logger.new(log) }
-
-    it 'hides passwords in the logs' do
-      command = Utils::CommandRunner.new(logger, 'echo "Password is secret"',
-        :hidden_patterns => [nil, 'secret'])
-      command.run
-      _(log.string).must_match "output of the command:\n Password is [FILTERED]\n"
-      _(log.string).must_match 'Running command echo "Password is [FILTERED]" with stdin nil'
-    end
-  end
-end

From 5e89e8cf16afdc53292132c7c329c94d801fbcbb Mon Sep 17 00:00:00 2001
From: Ewoud Kohl van Wijngaarden <ewoud@kohlvanwijngaarden.nl>
Date: Thu, 17 Oct 2024 17:52:39 +0200
Subject: [PATCH 13/14] Use <<~ for more predictable indenting

This wasn't used in the past for Ruby 1.9 or 2.0 support, but we can now
rely on this. It makes testing easier.
---
 lib/foreman_maintain/concerns/base_database.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/foreman_maintain/concerns/base_database.rb b/lib/foreman_maintain/concerns/base_database.rb
index 54e1e17ee..725f42db0 100644
--- a/lib/foreman_maintain/concerns/base_database.rb
+++ b/lib/foreman_maintain/concerns/base_database.rb
@@ -112,7 +112,7 @@ def dropdb
         if local?
           execute!("runuser - postgres -c 'dropdb #{configuration['database']}'")
         else
-          delete_statement = psql(<<-SQL)
+          delete_statement = psql(<<~SQL)
             select string_agg('drop table if exists \"' || tablename || '\" cascade;', '')
             from pg_tables
             where schemaname = 'public';

From 0ea3d4ec62870f36907fcdd3adec6c2b2cdf2e92 Mon Sep 17 00:00:00 2001
From: Evgeni Golov <evgeni@golov.de>
Date: Thu, 17 Oct 2024 11:22:39 +0200
Subject: [PATCH 14/14] add tests for base database concern

---
 test/lib/concerns/base_database_test.rb | 163 ++++++++++++++++++++++++
 1 file changed, 163 insertions(+)
 create mode 100644 test/lib/concerns/base_database_test.rb

diff --git a/test/lib/concerns/base_database_test.rb b/test/lib/concerns/base_database_test.rb
new file mode 100644
index 000000000..c65dd1f38
--- /dev/null
+++ b/test/lib/concerns/base_database_test.rb
@@ -0,0 +1,163 @@
+require 'test_helper'
+
+class FakeDatabase
+  include ForemanMaintain::Concerns::BaseDatabase
+  include ForemanMaintain::Concerns::SystemHelpers
+
+  attr_reader :configuration
+
+  def initialize(config)
+    @configuration = config
+  end
+end
+
+module ForemanMaintain
+  describe Concerns::BaseDatabase do
+    describe 'with local db' do
+      let(:db) { FakeDatabase.new(config) }
+      let(:config) do
+        {
+          'host' => 'localhost',
+          'database' => 'fakedb',
+          'username' => 'fakeuser',
+          'password' => 'fakepassword',
+        }
+      end
+      let(:expected_env) do
+        {
+          'PGHOST' => 'localhost',
+          'PGPORT' => nil,
+          'PGDATABASE' => 'fakedb',
+          'PGUSER' => 'fakeuser',
+          'PGPASSWORD' => 'fakepassword',
+        }
+      end
+
+      it 'accepts localhost as local' do
+        assert db.local?
+      end
+
+      it 'fetches server version' do
+        db.expects(:ping!)
+        db.expects(:execute!).with(
+          'psql --tuples-only --no-align',
+          stdin: 'SHOW server_version',
+          env: expected_env
+        ).returns('13.16')
+
+        assert db.db_version
+      end
+
+      it 'drops db' do
+        db.expects(:execute!).with("runuser - postgres -c 'dropdb fakedb'").returns('')
+
+        assert db.dropdb
+      end
+
+      it 'restores db' do
+        file = '/backup/fake.dump'
+
+        db.expects(:execute!).with("runuser - postgres -c 'pg_restore -C -d postgres #{file}'").
+          returns('')
+
+        assert db.restore_dump(file, true)
+      end
+
+      it 'dumps db' do
+        file = '/backup/fake.dump'
+
+        db.expects(:execute!).with(
+          "pg_dump -Fc -f /backup/fake.dump",
+          env: expected_env
+        ).returns('')
+
+        assert db.dump_db(file)
+      end
+
+      it 'pings db' do
+        db.expects(:execute?).with("psql",
+          stdin: "SELECT 1 as ping", env: expected_env).returns(true)
+
+        assert db.ping
+      end
+
+      it 'runs db queries' do
+        psql_return = <<~PSQL
+           test
+          ------
+             42
+          (1 row)
+        PSQL
+
+        db.expects(:ping!)
+        db.expects(:execute).with("psql",
+          stdin: "SELECT 42 as test", env: expected_env).returns(psql_return)
+
+        assert db.psql('SELECT 42 as test')
+      end
+    end
+
+    describe 'with remote db' do
+      let(:db) { FakeDatabase.new(config) }
+      let(:config) do
+        {
+          'host' => 'db.example.com',
+          'database' => 'fakedb',
+          'username' => 'fakeuser',
+          'password' => 'fakepassword',
+        }
+      end
+      let(:expected_env) do
+        {
+          'PGHOST' => 'db.example.com',
+          'PGPORT' => nil,
+          'PGDATABASE' => 'fakedb',
+          'PGUSER' => 'fakeuser',
+          'PGPASSWORD' => 'fakepassword',
+        }
+      end
+
+      it 'accepts db.example.com as remote' do
+        refute db.local?
+      end
+
+      it 'drops db' do
+        select_statement = <<~SQL
+          select string_agg('drop table if exists \"' || tablename || '\" cascade;', '')
+          from pg_tables
+          where schemaname = 'public';
+        SQL
+        delete_statement = 'drop table if exists \"faketable\"'
+        db.expects(:psql).with(select_statement).returns(delete_statement)
+        db.expects(:psql).with(delete_statement).returns('')
+        assert db.dropdb
+      end
+
+      it 'restores db' do
+        file = '/backup/fake.dump'
+        restore_cmd = <<~CMD.strip
+          pg_restore --no-privileges --clean --disable-triggers -n public -d fakedb #{file}
+        CMD
+
+        db.expects(:execute!).with(
+          restore_cmd,
+          valid_exit_statuses: [0, 1],
+          env: expected_env
+        ).returns('')
+
+        assert db.restore_dump(file, false)
+      end
+
+      it 'dumps remote db' do
+        file = '/backup/fake.dump'
+
+        db.expects(:execute!).with(
+          "pg_dump -Fc -f /backup/fake.dump",
+          env: expected_env
+        ).returns('')
+
+        assert db.dump_db(file)
+      end
+    end
+  end
+end