Merge pull request #94454 from aanderse/postgresql-cleanup

nixos/postgresql: fix several issues

nixos/doc/manual/release-notes/rl-2009.xml

@@ -615,6 +615,17 @@ services.dokuwiki."mywiki" = {
   };
   ...
 };
+</programlisting>
+    </para>
+   </listitem>
+   <listitem>
+    <para>
+      The <xref linkend="opt-services.postgresql.dataDir"/> option is now set to <literal>"/var/lib/postgresql/${cfg.package.psqlSchema}"</literal> regardless of your
+      <xref linkend="opt-system.stateVersion"/>. Users with an existing postgresql install that have a <xref linkend="opt-system.stateVersion"/> of <literal>17.09</literal> or below
+      should double check what the value of their <xref linkend="opt-services.postgresql.dataDir"/> option is (<literal>/var/db/postgresql</literal>) and then explicitly
+      set this value to maintain compatibility:
+<programlisting>
+services.postgresql.dataDir = "/var/db/postgresql";
 </programlisting>
     </para>
    </listitem>

nixos/modules/services/databases/postgresql.nix

@@ -21,7 +21,7 @@ let
       listen_addresses = '${if cfg.enableTCPIP then "*" else "localhost"}'
       port = ${toString cfg.port}
       ${cfg.extraConfig}
-    ''; 
+    '';
 
   groupAccessAvailable = versionAtLeast postgresql.version "11.0";
 
@@ -55,9 +55,13 @@ in
 
       dataDir = mkOption {
         type = types.path;
+        defaultText = "/var/lib/postgresql/\${config.services.postgresql.package.psqlSchema}";
         example = "/var/lib/postgresql/11";
         description = ''
-          Data directory for PostgreSQL.
+          The data directory for PostgreSQL. If left as the default value
+          this directory will automatically be created before the PostgreSQL server starts, otherwise
+          the sysadmin is responsible for ensuring the directory exists with appropriate ownership
+          and permissions.
         '';
       };
 
@@ -249,10 +253,7 @@ in
             else if versionAtLeast config.system.stateVersion "16.03" then pkgs.postgresql_9_5
             else throw "postgresql_9_4 was removed, please upgrade your postgresql version.");
 
-    services.postgresql.dataDir =
+    services.postgresql.dataDir = mkDefault "/var/lib/postgresql/${cfg.package.psqlSchema}";
-      mkDefault (if versionAtLeast config.system.stateVersion "17.09"
-                  then "/var/lib/postgresql/${cfg.package.psqlSchema}"
-                  else "/var/db/postgresql");
 
     services.postgresql.authentication = mkAfter
       ''
@@ -291,40 +292,28 @@ in
 
         preStart =
           ''
-            # Create data directory.
             if ! test -e ${cfg.dataDir}/PG_VERSION; then
-              mkdir -m 0700 -p ${cfg.dataDir}
+              # Cleanup the data directory.
               rm -f ${cfg.dataDir}/*.conf
-              chown -R postgres:postgres ${cfg.dataDir}
-            fi
-          ''; # */
 
-        script =
+              # Initialise the database.
-          ''
-            # Initialise the database.
-            if ! test -e ${cfg.dataDir}/PG_VERSION; then
               initdb -U ${cfg.superUser} ${concatStringsSep " " cfg.initdbArgs}
+
               # See postStart!
               touch "${cfg.dataDir}/.first_startup"
             fi
+
             ln -sfn "${configFile}" "${cfg.dataDir}/postgresql.conf"
             ${optionalString (cfg.recoveryConfig != null) ''
               ln -sfn "${pkgs.writeText "recovery.conf" cfg.recoveryConfig}" \
                 "${cfg.dataDir}/recovery.conf"
             ''}
-            ${optionalString (!groupAccessAvailable) ''
-              # postgresql pre 11.0 doesn't start if state directory mode is group accessible
-              chmod 0700 "${cfg.dataDir}"
-            ''}
-
-            exec postgres
           '';
 
