Merge pull request #151255 from aanderse/nixos/mysql-cleanup

nixos/mysql: module cleanup

nixos/modules/services/databases/mysql.nix

@@ -11,10 +11,8 @@ let
   mysqldOptions =
     "--user=${cfg.user} --datadir=${cfg.dataDir} --basedir=${cfg.package}";
 
-  settingsFile = pkgs.writeText "my.cnf" (
+  format = pkgs.formats.ini { listsAsDuplicateKeys = true; };
-    generators.toINI { listsAsDuplicateKeys = true; } cfg.settings +
+  configFile = format.generate "my.cnf" cfg.settings;
-    optionalString (cfg.extraOptions != null) "[mysqld]\n${cfg.extraOptions}"
-  );
 
 in
 
@@ -22,6 +20,9 @@ in
   imports = [
     (mkRemovedOptionModule [ "services" "mysql" "pidDir" ] "Don't wait for pidfiles, describe dependencies through systemd.")
     (mkRemovedOptionModule [ "services" "mysql" "rootPassword" ] "Use socket authentication or set the password outside of the nix store.")
+    (mkRemovedOptionModule [ "services" "mysql" "extraOptions" ] "Use services.mysql.settings.mysqld instead.")
+    (mkRemovedOptionModule [ "services" "mysql" "bind" ] "Use services.mysql.settings.mysqld.bind-address instead.")
+    (mkRemovedOptionModule [ "services" "mysql" "port" ] "Use services.mysql.settings.mysqld.port instead.")
   ];
 
   ###### interface
@@ -40,41 +41,53 @@ in
         ";
       };
 
-      bind = mkOption {
-        type = types.nullOr types.str;
-        default = null;
-        example = "0.0.0.0";
-        description = "Address to bind to. The default is to bind to all addresses.";
-      };
-
-      port = mkOption {
-        type = types.port;
-        default = 3306;
-        description = "Port of MySQL.";
-      };
-
       user = mkOption {
         type = types.str;
         default = "mysql";
-        description = "User account under which MySQL runs.";
+        description = ''
+          User account under which MySQL runs.
+
+          <note><para>
+          If left as the default value this user will automatically be created
+          on system activation, otherwise you are responsible for
+          ensuring the user exists before the MySQL service starts.
+          </para></note>
+        '';
       };
 
       group = mkOption {
         type = types.str;
         default = "mysql";
-        description = "Group under which MySQL runs.";
+        description = ''
+          Group account under which MySQL runs.
+
+          <note><para>
+          If left as the default value this group will automatically be created
+          on system activation, otherwise you are responsible for
+          ensuring the user exists before the MySQL service starts.
+          </para></note>
+        '';
       };
 
       dataDir = mkOption {
         type = types.path;
         example = "/var/lib/mysql";
-        description = "Location where MySQL stores its table files.";
+        description = ''
+          The data directory for MySQL.
+
+          <note><para>
+          If left as the default value of <literal>/var/lib/mysql</literal> this directory will automatically be created before the MySQL
+          server starts, otherwise you are responsible for ensuring the directory exists with appropriate ownership and permissions.
+          </para></note>
+        '';
       };
 
       configFile = mkOption {
         type = types.path;
-        default = settingsFile;
+        default = configFile;
-        defaultText = literalExpression "settingsFile";
+        defaultText = ''
+          A configuration file automatically generated by NixOS.
+        '';
         description = ''
           Override the configuration file used by MySQL. By default,
           NixOS generates one automatically from <option>services.mysql.settings</option>.
@@ -92,7 +105,7 @@ in
       };
 
       settings = mkOption {
-        type = with types; attrsOf (attrsOf (oneOf [ bool int str (listOf str) ]));
+        type = format.type;
         default = {};
         description = ''
           MySQL configuration. Refer to
@@ -125,23 +138,6 @@ in
         '';
       };
 
