fix(backup): reapply DB triggers on import and carry them in the exportable backup by JeGoi · Pull Request #9136 · inverse-inc/packetfence

JeGoi · 2026-06-30T15:58:11Z

Description

(REQUIRED)

Restoring a PacketFence DB silently dropped all triggers, so the restored
instance stopped cleaning up related rows — most visibly, deleting a user no
longer removed its password row (re-creating the user then failed with 409 Conflict), and the ip4log/ip6log/locationlog history triggers were lost.

Root cause: mysqldump runs as the pf user, which lacks the TRIGGER
privilege, so the dump has no triggers. On import the DB is dropped, recreated
and reloaded from that trigger-less dump, and the triggers were never reapplied.

Fix:

Reapply triggers on import. import_mysqldump reapplies triggers when the
dump has none — from triggers.sql in the export, falling back to the matching
pf-schema-<version>.sql. Idempotent, and runs before upgrade_database.
Self-contained export. export.sh (already root) also dumps triggers.sql
(triggers only). Routines already ride in the base dump; there are no DB events.
Replace --db-restore with --restore-as-is. A composable non-interactive
modifier (with --db/--conf/full) that skips upgrades and keeps the backup's
values.

Impacts

(REQUIRED)

export.sh emits triggers.sql alongside grants.sql.
database.functions (import_mysqldump) reapplies triggers when the dump lacks
them.
import.sh drops --db-restore, adds --restore-as-is.

Backport

(REQUIRED)
Bug was masked by cross-version upgrades but real for same-version restore, a
standard workflow since exportable-backup (14.1). Affected: 14.1 → 15.1.
Backport to at least maintenance/15.1.

Delete branch after merge

(REQUIRED)
YES

Checklist

(REQUIRED) - [yes, no or n/a]

