commit da32948edb28fa654ef2bdf8c80afecbc15eedcb
parent 0a4f998a33cd8597d0488d74dc52ca865afb8aa2
Author: Florian Dold <dold@taler.net>
Date: Tue, 8 Sep 2026 15:39:44 +0200
database tools: leave migrations to the operator
Remove the implicit postinst dbinit call and require GC to connect to a
current schema without creating tables or replacing stored procedures.
Keep reset precedence and return immediately after migration failure.
Make dbconfig validate its local database target before provisioning,
query role and database existence explicitly, and propagate SQL
failures. Document the stop, backup, install, migrate and start
sequence.
Stop the target and slice on removal, remove unreachable postinst code,
and preserve explicit service timing during upgrades.
Diffstat:
6 files changed, 110 insertions(+), 95 deletions(-)
diff --git a/contrib/taler-merchant-dbconfig b/contrib/taler-merchant-dbconfig
@@ -49,7 +49,7 @@ while getopts 'c:hrsu:' OPTION; do
DBUSER="$OPTARG"
;;
?)
- echo "Unrecognized command line option '$OPTION'" 1 &>2
+ echo 'Invalid command-line option; use -h for help.' >&2
exit 1
;;
esac
@@ -91,66 +91,71 @@ if ! id "$DBUSER" >/dev/null; then
exit 1
fi
-echo "Setting up database user $DBUSER." 1>&2
-
-if ! sudo -i -u postgres createuser "$DBUSER" 2>/dev/null; then
- echo "Database user '$DBUSER' already existed. Continuing anyway." 1>&2
-fi
-
-# Allow user to change the session_replication_role setting,
-# required during migrations.
-
-ret=0
-sudo -i -u postgres psql -v dbuser="$DBUSER" <<'EOF' || ret=$?
-GRANT SET ON PARAMETER session_replication_role TO :"dbuser";
-EOF
-
-if [[ $ret -ne 0 ]]; then
- echo "Failed to grant permissions to $DBUSER" >&2
- exit 1
-fi
-
+# Resolve and validate the target before making any database changes. The
+# provisioning commands below administer the default local PostgreSQL cluster.
DBPATH=$(taler-merchant-config \
-c "$CFGFILE" \
-s merchantdb-postgres \
-o CONFIG)
-if ! echo "$DBPATH" | grep "postgres://" >/dev/null; then
- echo "Invalid database configuration value '$DBPATH'." 1>&2
+if [[ ! "$DBPATH" =~ ^postgres:///([a-zA-Z0-9_.-]+)$ ]]; then
+ echo "Database provisioning requires postgres:///NAME (letters, digits, _, . or -)." >&2
+ echo "For other connections, provision the database separately and run taler-merchant-dbinit -c $CFGFILE as the database owner." >&2
exit 1
fi
+DBNAME="${BASH_REMATCH[1]}"
-DBNAME=$(echo "$DBPATH" |
- sed \
- -e "s/postgres:\/\/.*\///" \
- -e "s/?.*//")
-
-if sudo -i -u postgres psql "$DBNAME" </dev/null 2>/dev/null; then
- if [ 1 = "$RESET_DB" ]; then
- echo "Deleting existing database $DBNAME." 1>&2
- if ! sudo -i -u postgres dropdb "$DBNAME"; then
- echo "Failed to delete existing database '$DBNAME'"
- exit 1
- fi
- DO_CREATE=1
- else
- echo "Database '$DBNAME' already exists, continuing anyway."
- DO_CREATE=0
- fi
-else
- DO_CREATE=1
+# Query the catalogs explicitly: failure to connect is not evidence that a
+# role or database is absent. -X ignores psqlrc, and SQL errors must be fatal.
+ROLE_EXISTS=$(sudo -i -u postgres psql -X -At \
+ --set=ON_ERROR_STOP=1 --dbname=postgres --set=dbuser="$DBUSER" <<'EOF'
+SELECT EXISTS (SELECT FROM pg_roles WHERE rolname = :'dbuser');
+EOF
+)
+if [ "$ROLE_EXISTS" = f ]; then
+ echo "Creating database user $DBUSER." >&2
+ sudo -i -u postgres createuser -- "$DBUSER"
+elif [ "$ROLE_EXISTS" != t ]; then
+ echo "Could not determine whether database user $DBUSER exists." >&2
+ exit 1
fi
-if [ 1 = "$DO_CREATE" ]; then
- echo "Creating database $DBNAME." 1>&2
- if ! sudo -i -u postgres createdb -O "$DBUSER" "$DBNAME"; then
- echo "Failed to create database '$DBNAME'"
+# Required during migrations; this does not restore other privileges that an
+# administrator may have revoked from the database owner.
+sudo -i -u postgres psql -X --set=ON_ERROR_STOP=1 \
+ --dbname=postgres --set=dbuser="$DBUSER" <<'EOF'
+GRANT SET ON PARAMETER session_replication_role TO :"dbuser";
+EOF
+
+DB_EXISTS=$(sudo -i -u postgres psql -X -At \
+ --set=ON_ERROR_STOP=1 --dbname=postgres --set=dbname="$DBNAME" <<'EOF'
+SELECT EXISTS (SELECT FROM pg_database WHERE datname = :'dbname');
+EOF
+)
+case "$DB_EXISTS" in
+ t)
+ if [ 1 = "$RESET_DB" ]; then
+ echo "Deleting existing database $DBNAME." >&2
+ sudo -i -u postgres dropdb -- "$DBNAME"
+ DB_EXISTS=f
+ else
+ echo "Database '$DBNAME' already exists, continuing anyway." >&2
+ fi
+ ;;
+ f) ;;
+ *)
+ echo "Could not determine whether database $DBNAME exists." >&2
exit 1
- fi
+ ;;
+esac
+
+if [ "$DB_EXISTS" = f ]; then
+ echo "Creating database $DBNAME." >&2
+ sudo -i -u postgres createdb -O "$DBUSER" -- "$DBNAME"
fi
if [ 0 = "$SKIP_DBINIT" ]; then
- echo "Initializing database $DBNAME." 1>&2
+ echo "Initializing or upgrading database $DBNAME." >&2
if ! sudo -u "$DBUSER" "$DBINIT" -c "$CFGFILE"; then
echo "Failed to initialize database schema." >&2
echo "Command was: sudo -u $DBUSER $DBINIT -c $CFGFILE" >&2
diff --git a/debian/README-packaging.md b/debian/README-packaging.md
@@ -1,6 +1,21 @@
-## Database Setup
+## Database setup and upgrades
-We use dbconfig-common (https://www.debian.org/doc/manuals/dbconfig-common/)
-for database setup.
+For the standard local PostgreSQL setup, run `sudo taler-merchant-dbconfig`.
+It uses `/etc/taler-merchant/taler-merchant.conf`, provisions missing database
+objects, and invokes `taler-merchant-dbinit` as `taler-merchant-httpd`.
+Existing databases are preserved unless the destructive `-r` option is given.
+The package does not use dbconfig-common.
+For an upgrade, stop `taler-merchant.target` before installing packages, back up
+the database, install the packages, and run `sudo taler-merchant-dbconfig`.
+Start the desired services only after migration succeeds. Coordinate all hosts
+sharing the database. The helper does not stop services or make backups.
+For remote databases or separately managed privileges, provision the database
+separately and run `taler-merchant-dbinit -c /path/to/merchant.conf` as the
+appropriate migration identity. Restore any revoked migration privileges first.
+
+Package configuration and scheduled garbage collection do not run migrations.
+A package upgrade preserves running/stopped services, so the operator must stop
+them before installing an incompatible version: automatic process recovery can
+otherwise launch new binaries against the old schema.
diff --git a/debian/taler-merchant.NEWS b/debian/taler-merchant.NEWS
@@ -0,0 +1,15 @@
+taler-merchant (1.6.16) unstable; urgency=medium
+
+ Database upgrades now require explicit operator action. Before installing
+ a schema-changing upgrade, stop taler-merchant.target (on every host sharing
+ the database) and back up the database. After installation, run
+ taler-merchant-dbconfig for the standard local PostgreSQL setup, or dbinit
+ with explicit configuration and migration credentials for custom setups.
+ Start services only after migration succeeds. Do not use dbconfig -r:
+ it deletes the database. Scheduled GC no longer applies migrations.
+
+ Older packages may stop and disable the target during their removal phase
+ of an upgrade. Boot enablement is restored, but services remain stopped
+ until explicitly started. Subsequent upgrades preserve service state.
+
+ -- Florian Dold <dold@taler.net> Tue, 08 Sep 2026 12:00:00 +0200
diff --git a/debian/taler-merchant.postinst b/debian/taler-merchant.postinst
@@ -5,38 +5,6 @@ set -e
# Earlier packages disabled the target during upgrade and left this marker.
MARKER="/var/lib/taler-merchant/.was-enabled"
-SERVICES="taler-merchant-depositcheck taler-merchant-exchangekeyupdate taler-merchant-donaukeyupdate taler-merchant-httpd taler-merchant-kyccheck taler-merchant-reconciliation taler-merchant-report-generator taler-merchant-webhook taler-merchant-wirewatch"
-
-if [ -d /run/systemd/system ];
-then
- systemctl --system daemon-reload >/dev/null || true
-fi
-if [ "$1" = "remove" ];
-then
- if [ -x "/usr/bin/deb-systemd-helper" ];
- then
- for SERVICE in $SERVICES
- do
- deb-systemd-helper mask "${SERVICE}.service" >/dev/null || true
- done
- deb-systemd-helper mask 'taler-merchant.target' >/dev/null || true
- fi
-fi
-
-if [ "$1" = "purge" ];
-then
- if [ -x "/usr/bin/deb-systemd-helper" ];
- then
- for SERVICE in $SERVICES
- do
- deb-systemd-helper purge "${SERVICE}.service" >/dev/null || true
- deb-systemd-helper unmask "${SERVICE}.service" >/dev/null || true
- done
- deb-systemd-helper purge 'taler-merchant.target' >/dev/null || true
- deb-systemd-helper unmask 'taler-merchant.target' >/dev/null || true
- fi
-fi
-
TALER_HOME="/var/lib/taler-merchant"
. /usr/share/debconf/confmodule
@@ -65,11 +33,6 @@ configure)
/etc/taler-merchant/secrets/merchant-db.secret.conf
fi
- if [ -x /usr/bin/taler-merchant-dbinit ];
- then
- /usr/bin/taler-merchant-dbinit >/dev/null 2>&1 || true
- fi
-
# remove manual enables, only the target should be enabled!
for s in taler-merchant-httpd taler-merchant-depositcheck \
taler-merchant-exchangekeyupdate taler-merchant-donaukeyupdate \
@@ -86,6 +49,7 @@ configure)
# The marker does not record which services were deliberately stopped,
# so the administrator must explicitly start the desired services.
systemctl enable taler-merchant.target || true
+ echo "Merchant services remain stopped; complete the database upgrade before starting them." >&2
fi
# Cleanup marker file
diff --git a/debian/taler-merchant.prerm b/debian/taler-merchant.prerm
@@ -1,6 +1,15 @@
#!/bin/sh
+
set -e
+# --no-start also suppresses debhelper's stop-on-remove snippet on Trixie.
+# Stop the product explicitly on removal, while leaving upgrades passive.
+if [ -d /run/systemd/system ] && [ "$1" = remove ];
+then
+ deb-systemd-invoke stop 'taler-merchant.target' >/dev/null || true
+ deb-systemd-invoke stop 'taler-merchant.slice' >/dev/null || true
+fi
+
#DEBHELPER#
exit 0
diff --git a/src/merchant-tools/taler-merchant-dbinit.c b/src/merchant-tools/taler-merchant-dbinit.c
@@ -60,7 +60,12 @@ run (void *cls,
{
struct TALER_MERCHANTDB_PostgresContext *pg;
- pg = TALER_MERCHANTDB_connect_admin (cfg);
+ /* Reset retains precedence over GC. Scheduled GC must never migrate. */
+ if (reset_db)
+ gc_db = 0;
+ pg = gc_db
+ ? TALER_MERCHANTDB_connect (cfg)
+ : TALER_MERCHANTDB_connect_admin (cfg);
if (NULL == pg)
{
fprintf (stderr,
@@ -81,13 +86,6 @@ run (void *cls,
}
gc_db = 0; /* not useful */
}
- if (GNUNET_OK !=
- TALER_MERCHANTDB_create_tables (pg))
- {
- global_ret = 1;
- GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
- "Failed to initialize tables\n");
- }
if (gc_db)
{
if (GNUNET_OK !=
@@ -100,6 +98,15 @@ run (void *cls,
TALER_MERCHANTDB_disconnect (pg);
return;
}
+ if (GNUNET_OK !=
+ TALER_MERCHANTDB_create_tables (pg))
+ {
+ global_ret = 1;
+ GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
+ "Failed to initialize tables\n");
+ TALER_MERCHANTDB_disconnect (pg);
+ return;
+ }
TALER_MERCHANTDB_disconnect (pg);
}
