Project

General

Profile

Bug #23025

foreman-installer fails when ssl-verify is set to false for candlepin db

Added by Ales Dujicek over 3 years ago. Updated about 3 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Category:
Installer
Target version:
Difficulty:
Triaged:
Yes
Bugzilla link:

Description

foreman-installer fails when --katello-candlepin-db-ssl true and --katello-candlepin-db-ssl-verify false are set

/var/log/foreman-installer/katello.log
[DEBUG 2018-03-27T02:13:49 main] Exec[cpdb](provider=posix): Executing 'cpdb --create --schema-only --dbhost=dbhost.redhat.com --dbport=5432 --database=candlepin5db?ssl=true&sslfactory=org.postgresql.ssl.NonValidatingFactory --user=candlepin5 --password=candlepin5pw >> /var/log/candlepin/cpdb.log 2>&1 && touch /var/lib/candlepin/cpdb_done'
[DEBUG 2018-03-27T02:13:49 main] Executing: 'cpdb --create --schema-only --dbhost=dbhost.redhat.com --dbport=5432 --database=candlepin5db?ssl=true&sslfactory=org.postgresql.ssl.NonValidatingFactory --user=candlepin5 --password=candlepin5pw >> /var/log/candlepin/cpdb.log 2>&1 && touch /var/lib/candlepin/cpdb_done'
[ERROR 2018-03-27T02:13:49 main]
[ERROR 2018-03-27T02:13:49 main] /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/provider/exec.rb:68:in `block in run'
[ERROR 2018-03-27T02:13:49 main] /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/provider/exec.rb:30:in `chdir'
[ERROR 2018-03-27T02:13:49 main] /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/provider/exec.rb:30:in `run'
[ERROR 2018-03-27T02:13:49 main] /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/provider/exec/posix.rb:45:in `run'
[ERROR 2018-03-27T02:13:49 main] /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/type/exec.rb:130:in `block in sync'
[ERROR 2018-03-27T02:13:49 main] /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/type/exec.rb:127:in `times'
...
[ERROR 2018-03-27T02:13:49 main] /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/util/command_line.rb:72:in `execute'
[ERROR 2018-03-27T02:13:49 main] /opt/puppetlabs/puppet/bin/puppet:5:in `<main>'
[ERROR 2018-03-27T02:13:49 main] /Stage[main]/Candlepin::Database::Postgresql/Exec[cpdb]/returns: change from notrun to 0 failed:

/var/log/candlepin/cpdb.log:
sh: --user=candlepin5: command not found

It seems that we need to escape the ampersand in cpdb's --database parameter

Patch for puppet-candlepin

diff --git a/manifests/database/postgresql.pp b/manifests/database/postgresql.pp
index dc100ca..db18906 100644
--- a/manifests/database/postgresql.pp
+++ b/manifests/database/postgresql.pp
@@ -41,7 +41,7 @@ class candlepin::database::postgresql(

   if $init_db {
     $ssl_verify_options = $db_ssl_verify ? {
-      false => '&sslfactory=org.postgresql.ssl.NonValidatingFactory',
+      false => '\\&sslfactory=org.postgresql.ssl.NonValidatingFactory',
       default => ''
     }

@@ -56,7 +56,7 @@ class candlepin::database::postgresql(
                        --schema-only \
                        --dbhost=${db_host} \
                        --dbport=${db_port} \
-                       --database=${db_name}${ssl_options} \
+                       --database='${db_name}${ssl_options}' \
                        --user=${db_user}  \
                        --password=${db_password} \
                        >> ${log_dir}/cpdb.log \

Similar code is in katello-installer

diff --git a/hooks/pre/30-upgrade.rb b/hooks/pre/30-upgrade.rb
index 78a1e94..3b64fc9 100644
--- a/hooks/pre/30-upgrade.rb
+++ b/hooks/pre/30-upgrade.rb
@@ -28,7 +28,7 @@ def migrate_candlepin
   db_uri = "//#{db_host}" + (db_port.nil? ? '' : ":#{db_port}") + "/#{db_name}" 
   if db_ssl
     db_uri += "?ssl=true" 
-    db_uri += "&sslfactory=org.postgresql.ssl.NonValidatingFactory" unless db_ssl_verify
+    db_uri += "\\&sslfactory=org.postgresql.ssl.NonValidatingFactory" unless db_ssl_verify
   end
   Kafo::Helpers.execute("/usr/share/candlepin/cpdb --update --database '#{db_uri}' --user #{db_user} --password #{db_password}")
 end

Steps to reproduce:
1. create postgres users and databases on remote host for candlepin and foreman
2. # foreman-installer --scenario katello --foreman-db-manage false --foreman-db-host dbhost.redhat.com --foreman-db-database foreman5db --foreman-db-username foreman5 --foreman-db-password foreman5pw --foreman-db-port 5432 --katello-candlepin-manage-db false --katello-candlepin-db-host dbhost.redhat.com --katello-candlepin-db-name candlepin5db --katello-candlepin-db-user candlepin5 --katello-candlepin-db-password candlepin5pw --katello-candlepin-db-port 5432 --foreman-db-sslmode allow --katello-candlepin-db-ssl true --katello-candlepin-db-ssl-verify false


Related issues

Related to Katello - Feature #19667: Need additional supported database deployment options for Katello installation: such as External PostgresClosed2017-05-25

Associated revisions

Revision 34388103 (diff)
Added by Ales Dujicek about 3 years ago

Fixes #23025 - escape db connection, username and password

Revision 04c0e2c7 (diff)
Added by Ales Dujicek about 3 years ago

Refs #23025 quote dbconnection, username and password strings

History

#1 Updated by Ales Dujicek over 3 years ago

  • Related to Feature #19667: Need additional supported database deployment options for Katello installation: such as External Postgres added

#2 Updated by John Mitsch over 3 years ago

  • Legacy Backlogs Release (now unused) set to 338

If you were able to fix this in your environment, would you mind opening up pull requests for these changes? https://github.com/theforeman/puppet-candlepin

#3 Updated by The Foreman Bot over 3 years ago

  • Status changed from New to Ready For Testing
  • Pull request https://github.com/theforeman/puppet-candlepin/pull/101 added

#4 Updated by The Foreman Bot over 3 years ago

  • Pull request https://github.com/Katello/katello-installer/pull/607 added

#5 Updated by Ales Dujicek over 3 years ago

Created the pull requests, but I wanted someone from developers to look at it first.
Becuse if we patched candlepin to quote --url argument
https://github.com/candlepin/candlepin/blob/master/server/code/setup/cpdb#L90
-liquibase_options = "--driver=%s --classpath=%s --changeLogFile=%s --url=%s --username=%s" % (
+liquibase_options = "--driver=%s --classpath=%s --changeLogFile=%s --url='%s' --username=%s" % (
so there would be no need to escape the ampersand in the installer code

#6 Updated by Ewoud Kohl van Wijngaarden over 3 years ago

IMHO cpdb should properly handle this. I'm surprised it uses commands.getoutput and that uses a shell. I'd have expected it to use subprocess.check_output but that's python 2.7+ and would break on EL6. OTOH, you could use subprocess.Popen to achieve the same thing without opening a shell. You could even pass in an array of arguments without having to worry about escaping anything.

#8 Updated by Ales Dujicek about 3 years ago

  • % Done changed from 0 to 100
  • Status changed from Ready For Testing to Closed

Also available in: Atom PDF