Skip to content

System: Configuration: Backups migrate to MVC#10098

Open
sopex wants to merge 14 commits intoopnsense:masterfrom
sopex:pr-diag_backup
Open

System: Configuration: Backups migrate to MVC#10098
sopex wants to merge 14 commits intoopnsense:masterfrom
sopex:pr-diag_backup

Conversation

@sopex
Copy link
Copy Markdown
Member

@sopex sopex commented Apr 7, 2026

Closes #10084
Closes #9529
Closes #8422
Closes #9516

Let the fun begin :)

UI photos:

Click to see all photos
Screenshot 2026-04-07 132543 Screenshot 2026-04-07 115439 Screenshot 2026-04-04 120717 Screenshot 2026-04-07 132558

sopex added 2 commits April 7, 2026 13:28
* bulk

* ..

* ..

* Update BackupController.php

* Update BackupController.php

* Update BackupController.php

* Update backup.xml

* Update BackupController.php

* reorder tabs

* .

* Update backup.volt

* .

* Update BackupController.php

* Update BackupController.php

* Update BackupController.php

* Update BackupController.php

* Update BackupController.php

* tabs

* .

* Update backup.volt

* Update backup.volt

* Update backup.volt

* Update backup.volt

* Update backup.volt

* .

* Update backup.volt

* Update backup.volt

* .

* test

* Revert "test"

This reverts commit 710d2e7.

* test2

* Update backup.volt

* Update BackupController.php

* Update BackupController.php

* Update BackupController.php

* Update BackupController.php

* ..

* Update BackupController.php

* ll

* Update BackupController.php

* Update BackupController.php

* Update BackupController.php

* l

* v

* Update BackupController.php

* Update BackupController.php

* Update backup.volt

* Update backup.volt

* Revert "Update backup.volt"

This reverts commit e4df8dd.

* Revert "Update backup.volt"

This reverts commit 368aec2.

* Revert "Update BackupController.php"

This reverts commit 0045cba.

* Revert "Update BackupController.php"

This reverts commit 57444ab.

* Revert "v"

This reverts commit 98f95ef.

* Update BackupController.php

* Update core.inc

* Update BackupController.php

* Update BackupController.php

* Update backup.volt

* Update BackupController.php

* Update BackupController.php

* Update BackupController.php

* cron + plugin message

* Update backup.volt

* Update backup.volt

* Update BackupController.php

* Update BackupController.php

* .

* Revert "."

This reverts commit 1aebe2b.

* Revert "Update BackupController.php"

This reverts commit e80d9dd.

* Update core.inc

* Update BackupController.php

* Update BackupController.php

* Update BackupController.php

* .

* 2 checks working

* val1

* Update backup.volt

* Update backup.volt

* Update backup.volt

* mm

* Revert "mm"

This reverts commit 7f9326b.

* Revert "Update backup.volt"

This reverts commit 56ddeaa.

* Revert "Update backup.volt"

This reverts commit 865c663.

* Revert "Update backup.volt"

This reverts commit 7daccee.

* Revert "val1"

This reverts commit 5b93a5e.

* minor still working

* Update backup.volt

* .

* Update Backup.xml

* 8

* Update backup.volt

* Update backup.volt

* Update backup.volt

* Update backup.volt

* Update backup.volt

* Update backup.volt

* amusing

* 0

* 00

* Update backup.volt

* 000

* Update backup.volt
Comment thread src/opnsense/mvc/app/controllers/OPNsense/Core/BackupController.php Outdated
Comment thread src/opnsense/mvc/app/controllers/OPNsense/Core/BackupController.php Outdated
Comment thread src/opnsense/mvc/app/controllers/OPNsense/Core/BackupController.php Outdated
Comment thread src/opnsense/mvc/app/controllers/OPNsense/Core/BackupController.php
Comment on lines +56 to +65
$areas = [
'bridges' => gettext('Bridge Devices'),
'gifs' => gettext('GIF Devices'),
'interfaces' => gettext('Interfaces'),
'laggs' => gettext('LAGG Devices'),
'ppps' => gettext('Point-to-Point Devices'),
'rrddata' => gettext('RRD Data'),
'vlans' => gettext('VLAN Devices'),
'wireless' => gettext('Wireless Devices'),
];
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have to discuss where we want to anchor this

Comment thread src/opnsense/mvc/app/models/OPNsense/Core/ACL/ACL.xml Outdated
Comment thread src/etc/inc/plugins.inc.d/core.inc Outdated
Comment thread src/etc/inc/plugins.inc.d/core.inc Outdated
Comment thread src/opnsense/mvc/app/controllers/OPNsense/Core/Api/BackupController.php Outdated
Comment thread src/opnsense/mvc/app/controllers/OPNsense/Core/Api/BackupController.php Outdated
@sopex sopex marked this pull request as draft April 7, 2026 14:13
@sopex
Copy link
Copy Markdown
Member Author

