Bug#1056303: pg_createcluster destroys data directory under certain conditions

2024-01-02 Thread Michael Banck
severity 1056303 important
tags 1056303 +patch
thanks

Hi,

On Mon, Nov 20, 2023 at 09:49:34AM +0100, Max Kellermann wrote:
> When I tried to use "pg_createcluster" to configure my pre-existing
> PostgreSQL data directory with a new Debian install, it deleted the
> whole cluster with all databases instead.  (This serious data loss
> justifies "severity critical" according to
> https://www.debian.org/Bugs/Developer#severities)
 
> Steps to reproduce:
> 
>  pg_createcluster 15 test
>  cp /etc/postgresql/15/test/postgresql.conf /var/lib/postgresql/15/test/

You missed to copy the other configuration files, as cautioned in the
manpage:

| If the data directory already exists, it is integrated into the
| postgresql-common structure by moving the configuration file and
| setting the data_directory option. Please note that this only works
| for data directories which were created directly with initdb, i.e.
| all the configuration files (postgresql.conf etc.) must be present in
| the data directory.

>  rm -r /etc/postgresql/15/test
>  pg_createcluster 15 test
> 
> This creates a new cluster just for the demo, then deletes the
> configuration directory, after copying the postgresql.conf to the data
> directory.
> 
> I expect the second "pg_createcluster" call to find the existing data
> directory and configure it; and it tries to do so indeed:
> 
>  Configuring already existing cluster (configuration:
>  /etc/postgresql/15/test, data: /var/lib/postgresql/15/test, owner:
>  103:111)
> 
> After it finds and moves a "postgresql.conf", it however fails to find
> "pg_hba.conf"

Right, see above.
 
>  Error: move_conffile: required configuration file 
> /var/lib/postgresql/15/test/pg_hba.conf does not exist
 
> This fails the whole operation, and apparently, "pg_createcluster"
> tries to roll back by invoking "pg_dropcluster 15 test 2>/dev/null",
> destroying all pre-existing data.
> 
> If "postgresql.conf" does not exist (or is empty), "pg_dropcluster" is
> not invoked.

This is because pg_creatcluster copies postgresql.conf first, and gets
version and cluster from it - if those are set, the data directory is
removed in the cleanup phase on error.
 
I think it would be sensible to only remove the data directory if it
actually got created by pg_createcluster, so I propose the attached
patch; I have also opened a merge request on Salsa for this:
https://salsa.debian.org/postgresql/postgresql-common/-/merge_requests/19


Michael
>From c1e61705ab9384a005a009ed6327bb40d9027afb Mon Sep 17 00:00:00 2001
From: Michael Banck 
Date: Tue, 2 Jan 2024 15:54:04 +0100
Subject: [PATCH] pg_createcluster: Do not remove existing data directory on
 failure. (Closes: #105630)

---
 pg_createcluster | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/pg_createcluster b/pg_createcluster
index 66f0b82..0c36561 100755
--- a/pg_createcluster
+++ b/pg_createcluster
@@ -361,6 +361,8 @@ if (-f "$datadir/PG_VERSION") {
 ($owneruid, $ownergid) = (stat "$datadir/PG_VERSION")[4,5];
 if ($existingver == $version) {
 print "Configuring already existing cluster (configuration: $confdir, data: $datadir, owner: $owneruid:$ownergid)\n";
+# Do not remove already existing data directory on errors
+$cleanup_cruft = 0;
 } else {
 error "$datadir contains a version $existingver cluster, but $version was requested";
 }
-- 
2.39.2



Bug#1056303: pg_createcluster destroys data directory under certain conditions

2023-11-20 Thread Max Kellermann
Package: postgresql-common
Version: 248
Severity: critical

When I tried to use "pg_createcluster" to configure my pre-existing
PostgreSQL data directory with a new Debian install, it deleted the
whole cluster with all databases instead.  (This serious data loss
justifies "severity critical" according to
https://www.debian.org/Bugs/Developer#severities)

Steps to reproduce:

 pg_createcluster 15 test
 cp /etc/postgresql/15/test/postgresql.conf /var/lib/postgresql/15/test/
 rm -r /etc/postgresql/15/test
 pg_createcluster 15 test

This creates a new cluster just for the demo, then deletes the
configuration directory, after copying the postgresql.conf to the data
directory.

I expect the second "pg_createcluster" call to find the existing data
directory and configure it; and it tries to do so indeed:

 Configuring already existing cluster (configuration: /etc/postgresql/15/test, 
data: /var/lib/postgresql/15/test, owner: 103:111)

After it finds and moves a "postgresql.conf", it however fails to find
"pg_hba.conf"

 Error: move_conffile: required configuration file 
/var/lib/postgresql/15/test/pg_hba.conf does not exist

This fails the whole operation, and apparently, "pg_createcluster"
tries to roll back by invoking "pg_dropcluster 15 test 2>/dev/null",
destroying all pre-existing data.

If "postgresql.conf" does not exist (or is empty), "pg_dropcluster" is
not invoked.