Document the feature — n/a (documented in the script's --help)
Add unit tests — n/a (shell backup/import path)
Add acceptance tests — venom backup_db_and_restore checks the export
carries triggers and that triggers/routines survive the restore, and covers the
--restore-as-is db/conf/full restores, each restarting PacketFence
(pfcmd service pf start) so the API is reachable after.

NEWS file entries

(REQUIRED, but may be optional...)

Bug Fixes

Database import/restore no longer drops triggers, fixing orphaned password
rows after a user is deleted on restored databases

Enhancements

The exportable backup now includes the database triggers so a restore is
self-contained; import.sh gains --restore-as-is for non-interactive imports

UPGRADE file entries

(OPTIONAL, but may be required...)
Databases restored with older code may be missing triggers. Re-import with the
fixed code, or reapply from /usr/local/pf/db/pf-schema-<version>.sql as root.

Copilot

Pull request overview

This PR fixes a database restore/import failure mode where restores performed from a pf-user mysqldump lose all DB triggers (because the dump contains none), leading to broken cleanup behavior (e.g., orphaned password rows after deleting a user). The fix re-applies trigger definitions from the version-matching PacketFence schema after loading a trigger-less dump, and adds a Venom acceptance test suite to ensure triggers survive restore.

Changes:

Update import_mysqldump to detect trigger-less dumps and reapply triggers (only) from pf-schema-<major>.<minor>.sql.
Add Venom tests validating trigger presence and verifying that deleting a person removes the corresponding password row.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
`addons/functions/database.functions`	Reapplies trigger definitions from the schema after importing a trigger-less mysqldump.
`t/venom/test_suites/backup_db_and_restore/15_backup_db_and_restore.yml`	Adds acceptance checks ensuring triggers exist post-restore and that the `password` cleanup trigger works.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

A pf-user mysqldump omits triggers (the pf DB user lacks the TRIGGER privilege), so a restored/imported database loses password_delete_trigger and the ip4log/ip6log/locationlog history triggers. Reapply only the triggers from the matching schema after loading the dump, leaving restored rows untouched.

The base pf-user dump already carries routines (--opt --routines) and PacketFence defines no DB events, so re-dumping them only produced duplicate/no-op objects on restore.

…as-is pf-user dumps omit triggers, so reapply them (from triggers.sql or the matching schema) before any upgrade instead of only for --db-restore. Under --restore-as-is, skip mysql_upgrade and the credential prompt (socket auth).

--restore-as-is restores a same-version backup verbatim: no database/MariaDB/config upgrade, keeps the backup's DB host/port, and asks nothing. Also renames --skip-adjust-conf to --skip-adjust-db-conf since it only adjusts the DB host/port.

Verify config restoration with a marker file and that triggers/routines survive; all restores now use --restore-as-is.

import.sh isolates to packetfence-base and leaves the restart to the operator; the suites now do it via pfcmd service pf start before hitting the webadmin API.

Copilot

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

cluster.conf is restored verbatim since it joined stored_config_files, leaving the node with the source cluster's interfaces/IPs. Rewrite the local node's sections from the corrected pf.conf (and propagate interface renames to the CLUSTER VIP sections); skipped under --restore-as-is.

log.conf and several non-.conf files are restored but were undocumented or wrongly listed as excluded; also document cluster.conf adjustment and --restore-as-is verbatim behavior.

…r-conf Default import no longer adopts the export's cluster config: the server is left standalone with an empty cluster.conf and the export's copy is saved as cluster.conf.imported. --update-cluster-conf opts into restoring and adjusting it; --restore-as-is keeps the current cluster.conf untouched.

Copilot

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

JeGoi · 2026-07-07T08:49:56Z

There is an issue with the ignored table and a restore-as-is where these tables are missing

…st on restore The nightly dump omits large history tables via --ignore-table, leaving trigger targets (locationlog_history) missing after a restore. Export their structure and recreate them empty on import. Only tables that exist are dumped, so obsolete names (iplog_archive) never break the export.

Backups made before excluded_tables_schema.sql existed don't carry it, so fall back to the matching pf-schema (with the devel X.Y.sql fallback) to recreate the excluded tables empty on restore.

…r restore

Copilot

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

JeGoi added this to the +1 (patch release) milestone Jun 30, 2026

JeGoi added the Priority: High label Jun 30, 2026

JeGoi force-pushed the fix/restore-reapply-schema-triggers branch 2 times, most recently from 536bcdb to f586ff3 Compare July 2, 2026 06:43

satkunas requested a review from Copilot July 2, 2026 14:10

Copilot started reviewing on behalf of satkunas July 2, 2026 14:10 View session

Copilot AI reviewed Jul 2, 2026

View reviewed changes

Comment thread addons/functions/database.functions Outdated

JeGoi added 8 commits July 6, 2026 09:18

test(venom): verify DB triggers survive restore in backup_db_and_restore

a7163f3

fix(backup): export only triggers, not routines or events

650a45e

The base pf-user dump already carries routines (--opt --routines) and PacketFence defines no DB events, so re-dumping them only produced duplicate/no-op objects on restore.

docs(import): document import.sh options

a60075c

test(venom): cover --restore-as-is db/conf/full restores

e5c183b

Verify config restoration with a marker file and that triggers/routines survive; all restores now use --restore-as-is.

test(venom): restart PacketFence after import so API is reachable

be45534

import.sh isolates to packetfence-base and leaves the restart to the operator; the suites now do it via pfcmd service pf start before hitting the webadmin API.

JeGoi force-pushed the fix/restore-reapply-schema-triggers branch from 041129b to be45534 Compare July 6, 2026 13:19

JeGoi changed the title ~~fix(import): reapply DB schema on restore to recreate missing triggers~~ fix(backup): reapply DB triggers on import and carry them in the exportable backup Jul 6, 2026

JeGoi requested a review from Copilot July 6, 2026 15:19

Copilot started reviewing on behalf of JeGoi July 6, 2026 15:19 View session

Copilot AI reviewed Jul 6, 2026

View reviewed changes

Comment thread addons/functions/database.functions Outdated

Comment thread addons/functions/database.functions

JeGoi added 4 commits July 6, 2026 12:48

docs(import): correct restored-files list and cluster.conf handling

f244116

log.conf and several non-.conf files are restored but were undocumented or wrongly listed as excluded; also document cluster.conf adjustment and --restore-as-is verbatim behavior.

docs(import): document cluster.conf default and --update-cluster-conf

d68a7af

JeGoi marked this pull request as ready for review July 6, 2026 17:52

JeGoi requested a review from Copilot July 6, 2026 17:52

Copilot started reviewing on behalf of JeGoi July 6, 2026 17:52 View session

Copilot AI reviewed Jul 6, 2026

View reviewed changes

Comment thread addons/full-import/import.sh Outdated

JeGoi and others added 2 commits July 7, 2026 04:12

Potential fix for pull request finding

fc4aa36

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Potential fix for pull request finding

b9f676a

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Potential fix for pull request finding

5abaca1

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

JeGoi marked this pull request as draft July 7, 2026 08:48

JeGoi added 3 commits July 7, 2026 05:40

fix(import): recreate excluded tables from schema for old backups

beb58a3

Backups made before excluded_tables_schema.sql existed don't carry it, so fall back to the matching pf-schema (with the devel X.Y.sql fallback) to recreate the excluded tables empty on restore.

test(venom): verify all schema tables and excluded table present afte…

e3ce7fb

…r restore

JeGoi marked this pull request as ready for review July 7, 2026 11:48

JeGoi requested a review from Copilot July 7, 2026 11:48

Copilot started reviewing on behalf of JeGoi July 7, 2026 11:49 View session

Copilot AI reviewed Jul 7, 2026

View reviewed changes

Comment thread t/venom/test_suites/backup_db_and_restore/15_backup_db_and_restore.yml

Comment thread t/venom/test_suites/backup_db_and_restore/15_backup_db_and_restore.yml

satkunas and others added 2 commits July 7, 2026 09:17

Potential fix for pull request finding

6dbdda7

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Potential fix for pull request finding

0167d0a

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

satkunas merged commit 2a67635 into devel Jul 7, 2026
10 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(backup): reapply DB triggers on import and carry them in the exportable backup#9136

fix(backup): reapply DB triggers on import and carry them in the exportable backup#9136
satkunas merged 20 commits into
develfrom
fix/restore-reapply-schema-triggers

JeGoi commented Jun 30, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

JeGoi commented Jul 7, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

JeGoi commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Impacts

Backport

Delete branch after merge

Checklist

NEWS file entries

Bug Fixes

Enhancements

UPGRADE file entries

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

JeGoi commented Jul 7, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

JeGoi commented Jun 30, 2026 •

edited

Loading