sopex commented Apr 8, 2026

@fichtner Somewhere between 5e781d8 and 7a6a581 cron remote backup broke. Any ideas?

I thought it was me after incorporating your feedback, but it's not.

2026-04-08T10:29:00 Error configd.py action system remote backup not found for user nobody
2026-04-08T10:26:00 Informational configd.py action allowed system.remote.backup for user nobody

I am also tagging you because of 26.1.6

@fichtner
Copy link
Copy Markdown
Member

fichtner commented Apr 8, 2026

On 26.1.5 or master? On master this could potentially interfere c491376fdd3

@sopex
Copy link
Copy Markdown
Member Author

sopex commented Apr 8, 2026

On 26.1.5 or master? On master this could potentially interfere c491376fdd3

Yes, that's 8 hours I am never getting back :(

@fichtner
Copy link
Copy Markdown
Member

fichtner commented Apr 8, 2026

I know how you feel. If you think reverting the commit fixes your issue can you follow up here so we can reopen? #10075

I wasn't going to release this soon anyway.

Thanks,
Franco

@sopex
Copy link
Copy Markdown
Member Author

sopex commented Apr 8, 2026

I know how you feel. If you think reverting the commit fixes your issue can you follow up here so we can reopen? #10075

I wasn't going to release this soon anyway.

Thanks, Franco

Yes, that was it. I am blind.

Please do that.

Thanks :)

sopex added 5 commits April 8, 2026 17:35
* remove legacy requires

* ..

* Update actions_system.conf

* kk

* Update BackupController.php

* Update actions_system.conf

* aa

* Update backup_restore.php
}
$name = "config-" . $hostname . "-" . date("YmdHis") . ".xml";
$tmpfile = tempnam(sys_get_temp_dir(), 'opn_bck_');
$rrd_arg = empty($this->request->getPost('donotbackuprrd')) ? "rrd" : "norrd";
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't know this but we were talking about deprecating this part of the backup functionality for a while now to ease the MVC migration while at it.

Copy link
Copy Markdown
Member Author

@sopex sopex Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like rrd :(
Anyway, I like your top secret information. Let me know if there is anything else.
I would also appreciate it if you tried to break the new code :)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just the backups into the config.xml really

unfortunately schedules are quite tight at least until after next week

Copy link
Copy Markdown
Member Author

@sopex sopex Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just the backups into the config.xml really

Yes, I understood that but still :)

unfortunately schedules are quite tight at least until after next week

Of course, take your time. In any case, this is in working condition and probably fine by MVC standards. Surely there are improvements to be made, but I will make a small commit today and let it sit fot a bit. Let's see if there is some kind of major feedback that requires rebuilding it from scratch, and if not, I will focus on the minor stuff.

@sopex sopex marked this pull request as ready for review April 9, 2026 16:28
@sopex
Copy link
Copy Markdown
Member Author

sopex commented May 5, 2026

@fichtner Make copilot review this too :)

@fichtner
Copy link
Copy Markdown
Member

fichtner commented May 5, 2026

careful what you wish for ;)

@sopex
Copy link
Copy Markdown
Member Author

sopex commented May 5, 2026

Always fun wasting Azure compute :) I can live with the consequences...

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR migrates the “System: Configuration: Backups” page from the legacy PHP implementation (diag_backup.php) to an MVC-based UI with corresponding API endpoints and backend configd actions/scripts, while also introducing model-backed settings for backup count and remote backup push time.

Changes:

  • Replace the legacy backup/restore page with a new MVC UI (/ui/core/backup) plus new API endpoints for settings, download, restore, and provider setup.
  • Add configd actions and new backend scripts for exporting/downloading and restoring configurations.
  • Introduce a new OPNsense\Core\Backup model (including migration) and wire remote-backup scheduling to a configurable push time.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