-      extraOptions = mkOption {
-        type = with types; nullOr lines;
-        default = null;
-        example = ''
-          key_buffer_size = 6G
-          table_cache = 1600
-          log-error = /var/log/mysql_err.log
-        '';
-        description = ''
-          Provide extra options to the MySQL configuration file.
-
-          Please note, that these options are added to the
-          <literal>[mysqld]</literal> section so you don't need to explicitly
-          state it again.
-        '';
-      };
-
       initialDatabases = mkOption {
         type = types.listOf (types.submodule {
           options = {
@@ -287,7 +283,7 @@ in
         };
 
         masterPort = mkOption {
-          type = types.int;
+          type = types.port;
           default = 3306;
           description = "Port number on which the MySQL master server runs.";
         };
@@ -299,9 +295,7 @@ in
 
   ###### implementation
 
-  config = mkIf config.services.mysql.enable {
+  config = mkIf cfg.enable {
-
-    warnings = optional (cfg.extraOptions != null) "services.mysql.`extraOptions` is deprecated, please use services.mysql.`settings`.";
 
     services.mysql.dataDir =
       mkDefault (if versionAtLeast config.system.stateVersion "17.09" then "/var/lib/mysql"
@@ -310,8 +304,7 @@ in
     services.mysql.settings.mysqld = mkMerge [
       {
         datadir = cfg.dataDir;
-        bind-address = mkIf (cfg.bind != null) cfg.bind;
+        port = mkDefault 3306;
-        port = cfg.port;
       }
       (mkIf (cfg.replication.role == "master" || cfg.replication.role == "slave") {
         log-bin = "mysql-bin-${toString cfg.replication.serverId}";
@@ -341,156 +334,150 @@ in
 
     environment.etc."my.cnf".source = cfg.configFile;
 
-    systemd.tmpfiles.rules = [
+    systemd.services.mysql = {
-      "d '${cfg.dataDir}' 0700 '${cfg.user}' '${cfg.group}' - -"
+      description = "MySQL Server";
-      "z '${cfg.dataDir}' 0700 '${cfg.user}' '${cfg.group}' - -"
-    ];
 
-    systemd.services.mysql = let
+      after = [ "network.target" ];
-      hasNotify = isMariaDB;
+      wantedBy = [ "multi-user.target" ];
-    in {
+      restartTriggers = [ cfg.configFile ];
-        description = "MySQL Server";
 
-        after = [ "network.target" ];
+      unitConfig.RequiresMountsFor = cfg.dataDir;
-        wantedBy = [ "multi-user.target" ];
-        restartTriggers = [ cfg.configFile ];
 
-        unitConfig.RequiresMountsFor = "${cfg.dataDir}";
+      path = [
+        # Needed for the mysql_install_db command in the preStart script
+        # which calls the hostname command.
+        pkgs.nettools
+      ];
 
-        path = [
+      preStart = if isMariaDB then ''
-          # Needed for the mysql_install_db command in the preStart script
+        if ! test -e ${cfg.dataDir}/mysql; then
-          # which calls the hostname command.
+          ${cfg.package}/bin/mysql_install_db --defaults-file=/etc/my.cnf ${mysqldOptions}
-          pkgs.nettools
+          touch ${cfg.dataDir}/mysql_init
-        ];
+        fi
+      '' else ''
+        if ! test -e ${cfg.dataDir}/mysql; then
+          ${cfg.package}/bin/mysqld --defaults-file=/etc/my.cnf ${mysqldOptions} --initialize-insecure
+          touch ${cfg.dataDir}/mysql_init
+        fi
+      '';
 
-        preStart = if isMariaDB then ''
+      script = ''
-          if ! test -e ${cfg.dataDir}/mysql; then
+        # https://mariadb.com/kb/en/getting-started-with-mariadb-galera-cluster/#systemd-and-galera-recovery
-            ${cfg.package}/bin/mysql_install_db --defaults-file=/etc/my.cnf ${mysqldOptions}
+        if test -n "''${_WSREP_START_POSITION}"; then
-            touch ${cfg.dataDir}/mysql_init
+          if test -e "${cfg.package}/bin/galera_recovery"; then
+            VAR=$(cd ${cfg.package}/bin/..; ${cfg.package}/bin/galera_recovery); [[ $? -eq 0 ]] && export _WSREP_START_POSITION=$VAR || exit 1
           fi
-        '' else ''
+        fi
-          if ! test -e ${cfg.dataDir}/mysql; then
-            ${cfg.package}/bin/mysqld --defaults-file=/etc/my.cnf ${mysqldOptions} --initialize-insecure
-            touch ${cfg.dataDir}/mysql_init
-          fi
-        '';
 
-        script = ''
+        # The last two environment variables are used for starting Galera clusters
-          # https://mariadb.com/kb/en/getting-started-with-mariadb-galera-cluster/#systemd-and-galera-recovery
+        exec ${cfg.package}/bin/mysqld --defaults-file=/etc/my.cnf ${mysqldOptions} $_WSREP_NEW_CLUSTER $_WSREP_START_POSITION
-          if test -n "''${_WSREP_START_POSITION}"; then
+      '';
-            if test -e "${cfg.package}/bin/galera_recovery"; then
-              VAR=$(cd ${cfg.package}/bin/..; ${cfg.package}/bin/galera_recovery); [[ $? -eq 0 ]] && export _WSREP_START_POSITION=$VAR || exit 1
-            fi
-          fi
 
-          # The last two environment variables are used for starting Galera clusters
+      postStart = let
-          exec ${cfg.package}/bin/mysqld --defaults-file=/etc/my.cnf ${mysqldOptions} $_WSREP_NEW_CLUSTER $_WSREP_START_POSITION
+        # The super user account to use on *first* run of MySQL server
-        '';
+        superUser = if isMariaDB then cfg.user else "root";
+      in ''
+        ${optionalString (!isMariaDB) ''
+          # 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
+              echo "MySQL daemon not yet started. Waiting for 1 second..."
-          # The super user account to use on *first* run of MySQL server
+              count=$((count++))
-          superUser = if isMariaDB then cfg.user else "root";
+              sleep 1
-        in ''
+          done
-          ${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
 
-                echo "MySQL daemon not yet started. Waiting for 1 second..."
+        if [ -f ${cfg.dataDir}/mysql_init ]
-                count=$((count++))
+        then
-                sleep 1
+            # While MariaDB comes with a 'mysql' super user account since 10.4.x, MySQL does not
-            done
+            # Since we don't want to run this service as 'root' we need to ensure the account exists on first run
-          ''}
+            ( echo "CREATE USER IF NOT EXISTS '${cfg.user}'@'localhost' IDENTIFIED WITH ${if isMariaDB then "unix_socket" else "auth_socket"};"
+              echo "GRANT ALL PRIVILEGES ON *.* TO '${cfg.user}'@'localhost' WITH GRANT OPTION;"
+            ) | ${cfg.package}/bin/mysql -u ${superUser} -N
 
-          if [ -f ${cfg.dataDir}/mysql_init ]
-          then
-              # While MariaDB comes with a 'mysql' super user account since 10.4.x, MySQL does not
-              # Since we don't want to run this service as 'root' we need to ensure the account exists on first run
-              ( echo "CREATE USER IF NOT EXISTS '${cfg.user}'@'localhost' IDENTIFIED WITH ${if isMariaDB then "unix_socket" else "auth_socket"};"
-                echo "GRANT ALL PRIVILEGES ON *.* TO '${cfg.user}'@'localhost' WITH GRANT OPTION;"
-              ) | ${cfg.package}/bin/mysql -u ${superUser} -N
-
-              ${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
-                      ''}
-                    ) | ${cfg.package}/bin/mysql -u ${superUser} -N
-                fi
-              '') cfg.initialDatabases}
-
-              ${optionalString (cfg.replication.role == "master")
-                ''
-                  # Set up the replication master
-
-                  ( echo "use mysql;"
-                    echo "CREATE USER '${cfg.replication.masterUser}'@'${cfg.replication.slaveHost}' IDENTIFIED WITH mysql_native_password;"
-                    echo "SET PASSWORD FOR '${cfg.replication.masterUser}'@'${cfg.replication.slaveHost}' = PASSWORD('${cfg.replication.masterPassword}');"
-                    echo "GRANT REPLICATION SLAVE ON *.* TO '${cfg.replication.masterUser}'@'${cfg.replication.slaveHost}';"
-                  ) | ${cfg.package}/bin/mysql -u ${superUser} -N
-                ''}
-
-              ${optionalString (cfg.replication.role == "slave")
-                ''
-                  # Set up the replication slave
-
-                  ( echo "stop slave;"
-                    echo "change master to master_host='${cfg.replication.masterHost}', master_user='${cfg.replication.masterUser}', master_password='${cfg.replication.masterPassword}';"
-                    echo "start slave;"
-                  ) | ${cfg.package}/bin/mysql -u ${superUser} -N
-                ''}
-
-              ${optionalString (cfg.initialScript != null)
-                ''
-                  # Execute initial script
-                  # using toString to avoid copying the file to nix store if given as path instead of string,
-                  # as it might contain credentials
-                  cat ${toString cfg.initialScript} | ${cfg.package}/bin/mysql -u ${superUser} -N
-                ''}
-
-              rm ${cfg.dataDir}/mysql_init
-          fi
-
-          ${optionalString (cfg.ensureDatabases != []) ''
-            (
             ${concatMapStrings (database: ''
-              echo "CREATE DATABASE IF NOT EXISTS \`${database}\`;"
+              # Create initial databases
-            '') cfg.ensureDatabases}
+              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
+                    ''}
+                  ) | ${cfg.package}/bin/mysql -u ${superUser} -N
+              fi
+            '') cfg.initialDatabases}
+
+            ${optionalString (cfg.replication.role == "master")
+              ''
+                # Set up the replication master
+
+                ( echo "use mysql;"
+                  echo "CREATE USER '${cfg.replication.masterUser}'@'${cfg.replication.slaveHost}' IDENTIFIED WITH mysql_native_password;"
+                  echo "SET PASSWORD FOR '${cfg.replication.masterUser}'@'${cfg.replication.slaveHost}' = PASSWORD('${cfg.replication.masterPassword}');"
+                  echo "GRANT REPLICATION SLAVE ON *.* TO '${cfg.replication.masterUser}'@'${cfg.replication.slaveHost}';"
+                ) | ${cfg.package}/bin/mysql -u ${superUser} -N
+              ''}
+
+            ${optionalString (cfg.replication.role == "slave")
+              ''
+                # Set up the replication slave
+
+                ( echo "stop slave;"
+                  echo "change master to master_host='${cfg.replication.masterHost}', master_user='${cfg.replication.masterUser}', master_password='${cfg.replication.masterPassword}';"
+                  echo "start slave;"
+                ) | ${cfg.package}/bin/mysql -u ${superUser} -N
+              ''}
+
+            ${optionalString (cfg.initialScript != null)
+              ''
+                # Execute initial script
+                # using toString to avoid copying the file to nix store if given as path instead of string,
+                # as it might contain credentials
+                cat ${toString cfg.initialScript} | ${cfg.package}/bin/mysql -u ${superUser} -N
+              ''}
+
+            rm ${cfg.dataDir}/mysql_init
+        fi
+
+        ${optionalString (cfg.ensureDatabases != []) ''
+          (
+          ${concatMapStrings (database: ''
+            echo "CREATE DATABASE IF NOT EXISTS \`${database}\`;"
+          '') cfg.ensureDatabases}
+          ) | ${cfg.package}/bin/mysql -N
+        ''}
+
+        ${concatMapStrings (user:
+          ''
+            ( echo "CREATE USER IF NOT EXISTS '${user.name}'@'localhost' IDENTIFIED WITH ${if isMariaDB then "unix_socket" else "auth_socket"};"
+              ${concatStringsSep "\n" (mapAttrsToList (database: permission: ''
+                echo "GRANT ${permission} ON ${database} TO '${user.name}'@'localhost';"
+              '') user.ensurePermissions)}
             ) | ${cfg.package}/bin/mysql -N
-          ''}
+          '') cfg.ensureUsers}
+      '';
 
-          ${concatMapStrings (user:
+      serviceConfig = mkMerge [
-            ''
+        {
-              ( echo "CREATE USER IF NOT EXISTS '${user.name}'@'localhost' IDENTIFIED WITH ${if isMariaDB then "unix_socket" else "auth_socket"};"
+          Type = if isMariaDB then "notify" else "simple";
-                ${concatStringsSep "\n" (mapAttrsToList (database: permission: ''
-                  echo "GRANT ${permission} ON ${database} TO '${user.name}'@'localhost';"
-                '') user.ensurePermissions)}
-              ) | ${cfg.package}/bin/mysql -N
-            '') cfg.ensureUsers}
-        '';
-
-        serviceConfig = {
-          Type = if hasNotify then "notify" else "simple";
           Restart = "on-abort";
           RestartSec = "5s";
 
@@ -523,9 +510,12 @@ in
           PrivateMounts = true;
           # System Call Filtering
           SystemCallArchitectures = "native";
-        };
+        }
-      };
+        (mkIf (cfg.dataDir == "/var/lib/mysql") {
-
+          StateDirectory = "mysql";
+          StateDirectoryMode = "0700";
+        })
+      ];
+    };
   };
-
 }