launchpad-reviewers team mailing list archive

[Colin Watson <mp+440069@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:remote-db-creation into launchpad:master with ~cjwatson/launchpad:sqlbase-connect-set-role-after-connecting as a prerequisite.

Commit message:
Allow initializing a remote database

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/440069

This is usable from an instance of the `launchpad-admin` charm: it uses the configuration file to find the necessary database credentials.  As such, it can be used to set up a Juju-deployed instance of Launchpad from scratch.

So that I sleep better at night, I added an explicit check that refuses to destroy the production database this way.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:remote-db-creation into launchpad:master.

diff --git a/database/schema/Makefile b/database/schema/Makefile
index c3a6b0c..317d19e 100644
--- a/database/schema/Makefile
+++ b/database/schema/Makefile
@@ -31,8 +31,9 @@ TEST_PLAYGROUND_DBNAME=launchpad_ftest_playground
 # The session database name.
 SESSION_DBNAME=session_dev
 
-# The command we use to drop a database.
-DROPDB=../../utilities/pgmassacre.py
+# Database options to pass to PostgreSQL tools.
+DBOPTS:=$(shell ../../utilities/pgoptions.py)
+
 # The command we use to drop (if exists) and recreate a database.
 CREATEDB=../../utilities/pgmassacre.py -t
 
@@ -70,7 +71,7 @@ test: create
 	@ echo "* Creating database \"$(TEMPLATE_WITH_TEST_SAMPLEDATA)\"."
 	@ ${CREATEDB} ${EMPTY_DBNAME} ${TEMPLATE_WITH_TEST_SAMPLEDATA}
 	@ echo "* Loading sample data"
-	@ psql -v ON_ERROR_STOP=1 -d ${TEMPLATE_WITH_TEST_SAMPLEDATA} -f $(SAMPLEDATA) > /dev/null
+	@ psql $(DBOPTS) -v ON_ERROR_STOP=1 -d ${TEMPLATE_WITH_TEST_SAMPLEDATA} -f $(SAMPLEDATA) > /dev/null
 	@ echo "* Rebuilding full text indexes"
 	@ ./fti.py --live-rebuild -q -d ${TEMPLATE_WITH_TEST_SAMPLEDATA}
 	@ echo "* Resetting sequences"
@@ -78,7 +79,7 @@ test: create
 	@ echo "* Disabling autovacuum"
 	@ ./unautovacuumable.py -d ${TEMPLATE_WITH_TEST_SAMPLEDATA}
 	@ echo "* Vacuuming"
-	@ vacuumdb -fz ${TEMPLATE_WITH_TEST_SAMPLEDATA}
+	@ vacuumdb $(DBOPTS) -fz -d ${TEMPLATE_WITH_TEST_SAMPLEDATA}
 
 # Create a launchpad_dev_template DB and load the dev sample data into it.
 # Also create a launchpad_ftest_playground DB as a copy of
@@ -87,7 +88,7 @@ dev: test
 	@ echo "* Creating ${TEMPLATE_WITH_DEV_SAMPLEDATA}"
 	@ ${CREATEDB} ${EMPTY_DBNAME} ${TEMPLATE_WITH_DEV_SAMPLEDATA}
 	@ echo "* Loading sample data"
-	@ psql -v ON_ERROR_STOP=1 -d ${TEMPLATE_WITH_DEV_SAMPLEDATA} -f $(SAMPLEDATA_DEV) > /dev/null
+	@ psql $(DBOPTS) -v ON_ERROR_STOP=1 -d ${TEMPLATE_WITH_DEV_SAMPLEDATA} -f $(SAMPLEDATA_DEV) > /dev/null
 	@ echo "* Rebuilding full text indexes"
 	@ ./fti.py --live-rebuild -q -d ${TEMPLATE_WITH_DEV_SAMPLEDATA}
 	@ echo "* Resetting sequences"
@@ -95,7 +96,7 @@ dev: test
 	@ echo "* Disabling autovacuum"
 	@ ./unautovacuumable.py -d ${TEMPLATE_WITH_DEV_SAMPLEDATA}
 	@ echo "* Vacuuming"
-	@ vacuumdb -fz ${TEMPLATE_WITH_DEV_SAMPLEDATA}
+	@ vacuumdb $(DBOPTS) -fz -d ${TEMPLATE_WITH_DEV_SAMPLEDATA}
 	@ echo "* Creating ${DBNAME_DEV}"
 	@ ${CREATEDB} ${TEMPLATE_WITH_DEV_SAMPLEDATA} ${DBNAME_DEV}
 	@ echo "* Creating ${TEST_PLAYGROUND_DBNAME}"
@@ -112,7 +113,7 @@ create:
 	@ echo "* Creating database \"$(EMPTY_DBNAME)\"."
 	@ ${CREATEDB} template0 ${EMPTY_DBNAME}
 	@ echo "* Loading base database schema"
-	@ psql -d ${EMPTY_DBNAME} -f ${BASELINE} | grep : | cat
+	@ psql $(DBOPTS) -d ${EMPTY_DBNAME} -f ${BASELINE} | grep : | cat
 	@ echo "* Patching the database schema"
 	@ ./upgrade.py --separate-sessions -d ${EMPTY_DBNAME}
 	@ echo "* Security setup"
@@ -120,16 +121,16 @@ create:
 	@ echo "* Disabling autovacuum"
 	@ ./unautovacuumable.py -d ${EMPTY_DBNAME}
 	@ echo "* Vacuuming"
-	@ vacuumdb -fz ${EMPTY_DBNAME}
+	@ vacuumdb $(DBOPTS) -fz -d ${EMPTY_DBNAME}
 
 	@ echo "* Creating session database '${SESSION_DBNAME}' (if necessary)"
-	@if [ "$$((`psql -l | grep -w ${SESSION_DBNAME} | wc -l`))" = '0' ]; \
+	@if [ "$$((`psql $(DBOPTS) -l | grep -w ${SESSION_DBNAME} | wc -l`))" = '0' ]; \
 	    then ${CREATEDB} template0 ${SESSION_DBNAME} ; \
-	    psql -q -d ${SESSION_DBNAME} -f launchpad_session.sql ; \
+	    psql $(DBOPTS) -q -d ${SESSION_DBNAME} -f launchpad_session.sql ; \
 	fi
 	@ echo "* Creating session database '${TEST_SESSION_DBNAME}'"
 	@ ${CREATEDB} template0 ${TEST_SESSION_DBNAME}
-	@ psql -q -d ${TEST_SESSION_DBNAME} -f launchpad_session.sql
+	@ psql $(DBOPTS) -q -d ${TEST_SESSION_DBNAME} -f launchpad_session.sql
 
 # Confirm that launchpad-XX-00-0.sql hasn't been messed with - this file
 # is our baseline telling us what was installed into production
diff --git a/lib/lp/services/database/sqlbase.py b/lib/lp/services/database/sqlbase.py
index 04b3c7f..2a6aafe 100644
--- a/lib/lp/services/database/sqlbase.py
+++ b/lib/lp/services/database/sqlbase.py
@@ -599,7 +599,11 @@ def connect(user=None, dbname=None, isolation=ISOLATION_LEVEL_DEFAULT):
 
     con = psycopg2.connect(dsn)
     con.set_isolation_level(isolation)
-    if dbconfig.set_role_after_connecting and user != parsed_dsn["user"]:
+    if (
+        dbconfig.set_role_after_connecting
+        and user is not None
+        and user != parsed_dsn["user"]
+    ):
         con.cursor().execute("SET ROLE %s", (user,))
     return con
 
diff --git a/utilities/pgmassacre.py b/utilities/pgmassacre.py
index 4fbf9e2..deb5793 100755
--- a/utilities/pgmassacre.py
+++ b/utilities/pgmassacre.py
@@ -15,22 +15,29 @@ Cut off access, slaughter connections and burn the database to the ground
 
 import _pythonpath  # noqa: F401
 
+import os
 import sys
 import time
 from optparse import OptionParser
 
 import psycopg2
-import psycopg2.extensions
+from psycopg2.extensions import make_dsn, parse_dsn
 
+from lp.services.config import config, dbconfig
 from lp.services.database import activity_cols
 
 
 def connect(dbname="template1"):
     """Connect to the database, returning the DB-API connection."""
+    parsed_dsn = parse_dsn(dbconfig.rw_main_primary)
     if options.user is not None:
-        return psycopg2.connect("dbname=%s user=%s" % (dbname, options.user))
-    else:
-        return psycopg2.connect("dbname=%s" % dbname)
+        parsed_dsn["user"] = options.user
+    # For database administration, we only pass a username if we're
+    # connecting over TCP.
+    elif "host" not in parsed_dsn:
+        parsed_dsn.pop("user", None)
+    parsed_dsn["dbname"] = dbname
+    return psycopg2.connect(make_dsn(**parsed_dsn))
 
 
 def rollback_prepared_transactions(database):
@@ -223,7 +230,16 @@ options = None
 
 
 def main():
-    parser = OptionParser("Usage: %prog [options] DBNAME")
+    parser = OptionParser(
+        "Usage: %prog [options] DBNAME",
+        description=(
+            "Set LPCONFIG to choose which database cluster to connect to; "
+            "credentials are taken from the database.rw_main_primary "
+            "configuration option, ignoring the 'dbname' parameter.  The "
+            "default behaviour is to connect to a cluster on the local "
+            "machine using PostgreSQL's default port."
+        ),
+    )
     parser.add_option(
         "-U",
         "--user",
@@ -254,6 +270,16 @@ def main():
         parser.error(
             "Running this script against template1 or template0 is nuts."
         )
+    if (
+        "host" in parse_dsn(dbconfig.rw_main_primary)
+        and os.environ.get("LP_DESTROY_REMOTE_DATABASE") != "yes"
+    ):
+        parser.error(
+            "For safety, refusing to destroy a remote database.  Set "
+            "LP_DESTROY_REMOTE_DATABASE=yes to override this."
+        )
+    if config.vhost.mainsite.hostname == "launchpad.net":
+        parser.error("Flatly refusing to destroy production.")
 
     con = connect()
     cur = con.cursor()
diff --git a/utilities/pgoptions.py b/utilities/pgoptions.py
new file mode 100755
index 0000000..05996b0
--- /dev/null
+++ b/utilities/pgoptions.py
@@ -0,0 +1,40 @@
+#!/usr/bin/python3 -S
+#
+# Copyright 2022 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""
+Print PostgreSQL connection options matching the current Launchpad primary
+database configuration.
+
+To avoid leaking information via process command lines, any password in the
+configured connection string is ignored; passwords should be set in
+~/.pgpass instead.
+"""
+
+import _pythonpath  # noqa: F401
+
+from optparse import OptionParser
+
+from psycopg2.extensions import parse_dsn
+
+from lp.services.config import dbconfig
+
+if __name__ == "__main__":
+    parser = OptionParser()
+    _, args = parser.parse_args()
+    if args:
+        parser.error("Too many options given")
+    parsed_dsn = parse_dsn(dbconfig.rw_main_primary)
+    conn_opts = []
+    if "host" in parsed_dsn:
+        conn_opts.append("--host=%s" % parsed_dsn["host"])
+    if "port" in parsed_dsn:
+        conn_opts.append("--port=%s" % parsed_dsn["port"])
+    # For database administration, we only pass a username if we're
+    # connecting over TCP.
+    if "host" in parsed_dsn and "user" in parsed_dsn:
+        conn_opts.append("--username=%s" % parsed_dsn["user"])
+    if "dbname" in parsed_dsn:
+        conn_opts.append("--dbname=%s" % parsed_dsn["dbname"])
+    print(" ".join(conn_opts))
diff --git a/utilities/soyuz-sampledata-setup.py b/utilities/soyuz-sampledata-setup.py
index 68aa035..0c44cd6 100755
--- a/utilities/soyuz-sampledata-setup.py
+++ b/utilities/soyuz-sampledata-setup.py
@@ -38,6 +38,7 @@ from lp.registry.interfaces.codeofconduct import ISignedCodeOfConductSet
 from lp.registry.interfaces.person import IPersonSet
 from lp.registry.interfaces.series import SeriesStatus
 from lp.registry.model.codeofconduct import SignedCodeOfConduct
+from lp.services.config import config
 from lp.services.database.interfaces import IPrimaryStore, IStandbyStore
 from lp.services.scripts.base import LaunchpadScript
 from lp.soyuz.enums import SourcePackageFormat
@@ -88,7 +89,10 @@ def check_preconditions(options):
     # run.  Don't even accept --force there.
     forbidden_configs = re.compile("(edge|lpnet|production)")
     current_config = os.getenv("LPCONFIG", "an unknown config")
-    if forbidden_configs.match(current_config):
+    if (
+        forbidden_configs.match(current_config)
+        or config.vhost.mainsite.hostname == "launchpad.net"
+    ):
         raise DoNotRunOnProduction(
             "I won't delete Ubuntu data on %s and you can't --force me."
             % current_config