fix(cli): Database agnostic options for root db credentials (#15973)

* fix(bench): new-site params for root db credentials

allow root credentials for postgresql
use common cli option name for both database types

* fix(bench): backward compatible db params

Co-authored-by: gavin <gavin18d@gmail.com>

* fix(bench): use common db cred params

use --db-root-username and --db-root-password

* feat(bench): add --set-default to bench new-site

* fix: do not set default root user

* fix: indentation

Co-authored-by: gavin <gavin18d@gmail.com>

frappe/commands/site.py

@@ -19,36 +19,38 @@ from frappe.exceptions import SiteNotSpecifiedError
 @click.option('--db-type', default='mariadb', type=click.Choice(['mariadb', 'postgres']), help='Optional "postgres" or "mariadb". Default is "mariadb"')
 @click.option('--db-host', help='Database Host')
 @click.option('--db-port', type=int, help='Database Port')
-@click.option('--mariadb-root-username', default='root', help='Root username for MariaDB')
-@click.option('--mariadb-root-password', help='Root password for MariaDB')
+@click.option('--db-root-username', '--mariadb-root-username', help='Root username for MariaDB or PostgreSQL, Default is "root"')
+@click.option('--db-root-password', '--mariadb-root-password', help='Root password for MariaDB or PostgreSQL')
 @click.option('--no-mariadb-socket', is_flag=True, default=False, help='Set MariaDB host to % and use TCP/IP Socket instead of using the UNIX Socket')
 @click.option('--admin-password', help='Administrator password for new site', default=None)
 @click.option('--verbose', is_flag=True, default=False, help='Verbose')
 @click.option('--force', help='Force restore if site/database already exists', is_flag=True, default=False)
 @click.option('--source_sql', help='Initiate database with a SQL file')
 @click.option('--install-app', multiple=True, help='Install app after installation')
-def new_site(site, mariadb_root_username=None, mariadb_root_password=None, admin_password=None,
-			 verbose=False, install_apps=None, source_sql=None, force=None, no_mariadb_socket=False,
-			 install_app=None, db_name=None, db_password=None, db_type=None, db_host=None, db_port=None):
+@click.option('--set-default', is_flag=True, default=False, help='Set the new site as default site')
+def new_site(site, db_root_username=None, db_root_password=None, admin_password=None,
+			verbose=False, install_apps=None, source_sql=None, force=None, no_mariadb_socket=False,
+			install_app=None, db_name=None, db_password=None, db_type=None, db_host=None, db_port=None,
+			set_default=False):
 	"Create a new site"
 	from frappe.installer import _new_site
 
 	frappe.init(site=site, new_site=True)
 
-	_new_site(db_name, site, mariadb_root_username=mariadb_root_username,
-			  mariadb_root_password=mariadb_root_password, admin_password=admin_password,
-			  verbose=verbose, install_apps=install_app, source_sql=source_sql, force=force,
-			  no_mariadb_socket=no_mariadb_socket, db_password=db_password, db_type=db_type, db_host=db_host,
-			  db_port=db_port, new_site=True)
+	_new_site(db_name, site, db_root_username=db_root_username,
+			db_root_password=db_root_password, admin_password=admin_password,
+			verbose=verbose, install_apps=install_app, source_sql=source_sql, force=force,
+			no_mariadb_socket=no_mariadb_socket, db_password=db_password, db_type=db_type, db_host=db_host,
+			db_port=db_port, new_site=True)
 
-	if len(frappe.utils.get_sites()) == 1:
+	if set_default:
 		use(site)
 
 
 @click.command('restore')
 @click.argument('sql-file-path')
-@click.option('--mariadb-root-username', default='root', help='Root username for MariaDB')
-@click.option('--mariadb-root-password', help='Root password for MariaDB')
+@click.option('--db-root-username', '--mariadb-root-username', help='Root username for MariaDB or PostgreSQL, Default is "root"')
+@click.option('--db-root-password', '--mariadb-root-password', help='Root password for MariaDB or PostgreSQL')
 @click.option('--db-name', help='Database name for site in case it is a new one')
 @click.option('--admin-password', help='Administrator password for new site')
 @click.option('--install-app', multiple=True, help='Install app after installation')
@@ -57,7 +59,7 @@ def new_site(site, mariadb_root_username=None, mariadb_root_password=None, admin
 @click.option('--force', is_flag=True, default=False, help='Ignore the validations and downgrade warnings. This action is not recommended')
 @click.option('--encryption-key', help='Backup encryption key')
 @pass_context
-def restore(context, sql_file_path, encryption_key=None, mariadb_root_username=None, mariadb_root_password=None,
+def restore(context, sql_file_path, encryption_key=None, db_root_username=None, db_root_password=None,
 			db_name=None, verbose=None, install_app=None, admin_password=None, force=None, with_public_files=None,
 			with_private_files=None):
 	"Restore site database from an sql file"
@@ -150,8 +152,8 @@ def restore(context, sql_file_path, encryption_key=None, mariadb_root_username=N
 
 
 	try:
-		_new_site(frappe.conf.db_name, site, mariadb_root_username=mariadb_root_username,
-			mariadb_root_password=mariadb_root_password, admin_password=admin_password,
+		_new_site(frappe.conf.db_name, site, db_root_username=db_root_username,
+			db_root_password=db_root_password, admin_password=admin_password,
 			verbose=context.verbose, install_apps=install_app, source_sql=decompressed_file_name,
 			force=True, db_type=frappe.conf.db_type)
 
@@ -290,16 +292,16 @@ def partial_restore(context, sql_file_path, verbose,  encryption_key=None):
 
 @click.command('reinstall')
 @click.option('--admin-password', help='Administrator Password for reinstalled site')
-@click.option('--mariadb-root-username', help='Root username for MariaDB')
-@click.option('--mariadb-root-password', help='Root password for MariaDB')
+@click.option('--db-root-username', '--mariadb-root-username', help='Root username for MariaDB or PostgreSQL, Default is "root"')
+@click.option('--db-root-password', '--mariadb-root-password', help='Root password for MariaDB or PostgreSQL')
 @click.option('--yes', is_flag=True, default=False, help='Pass --yes to skip confirmation')
 @pass_context
-def reinstall(context, admin_password=None, mariadb_root_username=None, mariadb_root_password=None, yes=False):
+def reinstall(context, admin_password=None, db_root_username=None, db_root_password=None, yes=False):
 	"Reinstall site ie. wipe all data and start over"
 	site = get_site(context)
-	_reinstall(site, admin_password, mariadb_root_username, mariadb_root_password, yes, verbose=context.verbose)
+	_reinstall(site, admin_password, db_root_username, db_root_password, yes, verbose=context.verbose)
 
-def _reinstall(site, admin_password=None, mariadb_root_username=None, mariadb_root_password=None, yes=False, verbose=False):
+def _reinstall(site, admin_password=None, db_root_username=None, db_root_password=None, yes=False, verbose=False):
 	from frappe.installer import _new_site
 
 	if not yes:
@@ -319,7 +321,7 @@ def _reinstall(site, admin_password=None, mariadb_root_username=None, mariadb_ro
 
 	frappe.init(site=site)
 	_new_site(frappe.conf.db_name, site, verbose=verbose, force=True, reinstall=True, install_apps=installed,
-		mariadb_root_username=mariadb_root_username, mariadb_root_password=mariadb_root_password,
+		db_root_username=db_root_username, db_root_password=db_root_password,
 		admin_password=admin_password)
 
 @click.command('install-app')
@@ -656,16 +658,16 @@ def uninstall(context, app, dry_run, yes, no_backup, force):
 
 @click.command('drop-site')
 @click.argument('site')
-@click.option('--root-login', default='root')
-@click.option('--root-password')
+@click.option('--db-root-username', '--mariadb-root-username', '--root-login', help='Root username for MariaDB or PostgreSQL, Default is "root"')
+@click.option('--db-root-password', '--mariadb-root-password', '--root-password', help='Root password for MariaDB or PostgreSQL')
 @click.option('--archived-sites-path')
 @click.option('--no-backup', is_flag=True, default=False)
 @click.option('--force', help='Force drop-site even if an error is encountered', is_flag=True, default=False)
-def drop_site(site, root_login='root', root_password=None, archived_sites_path=None, force=False, no_backup=False):
-	_drop_site(site, root_login, root_password, archived_sites_path, force, no_backup)
+def drop_site(site, db_root_username='root', db_root_password=None, archived_sites_path=None, force=False, no_backup=False):
+	_drop_site(site, db_root_username, db_root_password, archived_sites_path, force, no_backup)
 
 
-def _drop_site(site, root_login='root', root_password=None, archived_sites_path=None, force=False, no_backup=False):
+def _drop_site(site, db_root_username=None, db_root_password=None, archived_sites_path=None, force=False, no_backup=False):
 	"Remove site from database and filesystem"
 	from frappe.database import drop_user_and_database
 	from frappe.utils.backups import scheduled_backup
@@ -690,7 +692,7 @@ def _drop_site(site, root_login='root', root_password=None, archived_sites_path=
 			click.echo("\n".join(messages))
 			sys.exit(1)
 
-	drop_user_and_database(frappe.conf.db_name, root_login, root_password)
+	drop_user_and_database(frappe.conf.db_name, db_root_username, db_root_password)
 
 	archived_sites_path = archived_sites_path or os.path.join(frappe.get_app_path('frappe'), '..', '..', '..', 'archived', 'sites')
 

frappe/database/postgres/setup_db.py

@@ -4,7 +4,7 @@ import frappe
 
 
 def setup_database(force, source_sql=None, verbose=False):
-	root_conn = get_root_connection()
+	root_conn = get_root_connection(frappe.flags.root_login, frappe.flags.root_password)
 	root_conn.commit()
 	root_conn.sql("DROP DATABASE IF EXISTS `{0}`".format(frappe.conf.db_name))
 	root_conn.sql("DROP USER IF EXISTS {0}".format(frappe.conf.db_name))
@@ -70,7 +70,7 @@ def import_db_from_sql(source_sql=None, verbose=False):
 		print(f"\nSTDOUT by psql:\n{restore_proc.stdout.decode()}\nImported from Database File: {source_sql}")
 
 def setup_help_database(help_db_name):
-	root_conn = get_root_connection()
+	root_conn = get_root_connection(frappe.flags.root_login, frappe.flags.root_password)
 	root_conn.sql("DROP DATABASE IF EXISTS `{0}`".format(help_db_name))
 	root_conn.sql("DROP USER IF EXISTS {0}".format(help_db_name))
 	root_conn.sql("CREATE DATABASE `{0}`".format(help_db_name))

frappe/installer.py

@@ -14,8 +14,8 @@ from frappe.defaults import _clear_cache
 def _new_site(
 	db_name,
 	site,
-	mariadb_root_username=None,
-	mariadb_root_password=None,
+	db_root_username=None,
+	db_root_password=None,
 	admin_password=None,
 	verbose=False,
 	install_apps=None,
@@ -60,8 +60,8 @@ def _new_site(
 	installing = touch_file(get_site_path("locks", "installing.lock"))
 
 	install_db(
-		root_login=mariadb_root_username,
-		root_password=mariadb_root_password,
+		root_login=db_root_username,
+		root_password=db_root_password,
 		db_name=db_name,
 		admin_password=admin_password,
 		verbose=verbose,
@@ -92,7 +92,7 @@ def _new_site(
 	print("*** Scheduler is", scheduler_status, "***")
 
 
-def install_db(root_login="root", root_password=None, db_name=None, source_sql=None,
+def install_db(root_login=None, root_password=None, db_name=None, source_sql=None,
 			   admin_password=None, verbose=True, force=0, site_config=None, reinstall=False,
 			   db_password=None, db_type=None, db_host=None, db_port=None, no_mariadb_socket=False):
 	import frappe.database
@@ -101,6 +101,11 @@ def install_db(root_login="root", root_password=None, db_name=None, source_sql=N
 	if not db_type:
 		db_type = frappe.conf.db_type or 'mariadb'
 
+	if not root_login and db_type == 'mariadb':
+		root_login='root'
+	elif not root_login and db_type == 'postgres':
+		root_login='postgres'
+
 	make_conf(db_name, site_config=site_config, db_password=db_password, db_type=db_type, db_host=db_host, db_port=db_port)
 	frappe.flags.in_install_db = True