-        serviceConfig =
+        serviceConfig = mkMerge [
           { ExecReload = "${pkgs.coreutils}/bin/kill -HUP $MAINPID";
             User = "postgres";
             Group = "postgres";
-            PermissionsStartOnly = true;
             RuntimeDirectory = "postgresql";
             Type = if versionAtLeast cfg.package.version "9.6"
                    then "notify"
@@ -338,36 +327,48 @@ in
             # Give Postgres a decent amount of time to clean up after
             # receiving systemd's SIGINT.
             TimeoutSec = 120;
-          };
 
-        # Wait for PostgreSQL to be ready to accept connections.
+            ExecStart = "${postgresql}/bin/postgres";
-        postStart =
-          ''
-            PSQL="${pkgs.utillinux}/bin/runuser -u ${cfg.superUser} -- psql --port=${toString cfg.port}"
 
-            while ! $PSQL -d postgres -c "" 2> /dev/null; do
+            # Wait for PostgreSQL to be ready to accept connections.
-                if ! kill -0 "$MAINPID"; then exit 1; fi
+            ExecStartPost =
-                sleep 0.1
+              let
-            done
+                setupScript = pkgs.writeScript "postgresql-setup" ''
+                  #!${pkgs.runtimeShell} -e
 
-            if test -e "${cfg.dataDir}/.first_startup"; then
+                  PSQL="${pkgs.utillinux}/bin/runuser -u ${cfg.superUser} -- psql --port=${toString cfg.port}"
-              ${optionalString (cfg.initialScript != null) ''
+
-                $PSQL -f "${cfg.initialScript}" -d postgres
+                  while ! $PSQL -d postgres -c "" 2> /dev/null; do
-              ''}
+                      if ! kill -0 "$MAINPID"; then exit 1; fi
-              rm -f "${cfg.dataDir}/.first_startup"
+                      sleep 0.1
-            fi
+                  done
-          '' + optionalString (cfg.ensureDatabases != []) ''
+
-            ${concatMapStrings (database: ''
+                  if test -e "${cfg.dataDir}/.first_startup"; then
-              $PSQL -tAc "SELECT 1 FROM pg_database WHERE datname = '${database}'" | grep -q 1 || $PSQL -tAc 'CREATE DATABASE "${database}"'
+                    ${optionalString (cfg.initialScript != null) ''
-            '') cfg.ensureDatabases}
+                      $PSQL -f "${cfg.initialScript}" -d postgres
-          '' + ''
+                    ''}
-            ${concatMapStrings (user: ''
+                    rm -f "${cfg.dataDir}/.first_startup"
-              $PSQL -tAc "SELECT 1 FROM pg_roles WHERE rolname='${user.name}'" | grep -q 1 || $PSQL -tAc 'CREATE USER "${user.name}"'
+                  fi
-              ${concatStringsSep "\n" (mapAttrsToList (database: permission: ''
+                '' + optionalString (cfg.ensureDatabases != []) ''
-                $PSQL -tAc 'GRANT ${permission} ON ${database} TO "${user.name}"'
+                  ${concatMapStrings (database: ''
-              '') user.ensurePermissions)}
+                    $PSQL -tAc "SELECT 1 FROM pg_database WHERE datname = '${database}'" | grep -q 1 || $PSQL -tAc 'CREATE DATABASE "${database}"'
-            '') cfg.ensureUsers}
+                  '') cfg.ensureDatabases}
-          '';
+                '' + ''
+                  ${concatMapStrings (user: ''
+                    $PSQL -tAc "SELECT 1 FROM pg_roles WHERE rolname='${user.name}'" | grep -q 1 || $PSQL -tAc 'CREATE USER "${user.name}"'
+                    ${concatStringsSep "\n" (mapAttrsToList (database: permission: ''
+                      $PSQL -tAc 'GRANT ${permission} ON ${database} TO "${user.name}"'
+                    '') user.ensurePermissions)}
+                  '') cfg.ensureUsers}
+                '';
+              in
+                "+${setupScript}";
+          }
+          (mkIf (cfg.dataDir == "/var/lib/postgresql/${cfg.package.psqlSchema}") {
+            StateDirectory = "postgresql postgresql/${cfg.package.psqlSchema}";
+            StateDirectoryMode = if groupAccessAvailable then "0750" else "0700";
+          })
+        ];
 
         unitConfig.RequiresMountsFor = "${cfg.dataDir}";
       };

nixos/tests/postgresql-wal-receiver.nix

@@ -28,6 +28,10 @@ let
     meta.maintainers = with maintainers; [ pacien ];
 
     machine = { ... }: {
+      # Needed because this test uses a non-default 'services.postgresql.dataDir'.
+      systemd.tmpfiles.rules = [
+        "d /var/db/postgresql 0700 postgres postgres"
+      ];
       services.postgresql = {
         package = postgresqlPackage;
         enable = true;