From ef9d8392f27b801b79b2b913cd338ddd0a612e59 Mon Sep 17 00:00:00 2001 From: "Parsons, Cliff (cp769u)" Date: Tue, 4 Feb 2020 23:38:16 +0000 Subject: [PATCH] Fix MariaDB Single Database Restore This patchset fixes a serious database restoration problem where the user is trying to restore a single database, but in the process of restoring the database, the script inadvertently also removes all tables from the other databases. The root cause was that the mysql "--one-database" restore option achieves the single database restoration, but somehow corrupts the other databases. The new approach taken in this patchset is to create a temporary database user which only has permission to restore the chosen database, and that will leave the other databases unharmed. This approach, which can be applied for restoring individual databases and even database tables, was recommended in (1). After the database is restored, the temporary user is deleted. (1) https://mariadb.com/kb/en/restoring-data-from-dump-files/ Also improved some of the error handling as well. Change-Id: I805c605ed2b424640ad6a0a379b1c0b9c0004e94 --- mariadb/templates/bin/_restore_mariadb.sh.tpl | 171 +++++++++++++----- 1 file changed, 123 insertions(+), 48 deletions(-) diff --git a/mariadb/templates/bin/_restore_mariadb.sh.tpl b/mariadb/templates/bin/_restore_mariadb.sh.tpl index 51b801c33..b316ea7af 100755 --- a/mariadb/templates/bin/_restore_mariadb.sh.tpl +++ b/mariadb/templates/bin/_restore_mariadb.sh.tpl @@ -14,18 +14,35 @@ # License for the specific language governing permissions and limitations # under the License. +log_error() { + echo $1 + exit 1 +} + ARCHIVE_DIR=${MARIADB_BACKUP_BASE_DIR}/db/${MARIADB_POD_NAMESPACE}/mariadb/archive RESTORE_DIR=${MARIADB_BACKUP_BASE_DIR}/db/${MARIADB_POD_NAMESPACE}/mariadb/restore ARGS=("$@") -LIST_OPTIONS=(list_archives list_databases) +RESTORE_USER='restoreuser' +RESTORE_PW=$(pwgen 16 1) +RESTORE_LOG='/tmp/restore_error.log' +rm -f $RESTORE_LOG #Create Restore Directory mkdir -p $RESTORE_DIR +# This is for commands which require admin access MYSQL="mysql \ - --defaults-file=/etc/mysql/admin_user.cnf \ - --host=$MARIADB_SERVER_SERVICE_HOST \ - --connect-timeout 10" + --defaults-file=/etc/mysql/admin_user.cnf \ + --host=$MARIADB_SERVER_SERVICE_HOST \ + --connect-timeout 10" + +# This is for commands which we want the temporary "restore" user +# to execute +RESTORE_CMD="mysql \ + --user=${RESTORE_USER} \ + --password=${RESTORE_PW} \ + --host=$MARIADB_SERVER_SERVICE_HOST \ + --connect-timeout 10" #Delete file delete_files() { @@ -50,10 +67,8 @@ list_archives() { do echo $archive | cut -d '/' -f 8 done - exit 0 else - echo "Archive directory is not available." - exit 1 + log_error "Archive directory is not available." fi } @@ -91,7 +106,45 @@ list_databases() { echo $db done fi +} +# Create temporary user for restoring specific databases. +create_restore_user() { + restore_db=$1 + + # Ensure any old restore user is removed first, if it exists. + # If it doesn't exist it may return error, so do not exit the + # script if that's the case. + delete_restore_user "dont_exit_on_error" + + $MYSQL --execute="GRANT SELECT ON *.* TO ${RESTORE_USER}@'%' IDENTIFIED BY '${RESTORE_PW}';" 2>>$RESTORE_LOG + if [ "$?" -eq 0 ] + then + $MYSQL --execute="GRANT ALL ON ${restore_db}.* TO ${RESTORE_USER}@'%' IDENTIFIED BY '${RESTORE_PW}';" 2>>$RESTORE_LOG + if [ "$?" -ne 0 ] + then + cat $RESTORE_LOG + log_error "Failed to grant restore user ALL permissions on database ${restore_db}" + fi + else + cat $RESTORE_LOG + log_error "Failed to grant restore user select permissions on all databases" + fi +} + +# Delete temporary restore user +delete_restore_user() { + error_handling=$1 + + $MYSQL --execute="DROP USER ${RESTORE_USER}@'%';" 2>>$RESTORE_LOG + if [ "$?" -ne 0 ] + then + if [ "$error_handling" == "exit_on_error" ] + then + cat $RESTORE_LOG + log_error "Failed to delete temporary restore user - needs attention to avoid a security hole" + fi + fi } #Restore a single database @@ -99,8 +152,7 @@ restore_single_db() { single_db_name=$1 if [ -z "$single_db_name" ] then - usage - exit 1 + log_error "Restore single DB called but with wrong parameter." fi if [ -f ${ARCHIVE_DIR}/${archive_file} ] then @@ -109,30 +161,42 @@ restore_single_db() { tar zxvf ${ARCHIVE_DIR}/${archive_file} -C ${RESTORE_DIR} 1>/dev/null if [ -f ${RESTORE_DIR}/mariadb.all.sql ] then - $MYSQL --one-database $single_db_name < ${RESTORE_DIR}/mariadb.all.sql + # Restoring a single database requires us to create a temporary user + # which has capability to only restore that ONE database. One gotcha + # is that the mysql command to restore the database is going to throw + # errors because of all the other databases that it cannot access. So + # because of this reason, the --force option is used to prevent the + # command from stopping on an error. + create_restore_user $single_db_name + $RESTORE_CMD --force < ${RESTORE_DIR}/mariadb.all.sql 2>>$RESTORE_LOG if [ "$?" -eq 0 ] then echo "Database $single_db_name Restore successful." else - echo "Database $single_db_name Restore failed." + cat $RESTORE_LOG + delete_restore_user "exit_on_error" + log_error "Database $single_db_name Restore failed." fi + delete_restore_user "exit_on_error" + if [ -f ${RESTORE_DIR}/${single_db_name}_grant.sql ] then - $MYSQL < ${RESTORE_DIR}/${single_db_name}_grant.sql + $MYSQL < ${RESTORE_DIR}/${single_db_name}_grant.sql 2>>$RESTORE_LOG if [ "$?" -eq 0 ] then echo "Database $single_db_name Permission Restore successful." else - echo "Database $single_db_name Permission Restore failed." + cat $RESTORE_LOG + log_error "Database $single_db_name Permission Restore failed." fi else - echo "There is no permission file available for $single_db_name" + log_error "There is no permission file available for $single_db_name" fi else - echo "There is no database file available to restore from" + log_error "There is no database file available to restore from" fi else - echo "Archive does not exist" + log_error "Archive does not exist" fi } @@ -145,12 +209,13 @@ restore_all_dbs() { tar zxvf ${ARCHIVE_DIR}/${archive_file} -C ${RESTORE_DIR} 1>/dev/null if [ -f ${RESTORE_DIR}/mariadb.all.sql ] then - $MYSQL < ${RESTORE_DIR}/mariadb.all.sql + $MYSQL < ${RESTORE_DIR}/mariadb.all.sql 2>$RESTORE_LOG if [ "$?" -eq 0 ] then echo "Databases $( echo $DBS | tr -d '\n') Restore successful." else - echo "Databases $( echo $DBS | tr -d '\n') Restore failed." + cat $RESTORE_LOG + log_error "Databases $( echo $DBS | tr -d '\n') Restore failed." fi if [ -n "$DBS" ] then @@ -158,34 +223,37 @@ restore_all_dbs() { do if [ -f ${RESTORE_DIR}/${db}_grant.sql ] then - $MYSQL < ${RESTORE_DIR}/${db}_grant.sql + $MYSQL < ${RESTORE_DIR}/${db}_grant.sql 2>>$RESTORE_LOG if [ "$?" -eq 0 ] then echo "Database $db Permission Restore successful." else - echo "Database $db Permission Restore failed." + cat $RESTORE_LOG + log_error "Database $db Permission Restore failed." fi else - echo "There is no permission file available for $db" + log_error "There is no permission file available for $db" fi done else - echo "There is no database file available to restore from" + log_error "There is no database file available to restore from" fi else - echo "Archive does not exist" + log_error "Archive does not exist" fi fi } usage() { + ret_val=$1 echo "Usage:" - echo "$0 options" + echo "Restore command options" echo "=============================" - echo "options: " + echo "help" echo "list_archives" - echo "list_databases archive_filename" - echo "restore archive_filename [DB_NAME or ALL/all]" + echo "list_databases " + echo "restore [ | ALL]" + exit $ret_val } is_Option() { @@ -205,57 +273,63 @@ is_Option() { #Main if [ ${#ARGS[@]} -gt 3 ] then - usage - exit + usage 1 elif [ ${#ARGS[@]} -eq 1 ] then - if [ $(is_Option "$LIST_OPTIONS" ${ARGS[0]}) -eq 1 ] + if [ "${ARGS[0]}" == "list_archives" ] then - ${ARGS[0]} - exit + list_archives + elif [ "${ARGS[0]}" == "help" ] + then + usage 0 else - usage - exit + usage 1 fi elif [ ${#ARGS[@]} -eq 2 ] then if [ "${ARGS[0]}" == "list_databases" ] then list_databases ${ARGS[1]} - exit 0 else - usage - exit + usage 1 fi elif [ ${#ARGS[@]} -eq 3 ] then if [ "${ARGS[0]}" != "restore" ] then - usage - exit 1 + usage 1 else if [ -f ${ARCHIVE_DIR}/${ARGS[1]} ] then #Get all the databases in that archive get_databases ${ARGS[1]} + #check if the requested database is available in the archive if [ $(is_Option "$DBS" ${ARGS[2]}) -eq 1 ] then - echo "Restoring Database ${ARGS[2]} And Grants" - echo "Creating Database ${ARGS[2]} if it does not exist" - $MYSQL -e "CREATE DATABASE IF NOT EXISTS \`${ARGS[2]}\`" + echo "Creating database ${ARGS[2]} if it does not exist" + $MYSQL -e "CREATE DATABASE IF NOT EXISTS \`${ARGS[2]}\`" 2>>$RESTORE_LOG + if [ "$?" -ne 0 ] + then + cat $RESTORE_LOG + log_error "Database ${ARGS[2]} could not be created." + fi + echo "Restoring database ${ARGS[2]} and grants...this could take a few minutes." restore_single_db ${ARGS[2]} - exit 0 elif [ "$( echo ${ARGS[2]} | tr '[a-z]' '[A-Z]')" == "ALL" ] then - echo "Restoring All The Database." - echo "Creating Database if it does not exist" + echo "Creating databases if they do not exist" for db in $DBS do $MYSQL -e "CREATE DATABASE IF NOT EXISTS \`$db\`" + if [ "$?" -ne 0 ] + then + cat $RESTORE_LOG + log_error "Database ${db} could not be created." + fi done + echo "Restoring all databases and grants...this could take a few minutes." restore_all_dbs - exit 0 else echo "Database ${ARGS[2]} does not exist." fi @@ -264,6 +338,7 @@ then fi fi else - usage - exit + usage 1 fi + +exit 0