From 50dda813e24db454b376866906e9281b7dafcb3a Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Fri, 17 May 2019 00:10:34 +0200 Subject: [PATCH 1/4] nixos/mysql: use systemd.tmpfiles to setup dataDir and pidDir We need to keep using `RuntimeDirectory=mysqld`, which translates to `/run/mysqld`, as this is used for the location of the file socket, that could differ with what is configured via `cfg.pidDir`. --- nixos/modules/services/databases/mysql.nix | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/nixos/modules/services/databases/mysql.nix b/nixos/modules/services/databases/mysql.nix index 7e3c230fff71..daa3b6c34dd4 100644 --- a/nixos/modules/services/databases/mysql.nix +++ b/nixos/modules/services/databases/mysql.nix @@ -296,6 +296,11 @@ in ${cfg.extraOptions} ''; + systemd.tmpfiles.rules = [ + "d '${cfg.dataDir}' 0700 ${cfg.user} mysql -" + "d '${cfg.pidDir}' 0755 ${cfg.user} mysql -" + ]; + systemd.services.mysql = let hasNotify = (cfg.package == pkgs.mariadb); in { @@ -316,19 +321,17 @@ in preStart = '' if ! test -e ${cfg.dataDir}/mysql; then - mkdir -m 0700 -p ${cfg.dataDir} - chown -R ${cfg.user} ${cfg.dataDir} ${mysql}/bin/mysql_install_db --defaults-file=/etc/my.cnf ${installOptions} touch /tmp/mysql_init fi - mkdir -m 0755 -p ${cfg.pidDir} - chown -R ${cfg.user} ${cfg.pidDir} ''; serviceConfig = { Type = if hasNotify then "notify" else "simple"; + # /run/mysqld needs to be created in addition to pidDir, as they could point to different locations RuntimeDirectory = "mysqld"; + RuntimeDirectoryMode = "0755"; # The last two environment variables are used for starting Galera clusters ExecStart = "${mysql}/bin/mysqld --defaults-file=/etc/my.cnf ${mysqldOptions} $_WSREP_NEW_CLUSTER $_WSREP_START_POSITION"; }; From 25494cc19388c63f620051fa7e60f6e5f0af4d5f Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Fri, 17 May 2019 00:12:20 +0200 Subject: [PATCH 2/4] nixos/mysql: reformat, move logical steps into variables define commands like "waiting for the mysql socket to appear" or "setup initial databases" in a let expression, so the main control flow becomes more readable. --- nixos/modules/services/databases/mysql.nix | 96 +++++++++++----------- 1 file changed, 48 insertions(+), 48 deletions(-) diff --git a/nixos/modules/services/databases/mysql.nix b/nixos/modules/services/databases/mysql.nix index daa3b6c34dd4..7b097e95e14b 100644 --- a/nixos/modules/services/databases/mysql.nix +++ b/nixos/modules/services/databases/mysql.nix @@ -318,14 +318,12 @@ in pkgs.nettools ]; - preStart = - '' - if ! test -e ${cfg.dataDir}/mysql; then - ${mysql}/bin/mysql_install_db --defaults-file=/etc/my.cnf ${installOptions} - touch /tmp/mysql_init - fi - - ''; + preStart = '' + if ! test -e ${cfg.dataDir}/mysql; then + ${mysql}/bin/mysql_install_db --defaults-file=/etc/my.cnf ${installOptions} + touch /tmp/mysql_init + fi + ''; serviceConfig = { Type = if hasNotify then "notify" else "simple"; @@ -336,50 +334,52 @@ in ExecStart = "${mysql}/bin/mysqld --defaults-file=/etc/my.cnf ${mysqldOptions} $_WSREP_NEW_CLUSTER $_WSREP_START_POSITION"; }; - postStart = '' - ${lib.optionalString (!hasNotify) '' - # Wait until the MySQL server is available for use - count=0 - while [ ! -e /run/mysqld/mysqld.sock ] - do - if [ $count -eq 30 ] - then - echo "Tried 30 times, giving up..." - exit 1 - fi + postStart = + let + cmdWatchForMysqlSocket = '' + # Wait until the MySQL server is available for use + count=0 + while [ ! -e /run/mysqld/mysqld.sock ] + do + if [ $count -eq 30 ] + then + echo "Tried 30 times, giving up..." + exit 1 + fi - echo "MySQL daemon not yet started. Waiting for 1 second..." - count=$((count++)) - sleep 1 - done - ''} + echo "MySQL daemon not yet started. Waiting for 1 second..." + count=$((count++)) + sleep 1 + done + ''; + cmdInitialDatabases = concatMapStrings (database: '' + # Create initial databases + if ! test -e "${cfg.dataDir}/${database.name}"; then + echo "Creating initial database: ${database.name}" + ( echo 'create database `${database.name}`;' + ${optionalString (database.schema != null) '' + echo 'use `${database.name}`;' + + # TODO: this silently falls through if database.schema does not exist, + # we should catch this somehow and exit, but can't do it here because we're in a subshell. + if [ -f "${database.schema}" ] + then + cat ${database.schema} + elif [ -d "${database.schema}" ] + then + cat ${database.schema}/mysql-databases/*.sql + fi + ''} + ) | ${mysql}/bin/mysql -u root -N + fi + '') cfg.initialDatabases; + in + + lib.optionalString (!hasNotify) cmdWatchForMysqlSocket + '' if [ -f /tmp/mysql_init ] then - ${concatMapStrings (database: - '' - # Create initial databases - if ! test -e "${cfg.dataDir}/${database.name}"; then - echo "Creating initial database: ${database.name}" - ( echo 'create database `${database.name}`;' - - ${optionalString (database.schema != null) '' - echo 'use `${database.name}`;' - - # TODO: this silently falls through if database.schema does not exist, - # we should catch this somehow and exit, but can't do it here because we're in a subshell. - if [ -f "${database.schema}" ] - then - cat ${database.schema} - elif [ -d "${database.schema}" ] - then - cat ${database.schema}/mysql-databases/*.sql - fi - ''} - ) | ${mysql}/bin/mysql -u root -N - fi - '') cfg.initialDatabases} - + ${cmdInitialDatabases} ${optionalString (cfg.replication.role == "master") '' # Set up the replication master From edd10c12f76145decf19f81e9c86ad5ad4a01c0e Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Fri, 17 May 2019 00:13:29 +0200 Subject: [PATCH 3/4] nixos/mysql: run as mysql user and group As we don't need to setup data directories from ExecStartPre= scripts anymore, which required root, but use systemd.tmpfiles.rules instead, everything can be run as just the mysql user. --- nixos/doc/manual/release-notes/rl-1909.xml | 11 +++++++++++ nixos/modules/services/databases/mysql.nix | 2 ++ 2 files changed, 13 insertions(+) diff --git a/nixos/doc/manual/release-notes/rl-1909.xml b/nixos/doc/manual/release-notes/rl-1909.xml index 60b4a3bc17b6..ef2b1d341893 100644 --- a/nixos/doc/manual/release-notes/rl-1909.xml +++ b/nixos/doc/manual/release-notes/rl-1909.xml @@ -161,6 +161,17 @@ The hunspellDicts.fr-any dictionary now ships with fr_FR.{aff,dic} which is linked to fr-toutesvariantes.{aff,dic}. + + + + The mysql service now runs as mysql + user. Previously, systemd did execute it as root, and mysql dropped privileges + itself. + This includes ExecStartPre= and + ExecStartPost= phases. + To accomplish that, runtime and data directory setup was delegated to + RuntimeDirectory and tmpfiles. + diff --git a/nixos/modules/services/databases/mysql.nix b/nixos/modules/services/databases/mysql.nix index 7b097e95e14b..97e58fd228f4 100644 --- a/nixos/modules/services/databases/mysql.nix +++ b/nixos/modules/services/databases/mysql.nix @@ -326,6 +326,8 @@ in ''; serviceConfig = { + User = cfg.user; + Group = "mysql"; Type = if hasNotify then "notify" else "simple"; # /run/mysqld needs to be created in addition to pidDir, as they could point to different locations RuntimeDirectory = "mysqld"; From 5ea7a3eb211d0c75f8f50bdfaeeba1ec72031b57 Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Fri, 17 May 2019 14:12:52 +0200 Subject: [PATCH 4/4] nixos/mysql: drop services.mysql.pidDir mysql already has its socket path hardcoded to to /run/mysqld/mysqld.sock. There's not much value in making the pidDir configurable, which also points to /run/mysqld by default. We only seem to use `services.mysql.pidDir` in the wordpress startup script, to wait for mysql to boot up, but we can also simply wait on the (hardcoded) socket location too. A much nicer way to accomplish that would be to properly describe a dependency on mysqld.service. This however is not easily doable, due to how the apache-httpd module was designed. --- nixos/doc/manual/release-notes/rl-1909.xml | 8 ++++++++ nixos/modules/rename.nix | 1 + nixos/modules/services/databases/mysql.nix | 15 ++------------- .../web-servers/apache-httpd/wordpress.nix | 2 +- 4 files changed, 12 insertions(+), 14 deletions(-) diff --git a/nixos/doc/manual/release-notes/rl-1909.xml b/nixos/doc/manual/release-notes/rl-1909.xml index ef2b1d341893..0b411005f1e0 100644 --- a/nixos/doc/manual/release-notes/rl-1909.xml +++ b/nixos/doc/manual/release-notes/rl-1909.xml @@ -93,6 +93,14 @@ the module for some time and so was removed as cleanup. + + + The option was removed, as it was only used by the wordpress + apache-httpd service to wait for mysql to have started up. + This can be accomplished by either describing a dependency on mysql.service (preferred) + or waiting for the (hardcoded) /run/mysqld/mysql.sock file to appear. + + The module has been removed, see diff --git a/nixos/modules/rename.nix b/nixos/modules/rename.nix index 7db0f7f050ac..a424e86038e7 100644 --- a/nixos/modules/rename.nix +++ b/nixos/modules/rename.nix @@ -212,6 +212,7 @@ with lib; (mkRemovedOptionModule [ "services" "logstash" "enableWeb" ] "The web interface was removed from logstash") (mkRemovedOptionModule [ "boot" "zfs" "enableLegacyCrypto" ] "The corresponding package was removed from nixpkgs.") (mkRemovedOptionModule [ "services" "winstone" ] "The corresponding package was removed from nixpkgs.") + (mkRemovedOptionModule [ "services" "mysql" "pidDir" ] "Don't wait for pidfiles, describe dependencies through systemd") # ZSH (mkRenamedOptionModule [ "programs" "zsh" "enableSyntaxHighlighting" ] [ "programs" "zsh" "syntaxHighlighting" "enable" ]) diff --git a/nixos/modules/services/databases/mysql.nix b/nixos/modules/services/databases/mysql.nix index 97e58fd228f4..66d55b650a45 100644 --- a/nixos/modules/services/databases/mysql.nix +++ b/nixos/modules/services/databases/mysql.nix @@ -18,16 +18,12 @@ let in (pName mysql == pName pkgs.mysql57) && ((builtins.compareVersions mysql.version "5.7") >= 0); - pidFile = "${cfg.pidDir}/mysqld.pid"; - - mysqldAndInstallOptions = - "--user=${cfg.user} --datadir=${cfg.dataDir} --basedir=${mysql}"; mysqldOptions = - "${mysqldAndInstallOptions} --pid-file=${pidFile}"; + "--user=${cfg.user} --datadir=${cfg.dataDir} --basedir=${mysql}"; # For MySQL 5.7+, --insecure creates the root user without password # (earlier versions and MariaDB do this by default). installOptions = - "${mysqldAndInstallOptions} ${lib.optionalString isMysqlAtLeast57 "--insecure"}"; + "${mysqldOptions} ${lib.optionalString isMysqlAtLeast57 "--insecure"}"; in @@ -80,11 +76,6 @@ in description = "Location where MySQL stores its table files"; }; - pidDir = mkOption { - default = "/run/mysqld"; - description = "Location of the file which stores the PID of the MySQL server"; - }; - extraOptions = mkOption { type = types.lines; default = ""; @@ -298,7 +289,6 @@ in systemd.tmpfiles.rules = [ "d '${cfg.dataDir}' 0700 ${cfg.user} mysql -" - "d '${cfg.pidDir}' 0755 ${cfg.user} mysql -" ]; systemd.services.mysql = let @@ -329,7 +319,6 @@ in User = cfg.user; Group = "mysql"; Type = if hasNotify then "notify" else "simple"; - # /run/mysqld needs to be created in addition to pidDir, as they could point to different locations RuntimeDirectory = "mysqld"; RuntimeDirectoryMode = "0755"; # The last two environment variables are used for starting Galera clusters diff --git a/nixos/modules/services/web-servers/apache-httpd/wordpress.nix b/nixos/modules/services/web-servers/apache-httpd/wordpress.nix index c68bfd25f6a8..3dddda138fed 100644 --- a/nixos/modules/services/web-servers/apache-httpd/wordpress.nix +++ b/nixos/modules/services/web-servers/apache-httpd/wordpress.nix @@ -273,7 +273,7 @@ in if [ ! -d ${serverInfo.fullConfig.services.mysql.dataDir}/${config.dbName} ]; then echo "Need to create the database '${config.dbName}' and grant permissions to user named '${config.dbUser}'." # Wait until MySQL is up - while [ ! -e ${serverInfo.fullConfig.services.mysql.pidDir}/mysqld.pid ]; do + while [ ! -S /run/mysqld/mysqld.sock ]; do sleep 1 done ${pkgs.mysql}/bin/mysql -e 'CREATE DATABASE ${config.dbName};'