diff --git a/AGENTS.md b/AGENTS.md index 5b0cd73..3ea7daf 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -68,7 +68,7 @@ Adding a feature: create `services/openldap-bootstrap/assets/scripts/NN-feature. `/usr/bin/openldap-ctl` → `services/openldap/assets/openldap-ctl/openldap-ctl.sh`, which dispatches to `command/.sh` (adding a file there adds a command). - `backup [--config] [--data] [--clean N] [--clean-prefix P]`: `slapcat | gzip` into `OPENLDAP_BACKUP_DIR`, written to a temp file then moved (atomic), umask 077. `--clean` only removes backup files (`*-config.gz`, `*-data.gz`) older than N days, starting with P if set; the cron uses `openldap-cron-` so manual and upgrade backups are kept. -- `restore [--config] [--data] [--force]`: first checks the backup files exist and are valid gzip (before stopping or deleting anything), then refuses if slapd runs or dirs are not empty unless `--force` (stops/restarts slapd, wipes dirs). +- `restore [--config] [--data] [--force]`: first checks the backup files exist and are valid gzip (before stopping or deleting anything), then refuses if slapd runs or dirs are not empty unless `--force` (stops/restarts slapd). Existing data is moved to `/.restore-previous` (same volume) and put back if the restore fails; a leftover `.restore-previous` (interrupted restore) blocks the next restore. - `password generate|hash`, `schema2ldif [deps]`. ## Coding conventions diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c1d385..4955aac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ Project-specific changes only. See upstream [OpenLDAP release notes](https://www - Fail bootstrap with an explicit error when `OPENLDAP_BOOTSTRAP_SUFFIX` does not start with `dc=`, instead of creating a root entry with an empty `dc`. - Fail bootstrap with an explicit error when `OPENLDAP_BOOTSTRAP_REPLICATION_HOSTS` contains more than 9 hosts. - Check that backup files exist and are valid gzip archives before `openldap-ctl restore` stops OpenLDAP or deletes existing data. +- Roll back to the previous config and data, and restart OpenLDAP if it was running, when `openldap-ctl restore` fails. Existing data is moved to a hidden `.restore-previous` directory in the same volume until the restore succeeds. - Print usage when `openldap-ctl schema2ldif` is called without arguments. - `openldap-ctl backup --clean` only removes backup files, and the new `--clean-prefix` option limits it to backups starting with a prefix. Cron backups are now named `openldap-cron--` and only clean these files, manual and upgrade backups are kept. Cron backups named with the previous format are no longer removed automatically. diff --git a/README.md b/README.md index 105dbc6..fd5db78 100644 --- a/README.md +++ b/README.md @@ -149,7 +149,7 @@ Restore a backup: docker exec openldap container run -- openldap-ctl restore my-backup --force ``` -`--force` stops the OpenLDAP process if needed, deletes existing data in the target directories, restores the selected databases, then starts OpenLDAP again. Backup files are checked first: if one is missing or corrupted, nothing is stopped or deleted. +`--force` stops the OpenLDAP process if needed, replaces existing data in the target directories with the selected databases, then starts OpenLDAP again. Backup files are checked first: if one is missing or corrupted, nothing is stopped or deleted. If the restore fails, the previous data is put back. The cron service is disabled by default. To schedule regular backups, run it in a separate container alongside OpenLDAP, sharing its configuration, data, and backup volumes: diff --git a/build/tests/upgrade.go b/build/tests/upgrade.go index d4172c5..3dcb04a 100644 --- a/build/tests/upgrade.go +++ b/build/tests/upgrade.go @@ -149,6 +149,29 @@ func BackupRestore(ctx context.Context, client *dagger.Client, image *dagger.Con return fmt.Errorf("data after the restore of a corrupted backup: %w", err) } + // a backup that fails to load is rolled back to the previous data + invalid := withVolumes(image, v). + WithExec([]string{"cp", backupDir + "/test-backup-config.gz", backupDir + "/invalid-config.gz"}). + WithExec([]string{"sh", "-c", "printf 'dn: not a valid entry\\nfoo\\n' | gzip > " + backupDir + "/invalid-data.gz"}) + + r, err = expect(ctx, invalid, false, []string{"Restore failed, rolling back to the previous data"}, "container", "run", "--", "openldap-ctl", "restore", "invalid", "--force") + if err != nil { + return fmt.Errorf("restore of an invalid backup: %w", err) + } + rolledBack := readVolumes(r.container) + if err := expectPeople(ctx, image, rolledBack); err != nil { + return fmt.Errorf("data after the rollback: %w", err) + } + if _, err := expect(ctx, withVolumes(image, rolledBack), false, nil, "ls", "-d", confDir+"/.restore-previous", dataDir+"/.restore-previous"); err != nil { + return fmt.Errorf("previous data directories after the rollback: %w", err) + } + + // an interrupted restore is detected + interrupted := withVolumes(image, v).WithExec([]string{"mkdir", dataDir + "/.restore-previous"}) + if _, err := expect(ctx, interrupted, false, []string{"a previous restore was interrupted"}, "container", "run", "--", "openldap-ctl", "restore", "test-backup", "--force"); err != nil { + return fmt.Errorf("restore after an interrupted restore: %w", err) + } + return nil } diff --git a/services/openldap/assets/openldap-ctl/command/restore.sh b/services/openldap/assets/openldap-ctl/command/restore.sh index a07bad6..9e5f63f 100755 --- a/services/openldap/assets/openldap-ctl/command/restore.sh +++ b/services/openldap/assets/openldap-ctl/command/restore.sh @@ -10,8 +10,14 @@ CONFIG=false DATA=false FORCE=false RESTART=false +RESTORED=false FILE_PREFIX="" +# existing config and data are moved to this hidden directory, in the same volume, until +# the restore succeeds +PREVIOUS_DIR_NAME=".restore-previous" +STASHED_DIRS=() + usage() { echo "Usage: openldap-ctl restore file-prefix [--config] [--data] [--force]" } @@ -43,6 +49,50 @@ restore_database() { fi } +# move the content of a directory to its previous directory +stash() { + local dir="$1" + local previous_dir="${dir}/${PREVIOUS_DIR_NAME}" + + container log info "Moving existing data in ${dir} to ${previous_dir} ..." + mkdir "${previous_dir}" + STASHED_DIRS+=("${dir}") + + find "${dir}" -mindepth 1 -maxdepth 1 ! -name "lost+found" ! -name ".*" \ + -exec mv -v {} "${previous_dir}/" \; 2>&1 | container log debug +} + +# replace the content of a directory by its previous directory content +unstash() { + local dir="$1" + local previous_dir="${dir}/${PREVIOUS_DIR_NAME}" + + container log warning "Restoring previous data in ${dir} ..." + + find "${dir}" -mindepth 1 -maxdepth 1 ! -name "lost+found" ! -name ".*" -exec rm -fr {} + + find "${previous_dir}" -mindepth 1 -maxdepth 1 -exec mv -v {} "${dir}/" \; 2>&1 | container log debug + rmdir "${previous_dir}" +} + +finish() { + local dir + + if [ "${RESTORED}" = true ]; then + for dir in "${STASHED_DIRS[@]}"; do + rm -fr "${dir:?}/${PREVIOUS_DIR_NAME}" + done + elif [ "${#STASHED_DIRS[@]}" -gt 0 ]; then + container log error "Restore failed, rolling back to the previous data ..." + for dir in "${STASHED_DIRS[@]}"; do + unstash "${dir}" + done + fi + + if [ "${RESTART}" = true ]; then + container processes start openldap + fi +} + while [ $# -gt 0 ]; do case "$1" in --config) @@ -94,6 +144,12 @@ if [ "${DATA}" = true ]; then check_backup_file "${DATA_BACKUP_FILE}" fi +for dir in "${OPENLDAP_CONF_DIR}" "${OPENLDAP_DATA_DIR}"; do + if [ -e "${dir}/${PREVIOUS_DIR_NAME}" ]; then + container log fatal "${dir}/${PREVIOUS_DIR_NAME} exists: a previous restore was interrupted. Check the data, then move its content back to ${dir} or delete it." + fi +done + if [ -e "/run/container/processes/openldap.pid" ]; then if [ "${FORCE}" != true ]; then @@ -115,28 +171,20 @@ if [ -n "$(find "${OPENLDAP_CONF_DIR}" -mindepth 1 -maxdepth 1 ! -name "lost+fou fi -if [ "${CONFIG}" = true ]; then +trap finish EXIT - if [ -n "$(find "${OPENLDAP_CONF_DIR}" -mindepth 1 -maxdepth 1 ! -name "lost+found" ! -name ".*")" ]; then - container log warning "Deleting data in ${OPENLDAP_CONF_DIR} ..." - rm -fr "${OPENLDAP_CONF_DIR:?}"/* - fi +if [ "${CONFIG}" = true ]; then + stash "${OPENLDAP_CONF_DIR}" container log info "Restoring OpenLDAP config ..." restore_database "${CONFIG_BACKUP_FILE}" 0 fi if [ "${DATA}" = true ]; then - - if [ -n "$(find "${OPENLDAP_DATA_DIR}" -mindepth 1 -maxdepth 1 ! -name "lost+found" ! -name ".*")" ]; then - container log warning "Deleting data in ${OPENLDAP_DATA_DIR} ..." - rm -fr "${OPENLDAP_DATA_DIR:?}"/* - fi + stash "${OPENLDAP_DATA_DIR}" container log info "Restoring OpenLDAP data ..." restore_database "${DATA_BACKUP_FILE}" 1 fi -if [ "${RESTART}" = true ]; then - container processes start openldap -fi +RESTORED=true