src/www/diag_backup.php Removes legacy backup/restore implementation (replaced by MVC).
src/opnsense/service/conf/actions.d/actions_system.conf Adds configd actions for xmlrpc options, config export, and config restore.
src/opnsense/scripts/system/backup_restore.php New configd-invoked restore implementation for full and area restores.
src/opnsense/scripts/system/backup_download.php New configd-invoked exporter for config.xml (optionally with RRD).
src/opnsense/mvc/app/views/OPNsense/Core/backup.volt New MVC UI for local backup, remote backup, and restore.
src/opnsense/mvc/app/models/OPNsense/Core/Migrations/M1_0_2.php Migrates legacy system.backupcount into the new model container.
src/opnsense/mvc/app/models/OPNsense/Core/Menu/Menu.xml Updates menu entry to point to /ui/core/backup.
src/opnsense/mvc/app/models/OPNsense/Core/Backup.xml Defines model-backed backup settings (backupcount, pushtime).
src/opnsense/mvc/app/models/OPNsense/Core/Backup.php Adds the MVC model class for backup settings.
src/opnsense/mvc/app/models/OPNsense/Core/ACL/ACL.xml Updates ACL patterns to cover new UI and API endpoints.
src/opnsense/mvc/app/library/OPNsense/Core/Config.php Adds backup-size helper; extends backupCount lookup to new model location.
src/opnsense/mvc/app/controllers/OPNsense/Core/forms/backup_remote.xml Adds remote backup “Push Time” form definition.
src/opnsense/mvc/app/controllers/OPNsense/Core/forms/backup_local.xml Adds local “Backup Count” form definition.
src/opnsense/mvc/app/controllers/OPNsense/Core/BackupController.php Adds MVC page controller to render the new backup UI with providers/areas.
src/opnsense/mvc/app/controllers/OPNsense/Core/Api/BackupController.php Adds API endpoints for settings, config download/export, restore, provider setup.
src/etc/inc/plugins.inc.d/core.inc Updates remote backup cron scheduling to use configured push time.
plist Installs new MVC files and scripts; removes legacy diag_backup.php.
Comments suppressed due to low confidence (1)

src/opnsense/mvc/app/library/OPNsense/Core/Config.php:720

  • backupCount() no longer accepts a legacy config value of 0 (previously allowed via >= 0). If a system has system->backupcount="0" (e.g. manual edit/older config) this now falls back to 100, which can unexpectedly increase disk usage. Either keep 0 as a valid value for the legacy key, or explicitly migrate/normalize legacy 0 to the new model’s semantics.
            $this->statusIsValid && isset($this->simplexml->system->backupcount)
            && intval($this->simplexml->system->backupcount) >= 1
        ) {
            return intval($this->simplexml->system->backupcount);
        } else {
            return 100;
        }

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

Comment on lines +206 to +240
file_put_contents($restore_file, $data);
if ($cnf->restoreBackup($restore_file)) {
@unlink($restore_file);
$config = \parse_config();
$flush = false;
if (!empty($params['keepconsole'])) {
foreach ($csettings as $fieldname => $fieldcontent) {
if ($fieldcontent === null && isset($config[$fieldname])) {
unset($config[$fieldname]);
} else {
$config['system'][$fieldname] = $fieldcontent;
}
}
$flush = true;
}
if (!empty($config['rrddata'])) {
\rrd_import();
unset($config['rrddata']);
$flush = true;
}
if ($flush) {
\OPNsense\Core\Config::getInstance()->save('Restored full configuration');
}
if (!empty($params['flush_history'])) {
mwexecf('/usr/local/opnsense/scripts/system/flush_config_history');
\OPNsense\Core\Config::getInstance()->save('System restore flushed local history');
}
if (\is_interface_mismatch(false)) {
$do_reboot = false;
echo json_encode(['status' => 'success', 'message' => gettext("The interface configuration was restored but physical interfaces could not be matched. No automatic reboot was performed."), 'reboot' => false]);
exit(0);
}
echo json_encode(['status' => 'success', 'message' => gettext("The configuration has been restored."), 'reboot' => $do_reboot]);
} else {
echo json_encode(['status' => 'failed', 'message' => gettext("The configuration could not be restored.")]);
Comment on lines +184 to +186
if ($do_reboot) {
}
echo json_encode(['status' => 'success', 'message' => gettext("The configuration area has been restored."), 'reboot' => $do_reboot]);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if there is a mismatch, shouldn't there be a message for that?


$restoreareas = !empty($params['restorearea']) ? $params['restorearea'] : [];
$do_reboot = !empty($params['rebootafterrestore']);

Comment on lines +213 to +214
if ($fieldcontent === null && isset($config[$fieldname])) {
unset($config[$fieldname]);
@unlink($tmpfile);

if (!empty($this->request->getPost('encrypt'))) {
$password = $this->request->getPost('encrypt_password');
Comment thread src/etc/inc/plugins.inc.d/core.inc Outdated
Comment thread src/etc/inc/plugins.inc.d/core.inc
function show_value(key) {
$('#show-' + key + '-btn').html('');
$('#show-' + key + '-val').show();
$("[name='" + key + "']").focus();
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants