Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nixos/postgresql: deprecate ensurePermissions, fix ensureUsers for postgresql15 #266270

Merged
merged 8 commits into from
Nov 17, 2023
3 changes: 3 additions & 0 deletions nixos/doc/manual/release-notes/rl-2311.section.md
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,9 @@

## Backward Incompatibilities {#sec-release-23.11-incompatibilities}

- `services.postgresql.ensurePermissions` has been deprecated in favor of `services.postgresql.ensureUsers.*.ensureDBOwnership` which simplifies the setup of database owned by a certain system user
in local database contexts (which make use of peer authentication via UNIX sockets), migration guidelines were provided in the NixOS manual, please refer to them if you are affected by a PostgreSQL 15 changing the way `GRANT ALL PRIVILEGES` is working. `services.postgresql.ensurePermissions` will be removed in 24.05. All NixOS modules were migrated using one of the strategy, e.g. `ensureDBOwnership` or `postStart`. More about this situation can be learnt in https://github.com/NixOS/nixpkgs/pull/266270.

- `network-online.target` has been fixed to no longer time out for systems with `networking.useDHCP = true` and `networking.useNetworkd = true`.
Workarounds for this can be removed.

Expand Down
58 changes: 47 additions & 11 deletions nixos/modules/services/databases/postgresql.nix
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,12 @@ in
ensurePermissions = mkOption {
bendlas marked this conversation as resolved.
Show resolved Hide resolved
type = types.attrsOf types.str;
default = {};
visible = false; # This option has been deprecated.
description = lib.mdDoc ''
This option is DEPRECATED and should not be used in nixpkgs anymore,
use `ensureDBOwnership` instead. It can also break with newer
versions of PostgreSQL (≥ 15).

Permissions to ensure for the user, specified as an attribute set.
The attribute names specify the database and tables to grant the permissions for.
The attribute values specify the permissions to grant. You may specify one or
Expand All @@ -187,6 +192,16 @@ in
'';
};

ensureDBOwnership = mkOption {
type = types.bool;
default = false;
bendlas marked this conversation as resolved.
Show resolved Hide resolved
description = mdDoc ''
Grants the user ownership to a database with the same name.
This database must be defined manually in
[](#opt-services.postgresql.ensureDatabases).
'';
};

ensureClauses = mkOption {
description = lib.mdDoc ''
An attrset of clauses to grant to the user. Under the hood this uses the
Expand Down Expand Up @@ -338,26 +353,21 @@ in
});
default = [];
description = lib.mdDoc ''
Ensures that the specified users exist and have at least the ensured permissions.
Ensures that the specified users exist.
The PostgreSQL users will be identified using peer authentication. This authenticates the Unix user with the
same name only, and that without the need for a password.
This option will never delete existing users or remove permissions, especially not when the value of this
option is changed. This means that users created and permissions assigned once through this option or
otherwise have to be removed manually.
This option will never delete existing users or remove DB ownership of databases
once granted with `ensureDBOwnership = true;`. This means that this must be
cleaned up manually when changing after changing the config in here.
'';
example = literalExpression ''
[
{
name = "nextcloud";
ensurePermissions = {
"DATABASE nextcloud" = "ALL PRIVILEGES";
};
}
{
name = "superuser";
ensurePermissions = {
"ALL TABLES IN SCHEMA public" = "ALL PRIVILEGES";
};
ensureDBOwnership = true;
}
]
'';
Expand Down Expand Up @@ -445,6 +455,27 @@ in

config = mkIf cfg.enable {

assertions = map ({ name, ensureDBOwnership, ... }: {
assertion = ensureDBOwnership -> builtins.elem name cfg.ensureDatabases;
message = ''
For each database user defined with `services.postgresql.ensureUsers` and
`ensureDBOwnership = true;`, a database with the same name must be defined
in `services.postgresql.ensureDatabases`.

Offender: ${name} has not been found among databases.
'';
}) cfg.ensureUsers;
# `ensurePermissions` is now deprecated, let's avoid it.
warnings = lib.optional (any ({ ensurePermissions, ... }: ensurePermissions != {}) cfg.ensureUsers) "
`services.postgresql.*.ensurePermissions` is used in your expressions,
this option is known to be broken with newer PostgreSQL versions,
consider migrating to `services.postgresql.*.ensureDBOwnership` or
consult the release notes or manual for more migration guidelines.

This option will be removed in NixOS 24.05 unless it sees significant
maintenance improvements.
";

services.postgresql.settings =
{
hba_file = "${pkgs.writeText "pg_hba.conf" cfg.authentication}";
Expand Down Expand Up @@ -556,12 +587,15 @@ in
${
concatMapStrings
(user:
let
let
userPermissions = concatStringsSep "\n"
(mapAttrsToList
(database: permission: ''$PSQL -tAc 'GRANT ${permission} ON ${database} TO "${user.name}"' '')
user.ensurePermissions
);
dbOwnershipStmt = optionalString
user.ensureDBOwnership
''$PSQL -tAc 'ALTER DATABASE "${user.name}" OWNER TO "${user.name}";' '';

filteredClauses = filterAttrs (name: value: value != null) user.ensureClauses;

Expand All @@ -572,6 +606,8 @@ in
$PSQL -tAc "SELECT 1 FROM pg_roles WHERE rolname='${user.name}'" | grep -q 1 || $PSQL -tAc 'CREATE USER "${user.name}"'
${userPermissions}
${userClauses}

${dbOwnershipStmt}
''
)
cfg.ensureUsers
Expand Down
4 changes: 2 additions & 2 deletions nixos/modules/services/development/zammad.nix
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ in

assertions = [
{
assertion = cfg.database.createLocally -> cfg.database.user == "zammad";
assertion = cfg.database.createLocally -> cfg.database.user == "zammad" && cfg.database.name == "zammad";
message = "services.zammad.database.user must be set to \"zammad\" if services.zammad.database.createLocally is set to true";
}
{
Expand All @@ -231,7 +231,7 @@ in
ensureUsers = [
{
name = cfg.database.user;
ensurePermissions = { "DATABASE ${cfg.database.name}" = "ALL PRIVILEGES"; };
ensureDBOwnership = true;
}
];
};
Expand Down
2 changes: 1 addition & 1 deletion nixos/modules/services/finance/odoo.nix
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ in
ensureDatabases = [ "odoo" ];
ensureUsers = [{
name = "odoo";
ensurePermissions = { "DATABASE odoo" = "ALL PRIVILEGES"; };
ensureDBOwnership = true;
}];
};
});
Expand Down
2 changes: 1 addition & 1 deletion nixos/modules/services/mail/listmonk.nix
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ in {

ensureUsers = [{
name = "listmonk";
ensurePermissions = { "DATABASE listmonk" = "ALL PRIVILEGES"; };
ensureDBOwnership = true;
}];

ensureDatabases = [ "listmonk" ];
Expand Down
14 changes: 11 additions & 3 deletions nixos/modules/services/mail/roundcube.nix
Original file line number Diff line number Diff line change
Expand Up @@ -179,14 +179,22 @@ in
};
};

assertions = [
{
assertion = localDB -> cfg.database.username == cfg.database.dbname;
message = ''
When setting up a DB and its owner user, the owner and the DB name must be
equal!
'';
}
];

services.postgresql = mkIf localDB {
enable = true;
ensureDatabases = [ cfg.database.dbname ];
ensureUsers = [ {
name = cfg.database.username;
ensurePermissions = {
"DATABASE ${cfg.database.username}" = "ALL PRIVILEGES";
};
ensureDBOwnership = true;
} ];
};

Expand Down
10 changes: 4 additions & 6 deletions nixos/modules/services/mail/sympa.nix
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ in
default = null;
example = "/run/keys/sympa-dbpassword";
description = lib.mdDoc ''
A file containing the password for {option}`services.sympa.database.user`.
A file containing the password for {option}`services.sympa.database.name`.
'';
};

Expand Down Expand Up @@ -342,6 +342,7 @@ in

db_type = cfg.database.type;
db_name = cfg.database.name;
db_user = cfg.database.name;
}
// (optionalAttrs (cfg.database.host != null) {
db_host = cfg.database.host;
Expand All @@ -355,9 +356,6 @@ in
// (optionalAttrs (cfg.database.port != null) {
db_port = cfg.database.port;
})
// (optionalAttrs (cfg.database.user != null) {
db_user = cfg.database.user;
})
// (optionalAttrs (cfg.mta.type == "postfix") {
sendmail_aliases = "${dataDir}/sympa_transport";
aliases_program = "${pkgs.postfix}/bin/postmap";
Expand Down Expand Up @@ -393,7 +391,7 @@ in
users.groups.${group} = {};

assertions = [
{ assertion = cfg.database.createLocally -> cfg.database.user == user;
{ assertion = cfg.database.createLocally -> cfg.database.user == user && cfg.database.name == cfg.database.user;
message = "services.sympa.database.user must be set to ${user} if services.sympa.database.createLocally is set to true";
}
{ assertion = cfg.database.createLocally -> cfg.database.passwordFile == null;
Expand Down Expand Up @@ -579,7 +577,7 @@ in
ensureDatabases = [ cfg.database.name ];
ensureUsers = [
{ name = cfg.database.user;
ensurePermissions = { "DATABASE ${cfg.database.name}" = "ALL PRIVILEGES"; };
ensureDBOwnership = true;
}
];
};
Expand Down
4 changes: 2 additions & 2 deletions nixos/modules/services/matrix/matrix-sliding-sync.nix
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,9 @@ in
services.postgresql = lib.optionalAttrs cfg.createDatabase {
enable = true;
ensureDatabases = [ "matrix-sliding-sync" ];
ensureUsers = [ rec {
ensureUsers = [ {
name = "matrix-sliding-sync";
ensurePermissions."DATABASE \"${name}\"" = "ALL PRIVILEGES";
ensureDBOwnership = true;
} ];
};

Expand Down
4 changes: 1 addition & 3 deletions nixos/modules/services/matrix/mautrix-facebook.nix
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,7 @@ in {
ensureDatabases = ["mautrix-facebook"];
ensureUsers = [{
name = "mautrix-facebook";
ensurePermissions = {
"DATABASE \"mautrix-facebook\"" = "ALL PRIVILEGES";
};
ensureDBOwnership = true;
}];
};

Expand Down
4 changes: 1 addition & 3 deletions nixos/modules/services/misc/atuin.nix
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,7 @@ in
enable = true;
ensureUsers = [{
name = "atuin";
ensurePermissions = {
"DATABASE atuin" = "ALL PRIVILEGES";
};
ensureDBOwnership = true;
}];
ensureDatabases = [ "atuin" ];
};
Expand Down
10 changes: 9 additions & 1 deletion nixos/modules/services/misc/forgejo.nix
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,14 @@ in
assertion = cfg.database.createDatabase -> useSqlite || cfg.database.user == cfg.user;
message = "services.forgejo.database.user must match services.forgejo.user if the database is to be automatically provisioned";
}
{ assertion = cfg.database.createDatabase && usePostgresql -> cfg.database.user == cfg.database.name;
message = ''
When creating a database via NixOS, the db user and db name must be equal!
If you already have an existing DB+user and this assertion is new, you can safely set
`services.forgejo.createDatabase` to `false` because removal of `ensureUsers`
and `ensureDatabases` doesn't have any effect.
'';
}
];

services.forgejo.settings = {
Expand Down Expand Up @@ -423,7 +431,7 @@ in
ensureUsers = [
{
name = cfg.database.user;
ensurePermissions = { "DATABASE ${cfg.database.name}" = "ALL PRIVILEGES"; };
ensureDBOwnership = true;
}
];
};
Expand Down
10 changes: 9 additions & 1 deletion nixos/modules/services/misc/gitea.nix
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,14 @@ in
{ assertion = cfg.database.createDatabase -> useSqlite || cfg.database.user == cfg.user;
message = "services.gitea.database.user must match services.gitea.user if the database is to be automatically provisioned";
}
{ assertion = cfg.database.createDatabase && usePostgresql -> cfg.database.user == cfg.database.name;
message = ''
When creating a database via NixOS, the db user and db name must be equal!
If you already have an existing DB+user and this assertion is new, you can safely set
`services.gitea.createDatabase` to `false` because removal of `ensureUsers`
and `ensureDatabases` doesn't have any effect.
'';
}
];

services.gitea.settings = {
Expand Down Expand Up @@ -461,7 +469,7 @@ in
ensureDatabases = [ cfg.database.name ];
ensureUsers = [
{ name = cfg.database.user;
ensurePermissions = { "DATABASE ${cfg.database.name}" = "ALL PRIVILEGES"; };
ensureDBOwnership = true;
}
];
};
Expand Down
4 changes: 2 additions & 2 deletions nixos/modules/services/misc/redmine.nix
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ in
{ assertion = cfg.database.passwordFile != null || cfg.database.socket != null;
message = "one of services.redmine.database.socket or services.redmine.database.passwordFile must be set";
}
{ assertion = cfg.database.createLocally -> cfg.database.user == cfg.user;
{ assertion = cfg.database.createLocally -> cfg.database.user == cfg.user && cfg.database.user == cfg.database.name;
message = "services.redmine.database.user must be set to ${cfg.user} if services.redmine.database.createLocally is set true";
}
{ assertion = cfg.database.createLocally -> cfg.database.socket != null;
Expand Down Expand Up @@ -315,7 +315,7 @@ in
ensureDatabases = [ cfg.database.name ];
ensureUsers = [
{ name = cfg.database.user;
ensurePermissions = { "DATABASE ${cfg.database.name}" = "ALL PRIVILEGES"; };
ensureDBOwnership = true;
}
];
};
Expand Down
10 changes: 7 additions & 3 deletions nixos/modules/services/misc/sourcehut/service.nix
Original file line number Diff line number Diff line change
Expand Up @@ -249,10 +249,13 @@ in
ensureDatabases = [ srvCfg.postgresql.database ];
ensureUsers = map (name: {
inherit name;
ensurePermissions = { "DATABASE \"${srvCfg.postgresql.database}\"" = "ALL PRIVILEGES"; };
# We don't use it because we have a special default database name with dots.
# TODO(for maintainers of sourcehut): migrate away from custom preStart script.
ensureDBOwnership = false;
}) [srvCfg.user];
};


services.sourcehut.settings = mkMerge [
{
"${srv}.sr.ht".origin = mkDefault "https://${srv}.${cfg.settings."sr.ht".global-domain}";
Expand Down Expand Up @@ -378,10 +381,11 @@ in
extraService
])) extraServices)

# Work around 'pq: permission denied for schema public' with postgres v15, until a
# solution for `services.postgresql.ensureUsers` is found.
# Work around 'pq: permission denied for schema public' with postgres v15.
# See https://github.com/NixOS/nixpkgs/issues/216989
# Workaround taken from nixos/forgejo: https://github.com/NixOS/nixpkgs/pull/262741
# TODO(to maintainers of sourcehut): please migrate away from this workaround
# by migrating away from database name defaults with dots.
(lib.mkIf (
cfg.postgresql.enable
&& lib.strings.versionAtLeast config.services.postgresql.package.version "15.0"
Expand Down
4 changes: 2 additions & 2 deletions nixos/modules/services/monitoring/zabbix-proxy.nix
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ in
{ assertion = !config.services.zabbixServer.enable;
message = "Please choose one of services.zabbixServer or services.zabbixProxy.";
}
{ assertion = cfg.database.createLocally -> cfg.database.user == user;
{ assertion = cfg.database.createLocally -> cfg.database.user == user && cfg.database.name == cfg.database.user;
message = "services.zabbixProxy.database.user must be set to ${user} if services.zabbixProxy.database.createLocally is set true";
}
{ assertion = cfg.database.createLocally -> cfg.database.passwordFile == null;
Expand Down Expand Up @@ -252,7 +252,7 @@ in
ensureDatabases = [ cfg.database.name ];
ensureUsers = [
{ name = cfg.database.user;
ensurePermissions = { "DATABASE ${cfg.database.name}" = "ALL PRIVILEGES"; };
ensureDBOwnership = true;
}
];
};
Expand Down
4 changes: 2 additions & 2 deletions nixos/modules/services/monitoring/zabbix-server.nix
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ in
config = mkIf cfg.enable {

assertions = [
{ assertion = cfg.database.createLocally -> cfg.database.user == user;
{ assertion = cfg.database.createLocally -> cfg.database.user == user && cfg.database.user == cfg.database.name;
message = "services.zabbixServer.database.user must be set to ${user} if services.zabbixServer.database.createLocally is set true";
}
{ assertion = cfg.database.createLocally -> cfg.database.passwordFile == null;
Expand Down Expand Up @@ -240,7 +240,7 @@ in
ensureDatabases = [ cfg.database.name ];
ensureUsers = [
{ name = cfg.database.user;
ensurePermissions = { "DATABASE ${cfg.database.name}" = "ALL PRIVILEGES"; };
ensureDBOwnership = true;
}
];
};
Expand Down
Loading