From b643e0aee32a1c12ee48da3d62895e4f8f77af79 Mon Sep 17 00:00:00 2001
From: danbst <abcz2.uprola@gmail.com>
Date: Wed, 24 Jul 2019 23:34:21 +0300
Subject: [PATCH] addressed review comments and some fixes

---
 .../modules/services/databases/postgresql.nix | 26 ++++++---------
 nixos/tests/postgresql.nix                    | 33 +++----------------
 2 files changed, 15 insertions(+), 44 deletions(-)

diff --git a/nixos/modules/services/databases/postgresql.nix b/nixos/modules/services/databases/postgresql.nix
index b89f4a57253a..eb1ac6bcb307 100644
--- a/nixos/modules/services/databases/postgresql.nix
+++ b/nixos/modules/services/databases/postgresql.nix
@@ -85,17 +85,9 @@ in
       groupAccess = mkOption {
         type = types.bool;
         default = false;
-        example = true;
         description = ''
           Allow read access for group (0750 mask for data directory).
-          Supported only for PostgreSQL 11+. PostgreSQL 10 and lower doesn't
-          support starting server with 0750 mask, but a workaround like
-          <programlisting>
-          systemd.services.postgresql.postStart = lib.mkAfter '''
-            chmod 750 ''${config.services.postgresql.dataDir}
-          ''';
-          </programlisting>
-          may be used instead.
+          Supported only for PostgreSQL 11+.
         '';
       };
 
@@ -119,11 +111,12 @@ in
         '';
       };
 
-      initdbFlags = mkOption {
+      initdbArgs = mkOption {
         type = with types; listOf str;
         default = [];
+        example = [ "--data-checksums" ];
         description = ''
-          Additional flags passed to <literal>initdb<literal> during data dir
+          Additional arguments passed to <literal>initdb<literal> during data dir
           initialisation.
         '';
       };
@@ -289,8 +282,8 @@ in
                   then "/var/lib/postgresql/${cfg.package.psqlSchema}"
                   else "/var/db/postgresql");
 
-    services.postgresql.initdbFlags =
-      mkDefault (lib.optional cfg.groupAccess "--allow-group-access");
+    services.postgresql.initdbArgs =
+      mkBefore (optional cfg.groupAccess "--allow-group-access");
 
     services.postgresql.authentication = mkAfter
       ''
@@ -329,7 +322,7 @@ in
             if ! test -e ${cfg.dataDir}/PG_VERSION; then
               mkdir -m ${dirMode} -p ${cfg.dataDir}
               rm -f ${cfg.dataDir}/*.conf
-              chown -R postgres ${cfg.dataDir}
+              chown -R postgres:postgres ${cfg.dataDir}
             fi
           ''; # */
 
@@ -337,7 +330,7 @@ in
           ''
             # Initialise the database.
             if ! test -e ${cfg.dataDir}/PG_VERSION; then
-              initdb -U ${cfg.superUser} ${lib.concatStringsSep " " cfg.initdbFlags}
+              initdb -U ${cfg.superUser} ${concatStringsSep " " cfg.initdbArgs}
               # See postStart!
               touch "${cfg.dataDir}/.first_startup"
             fi
@@ -346,6 +339,7 @@ in
               ln -sfn "${pkgs.writeText "recovery.conf" cfg.recoveryConfig}" \
                 "${cfg.dataDir}/recovery.conf"
             ''}
+            echo chmod ${dirMode} "${cfg.dataDir}"
             chmod ${dirMode} "${cfg.dataDir}"
 
             exec postgres
@@ -357,7 +351,7 @@ in
             Group = "postgres";
             PermissionsStartOnly = true;
             RuntimeDirectory = "postgresql";
-            Type = if lib.versionAtLeast cfg.package.version "9.6"
+            Type = if versionAtLeast cfg.package.version "9.6"
                    then "notify"
                    else "simple";
 
diff --git a/nixos/tests/postgresql.nix b/nixos/tests/postgresql.nix
index 81ec4d698b66..433a64e9fab8 100644
--- a/nixos/tests/postgresql.nix
+++ b/nixos/tests/postgresql.nix
@@ -84,53 +84,30 @@ in
           services.postgresql.package = pkgs.postgresql_11;
           services.postgresql.dataDir = dataDir;
 
-          # users.groups.backup = {};
-          users.users.backup.isNormalUser = true;
-          users.users.backup.group = "wheel";
-
-          systemd.tmpfiles.rules = [
-            "d ${dataDir} 0750 postgres wheel -"
-          ];
+          users.users.admin.isNormalUser = true;
+          users.users.admin.extraGroups = [ "postgres" ];
 
           nesting.clone = [
             {
               services.postgresql.groupAccess = true;
             }
-
-            ({ config, lib, ... }: {
-              services.postgresql.package = lib.mkForce pkgs.postgresql_10;
-              services.postgresql.dataDir = lib.mkForce (dataDir + "_10");
-              systemd.tmpfiles.rules = [
-                "d ${dataDir}_10 0750 postgres wheel -"
-              ];
-              systemd.services.postgresql.postStart = lib.mkAfter ''
-                chmod 750 ${config.services.postgresql.dataDir}
-              '';
-            })
           ];
         };
     testScript = { nodes, ... }: let
       c1 = "${nodes.machine.config.system.build.toplevel}/fine-tune/child-1";
-      c2 = "${nodes.machine.config.system.build.toplevel}/fine-tune/child-2";
     in ''
       $machine->start;
       $machine->waitForUnit("postgresql");
       $machine->succeed("echo select 1 | sudo -u postgres psql");
 
       # by default, mode is 0700
-      $machine->fail("sudo -u backup ls ${dataDir}");
+      $machine->fail("sudo -u admin ls ${dataDir}");
 
       $machine->succeed("${c1}/bin/switch-to-configuration test >&2");
       $machine->succeed("journalctl -u postgresql | grep -q -i stopped"); # was restarted
       $machine->succeed("echo select 1 | sudo -u postgres psql"); # works after restart
-      $machine->succeed("sudo -u backup ls ${dataDir}");
-
-      # This tests a hack for PG <11: restore permissions to 0700 just before PG starts
-      # and put it back to 0750 after PG had started
-      $machine->succeed("${c2}/bin/switch-to-configuration test >&2");
-      $machine->succeed("systemctl restart postgresql");
-      $machine->waitForUnit("postgresql");   # works after restart
-      $machine->succeed("sudo -u backup ls ${dataDir}_10");
+      $machine->succeed("sudo -u admin ls -la / >&2");
+      $machine->succeed("sudo -u admin ls ${dataDir}");
 
       $machine->shutdown;
     '';