Merge pull request #132593 from rycee/postgresql-backup-compression

nixos postgresql-backup: add `compression` option

nixos/modules/services/backup/postgresql-backup.nix

@@ -7,28 +7,49 @@ let
   cfg = config.services.postgresqlBackup;
 
   postgresqlBackupService = db: dumpCmd:
-    {
+    let
+      compressSuffixes = {
+        "none" = "";
+        "gzip" = ".gz";
+        "zstd" = ".zstd";
+      };
+      compressSuffix = getAttr cfg.compression compressSuffixes;
+
+      compressCmd = getAttr cfg.compression {
+        "none" = "cat";
+        "gzip" = "${pkgs.gzip}/bin/gzip -c";
+        "zstd" = "${pkgs.zstd}/bin/zstd -c";
+      };
+
+      mkSqlPath = prefix: suffix: "${cfg.location}/${db}${prefix}.sql${suffix}";
+      curFile = mkSqlPath "" compressSuffix;
+      prevFile = mkSqlPath ".prev" compressSuffix;
+      prevFiles = map (mkSqlPath ".prev") (attrValues compressSuffixes);
+      inProgressFile = mkSqlPath ".in-progress" compressSuffix;
+    in {
       enable = true;
 
       description = "Backup of ${db} database(s)";
 
       requires = [ "postgresql.service" ];
 
-      path = [ pkgs.coreutils pkgs.gzip config.services.postgresql.package ];
+      path = [ pkgs.coreutils config.services.postgresql.package ];
 
       script = ''
         set -e -o pipefail
 
         umask 0077 # ensure backup is only readable by postgres user
 
-        if [ -e ${cfg.location}/${db}.sql.gz ]; then
-          mv ${cfg.location}/${db}.sql.gz ${cfg.location}/${db}.prev.sql.gz
+        if [ -e ${curFile} ]; then
+          rm -f ${toString prevFiles}
+          mv ${curFile} ${prevFile}
         fi
 
-        ${dumpCmd} | \
-          gzip -c > ${cfg.location}/${db}.in-progress.sql.gz
+        ${dumpCmd} \
+          | ${compressCmd} \
+          > ${inProgressFile}
 
-        mv ${cfg.location}/${db}.in-progress.sql.gz ${cfg.location}/${db}.sql.gz
+        mv ${inProgressFile} ${curFile}
       '';
 
       serviceConfig = {
@@ -87,7 +108,7 @@ in {
         default = "/var/backup/postgresql";
         type = types.path;
         description = ''
-          Location to put the gzipped PostgreSQL database dumps.
+          Path of directory where the PostgreSQL database dumps will be placed.
         '';
       };
 
@@ -101,6 +122,14 @@ in {
           when no databases where specified.
         '';
       };
+
+      compression = mkOption {
+        type = types.enum ["none" "gzip" "zstd"];
+        default = "gzip";
+        description = ''
+          The type of compression to use on the generated database dump.
+        '';
+      };
     };
 
   };

nixos/tests/postgresql.nix

@@ -43,6 +43,7 @@ let
     testScript = let
       backupName = if backup-all then "all" else "postgres";
       backupService = if backup-all then "postgresqlBackup" else "postgresqlBackup-postgres";
+      backupFileBase = "/var/backup/postgresql/${backupName}";
     in ''
       def check_count(statement, lines):
           return 'test $(sudo -u postgres psql postgres -tAc "{}"|wc -l) -eq {}'.format(
@@ -72,9 +73,32 @@ let
       with subtest("Backup service works"):
           machine.succeed(
               "systemctl start ${backupService}.service",
-              "zcat /var/backup/postgresql/${backupName}.sql.gz | grep '<test>ok</test>'",
+              "zcat ${backupFileBase}.sql.gz | grep '<test>ok</test>'",
               "ls -hal /var/backup/postgresql/ >/dev/console",
-              "stat -c '%a' /var/backup/postgresql/${backupName}.sql.gz | grep 600",
+              "stat -c '%a' ${backupFileBase}.sql.gz | grep 600",
+          )
+      with subtest("Backup service removes prev files"):
+          machine.succeed(
+              # Create dummy prev files.
+              "touch ${backupFileBase}.prev.sql{,.gz,.zstd}",
+              "chown postgres:postgres ${backupFileBase}.prev.sql{,.gz,.zstd}",
+
+              # Run backup.
+              "systemctl start ${backupService}.service",
+              "ls -hal /var/backup/postgresql/ >/dev/console",
+
+              # Since nothing has changed in the database, the cur and prev files
+              # should match.
+              "zcat ${backupFileBase}.sql.gz | grep '<test>ok</test>'",
+              "cmp ${backupFileBase}.sql.gz ${backupFileBase}.prev.sql.gz",
+
+              # The prev files with unused suffix should be removed.
+              "[ ! -f '${backupFileBase}.prev.sql' ]",
+              "[ ! -f '${backupFileBase}.prev.sql.zstd' ]",
+
+              # Both cur and prev file should only be accessible by the postgres user.
+              "stat -c '%a' ${backupFileBase}.sql.gz | grep 600",
+              "stat -c '%a' '${backupFileBase}.prev.sql.gz' | grep 600",
           )
       with subtest("Backup service fails gracefully"):
           # Sabotage the backup process
@@ -84,8 +108,8 @@ let
           )
           machine.succeed(
               "ls -hal /var/backup/postgresql/ >/dev/console",
-              "zcat /var/backup/postgresql/${backupName}.prev.sql.gz | grep '<test>ok</test>'",
-              "stat /var/backup/postgresql/${backupName}.in-progress.sql.gz",
+              "zcat ${backupFileBase}.prev.sql.gz | grep '<test>ok</test>'",
+              "stat ${backupFileBase}.in-progress.sql.gz",
           )
           # In a previous version, the second run would overwrite prev.sql.gz,
           # so we test a second run as well.
@@ -93,8 +117,8 @@ let
               "systemctl start ${backupService}.service",
           )
           machine.succeed(
-              "stat /var/backup/postgresql/${backupName}.in-progress.sql.gz",
-              "zcat /var/backup/postgresql/${backupName}.prev.sql.gz | grep '<test>ok</test>'",
+              "stat ${backupFileBase}.in-progress.sql.gz",
+              "zcat ${backupFileBase}.prev.sql.gz | grep '<test>ok</test>'",
           )